fstests: new way to run overlay tests

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 <[email protected]>

---
 common/rc | 83 +++++++++++++++++++++++++++++++++++++--------------------------
 1 file changed, 49 insertions(+), 34 deletions(-)

common/rc +40 -33

index 76515e2..d831b17 100644
@@ -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

common/rc +1 -0

+
 	# 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"

Signature · 1 line

2.7.4

Quoted 26 lines · 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 <[email protected]>

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

Quoted 27 lines · Eryu Guan

Quoted 26 lines · 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 <[email protected]>

---
 common/rc | 83 +++++++++++++++++++++++++++++++++++++--------------------------
 1 file changed, 49 insertions(+), 34 deletions(-)

...

Quoted 19 lines

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

Quoted 9 lines

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

(quoted diff) +0 -2

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

Quoted 60 lines · Amir Goldstein

Quoted 27 lines · Eryu Guan

Quoted 26 lines · 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 <[email protected]>

---
 common/rc | 83 +++++++++++++++++++++++++++++++++++++--------------------------
 1 file changed, 49 insertions(+), 34 deletions(-)

...

Quoted 28 lines

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.

Quoted 6 lines

Actually, there that test already exists in:

_scratch_mkfs
_scratch_cleanup_files
_overlay_base_scratch_mount
_check_mounted_on

Quoted 14 lines

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

(quoted diff) +0 -2

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

Quoted 2 lines

With this patch, test does abort for _check_mounted_on() failure, (please verify)

Quoted 4 lines

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

(quoted diff) +10 -4

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

Quoted 61 lines · Amir Goldstein

Quoted 60 lines · Amir Goldstein

Quoted 27 lines · Eryu Guan

Quoted 26 lines · 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 <[email protected]>

---
 common/rc | 83 +++++++++++++++++++++++++++++++++++++--------------------------
 1 file changed, 49 insertions(+), 34 deletions(-)

...

Quoted 28 lines

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.

Quoted 6 lines

Quoted 6 lines

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?

Quoted 36 lines

Quoted 29 lines

Quoted 29 lines

Quoted 28 lines

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.

Quoted 6 lines

Quoted 6 lines

Actually, there that test already exists in:

_scratch_mkfs
_scratch_cleanup_files
_overlay_base_scratch_mount
_check_mounted_on

Quoted 8 lines

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

On Tue, Feb 14, 2017 at 08:02:27AM +0200, Amir Goldstein wrote:
[snip]

Quoted 52 lines

Quoted 36 lines

Quoted 29 lines

Quoted 29 lines

Quoted 28 lines

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.

Quoted 6 lines

Quoted 6 lines

Actually, there that test already exists in:

_scratch_mkfs
_scratch_cleanup_files
_overlay_base_scratch_mount
_check_mounted_on

Quoted 8 lines

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.

Quoted 4 lines

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

Quoted 61 lines · Amir Goldstein

Quoted 60 lines · Amir Goldstein

Quoted 27 lines · Eryu Guan

Quoted 26 lines · 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 <[email protected]>

---
 common/rc | 83 +++++++++++++++++++++++++++++++++++++--------------------------
 1 file changed, 49 insertions(+), 34 deletions(-)

...

Quoted 28 lines

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.

Quoted 6 lines

Quoted 6 lines

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?

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 <[email protected]>

---
 check     | 22 ++++++++++------------
 common/rc |  6 +++---
 2 files changed, 13 insertions(+), 15 deletions(-)

check +1 -1

index 5a93c94..ec9798a 100755
@@ -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

check +5 -6

 		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
 

common/rc +3 -3

index d831b17..00afb31 100644
@@ -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

Signature · 1 line

2.7.4

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 <[email protected]>

---
 README        | 16 ++++++-----
 check         |  2 +-
 common/config | 85 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
 common/rc     | 23 +++++++++-------
 4 files changed, 105 insertions(+), 21 deletions(-)

README +9 -7

index bb42985..842e835 100644
@@ -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

check +1 -1

index ec9798a..c51fc0c 100755
@@ -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 ;

common/config +70 -3

index a7ab2d9..5d90138 100644
@@ -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

common/config +6 -0

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

common/rc +13 -10

index 4e0cbdf..74168ea 100644
@@ -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"

Signature · 1 line

2.7.4

Quoted 28 lines · 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 <[email protected]>

---

Quoted 19 lines

# Parse config section options. This function will parse all the configuration

common/config +9 -0

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

common/config get_next_config() { · line 619

Quoted 9 lines

common/config +6 -0

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

Quoted 29 lines · Eryu Guan

Quoted 28 lines · 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 <[email protected]>

---

...

Quoted 41 lines

Quoted 19 lines

# Parse config section options. This function will parse all the configuration

common/config +9 -0

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

common/config get_next_config() { · line 619

Quoted 9 lines

common/config +6 -0

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

Quoted 6 lines

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.

Quoted 1 line

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.

Quoted 72 lines · Amir Goldstein

Quoted 29 lines · Eryu Guan

Quoted 28 lines · 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 <[email protected]>

---

...

Quoted 41 lines

Quoted 19 lines

# Parse config section options. This function will parse all the configuration

common/config +9 -0

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

common/config get_next_config() { · line 619

Quoted 9 lines

common/config +6 -0

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

Quoted 6 lines

Quoted 6 lines

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

Quoted 5 lines

Quoted 1 line

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.

Quoted 3 lines

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

Quoted 1 line

Quoted 2 lines

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

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 <[email protected]>

---
 common/filter | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

common/filter +4 -4

index 4328159..ef20ea6 100644
@@ -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

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 <[email protected]>

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

README.config-sections +6 -0

index df7c929..d45d6da 100644
@@ -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"

common/rc +0 -0

index 74168ea..e9ac0df 100644
@@ -328,24 +328,70 @@ _overlay_mount()
 			    $SELINUX_MOUNT_OPTIONS $* $dir $mnt
 }
 

Quoted 65 lines · 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 <[email protected]>

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

README.config-sections +6 -0

index df7c929..d45d6da 100644
@@ -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"

common/rc +0 -0

index 74168ea..e9ac0df 100644
@@ -328,24 +328,70 @@ _overlay_mount()
                          $SELINUX_MOUNT_OPTIONS $* $dir $mnt
 }
 

common/rc _overlay_mount() · line 340

Quoted 10 lines

common/rc +10 -0

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

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.

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

Quoted 4 lines · 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.

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:

Quoted 8 lines

README.config-sections +0 -0

index df7c929..d45d6da 100644
@@ -102,6 +102,9 @@ MKFS_OPTIONS="-q -F -b4096"
 FSTYP=ext4
 RESULT_BASE="`pwd`/results/`date +%d%m%y_%H%M%S`"
 

README.config-sections MKFS_OPTIONS="-q -F -b4096" · line 109

Quoted 5 lines

README.config-sections +3 -0

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

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

Quoted 7 lines · Amir Goldstein

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

Quoted 11 lines · Theodore Ts'o

Quoted 7 lines · Amir Goldstein

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:

Quoted 9 lines

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.

Quoted 2 lines

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

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

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

Signature · 6 lines

2.7.4

--
To unsubscribe from this list: send the line "unsubscribe fstests" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Quoted 4 lines · 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.

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

Thanks,
Xiong

Quoted 108 lines

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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 8 lines · Xiong Zhou

Quoted 4 lines · 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.

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.

Quoted 111 lines

Thanks,
Xiong

Quoted 108 lines

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 [email protected]
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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 14 lines · Amir Goldstein

Quoted 8 lines · Xiong Zhou

Quoted 4 lines · 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.

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.

Quoted 7 lines

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.

Quoted 24 lines

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.

Quoted 3 lines

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 ?

Quoted 53 lines · Xiong Zhou

Quoted 14 lines · Amir Goldstein

Quoted 8 lines · Xiong Zhou

Quoted 4 lines · 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.

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.

Quoted 7 lines

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.

Quoted 24 lines

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.

Quoted 3 lines

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

Quoted 6 lines

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.

Quoted 7 lines

Quoted 7 lines

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.

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.

Quoted 66 lines · Amir Goldstein

Quoted 53 lines · Xiong Zhou

Quoted 14 lines · Amir Goldstein

Quoted 8 lines · Xiong Zhou

Quoted 4 lines · 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.

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.

Quoted 7 lines

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.

Quoted 24 lines

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.

Quoted 3 lines

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

Quoted 6 lines

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

Quoted 2 lines

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

Quoted 8 lines

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

Quoted 5 lines

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:

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

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.

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

Signature · 1 line

2.7.4

Quoted 8 lines · 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.

Quoted 8 lines

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.

Quoted 10 lines

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!

Quoted 17 lines

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.

Quoted 3 lines

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.

Quoted 56 lines

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