[PATCH v3 0/9] fstests: new way to run overlay tests

[PATCH v3 0/9] fstests: new way to run overlay tests

[Amir Goldstein]

Hi Eryu and all,

The reason I started this work was to help catch overlayfs bugs
related to leaking objects in underlying (base) fs.

As a by-product, following Eryu's comments on v2, configuring
xfstest to run overlay tests over any file system is now easier then ever.

With this change, all you have to do to run overlay tests if you
already have a local.config setup to test a local file system is:
 ./check -overlay

It uses existing local.config that was setup to run tests on
the base fs (e.g. xfs) and you can run './check' and './check -overlay'
without re-formatting the test partitions and without changing the
config file.

The legacy overlayfs configuration, where TEST_DEV is a directory
still works, but it should be deprecated.

I tested ./check -overlay -g quick with both legacy overlay configuration
and the new base fs configuration.

Until now, overlay test configuration was not documented at all.
I updated README per Eryu's request and tried to keep the documentation
short and simple.

Also updated README.config-sections with an easy example how to
interleave overlay tests on every base fs in the multi section config
file.

I honestly think that it is important for file system developers these days,
to test changes to their file systems with -overlay, to verify no breakage
is caused when their file systems are used as base fs to overlay containers.
I dare to say, that it is probably more important than testing 1k block size,
which most of the maintainers do test with regularly.

Some of the bugs I fixed in patches 1-3 indicate that people have not
been 'stress testing' the xfstest config file and all of its gloious
configurable options.  I tried to run with some basic configurations
to check my changes, but I doubt that I have covered more then a small
fraction of the configurations that people are using.

I would very much appreciate if anyone could test these changes with their
own set of configuration, with or without adding overlay tests into the mix.
You can get the branch for testing from my github tree [1].

Thanks,
Amir.

[1] https://github.com/amir73il/xfstests/tree/ovl_base_fs

v3:
- Mount cycle base test fs
- Fix bugs in non overlay specific sanity checks
- Run -overlay test with existing config file of base fs
- Run overlay tests per base fs by adding overlay config sections

v2:
- Test and scratch base dirs each have thier own base fs
- Support mount cycles of base fs for scratch tests

v1:
- Both test and scratch base dirs on a single base fs


Amir Goldstein (9):
  fstests: sanity check that test partitions are not mounted elsewhere
  fstests: use _test_mount() consistently
  fstests: canonicalize mount points on every config section
  overlay: rename OVERLAY_LOWER/UPPER/WORK_DIR
  overlay: allow SCRATCH_DEV to be the base fs mount point
  overlay: configure TEST/SCRATCH vars to base fs
  overlay: use OVL_BASE_SCRATCH_MNT instead of SCRATCH_DEV
  overlay: fix test and scratch filters for overlay base fs
  overlay: mount/unmount base fs before/after running tests

 README                 |  16 +++--
 README.config-sections |   6 ++
 check                  |  24 +++----
 common/config          | 121 +++++++++++++++++++++++++++-----
 common/filter          |   7 +-
 common/rc              | 184 +++++++++++++++++++++++++++++++++++--------------
 tests/overlay/001      |   7 +-
 tests/overlay/002      |   2 +-
 tests/overlay/003      |   5 +-
 tests/overlay/004      |   7 +-
 tests/overlay/005      |  30 ++++----
 tests/overlay/006      |  10 +--
 tests/overlay/008      |   8 +--
 tests/overlay/009      |   2 +-
 tests/overlay/010      |  10 +--
 tests/overlay/011      |   6 +-
 tests/overlay/012      |   4 +-
 tests/overlay/013      |   4 +-
 tests/overlay/014      |  19 ++---
 tests/overlay/015      |   2 +-
 tests/overlay/016      |   2 +-
 tests/overlay/017      |   2 +-
 tests/overlay/018      |   2 +-
 tests/overlay/019      |   2 +-
 tests/overlay/020      |   2 +-
 tests/overlay/021      |   6 +-
 26 files changed, 333 insertions(+), 157 deletions(-)

-- 
2.7.4

[PATCH v3 1/9] fstests: sanity check that test partitions are not mounted elsewhere

[Amir Goldstein]

When $TEST_DEV is mounted at a different location then $TEST_DIR,
_require_test() aborts the test with an error:
 TEST_DEV=/dev/sda5 is mounted but not on TEST_DIR=/mnt/test

There are several problems with current sanity check:
1. the output of the error is mixed into out.bad and hard to see
2. the test partition is unmounted at the end of the test regardless
   of the fact that it not pass the sanity that we have exclusivity
3. scratch partition has a similar sanity check in _require_scratch(),
   but we may not get to it, because $SCRATCH_DEV is unmounted prior
   to running the tests (which could unmount another mount point).

To solve all these problems, introduce a helper _check_mounted_on().
It checks if a device is mounted on a given mount point and optionally
checks the mounted fs type.

The sanity checks in _require_scratch() and _require_test() are
converted to use the helper and gain the check for correct fs type.

The helper is used in init_rc() to sanity check both test and scratch
partitions, before tests are run and before $SCRATCH_DEV is unmounted.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 common/rc | 83 +++++++++++++++++++++++++++++++++++++--------------------------
 1 file changed, 49 insertions(+), 34 deletions(-)

diff --git a/common/rc b/common/rc
index 76515e2..d831b17 100644
--- a/common/rc
+++ b/common/rc
@@ -1319,6 +1319,43 @@ _supported_os()
     _notrun "not suitable for this OS: $HOSTOS"
 }
 
+# check if a FS on a device is mounted
+# if so, verify that it is mounted on mount point
+# if fstype is given as argument, verify that it is also
+# mounted with correct fs type
+#
+_check_mounted_on()
+{
+	local devname=$1
+	local dev=$2
+	local mntname=$3
+	local mnt=$4
+	local type=$5
+
+	# Note that we use -F here so grep doesn't try to interpret an NFS over
+	# IPv6 server as a regular expression
+	local mount_rec=`_mount | grep -F "$dev"`
+	[ -n "$mount_rec" ] || return 1
+
+	# if it's mounted, make sure its on $mnt
+	if ! (echo $mount_rec | grep -q "$mnt")
+	then
+		echo "$devname=$dev is mounted but not on $mntname=$mnt - aborting"
+		echo "Already mounted result:"
+		echo $mount_rec
+		exit 1
+	fi
+
+	if [ -n "$type" -a "`_fs_type $dev`" != "$type" ]
+	then
+		echo "$devname=$dev is mounted but not a type $type filesystem"
+		# raw $DF_PROG cannot handle NFS/CIFS/overlay correctly
+		_df_device $dev
+		exit 1
+	fi
+	return 0
+}
+
 # this test needs a scratch partition - check we're ok & unmount it
 # No post-test check of the device is required. e.g. the test intentionally
 # finishes the test with the filesystem in a corrupt state
@@ -1373,21 +1410,9 @@ _require_scratch_nocheck()
 		 ;;
     esac
 
-    # mounted?
-    # Note that we use -F here so grep doesn't try to interpret an NFS over
-    # IPv6 server as a regular expression.
-    mount_rec=`_mount | grep -F $SCRATCH_DEV`
-    if [ "$mount_rec" ]
+    if _check_mounted_on SCRATCH_DEV $SCRATCH_DEV SCRATCH_MNT $SCRATCH_MNT
     then
-        # if it's mounted, make sure its on $SCRATCH_MNT
-        if ! echo $mount_rec | grep -q $SCRATCH_MNT
-        then
-            echo "\$SCRATCH_DEV=$SCRATCH_DEV is mounted but not on \$SCRATCH_MNT=$SCRATCH_MNT - aborting"
-            echo "Already mounted result:"
-            echo $mount_rec
-            exit 1
-        fi
-        # and then unmount it
+        # if it's mounted, unmount it
         if ! _scratch_unmount
         then
             echo "failed to unmount $SCRATCH_DEV"
@@ -1458,21 +1483,8 @@ _require_test()
 		 ;;
     esac
 
-    # mounted?
-    # Note that we use -F here so grep doesn't try to interpret an NFS over
-    # IPv6 server as a regular expression.
-    mount_rec=`_mount | grep -F $TEST_DEV`
-    if [ "$mount_rec" ]
+    if ! _check_mounted_on TEST_DEV $TEST_DEV TEST_DIR $TEST_DIR
     then
-        # if it's mounted, make sure its on $TEST_DIR
-        if ! echo $mount_rec | grep -q $TEST_DIR
-        then
-            echo "\$TEST_DEV=$TEST_DEV is mounted but not on \$TEST_DIR=$TEST_DIR - aborting"
-            echo "Already mounted result:"
-            echo $mount_rec
-            exit 1
-        fi
-    else
 	out=`_mount_or_remount_rw "$MOUNT_OPTIONS" $TEST_DEV $TEST_DIR`
 	if [ $? -ne 1 ]; then
 		echo $out
@@ -3075,13 +3087,16 @@ init_rc()
 		fi
 	fi
 
-	if [ "`_fs_type $TEST_DEV`" != "$FSTYP" ]
-	then
-		echo "common/rc: Error: \$TEST_DEV ($TEST_DEV) is not a MOUNTED $FSTYP filesystem"
-		# raw $DF_PROG cannot handle NFS/CIFS/overlay correctly
-		_df_device $TEST_DEV
-		exit 1
+	# Sanity check that TEST partition is not mounted at another mount point
+	# or as another fs type
+	_check_mounted_on TEST_DEV $TEST_DEV TEST_DIR $TEST_DIR $FSTYP
+	if [ -n "$SCRATCH_DEV" ]; then
+		# Sanity check that SCRATCH partition is not mounted at another
+		# mount point, because it is about to be unmounted and formatted.
+		# Another fs type for scratch is fine (bye bye old fs type).
+		_check_mounted_on SCRATCH_DEV $SCRATCH_DEV SCRATCH_MNT $SCRATCH_MNT
 	fi
+
 	# Figure out if we need to add -F ("foreign", deprecated) option to xfs_io
 	$XFS_IO_PROG -c stat $TEST_DIR 2>&1 | grep -q "is not on an XFS filesystem" && \
 		export XFS_IO_PROG="$XFS_IO_PROG -F"
-- 
2.7.4

Re: [PATCH v3 1/9] fstests: sanity check that test partitions are not mounted elsewhere

[Eryu Guan]

On Sun, Feb 12, 2017 at 10:43:36PM +0200, Amir Goldstein wrote:
> When $TEST_DEV is mounted at a different location then $TEST_DIR,
> _require_test() aborts the test with an error:
>  TEST_DEV=/dev/sda5 is mounted but not on TEST_DIR=/mnt/test
> 
> There are several problems with current sanity check:
> 1. the output of the error is mixed into out.bad and hard to see
> 2. the test partition is unmounted at the end of the test regardless
>    of the fact that it not pass the sanity that we have exclusivity
> 3. scratch partition has a similar sanity check in _require_scratch(),
>    but we may not get to it, because $SCRATCH_DEV is unmounted prior
>    to running the tests (which could unmount another mount point).
> 
> To solve all these problems, introduce a helper _check_mounted_on().
> It checks if a device is mounted on a given mount point and optionally
> checks the mounted fs type.
> 
> The sanity checks in _require_scratch() and _require_test() are
> converted to use the helper and gain the check for correct fs type.
> 
> The helper is used in init_rc() to sanity check both test and scratch
> partitions, before tests are run and before $SCRATCH_DEV is unmounted.
> 
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
>  common/rc | 83 +++++++++++++++++++++++++++++++++++++--------------------------
>  1 file changed, 49 insertions(+), 34 deletions(-)
> 
> diff --git a/common/rc b/common/rc
> index 76515e2..d831b17 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -1319,6 +1319,43 @@ _supported_os()
>      _notrun "not suitable for this OS: $HOSTOS"
>  }
>  
> +# check if a FS on a device is mounted
> +# if so, verify that it is mounted on mount point
> +# if fstype is given as argument, verify that it is also
> +# mounted with correct fs type
> +#
> +_check_mounted_on()
> +{
> +	local devname=$1
> +	local dev=$2
> +	local mntname=$3
> +	local mnt=$4
> +	local type=$5
> +
> +	# Note that we use -F here so grep doesn't try to interpret an NFS over
> +	# IPv6 server as a regular expression
> +	local mount_rec=`_mount | grep -F "$dev"`
> +	[ -n "$mount_rec" ] || return 1
> +
> +	# if it's mounted, make sure its on $mnt
> +	if ! (echo $mount_rec | grep -q "$mnt")
> +	then
> +		echo "$devname=$dev is mounted but not on $mntname=$mnt - aborting"
> +		echo "Already mounted result:"
> +		echo $mount_rec
> +		exit 1
> +	fi
> +
> +	if [ -n "$type" -a "`_fs_type $dev`" != "$type" ]
> +	then
> +		echo "$devname=$dev is mounted but not a type $type filesystem"
> +		# raw $DF_PROG cannot handle NFS/CIFS/overlay correctly
> +		_df_device $dev
> +		exit 1
> +	fi
> +	return 0
> +}
> +
>  # this test needs a scratch partition - check we're ok & unmount it
>  # No post-test check of the device is required. e.g. the test intentionally
>  # finishes the test with the filesystem in a corrupt state
> @@ -1373,21 +1410,9 @@ _require_scratch_nocheck()
>  		 ;;
>      esac
>  
> -    # mounted?
> -    # Note that we use -F here so grep doesn't try to interpret an NFS over
> -    # IPv6 server as a regular expression.
> -    mount_rec=`_mount | grep -F $SCRATCH_DEV`
> -    if [ "$mount_rec" ]
> +    if _check_mounted_on SCRATCH_DEV $SCRATCH_DEV SCRATCH_MNT $SCRATCH_MNT
>      then
> -        # if it's mounted, make sure its on $SCRATCH_MNT
> -        if ! echo $mount_rec | grep -q $SCRATCH_MNT
> -        then
> -            echo "\$SCRATCH_DEV=$SCRATCH_DEV is mounted but not on \$SCRATCH_MNT=$SCRATCH_MNT - aborting"
> -            echo "Already mounted result:"
> -            echo $mount_rec
> -            exit 1
> -        fi
> -        # and then unmount it
> +        # if it's mounted, unmount it
>          if ! _scratch_unmount
>          then
>              echo "failed to unmount $SCRATCH_DEV"
> @@ -1458,21 +1483,8 @@ _require_test()
>  		 ;;
>      esac
>  
> -    # mounted?
> -    # Note that we use -F here so grep doesn't try to interpret an NFS over
> -    # IPv6 server as a regular expression.
> -    mount_rec=`_mount | grep -F $TEST_DEV`
> -    if [ "$mount_rec" ]
> +    if ! _check_mounted_on TEST_DEV $TEST_DEV TEST_DIR $TEST_DIR
>      then
> -        # if it's mounted, make sure its on $TEST_DIR
> -        if ! echo $mount_rec | grep -q $TEST_DIR
> -        then
> -            echo "\$TEST_DEV=$TEST_DEV is mounted but not on \$TEST_DIR=$TEST_DIR - aborting"
> -            echo "Already mounted result:"
> -            echo $mount_rec
> -            exit 1
> -        fi
> -    else
>  	out=`_mount_or_remount_rw "$MOUNT_OPTIONS" $TEST_DEV $TEST_DIR`
>  	if [ $? -ne 1 ]; then
>  		echo $out
> @@ -3075,13 +3087,16 @@ init_rc()
>  		fi
>  	fi
>  
> -	if [ "`_fs_type $TEST_DEV`" != "$FSTYP" ]
> -	then
> -		echo "common/rc: Error: \$TEST_DEV ($TEST_DEV) is not a MOUNTED $FSTYP filesystem"
> -		# raw $DF_PROG cannot handle NFS/CIFS/overlay correctly
> -		_df_device $TEST_DEV
> -		exit 1
> +	# Sanity check that TEST partition is not mounted at another mount point
> +	# or as another fs type
> +	_check_mounted_on TEST_DEV $TEST_DEV TEST_DIR $TEST_DIR $FSTYP
> +	if [ -n "$SCRATCH_DEV" ]; then
> +		# Sanity check that SCRATCH partition is not mounted at another
> +		# mount point, because it is about to be unmounted and formatted.
> +		# Another fs type for scratch is fine (bye bye old fs type).
> +		_check_mounted_on SCRATCH_DEV $SCRATCH_DEV SCRATCH_MNT $SCRATCH_MNT
>  	fi

My test configs look like:

TEST_DEV=/dev/sda5
SCRATCH_DEV=/dev/sda6
TEST_DIR=/mnt/testarea/test
SCRATCH_MNT=/mnt/testarea/scratch

and if I mount SCRATCH_DEV at /mnt/xfs (or some other mountpoints rather
than SCRATCH_MNT), ./check -overlay overlay/??? isn't able to detect
this mis-configuration.

[root@dhcp-66-86-11 xfstests]# ./check  -overlay overlay/002
FSTYP         -- overlay
PLATFORM      -- Linux/x86_64 dhcp-66-86-11 4.10.0-rc7
MKFS_OPTIONS  -- /mnt/testarea/scratch
MOUNT_OPTIONS -- -o context=system_u:object_r:nfs_t:s0 -o lowerdir=/mnt/testarea/scratch/ovl-lower,upperdir=/mnt/testarea/scratch/ovl-upper,workdir=/mnt/testarea/scratch/ovl-work

[root@dhcp-66-86-11 xfstests]#

And nothing useful was printed. This is because my rootfs has no
filetype support, but the _notrun message is redirected to a file in
check, as

"if ! _scratch_mkfs >$tmp.err 2>&1"

Adding _check_mounted_on against OVL_BASE_TEST/SCRATCH_DEV here could
fix it for me.

Thanks,
Eryu

> +
>  	# Figure out if we need to add -F ("foreign", deprecated) option to xfs_io
>  	$XFS_IO_PROG -c stat $TEST_DIR 2>&1 | grep -q "is not on an XFS filesystem" && \
>  		export XFS_IO_PROG="$XFS_IO_PROG -F"
> -- 
> 2.7.4
> 

Re: [PATCH v3 1/9] fstests: sanity check that test partitions are not mounted elsewhere

[Amir Goldstein]

On Mon, Feb 13, 2017 at 1:10 PM, Eryu Guan <eguan@redhat.com> wrote:
> On Sun, Feb 12, 2017 at 10:43:36PM +0200, Amir Goldstein wrote:
>> When $TEST_DEV is mounted at a different location then $TEST_DIR,
>> _require_test() aborts the test with an error:
>>  TEST_DEV=/dev/sda5 is mounted but not on TEST_DIR=/mnt/test
>>
>> There are several problems with current sanity check:
>> 1. the output of the error is mixed into out.bad and hard to see
>> 2. the test partition is unmounted at the end of the test regardless
>>    of the fact that it not pass the sanity that we have exclusivity
>> 3. scratch partition has a similar sanity check in _require_scratch(),
>>    but we may not get to it, because $SCRATCH_DEV is unmounted prior
>>    to running the tests (which could unmount another mount point).
>>
>> To solve all these problems, introduce a helper _check_mounted_on().
>> It checks if a device is mounted on a given mount point and optionally
>> checks the mounted fs type.
>>
>> The sanity checks in _require_scratch() and _require_test() are
>> converted to use the helper and gain the check for correct fs type.
>>
>> The helper is used in init_rc() to sanity check both test and scratch
>> partitions, before tests are run and before $SCRATCH_DEV is unmounted.
>>
>> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
>> ---
>>  common/rc | 83 +++++++++++++++++++++++++++++++++++++--------------------------
>>  1 file changed, 49 insertions(+), 34 deletions(-)

...

> My test configs look like:
>
> TEST_DEV=/dev/sda5
> SCRATCH_DEV=/dev/sda6
> TEST_DIR=/mnt/testarea/test
> SCRATCH_MNT=/mnt/testarea/scratch
>
> and if I mount SCRATCH_DEV at /mnt/xfs (or some other mountpoints rather
> than SCRATCH_MNT), ./check -overlay overlay/??? isn't able to detect
> this mis-configuration.
>
> [root@dhcp-66-86-11 xfstests]# ./check  -overlay overlay/002
> FSTYP         -- overlay
> PLATFORM      -- Linux/x86_64 dhcp-66-86-11 4.10.0-rc7
> MKFS_OPTIONS  -- /mnt/testarea/scratch
> MOUNT_OPTIONS -- -o context=system_u:object_r:nfs_t:s0 -o lowerdir=/mnt/testarea/scratch/ovl-lower,upperdir=/mnt/testarea/scratch/ovl-upper,workdir=/mnt/testarea/scratch/ovl-work
>
> [root@dhcp-66-86-11 xfstests]#
>
> And nothing useful was printed. This is because my rootfs has no
> filetype support, but the _notrun message is redirected to a file in
> check, as
>
> "if ! _scratch_mkfs >$tmp.err 2>&1"
>
> Adding _check_mounted_on against OVL_BASE_TEST/SCRATCH_DEV here could
> fix it for me.
>

Actually, there that test already exists in:

_scratch_mkfs
  _scratch_cleanup_files
    _overlay_base_scratch_mount
      _check_mounted_on

Only I forgot to cleanup an unneeded _overlay_base_scratch_unmount()
from _scratch_cleanup_files()..

@@ -698,8 +698,6 @@ _scratch_cleanup_files()
        overlay)
                # Avoid rm -rf /* if we messed up
                [ -n "$OVL_BASE_SCRATCH_MNT" ] || return 1
-               # overlay 'mkfs' needs to make sure base fs is mounted and clean
-               _overlay_base_scratch_unmount 2>/dev/null
                _overlay_base_scratch_mount
                rm -rf $OVL_BASE_SCRATCH_MNT/*
                _overlay_mkdirs $OVL_BASE_SCRATCH_MNT

With this patch, test does abort for _check_mounted_on() failure,
(please verify)
but output is still lost inside tmp.err.
To fix this I would need to not exit 1 inside _check_mounted_on() but
always by callers.
Is that what you prefer that I do?

Re: [PATCH v3 1/9] fstests: sanity check that test partitions are not mounted elsewhere

[Amir Goldstein]

On Mon, Feb 13, 2017 at 1:44 PM, Amir Goldstein <amir73il@gmail.com> wrote:
> On Mon, Feb 13, 2017 at 1:10 PM, Eryu Guan <eguan@redhat.com> wrote:
>> On Sun, Feb 12, 2017 at 10:43:36PM +0200, Amir Goldstein wrote:
>>> When $TEST_DEV is mounted at a different location then $TEST_DIR,
>>> _require_test() aborts the test with an error:
>>>  TEST_DEV=/dev/sda5 is mounted but not on TEST_DIR=/mnt/test
>>>
>>> There are several problems with current sanity check:
>>> 1. the output of the error is mixed into out.bad and hard to see
>>> 2. the test partition is unmounted at the end of the test regardless
>>>    of the fact that it not pass the sanity that we have exclusivity
>>> 3. scratch partition has a similar sanity check in _require_scratch(),
>>>    but we may not get to it, because $SCRATCH_DEV is unmounted prior
>>>    to running the tests (which could unmount another mount point).
>>>
>>> To solve all these problems, introduce a helper _check_mounted_on().
>>> It checks if a device is mounted on a given mount point and optionally
>>> checks the mounted fs type.
>>>
>>> The sanity checks in _require_scratch() and _require_test() are
>>> converted to use the helper and gain the check for correct fs type.
>>>
>>> The helper is used in init_rc() to sanity check both test and scratch
>>> partitions, before tests are run and before $SCRATCH_DEV is unmounted.
>>>
>>> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
>>> ---
>>>  common/rc | 83 +++++++++++++++++++++++++++++++++++++--------------------------
>>>  1 file changed, 49 insertions(+), 34 deletions(-)
>
> ...
>
>> My test configs look like:
>>
>> TEST_DEV=/dev/sda5
>> SCRATCH_DEV=/dev/sda6
>> TEST_DIR=/mnt/testarea/test
>> SCRATCH_MNT=/mnt/testarea/scratch
>>
>> and if I mount SCRATCH_DEV at /mnt/xfs (or some other mountpoints rather
>> than SCRATCH_MNT), ./check -overlay overlay/??? isn't able to detect
>> this mis-configuration.
>>
>> [root@dhcp-66-86-11 xfstests]# ./check  -overlay overlay/002
>> FSTYP         -- overlay
>> PLATFORM      -- Linux/x86_64 dhcp-66-86-11 4.10.0-rc7
>> MKFS_OPTIONS  -- /mnt/testarea/scratch
>> MOUNT_OPTIONS -- -o context=system_u:object_r:nfs_t:s0 -o lowerdir=/mnt/testarea/scratch/ovl-lower,upperdir=/mnt/testarea/scratch/ovl-upper,workdir=/mnt/testarea/scratch/ovl-work
>>
>> [root@dhcp-66-86-11 xfstests]#
>>
>> And nothing useful was printed. This is because my rootfs has no
>> filetype support, but the _notrun message is redirected to a file in
>> check, as
>>
>> "if ! _scratch_mkfs >$tmp.err 2>&1"
>>
>> Adding _check_mounted_on against OVL_BASE_TEST/SCRATCH_DEV here could
>> fix it for me.
>>
>
> Actually, there that test already exists in:
>
> _scratch_mkfs
>   _scratch_cleanup_files
>     _overlay_base_scratch_mount
>       _check_mounted_on
>
> Only I forgot to cleanup an unneeded _overlay_base_scratch_unmount()
> from _scratch_cleanup_files()..
>
> @@ -698,8 +698,6 @@ _scratch_cleanup_files()
>         overlay)
>                 # Avoid rm -rf /* if we messed up
>                 [ -n "$OVL_BASE_SCRATCH_MNT" ] || return 1
> -               # overlay 'mkfs' needs to make sure base fs is mounted and clean
> -               _overlay_base_scratch_unmount 2>/dev/null
>                 _overlay_base_scratch_mount
>                 rm -rf $OVL_BASE_SCRATCH_MNT/*
>                 _overlay_mkdirs $OVL_BASE_SCRATCH_MNT
>
> With this patch, test does abort for _check_mounted_on() failure,
> (please verify)
> but output is still lost inside tmp.err.
> To fix this I would need to not exit 1 inside _check_mounted_on() but
> always by callers.
> Is that what you prefer that I do?

How about something more general like this:
(tested with your test case and with wipefs -a $SCRATCH_DEV):

@@ -376,6 +376,11 @@ _wrapup()
        seq="check"
        check="$RESULT_BASE/check"

+       if [ -n "$check_err" ]; then
+               echo "check: $check_err"
+               check_err=""
+       fi
+

@@ -466,6 +471,7 @@ _check_filesystems()

 _prepare_test_list

+check_err=""
 if $OPTIONS_HAVE_SECTIONS; then
        trap "_summary; exit \$status" 0 1 2 3 15
 else

@@ -567,10 +576,10 @@ for section in $HOST_OPTIONS_SECTIONS; do
          # call the overridden mkfs - make sure the FS is built
          # the same as we'll create it later.

-         if ! _scratch_mkfs >$tmp.err 2>&1
+         check_err=`_scratch_mkfs 2>&1`
+         if [ $? -ne 0 ]
          then
              echo "our local _scratch_mkfs routine ..."
-             cat $tmp.err
              echo "check: failed to mkfs \$SCRATCH_DEV using specified options"
              status=1
              exit
@@ -578,14 +587,15 @@ for section in $HOST_OPTIONS_SECTIONS; do

          # call the overridden mount - make sure the FS mounts with
          # the same options that we'll mount with later.
-         if ! _scratch_mount >$tmp.err 2>&1
+         check_err=`_scratch_mount 2>&1`
+         if [ $? -ne 0 ]
          then
              echo "our local mount routine ..."
-             cat $tmp.err
              echo "check: failed to mount \$SCRATCH_DEV using
specified options"
              status=1
              exit
          fi
+         check_err=""
        fi

Re: [PATCH v3 1/9] fstests: sanity check that test partitions are not mounted elsewhere

[Eryu Guan]

On Mon, Feb 13, 2017 at 03:33:23PM +0200, Amir Goldstein wrote:
> On Mon, Feb 13, 2017 at 1:44 PM, Amir Goldstein <amir73il@gmail.com> wrote:
> > On Mon, Feb 13, 2017 at 1:10 PM, Eryu Guan <eguan@redhat.com> wrote:
> >> On Sun, Feb 12, 2017 at 10:43:36PM +0200, Amir Goldstein wrote:
> >>> When $TEST_DEV is mounted at a different location then $TEST_DIR,
> >>> _require_test() aborts the test with an error:
> >>>  TEST_DEV=/dev/sda5 is mounted but not on TEST_DIR=/mnt/test
> >>>
> >>> There are several problems with current sanity check:
> >>> 1. the output of the error is mixed into out.bad and hard to see
> >>> 2. the test partition is unmounted at the end of the test regardless
> >>>    of the fact that it not pass the sanity that we have exclusivity
> >>> 3. scratch partition has a similar sanity check in _require_scratch(),
> >>>    but we may not get to it, because $SCRATCH_DEV is unmounted prior
> >>>    to running the tests (which could unmount another mount point).
> >>>
> >>> To solve all these problems, introduce a helper _check_mounted_on().
> >>> It checks if a device is mounted on a given mount point and optionally
> >>> checks the mounted fs type.
> >>>
> >>> The sanity checks in _require_scratch() and _require_test() are
> >>> converted to use the helper and gain the check for correct fs type.
> >>>
> >>> The helper is used in init_rc() to sanity check both test and scratch
> >>> partitions, before tests are run and before $SCRATCH_DEV is unmounted.
> >>>
> >>> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> >>> ---
> >>>  common/rc | 83 +++++++++++++++++++++++++++++++++++++--------------------------
> >>>  1 file changed, 49 insertions(+), 34 deletions(-)
> >
> > ...
> >
> >> My test configs look like:
> >>
> >> TEST_DEV=/dev/sda5
> >> SCRATCH_DEV=/dev/sda6
> >> TEST_DIR=/mnt/testarea/test
> >> SCRATCH_MNT=/mnt/testarea/scratch
> >>
> >> and if I mount SCRATCH_DEV at /mnt/xfs (or some other mountpoints rather
> >> than SCRATCH_MNT), ./check -overlay overlay/??? isn't able to detect
> >> this mis-configuration.
> >>
> >> [root@dhcp-66-86-11 xfstests]# ./check  -overlay overlay/002
> >> FSTYP         -- overlay
> >> PLATFORM      -- Linux/x86_64 dhcp-66-86-11 4.10.0-rc7
> >> MKFS_OPTIONS  -- /mnt/testarea/scratch
> >> MOUNT_OPTIONS -- -o context=system_u:object_r:nfs_t:s0 -o lowerdir=/mnt/testarea/scratch/ovl-lower,upperdir=/mnt/testarea/scratch/ovl-upper,workdir=/mnt/testarea/scratch/ovl-work
> >>
> >> [root@dhcp-66-86-11 xfstests]#
> >>
> >> And nothing useful was printed. This is because my rootfs has no
> >> filetype support, but the _notrun message is redirected to a file in
> >> check, as
> >>
> >> "if ! _scratch_mkfs >$tmp.err 2>&1"
> >>
> >> Adding _check_mounted_on against OVL_BASE_TEST/SCRATCH_DEV here could
> >> fix it for me.
> >>
> >
> > Actually, there that test already exists in:
> >
> > _scratch_mkfs
> >   _scratch_cleanup_files
> >     _overlay_base_scratch_mount
> >       _check_mounted_on

Hmm, I don't think this kind of basic config sanity check belongs here,
this should be done in config and env setup time. (So I think
_overlay_mount should be fixed too, that _supports_filetype check
doesn't belong there either.)

How about adding these checks in init_rc, along with other
_check_mounted_on checks against TEST_DEV and SCRATCH_DEV?

> >
> > Only I forgot to cleanup an unneeded _overlay_base_scratch_unmount()
> > from _scratch_cleanup_files()..
> >
> > @@ -698,8 +698,6 @@ _scratch_cleanup_files()
> >         overlay)
> >                 # Avoid rm -rf /* if we messed up
> >                 [ -n "$OVL_BASE_SCRATCH_MNT" ] || return 1
> > -               # overlay 'mkfs' needs to make sure base fs is mounted and clean
> > -               _overlay_base_scratch_unmount 2>/dev/null
> >                 _overlay_base_scratch_mount
> >                 rm -rf $OVL_BASE_SCRATCH_MNT/*
> >                 _overlay_mkdirs $OVL_BASE_SCRATCH_MNT
> >
> > With this patch, test does abort for _check_mounted_on() failure,
> > (please verify)

Yes, this works.

Thanks,
Eryu

> > but output is still lost inside tmp.err.
> > To fix this I would need to not exit 1 inside _check_mounted_on() but
> > always by callers.
> > Is that what you prefer that I do?
> 
> How about something more general like this:
> (tested with your test case and with wipefs -a $SCRATCH_DEV):
> 
> @@ -376,6 +376,11 @@ _wrapup()
>         seq="check"
>         check="$RESULT_BASE/check"
> 
> +       if [ -n "$check_err" ]; then
> +               echo "check: $check_err"
> +               check_err=""
> +       fi
> +
> 
> @@ -466,6 +471,7 @@ _check_filesystems()
> 
>  _prepare_test_list
> 
> +check_err=""
>  if $OPTIONS_HAVE_SECTIONS; then
>         trap "_summary; exit \$status" 0 1 2 3 15
>  else
> 
> @@ -567,10 +576,10 @@ for section in $HOST_OPTIONS_SECTIONS; do
>           # call the overridden mkfs - make sure the FS is built
>           # the same as we'll create it later.
> 
> -         if ! _scratch_mkfs >$tmp.err 2>&1
> +         check_err=`_scratch_mkfs 2>&1`
> +         if [ $? -ne 0 ]
>           then
>               echo "our local _scratch_mkfs routine ..."
> -             cat $tmp.err
>               echo "check: failed to mkfs \$SCRATCH_DEV using specified options"
>               status=1
>               exit
> @@ -578,14 +587,15 @@ for section in $HOST_OPTIONS_SECTIONS; do
> 
>           # call the overridden mount - make sure the FS mounts with
>           # the same options that we'll mount with later.
> -         if ! _scratch_mount >$tmp.err 2>&1
> +         check_err=`_scratch_mount 2>&1`
> +         if [ $? -ne 0 ]
>           then
>               echo "our local mount routine ..."
> -             cat $tmp.err
>               echo "check: failed to mount \$SCRATCH_DEV using
> specified options"
>               status=1
>               exit
>           fi
> +         check_err=""
>         fi

Re: [PATCH v3 1/9] fstests: sanity check that test partitions are not mounted elsewhere

[Amir Goldstein]

On Tue, Feb 14, 2017 at 7:51 AM, Eryu Guan <eguan@redhat.com> wrote:
> On Mon, Feb 13, 2017 at 03:33:23PM +0200, Amir Goldstein wrote:
>> On Mon, Feb 13, 2017 at 1:44 PM, Amir Goldstein <amir73il@gmail.com> wrote:
>> > On Mon, Feb 13, 2017 at 1:10 PM, Eryu Guan <eguan@redhat.com> wrote:
>> >> On Sun, Feb 12, 2017 at 10:43:36PM +0200, Amir Goldstein wrote:
>> >>> When $TEST_DEV is mounted at a different location then $TEST_DIR,
>> >>> _require_test() aborts the test with an error:
>> >>>  TEST_DEV=/dev/sda5 is mounted but not on TEST_DIR=/mnt/test
>> >>>
>> >>> There are several problems with current sanity check:
>> >>> 1. the output of the error is mixed into out.bad and hard to see
>> >>> 2. the test partition is unmounted at the end of the test regardless
>> >>>    of the fact that it not pass the sanity that we have exclusivity
>> >>> 3. scratch partition has a similar sanity check in _require_scratch(),
>> >>>    but we may not get to it, because $SCRATCH_DEV is unmounted prior
>> >>>    to running the tests (which could unmount another mount point).
>> >>>
>> >>> To solve all these problems, introduce a helper _check_mounted_on().
>> >>> It checks if a device is mounted on a given mount point and optionally
>> >>> checks the mounted fs type.
>> >>>
>> >>> The sanity checks in _require_scratch() and _require_test() are
>> >>> converted to use the helper and gain the check for correct fs type.
>> >>>
>> >>> The helper is used in init_rc() to sanity check both test and scratch
>> >>> partitions, before tests are run and before $SCRATCH_DEV is unmounted.
>> >>>
>> >>> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
>> >>> ---
>> >>>  common/rc | 83 +++++++++++++++++++++++++++++++++++++--------------------------
>> >>>  1 file changed, 49 insertions(+), 34 deletions(-)
>> >
>> > ...
>> >
>> >> My test configs look like:
>> >>
>> >> TEST_DEV=/dev/sda5
>> >> SCRATCH_DEV=/dev/sda6
>> >> TEST_DIR=/mnt/testarea/test
>> >> SCRATCH_MNT=/mnt/testarea/scratch
>> >>
>> >> and if I mount SCRATCH_DEV at /mnt/xfs (or some other mountpoints rather
>> >> than SCRATCH_MNT), ./check -overlay overlay/??? isn't able to detect
>> >> this mis-configuration.
>> >>
>> >> [root@dhcp-66-86-11 xfstests]# ./check  -overlay overlay/002
>> >> FSTYP         -- overlay
>> >> PLATFORM      -- Linux/x86_64 dhcp-66-86-11 4.10.0-rc7
>> >> MKFS_OPTIONS  -- /mnt/testarea/scratch
>> >> MOUNT_OPTIONS -- -o context=system_u:object_r:nfs_t:s0 -o lowerdir=/mnt/testarea/scratch/ovl-lower,upperdir=/mnt/testarea/scratch/ovl-upper,workdir=/mnt/testarea/scratch/ovl-work
>> >>
>> >> [root@dhcp-66-86-11 xfstests]#
>> >>
>> >> And nothing useful was printed. This is because my rootfs has no
>> >> filetype support, but the _notrun message is redirected to a file in
>> >> check, as
>> >>
>> >> "if ! _scratch_mkfs >$tmp.err 2>&1"
>> >>
>> >> Adding _check_mounted_on against OVL_BASE_TEST/SCRATCH_DEV here could
>> >> fix it for me.
>> >>
>> >
>> > Actually, there that test already exists in:
>> >
>> > _scratch_mkfs
>> >   _scratch_cleanup_files
>> >     _overlay_base_scratch_mount
>> >       _check_mounted_on
>
> Hmm, I don't think this kind of basic config sanity check belongs here,
> this should be done in config and env setup time. (So I think
> _overlay_mount should be fixed too, that _supports_filetype check
> doesn't belong there either.)
>
> How about adding these checks in init_rc, along with other
> _check_mounted_on checks against TEST_DEV and SCRATCH_DEV?
>

Yes, that makes sense.

But I still wonder how "exit 1" from within helpers should be handled
from check when stdout/err are redirected to $tmp.err.
Trying to catch the config error earlier is a good practice, but
it won't ensure against the same type of problem in the future.

What did you think about my approach to store mkfs output in  $check_err
var instead of $tmp.err file and spew $check_err in _wrapup?
BTW I tries 'cat $tmp.err' in _wrapup, but output is still redirected to
$tmp.err while in trap, so cat says: "cat: input file is output file".

Re: [PATCH v3 1/9] fstests: sanity check that test partitions are not mounted elsewhere

[Eryu Guan]

On Tue, Feb 14, 2017 at 08:02:27AM +0200, Amir Goldstein wrote:
[snip]
> >> >> My test configs look like:
> >> >>
> >> >> TEST_DEV=/dev/sda5
> >> >> SCRATCH_DEV=/dev/sda6
> >> >> TEST_DIR=/mnt/testarea/test
> >> >> SCRATCH_MNT=/mnt/testarea/scratch
> >> >>
> >> >> and if I mount SCRATCH_DEV at /mnt/xfs (or some other mountpoints rather
> >> >> than SCRATCH_MNT), ./check -overlay overlay/??? isn't able to detect
> >> >> this mis-configuration.
> >> >>
> >> >> [root@dhcp-66-86-11 xfstests]# ./check  -overlay overlay/002
> >> >> FSTYP         -- overlay
> >> >> PLATFORM      -- Linux/x86_64 dhcp-66-86-11 4.10.0-rc7
> >> >> MKFS_OPTIONS  -- /mnt/testarea/scratch
> >> >> MOUNT_OPTIONS -- -o context=system_u:object_r:nfs_t:s0 -o lowerdir=/mnt/testarea/scratch/ovl-lower,upperdir=/mnt/testarea/scratch/ovl-upper,workdir=/mnt/testarea/scratch/ovl-work
> >> >>
> >> >> [root@dhcp-66-86-11 xfstests]#
> >> >>
> >> >> And nothing useful was printed. This is because my rootfs has no
> >> >> filetype support, but the _notrun message is redirected to a file in
> >> >> check, as
> >> >>
> >> >> "if ! _scratch_mkfs >$tmp.err 2>&1"
> >> >>
> >> >> Adding _check_mounted_on against OVL_BASE_TEST/SCRATCH_DEV here could
> >> >> fix it for me.
> >> >>
> >> >
> >> > Actually, there that test already exists in:
> >> >
> >> > _scratch_mkfs
> >> >   _scratch_cleanup_files
> >> >     _overlay_base_scratch_mount
> >> >       _check_mounted_on
> >
> > Hmm, I don't think this kind of basic config sanity check belongs here,
> > this should be done in config and env setup time. (So I think
> > _overlay_mount should be fixed too, that _supports_filetype check
> > doesn't belong there either.)
> >
> > How about adding these checks in init_rc, along with other
> > _check_mounted_on checks against TEST_DEV and SCRATCH_DEV?
> >
> 
> Yes, that makes sense.
> 
> But I still wonder how "exit 1" from within helpers should be handled
> from check when stdout/err are redirected to $tmp.err.
> Trying to catch the config error earlier is a good practice, but
> it won't ensure against the same type of problem in the future.
> 
> What did you think about my approach to store mkfs output in  $check_err
> var instead of $tmp.err file and spew $check_err in _wrapup?
> BTW I tries 'cat $tmp.err' in _wrapup, but output is still redirected to
> $tmp.err while in trap, so cat says: "cat: input file is output file".

I don't think we need to worry about that or handle it. IMO, if we
redirect a helper that does "exit", we're doing something wrong, so
either fix the redirection or fix the helper (just return not exit).

I went through helpers in common/rc roughly, except the _overlay_mount
issue, seems we don't have other such problems right now :)

Thanks,
Eryu

Re: [PATCH v3 1/9] fstests: sanity check that test partitions are not mounted elsewhere

[Amir Goldstein]

On Tue, Feb 14, 2017 at 9:23 AM, Eryu Guan <eguan@redhat.com> wrote:
> On Tue, Feb 14, 2017 at 08:02:27AM +0200, Amir Goldstein wrote:
> [snip]
>> >> >> My test configs look like:
>> >> >>
>> >> >> TEST_DEV=/dev/sda5
>> >> >> SCRATCH_DEV=/dev/sda6
>> >> >> TEST_DIR=/mnt/testarea/test
>> >> >> SCRATCH_MNT=/mnt/testarea/scratch
>> >> >>
>> >> >> and if I mount SCRATCH_DEV at /mnt/xfs (or some other mountpoints rather
>> >> >> than SCRATCH_MNT), ./check -overlay overlay/??? isn't able to detect
>> >> >> this mis-configuration.
>> >> >>
>> >> >> [root@dhcp-66-86-11 xfstests]# ./check  -overlay overlay/002
>> >> >> FSTYP         -- overlay
>> >> >> PLATFORM      -- Linux/x86_64 dhcp-66-86-11 4.10.0-rc7
>> >> >> MKFS_OPTIONS  -- /mnt/testarea/scratch
>> >> >> MOUNT_OPTIONS -- -o context=system_u:object_r:nfs_t:s0 -o lowerdir=/mnt/testarea/scratch/ovl-lower,upperdir=/mnt/testarea/scratch/ovl-upper,workdir=/mnt/testarea/scratch/ovl-work
>> >> >>
>> >> >> [root@dhcp-66-86-11 xfstests]#
>> >> >>
>> >> >> And nothing useful was printed. This is because my rootfs has no
>> >> >> filetype support, but the _notrun message is redirected to a file in
>> >> >> check, as
>> >> >>
>> >> >> "if ! _scratch_mkfs >$tmp.err 2>&1"
>> >> >>
>> >> >> Adding _check_mounted_on against OVL_BASE_TEST/SCRATCH_DEV here could
>> >> >> fix it for me.
>> >> >>
>> >> >
>> >> > Actually, there that test already exists in:
>> >> >
>> >> > _scratch_mkfs
>> >> >   _scratch_cleanup_files
>> >> >     _overlay_base_scratch_mount
>> >> >       _check_mounted_on
>> >
>> > Hmm, I don't think this kind of basic config sanity check belongs here,
>> > this should be done in config and env setup time. (So I think
>> > _overlay_mount should be fixed too, that _supports_filetype check
>> > doesn't belong there either.)
>> >
>> > How about adding these checks in init_rc, along with other
>> > _check_mounted_on checks against TEST_DEV and SCRATCH_DEV?
>> >
>>
>> Yes, that makes sense.
>>
>> But I still wonder how "exit 1" from within helpers should be handled
>> from check when stdout/err are redirected to $tmp.err.
>> Trying to catch the config error earlier is a good practice, but
>> it won't ensure against the same type of problem in the future.
>>
>> What did you think about my approach to store mkfs output in  $check_err
>> var instead of $tmp.err file and spew $check_err in _wrapup?
>> BTW I tries 'cat $tmp.err' in _wrapup, but output is still redirected to
>> $tmp.err while in trap, so cat says: "cat: input file is output file".
>
> I don't think we need to worry about that or handle it. IMO, if we
> redirect a helper that does "exit", we're doing something wrong, so
> either fix the redirection or fix the helper (just return not exit).
>
> I went through helpers in common/rc roughly, except the _overlay_mount
> issue, seems we don't have other such problems right now :)
>

OK. less work for me :)
I'll just fix the helper then.

Re: [PATCH v3 1/9] fstests: sanity check that test partitions are not mounted elsewhere

[Amir Goldstein]

On Tue, Feb 14, 2017 at 7:51 AM, Eryu Guan <eguan@redhat.com> wrote:
> On Mon, Feb 13, 2017 at 03:33:23PM +0200, Amir Goldstein wrote:
>> On Mon, Feb 13, 2017 at 1:44 PM, Amir Goldstein <amir73il@gmail.com> wrote:
>> > On Mon, Feb 13, 2017 at 1:10 PM, Eryu Guan <eguan@redhat.com> wrote:
>> >> On Sun, Feb 12, 2017 at 10:43:36PM +0200, Amir Goldstein wrote:
>> >>> When $TEST_DEV is mounted at a different location then $TEST_DIR,
>> >>> _require_test() aborts the test with an error:
>> >>>  TEST_DEV=/dev/sda5 is mounted but not on TEST_DIR=/mnt/test
>> >>>
>> >>> There are several problems with current sanity check:
>> >>> 1. the output of the error is mixed into out.bad and hard to see
>> >>> 2. the test partition is unmounted at the end of the test regardless
>> >>>    of the fact that it not pass the sanity that we have exclusivity
>> >>> 3. scratch partition has a similar sanity check in _require_scratch(),
>> >>>    but we may not get to it, because $SCRATCH_DEV is unmounted prior
>> >>>    to running the tests (which could unmount another mount point).
>> >>>
>> >>> To solve all these problems, introduce a helper _check_mounted_on().
>> >>> It checks if a device is mounted on a given mount point and optionally
>> >>> checks the mounted fs type.
>> >>>
>> >>> The sanity checks in _require_scratch() and _require_test() are
>> >>> converted to use the helper and gain the check for correct fs type.
>> >>>
>> >>> The helper is used in init_rc() to sanity check both test and scratch
>> >>> partitions, before tests are run and before $SCRATCH_DEV is unmounted.
>> >>>
>> >>> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
>> >>> ---
>> >>>  common/rc | 83 +++++++++++++++++++++++++++++++++++++--------------------------
>> >>>  1 file changed, 49 insertions(+), 34 deletions(-)
>> >
>> > ...
>> >
>> >> My test configs look like:
>> >>
>> >> TEST_DEV=/dev/sda5
>> >> SCRATCH_DEV=/dev/sda6
>> >> TEST_DIR=/mnt/testarea/test
>> >> SCRATCH_MNT=/mnt/testarea/scratch
>> >>
>> >> and if I mount SCRATCH_DEV at /mnt/xfs (or some other mountpoints rather
>> >> than SCRATCH_MNT), ./check -overlay overlay/??? isn't able to detect
>> >> this mis-configuration.
>> >>
>> >> [root@dhcp-66-86-11 xfstests]# ./check  -overlay overlay/002
>> >> FSTYP         -- overlay
>> >> PLATFORM      -- Linux/x86_64 dhcp-66-86-11 4.10.0-rc7
>> >> MKFS_OPTIONS  -- /mnt/testarea/scratch
>> >> MOUNT_OPTIONS -- -o context=system_u:object_r:nfs_t:s0 -o lowerdir=/mnt/testarea/scratch/ovl-lower,upperdir=/mnt/testarea/scratch/ovl-upper,workdir=/mnt/testarea/scratch/ovl-work
>> >>
>> >> [root@dhcp-66-86-11 xfstests]#
>> >>
>> >> And nothing useful was printed. This is because my rootfs has no
>> >> filetype support, but the _notrun message is redirected to a file in
>> >> check, as
>> >>
>> >> "if ! _scratch_mkfs >$tmp.err 2>&1"
>> >>
>> >> Adding _check_mounted_on against OVL_BASE_TEST/SCRATCH_DEV here could
>> >> fix it for me.
>> >>
>> >
>> > Actually, there that test already exists in:
>> >
>> > _scratch_mkfs
>> >   _scratch_cleanup_files
>> >     _overlay_base_scratch_mount
>> >       _check_mounted_on
>
> Hmm, I don't think this kind of basic config sanity check belongs here,
> this should be done in config and env setup time. (So I think
> _overlay_mount should be fixed too, that _supports_filetype check
> doesn't belong there either.)
>
> How about adding these checks in init_rc, along with other
> _check_mounted_on checks against TEST_DEV and SCRATCH_DEV?
>

Sorry, I tried adding those base fs tests in init_rc, but it got too
complicated.
So in V4, I left the sanity checks inside _overlay_base_scratch_mount and
modified the _check_mounted_on helper to not exit and return error values
which callers acts upon.

[PATCH v3 2/9] fstests: use _test_mount() consistently

[Amir Goldstein]

On start of every test run and on every test, in init_rc() helper,
the test partition is verified to be mounted, or is mounted by
the helper _test_mount().

_test_mount() uses mount options $TEST_FS_MOUNT_OPTS and not
$MOUNT_OPTIONS like _scratch_mount() does.

_test_cycle_mount(), which is called by some tests uses the
_test_mount() helper as well.

Contrary to those cases, in _require_test() helper, if test
partition is not mounted, the helper _mount_or_remount_rw()
is called to mount the test partition with $MOUNT_OPTIONS.
Although this should never happen, because of the test in
init_rc(), this case is inconsistent with the rest of the code,
so it has been changed to use _test_mount() as it should.

When running tests with a multi section configuration, and
either FSTYP or MOUNT_OPTIONS change between sections, the
helper _test_unmount() is called to unmount the old test mount
and then _mount_or_remount_rw() is called to mount it again
with new FSTYP and/or MOUNT_OPTIONS.
This is again inconsistent with the rest of the code, so
was changed to use _test_mount() and instead of checking
if MOUNT_OPTIONS have changed between sections, we check if
TEST_FS_MOUNT_OPTS were changed between sections.
Otherwise, we can leave the test partition mounted.

This change is needed to support overlay base fs mount
and for multi section config files which include overlay FSTYP.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 check     | 22 ++++++++++------------
 common/rc |  6 +++---
 2 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/check b/check
index 5a93c94..ec9798a 100755
--- a/check
+++ b/check
@@ -474,7 +474,7 @@ fi
 
 for section in $HOST_OPTIONS_SECTIONS; do
 	OLD_FSTYP=$FSTYP
-	OLD_MOUNT_OPTIONS=$MOUNT_OPTIONS
+	OLD_TEST_FS_MOUNT_OPTS=$TEST_FS_MOUNT_OPTS
 	get_next_config $section
 
 	# Do we need to run only some sections ?
@@ -527,20 +527,18 @@ for section in $HOST_OPTIONS_SECTIONS; do
 			status=1
 			exit
 		fi
-		out=`_mount_or_remount_rw "$MOUNT_OPTIONS" $TEST_DEV $TEST_DIR`
-		if [ $? -ne 1 ]; then
-			echo $out
-			status=1
-			exit
+		if ! _test_mount
+		then
+			echo "check: failed to mount $TEST_DEV on $TEST_DIR"
+			exit 1
 		fi
 		_prepare_test_list
-	elif [ "$OLD_MOUNT_OPTIONS" != "$MOUNT_OPTIONS" ]; then
+	elif [ "$OLD_TEST_FS_MOUNT_OPTS" != "$TEST_FS_MOUNT_OPTS" ]; then
 		_test_unmount 2> /dev/null
-		out=`_mount_or_remount_rw "$MOUNT_OPTIONS" $TEST_DEV $TEST_DIR`
-		if [ $? -ne 1 ]; then
-			echo $out
-			status=1
-			exit
+		if ! _test_mount
+		then
+			echo "check: failed to mount $TEST_DEV on $TEST_DIR"
+			exit 1
 		fi
 	fi
 
diff --git a/common/rc b/common/rc
index d831b17..00afb31 100644
--- a/common/rc
+++ b/common/rc
@@ -1485,9 +1485,9 @@ _require_test()
 
     if ! _check_mounted_on TEST_DEV $TEST_DEV TEST_DIR $TEST_DIR
     then
-	out=`_mount_or_remount_rw "$MOUNT_OPTIONS" $TEST_DEV $TEST_DIR`
-	if [ $? -ne 1 ]; then
-		echo $out
+	if ! _test_mount
+	then
+		echo "!!! failed to mount $TEST_DEV on $TEST_DIR"
 		exit 1
 	fi
     fi
-- 
2.7.4

Re: [PATCH v3 2/9] fstests: use _test_mount() consistently

[Eryu Guan]

On Sun, Feb 12, 2017 at 10:43:37PM +0200, Amir Goldstein wrote:
> On start of every test run and on every test, in init_rc() helper,
> the test partition is verified to be mounted, or is mounted by
> the helper _test_mount().
> 
> _test_mount() uses mount options $TEST_FS_MOUNT_OPTS and not
> $MOUNT_OPTIONS like _scratch_mount() does.
> 
> _test_cycle_mount(), which is called by some tests uses the
> _test_mount() helper as well.
> 
> Contrary to those cases, in _require_test() helper, if test
> partition is not mounted, the helper _mount_or_remount_rw()
> is called to mount the test partition with $MOUNT_OPTIONS.
> Although this should never happen, because of the test in
> init_rc(), this case is inconsistent with the rest of the code,
> so it has been changed to use _test_mount() as it should.
> 
> When running tests with a multi section configuration, and
> either FSTYP or MOUNT_OPTIONS change between sections, the
> helper _test_unmount() is called to unmount the old test mount
> and then _mount_or_remount_rw() is called to mount it again
> with new FSTYP and/or MOUNT_OPTIONS.
> This is again inconsistent with the rest of the code, so
> was changed to use _test_mount() and instead of checking
> if MOUNT_OPTIONS have changed between sections, we check if
> TEST_FS_MOUNT_OPTS were changed between sections.
> Otherwise, we can leave the test partition mounted.
> 
> This change is needed to support overlay base fs mount
> and for multi section config files which include overlay FSTYP.
> 
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
>  check     | 22 ++++++++++------------
>  common/rc |  6 +++---
>  2 files changed, 13 insertions(+), 15 deletions(-)
> 
> diff --git a/check b/check
> index 5a93c94..ec9798a 100755
> --- a/check
> +++ b/check
> @@ -474,7 +474,7 @@ fi
>  
>  for section in $HOST_OPTIONS_SECTIONS; do
>  	OLD_FSTYP=$FSTYP
> -	OLD_MOUNT_OPTIONS=$MOUNT_OPTIONS
> +	OLD_TEST_FS_MOUNT_OPTS=$TEST_FS_MOUNT_OPTS
>  	get_next_config $section
>  
>  	# Do we need to run only some sections ?
> @@ -527,20 +527,18 @@ for section in $HOST_OPTIONS_SECTIONS; do
>  			status=1
>  			exit
>  		fi
> -		out=`_mount_or_remount_rw "$MOUNT_OPTIONS" $TEST_DEV $TEST_DIR`
> -		if [ $? -ne 1 ]; then
> -			echo $out
> -			status=1
> -			exit
> +		if ! _test_mount
> +		then
> +			echo "check: failed to mount $TEST_DEV on $TEST_DIR"
> +			exit 1

Have to set status=1 before exiting here and below, otherwise return
value of check is zero even when error happens. See commit a276c261c9c

Thanks,
Eryu

>  		fi
>  		_prepare_test_list
> -	elif [ "$OLD_MOUNT_OPTIONS" != "$MOUNT_OPTIONS" ]; then
> +	elif [ "$OLD_TEST_FS_MOUNT_OPTS" != "$TEST_FS_MOUNT_OPTS" ]; then
>  		_test_unmount 2> /dev/null
> -		out=`_mount_or_remount_rw "$MOUNT_OPTIONS" $TEST_DEV $TEST_DIR`
> -		if [ $? -ne 1 ]; then
> -			echo $out
> -			status=1
> -			exit
> +		if ! _test_mount
> +		then
> +			echo "check: failed to mount $TEST_DEV on $TEST_DIR"
> +			exit 1
>  		fi
>  	fi
>  
> diff --git a/common/rc b/common/rc
> index d831b17..00afb31 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -1485,9 +1485,9 @@ _require_test()
>  
>      if ! _check_mounted_on TEST_DEV $TEST_DEV TEST_DIR $TEST_DIR
>      then
> -	out=`_mount_or_remount_rw "$MOUNT_OPTIONS" $TEST_DEV $TEST_DIR`
> -	if [ $? -ne 1 ]; then
> -		echo $out
> +	if ! _test_mount
> +	then
> +		echo "!!! failed to mount $TEST_DEV on $TEST_DIR"
>  		exit 1
>  	fi
>      fi
> -- 
> 2.7.4
> 

[PATCH v3 3/9] fstests: canonicalize mount points on every config section

[Amir Goldstein]

Create helper _canonicalize_mountpoint() to check and canonicalize
a mount point path.
Use helper to canonicalize TEST_DIR and SCRATCH_MNT after parse
of every config section.

This is needed for overlay base fs mount.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 common/config | 34 +++++++++++++++++++++-------------
 1 file changed, 21 insertions(+), 13 deletions(-)

diff --git a/common/config b/common/config
index fa89f42..bfd6429 100644
--- a/common/config
+++ b/common/config
@@ -454,6 +454,20 @@ _check_device()
 	fi
 }
 
+# check and return a canonical mount point path
+_canonicalize_mountpoint()
+{
+	local name=$1
+	local dir=$2
+
+	if [ ! -d "$dir" ]; then
+		_fatal "common/config: $name ($dir) is not a directory"
+	fi
+
+	# this follows symlinks and removes all trailing "/"s
+	readlink -e "$dir"
+}
+
 # Parse config section options. This function will parse all the configuration
 # within a single section which name is passed as an argument. For section
 # name format see comments in get_config_sections().
@@ -542,10 +556,7 @@ get_next_config() {
 	fi
 
 	_check_device TEST_DEV required $TEST_DEV
-	if [ ! -d "$TEST_DIR" ]; then
-		echo "common/config: Error: \$TEST_DIR ($TEST_DIR) is not a directory"
-		exit 1
-	fi
+	export TEST_DIR=`_canonicalize_mountpoint TEST_DIR $TEST_DIR`
 
 	# a btrfs tester will set only SCRATCH_DEV_POOL, we will put first of its dev
 	# to SCRATCH_DEV and rest to SCRATCH_DEV_POOL to maintain the backward compatibility
@@ -560,10 +571,7 @@ get_next_config() {
 	fi
 
 	_check_device SCRATCH_DEV optional $SCRATCH_DEV
-	if [ ! -z "$SCRATCH_MNT" -a ! -d "$SCRATCH_MNT" ]; then
-		echo "common/config: Error: \$SCRATCH_MNT ($SCRATCH_MNT) is not a directory"
-		exit 1
-	fi
+	export SCRATCH_MNT=`_canonicalize_mountpoint SCRATCH_MNT $SCRATCH_MNT`
 
 	if [ -n "$USE_EXTERNAL" ]; then
 		_check_device TEST_RTDEV optional $TEST_RTDEV
@@ -590,12 +598,12 @@ if [ -z "$CONFIG_INCLUDED" ]; then
 	[ -z "$TEST_FS_MOUNT_OPTS" ] && _test_mount_opts
 	[ -z "$MKFS_OPTIONS" ] && _mkfs_opts
 	[ -z "$FSCK_OPTIONS" ] && _fsck_opts
+else
+	# canonicalize the mount points
+	# this follows symlinks and removes all trailing "/"s
+	export TEST_DIR=`_canonicalize_mountpoint TEST_DIR $TEST_DIR`
+	export SCRATCH_MNT=`_canonicalize_mountpoint SCRATCH_MNT $SCRATCH_MNT`
 fi
 
-# canonicalize the mount points
-# this follows symlinks and removes all trailing "/"s
-export TEST_DIR=`readlink -e "$TEST_DIR"`
-export SCRATCH_MNT=`readlink -e "$SCRATCH_MNT"`
-
 # make sure this script returns success
 /bin/true
-- 
2.7.4

[PATCH v3 4/9] overlay: rename OVERLAY_LOWER/UPPER/WORK_DIR

[Amir Goldstein]

As preparation step for configuring test/scratch base fs
for overlay tests:

- Rename OVERLAY_LOWER/UPPER/WORK_DIR => OVL_LOWER/UPPER/WORK
  because we want to use OVL_ prefix for all base fs vars

- Prepend "ovl-" prefix to lower/upper/work path values to
  isolate the overlay test dirs when running on a base fs
  that is also used to run non overlay tests

- Make those vars values constant, because lower/upper/work
  directory names are an internal test detail which should
  not concern the user and because we wish to simplify
  and document the overlay tests setup

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 common/config     |  8 +++++---
 common/rc         | 19 +++++++++++++------
 tests/overlay/001 |  2 +-
 tests/overlay/002 |  2 +-
 tests/overlay/003 |  2 +-
 tests/overlay/004 |  4 ++--
 tests/overlay/006 | 10 +++++-----
 tests/overlay/008 |  8 ++++----
 tests/overlay/009 |  2 +-
 tests/overlay/010 |  8 ++++----
 tests/overlay/011 |  4 ++--
 tests/overlay/012 |  4 ++--
 tests/overlay/013 |  4 ++--
 tests/overlay/015 |  2 +-
 tests/overlay/016 |  2 +-
 tests/overlay/017 |  2 +-
 tests/overlay/018 |  2 +-
 tests/overlay/019 |  2 +-
 tests/overlay/020 |  2 +-
 tests/overlay/021 |  2 +-
 20 files changed, 50 insertions(+), 41 deletions(-)

diff --git a/common/config b/common/config
index bfd6429..a7ab2d9 100644
--- a/common/config
+++ b/common/config
@@ -77,9 +77,11 @@ export XFS_MKFS_OPTIONS=${XFS_MKFS_OPTIONS:=-bsize=4096}
 export TIME_FACTOR=${TIME_FACTOR:=1}
 export LOAD_FACTOR=${LOAD_FACTOR:=1}
 export DEBUGFS_MNT=${DEBUGFS_MNT:="/sys/kernel/debug"}
-export OVERLAY_UPPER_DIR=${OVERLAY_UPPER_DIR:="upper"}
-export OVERLAY_LOWER_DIR=${OVERLAY_LOWER_DIR:="lower"}
-export OVERLAY_WORK_DIR=${OVERLAY_WORK_DIR:="work"}
+
+# some constants for overlayfs setup
+export OVL_UPPER="ovl-upper"
+export OVL_LOWER="ovl-lower"
+export OVL_WORK="ovl-work"
 
 export PWD=`pwd`
 #export DEBUG=${DEBUG:=...} # arbitrary CFLAGS really.
diff --git a/common/rc b/common/rc
index 00afb31..2827ff4 100644
--- a/common/rc
+++ b/common/rc
@@ -242,7 +242,7 @@ _common_dev_mount_options()
 
 _overlay_basic_mount_options()
 {
-	echo "-o lowerdir=$1/$OVERLAY_LOWER_DIR,upperdir=$1/$OVERLAY_UPPER_DIR,workdir=$1/$OVERLAY_WORK_DIR"
+	echo "-o lowerdir=$1/$OVL_LOWER,upperdir=$1/$OVL_UPPER,workdir=$1/$OVL_WORK"
 }
 
 _overlay_mount_options()
@@ -301,6 +301,15 @@ _overlay_mount_dirs()
 		    -o workdir=$workdir $*
 }
 
+_overlay_mkdirs()
+{
+	local dir=$1
+
+	mkdir -p $dir/$OVL_UPPER
+	mkdir -p $dir/$OVL_LOWER
+	mkdir -p $dir/$OVL_WORK
+}
+
 # Given a dir, set up 3 subdirectories and mount on the given mnt.
 # The dir is used as the mount device so it can be seen from df or mount
 _overlay_mount()
@@ -311,12 +320,10 @@ _overlay_mount()
 
 	_supports_filetype $dir || _notrun "upper fs needs to support d_type"
 
-	mkdir -p $dir/$OVERLAY_UPPER_DIR
-	mkdir -p $dir/$OVERLAY_LOWER_DIR
-	mkdir -p $dir/$OVERLAY_WORK_DIR
+	_overlay_mkdirs $dir
 
-	_overlay_mount_dirs $dir/$OVERLAY_LOWER_DIR $dir/$OVERLAY_UPPER_DIR \
-			    $dir/$OVERLAY_WORK_DIR $OVERLAY_MOUNT_OPTIONS \
+	_overlay_mount_dirs $dir/$OVL_LOWER $dir/$OVL_UPPER \
+			    $dir/$OVL_WORK $OVERLAY_MOUNT_OPTIONS \
 			    $SELINUX_MOUNT_OPTIONS $* $dir $mnt
 }
 
diff --git a/tests/overlay/001 b/tests/overlay/001
index 7be9ae5..099ddd5 100755
--- a/tests/overlay/001
+++ b/tests/overlay/001
@@ -60,7 +60,7 @@ _scratch_mkfs
 _require_fs_space $SCRATCH_DEV $((4*1024*1024*2 + 8))
 
 # Create test files with different sizes in lower dir
-lowerdir=$SCRATCH_DEV/$OVERLAY_LOWER_DIR
+lowerdir=$SCRATCH_DEV/$OVL_LOWER
 mkdir -p $lowerdir
 touch $lowerdir/zero_size
 $XFS_IO_PROG -fc "pwrite 0 4k" $lowerdir/less_than_4g >>$seqres.full
diff --git a/tests/overlay/002 b/tests/overlay/002
index ec7874e..eaf9a91 100755
--- a/tests/overlay/002
+++ b/tests/overlay/002
@@ -59,7 +59,7 @@ _require_scratch
 _scratch_mkfs
 
 # Create our test file.
-lowerdir=$SCRATCH_DEV/$OVERLAY_LOWER_DIR
+lowerdir=$SCRATCH_DEV/$OVL_LOWER
 mkdir -p $lowerdir
 touch $lowerdir/foobar
 
diff --git a/tests/overlay/003 b/tests/overlay/003
index fbc242a..5e610c8 100755
--- a/tests/overlay/003
+++ b/tests/overlay/003
@@ -60,7 +60,7 @@ _require_scratch
 _scratch_mkfs
 
 # Create test files dirs in lower dir
-lowerdir=${SCRATCH_DEV}/${OVERLAY_LOWER_DIR}
+lowerdir=${SCRATCH_DEV}/${OVL_LOWER}
 mkdir -p $lowerdir
 
 touch ${lowerdir}/test_file
diff --git a/tests/overlay/004 b/tests/overlay/004
index 4075094..8da5170 100755
--- a/tests/overlay/004
+++ b/tests/overlay/004
@@ -53,8 +53,8 @@ _require_user
 _scratch_mkfs
 
 # Create test file in lower dir
-lowerdir=${SCRATCH_DEV}/${OVERLAY_LOWER_DIR}
-upperdir=${SCRATCH_DEV}/${OVERLAY_UPPER_DIR}
+lowerdir=${SCRATCH_DEV}/${OVL_LOWER}
+upperdir=${SCRATCH_DEV}/${OVL_UPPER}
 mkdir -p $lowerdir
 touch ${lowerdir}/attr_file1
 chmod 000 ${lowerdir}/attr_file1
diff --git a/tests/overlay/006 b/tests/overlay/006
index 31f11ef..55fc2dd 100755
--- a/tests/overlay/006
+++ b/tests/overlay/006
@@ -57,15 +57,15 @@ echo "Silence is golden"
 _scratch_mkfs
 
 # Create test file/dir before mount
-mkdir $SCRATCH_DEV/$OVERLAY_LOWER_DIR
-mkdir $SCRATCH_DEV/$OVERLAY_UPPER_DIR
-touch $SCRATCH_DEV/$OVERLAY_LOWER_DIR/lowertestfile
-mkdir $SCRATCH_DEV/$OVERLAY_UPPER_DIR/uppertestdir
+mkdir -p $SCRATCH_DEV/$OVL_LOWER
+mkdir -p $SCRATCH_DEV/$OVL_UPPER
+touch $SCRATCH_DEV/$OVL_LOWER/lowertestfile
+mkdir $SCRATCH_DEV/$OVL_UPPER/uppertestdir
 
 _scratch_mount
 
 # rename lowertestfile to uppertestdir, this triggers copyup and creates
-# whiteout in $OVERLAY_UPPER_DIR
+# whiteout in $OVL_UPPER
 mv $SCRATCH_MNT/lowertestfile $SCRATCH_MNT/uppertestdir
 # the lowertestfile can be removed
 rm $SCRATCH_MNT/uppertestdir/lowertestfile
diff --git a/tests/overlay/008 b/tests/overlay/008
index cb8667c..b81cff8 100755
--- a/tests/overlay/008
+++ b/tests/overlay/008
@@ -56,14 +56,14 @@ _require_user
 _scratch_mkfs
 
 # Create test file on lower dir, and chown to fsgqa user
-lowerdir=$SCRATCH_DEV/$OVERLAY_LOWER_DIR
-mkdir $lowerdir
+lowerdir=$SCRATCH_DEV/$OVL_LOWER
+mkdir -p $lowerdir
 touch $lowerdir/testfile
 chown fsgqa:fsgqa $lowerdir/testfile
 
 # chown upperdir to fsgqa user, so new file/dir can be created by the user
-upperdir=$SCRATCH_DEV/$OVERLAY_UPPER_DIR
-mkdir $upperdir
+upperdir=$SCRATCH_DEV/$OVL_UPPER
+mkdir -p $upperdir
 chown fsgqa:fsgqa $upperdir
 
 _scratch_mount
diff --git a/tests/overlay/009 b/tests/overlay/009
index de94ca4..bdac6b7 100755
--- a/tests/overlay/009
+++ b/tests/overlay/009
@@ -54,7 +54,7 @@ _require_scratch
 _scratch_mkfs
 
 # Create test file in lowerdir
-lowerdir=$SCRATCH_DEV/$OVERLAY_LOWER_DIR
+lowerdir=$SCRATCH_DEV/$OVL_LOWER
 mkdir -p $lowerdir
 touch $lowerdir/testfile
 
diff --git a/tests/overlay/010 b/tests/overlay/010
index a302d74..5882db9 100755
--- a/tests/overlay/010
+++ b/tests/overlay/010
@@ -55,10 +55,10 @@ _scratch_mkfs
 
 # Need two lower dirs in this test, and we mount overlay ourselves,
 # create upper and workdir as well
-lowerdir1=$SCRATCH_DEV/$OVERLAY_LOWER_DIR.1
-lowerdir2=$SCRATCH_DEV/$OVERLAY_LOWER_DIR.2
-upperdir=$SCRATCH_DEV/$OVERLAY_UPPER_DIR
-workdir=$SCRATCH_DEV/$OVERLAY_WORK_DIR
+lowerdir1=$SCRATCH_DEV/$OVL_LOWER.1
+lowerdir2=$SCRATCH_DEV/$OVL_LOWER.2
+upperdir=$SCRATCH_DEV/$OVL_UPPER
+workdir=$SCRATCH_DEV/$OVL_WORK
 mkdir -p $lowerdir1 $lowerdir2 $upperdir $workdir
 
 # One lowerdir contains test dir and test files, the other contains whiteout
diff --git a/tests/overlay/011 b/tests/overlay/011
index 4b697b8..1d6c44a 100755
--- a/tests/overlay/011
+++ b/tests/overlay/011
@@ -58,8 +58,8 @@ _require_attrs
 _scratch_mkfs
 
 # Create test dir on upper and make it opaque by setting proper xattr
-lowerdir=$SCRATCH_DEV/$OVERLAY_LOWER_DIR
-upperdir=$SCRATCH_DEV/$OVERLAY_UPPER_DIR
+lowerdir=$SCRATCH_DEV/$OVL_LOWER
+upperdir=$SCRATCH_DEV/$OVL_UPPER
 mkdir -p $lowerdir/testdir
 mkdir -p $upperdir/testdir
 $SETFATTR_PROG -n "trusted.overlay.opaque" -v "y" $upperdir/testdir
diff --git a/tests/overlay/012 b/tests/overlay/012
index cfe16f2..1c4f7c6 100755
--- a/tests/overlay/012
+++ b/tests/overlay/012
@@ -55,8 +55,8 @@ _require_scratch
 # remove all files from previous runs
 _scratch_mkfs
 
-lowerdir=$SCRATCH_DEV/$OVERLAY_LOWER_DIR
-upperdir=$SCRATCH_DEV/$OVERLAY_UPPER_DIR
+lowerdir=$SCRATCH_DEV/$OVL_LOWER
+upperdir=$SCRATCH_DEV/$OVL_UPPER
 mkdir -p $lowerdir/test
 
 _scratch_mount
diff --git a/tests/overlay/013 b/tests/overlay/013
index e99e10a..536e981 100755
--- a/tests/overlay/013
+++ b/tests/overlay/013
@@ -54,8 +54,8 @@ _require_test_program "t_truncate_self"
 _scratch_mkfs
 
 # copy test program to lower and upper dir
-lowerdir=$SCRATCH_DEV/$OVERLAY_LOWER_DIR
-upperdir=$SCRATCH_DEV/$OVERLAY_UPPER_DIR
+lowerdir=$SCRATCH_DEV/$OVL_LOWER
+upperdir=$SCRATCH_DEV/$OVL_UPPER
 mkdir -p $lowerdir
 mkdir -p $upperdir
 cp $here/src/t_truncate_self $lowerdir/test_lower
diff --git a/tests/overlay/015 b/tests/overlay/015
index c39caed..0e09e0c 100755
--- a/tests/overlay/015
+++ b/tests/overlay/015
@@ -57,7 +57,7 @@ _scratch_mkfs
 umask 022
 
 # Create test dir in lower dir and set sgid bit
-lowerdir=$SCRATCH_DEV/$OVERLAY_LOWER_DIR
+lowerdir=$SCRATCH_DEV/$OVL_LOWER
 mkdir -p $lowerdir/dir
 chown $qa_user:$qa_group $lowerdir/dir
 chmod 2775 $lowerdir/dir
diff --git a/tests/overlay/016 b/tests/overlay/016
index cffcde7..c678ea4 100755
--- a/tests/overlay/016
+++ b/tests/overlay/016
@@ -57,7 +57,7 @@ rm -f $seqres.full
 _scratch_mkfs >>$seqres.full 2>&1
 
 # Create our test files.
-lowerdir=$SCRATCH_DEV/$OVERLAY_LOWER_DIR
+lowerdir=$SCRATCH_DEV/$OVL_LOWER
 mkdir -p $lowerdir
 echo "This is old news" > $lowerdir/foo
 echo "This is old news" > $lowerdir/bar
diff --git a/tests/overlay/017 b/tests/overlay/017
index 5330de2..6b12722 100755
--- a/tests/overlay/017
+++ b/tests/overlay/017
@@ -60,7 +60,7 @@ _scratch_mkfs >>$seqres.full 2>&1
 # Not dealing with hardlinks here, because there is more to test
 # then stable inode number.
 # Hardlinks will get a test of their own.
-lowerdir=$SCRATCH_DEV/$OVERLAY_LOWER_DIR
+lowerdir=$SCRATCH_DEV/$OVL_LOWER
 mkdir -p $lowerdir
 mkdir $lowerdir/dir
 touch $lowerdir/file
diff --git a/tests/overlay/018 b/tests/overlay/018
index df631fc..37bf11c 100755
--- a/tests/overlay/018
+++ b/tests/overlay/018
@@ -55,7 +55,7 @@ rm -f $seqres.full
 _scratch_mkfs >>$seqres.full 2>&1
 
 # Create 2 hardlinked files in lower
-lowerdir=$SCRATCH_DEV/$OVERLAY_LOWER_DIR
+lowerdir=$SCRATCH_DEV/$OVL_LOWER
 mkdir -p $lowerdir
 echo "patient zero" >> $lowerdir/foo
 ln $lowerdir/foo $lowerdir/bar
diff --git a/tests/overlay/019 b/tests/overlay/019
index 41ce63b..9177acb 100755
--- a/tests/overlay/019
+++ b/tests/overlay/019
@@ -51,7 +51,7 @@ _require_scratch
 # Remove all files from previous tests
 _scratch_mkfs
 
-lowerdir=$SCRATCH_DEV/$OVERLAY_LOWER_DIR
+lowerdir=$SCRATCH_DEV/$OVL_LOWER
 mkdir -p $lowerdir
 
 _scratch_mount
diff --git a/tests/overlay/020 b/tests/overlay/020
index 4afd40a..3cd0f05 100755
--- a/tests/overlay/020
+++ b/tests/overlay/020
@@ -55,7 +55,7 @@ _require_scratch
 # Remove all files from previous tests
 _scratch_mkfs
 
-lowerdir=$SCRATCH_DEV/$OVERLAY_LOWER_DIR
+lowerdir=$SCRATCH_DEV/$OVL_LOWER
 mkdir -p $lowerdir/dir
 
 _scratch_mount
diff --git a/tests/overlay/021 b/tests/overlay/021
index c79bb01..c6d68aa 100755
--- a/tests/overlay/021
+++ b/tests/overlay/021
@@ -60,7 +60,7 @@ _scratch_mkfs
 # conservative and reserve space for 16 data copy ups per directory.
 _require_fs_space $SCRATCH_DEV $((16*(16+32+64+128)*1024))
 
-lowerdir=$SCRATCH_DEV/$OVERLAY_LOWER_DIR
+lowerdir=$SCRATCH_DEV/$OVL_LOWER
 mkdir -p $lowerdir
 
 testdir=arena
-- 
2.7.4

[PATCH v3 5/9] overlay: allow SCRATCH_DEV to be the base fs mount point

[Amir Goldstein]

When configure SCRATCH_DEV to a mount point (and not to a directory therein)
then user will get a false positive error in scratch tests:

 $SCRATCH_DEV=/mnt/base/scratch is mounted but not on $SCRATCH_MNT=/mnt/scratch
 Already mounted result:
 /dev/sda6 on /mnt/base/scratch type xfs (rw,relatime,attr2,inode64,noquota)

This is due to the wrong `grep -F $SCRATCH_DEV` which matches the mount
point instead of the device in that mount.
Fix _check_mounted_on() to grep the pattern "$dev on " and "$dev on $mnt"
instead of just grepping for "$dev" and "$mnt" without the " on " anchor.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 common/rc | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/common/rc b/common/rc
index 2827ff4..4e0cbdf 100644
--- a/common/rc
+++ b/common/rc
@@ -1340,12 +1340,14 @@ _check_mounted_on()
 	local type=$5
 
 	# Note that we use -F here so grep doesn't try to interpret an NFS over
-	# IPv6 server as a regular expression
-	local mount_rec=`_mount | grep -F "$dev"`
+	# IPv6 server as a regular expression.  Because of that, we cannot use
+	# ^$dev so we use "$dev on " to avoid matching $dev to mount point field
+	# for overlay case, where $dev is a directory.
+	local mount_rec=`_mount | grep -F "$dev on "`
 	[ -n "$mount_rec" ] || return 1
 
 	# if it's mounted, make sure its on $mnt
-	if ! (echo $mount_rec | grep -q "$mnt")
+	if ! (echo $mount_rec | grep -q "$dev on $mnt")
 	then
 		echo "$devname=$dev is mounted but not on $mntname=$mnt - aborting"
 		echo "Already mounted result:"
-- 
2.7.4

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

[Amir Goldstein]

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>
---
 README        | 16 ++++++-----
 check         |  2 +-
 common/config | 85 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
 common/rc     | 23 +++++++++-------
 4 files changed, 105 insertions(+), 21 deletions(-)

diff --git a/README b/README
index bb42985..842e835 100644
--- a/README
+++ b/README
@@ -117,16 +117,18 @@ Running tests:
     - By default the tests suite will run xfs tests:
     - ./check '*/001' '*/002' '*/003'
     - ./check '*/06?'
-    - You can explicitly specify NFS/CIFS/UDF, otherwise the filesystem type will
-      be autodetected from $TEST_DEV:
-      ./check -nfs [test(s)]
     - Groups of tests maybe ran by: ./check -g [group(s)]
       See the 'group' file for details on groups
-    - for udf tests: ./check -udf [test(s)]
-      Running all the udf tests: ./check -udf -g udf
-    - for running nfs tests: ./check -nfs [test(s)]
-    - for running cifs/smb3 tests: ./check -cifs [test(s)]
     - To randomize test order: ./check -r [test(s)]
+    - You can explicitly specify NFS/CIFS/UDF/OVERLAY, otherwise
+      the filesystem type will be autodetected from $TEST_DEV:
+        - for running nfs tests: ./check -nfs [test(s)]
+        - for running cifs/smb3 tests: ./check -cifs [test(s)]
+        - for udf tests: ./check -udf [test(s)]
+          Running all the udf tests: ./check -udf -g udf
+        - for overlay tests: ./check -overlay [test(s)]
+          The TEST and SCRATCH partitions should be pre-formatted
+          with another base fs, where the overlay dirs will be created
 
     
     The check script tests the return value of each script, and
diff --git a/check b/check
index ec9798a..c51fc0c 100755
--- a/check
+++ b/check
@@ -258,7 +258,7 @@ while [ $# -gt 0 ]; do
 
 	-nfs)		FSTYP=nfs ;;
 	-cifs)		FSTYP=cifs ;;
-	-overlay)	FSTYP=overlay ;;
+	-overlay)	FSTYP=overlay; export OVERLAY=true ;;
 	-tmpfs)		FSTYP=tmpfs ;;
 
 	-g)	group=$2 ; shift ;
diff --git a/common/config b/common/config
index a7ab2d9..5d90138 100644
--- a/common/config
+++ b/common/config
@@ -82,6 +82,8 @@ export DEBUGFS_MNT=${DEBUGFS_MNT:="/sys/kernel/debug"}
 export OVL_UPPER="ovl-upper"
 export OVL_LOWER="ovl-lower"
 export OVL_WORK="ovl-work"
+# overlay mount point parent must be the base fs root
+export OVL_MNT="ovl-mnt"
 
 export PWD=`pwd`
 #export DEBUG=${DEBUG:=...} # arbitrary CFLAGS really.
@@ -462,12 +464,68 @@ _canonicalize_mountpoint()
 	local name=$1
 	local dir=$2
 
-	if [ ! -d "$dir" ]; then
+	if [ -d "$dir" ]; then
+		# this follows symlinks and removes all trailing "/"s
+		readlink -e "$dir"
+		return 0
+	fi
+
+	if [ "$FSTYP" != "overlay" ] || [[ "$name" == OVL_BASE_* ]]; then
 		_fatal "common/config: $name ($dir) is not a directory"
 	fi
 
-	# this follows symlinks and removes all trailing "/"s
-	readlink -e "$dir"
+	# base fs may not be mounted yet, so just check that parent dir
+	# exists (where base fs will be mounted) because we are going to
+	# mkdir the overlay mount point dir anyway
+	local base=`basename $dir`
+	local parent=`dirname $dir`
+	local parent=`_canonicalize_mountpoint OVL_BASE_$name "$parent"`
+
+	# prepend the overlay mount point to canonical parent path
+	echo "$parent/$base"
+}
+
+_config_overlay()
+{
+	# There are 2 options for configuring overlayfs tests:
+	#
+	# 1. (legacy) SCRATCH/TEST_DEV point to existing directories
+	#    on an already mounted fs.  In this case, the new
+	#    OVL_BASE_SCRATCH/TEST_* vars are set to use the legacy
+	#    vars values (even though they may not be mount points).
+	#
+	[ ! -d "$TEST_DEV" ] || export OVL_BASE_TEST_DIR="$TEST_DEV"
+	[ ! -d "$SCRATCH_DEV" ] || export OVL_BASE_SCRATCH_MNT="$SCRATCH_DEV"
+
+	# 2. SCRATCH/TEST_DEV point to the base fs partitions.  In this case,
+	#    the new OVL_BASE_SCRATCH/TEST_DEV/MNT vars are set to the values
+	#    of the configured base fs and SCRATCH/TEST_DEV vars are set to the
+	#    overlayfs base and mount dirs inside base fs mount.
+	[ -b "$TEST_DEV" ] || return 0
+
+	# Config file may specify base fs type, but we obay -overlay flag
+	export OVL_BASE_FSTYP="$FSTYP"
+	export FSTYP=overlay
+
+	# Store original base fs vars
+	export OVL_BASE_TEST_DEV="$TEST_DEV"
+	export OVL_BASE_TEST_DIR=`_canonicalize_mountpoint OVL_BASE_TEST_DIR $TEST_DIR`
+	export OVL_BASE_MOUNT_OPTIONS="$MOUNT_OPTIONS"
+
+	# Set TEST vars to overlay base and mount dirs inside base fs
+	export TEST_DEV="$OVL_BASE_TEST_DIR"
+	export TEST_DIR="$OVL_BASE_TEST_DIR/$OVL_MNT"
+	export MOUNT_OPTIONS="$OVERLAY_MOUNT_OPTIONS"
+
+	[ -b "$SCRATCH_DEV" ] || return 0
+
+	# Store original base fs vars
+	export OVL_BASE_SCRATCH_DEV="$SCRATCH_DEV"
+	export OVL_BASE_SCRATCH_MNT=`_canonicalize_mountpoint OVL_BASE_SCRATCH_MNT $SCRATCH_MNT`
+
+	# Set SCRATCH vars to overlay base and mount dirs inside base fs
+	export SCRATCH_DEV="$OVL_BASE_SCRATCH_MNT"
+	export SCRATCH_MNT="$OVL_BASE_SCRATCH_MNT/$OVL_MNT"
 }
 
 # 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
+
 	#  Mandatory Config values.
 	MC=""
 	[ -z "$EMAIL" ]          && MC="$MC EMAIL"
@@ -601,6 +674,12 @@ if [ -z "$CONFIG_INCLUDED" ]; then
 	[ -z "$MKFS_OPTIONS" ] && _mkfs_opts
 	[ -z "$FSCK_OPTIONS" ] && _fsck_opts
 else
+	# 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
+
 	# canonicalize the mount points
 	# this follows symlinks and removes all trailing "/"s
 	export TEST_DIR=`_canonicalize_mountpoint TEST_DIR $TEST_DIR`
diff --git a/common/rc b/common/rc
index 4e0cbdf..74168ea 100644
--- a/common/rc
+++ b/common/rc
@@ -257,7 +257,7 @@ _scratch_mount_options()
 	_scratch_options mount
 
 	if [ "$FSTYP" == "overlay" ]; then
-		echo `_overlay_mount_options $SCRATCH_DEV`
+		echo `_overlay_mount_options $OVL_BASE_SCRATCH_MNT`
 		return 0
 	fi
 	echo `_common_dev_mount_options $*` $SCRATCH_OPTIONS \
@@ -308,9 +308,10 @@ _overlay_mkdirs()
 	mkdir -p $dir/$OVL_UPPER
 	mkdir -p $dir/$OVL_LOWER
 	mkdir -p $dir/$OVL_WORK
+	mkdir -p $dir/$OVL_MNT
 }
 
-# Given a dir, set up 3 subdirectories and mount on the given mnt.
+# Given a base fs dir, set up overlay directories and mount on the given mnt.
 # The dir is used as the mount device so it can be seen from df or mount
 _overlay_mount()
 {
@@ -329,12 +330,12 @@ _overlay_mount()
 
 _overlay_test_mount()
 {
-	_overlay_mount $TEST_DEV $TEST_DIR $*
+	_overlay_mount $OVL_BASE_TEST_DIR $TEST_DIR $*
 }
 
 _overlay_scratch_mount()
 {
-	_overlay_mount $SCRATCH_DEV $SCRATCH_MNT $*
+	_overlay_mount $OVL_BASE_SCRATCH_MNT $SCRATCH_MNT $*
 }
 
 _overlay_test_unmount()
@@ -649,10 +650,12 @@ _scratch_cleanup_files()
 {
 	case $FSTYP in
 	overlay)
-		# $SCRATCH_DEV is a valid directory in overlay case
-		rm -rf $SCRATCH_DEV/*
+		# Avoid rm -rf /* if we messed up
+		[ -n "$OVL_BASE_SCRATCH_MNT" ] || return 1
+		rm -rf $OVL_BASE_SCRATCH_MNT/*
 		;;
 	*)
+		[ -n "$SCRATCH_MNT" ] || return 1
 		_scratch_mount
 		rm -rf $SCRATCH_MNT/*
 		_scratch_unmount
@@ -1390,8 +1393,8 @@ _require_scratch_nocheck()
 		fi
 		;;
 	overlay)
-		if [ -z "$SCRATCH_DEV" -o ! -d "$SCRATCH_DEV" ]; then
-			_notrun "this test requires a valid \$SCRATCH_DEV as ovl base dir"
+		if [ -z "$OVL_BASE_SCRATCH_MNT" -o ! -d "$OVL_BASE_SCRATCH_MNT" ]; then
+			_notrun "this test requires a valid \$SCRATCH_MNT as ovl base dir"
 		fi
 		if [ ! -d "$SCRATCH_MNT" ]; then
 			_notrun "this test requires a valid \$SCRATCH_MNT"
@@ -1463,8 +1466,8 @@ _require_test()
 		fi
 		;;
 	overlay)
-		if [ -z "$TEST_DEV" -o ! -d "$TEST_DEV" ]; then
-			_notrun "this test requires a valid \$TEST_DEV as ovl base dir"
+		if [ -z "$OVL_BASE_TEST_DIR" -o ! -d "$OVL_BASE_TEST_DIR" ]; then
+			_notrun "this test requires a valid \$TEST_DIR as ovl base dir"
 		fi
 		if [ ! -d "$TEST_DIR" ]; then
 			_notrun "this test requires a valid \$TEST_DIR"
-- 
2.7.4

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

[Eryu Guan]

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>
> ---
>  README        | 16 ++++++-----
>  check         |  2 +-
>  common/config | 85 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
>  common/rc     | 23 +++++++++-------
>  4 files changed, 105 insertions(+), 21 deletions(-)
> 
> diff --git a/README b/README
> index bb42985..842e835 100644
> --- a/README
> +++ b/README
> @@ -117,16 +117,18 @@ Running tests:
>      - By default the tests suite will run xfs tests:
>      - ./check '*/001' '*/002' '*/003'
>      - ./check '*/06?'
> -    - You can explicitly specify NFS/CIFS/UDF, otherwise the filesystem type will
> -      be autodetected from $TEST_DEV:
> -      ./check -nfs [test(s)]
>      - Groups of tests maybe ran by: ./check -g [group(s)]
>        See the 'group' file for details on groups
> -    - for udf tests: ./check -udf [test(s)]
> -      Running all the udf tests: ./check -udf -g udf
> -    - for running nfs tests: ./check -nfs [test(s)]
> -    - for running cifs/smb3 tests: ./check -cifs [test(s)]
>      - To randomize test order: ./check -r [test(s)]
> +    - You can explicitly specify NFS/CIFS/UDF/OVERLAY, otherwise
> +      the filesystem type will be autodetected from $TEST_DEV:
> +        - for running nfs tests: ./check -nfs [test(s)]
> +        - for running cifs/smb3 tests: ./check -cifs [test(s)]
> +        - for udf tests: ./check -udf [test(s)]
> +          Running all the udf tests: ./check -udf -g udf
> +        - for overlay tests: ./check -overlay [test(s)]
> +          The TEST and SCRATCH partitions should be pre-formatted
> +          with another base fs, where the overlay dirs will be created
>  
>      
>      The check script tests the return value of each script, and
> diff --git a/check b/check
> index ec9798a..c51fc0c 100755
> --- a/check
> +++ b/check
> @@ -258,7 +258,7 @@ while [ $# -gt 0 ]; do
>  
>  	-nfs)		FSTYP=nfs ;;
>  	-cifs)		FSTYP=cifs ;;
> -	-overlay)	FSTYP=overlay ;;
> +	-overlay)	FSTYP=overlay; export OVERLAY=true ;;
>  	-tmpfs)		FSTYP=tmpfs ;;
>  
>  	-g)	group=$2 ; shift ;
> diff --git a/common/config b/common/config
> index a7ab2d9..5d90138 100644
> --- a/common/config
> +++ b/common/config
> @@ -82,6 +82,8 @@ export DEBUGFS_MNT=${DEBUGFS_MNT:="/sys/kernel/debug"}
>  export OVL_UPPER="ovl-upper"
>  export OVL_LOWER="ovl-lower"
>  export OVL_WORK="ovl-work"
> +# overlay mount point parent must be the base fs root
> +export OVL_MNT="ovl-mnt"
>  
>  export PWD=`pwd`
>  #export DEBUG=${DEBUG:=...} # arbitrary CFLAGS really.
> @@ -462,12 +464,68 @@ _canonicalize_mountpoint()
>  	local name=$1
>  	local dir=$2
>  
> -	if [ ! -d "$dir" ]; then
> +	if [ -d "$dir" ]; then
> +		# this follows symlinks and removes all trailing "/"s
> +		readlink -e "$dir"
> +		return 0
> +	fi
> +
> +	if [ "$FSTYP" != "overlay" ] || [[ "$name" == OVL_BASE_* ]]; then
>  		_fatal "common/config: $name ($dir) is not a directory"
>  	fi
>  
> -	# this follows symlinks and removes all trailing "/"s
> -	readlink -e "$dir"
> +	# base fs may not be mounted yet, so just check that parent dir
> +	# exists (where base fs will be mounted) because we are going to
> +	# mkdir the overlay mount point dir anyway
> +	local base=`basename $dir`
> +	local parent=`dirname $dir`
> +	local parent=`_canonicalize_mountpoint OVL_BASE_$name "$parent"`
> +
> +	# prepend the overlay mount point to canonical parent path
> +	echo "$parent/$base"
> +}
> +
> +_config_overlay()
> +{
> +	# There are 2 options for configuring overlayfs tests:
> +	#
> +	# 1. (legacy) SCRATCH/TEST_DEV point to existing directories
> +	#    on an already mounted fs.  In this case, the new
> +	#    OVL_BASE_SCRATCH/TEST_* vars are set to use the legacy
> +	#    vars values (even though they may not be mount points).
> +	#
> +	[ ! -d "$TEST_DEV" ] || export OVL_BASE_TEST_DIR="$TEST_DEV"
> +	[ ! -d "$SCRATCH_DEV" ] || export OVL_BASE_SCRATCH_MNT="$SCRATCH_DEV"
> +
> +	# 2. SCRATCH/TEST_DEV point to the base fs partitions.  In this case,
> +	#    the new OVL_BASE_SCRATCH/TEST_DEV/MNT vars are set to the values
> +	#    of the configured base fs and SCRATCH/TEST_DEV vars are set to the
> +	#    overlayfs base and mount dirs inside base fs mount.
> +	[ -b "$TEST_DEV" ] || return 0
> +
> +	# Config file may specify base fs type, but we obay -overlay flag
> +	export OVL_BASE_FSTYP="$FSTYP"
> +	export FSTYP=overlay
> +
> +	# Store original base fs vars
> +	export OVL_BASE_TEST_DEV="$TEST_DEV"
> +	export OVL_BASE_TEST_DIR=`_canonicalize_mountpoint OVL_BASE_TEST_DIR $TEST_DIR`
> +	export OVL_BASE_MOUNT_OPTIONS="$MOUNT_OPTIONS"
> +
> +	# Set TEST vars to overlay base and mount dirs inside base fs
> +	export TEST_DEV="$OVL_BASE_TEST_DIR"
> +	export TEST_DIR="$OVL_BASE_TEST_DIR/$OVL_MNT"
> +	export MOUNT_OPTIONS="$OVERLAY_MOUNT_OPTIONS"
> +
> +	[ -b "$SCRATCH_DEV" ] || return 0
> +
> +	# Store original base fs vars
> +	export OVL_BASE_SCRATCH_DEV="$SCRATCH_DEV"
> +	export OVL_BASE_SCRATCH_MNT=`_canonicalize_mountpoint OVL_BASE_SCRATCH_MNT $SCRATCH_MNT`
> +
> +	# Set SCRATCH vars to overlay base and mount dirs inside base fs
> +	export SCRATCH_DEV="$OVL_BASE_SCRATCH_MNT"
> +	export SCRATCH_MNT="$OVL_BASE_SCRATCH_MNT/$OVL_MNT"
>  }
>  
>  # 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.

Thanks,
Eryu

>  	#  Mandatory Config values.
>  	MC=""
>  	[ -z "$EMAIL" ]          && MC="$MC EMAIL"
> @@ -601,6 +674,12 @@ if [ -z "$CONFIG_INCLUDED" ]; then
>  	[ -z "$MKFS_OPTIONS" ] && _mkfs_opts
>  	[ -z "$FSCK_OPTIONS" ] && _fsck_opts
>  else
> +	# 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
> +
>  	# canonicalize the mount points
>  	# this follows symlinks and removes all trailing "/"s
>  	export TEST_DIR=`_canonicalize_mountpoint TEST_DIR $TEST_DIR`
> diff --git a/common/rc b/common/rc
> index 4e0cbdf..74168ea 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -257,7 +257,7 @@ _scratch_mount_options()
>  	_scratch_options mount
>  
>  	if [ "$FSTYP" == "overlay" ]; then
> -		echo `_overlay_mount_options $SCRATCH_DEV`
> +		echo `_overlay_mount_options $OVL_BASE_SCRATCH_MNT`
>  		return 0
>  	fi
>  	echo `_common_dev_mount_options $*` $SCRATCH_OPTIONS \
> @@ -308,9 +308,10 @@ _overlay_mkdirs()
>  	mkdir -p $dir/$OVL_UPPER
>  	mkdir -p $dir/$OVL_LOWER
>  	mkdir -p $dir/$OVL_WORK
> +	mkdir -p $dir/$OVL_MNT
>  }
>  
> -# Given a dir, set up 3 subdirectories and mount on the given mnt.
> +# Given a base fs dir, set up overlay directories and mount on the given mnt.
>  # The dir is used as the mount device so it can be seen from df or mount
>  _overlay_mount()
>  {
> @@ -329,12 +330,12 @@ _overlay_mount()
>  
>  _overlay_test_mount()
>  {
> -	_overlay_mount $TEST_DEV $TEST_DIR $*
> +	_overlay_mount $OVL_BASE_TEST_DIR $TEST_DIR $*
>  }
>  
>  _overlay_scratch_mount()
>  {
> -	_overlay_mount $SCRATCH_DEV $SCRATCH_MNT $*
> +	_overlay_mount $OVL_BASE_SCRATCH_MNT $SCRATCH_MNT $*
>  }
>  
>  _overlay_test_unmount()
> @@ -649,10 +650,12 @@ _scratch_cleanup_files()
>  {
>  	case $FSTYP in
>  	overlay)
> -		# $SCRATCH_DEV is a valid directory in overlay case
> -		rm -rf $SCRATCH_DEV/*
> +		# Avoid rm -rf /* if we messed up
> +		[ -n "$OVL_BASE_SCRATCH_MNT" ] || return 1
> +		rm -rf $OVL_BASE_SCRATCH_MNT/*
>  		;;
>  	*)
> +		[ -n "$SCRATCH_MNT" ] || return 1
>  		_scratch_mount
>  		rm -rf $SCRATCH_MNT/*
>  		_scratch_unmount
> @@ -1390,8 +1393,8 @@ _require_scratch_nocheck()
>  		fi
>  		;;
>  	overlay)
> -		if [ -z "$SCRATCH_DEV" -o ! -d "$SCRATCH_DEV" ]; then
> -			_notrun "this test requires a valid \$SCRATCH_DEV as ovl base dir"
> +		if [ -z "$OVL_BASE_SCRATCH_MNT" -o ! -d "$OVL_BASE_SCRATCH_MNT" ]; then
> +			_notrun "this test requires a valid \$SCRATCH_MNT as ovl base dir"
>  		fi
>  		if [ ! -d "$SCRATCH_MNT" ]; then
>  			_notrun "this test requires a valid \$SCRATCH_MNT"
> @@ -1463,8 +1466,8 @@ _require_test()
>  		fi
>  		;;
>  	overlay)
> -		if [ -z "$TEST_DEV" -o ! -d "$TEST_DEV" ]; then
> -			_notrun "this test requires a valid \$TEST_DEV as ovl base dir"
> +		if [ -z "$OVL_BASE_TEST_DIR" -o ! -d "$OVL_BASE_TEST_DIR" ]; then
> +			_notrun "this test requires a valid \$TEST_DIR as ovl base dir"
>  		fi
>  		if [ ! -d "$TEST_DIR" ]; then
>  			_notrun "this test requires a valid \$TEST_DIR"
> -- 
> 2.7.4
> 

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

[Amir Goldstein]

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.
>

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??

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

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

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

[Eryu Guan]

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

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

[Amir Goldstein]

On Tue, Feb 14, 2017 at 1:03 PM, Eryu Guan <eguan@redhat.com> wrote:
> 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..
>

OK, I think I managed to wrap my head around this.
Next version is going to look simpler:
- _overlay_config_override() at the very end of get_next_config()
- _overlay_config_restore() at the very beginning of get_next_config()

[PATCH v3 7/9] overlay: use OVL_BASE_SCRATCH_MNT instead of SCRATCH_DEV

[Amir Goldstein]

Use the new var OVL_BASE_SCRATCH_MNT to refer to overlay
base dir instead of the legacy SCRATCH_DEV var.

In overlay/011, when using MOUNT_PROG directly, provide
OVL_BASE_SCRATCH_MNT as device argument instead of 'none'.
This fix is required to support mount of base fs.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>

fix OVL_BASE_SCRATCH_MNT
---
 tests/overlay/001 |  7 +++----
 tests/overlay/002 |  2 +-
 tests/overlay/003 |  2 +-
 tests/overlay/004 |  4 ++--
 tests/overlay/005 | 30 +++++++++++++++---------------
 tests/overlay/006 |  8 ++++----
 tests/overlay/008 |  4 ++--
 tests/overlay/009 |  2 +-
 tests/overlay/010 | 10 +++++-----
 tests/overlay/011 |  6 +++---
 tests/overlay/012 |  4 ++--
 tests/overlay/013 |  4 ++--
 tests/overlay/014 | 14 +++++++-------
 tests/overlay/015 |  2 +-
 tests/overlay/016 |  2 +-
 tests/overlay/017 |  2 +-
 tests/overlay/018 |  2 +-
 tests/overlay/019 |  2 +-
 tests/overlay/020 |  2 +-
 tests/overlay/021 |  6 +++---
 20 files changed, 57 insertions(+), 58 deletions(-)

diff --git a/tests/overlay/001 b/tests/overlay/001
index 099ddd5..c0abbfb 100755
--- a/tests/overlay/001
+++ b/tests/overlay/001
@@ -55,12 +55,11 @@ _require_scratch
 _scratch_mkfs
 
 # overlay copy_up doesn't deal with sparse file well, holes will be filled by
-# zeros, so at least (4G + 4G + 8k) free space is needed on $SCRATCH_DEV,
-# where $SCRATCH_DEV is actually a test dir used in overlay testing
-_require_fs_space $SCRATCH_DEV $((4*1024*1024*2 + 8))
+# zeros, so at least (4G + 4G + 8k) free space is needed on scratch base fs
+_require_fs_space $OVL_BASE_SCRATCH_MNT $((4*1024*1024*2 + 8))
 
 # Create test files with different sizes in lower dir
-lowerdir=$SCRATCH_DEV/$OVL_LOWER
+lowerdir=$OVL_BASE_SCRATCH_MNT/$OVL_LOWER
 mkdir -p $lowerdir
 touch $lowerdir/zero_size
 $XFS_IO_PROG -fc "pwrite 0 4k" $lowerdir/less_than_4g >>$seqres.full
diff --git a/tests/overlay/002 b/tests/overlay/002
index eaf9a91..ffdec18 100755
--- a/tests/overlay/002
+++ b/tests/overlay/002
@@ -59,7 +59,7 @@ _require_scratch
 _scratch_mkfs
 
 # Create our test file.
-lowerdir=$SCRATCH_DEV/$OVL_LOWER
+lowerdir=$OVL_BASE_SCRATCH_MNT/$OVL_LOWER
 mkdir -p $lowerdir
 touch $lowerdir/foobar
 
diff --git a/tests/overlay/003 b/tests/overlay/003
index 5e610c8..7c7f41b 100755
--- a/tests/overlay/003
+++ b/tests/overlay/003
@@ -60,7 +60,7 @@ _require_scratch
 _scratch_mkfs
 
 # Create test files dirs in lower dir
-lowerdir=${SCRATCH_DEV}/${OVL_LOWER}
+lowerdir=${OVL_BASE_SCRATCH_MNT}/${OVL_LOWER}
 mkdir -p $lowerdir
 
 touch ${lowerdir}/test_file
diff --git a/tests/overlay/004 b/tests/overlay/004
index 8da5170..bc08f34 100755
--- a/tests/overlay/004
+++ b/tests/overlay/004
@@ -53,8 +53,8 @@ _require_user
 _scratch_mkfs
 
 # Create test file in lower dir
-lowerdir=${SCRATCH_DEV}/${OVL_LOWER}
-upperdir=${SCRATCH_DEV}/${OVL_UPPER}
+lowerdir=${OVL_BASE_SCRATCH_MNT}/${OVL_LOWER}
+upperdir=${OVL_BASE_SCRATCH_MNT}/${OVL_UPPER}
 mkdir -p $lowerdir
 touch ${lowerdir}/attr_file1
 chmod 000 ${lowerdir}/attr_file1
diff --git a/tests/overlay/005 b/tests/overlay/005
index baadb69..f885fdc 100755
--- a/tests/overlay/005
+++ b/tests/overlay/005
@@ -61,31 +61,31 @@ _require_loop
 _scratch_mkfs
 
 # setup loop images fs for overlayfs
-lower_img=${SCRATCH_DEV}/${seq}.$$.img
+lower_img=${OVL_BASE_SCRATCH_MNT}/${seq}.$$.img
 $XFS_IO_PROG -f -c "truncate 120m" $lower_img >>$seqres.full 2>&1
 fs_loop_dev=`_create_loop_device $lower_img`
 $MKFS_XFS_PROG -f -n ftype=1 $fs_loop_dev >>$seqres.full 2>&1
 
 # only 20m for upper dir
-upper_img=${SCRATCH_DEV}/$$.${seq}.img
+upper_img=${OVL_BASE_SCRATCH_MNT}/$$.${seq}.img
 $XFS_IO_PROG -f -c "truncate 20m" $upper_img >>$seqres.full 2>&1
 upper_loop_dev=`_create_loop_device $upper_img`
 $MKFS_XFS_PROG -f -n ftype=1 $upper_loop_dev >>$seqres.full 2>&1
 
 # mount underlying xfs
-mkdir -p ${SCRATCH_DEV}/lowermnt
-mkdir -p ${SCRATCH_DEV}/uppermnt
-$MOUNT_PROG $fs_loop_dev ${SCRATCH_DEV}/lowermnt
-$MOUNT_PROG $upper_loop_dev ${SCRATCH_DEV}/uppermnt
+mkdir -p ${OVL_BASE_SCRATCH_MNT}/lowermnt
+mkdir -p ${OVL_BASE_SCRATCH_MNT}/uppermnt
+$MOUNT_PROG $fs_loop_dev ${OVL_BASE_SCRATCH_MNT}/lowermnt
+$MOUNT_PROG $upper_loop_dev ${OVL_BASE_SCRATCH_MNT}/uppermnt
 
 # prepare dirs
-mkdir -p ${SCRATCH_DEV}/lowermnt/lower
-mkdir -p ${SCRATCH_DEV}/uppermnt/upper
-mkdir -p ${SCRATCH_DEV}/uppermnt/work
+mkdir -p ${OVL_BASE_SCRATCH_MNT}/lowermnt/lower
+mkdir -p ${OVL_BASE_SCRATCH_MNT}/uppermnt/upper
+mkdir -p ${OVL_BASE_SCRATCH_MNT}/uppermnt/work
 
-lowerd=${SCRATCH_DEV}/lowermnt/lower
-upperd=${SCRATCH_DEV}/uppermnt/upper
-workd=${SCRATCH_DEV}/uppermnt/work
+lowerd=${OVL_BASE_SCRATCH_MNT}/lowermnt/lower
+upperd=${OVL_BASE_SCRATCH_MNT}/uppermnt/upper
+workd=${OVL_BASE_SCRATCH_MNT}/uppermnt/work
 
 # Create test file in lower dir, with too big a size for
 # upper dir to copy up.
@@ -93,7 +93,7 @@ $XFS_IO_PROG -f -c "truncate 48m" ${lowerd}/test_file \
 	>>$seqres.full 2>&1
 
 # mount new overlayfs
-_overlay_mount_dirs $lowerd $upperd $workd $SCRATCH_DEV $SCRATCH_MNT
+_overlay_mount_dirs $lowerd $upperd $workd $OVL_BASE_SCRATCH_MNT $SCRATCH_MNT
 
 # the open call triggers copy-up and it will fail ENOSPC
 $XFS_IO_PROG -f -c "o" ${SCRATCH_MNT}/test_file \
@@ -103,8 +103,8 @@ $XFS_IO_PROG -f -c "o" ${SCRATCH_MNT}/test_file \
 $UMOUNT_PROG $SCRATCH_MNT
 
 # unmount undelying xfs, this tiggers panic if memleak happens
-$UMOUNT_PROG ${SCRATCH_DEV}/uppermnt
-$UMOUNT_PROG ${SCRATCH_DEV}/lowermnt
+$UMOUNT_PROG ${OVL_BASE_SCRATCH_MNT}/uppermnt
+$UMOUNT_PROG ${OVL_BASE_SCRATCH_MNT}/lowermnt
 
 # success, all done
 echo "Silence is golden"
diff --git a/tests/overlay/006 b/tests/overlay/006
index 55fc2dd..815657f 100755
--- a/tests/overlay/006
+++ b/tests/overlay/006
@@ -57,10 +57,10 @@ echo "Silence is golden"
 _scratch_mkfs
 
 # Create test file/dir before mount
-mkdir -p $SCRATCH_DEV/$OVL_LOWER
-mkdir -p $SCRATCH_DEV/$OVL_UPPER
-touch $SCRATCH_DEV/$OVL_LOWER/lowertestfile
-mkdir $SCRATCH_DEV/$OVL_UPPER/uppertestdir
+mkdir -p $OVL_BASE_SCRATCH_MNT/$OVL_LOWER
+mkdir -p $OVL_BASE_SCRATCH_MNT/$OVL_UPPER
+touch $OVL_BASE_SCRATCH_MNT/$OVL_LOWER/lowertestfile
+mkdir $OVL_BASE_SCRATCH_MNT/$OVL_UPPER/uppertestdir
 
 _scratch_mount
 
diff --git a/tests/overlay/008 b/tests/overlay/008
index b81cff8..d5bc1df 100755
--- a/tests/overlay/008
+++ b/tests/overlay/008
@@ -56,13 +56,13 @@ _require_user
 _scratch_mkfs
 
 # Create test file on lower dir, and chown to fsgqa user
-lowerdir=$SCRATCH_DEV/$OVL_LOWER
+lowerdir=$OVL_BASE_SCRATCH_MNT/$OVL_LOWER
 mkdir -p $lowerdir
 touch $lowerdir/testfile
 chown fsgqa:fsgqa $lowerdir/testfile
 
 # chown upperdir to fsgqa user, so new file/dir can be created by the user
-upperdir=$SCRATCH_DEV/$OVL_UPPER
+upperdir=$OVL_BASE_SCRATCH_MNT/$OVL_UPPER
 mkdir -p $upperdir
 chown fsgqa:fsgqa $upperdir
 
diff --git a/tests/overlay/009 b/tests/overlay/009
index bdac6b7..f367887 100755
--- a/tests/overlay/009
+++ b/tests/overlay/009
@@ -54,7 +54,7 @@ _require_scratch
 _scratch_mkfs
 
 # Create test file in lowerdir
-lowerdir=$SCRATCH_DEV/$OVL_LOWER
+lowerdir=$OVL_BASE_SCRATCH_MNT/$OVL_LOWER
 mkdir -p $lowerdir
 touch $lowerdir/testfile
 
diff --git a/tests/overlay/010 b/tests/overlay/010
index 5882db9..dee99cf 100755
--- a/tests/overlay/010
+++ b/tests/overlay/010
@@ -55,10 +55,10 @@ _scratch_mkfs
 
 # Need two lower dirs in this test, and we mount overlay ourselves,
 # create upper and workdir as well
-lowerdir1=$SCRATCH_DEV/$OVL_LOWER.1
-lowerdir2=$SCRATCH_DEV/$OVL_LOWER.2
-upperdir=$SCRATCH_DEV/$OVL_UPPER
-workdir=$SCRATCH_DEV/$OVL_WORK
+lowerdir1=$OVL_BASE_SCRATCH_MNT/$OVL_LOWER.1
+lowerdir2=$OVL_BASE_SCRATCH_MNT/$OVL_LOWER.2
+upperdir=$OVL_BASE_SCRATCH_MNT/$OVL_UPPER
+workdir=$OVL_BASE_SCRATCH_MNT/$OVL_WORK
 mkdir -p $lowerdir1 $lowerdir2 $upperdir $workdir
 
 # One lowerdir contains test dir and test files, the other contains whiteout
@@ -68,7 +68,7 @@ mknod $lowerdir2/testdir/a c 0 0
 
 # Mount overlayfs and remove testdir, which led to kernel crash
 _overlay_mount_dirs "$lowerdir2:$lowerdir1" $upperdir $workdir \
-		    $SCRATCH_DEV $SCRATCH_MNT
+		    $OVL_BASE_SCRATCH_MNT $SCRATCH_MNT
 rm -rf $SCRATCH_MNT/testdir
 
 # success, all done
diff --git a/tests/overlay/011 b/tests/overlay/011
index 1d6c44a..87ac17b 100755
--- a/tests/overlay/011
+++ b/tests/overlay/011
@@ -58,8 +58,8 @@ _require_attrs
 _scratch_mkfs
 
 # Create test dir on upper and make it opaque by setting proper xattr
-lowerdir=$SCRATCH_DEV/$OVL_LOWER
-upperdir=$SCRATCH_DEV/$OVL_UPPER
+lowerdir=$OVL_BASE_SCRATCH_MNT/$OVL_LOWER
+upperdir=$OVL_BASE_SCRATCH_MNT/$OVL_UPPER
 mkdir -p $lowerdir/testdir
 mkdir -p $upperdir/testdir
 $SETFATTR_PROG -n "trusted.overlay.opaque" -v "y" $upperdir/testdir
@@ -68,7 +68,7 @@ $SETFATTR_PROG -n "trusted.overlay.opaque" -v "y" $upperdir/testdir
 # $upperdir overlaid on top of $lowerdir, so that "trusted.overlay.opaque"
 # xattr should be honored and should not be listed
 # mount readonly, because there's no upper and workdir
-$MOUNT_PROG -t overlay -o ro -o lowerdir=$upperdir:$lowerdir none $SCRATCH_MNT
+$MOUNT_PROG -t overlay -o ro -o lowerdir=$upperdir:$lowerdir $OVL_BASE_SCRATCH_MNT $SCRATCH_MNT
 
 # Dump trusted.overlay xattr, we should not see the "opaque" xattr
 $GETFATTR_PROG -d -m overlay $SCRATCH_MNT/testdir
diff --git a/tests/overlay/012 b/tests/overlay/012
index 1c4f7c6..9cf1526 100755
--- a/tests/overlay/012
+++ b/tests/overlay/012
@@ -55,8 +55,8 @@ _require_scratch
 # remove all files from previous runs
 _scratch_mkfs
 
-lowerdir=$SCRATCH_DEV/$OVL_LOWER
-upperdir=$SCRATCH_DEV/$OVL_UPPER
+lowerdir=$OVL_BASE_SCRATCH_MNT/$OVL_LOWER
+upperdir=$OVL_BASE_SCRATCH_MNT/$OVL_UPPER
 mkdir -p $lowerdir/test
 
 _scratch_mount
diff --git a/tests/overlay/013 b/tests/overlay/013
index 536e981..de0fc26 100755
--- a/tests/overlay/013
+++ b/tests/overlay/013
@@ -54,8 +54,8 @@ _require_test_program "t_truncate_self"
 _scratch_mkfs
 
 # copy test program to lower and upper dir
-lowerdir=$SCRATCH_DEV/$OVL_LOWER
-upperdir=$SCRATCH_DEV/$OVL_UPPER
+lowerdir=$OVL_BASE_SCRATCH_MNT/$OVL_LOWER
+upperdir=$OVL_BASE_SCRATCH_MNT/$OVL_UPPER
 mkdir -p $lowerdir
 mkdir -p $upperdir
 cp $here/src/t_truncate_self $lowerdir/test_lower
diff --git a/tests/overlay/014 b/tests/overlay/014
index 36d7077..491f735 100755
--- a/tests/overlay/014
+++ b/tests/overlay/014
@@ -61,16 +61,16 @@ _require_scratch
 _scratch_mkfs
 
 # Create multiple lowerdirs and upperdir, workdir, and testdir on lowerdir
-lowerdir1=$SCRATCH_DEV/lower1
-lowerdir2=$SCRATCH_DEV/lower2
-upperdir=$SCRATCH_DEV/upper
-workdir=$SCRATCH_DEV/workdir
+lowerdir1=$OVL_BASE_SCRATCH_MNT/lower1
+lowerdir2=$OVL_BASE_SCRATCH_MNT/lower2
+upperdir=$OVL_BASE_SCRATCH_MNT/upper
+workdir=$OVL_BASE_SCRATCH_MNT/workdir
 mkdir -p $lowerdir1 $lowerdir2 $upperdir $workdir
 mkdir -p $lowerdir1/testdir/d
 
 # mount overlay with $lowerdir2 as upperdir, and remove & recreate testdir,
 # make testdir on $lowerdir2 opaque
-_overlay_mount_dirs $lowerdir1 $lowerdir2 $workdir $SCRATCH_DEV $SCRATCH_MNT
+_overlay_mount_dirs $lowerdir1 $lowerdir2 $workdir $OVL_BASE_SCRATCH_MNT $SCRATCH_MNT
 rm -rf $SCRATCH_MNT/testdir
 mkdir -p $SCRATCH_MNT/testdir/visibledir
 _scratch_unmount
@@ -79,14 +79,14 @@ _scratch_unmount
 # and create a new file in testdir, triggers copyup from lowerdir,
 # copyup should not copy overlayfs private xattr
 _overlay_mount_dirs "$lowerdir2:$lowerdir1" $upperdir $workdir \
-		    $SCRATCH_DEV $SCRATCH_MNT
+		    $OVL_BASE_SCRATCH_MNT $SCRATCH_MNT
 touch $SCRATCH_MNT/testdir/visiblefile
 
 # umount and mount overlay again, buggy kernel treats the copied-up dir as
 # opaque, visibledir is not seen in merged dir.
 _scratch_unmount
 _overlay_mount_dirs "$lowerdir2:$lowerdir1" $upperdir $workdir \
-		    $SCRATCH_DEV $SCRATCH_MNT
+		    $OVL_BASE_SCRATCH_MNT $SCRATCH_MNT
 ls $SCRATCH_MNT/testdir
 
 # success, all done
diff --git a/tests/overlay/015 b/tests/overlay/015
index 0e09e0c..4894097 100755
--- a/tests/overlay/015
+++ b/tests/overlay/015
@@ -57,7 +57,7 @@ _scratch_mkfs
 umask 022
 
 # Create test dir in lower dir and set sgid bit
-lowerdir=$SCRATCH_DEV/$OVL_LOWER
+lowerdir=$OVL_BASE_SCRATCH_MNT/$OVL_LOWER
 mkdir -p $lowerdir/dir
 chown $qa_user:$qa_group $lowerdir/dir
 chmod 2775 $lowerdir/dir
diff --git a/tests/overlay/016 b/tests/overlay/016
index c678ea4..86a7d28 100755
--- a/tests/overlay/016
+++ b/tests/overlay/016
@@ -57,7 +57,7 @@ rm -f $seqres.full
 _scratch_mkfs >>$seqres.full 2>&1
 
 # Create our test files.
-lowerdir=$SCRATCH_DEV/$OVL_LOWER
+lowerdir=$OVL_BASE_SCRATCH_MNT/$OVL_LOWER
 mkdir -p $lowerdir
 echo "This is old news" > $lowerdir/foo
 echo "This is old news" > $lowerdir/bar
diff --git a/tests/overlay/017 b/tests/overlay/017
index 6b12722..5ca3040 100755
--- a/tests/overlay/017
+++ b/tests/overlay/017
@@ -60,7 +60,7 @@ _scratch_mkfs >>$seqres.full 2>&1
 # Not dealing with hardlinks here, because there is more to test
 # then stable inode number.
 # Hardlinks will get a test of their own.
-lowerdir=$SCRATCH_DEV/$OVL_LOWER
+lowerdir=$OVL_BASE_SCRATCH_MNT/$OVL_LOWER
 mkdir -p $lowerdir
 mkdir $lowerdir/dir
 touch $lowerdir/file
diff --git a/tests/overlay/018 b/tests/overlay/018
index 37bf11c..7e47732 100755
--- a/tests/overlay/018
+++ b/tests/overlay/018
@@ -55,7 +55,7 @@ rm -f $seqres.full
 _scratch_mkfs >>$seqres.full 2>&1
 
 # Create 2 hardlinked files in lower
-lowerdir=$SCRATCH_DEV/$OVL_LOWER
+lowerdir=$OVL_BASE_SCRATCH_MNT/$OVL_LOWER
 mkdir -p $lowerdir
 echo "patient zero" >> $lowerdir/foo
 ln $lowerdir/foo $lowerdir/bar
diff --git a/tests/overlay/019 b/tests/overlay/019
index 9177acb..3e2bc4e 100755
--- a/tests/overlay/019
+++ b/tests/overlay/019
@@ -51,7 +51,7 @@ _require_scratch
 # Remove all files from previous tests
 _scratch_mkfs
 
-lowerdir=$SCRATCH_DEV/$OVL_LOWER
+lowerdir=$OVL_BASE_SCRATCH_MNT/$OVL_LOWER
 mkdir -p $lowerdir
 
 _scratch_mount
diff --git a/tests/overlay/020 b/tests/overlay/020
index 3cd0f05..de3816c 100755
--- a/tests/overlay/020
+++ b/tests/overlay/020
@@ -55,7 +55,7 @@ _require_scratch
 # Remove all files from previous tests
 _scratch_mkfs
 
-lowerdir=$SCRATCH_DEV/$OVL_LOWER
+lowerdir=$OVL_BASE_SCRATCH_MNT/$OVL_LOWER
 mkdir -p $lowerdir/dir
 
 _scratch_mount
diff --git a/tests/overlay/021 b/tests/overlay/021
index c6d68aa..1347f52 100755
--- a/tests/overlay/021
+++ b/tests/overlay/021
@@ -54,13 +54,13 @@ _scratch_mkfs
 
 # overlay copy_up doesn't deal with sparse file well, holes will be filled by
 # zeros, so for the worst case of hitting all the copy up bomb files, we need
-# (64*(16+32+64+128)M) free space on $SCRATCH_DEV.
+# (64*(16+32+64+128)M) free space on $OVL_BASE_SCRATCH_MNT.
 # However, triggering more than a total of 16 copy up bombs would require
 # really fast data copy (clone up doesn't take up space at all), so let's be
 # conservative and reserve space for 16 data copy ups per directory.
-_require_fs_space $SCRATCH_DEV $((16*(16+32+64+128)*1024))
+_require_fs_space $OVL_BASE_SCRATCH_MNT $((16*(16+32+64+128)*1024))
 
-lowerdir=$SCRATCH_DEV/$OVL_LOWER
+lowerdir=$OVL_BASE_SCRATCH_MNT/$OVL_LOWER
 mkdir -p $lowerdir
 
 testdir=arena
-- 
2.7.4

[PATCH v3 8/9] overlay: fix test and scratch filters for overlay base fs

[Amir Goldstein]

When configuring overlay base fs, TEST_DEV/DIR and SCRATCH_DEV/MNT
are derived from the base fs mount points, where *_DEV are the
path of the base fs mount point and TEST_DIR/SCRATCH_MNT are
a directory under the base fs mount point.

This means that the overlay DEV paths are prefixes of the overlay
mount points.
Fix the test and sctach filters to try and match TEST_DIR/SCRATCH_MNT
first and only then try and match the shorter *_DEV.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 common/filter | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/common/filter b/common/filter
index 4328159..ef20ea6 100644
--- a/common/filter
+++ b/common/filter
@@ -280,13 +280,14 @@ _filter_xfs_io_pages_modified()
 
 _filter_test_dir()
 {
-	sed -e "s,$TEST_DEV,TEST_DEV,g" -e "s,$TEST_DIR,TEST_DIR,g"
+	sed -e "s,$TEST_DIR,TEST_DIR,g" \
+	    -e "s,$TEST_DEV,TEST_DEV,g"
 }
 
 _filter_scratch()
 {
-	sed -e "s,$SCRATCH_DEV,SCRATCH_DEV,g" \
-	    -e "s,$SCRATCH_MNT,SCRATCH_MNT,g" \
+	sed -e "s,$SCRATCH_MNT,SCRATCH_MNT,g" \
+	    -e "s,$SCRATCH_DEV,SCRATCH_DEV,g" \
 	    -e "/.use_space/d"
 }
 
-- 
2.7.4

Re: [PATCH v3 8/9] overlay: fix test and scratch filters for overlay base fs

[Amir Goldstein]

On Sun, Feb 12, 2017 at 10:43 PM, Amir Goldstein <amir73il@gmail.com> wrote:
> When configuring overlay base fs, TEST_DEV/DIR and SCRATCH_DEV/MNT
> are derived from the base fs mount points, where *_DEV are the
> path of the base fs mount point and TEST_DIR/SCRATCH_MNT are
> a directory under the base fs mount point.
>
> This means that the overlay DEV paths are prefixes of the overlay
> mount points.
> Fix the test and sctach filters to try and match TEST_DIR/SCRATCH_MNT
> first and only then try and match the shorter *_DEV.
>

And who would have thought that MNT could also be a substr of DEV?
Well is kvm-xfstests it is!

I will have to check for echo $SCRATCH_DEV | grep $SCRATCH_MNT
and vice versa explicitly before deciding which filter order to use.

FSTYP         -- xfs (debug)
PLATFORM      -- Linux/x86_64 kvm-xfstests 4.9.0-debug-12264-g60ae0f1
MKFS_OPTIONS  -- -f -m rmapbt=1,reflink=1 /dev/vdc
MOUNT_OPTIONS -- /dev/vdc /vdc

 [20:36:04] - output mismatch (see
/results/xfs/results-reflink/generic/050.out.bad)
    --- tests/generic/050.out 2017-02-13 08:36:18.000000000 +0000
    +++ /results/xfs/results-reflink/generic/050.out.bad 2017-02-13
20:36:04.054172980 +0000
    @@ -1,7 +1,7 @@
     QA output created by 050
     setting device read-only
     mounting read-only block device:
    -mount: SCRATCH_DEV is write-protected, mounting read-only
    +mount: /devSCRATCH_MNT is write-protected, mounting read-only
     touching file on read-only filesystem (should fail)
     touch: cannot touch 'SCRATCH_MNT/foo': Read-only file system
    ...


> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
>  common/filter | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/common/filter b/common/filter
> index 4328159..ef20ea6 100644
> --- a/common/filter
> +++ b/common/filter
> @@ -280,13 +280,14 @@ _filter_xfs_io_pages_modified()
>
>  _filter_test_dir()
>  {
> -       sed -e "s,$TEST_DEV,TEST_DEV,g" -e "s,$TEST_DIR,TEST_DIR,g"
> +       sed -e "s,$TEST_DIR,TEST_DIR,g" \
> +           -e "s,$TEST_DEV,TEST_DEV,g"
>  }
>
>  _filter_scratch()
>  {
> -       sed -e "s,$SCRATCH_DEV,SCRATCH_DEV,g" \
> -           -e "s,$SCRATCH_MNT,SCRATCH_MNT,g" \
> +       sed -e "s,$SCRATCH_MNT,SCRATCH_MNT,g" \
> +           -e "s,$SCRATCH_DEV,SCRATCH_DEV,g" \
>             -e "/.use_space/d"
>  }
>
> --
> 2.7.4
>

[PATCH v3 9/9] overlay: mount/unmount base fs before/after running tests

[Amir Goldstein]

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.

This helps catching overlayfs bugs related to leaking objects in
underlying (base) fs.

To preserve expected tests behavior, the semantics are:
- _scratch_mkfs mounts the base fs, cleans all files, creates
  lower/upper dirs and keeps base fs mounted
- _scratch_mount mounts base fs (if needed) and mounts overlay
- _scratch_unmount unmounts overlay and base fs

Tests that use _scratch_unmount to unmount a custom overlay mount
and expect to have access to overlay base dir, were fixed to use
explicit umount $SCRATCH_MNT instead.

The overlay test itself, does not support formatting the base fs.
However, it is possible to add overlay sections to a multi section
config file after every base fs configuration, to run the overlay
tests with the file system that was created on the test partitions
in the previous section (see example in README.config-sections).

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 README.config-sections |  6 ++++++
 common/rc              | 55 ++++++++++++++++++++++++++++++++++++++++++++++++--
 tests/overlay/003      |  3 ++-
 tests/overlay/004      |  3 ++-
 tests/overlay/014      |  5 +++--
 5 files changed, 66 insertions(+), 6 deletions(-)

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"
 
@@ -112,6 +115,9 @@ MKFS_OPTIONS="-q -F -b4096 -O ^has_journal"
 MKFS_OPTIONS="-f"
 FSTYP=xfs
 
+[xfs_overlay]
+FSTYP=overlay
+
 [ext3_filesystem]
 FSTYP=ext3
 MOUNT_OPTIONS="-o noatime"
diff --git a/common/rc b/common/rc
index 74168ea..e9ac0df 100644
--- a/common/rc
+++ b/common/rc
@@ -328,24 +328,70 @@ _overlay_mount()
 			    $SELINUX_MOUNT_OPTIONS $* $dir $mnt
 }
 
+_overlay_base_test_mount()
+{
+	if [ -z "$OVL_BASE_TEST_DEV" -o -z "$OVL_BASE_TEST_DIR" ] || \
+		_check_mounted_on OVL_BASE_TEST_DEV $OVL_BASE_TEST_DEV \
+				OVL_BASE_TEST_DIR $OVL_BASE_TEST_DIR
+	then
+		return 0
+	fi
+
+	_mount $OVL_BASE_MOUNT_OPTIONS \
+		$SELINUX_MOUNT_OPTIONS \
+		$OVL_BASE_TEST_DEV $OVL_BASE_TEST_DIR
+}
+
 _overlay_test_mount()
 {
-	_overlay_mount $OVL_BASE_TEST_DIR $TEST_DIR $*
+	_overlay_base_test_mount && \
+		_overlay_mount $OVL_BASE_TEST_DIR $TEST_DIR $*
+}
+
+_overlay_base_scratch_mount()
+{
+	if [ -z "$OVL_BASE_SCRATCH_DEV" -o -z "$OVL_BASE_SCRATCH_MNT" ] || \
+		_check_mounted_on OVL_BASE_SCRATCH_DEV $OVL_BASE_SCRATCH_DEV \
+				OVL_BASE_SCRATCH_MNT $OVL_BASE_SCRATCH_MNT
+	then
+		return 0
+	fi
+
+	_mount $OVL_BASE_MOUNT_OPTIONS \
+		$SELINUX_MOUNT_OPTIONS \
+		$OVL_BASE_SCRATCH_DEV $OVL_BASE_SCRATCH_MNT
+}
+
+_overlay_base_scratch_unmount()
+{
+	[ -n "$OVL_BASE_SCRATCH_DEV" -a -n "$OVL_BASE_SCRATCH_MNT" ] || return 0
+
+	$UMOUNT_PROG $OVL_BASE_SCRATCH_MNT
 }
 
 _overlay_scratch_mount()
 {
-	_overlay_mount $OVL_BASE_SCRATCH_MNT $SCRATCH_MNT $*
+	_overlay_base_scratch_mount && \
+		_overlay_mount $OVL_BASE_SCRATCH_MNT $SCRATCH_MNT $*
+}
+
+_overlay_base_test_unmount()
+{
+	[ -n "$OVL_BASE_TEST_DEV" -a -n "$OVL_BASE_TEST_DIR" ] || return 0
+
+	$UMOUNT_PROG $OVL_BASE_TEST_DIR
 }
 
 _overlay_test_unmount()
 {
 	$UMOUNT_PROG $TEST_DIR
+	_overlay_base_test_unmount
 }
 
 _overlay_scratch_unmount()
 {
 	$UMOUNT_PROG $SCRATCH_MNT
+	_overlay_base_scratch_unmount
 }
 
 _scratch_mount()
@@ -652,7 +698,12 @@ _scratch_cleanup_files()
 	overlay)
 		# Avoid rm -rf /* if we messed up
 		[ -n "$OVL_BASE_SCRATCH_MNT" ] || return 1
+		# overlay 'mkfs' needs to make sure base fs is mounted and clean
+		_overlay_base_scratch_unmount 2>/dev/null
+		_overlay_base_scratch_mount
 		rm -rf $OVL_BASE_SCRATCH_MNT/*
+		_overlay_mkdirs $OVL_BASE_SCRATCH_MNT
+		# leave base fs mouted so tests can setup lower/upper dir files
 		;;
 	*)
 		[ -n "$SCRATCH_MNT" ] || return 1
diff --git a/tests/overlay/003 b/tests/overlay/003
index 7c7f41b..f980edb 100755
--- a/tests/overlay/003
+++ b/tests/overlay/003
@@ -89,7 +89,8 @@ rm -rf ${SCRATCH_MNT}/*
 # nothing should be listed
 ls ${SCRATCH_MNT}/
 
-_scratch_unmount
+# unmount overlayfs but not base fs
+$UMOUNT_PROG $SCRATCH_MNT
 
 rm -rf $lowerdir
 echo "Silence is golden"
diff --git a/tests/overlay/004 b/tests/overlay/004
index bc08f34..611847a 100755
--- a/tests/overlay/004
+++ b/tests/overlay/004
@@ -85,7 +85,8 @@ _user_do "chmod g+t ${SCRATCH_MNT}/attr_file2 > /dev/null 2>&1"
 _user_do "chmod u-X ${SCRATCH_MNT}/attr_file2 > /dev/null 2>&1"
 stat -c %a ${SCRATCH_MNT}/attr_file2
 
-_scratch_unmount
+# unmount overlayfs but not base fs
+$UMOUNT_PROG $SCRATCH_MNT
 
 # check mode bits of the file that has been copied up, and
 # the file that should not have been copied up.
diff --git a/tests/overlay/014 b/tests/overlay/014
index 491f735..653b7d6 100755
--- a/tests/overlay/014
+++ b/tests/overlay/014
@@ -73,7 +73,8 @@ mkdir -p $lowerdir1/testdir/d
 _overlay_mount_dirs $lowerdir1 $lowerdir2 $workdir $OVL_BASE_SCRATCH_MNT $SCRATCH_MNT
 rm -rf $SCRATCH_MNT/testdir
 mkdir -p $SCRATCH_MNT/testdir/visibledir
-_scratch_unmount
+# unmount overlayfs but not base fs
+$UMOUNT_PROG $SCRATCH_MNT
 
 # mount overlay again, with lowerdir1 and lowerdir2 as multiple lowerdirs,
 # and create a new file in testdir, triggers copyup from lowerdir,
@@ -84,7 +85,7 @@ touch $SCRATCH_MNT/testdir/visiblefile
 
 # umount and mount overlay again, buggy kernel treats the copied-up dir as
 # opaque, visibledir is not seen in merged dir.
-_scratch_unmount
+$UMOUNT_PROG $SCRATCH_MNT
 _overlay_mount_dirs "$lowerdir2:$lowerdir1" $upperdir $workdir \
 		    $OVL_BASE_SCRATCH_MNT $SCRATCH_MNT
 ls $SCRATCH_MNT/testdir
-- 
2.7.4

Re: [PATCH v3 9/9] overlay: mount/unmount base fs before/after running tests

[Eryu Guan]

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.
> 
> This helps catching overlayfs bugs related to leaking objects in
> underlying (base) fs.
> 
> To preserve expected tests behavior, the semantics are:
> - _scratch_mkfs mounts the base fs, cleans all files, creates
>   lower/upper dirs and keeps base fs mounted
> - _scratch_mount mounts base fs (if needed) and mounts overlay
> - _scratch_unmount unmounts overlay and base fs
> 
> Tests that use _scratch_unmount to unmount a custom overlay mount
> and expect to have access to overlay base dir, were fixed to use
> explicit umount $SCRATCH_MNT instead.
> 
> The overlay test itself, does not support formatting the base fs.
> However, it is possible to add overlay sections to a multi section
> config file after every base fs configuration, to run the overlay
> tests with the file system that was created on the test partitions
> in the previous section (see example in README.config-sections).
> 
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
>  README.config-sections |  6 ++++++
>  common/rc              | 55 ++++++++++++++++++++++++++++++++++++++++++++++++--
>  tests/overlay/003      |  3 ++-
>  tests/overlay/004      |  3 ++-
>  tests/overlay/014      |  5 +++--
>  5 files changed, 66 insertions(+), 6 deletions(-)
> 
> 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"
>  
> @@ -112,6 +115,9 @@ MKFS_OPTIONS="-q -F -b4096 -O ^has_journal"
>  MKFS_OPTIONS="-f"
>  FSTYP=xfs
>  
> +[xfs_overlay]
> +FSTYP=overlay
> +
>  [ext3_filesystem]
>  FSTYP=ext3
>  MOUNT_OPTIONS="-o noatime"
> diff --git a/common/rc b/common/rc
> index 74168ea..e9ac0df 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -328,24 +328,70 @@ _overlay_mount()
>  			    $SELINUX_MOUNT_OPTIONS $* $dir $mnt
>  }
>  
> +_overlay_base_test_mount()
> +{
> +	if [ -z "$OVL_BASE_TEST_DEV" -o -z "$OVL_BASE_TEST_DIR" ] || \
> +		_check_mounted_on OVL_BASE_TEST_DEV $OVL_BASE_TEST_DEV \
> +				OVL_BASE_TEST_DIR $OVL_BASE_TEST_DIR
> +	then
> +		return 0
> +	fi
> +
> +	_mount $OVL_BASE_MOUNT_OPTIONS \

OVL_BASE_MOUNT_OPTIONS is from MOUNT_OPTIONS, which is used for scratch
mount. Do we need a ovl base counter part of TEST_FS_MOUNT_OPTS?

Thanks,
Eryu

Re: [PATCH v3 9/9] overlay: mount/unmount base fs before/after running tests

[Amir Goldstein]

On Mon, Feb 13, 2017 at 1:31 PM, Eryu Guan <eguan@redhat.com> wrote:
> 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.
>>
>> This helps catching overlayfs bugs related to leaking objects in
>> underlying (base) fs.
>>
>> To preserve expected tests behavior, the semantics are:
>> - _scratch_mkfs mounts the base fs, cleans all files, creates
>>   lower/upper dirs and keeps base fs mounted
>> - _scratch_mount mounts base fs (if needed) and mounts overlay
>> - _scratch_unmount unmounts overlay and base fs
>>
>> Tests that use _scratch_unmount to unmount a custom overlay mount
>> and expect to have access to overlay base dir, were fixed to use
>> explicit umount $SCRATCH_MNT instead.
>>
>> The overlay test itself, does not support formatting the base fs.
>> However, it is possible to add overlay sections to a multi section
>> config file after every base fs configuration, to run the overlay
>> tests with the file system that was created on the test partitions
>> in the previous section (see example in README.config-sections).
>>
>> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
>> ---
>>  README.config-sections |  6 ++++++
>>  common/rc              | 55 ++++++++++++++++++++++++++++++++++++++++++++++++--
>>  tests/overlay/003      |  3 ++-
>>  tests/overlay/004      |  3 ++-
>>  tests/overlay/014      |  5 +++--
>>  5 files changed, 66 insertions(+), 6 deletions(-)
>>
>> 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"
>>
>> @@ -112,6 +115,9 @@ MKFS_OPTIONS="-q -F -b4096 -O ^has_journal"
>>  MKFS_OPTIONS="-f"
>>  FSTYP=xfs
>>
>> +[xfs_overlay]
>> +FSTYP=overlay
>> +
>>  [ext3_filesystem]
>>  FSTYP=ext3
>>  MOUNT_OPTIONS="-o noatime"
>> diff --git a/common/rc b/common/rc
>> index 74168ea..e9ac0df 100644
>> --- a/common/rc
>> +++ b/common/rc
>> @@ -328,24 +328,70 @@ _overlay_mount()
>>                           $SELINUX_MOUNT_OPTIONS $* $dir $mnt
>>  }
>>
>> +_overlay_base_test_mount()
>> +{
>> +     if [ -z "$OVL_BASE_TEST_DEV" -o -z "$OVL_BASE_TEST_DIR" ] || \
>> +             _check_mounted_on OVL_BASE_TEST_DEV $OVL_BASE_TEST_DEV \
>> +                             OVL_BASE_TEST_DIR $OVL_BASE_TEST_DIR
>> +     then
>> +             return 0
>> +     fi
>> +
>> +     _mount $OVL_BASE_MOUNT_OPTIONS \
>
> OVL_BASE_MOUNT_OPTIONS is from MOUNT_OPTIONS, which is used for scratch
> mount. Do we need a ovl base counter part of TEST_FS_MOUNT_OPTS?
>

I suggest that we change this line to:
+     _mount $TEST_FS_MOUNT_OPTS \

MOUNT_OPTIONS in config is used for base scratch mount options
TEST_FS_MOUNT_OPTS in config is used for base test mount options.
OVERLAY_MOUNT_OPTIONS in config is used for both scratch and test overlay mounts

The only case where TEST_FS_MOUNT_OPTS is configured not by user is
for cifs and cheph (in _test_mount_opts) and I don't think that those fs
can be used as overlay base fs. can they?

Good enough?

Re: [PATCH v3 9/9] overlay: mount/unmount base fs before/after running tests

[Theodore Ts'o]

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.

Re: [PATCH v3 9/9] overlay: mount/unmount base fs before/after running tests

[Eryu Guan]

On Mon, Feb 13, 2017 at 07:23:56PM -0500, Theodore Ts'o wrote:
[snip]
> 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.

I agreed, the section configs are not perfect, sometimes I have
SCRATCH_RTDEV "leaked" to other non-rt test sections. I was considering
fixing this problem, but haven't got enough time to figure out a
reasonable idea yet.

> 
> 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

I'm considering making the old way unsupported eventually at some point,
after a long enough soak time. But if people still want it, I can live
with it too :)

> 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

With this update, things get simpler if you're not using multi-section
config file, there's an example in commit log of patch 6/9, README is
also updated, but seems we need that kind of detailed example in README
too.

"
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'...
"

> description of how things like the mkfs mount options, etc. are side
> effected by previous config sections, and how to set up overlayfs

FWIW, Multi-section configs are optional not mandatory for testing
overlayfs. But I agreed that we need more documentation.

> 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

Yes, four paramters are not carried across sections, they're unset at
the beginning of each section. In common/config we have

        unset MOUNT_OPTIONS                                                                                                                                                                    
        unset MKFS_OPTIONS                                                                                                                                                                     
        unset FSCK_OPTIONS                                                                                                                                                                     
        unset USE_EXTERNAL

Thanks,
Eryu

Re: [PATCH v3 9/9] overlay: mount/unmount base fs before/after running tests

[Amir Goldstein]

On Tue, Feb 14, 2017 at 2:23 AM, Theodore Ts'o <tytso@mit.edu> wrote:
> 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:
>

Indeed. Multi section config does not give the tester super power.
Tester needs to know what she is doing.
As I replied on the linux-fsdevel thread, I found out that interleaving
overlay sections post non-overlay sections work and thought
it may be useful for some, so I added the example.
I do not intend to fix problems with multi section config + overlay
so I may as well remove that example and leave this a useful but
undocumented feature.

Having written that disclaimer, I will go on to answer your questions.

>> 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?
>

Correct. The only thing that overlay does not do is format the base fs,
as the README says:
        - for overlay tests: ./check -overlay [test(s)]
          The TEST and SCRATCH partitions should be pre-formatted
          with another base fs, where the overlay dirs will be created

> 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
>
> ?

You know what will happen :-) only bad things.

> 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.
>

It seems to me that multi section config file is not the right tool for
the job to test many types of file systems, but it may be useful
for testing different MKFS/MOUNT options for the same FSTYP.

For that purpose, if interleaving overlay section is helpful,
it could be used, but what won't work correctly is './check -overlay'
because it will not mkfs base fs when changing sections.

Actually, it may not be that hard to add support for mkfs of base
fs so check -overlay on multi section config will work, but I did
not know if that was an important use case.

Therefore, the feedback from prospect overlay testers and
the configurations that they use is important.
What would be your config if you were to test overlay?
P.S. I still did not get around to migrate kvm-xfstests to use new config,
but I will. For now patch 8/9 breaks some tests in kvm-xfstests
(old config) so I still need to fix that.

Amir.

Re: [PATCH v3 9/9] overlay: mount/unmount base fs before/after running tests

[Theodore Ts'o]

On Tue, Feb 14, 2017 at 08:43:36AM +0200, Amir Goldstein wrote:
> 
> Correct. The only thing that overlay does not do is format the base fs,
> as the README says:
>         - for overlay tests: ./check -overlay [test(s)]
>           The TEST and SCRATCH partitions should be pre-formatted
>           with another base fs, where the overlay dirs will be created
> 

So this means that if I only want to do the equivalent of:

   gce-xfstests -c overlay generic/013

as part of a bisection search, it's impossible to do this via the
config file and "./check -s ext4-overlay generic/013" alone.  Someone
who wants to do this will have to use their own wrapper script to
format the base file system with the proper file system.

Correct?

Which is fine, I have my own wrapper script system.  :-)

      	       	      	     	     	    - Ted

Re: [PATCH v3 9/9] overlay: mount/unmount base fs before/after running tests

[Amir Goldstein]

On Tue, Feb 14, 2017 at 7:07 PM, Theodore Ts'o <tytso@mit.edu> wrote:
> On Tue, Feb 14, 2017 at 08:43:36AM +0200, Amir Goldstein wrote:
>>
>> Correct. The only thing that overlay does not do is format the base fs,
>> as the README says:
>>         - for overlay tests: ./check -overlay [test(s)]
>>           The TEST and SCRATCH partitions should be pre-formatted
>>           with another base fs, where the overlay dirs will be created
>>
>
> So this means that if I only want to do the equivalent of:
>
>    gce-xfstests -c overlay generic/013
>
> as part of a bisection search, it's impossible to do this via the
> config file and "./check -s ext4-overlay generic/013" alone.  Someone
> who wants to do this will have to use their own wrapper script to
> format the base file system with the proper file system.
>
> Correct?
>

Correct.

I guess it wouldn't be that hard to add mkfs/fsck support for base fs,
but for now my budget to deal with this is over. I may get to it another
time.

> Which is fine, I have my own wrapper script system.  :-)
>

Do we have something like:
./check -s ext4 --setup-only

Should we have something like this?

Will this do the trick?
./check -s ext4 -n generic/013

Re: [PATCH v3 9/9] overlay: mount/unmount base fs before/after running tests

[Amir Goldstein]

On Tue, Feb 14, 2017 at 7:55 PM, Amir Goldstein <amir73il@gmail.com> wrote:
> On Tue, Feb 14, 2017 at 7:07 PM, Theodore Ts'o <tytso@mit.edu> wrote:
>> On Tue, Feb 14, 2017 at 08:43:36AM +0200, Amir Goldstein wrote:
>>>
>>> Correct. The only thing that overlay does not do is format the base fs,
>>> as the README says:
>>>         - for overlay tests: ./check -overlay [test(s)]
>>>           The TEST and SCRATCH partitions should be pre-formatted
>>>           with another base fs, where the overlay dirs will be created
>>>
>>
>> So this means that if I only want to do the equivalent of:
>>
>>    gce-xfstests -c overlay generic/013
>>
>> as part of a bisection search, it's impossible to do this via the
>> config file and "./check -s ext4-overlay generic/013" alone.  Someone
>> who wants to do this will have to use their own wrapper script to
>> format the base file system with the proper file system.
>>
>> Correct?
>>
>
> Correct.
>
> I guess it wouldn't be that hard to add mkfs/fsck support for base fs,
> but for now my budget to deal with this is over. I may get to it another
> time.
>
>> Which is fine, I have my own wrapper script system.  :-)
>>
>
> Do we have something like:
> ./check -s ext4 --setup-only
>
> Should we have something like this?
>
> Will this do the trick?
> ./check -s ext4 -n generic/013

FYI, for upcoming V4, I recalled the support for overlay sections
(i.e. it is an undocumented feature), but verified that multi section can run
with -overlay, as long as the sections only change mount options, e.g.:

[xfs]
TEST_DEV=/dev/sda5
TEST_DIR=/mnt/test
SCRATCH_DEV=/dev/sda6
SCRATCH_MNT=/mnt/scratch
MKFS_OPTIONS="-f -m rmapbt=1,reflink=1"
FSTYP=xfs

[xfs_pquota]
MOUNT_OPTIONS="-o pquota"
TEST_FS_MOUNT_OPTS="-o noatime"
OVERLAY_MOUNT_OPTIONS="-o redirect_dir=off"

I will collect some of these example in README.overlay

Re: [PATCH v3 0/9] fstests: new way to run overlay tests

[Amir Goldstein]

On Sun, Feb 12, 2017 at 10:43 PM, Amir Goldstein <amir73il@gmail.com> wrote:
> Hi Eryu and all,
>
> The reason I started this work was to help catch overlayfs bugs
> related to leaking objects in underlying (base) fs.
>
> As a by-product, following Eryu's comments on v2, configuring
> xfstest to run overlay tests over any file system is now easier then ever.
>
> With this change, all you have to do to run overlay tests if you
> already have a local.config setup to test a local file system is:
>  ./check -overlay
>
> It uses existing local.config that was setup to run tests on
> the base fs (e.g. xfs) and you can run './check' and './check -overlay'
> without re-formatting the test partitions and without changing the
> config file.
>
> The legacy overlayfs configuration, where TEST_DEV is a directory
> still works, but it should be deprecated.
>
> I tested ./check -overlay -g quick with both legacy overlay configuration
> and the new base fs configuration.
>
> Until now, overlay test configuration was not documented at all.
> I updated README per Eryu's request and tried to keep the documentation
> short and simple.
>
> Also updated README.config-sections with an easy example how to
> interleave overlay tests on every base fs in the multi section config
> file.
>
> I honestly think that it is important for file system developers these days,
> to test changes to their file systems with -overlay, to verify no breakage
> is caused when their file systems are used as base fs to overlay containers.
> I dare to say, that it is probably more important than testing 1k block size,
> which most of the maintainers do test with regularly.
>
> Some of the bugs I fixed in patches 1-3 indicate that people have not
> been 'stress testing' the xfstest config file and all of its gloious
> configurable options.  I tried to run with some basic configurations
> to check my changes, but I doubt that I have covered more then a small
> fraction of the configurations that people are using.
>
> I would very much appreciate if anyone could test these changes with their
> own set of configuration, with or without adding overlay tests into the mix.
> You can get the branch for testing from my github tree [1].
>

Extending my request to linux-fsdevel: any of you developers using either
multi section config files or more that the most standard config options,
please verify that these changes don't break anything on your setup.


> Thanks,
> Amir.
>
> [1] https://github.com/amir73il/xfstests/tree/ovl_base_fs
>
> v3:
> - Mount cycle base test fs
> - Fix bugs in non overlay specific sanity checks
> - Run -overlay test with existing config file of base fs
> - Run overlay tests per base fs by adding overlay config sections
>
> v2:
> - Test and scratch base dirs each have thier own base fs
> - Support mount cycles of base fs for scratch tests
>
> v1:
> - Both test and scratch base dirs on a single base fs
>
>
> Amir Goldstein (9):
>   fstests: sanity check that test partitions are not mounted elsewhere
>   fstests: use _test_mount() consistently
>   fstests: canonicalize mount points on every config section
>   overlay: rename OVERLAY_LOWER/UPPER/WORK_DIR
>   overlay: allow SCRATCH_DEV to be the base fs mount point
>   overlay: configure TEST/SCRATCH vars to base fs
>   overlay: use OVL_BASE_SCRATCH_MNT instead of SCRATCH_DEV
>   overlay: fix test and scratch filters for overlay base fs
>   overlay: mount/unmount base fs before/after running tests
>
>  README                 |  16 +++--
>  README.config-sections |   6 ++
>  check                  |  24 +++----
>  common/config          | 121 +++++++++++++++++++++++++++-----
>  common/filter          |   7 +-
>  common/rc              | 184 +++++++++++++++++++++++++++++++++++--------------
>  tests/overlay/001      |   7 +-
>  tests/overlay/002      |   2 +-
>  tests/overlay/003      |   5 +-
>  tests/overlay/004      |   7 +-
>  tests/overlay/005      |  30 ++++----
>  tests/overlay/006      |  10 +--
>  tests/overlay/008      |   8 +--
>  tests/overlay/009      |   2 +-
>  tests/overlay/010      |  10 +--
>  tests/overlay/011      |   6 +-
>  tests/overlay/012      |   4 +-
>  tests/overlay/013      |   4 +-
>  tests/overlay/014      |  19 ++---
>  tests/overlay/015      |   2 +-
>  tests/overlay/016      |   2 +-
>  tests/overlay/017      |   2 +-
>  tests/overlay/018      |   2 +-
>  tests/overlay/019      |   2 +-
>  tests/overlay/020      |   2 +-
>  tests/overlay/021      |   6 +-
>  26 files changed, 333 insertions(+), 157 deletions(-)
>
> --
> 2.7.4
>

Re: [PATCH v3 0/9] fstests: new way to run overlay tests

[Xiong Zhou]

On Sun, Feb 12, 2017 at 10:43:35PM +0200, Amir Goldstein wrote:
> Hi Eryu and all,
> 
> The reason I started this work was to help catch overlayfs bugs
> related to leaking objects in underlying (base) fs.

So firstly, what's wrong with the existing way exactly ?

Thanks,
Xiong
> 
> As a by-product, following Eryu's comments on v2, configuring
> xfstest to run overlay tests over any file system is now easier then ever.
> 
> With this change, all you have to do to run overlay tests if you
> already have a local.config setup to test a local file system is:
>  ./check -overlay
> 
> It uses existing local.config that was setup to run tests on
> the base fs (e.g. xfs) and you can run './check' and './check -overlay'
> without re-formatting the test partitions and without changing the
> config file.
> 
> The legacy overlayfs configuration, where TEST_DEV is a directory
> still works, but it should be deprecated.
> 
> I tested ./check -overlay -g quick with both legacy overlay configuration
> and the new base fs configuration.
> 
> Until now, overlay test configuration was not documented at all.
> I updated README per Eryu's request and tried to keep the documentation
> short and simple.
> 
> Also updated README.config-sections with an easy example how to
> interleave overlay tests on every base fs in the multi section config
> file.
> 
> I honestly think that it is important for file system developers these days,
> to test changes to their file systems with -overlay, to verify no breakage
> is caused when their file systems are used as base fs to overlay containers.
> I dare to say, that it is probably more important than testing 1k block size,
> which most of the maintainers do test with regularly.
> 
> Some of the bugs I fixed in patches 1-3 indicate that people have not
> been 'stress testing' the xfstest config file and all of its gloious
> configurable options.  I tried to run with some basic configurations
> to check my changes, but I doubt that I have covered more then a small
> fraction of the configurations that people are using.
> 
> I would very much appreciate if anyone could test these changes with their
> own set of configuration, with or without adding overlay tests into the mix.
> You can get the branch for testing from my github tree [1].
> 
> Thanks,
> Amir.
> 
> [1] https://github.com/amir73il/xfstests/tree/ovl_base_fs
> 
> v3:
> - Mount cycle base test fs
> - Fix bugs in non overlay specific sanity checks
> - Run -overlay test with existing config file of base fs
> - Run overlay tests per base fs by adding overlay config sections
> 
> v2:
> - Test and scratch base dirs each have thier own base fs
> - Support mount cycles of base fs for scratch tests
> 
> v1:
> - Both test and scratch base dirs on a single base fs
> 
> 
> Amir Goldstein (9):
>   fstests: sanity check that test partitions are not mounted elsewhere
>   fstests: use _test_mount() consistently
>   fstests: canonicalize mount points on every config section
>   overlay: rename OVERLAY_LOWER/UPPER/WORK_DIR
>   overlay: allow SCRATCH_DEV to be the base fs mount point
>   overlay: configure TEST/SCRATCH vars to base fs
>   overlay: use OVL_BASE_SCRATCH_MNT instead of SCRATCH_DEV
>   overlay: fix test and scratch filters for overlay base fs
>   overlay: mount/unmount base fs before/after running tests
> 
>  README                 |  16 +++--
>  README.config-sections |   6 ++
>  check                  |  24 +++----
>  common/config          | 121 +++++++++++++++++++++++++++-----
>  common/filter          |   7 +-
>  common/rc              | 184 +++++++++++++++++++++++++++++++++++--------------
>  tests/overlay/001      |   7 +-
>  tests/overlay/002      |   2 +-
>  tests/overlay/003      |   5 +-
>  tests/overlay/004      |   7 +-
>  tests/overlay/005      |  30 ++++----
>  tests/overlay/006      |  10 +--
>  tests/overlay/008      |   8 +--
>  tests/overlay/009      |   2 +-
>  tests/overlay/010      |  10 +--
>  tests/overlay/011      |   6 +-
>  tests/overlay/012      |   4 +-
>  tests/overlay/013      |   4 +-
>  tests/overlay/014      |  19 ++---
>  tests/overlay/015      |   2 +-
>  tests/overlay/016      |   2 +-
>  tests/overlay/017      |   2 +-
>  tests/overlay/018      |   2 +-
>  tests/overlay/019      |   2 +-
>  tests/overlay/020      |   2 +-
>  tests/overlay/021      |   6 +-
>  26 files changed, 333 insertions(+), 157 deletions(-)
> 
> -- 
> 2.7.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 0/9] fstests: new way to run overlay tests

[Amir Goldstein]

On Mon, Feb 13, 2017 at 6:19 AM, Xiong Zhou <xzhou@redhat.com> wrote:
> On Sun, Feb 12, 2017 at 10:43:35PM +0200, Amir Goldstein wrote:
>> Hi Eryu and all,
>>
>> The reason I started this work was to help catch overlayfs bugs
>> related to leaking objects in underlying (base) fs.
>
> So firstly, what's wrong with the existing way exactly ?
>

One of the reasons that xfstests cycle mounts scratch partition
is that some bugs, such as object leaking bugs, are only discovered
when trying to unmount the file system.

With overlayfs tests the cycling of overlay mount can detect bugs
related to overlayfs objects leaking, but not to base fs objects leaking.
OTOH, some bugs may be triggered in base fs when used under
overlayfs, but not when tests are run directly over the fs.

The solution is to cycle mount both overlay mount and the base fs
underneath it to detect all those bugs.

I started off (in V1) with a more complex way to manually configure
OVL_BASE_TEST_DEV, OVL_BASE_TEST_DIR, etc, but that
became very cumbersome.

Eryu pointed out the over complication of this configuration, so
I decided to re-think the "old way", which I always thought was
a bit hackish.

The "new way" enables running overlay tests over a range of
base fs configurations, which was not possible before without
using external scripts to wrap xfstests.

But above all, the "new way" represents a different perspective
of overlayfs testing - testing overlayfs independently of the
underlying fs is doing half the job.

You may say that overlayfs is closer to "MOUNT_OPTIONS"
then it is to "FSTYP". The truth is somewhere in the middle,
but the fact is that running a single "overlay" test does not cut it.
To tests overlay thoroughly, one would need to test overlay over
xfs, ext4, btrfs and to test xfs thoroughly, one would need to test
xfs, xfs+reflink, xfs+overlay, xfs+reflink+overlay, etc.

The new way provides an easy way to configure those tests,
using semantics that people are used to when testing multiple
flavors of configurations.

> Thanks,
> Xiong
>>
>> As a by-product, following Eryu's comments on v2, configuring
>> xfstest to run overlay tests over any file system is now easier then ever.
>>
>> With this change, all you have to do to run overlay tests if you
>> already have a local.config setup to test a local file system is:
>>  ./check -overlay
>>
>> It uses existing local.config that was setup to run tests on
>> the base fs (e.g. xfs) and you can run './check' and './check -overlay'
>> without re-formatting the test partitions and without changing the
>> config file.
>>
>> The legacy overlayfs configuration, where TEST_DEV is a directory
>> still works, but it should be deprecated.
>>
>> I tested ./check -overlay -g quick with both legacy overlay configuration
>> and the new base fs configuration.
>>
>> Until now, overlay test configuration was not documented at all.
>> I updated README per Eryu's request and tried to keep the documentation
>> short and simple.
>>
>> Also updated README.config-sections with an easy example how to
>> interleave overlay tests on every base fs in the multi section config
>> file.
>>
>> I honestly think that it is important for file system developers these days,
>> to test changes to their file systems with -overlay, to verify no breakage
>> is caused when their file systems are used as base fs to overlay containers.
>> I dare to say, that it is probably more important than testing 1k block size,
>> which most of the maintainers do test with regularly.
>>
>> Some of the bugs I fixed in patches 1-3 indicate that people have not
>> been 'stress testing' the xfstest config file and all of its gloious
>> configurable options.  I tried to run with some basic configurations
>> to check my changes, but I doubt that I have covered more then a small
>> fraction of the configurations that people are using.
>>
>> I would very much appreciate if anyone could test these changes with their
>> own set of configuration, with or without adding overlay tests into the mix.
>> You can get the branch for testing from my github tree [1].
>>
>> Thanks,
>> Amir.
>>
>> [1] https://github.com/amir73il/xfstests/tree/ovl_base_fs
>>
>> v3:
>> - Mount cycle base test fs
>> - Fix bugs in non overlay specific sanity checks
>> - Run -overlay test with existing config file of base fs
>> - Run overlay tests per base fs by adding overlay config sections
>>
>> v2:
>> - Test and scratch base dirs each have thier own base fs
>> - Support mount cycles of base fs for scratch tests
>>
>> v1:
>> - Both test and scratch base dirs on a single base fs
>>
>>
>> Amir Goldstein (9):
>>   fstests: sanity check that test partitions are not mounted elsewhere
>>   fstests: use _test_mount() consistently
>>   fstests: canonicalize mount points on every config section
>>   overlay: rename OVERLAY_LOWER/UPPER/WORK_DIR
>>   overlay: allow SCRATCH_DEV to be the base fs mount point
>>   overlay: configure TEST/SCRATCH vars to base fs
>>   overlay: use OVL_BASE_SCRATCH_MNT instead of SCRATCH_DEV
>>   overlay: fix test and scratch filters for overlay base fs
>>   overlay: mount/unmount base fs before/after running tests
>>
>>  README                 |  16 +++--
>>  README.config-sections |   6 ++
>>  check                  |  24 +++----
>>  common/config          | 121 +++++++++++++++++++++++++++-----
>>  common/filter          |   7 +-
>>  common/rc              | 184 +++++++++++++++++++++++++++++++++++--------------
>>  tests/overlay/001      |   7 +-
>>  tests/overlay/002      |   2 +-
>>  tests/overlay/003      |   5 +-
>>  tests/overlay/004      |   7 +-
>>  tests/overlay/005      |  30 ++++----
>>  tests/overlay/006      |  10 +--
>>  tests/overlay/008      |   8 +--
>>  tests/overlay/009      |   2 +-
>>  tests/overlay/010      |  10 +--
>>  tests/overlay/011      |   6 +-
>>  tests/overlay/012      |   4 +-
>>  tests/overlay/013      |   4 +-
>>  tests/overlay/014      |  19 ++---
>>  tests/overlay/015      |   2 +-
>>  tests/overlay/016      |   2 +-
>>  tests/overlay/017      |   2 +-
>>  tests/overlay/018      |   2 +-
>>  tests/overlay/019      |   2 +-
>>  tests/overlay/020      |   2 +-
>>  tests/overlay/021      |   6 +-
>>  26 files changed, 333 insertions(+), 157 deletions(-)
>>
>> --
>> 2.7.4
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe fstests" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 0/9] fstests: new way to run overlay tests

[Xiong Zhou]

On Mon, Feb 13, 2017 at 07:37:45AM +0200, Amir Goldstein wrote:
> On Mon, Feb 13, 2017 at 6:19 AM, Xiong Zhou <xzhou@redhat.com> wrote:
> > On Sun, Feb 12, 2017 at 10:43:35PM +0200, Amir Goldstein wrote:
> >> Hi Eryu and all,
> >>
> >> The reason I started this work was to help catch overlayfs bugs
> >> related to leaking objects in underlying (base) fs.
> >
> > So firstly, what's wrong with the existing way exactly ?
> >
> 
> One of the reasons that xfstests cycle mounts scratch partition
> is that some bugs, such as object leaking bugs, are only discovered
> when trying to unmount the file system.
> 
> With overlayfs tests the cycling of overlay mount can detect bugs
> related to overlayfs objects leaking, but not to base fs objects leaking.
> OTOH, some bugs may be triggered in base fs when used under
> overlayfs, but not when tests are run directly over the fs.
> 
> The solution is to cycle mount both overlay mount and the base fs
> underneath it to detect all those bugs.

Good job.

> 
> I started off (in V1) with a more complex way to manually configure
> OVL_BASE_TEST_DEV, OVL_BASE_TEST_DIR, etc, but that
> became very cumbersome.
> 
> Eryu pointed out the over complication of this configuration, so
> I decided to re-think the "old way", which I always thought was
> a bit hackish.
> 
> The "new way" enables running overlay tests over a range of
> base fs configurations, which was not possible before without
> using external scripts to wrap xfstests.
> 
> But above all, the "new way" represents a different perspective
> of overlayfs testing - testing overlayfs independently of the
> underlying fs is doing half the job.
> 
> You may say that overlayfs is closer to "MOUNT_OPTIONS"
> then it is to "FSTYP". The truth is somewhere in the middle,
> but the fact is that running a single "overlay" test does not cut it.
> To tests overlay thoroughly, one would need to test overlay over
> xfs, ext4, btrfs and to test xfs thoroughly, one would need to test
> xfs, xfs+reflink, xfs+overlay, xfs+reflink+overlay, etc.
> 
> The new way provides an easy way to configure those tests,
> using semantics that people are used to when testing multiple
> flavors of configurations.

I'd like more comments or document for these NEW configurations, other
than one or two lines in Examples.

In other words, what kind of configuration your patches bringing
support to ?

> 
> > Thanks,
> > Xiong
> >>
> >> As a by-product, following Eryu's comments on v2, configuring
> >> xfstest to run overlay tests over any file system is now easier then ever.
> >>
> >> With this change, all you have to do to run overlay tests if you
> >> already have a local.config setup to test a local file system is:
> >>  ./check -overlay
> >>
> >> It uses existing local.config that was setup to run tests on
> >> the base fs (e.g. xfs) and you can run './check' and './check -overlay'
> >> without re-formatting the test partitions and without changing the
> >> config file.
> >>
> >> The legacy overlayfs configuration, where TEST_DEV is a directory
> >> still works, but it should be deprecated.
> >>
> >> I tested ./check -overlay -g quick with both legacy overlay configuration
> >> and the new base fs configuration.
> >>
> >> Until now, overlay test configuration was not documented at all.
> >> I updated README per Eryu's request and tried to keep the documentation
> >> short and simple.
> >>
> >> Also updated README.config-sections with an easy example how to
> >> interleave overlay tests on every base fs in the multi section config
> >> file.
> >>
> >> I honestly think that it is important for file system developers these days,
> >> to test changes to their file systems with -overlay, to verify no breakage
> >> is caused when their file systems are used as base fs to overlay containers.
> >> I dare to say, that it is probably more important than testing 1k block size,
> >> which most of the maintainers do test with regularly.
> >>
> >> Some of the bugs I fixed in patches 1-3 indicate that people have not
> >> been 'stress testing' the xfstest config file and all of its gloious
> >> configurable options.  I tried to run with some basic configurations
> >> to check my changes, but I doubt that I have covered more then a small
> >> fraction of the configurations that people are using.
> >>
> >> I would very much appreciate if anyone could test these changes with their
> >> own set of configuration, with or without adding overlay tests into the mix.
> >> You can get the branch for testing from my github tree [1].
> >>
> >> Thanks,
> >> Amir.
> >>
> >> [1] https://github.com/amir73il/xfstests/tree/ovl_base_fs
> >>
> >> v3:
> >> - Mount cycle base test fs
> >> - Fix bugs in non overlay specific sanity checks
> >> - Run -overlay test with existing config file of base fs
> >> - Run overlay tests per base fs by adding overlay config sections
> >>
> >> v2:
> >> - Test and scratch base dirs each have thier own base fs
> >> - Support mount cycles of base fs for scratch tests
> >>
> >> v1:
> >> - Both test and scratch base dirs on a single base fs
> >>
> >>
> >> Amir Goldstein (9):
> >>   fstests: sanity check that test partitions are not mounted elsewhere
> >>   fstests: use _test_mount() consistently
> >>   fstests: canonicalize mount points on every config section
> >>   overlay: rename OVERLAY_LOWER/UPPER/WORK_DIR
> >>   overlay: allow SCRATCH_DEV to be the base fs mount point
> >>   overlay: configure TEST/SCRATCH vars to base fs
> >>   overlay: use OVL_BASE_SCRATCH_MNT instead of SCRATCH_DEV
> >>   overlay: fix test and scratch filters for overlay base fs
> >>   overlay: mount/unmount base fs before/after running tests
> >>
> >>  README                 |  16 +++--
> >>  README.config-sections |   6 ++
> >>  check                  |  24 +++----
> >>  common/config          | 121 +++++++++++++++++++++++++++-----
> >>  common/filter          |   7 +-
> >>  common/rc              | 184 +++++++++++++++++++++++++++++++++++--------------
> >>  tests/overlay/001      |   7 +-
> >>  tests/overlay/002      |   2 +-
> >>  tests/overlay/003      |   5 +-
> >>  tests/overlay/004      |   7 +-
> >>  tests/overlay/005      |  30 ++++----
> >>  tests/overlay/006      |  10 +--
> >>  tests/overlay/008      |   8 +--
> >>  tests/overlay/009      |   2 +-
> >>  tests/overlay/010      |  10 +--
> >>  tests/overlay/011      |   6 +-
> >>  tests/overlay/012      |   4 +-
> >>  tests/overlay/013      |   4 +-
> >>  tests/overlay/014      |  19 ++---
> >>  tests/overlay/015      |   2 +-
> >>  tests/overlay/016      |   2 +-
> >>  tests/overlay/017      |   2 +-
> >>  tests/overlay/018      |   2 +-
> >>  tests/overlay/019      |   2 +-
> >>  tests/overlay/020      |   2 +-
> >>  tests/overlay/021      |   6 +-
> >>  26 files changed, 333 insertions(+), 157 deletions(-)
> >>
> >> --
> >> 2.7.4
> >>
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe fstests" in
> >> the body of a message to majordomo@vger.kernel.org
> >> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 0/9] fstests: new way to run overlay tests

[Amir Goldstein]

On Tue, Feb 14, 2017 at 6:40 AM, Xiong Zhou <xzhou@redhat.com> wrote:
> On Mon, Feb 13, 2017 at 07:37:45AM +0200, Amir Goldstein wrote:
>> On Mon, Feb 13, 2017 at 6:19 AM, Xiong Zhou <xzhou@redhat.com> wrote:
>> > On Sun, Feb 12, 2017 at 10:43:35PM +0200, Amir Goldstein wrote:
>> >> Hi Eryu and all,
>> >>
>> >> The reason I started this work was to help catch overlayfs bugs
>> >> related to leaking objects in underlying (base) fs.
>> >
>> > So firstly, what's wrong with the existing way exactly ?
>> >
>>
>> One of the reasons that xfstests cycle mounts scratch partition
>> is that some bugs, such as object leaking bugs, are only discovered
>> when trying to unmount the file system.
>>
>> With overlayfs tests the cycling of overlay mount can detect bugs
>> related to overlayfs objects leaking, but not to base fs objects leaking.
>> OTOH, some bugs may be triggered in base fs when used under
>> overlayfs, but not when tests are run directly over the fs.
>>
>> The solution is to cycle mount both overlay mount and the base fs
>> underneath it to detect all those bugs.
>
> Good job.
>
>>
>> I started off (in V1) with a more complex way to manually configure
>> OVL_BASE_TEST_DEV, OVL_BASE_TEST_DIR, etc, but that
>> became very cumbersome.
>>
>> Eryu pointed out the over complication of this configuration, so
>> I decided to re-think the "old way", which I always thought was
>> a bit hackish.
>>
>> The "new way" enables running overlay tests over a range of
>> base fs configurations, which was not possible before without
>> using external scripts to wrap xfstests.
>>
>> But above all, the "new way" represents a different perspective
>> of overlayfs testing - testing overlayfs independently of the
>> underlying fs is doing half the job.
>>
>> You may say that overlayfs is closer to "MOUNT_OPTIONS"
>> then it is to "FSTYP". The truth is somewhere in the middle,
>> but the fact is that running a single "overlay" test does not cut it.
>> To tests overlay thoroughly, one would need to test overlay over
>> xfs, ext4, btrfs and to test xfs thoroughly, one would need to test
>> xfs, xfs+reflink, xfs+overlay, xfs+reflink+overlay, etc.
>>
>> The new way provides an easy way to configure those tests,
>> using semantics that people are used to when testing multiple
>> flavors of configurations.
>
> I'd like more comments or document for these NEW configurations, other
> than one or two lines in Examples.
>
> In other words, what kind of configuration your patches bringing
> support to ?
>

There are 2 parts for this answer:

1. Single section config file
====================

The answer here is there is no new configuration.
All you need to do is forget everything you knew about old overlay test config.

Take an existing test configuration you have and already use:
     TEST_DEV=/dev/sda5
     TEST_DIR=/mnt/test
     SCRATCH_DEV=/dev/sda6
     SCRATCH_MNT=/mnt/scratch
     FSTYP=xfs

and just run './check -overlay' whenever you want to test overlayfs
and './check' whenever you want to test native fs.

I could add more and more examples to README, but it's not like
README tells you much about how to use other configurations.
IMO, these 3 lines sums it up well:

        - for overlay tests: ./check -overlay [test(s)]
          The TEST and SCRATCH partitions should be pre-formatted
          with another base fs, where the overlay dirs will be created

2. Multi section config file
===================

Honestly, I never intended to make it work and there are many combinations
of sections that do not work today and some that will not work with overlay
(see email from Ted about patch 9/9)

I just realized that it may be useful to some and added the example howto
add overlay sections post non-overlay section, but I do not intend to fix
problems with this sort of setup, so I might as well remove the example
and leave this an undocumented feature for people who understand
how multi section works.

Re: [PATCH v3 0/9] fstests: new way to run overlay tests

[Xiong Zhou]

On Tue, Feb 14, 2017 at 08:15:22AM +0200, Amir Goldstein wrote:
> On Tue, Feb 14, 2017 at 6:40 AM, Xiong Zhou <xzhou@redhat.com> wrote:
> > On Mon, Feb 13, 2017 at 07:37:45AM +0200, Amir Goldstein wrote:
> >> On Mon, Feb 13, 2017 at 6:19 AM, Xiong Zhou <xzhou@redhat.com> wrote:
> >> > On Sun, Feb 12, 2017 at 10:43:35PM +0200, Amir Goldstein wrote:
> >> >> Hi Eryu and all,
> >> >>
> >> >> The reason I started this work was to help catch overlayfs bugs
> >> >> related to leaking objects in underlying (base) fs.
> >> >
> >> > So firstly, what's wrong with the existing way exactly ?
> >> >
> >>
> >> One of the reasons that xfstests cycle mounts scratch partition
> >> is that some bugs, such as object leaking bugs, are only discovered
> >> when trying to unmount the file system.
> >>
> >> With overlayfs tests the cycling of overlay mount can detect bugs
> >> related to overlayfs objects leaking, but not to base fs objects leaking.
> >> OTOH, some bugs may be triggered in base fs when used under
> >> overlayfs, but not when tests are run directly over the fs.
> >>
> >> The solution is to cycle mount both overlay mount and the base fs
> >> underneath it to detect all those bugs.
> >
> > Good job.
> >
> >>
> >> I started off (in V1) with a more complex way to manually configure
> >> OVL_BASE_TEST_DEV, OVL_BASE_TEST_DIR, etc, but that
> >> became very cumbersome.
> >>
> >> Eryu pointed out the over complication of this configuration, so
> >> I decided to re-think the "old way", which I always thought was
> >> a bit hackish.
> >>
> >> The "new way" enables running overlay tests over a range of
> >> base fs configurations, which was not possible before without
> >> using external scripts to wrap xfstests.
> >>
> >> But above all, the "new way" represents a different perspective
> >> of overlayfs testing - testing overlayfs independently of the
> >> underlying fs is doing half the job.
> >>
> >> You may say that overlayfs is closer to "MOUNT_OPTIONS"
> >> then it is to "FSTYP". The truth is somewhere in the middle,
> >> but the fact is that running a single "overlay" test does not cut it.
> >> To tests overlay thoroughly, one would need to test overlay over
> >> xfs, ext4, btrfs and to test xfs thoroughly, one would need to test
> >> xfs, xfs+reflink, xfs+overlay, xfs+reflink+overlay, etc.
> >>
> >> The new way provides an easy way to configure those tests,
> >> using semantics that people are used to when testing multiple
> >> flavors of configurations.
> >
> > I'd like more comments or document for these NEW configurations, other
> > than one or two lines in Examples.
> >
> > In other words, what kind of configuration your patches bringing
> > support to ?
> >
> 
> There are 2 parts for this answer:
> 
> 1. Single section config file
> ====================
> 
> The answer here is there is no new configuration.
> All you need to do is forget everything you knew about old overlay test config.

That's okay, i just need to know what I am testing for. With a
"new way" to run tests, how I can control my test configuration
with the config file, what every line in the config file means
to the "new way".

> 
> Take an existing test configuration you have and already use:
>      TEST_DEV=/dev/sda5
>      TEST_DIR=/mnt/test
>      SCRATCH_DEV=/dev/sda6
>      SCRATCH_MNT=/mnt/scratch
>      FSTYP=xfs
> 
> and just run './check -overlay' whenever you want to test overlayfs
> and './check' whenever you want to test native fs.
> 
> I could add more and more examples to README, but it's not like
> README tells you much about how to use other configurations.

Agree. It's fair enough to keep this "how the new way interprets
old config file" in commit message. Ya.. finally i found that..

Still I think you can add more config examples in commit message
or somewhere, for your saying:
> >> To tests overlay thoroughly, one would need to test overlay over
> >> xfs, ext4, btrfs and to test xfs thoroughly, one would need to test
> >> xfs, xfs+reflink, xfs+overlay, xfs+reflink+overlay, etc.
> >>
> >> The new way provides an easy way to configure those tests,
> >> using semantics that people are used to when testing multiple
> >> flavors of configurations.

Then people can test your code with your examples. Ya, i'am lazy :)

I guess -overlay option consider FSTYP as fs type for base fs.
While with above config file FSTYP=xfs,
if /dev/sda{5,6} are formated as ext4 fs before running,
./check -overlay generic/001 will test with base fs as ext4,
(I don't get what i config for)
./check generic/001 will stop test "wrong fs type".

Yes, it's human error. It's great if -overlay option can also stop
test as general, to correct this error.

Thanks for you time and Sorry for my bad questions.

> IMO, these 3 lines sums it up well:
> 
>         - for overlay tests: ./check -overlay [test(s)]
>           The TEST and SCRATCH partitions should be pre-formatted
>           with another base fs, where the overlay dirs will be created
> 
> 2. Multi section config file
> ===================
> 
> Honestly, I never intended to make it work and there are many combinations
> of sections that do not work today and some that will not work with overlay
> (see email from Ted about patch 9/9)
> 
> I just realized that it may be useful to some and added the example howto
> add overlay sections post non-overlay section, but I do not intend to fix
> problems with this sort of setup, so I might as well remove the example
> and leave this an undocumented feature for people who understand
> how multi section works.

Re: [PATCH v3 0/9] fstests: new way to run overlay tests

[Amir Goldstein]

On Tue, Feb 14, 2017 at 11:25 AM, Xiong Zhou <xzhou@redhat.com> wrote:
> On Tue, Feb 14, 2017 at 08:15:22AM +0200, Amir Goldstein wrote:
>> On Tue, Feb 14, 2017 at 6:40 AM, Xiong Zhou <xzhou@redhat.com> wrote:
>> > On Mon, Feb 13, 2017 at 07:37:45AM +0200, Amir Goldstein wrote:
>> >> On Mon, Feb 13, 2017 at 6:19 AM, Xiong Zhou <xzhou@redhat.com> wrote:
>> >> > On Sun, Feb 12, 2017 at 10:43:35PM +0200, Amir Goldstein wrote:
>> >> >> Hi Eryu and all,
>> >> >>
>> >> >> The reason I started this work was to help catch overlayfs bugs
>> >> >> related to leaking objects in underlying (base) fs.
>> >> >
>> >> > So firstly, what's wrong with the existing way exactly ?
>> >> >
>> >>
>> >> One of the reasons that xfstests cycle mounts scratch partition
>> >> is that some bugs, such as object leaking bugs, are only discovered
>> >> when trying to unmount the file system.
>> >>
>> >> With overlayfs tests the cycling of overlay mount can detect bugs
>> >> related to overlayfs objects leaking, but not to base fs objects leaking.
>> >> OTOH, some bugs may be triggered in base fs when used under
>> >> overlayfs, but not when tests are run directly over the fs.
>> >>
>> >> The solution is to cycle mount both overlay mount and the base fs
>> >> underneath it to detect all those bugs.
>> >
>> > Good job.
>> >
>> >>
>> >> I started off (in V1) with a more complex way to manually configure
>> >> OVL_BASE_TEST_DEV, OVL_BASE_TEST_DIR, etc, but that
>> >> became very cumbersome.
>> >>
>> >> Eryu pointed out the over complication of this configuration, so
>> >> I decided to re-think the "old way", which I always thought was
>> >> a bit hackish.
>> >>
>> >> The "new way" enables running overlay tests over a range of
>> >> base fs configurations, which was not possible before without
>> >> using external scripts to wrap xfstests.
>> >>
>> >> But above all, the "new way" represents a different perspective
>> >> of overlayfs testing - testing overlayfs independently of the
>> >> underlying fs is doing half the job.
>> >>
>> >> You may say that overlayfs is closer to "MOUNT_OPTIONS"
>> >> then it is to "FSTYP". The truth is somewhere in the middle,
>> >> but the fact is that running a single "overlay" test does not cut it.
>> >> To tests overlay thoroughly, one would need to test overlay over
>> >> xfs, ext4, btrfs and to test xfs thoroughly, one would need to test
>> >> xfs, xfs+reflink, xfs+overlay, xfs+reflink+overlay, etc.
>> >>
>> >> The new way provides an easy way to configure those tests,
>> >> using semantics that people are used to when testing multiple
>> >> flavors of configurations.
>> >
>> > I'd like more comments or document for these NEW configurations, other
>> > than one or two lines in Examples.
>> >
>> > In other words, what kind of configuration your patches bringing
>> > support to ?
>> >
>>
>> There are 2 parts for this answer:
>>
>> 1. Single section config file
>> ====================
>>
>> The answer here is there is no new configuration.
>> All you need to do is forget everything you knew about old overlay test config.
>
> That's okay, i just need to know what I am testing for. With a
> "new way" to run tests, how I can control my test configuration
> with the config file, what every line in the config file means
> to the "new way".
>

All the lines in config mean exactly what they meant before
and they all refer to the configuration on the base fs partitions.
There are some vars that are ignored for -overlay run, like MKFS_OPTIONS
I can document that, but this is implied from
"The TEST and SCRATCH partitions should be pre-formatted"

>>
>> Take an existing test configuration you have and already use:
>>      TEST_DEV=/dev/sda5
>>      TEST_DIR=/mnt/test
>>      SCRATCH_DEV=/dev/sda6
>>      SCRATCH_MNT=/mnt/scratch
>>      FSTYP=xfs
>>
>> and just run './check -overlay' whenever you want to test overlayfs
>> and './check' whenever you want to test native fs.
>>
>> I could add more and more examples to README, but it's not like
>> README tells you much about how to use other configurations.
>
> Agree. It's fair enough to keep this "how the new way interprets
> old config file" in commit message. Ya.. finally i found that..

"old way" was never documented and Eryu want to deprecate it
so I see no reason to document how old configuration is supported
it suffice to say that current change should not break existing
config files of  users that test overlay

>
> Still I think you can add more config examples in commit message
> or somewhere, for your saying:
>> >> To tests overlay thoroughly, one would need to test overlay over
>> >> xfs, ext4, btrfs and to test xfs thoroughly, one would need to test
>> >> xfs, xfs+reflink, xfs+overlay, xfs+reflink+overlay, etc.
>> >>
>> >> The new way provides an easy way to configure those tests,
>> >> using semantics that people are used to when testing multiple
>> >> flavors of configurations.
>
> Then people can test your code with your examples. Ya, i'am lazy :)
>
> I guess -overlay option consider FSTYP as fs type for base fs.
> While with above config file FSTYP=xfs,
> if /dev/sda{5,6} are formated as ext4 fs before running,
> ./check -overlay generic/001 will test with base fs as ext4,
> (I don't get what i config for)
> ./check generic/001 will stop test "wrong fs type".
>
> Yes, it's human error. It's great if -overlay option can also stop
> test as general, to correct this error.
>

I now regret getting into the multi section example.
I will withdraw any support for this scenario.

> Thanks for you time and Sorry for my bad questions.
>

Not at all. Serves me right for publishing instructions to a half baked feature.

Thanks for keeping me honest!

Amir.

Re: [PATCH v3 0/9] fstests: new way to run overlay tests

[Eryu Guan]

On Sun, Feb 12, 2017 at 10:43:35PM +0200, Amir Goldstein wrote:
> Hi Eryu and all,
> 
> The reason I started this work was to help catch overlayfs bugs
> related to leaking objects in underlying (base) fs.
> 
> As a by-product, following Eryu's comments on v2, configuring
> xfstest to run overlay tests over any file system is now easier then ever.
> 
> With this change, all you have to do to run overlay tests if you
> already have a local.config setup to test a local file system is:
>  ./check -overlay
> 
> It uses existing local.config that was setup to run tests on
> the base fs (e.g. xfs) and you can run './check' and './check -overlay'
> without re-formatting the test partitions and without changing the
> config file.

This sounds handy to me, thanks a lot for the improvements!

The patchset looks fine to me overall, some minor comments go to
individual patch.

And I want to leave these patches sitting in the list for a longer time
for broader review, as it's quite an invasive update.

> 
> The legacy overlayfs configuration, where TEST_DEV is a directory
> still works, but it should be deprecated.
> 
> I tested ./check -overlay -g quick with both legacy overlay configuration
> and the new base fs configuration.
> 
> Until now, overlay test configuration was not documented at all.
> I updated README per Eryu's request and tried to keep the documentation
> short and simple.

Thanks!

> 
> Also updated README.config-sections with an easy example how to
> interleave overlay tests on every base fs in the multi section config
> file.
> 
> I honestly think that it is important for file system developers these days,
> to test changes to their file systems with -overlay, to verify no breakage
> is caused when their file systems are used as base fs to overlay containers.
> I dare to say, that it is probably more important than testing 1k block size,
> which most of the maintainers do test with regularly.
> 
> Some of the bugs I fixed in patches 1-3 indicate that people have not
> been 'stress testing' the xfstest config file and all of its gloious
> configurable options.  I tried to run with some basic configurations
> to check my changes, but I doubt that I have covered more then a small
> fraction of the configurations that people are using.
> 
> I would very much appreciate if anyone could test these changes with their
> own set of configuration, with or without adding overlay tests into the mix.
> You can get the branch for testing from my github tree [1].

I hit a bug if I defined SCRATCH_DEV_POOL for btrfs, I'll describe the
details in reply to patch 6/9.

Thanks,
Eryu

> 
> Thanks,
> Amir.
> 
> [1] https://github.com/amir73il/xfstests/tree/ovl_base_fs
> 
> v3:
> - Mount cycle base test fs
> - Fix bugs in non overlay specific sanity checks
> - Run -overlay test with existing config file of base fs
> - Run overlay tests per base fs by adding overlay config sections
> 
> v2:
> - Test and scratch base dirs each have thier own base fs
> - Support mount cycles of base fs for scratch tests
> 
> v1:
> - Both test and scratch base dirs on a single base fs
> 
> 
> Amir Goldstein (9):
>   fstests: sanity check that test partitions are not mounted elsewhere
>   fstests: use _test_mount() consistently
>   fstests: canonicalize mount points on every config section
>   overlay: rename OVERLAY_LOWER/UPPER/WORK_DIR
>   overlay: allow SCRATCH_DEV to be the base fs mount point
>   overlay: configure TEST/SCRATCH vars to base fs
>   overlay: use OVL_BASE_SCRATCH_MNT instead of SCRATCH_DEV
>   overlay: fix test and scratch filters for overlay base fs
>   overlay: mount/unmount base fs before/after running tests
> 
>  README                 |  16 +++--
>  README.config-sections |   6 ++
>  check                  |  24 +++----
>  common/config          | 121 +++++++++++++++++++++++++++-----
>  common/filter          |   7 +-
>  common/rc              | 184 +++++++++++++++++++++++++++++++++++--------------
>  tests/overlay/001      |   7 +-
>  tests/overlay/002      |   2 +-
>  tests/overlay/003      |   5 +-
>  tests/overlay/004      |   7 +-
>  tests/overlay/005      |  30 ++++----
>  tests/overlay/006      |  10 +--
>  tests/overlay/008      |   8 +--
>  tests/overlay/009      |   2 +-
>  tests/overlay/010      |  10 +--
>  tests/overlay/011      |   6 +-
>  tests/overlay/012      |   4 +-
>  tests/overlay/013      |   4 +-
>  tests/overlay/014      |  19 ++---
>  tests/overlay/015      |   2 +-
>  tests/overlay/016      |   2 +-
>  tests/overlay/017      |   2 +-
>  tests/overlay/018      |   2 +-
>  tests/overlay/019      |   2 +-
>  tests/overlay/020      |   2 +-
>  tests/overlay/021      |   6 +-
>  26 files changed, 333 insertions(+), 157 deletions(-)
> 
> -- 
> 2.7.4
> 

Re: [PATCH v3 0/9] fstests: new way to run overlay tests

[Amir Goldstein]

On Mon, Feb 13, 2017 at 1:02 PM, Eryu Guan <eguan@redhat.com> wrote:
> On Sun, Feb 12, 2017 at 10:43:35PM +0200, Amir Goldstein wrote:
>> Hi Eryu and all,
>>
>> The reason I started this work was to help catch overlayfs bugs
>> related to leaking objects in underlying (base) fs.
>>
>> As a by-product, following Eryu's comments on v2, configuring
>> xfstest to run overlay tests over any file system is now easier then ever.
>>
>> With this change, all you have to do to run overlay tests if you
>> already have a local.config setup to test a local file system is:
>>  ./check -overlay
>>
>> It uses existing local.config that was setup to run tests on
>> the base fs (e.g. xfs) and you can run './check' and './check -overlay'
>> without re-formatting the test partitions and without changing the
>> config file.
>
> This sounds handy to me, thanks a lot for the improvements!
>
> The patchset looks fine to me overall, some minor comments go to
> individual patch.
>
> And I want to leave these patches sitting in the list for a longer time
> for broader review, as it's quite an invasive update.
>
>>
>> The legacy overlayfs configuration, where TEST_DEV is a directory
>> still works, but it should be deprecated.
>>
>> I tested ./check -overlay -g quick with both legacy overlay configuration
>> and the new base fs configuration.
>>
>> Until now, overlay test configuration was not documented at all.
>> I updated README per Eryu's request and tried to keep the documentation
>> short and simple.
>
> Thanks!
>
>>
>> Also updated README.config-sections with an easy example how to
>> interleave overlay tests on every base fs in the multi section config
>> file.
>>
>> I honestly think that it is important for file system developers these days,
>> to test changes to their file systems with -overlay, to verify no breakage
>> is caused when their file systems are used as base fs to overlay containers.
>> I dare to say, that it is probably more important than testing 1k block size,
>> which most of the maintainers do test with regularly.
>>
>> Some of the bugs I fixed in patches 1-3 indicate that people have not
>> been 'stress testing' the xfstest config file and all of its gloious
>> configurable options.  I tried to run with some basic configurations
>> to check my changes, but I doubt that I have covered more then a small
>> fraction of the configurations that people are using.
>>
>> I would very much appreciate if anyone could test these changes with their
>> own set of configuration, with or without adding overlay tests into the mix.
>> You can get the branch for testing from my github tree [1].
>
> I hit a bug if I defined SCRATCH_DEV_POOL for btrfs, I'll describe the
> details in reply to patch 6/9.
>

Eryu,

Tested that SCRATCH_DEV_POOL works with single/multi section config.
Tested "old" and "new" overlay config.
Tested -overlay with single/multi section config.
Tested kvm-xfstests -c overlay.

I will write up some README.overlay with examples for supported configs
and re-post patches later today.

In the mean while, I pushed V4 patches to github [1].
If you have some time, please give it a quick sanity check
with your configs.

Thanks,
Amir.

[1] https://github.com/amir73il/xfstests/tree/ovl_base_fs

>> v3:
>> - Mount cycle base test fs
>> - Fix bugs in non overlay specific sanity checks
>> - Run -overlay test with existing config file of base fs
>> - Run overlay tests per base fs by adding overlay config sections
>>
>> v2:
>> - Test and scratch base dirs each have thier own base fs
>> - Support mount cycles of base fs for scratch tests
>>
>> v1:
>> - Both test and scratch base dirs on a single base fs
>>
>>
>> Amir Goldstein (9):
>>   fstests: sanity check that test partitions are not mounted elsewhere
>>   fstests: use _test_mount() consistently
>>   fstests: canonicalize mount points on every config section
>>   overlay: rename OVERLAY_LOWER/UPPER/WORK_DIR
>>   overlay: allow SCRATCH_DEV to be the base fs mount point
>>   overlay: configure TEST/SCRATCH vars to base fs
>>   overlay: use OVL_BASE_SCRATCH_MNT instead of SCRATCH_DEV
>>   overlay: fix test and scratch filters for overlay base fs
>>   overlay: mount/unmount base fs before/after running tests
>>
>>  README                 |  16 +++--
>>  README.config-sections |   6 ++
>>  check                  |  24 +++----
>>  common/config          | 121 +++++++++++++++++++++++++++-----
>>  common/filter          |   7 +-
>>  common/rc              | 184 +++++++++++++++++++++++++++++++++++--------------
>>  tests/overlay/001      |   7 +-
>>  tests/overlay/002      |   2 +-
>>  tests/overlay/003      |   5 +-
>>  tests/overlay/004      |   7 +-
>>  tests/overlay/005      |  30 ++++----
>>  tests/overlay/006      |  10 +--
>>  tests/overlay/008      |   8 +--
>>  tests/overlay/009      |   2 +-
>>  tests/overlay/010      |  10 +--
>>  tests/overlay/011      |   6 +-
>>  tests/overlay/012      |   4 +-
>>  tests/overlay/013      |   4 +-
>>  tests/overlay/014      |  19 ++---
>>  tests/overlay/015      |   2 +-
>>  tests/overlay/016      |   2 +-
>>  tests/overlay/017      |   2 +-
>>  tests/overlay/018      |   2 +-
>>  tests/overlay/019      |   2 +-
>>  tests/overlay/020      |   2 +-
>>  tests/overlay/021      |   6 +-
>>  26 files changed, 333 insertions(+), 157 deletions(-)
>>
>> --
>> 2.7.4
>>