All of lore.kernel.org
 help / color / mirror / Atom feed
From: Theodore Ts'o <tytso@mit.edu>
To: Amir Goldstein <amir73il@gmail.com>
Cc: Eryu Guan <eguan@redhat.com>, Miklos Szeredi <miklos@szeredi.hu>,
	linux-unionfs@vger.kernel.org, fstests@vger.kernel.org
Subject: Re: [PATCH v3 9/9] overlay: mount/unmount base fs before/after running tests
Date: Mon, 13 Feb 2017 19:23:56 -0500	[thread overview]
Message-ID: <20170214002356.dxrmf5jzwr6sivlu@thunk.org> (raw)
In-Reply-To: <1486932224-17075-10-git-send-email-amir73il@gmail.com>

On Sun, Feb 12, 2017 at 10:43:44PM +0200, Amir Goldstein wrote:
> When TEST/SCRATCH_DEV are configured to the base fs block device,
> use this information to mount base fs before running tests,
> unmount it after running tests and cycle on _test_cycle_mount
> along with the overlay mounts.

So I don't normally use the multi-section config file --- and I
confess the exact semantics of the config file, and how things are
inherited across sections has always been confusing to me (there are
bits of README.config-sections which seem to be mutually
contradictory[1]).  So apologies in advance, but when you have
something like this:

> diff --git a/README.config-sections b/README.config-sections
> index df7c929..d45d6da 100644
> --- a/README.config-sections
> +++ b/README.config-sections
> @@ -102,6 +102,9 @@ MKFS_OPTIONS="-q -F -b4096"
>  FSTYP=ext4
>  RESULT_BASE="`pwd`/results/`date +%d%m%y_%H%M%S`"
>  
> +[ext4_overlay]
> +FSTYP=overlay
> +
>  [ext4_1k_block_size]
>  MKFS_OPTIONS="-q -F -b1024"

How do you know that the base file system type should be ext4?  Is it
because the previous file system type in a previous config section
(OLD_FSTYP in the check script) is ext4?

If so, what happens if the first sectionis FSTYP=overlay.  Also, what
happens previous sections are skipped?  Suppose you have a config file
that looks like this:

[ext4]
...
FSTYP=ext4

[ext4_overlay]
FSTYP=overlay

[xfs]
...
FSTYP=xfs

[xfs_overlay]
FSTYP=overlay

What is supposed to happen (and what does happen) if the user runs:

./check -s xfs_overlay

or

./check -s ext4 -s xfs_overlay

?

Speaking more generally, I'm not a big fan of the config file approach
for handling iterations, because of the fact that previous sections
will have side effects on follow-on sections, and I'm interested in
adding support for test sharding, where different file system test
scenarios are run on different GCE VM's, and the ambiguities of how
variables are carried over from one section to another makes life hard.

It also makes it hard to have multiple file system developers editing
a single config file since you have to worry about side effects.
Having separate files and separate directories for differnt file
system types means that patch collisions are much less likely to have
unanticipated side effects, or cause merge conflicts for that matter.
I recognize that the local config file is not something that is
intended to be managed centrally, but I acutally *like* the fact that
I can separate file system test scenarios (and where I want to have a
common understanding across ext4 file system developers for what the
"bigalloc_1k" test scenario means), from the details of the local
hardware configuration.

All of this being said, I doubt I'll be able to convince others about
changing how the local config system works.  I do want to be sure I
understand what are the supported way of testing overlayfs (e.g., will
the "deprecated" way continue to work forever, or is it going to
disappear eventually), and while I'd prefer to not have to play games
with the config file if I want to test using overlayfs, if I *do* get
forced to do it, it would be useful if there were a bit more explicit
description of how things like the mkfs mount options, etc. are side
effected by previous config sections, and how to set up overlayfs
correctly using such a scheme.  (e.g., more documentation than just an
a few lines demonstration of what might go in the config file without
any detailed semantic explanation of how it all works.)

						- Ted

[1] The ambiguity I was taking about.  In one part of the
README.config-state file, it states:

   Note that options are carried between sections so the same options does not
   have to be specified in each and every sections. However caution should be
   exercised not to leave unwanted options set from previous sections.

(What does this mean when stanzas are skipped?)

and later on, it says this:

   Multiple file systems
   ---------------------

   Having different file systems in different config sections is allowed. When
   FSTYP differs in the following section the FSTYP file system will be created
   automatically before running the test.

   Note that if MOUNT_OPTIONS, MKFS_OPTIONS, or FSCK_OPTIONS are not directly
   specified in the section it will be reset to the default for a given file
   system.

This seems to imply that configuration paramters such as MKFS_OPTIONS
do *not* carry over from one config section to another, and is in
direct contradiction to the above paragraph.

  parent reply	other threads:[~2017-02-14  0:24 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-12 20:43 [PATCH v3 0/9] fstests: new way to run overlay tests Amir Goldstein
2017-02-12 20:43 ` [PATCH v3 1/9] fstests: sanity check that test partitions are not mounted elsewhere Amir Goldstein
2017-02-13 11:10   ` Eryu Guan
2017-02-13 11:44     ` Amir Goldstein
2017-02-13 13:33       ` Amir Goldstein
2017-02-14  5:51         ` Eryu Guan
2017-02-14  6:02           ` Amir Goldstein
2017-02-14  7:23             ` Eryu Guan
2017-02-14  8:05               ` Amir Goldstein
2017-02-16  8:53           ` Amir Goldstein
2017-02-12 20:43 ` [PATCH v3 2/9] fstests: use _test_mount() consistently Amir Goldstein
2017-02-13 11:17   ` Eryu Guan
2017-02-12 20:43 ` [PATCH v3 3/9] fstests: canonicalize mount points on every config section Amir Goldstein
2017-02-12 20:43 ` [PATCH v3 4/9] overlay: rename OVERLAY_LOWER/UPPER/WORK_DIR Amir Goldstein
2017-02-12 20:43 ` [PATCH v3 5/9] overlay: allow SCRATCH_DEV to be the base fs mount point Amir Goldstein
2017-02-12 20:43 ` [PATCH v3 6/9] overlay: configure TEST/SCRATCH vars to base fs Amir Goldstein
2017-02-13 11:28   ` Eryu Guan
2017-02-13 20:31     ` Amir Goldstein
2017-02-14 11:03       ` Eryu Guan
2017-02-15 14:59         ` Amir Goldstein
2017-02-12 20:43 ` [PATCH v3 7/9] overlay: use OVL_BASE_SCRATCH_MNT instead of SCRATCH_DEV Amir Goldstein
2017-02-12 20:43 ` [PATCH v3 8/9] overlay: fix test and scratch filters for overlay base fs Amir Goldstein
2017-02-13 20:39   ` Amir Goldstein
2017-02-12 20:43 ` [PATCH v3 9/9] overlay: mount/unmount base fs before/after running tests Amir Goldstein
2017-02-13 11:31   ` Eryu Guan
2017-02-13 11:59     ` Amir Goldstein
2017-02-14  0:23   ` Theodore Ts'o [this message]
2017-02-14  5:24     ` Eryu Guan
2017-02-14  6:43     ` Amir Goldstein
2017-02-14 17:07       ` Theodore Ts'o
2017-02-14 17:55         ` Amir Goldstein
2017-02-16  8:50           ` Amir Goldstein
2017-02-12 20:51 ` [PATCH v3 0/9] fstests: new way to run overlay tests Amir Goldstein
2017-02-13  4:19 ` Xiong Zhou
2017-02-13  5:37   ` Amir Goldstein
2017-02-14  4:40     ` Xiong Zhou
2017-02-14  6:15       ` Amir Goldstein
2017-02-14  9:25         ` Xiong Zhou
2017-02-14  9:51           ` Amir Goldstein
2017-02-13 11:02 ` Eryu Guan
2017-02-16  9:02   ` Amir Goldstein

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170214002356.dxrmf5jzwr6sivlu@thunk.org \
    --to=tytso@mit.edu \
    --cc=amir73il@gmail.com \
    --cc=eguan@redhat.com \
    --cc=fstests@vger.kernel.org \
    --cc=linux-unionfs@vger.kernel.org \
    --cc=miklos@szeredi.hu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.