Re: [PATCH v3 6/9] overlay: configure TEST/SCRATCH vars to base fs

Eryu Guan
2017-02-14T11:03:16Z

On Mon, Feb 13, 2017 at 10:31:30PM +0200, Amir Goldstein wrote:
> On Mon, Feb 13, 2017 at 1:28 PM, Eryu Guan <eguan@redhat.com> wrote:
> > On Sun, Feb 12, 2017 at 10:43:41PM +0200, Amir Goldstein wrote:
> >> Instead of setting the vars TEST/SCRATCH_DEV to overlay base dirs,
> >> allow setting them to block devices to configure the base fs partition,
> >> where overlay dirs will be created.
> >>
> >> For example, the following config file can be used to run tests on
> >> xfs test/scratch partitions:
> >>
> >> TEST_DEV=/dev/sda5
> >> TEST_DIR=/mnt/test
> >> SCRATCH_DEV=/dev/sda6
> >> SCRATCH_MNT=/mnt/scratch
> >> FSTYP=xfs
> >>
> >> Using the same config file, but executing './check -overlay' will
> >> use the same partitions as base fs for overlayfs directories
> >> and set TEST_DIR/SCRATCH_MNT values to overlay mount points, i.e.:
> >> /mnt/test/ovl-mnt and /mnt/scratch/ovl-mnt.
> >>
> >> The base fs should be pre-formatted and mounted when starting the test.
> >> An upcoming change is going to support mount/umount of base fs.
> >>
> >> The new OVL_BASE_SCRATCH/TEST_* vars are set to point at the
> >> overlayfs base dirs in either legacy or new config method.
> >> Tests should always use these vars and not the legacy SCRATCH/TEST_DEV
> >> vars when referring to overlay base dir.
> >>
> >> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> >> ---
> ...
> >>
> >> # Parse config section options. This function will parse all the configuration
> >> @@ -516,6 +574,15 @@ get_next_config() {
> >> unset SCRATCH_DEV
> >> fi
> >>
> >> + # We might have overriden TEST/SCRATCH vars with overlay base dir in the
> >> + # previous run, so restore them to original values stored in OVL_BASE_*
> >> + if [ "$FSTYP" == "overlay" ]; then
> >> + [ -z "$OVL_BASE_TEST_DEV" ] || export TEST_DEV=$OVL_BASE_TEST_DEV
> >> + [ -z "$OVL_BASE_TEST_DIR" ] || export TEST_DIR=$OVL_BASE_TEST_DIR
> >> + [ -z "$OVL_BASE_SCRATCH_DEV" ] || export SCRATCH_DEV=$OVL_BASE_SCRATCH_DEV
> >> + [ -z "$OVL_BASE_SCRATCH_MNT" ] || export SCRATCH_MNT=$OVL_BASE_SCRATCH_MNT
> >> + fi
> >> +
> >> parse_config_section $1
> >>
> >> if [ ! -z "$OLD_FSTYP" ] && [ $OLD_FSTYP != $FSTYP ]; then
> >> @@ -544,6 +611,12 @@ get_next_config() {
> >> export RESULT_BASE="$here/results/"
> >> fi
> >>
> >> + # Override FSTYP from config when running ./check -overlay
> >> + # and maybe derive overlay TEST/SCRATCH from base fs values
> >> + if [ "$OVERLAY" == "true" -o "$FSTYP" == "overlay" ]; then
> >> + _config_overlay
> >> + fi
> >> +
> >
> > If I have SCRATCH_DEV_POOL defined, e.g.
> >
> > TEST_DEV=/dev/sda5
> > TEST_DIR=/mnt/testarea/test
> > SCRATCH_DEV_POOL="/dev/sda6 /dev/sda7"
> > SCRATCH_MNT=/mnt/testarea/scratch
> >
> > I got error like:
> >
> > [root@dhcp-66-86-11 xfstests]# ./check -overlay overlay/002
> > common/config: SCRATCH_DEV (/dev/sda6) is not a directory for overlay
> >
> > Because we obtain SCRATCH_DEV from SCRATCH_DEV_POOL after we call
> > _config_overlay, and SCRATCH_DEV is not set when configing overlay, so
> > SCRATCH_DEV stays as a block device and fails _check_device.
> >
> > Moving the SCRATCH_DEV_POOL handling just after "unset SCRATCH_DEV"
> > fixes this problem for me.

Seems this only works for multi-section config, not standalone
local.config file..

> >
>
> Right, I didn't test this setup. It appears to me that SCRATCH_DEV_POOL handling
> should be *after* parse_config_section for multi section config, where
> SCRATCH_DEV_POOL is assigned a new value in current section??

I set SCRATCH_DEV_POOL as a default var, so it's inherited by all
sections.

>
> Can you check if SCRATCH_DEV_POOL handling could be moved right before
> _config_overlay?

No, test results show that it should be before restoring TEST_DEV/DIR
etc. from previous run, this hunk:

# We might have overriden TEST/SCRATCH vars with overlay base dir in the
# previous run, so restore them to original values stored in OVL_BASE_*
if [ "$FSTYP" == "overlay" ]; then
...

>
> I'm not really sure how that configuration should be working with
> overlay anyway.

In general, I think SCRATCH_DEV_POOL should be handled just after
reading in configs, as what you said. But multi-section config (maybe
and the new overlayfs config) makes things more complex, and my brain
stops working today..

Thanks,
Eryu