[xfstests PATCH v3 0/6] overlay: add overlay filesystem dirs check

[xfstests PATCH v3 0/6] overlay: add overlay filesystem dirs check

[zhangyi (F)]

Hi, Eryu:

I have handle all comments from last iterations and fix some mistakes, please
review.

Patch 1-5: hook overlay filesystem check and correct current test cases.
Patch 6: handle the test mount options issue, it add the counterpart of the
         scratch mount options.

Thanks,
Yi.

Changes since v2:
- Rename _is_mounted to _is_dev_mounted and use findmnt instead of mount.
- Fix mistakes in patch 2/6 and introduce _is_dir_mountpoint helper, also
  introduce fsck options use for overlayfs.
- Add overlay/049 into patch 4/6 to skip fs check.
- Add nfs export cases into patch 5/6 to do correct dirs check.
- Add the "missing feature" of test mount options.

Changes since v1:
- Details comments in patch 1/5.
- Improve _overlay_check_scratch_dirs to accept extra options.
- Fix basic filesystem mounted check in _overlay_check_fs.
- Improve overlay/044 in patch 5/5 to add index=on option.

----------

Background:

This patchset implement filesystem check for overlay filesystem, base on
my "overlay: add fsck.overlay basic tests" patchset and works only if
fsck.overlay[1] exists (otherwise not run).

[1] https://github.com/hisilicon/overlayfs-progs

zhangyi (F) (6):
  common/rc: improve dev mounted check helper
  overlay: hook filesystem check helper
  overlay/003: fix fs check failure
  overlay: skip check for tests finished with corrupt filesystem
  overlay: correct scratch dirs check
  overlay: correct test mount options

 README.overlay    |  10 +++-
 common/config     |  21 ++++++--
 common/overlay    | 152 +++++++++++++++++++++++++++++++++++++++++++++++++++---
 common/rc         |  47 +++++++++--------
 common/xfs        |   2 +-
 tests/overlay/003 |   1 -
 tests/overlay/005 |   7 ++-
 tests/overlay/010 |   7 ++-
 tests/overlay/014 |  10 +++-
 tests/overlay/019 |   2 +-
 tests/overlay/022 |   2 +-
 tests/overlay/025 |   2 +-
 tests/overlay/029 |   6 +--
 tests/overlay/031 |   2 +-
 tests/overlay/035 |   7 ++-
 tests/overlay/036 |  26 ++++++----
 tests/overlay/037 |   7 ++-
 tests/overlay/038 |  10 +++-
 tests/overlay/041 |  10 +++-
 tests/overlay/043 |   7 ++-
 tests/overlay/044 |   8 ++-
 tests/overlay/049 |   2 +-
 tests/overlay/051 |  19 ++++++-
 tests/overlay/053 |  15 +++++-
 tests/overlay/055 |  12 ++++-
 25 files changed, 328 insertions(+), 66 deletions(-)

-- 
2.5.0

[xfstests PATCH v3 1/6] common/rc: improve dev mounted check helper

[zhangyi (F)]

There is a problem of missing fstype check in _is_mounted() helper,
it will return the mountpoint if only the device arguments matches.

For example:
  Base mounted filesystem:
    /dev/sda2 on /boot type ext4 (rw,relatime,data=ordered)

  FSTYPE=xfs
  mountpoint=`_is_mounted /dev/sda1`
  echo "$mountpoint"

  Output: /boot

This patch rename _is_mounted to _is_dev_mounted because it check
the given device only (not mount dir), and add an optional "fstype"
parameter, let user specify file system type instead of default
FSTYPE. Finally, use findmnt instead of mount to avoid complex
processing of mount info and fix this problem simply.

Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
---
 common/rc  | 29 ++++++++++-------------------
 common/xfs |  2 +-
 2 files changed, 11 insertions(+), 20 deletions(-)

diff --git a/common/rc b/common/rc
index 4ad59b1..2fae2be 100644
--- a/common/rc
+++ b/common/rc
@@ -2372,27 +2372,18 @@ _scratch_mkfs_richacl()
 	esac
 }
 
-# check that a FS on a device is mounted
-# if so, return mount point
-#
-_is_mounted()
+# check if the given device is mounted, if so, return mount point
+_is_dev_mounted()
 {
-    if [ $# -ne 1 ]
-    then
-	echo "Usage: _is_mounted device" 1>&2
-	exit 1
-    fi
+	local dev=$1
+	local fstype=${2:-$FSTYP}
 
-    device=$1
+	if [ $# -lt 1 ]; then
+		echo "Usage: _is_dev_mounted <device> [fstype]" 1>&2
+		exit 1
+	fi
 
-    if _mount | grep "$device " | $AWK_PROG -v pattern="type $FSTYP" '
-        pattern        { print $3 ; exit 0 }
-        END            { exit 1 }
-    '
-    then
-        echo "_is_mounted: $device is not a mounted $FSTYP FS"
-        exit 1
-    fi
+	findmnt -rncv -S $dev -t $fstype -o TARGET | head -1
 }
 
 # remount a FS to a new mode (ro or rw)
@@ -2432,7 +2423,7 @@ _umount_or_remount_ro()
     fi
 
     device=$1
-    mountpoint=`_is_mounted $device`
+    mountpoint=`_is_dev_mounted $device`
 
     if [ $USE_REMOUNT -eq 0 ]; then
         $UMOUNT_PROG $device
diff --git a/common/xfs b/common/xfs
index 9b20b03..572b084 100644
--- a/common/xfs
+++ b/common/xfs
@@ -355,7 +355,7 @@ _check_xfs_filesystem()
 	ok=1
 
 	# Run online scrub if we can.
-	mntpt="$(_is_mounted $device)"
+	mntpt="$(_is_dev_mounted $device)"
 	if [ -n "$mntpt" ] && _supports_xfs_scrub "$mntpt" "$device"; then
 		"$XFS_SCRUB_PROG" $scrubflag -v -d -n $device > $tmp.scrub 2>&1
 		if [ $? -ne 0 ]; then
-- 
2.5.0

[xfstests PATCH v3 2/6] overlay: hook filesystem check helper

[zhangyi (F)]

Hook filesystem check helper to _check_test_fs and _check_scratch_fs for
constants underlying dirs of overlay filesystem, and introduce scratch
check helpers for optionally lower/upper/work dirs. These helpers works
only if fsck.overlay exists.

This patch introduce OVERLAY_FSCK_OPTIONS use for check overlayfs like
OVERLAY_MOUNT_OPTIONS, and also introduce a mount point check helper in
common/rc to detect a dir is a mount point or not.

[ _check_test_fs/_check_scratch_fs part picked from Amir's patch, thanks ]

Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
---
 README.overlay |  10 ++++-
 common/config  |   4 ++
 common/overlay | 125 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 common/rc      |  18 ++++++++-
 4 files changed, 153 insertions(+), 4 deletions(-)

diff --git a/README.overlay b/README.overlay
index dfb8234..9feaa6a 100644
--- a/README.overlay
+++ b/README.overlay
@@ -22,6 +22,10 @@ the base fs should be pre-formatted before starting the -overlay run.
 An easy way to accomplish this is by running './check <some test>' once,
 before running './check -overlay'.
 
+'./check -overlay' support check overlay test and scratch dirs,
+OVERLAY_FSCK_OPTIONS should be set instead of FSCK_OPTIONS if fsck
+options need to given directly.
+
 Because of the lack of mkfs support, multi-section config files are only
 partly supported with './check -overlay'. Only multi-section files that
 do not change FSTYP and MKFS_OPTIONS can be safely used with -overlay.
@@ -40,7 +44,9 @@ run overlay tests on the same base fs, but with different mount options:
  MOUNT_OPTIONS="-o pquota"
  TEST_FS_MOUNT_OPTS="-o noatime"
  OVERLAY_MOUNT_OPTIONS="-o redirect_dir=off"
+ OVERLAY_FSCK_OPTIONS="-n"
 
 In the example above, MOUNT_OPTIONS will be used to mount the base scratch fs,
-TEST_FS_MOUNT_OPTS will be used to mount the base test fs and
-OVERLAY_MOUNT_OPTIONS will be used to mount both test and scratch overlays.
+TEST_FS_MOUNT_OPTS will be used to mount the base test fs,
+OVERLAY_MOUNT_OPTIONS will be used to mount both test and scratch overlay and
+OVERLAY_FSCK_OPTIONS will be used to check both test and scratch overlay.
diff --git a/common/config b/common/config
index 71115bd..20f0e5f 100644
--- a/common/config
+++ b/common/config
@@ -566,6 +566,10 @@ _overlay_config_override()
 	# 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"
+
+	# Set fsck options, use default if user not set directly.
+	export FSCK_OPTIONS="$OVERLAY_FSCK_OPTIONS"
+	[ -z "$FSCK_OPTIONS" ] && _fsck_opts
 }
 
 _overlay_config_restore()
diff --git a/common/overlay b/common/overlay
index a8b0e93..29f9bf8 100644
--- a/common/overlay
+++ b/common/overlay
@@ -190,3 +190,128 @@ _overlay_fsck_dirs()
 	$FSCK_OVERLAY_PROG -o lowerdir=$lowerdir -o upperdir=$upperdir \
 			   -o workdir=$workdir $*
 }
+
+_overlay_check_dirs()
+{
+	local lowerdir=$1
+	local upperdir=$2
+	local workdir=$3
+	local err=0
+
+	_overlay_fsck_dirs $* $FSCK_OPTIONS >>$tmp.fsck 2>&1
+	if [ $? -ne 0 ]; then
+		_log_err "_overlay_check_fs: overlayfs on $lowerdir,$upperdir,$workdir is inconsistent"
+
+		echo "*** fsck.overlay output ***"	>>$seqres.full
+		cat $tmp.fsck				>>$seqres.full
+		echo "*** end fsck.overlay output"	>>$seqres.full
+
+		echo "*** mount output ***"		>>$seqres.full
+		_mount					>>$seqres.full
+		echo "*** end mount output"		>>$seqres.full
+
+		err=1
+	fi
+	rm -f $tmp.fsck
+
+	return $err
+}
+
+# Check the same mnt/dev of _check_overlay_scratch_fs, but check
+# optionally lower/upper/work dirs of overlay filesystem, such as
+# multiple lower layers.
+_overlay_check_scratch_dirs()
+{
+	local lowerdir=$1
+	local upperdir=$2
+	local workdir=$3
+	shift 3
+
+	# Need to umount overlay for scratch dir check
+	local ovl_mounted=`_is_dir_mountpoint $SCRATCH_MNT`
+	[ -z "$ovl_mounted" ] || $UMOUNT_PROG $SCRATCH_MNT
+
+	# Check dirs with extra overlay options
+	_overlay_check_dirs $lowerdir $upperdir $workdir $*
+	local ret=$?
+
+	if [ $ret -eq 0 -a -n "$ovl_mounted" ]; then
+		# overlay was mounted, remount with extra mount options
+		_overlay_scratch_mount_dirs $lowerdir $upperdir \
+					    $workdir $*
+		ret=$?
+	fi
+
+	return $ret
+}
+
+_overlay_check_fs()
+{
+	# The first arguments is overlay mount point use for checking
+	# overlay filesystem is mounted or not, the remaining arquments
+	# use for mounting overlay base filesystem if it was not mounted.
+	# We shift one to aligns arguments for _overlay_base_mount.
+	local ovl_mnt=$1
+	shift 1
+
+	local base_dev=$3
+	local base_mnt=$4
+
+	[ "$FSTYP" = overlay ] || return 0
+
+	# Base fs needs to be mounted to check overlay dirs
+	local base_fstype=""
+	local ovl_mounted=""
+
+	[ -z "$base_dev" ] || \
+		base_fstype=`_fs_type $base_dev`
+
+	# If base_dev is set but base_fstype is empty, base fs is not
+	# mounted, we need to mount base fs. Otherwise, we need to
+	# check and umount overlayfs if it was mounted.
+	if [ -n "$base_dev" -a -z "$base_fstype" ]; then
+		_overlay_base_mount $*
+	else
+		# Check and umount overlay for dir check
+		ovl_mounted=`_is_dir_mountpoint $ovl_mnt`
+		[ -z "$ovl_mounted" ] || $UMOUNT_PROG $ovl_mnt
+	fi
+
+	_overlay_check_dirs $base_mnt/$OVL_LOWER $base_mnt/$OVL_UPPER \
+			    $base_mnt/$OVL_WORK
+	local ret=$?
+
+	if [ -n "$base_dev" -a -z "$base_fstype" ]; then
+		_overlay_base_unmount "$base_dev" "$base_mnt"
+	elif [ $ret -eq 0 -a -n "$ovl_mounted" ]; then
+		# overlay was mounted, remount besides extra mount options
+		_overlay_mount $base_mnt $ovl_mnt
+		ret=$?
+	fi
+
+	if [ $ret != 0 ]; then
+		status=1
+		if [ "$iam" != "check" ]; then
+			exit 1
+		fi
+		return 1
+	fi
+
+	return 0
+}
+
+_check_overlay_test_fs()
+{
+	_overlay_check_fs "$TEST_DIR" \
+		OVL_BASE_TEST_DEV OVL_BASE_TEST_DIR \
+		"$OVL_BASE_TEST_DEV" "$OVL_BASE_TEST_DIR" \
+		$TEST_FS_MOUNT_OPTS $SELINUX_MOUNT_OPTIONS
+}
+
+_check_overlay_scratch_fs()
+{
+	_overlay_check_fs "$SCRATCH_MNT" \
+		OVL_BASE_SCRATCH_DEV OVL_BASE_SCRATCH_MNT \
+		"$OVL_BASE_SCRATCH_DEV" "$OVL_BASE_SCRATCH_MNT" \
+		$OVL_BASE_MOUNT_OPTIONS $SELINUX_MOUNT_OPTIONS
+}
diff --git a/common/rc b/common/rc
index 2fae2be..5d77229 100644
--- a/common/rc
+++ b/common/rc
@@ -2386,6 +2386,20 @@ _is_dev_mounted()
 	findmnt -rncv -S $dev -t $fstype -o TARGET | head -1
 }
 
+# check if the given dir is a mount point, if so, return mount point
+_is_dir_mountpoint()
+{
+	local dir=$1
+	local fstype=${2:-$FSTYP}
+
+	if [ $# -lt 1 ]; then
+		echo "Uasge: _is_dir_mountpoint <dir> [fstype]" 1>&2
+		exit 1
+	fi
+
+	findmnt -rncv -t $fstype -o TARGET $dir | head -1
+}
+
 # remount a FS to a new mode (ro or rw)
 #
 _remount()
@@ -2581,7 +2595,7 @@ _check_test_fs()
 	# no way to check consistency for GlusterFS
 	;;
     overlay)
-	# no way to check consistency for overlay
+	_check_overlay_test_fs
 	;;
     pvfs2)
 	;;
@@ -2639,7 +2653,7 @@ _check_scratch_fs()
 	# no way to check consistency for GlusterFS
 	;;
     overlay)
-	# no way to check consistency for overlay
+	_check_overlay_scratch_fs
 	;;
     pvfs2)
 	;;
-- 
2.5.0

[xfstests PATCH v3 3/6] overlay/003: fix fs check failure

[zhangyi (F)]

_check_overlay_scratch_fs() will check lowerdir of overlay filesystem,
this case remove this directory after test will lead to check failure,
and it is not really necessary to remove this directory, so keep this
directory.

Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
---
 tests/overlay/003 | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tests/overlay/003 b/tests/overlay/003
index f980edb..154531e 100755
--- a/tests/overlay/003
+++ b/tests/overlay/003
@@ -92,7 +92,6 @@ ls ${SCRATCH_MNT}/
 # unmount overlayfs but not base fs
 $UMOUNT_PROG $SCRATCH_MNT
 
-rm -rf $lowerdir
 echo "Silence is golden"
 # success, all done
 status=0
-- 
2.5.0

[xfstests PATCH v3 4/6] overlay: skip check for tests finished with corrupt filesystem

[zhangyi (F)]

No post-test check of the overlay dirs is required if case leaves
corrupt filesystem after test. We shoud use _require_scratch_nocheck()
instead of _require_scratch() in these cases.

Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
---
 tests/overlay/019 | 2 +-
 tests/overlay/031 | 2 +-
 tests/overlay/049 | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/overlay/019 b/tests/overlay/019
index 3e2bc4e..575aaf7 100755
--- a/tests/overlay/019
+++ b/tests/overlay/019
@@ -46,7 +46,7 @@ rm -f $seqres.full
 # real QA test starts here
 _supported_fs overlay
 _supported_os Linux
-_require_scratch
+_require_scratch_nocheck
 
 # Remove all files from previous tests
 _scratch_mkfs
diff --git a/tests/overlay/031 b/tests/overlay/031
index 186b409..90a06af 100755
--- a/tests/overlay/031
+++ b/tests/overlay/031
@@ -67,7 +67,7 @@ rm -f $seqres.full
 # real QA test starts here
 _supported_fs overlay
 _supported_os Linux
-_require_scratch
+_require_scratch_nocheck
 
 # remove all files from previous runs
 _scratch_mkfs
diff --git a/tests/overlay/049 b/tests/overlay/049
index ef00138..1dafc46 100755
--- a/tests/overlay/049
+++ b/tests/overlay/049
@@ -72,7 +72,7 @@ rm -f $seqres.full
 # real QA test starts here
 _supported_fs overlay
 _supported_os Linux
-_require_scratch
+_require_scratch_nocheck
 _require_scratch_feature redirect_dir
 
 # remove all files from previous runs
-- 
2.5.0

[xfstests PATCH v3 5/6] overlay: correct scratch dirs check

[zhangyi (F)]

Tests that use _overlay_scratch_mount_dirs instead of _scratch_mount
should use _require_overlay_scratch_dirs instead of _require_scratch
because these tests are either mounting with multiple lower dirs or
mounting with non-default lower/upper/work dir, so
_check_overlay_scratch_fs won't handle these cases correctly, we need
to call _overlay_check_scratch_dirs with the correct arguments.

This patch modify these tests to optionally call
_overlay_check_scratch_dirs at the end of the test or before
_scratch_umount to umount $SCRATCH_MNT and run the checker.

Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
---
 tests/overlay/005 |  7 ++++++-
 tests/overlay/010 |  7 ++++++-
 tests/overlay/014 | 10 +++++++++-
 tests/overlay/035 |  7 ++++++-
 tests/overlay/036 |  6 +++++-
 tests/overlay/037 |  7 ++++++-
 tests/overlay/038 | 10 +++++++++-
 tests/overlay/041 | 10 +++++++++-
 tests/overlay/043 |  7 ++++++-
 tests/overlay/044 |  8 +++++++-
 tests/overlay/051 | 19 ++++++++++++++++++-
 tests/overlay/053 | 15 ++++++++++++++-
 tests/overlay/055 | 12 +++++++++++-
 13 files changed, 112 insertions(+), 13 deletions(-)

diff --git a/tests/overlay/005 b/tests/overlay/005
index 17992a3..87fc9bb 100755
--- a/tests/overlay/005
+++ b/tests/overlay/005
@@ -54,7 +54,9 @@ rm -f $seqres.full
 # Modify as appropriate.
 _supported_fs overlay
 _supported_os Linux
-_require_scratch
+# Use non-default scratch underlying overlay dirs, we need to check
+# them explicity after test.
+_require_scratch_nocheck
 _require_loop
 
 # Remove all files from previous tests
@@ -102,6 +104,9 @@ $XFS_IO_PROG -f -c "o" ${SCRATCH_MNT}/test_file \
 # unmount overlayfs
 $UMOUNT_PROG $SCRATCH_MNT
 
+# check overlayfs
+_overlay_check_scratch_dirs $lowerd $upperd $workd
+
 # unmount undelying xfs, this tiggers panic if memleak happens
 $UMOUNT_PROG ${OVL_BASE_SCRATCH_MNT}/uppermnt
 $UMOUNT_PROG ${OVL_BASE_SCRATCH_MNT}/lowermnt
diff --git a/tests/overlay/010 b/tests/overlay/010
index f55ebec..32af89c 100755
--- a/tests/overlay/010
+++ b/tests/overlay/010
@@ -48,7 +48,9 @@ rm -f $seqres.full
 # real QA test starts here
 _supported_fs overlay
 _supported_os Linux
-_require_scratch
+# Use non-default scratch underlying overlay dirs, we need to check
+# them explicity after test.
+_require_scratch_nocheck
 
 # Remove all files from previous tests
 _scratch_mkfs
@@ -70,6 +72,9 @@ mknod $lowerdir2/testdir/a c 0 0
 _overlay_scratch_mount_dirs "$lowerdir2:$lowerdir1" $upperdir $workdir
 rm -rf $SCRATCH_MNT/testdir
 
+# check overlayfs
+_overlay_check_scratch_dirs "$lowerdir2:$lowerdir1" $upperdir $workdir
+
 # success, all done
 echo "Silence is golden"
 status=0
diff --git a/tests/overlay/014 b/tests/overlay/014
index 9f308d3..67fad9f 100755
--- a/tests/overlay/014
+++ b/tests/overlay/014
@@ -53,7 +53,9 @@ rm -f $seqres.full
 # real QA test starts here
 _supported_fs overlay
 _supported_os Linux
-_require_scratch
+# Use non-default scratch underlying overlay dirs, we need to check
+# them explicity after test.
+_require_scratch_nocheck
 
 # Remove all files from previous tests
 _scratch_mkfs
@@ -78,6 +80,9 @@ mkdir -p $SCRATCH_MNT/testdir/visibledir
 # unmount overlayfs but not base fs
 $UMOUNT_PROG $SCRATCH_MNT
 
+# check overlayfs
+_overlay_check_scratch_dirs $lowerdir1 $lowerdir2 $workdir2
+
 # mount overlay again, with lowerdir1 and lowerdir2 as multiple lowerdirs,
 # and create a new file in testdir, triggers copyup from lowerdir,
 # copyup should not copy overlayfs private xattr
@@ -90,6 +95,9 @@ $UMOUNT_PROG $SCRATCH_MNT
 _overlay_scratch_mount_dirs "$lowerdir2:$lowerdir1" $upperdir $workdir
 ls $SCRATCH_MNT/testdir
 
+# check overlayfs
+_overlay_check_scratch_dirs "$lowerdir2:$lowerdir1" $upperdir $workdir
+
 # success, all done
 status=0
 exit
diff --git a/tests/overlay/035 b/tests/overlay/035
index 0544774..d1b2c19 100755
--- a/tests/overlay/035
+++ b/tests/overlay/035
@@ -51,7 +51,9 @@ rm -f $seqres.full
 # real QA test starts here
 _supported_fs overlay
 _supported_os Linux
-_require_scratch
+# Use non-default scratch underlying overlay dirs, we need to check
+# them explicity after test.
+_require_scratch_nocheck
 _require_chattr i
 
 # Remove all files from previous tests
@@ -81,6 +83,9 @@ _overlay_scratch_mount_dirs $lowerdir2 $upperdir $workdir
 touch $SCRATCH_MNT/bar 2>&1 | _filter_scratch
 _scratch_remount rw 2>&1 | _filter_ro_mount
 
+# check overlayfs
+_overlay_check_scratch_dirs $lowerdir2 $upperdir $workdir
+
 # success, all done
 status=0
 exit
diff --git a/tests/overlay/036 b/tests/overlay/036
index e04aaee..8d3ccf4 100755
--- a/tests/overlay/036
+++ b/tests/overlay/036
@@ -69,7 +69,9 @@ rm -f $seqres.full
 # real QA test starts here
 _supported_fs overlay
 _supported_os Linux
-_require_scratch
+# Use non-default scratch underlying overlay dirs, we need to check
+# them explicity after test.
+_require_scratch_nocheck
 _require_scratch_feature index
 
 # Remove all files from previous tests
@@ -110,6 +112,8 @@ _overlay_mount_dirs $lowerdir $upperdir $workdir2 \
 _overlay_mount_dirs $lowerdir2 $upperdir2 $workdir \
 	    overlay3 $SCRATCH_MNT -oindex=on 2>&1 | _filter_busy_mount
 
+# check overlayfs
+_overlay_check_scratch_dirs $lowerdir $upperdir $workdir
 
 # success, all done
 status=0
diff --git a/tests/overlay/037 b/tests/overlay/037
index 6710dda..4e2e5df 100755
--- a/tests/overlay/037
+++ b/tests/overlay/037
@@ -55,7 +55,9 @@ rm -f $seqres.full
 # real QA test starts here
 _supported_fs overlay
 _supported_os Linux
-_require_scratch
+# Use non-default scratch underlying overlay dirs, we need to check
+# them explicity after test.
+_require_scratch_nocheck
 _require_scratch_feature index
 
 # Remove all files from previous tests
@@ -87,6 +89,9 @@ $UMOUNT_PROG $SCRATCH_MNT 2>/dev/null
 # Mount overlay with original lowerdir, upperdir, workdir and index=on - expect success
 _overlay_scratch_mount_dirs $lowerdir $upperdir $workdir -oindex=on
 
+# check overlayfs
+_overlay_check_scratch_dirs $lowerdir $upperdir $workdir -oindex=on
+
 # success, all done
 status=0
 exit
diff --git a/tests/overlay/038 b/tests/overlay/038
index bd87156..76a717b 100755
--- a/tests/overlay/038
+++ b/tests/overlay/038
@@ -46,7 +46,9 @@ _cleanup()
 # real QA test starts here
 _supported_fs overlay
 _supported_os Linux
-_require_scratch
+# Use non-default scratch underlying overlay dirs, we need to check
+# them explicity after test.
+_require_scratch_nocheck
 _require_attrs
 _require_test_program "t_dir_type"
 
@@ -161,6 +163,9 @@ subdir_d=$($here/src/t_dir_type $pure_lower_dir $pure_lower_subdir_st_ino)
 [[ $subdir_d == "subdir d" ]] || \
 	echo "Merged dir: Invalid d_ino reported for subdir"
 
+# check overlayfs
+_check_scratch_fs
+
 _scratch_unmount
 
 # Verify pure lower residing in dir which has another lower layer
@@ -202,6 +207,9 @@ subdir_d=$($here/src/t_dir_type $pure_lower_dir $pure_lower_subdir_st_ino)
 [[ $subdir_d == "subdir d" ]] || \
 	echo "Pure lower in dir which has another lower layer: Invalid d_ino reported for subdir"
 
+# check overlayfs
+_overlay_check_scratch_dirs "$middir:$lowerdir" $upperdir $workdir
+
 echo "Silence is golden"
 status=0
 exit
diff --git a/tests/overlay/041 b/tests/overlay/041
index 11efacb..da50e0b 100755
--- a/tests/overlay/041
+++ b/tests/overlay/041
@@ -48,7 +48,9 @@ _cleanup()
 # real QA test starts here
 _supported_fs overlay
 _supported_os Linux
-_require_scratch
+# Use non-default scratch underlying overlay dirs, we need to check
+# them explicity after test.
+_require_scratch_nocheck
 _require_test
 _require_attrs
 _require_test_program "t_dir_type"
@@ -166,6 +168,9 @@ subdir_d=$($here/src/t_dir_type $pure_lower_dir $pure_lower_subdir_st_ino)
 [[ $subdir_d == "subdir d" ]] || \
 	echo "Merged dir: Invalid d_ino reported for subdir"
 
+# check overlayfs
+_overlay_check_scratch_dirs $lowerdir $upperdir $workdir
+
 _scratch_unmount
 
 # Verify pure lower residing in dir which has another lower layer
@@ -206,6 +211,9 @@ subdir_d=$($here/src/t_dir_type $pure_lower_dir $pure_lower_subdir_st_ino)
 [[ $subdir_d == "subdir d" ]] || \
 	echo "Pure lower in dir which has another lower layer: Invalid d_ino reported for subdir"
 
+# check overlayfs
+_overlay_check_scratch_dirs "$middir:$lowerdir" $upperdir $workdir
+
 echo "Silence is golden"
 status=0
 exit
diff --git a/tests/overlay/043 b/tests/overlay/043
index 699c4e1..46df686 100755
--- a/tests/overlay/043
+++ b/tests/overlay/043
@@ -56,7 +56,9 @@ _cleanup()
 # real QA test starts here
 _supported_fs overlay
 _supported_os Linux
-_require_scratch
+# Use non-default scratch underlying overlay dirs, we need to check
+# them explicity after test.
+_require_scratch_nocheck
 _require_test
 _require_test_program "af_unix"
 _require_test_program "t_dir_type"
@@ -153,6 +155,9 @@ _overlay_scratch_mount_dirs $lowerdir $upperdir $workdir
 # Compare inode numbers before/after mount cycle
 check_inode_numbers $testdir $tmp.after_move $tmp.after_cycle
 
+# check overlayfs
+_overlay_check_scratch_dirs $lowerdir $upperdir $workdir
+
 echo "Silence is golden"
 status=0
 exit
diff --git a/tests/overlay/044 b/tests/overlay/044
index e57f6f7..2ab3035 100755
--- a/tests/overlay/044
+++ b/tests/overlay/044
@@ -49,7 +49,9 @@ _cleanup()
 # real QA test starts here
 _supported_fs overlay
 _supported_os Linux
-_require_scratch
+# Use non-default scratch underlying overlay dirs, we need to check
+# them explicity after test.
+_require_scratch_nocheck
 _require_test
 _require_scratch_feature index
 _require_test_program "t_dir_type"
@@ -122,6 +124,7 @@ check_ino_nlink $SCRATCH_MNT $tmp.before $tmp.after_one
 
 # Verify that the hardlinks survive a mount cycle
 $UMOUNT_PROG $SCRATCH_MNT
+_overlay_check_scratch_dirs $lowerdir $upperdir $workdir -o index=on
 _overlay_scratch_mount_dirs $lowerdir $upperdir $workdir -o index=on
 
 echo "== After mount cycle =="
@@ -138,5 +141,8 @@ echo "== After write two =="
 cat $FILES
 check_ino_nlink $SCRATCH_MNT $tmp.after_one $tmp.after_two
 
+# check overlayfs
+_overlay_check_scratch_dirs $lowerdir $upperdir $workdir -o index=on
+
 status=0
 exit
diff --git a/tests/overlay/051 b/tests/overlay/051
index ae7844d..b888f1e 100755
--- a/tests/overlay/051
+++ b/tests/overlay/051
@@ -61,8 +61,10 @@ _cleanup()
 _supported_fs overlay
 _supported_os Linux
 _require_test
-_require_scratch
 _require_test_program "open_by_handle"
+# Use non-default scratch underlying overlay dirs, we need to check
+# them explicity after test.
+_require_scratch_nocheck
 # We need to require both features together, because nfs_export cannot
 # be enabled when index is disabled
 _require_scratch_overlay_features index nfs_export
@@ -128,6 +130,13 @@ unmount_dirs()
 	$UMOUNT_PROG $SCRATCH_MNT
 }
 
+# Check underlying dirs of overlayfs
+check_dirs()
+{
+	_overlay_check_scratch_dirs $middle:$lower $upper $work \
+				-o "index=on,nfs_export=on"
+}
+
 # Check non-stale file handles of lower/upper files and verify
 # that handle encoded before copy up is decoded to upper after
 # copy up. Verify reading data from file open by file handle
@@ -149,6 +158,7 @@ test_file_handles $SCRATCH_MNT/lowertestdir -p
 # Check encode/write/decode/read/write of lower file handles across copy up
 test_file_handles $SCRATCH_MNT/lowertestdir -wrap
 unmount_dirs
+check_dirs
 
 # Check copy up after encode/decode of lower/upper files
 # (copy up of disconnected dentry to index dir)
@@ -163,6 +173,7 @@ test_file_handles $SCRATCH_MNT/uppertestdir -r
 test_file_handles $SCRATCH_MNT/lowertestdir -a
 test_file_handles $SCRATCH_MNT/lowertestdir -r
 unmount_dirs
+check_dirs
 
 # Check non-stale handles to unlinked but open lower/upper files
 create_dirs
@@ -178,6 +189,7 @@ test_file_handles $SCRATCH_MNT/lowertestdir -dk
 # Check encode/write/unlink/decode/read of lower file handles across copy up
 test_file_handles $SCRATCH_MNT/lowertestdir.rw -rwdk
 unmount_dirs
+check_dirs
 
 # Check stale handles of unlinked lower/upper files (nlink = 0)
 create_dirs
@@ -189,6 +201,7 @@ test_file_handles $SCRATCH_MNT/uppertestdir -dp
 # Check decode of lower file handles after unlink/rmdir (nlink == 0)
 test_file_handles $SCRATCH_MNT/lowertestdir -dp
 unmount_dirs
+check_dirs
 
 # Check non-stale file handles of linked lower/upper files (nlink = 2,1)
 create_dirs
@@ -204,6 +217,7 @@ test_file_handles $SCRATCH_MNT/lowertestdir -wlr
 # Check decode/read of lower file handles after copy up + link + unlink (nlink == 1)
 test_file_handles $SCRATCH_MNT/lowertestdir -ur
 unmount_dirs
+check_dirs
 
 # Check non-stale file handles of linked lower/upper hardlinks (nlink = 2,1)
 create_dirs
@@ -222,6 +236,7 @@ test_file_handles $SCRATCH_MNT/lowertestdir
 # Check decode/read of lower hardlink file handles after copy up + unlink (nlink == 1)
 test_file_handles $SCRATCH_MNT/lowertestdir -wur
 unmount_dirs
+check_dirs
 
 # Check stale file handles of unlinked lower/upper hardlinks (nlink = 2,0)
 create_dirs
@@ -240,6 +255,7 @@ test_file_handles $SCRATCH_MNT/lowertestdir
 # Check decode of lower hardlink file handles after copy up + 2*unlink (nlink == 0)
 test_file_handles $SCRATCH_MNT/lowertestdir -d
 unmount_dirs
+check_dirs
 
 # Check non-stale file handles of lower/upper renamed files
 create_dirs
@@ -251,6 +267,7 @@ test_file_handles $SCRATCH_MNT/uppertestdir -wmr
 # Check decode/read of lower file handles after copy up + rename in same merge parent
 test_file_handles $SCRATCH_MNT/lowertestdir -wmr
 unmount_dirs
+check_dirs
 
 status=0
 exit
diff --git a/tests/overlay/053 b/tests/overlay/053
index d4fdcde..1857d85 100755
--- a/tests/overlay/053
+++ b/tests/overlay/053
@@ -63,8 +63,10 @@ _cleanup()
 _supported_fs overlay
 _supported_os Linux
 _require_test
-_require_scratch
 _require_test_program "open_by_handle"
+# Use non-default scratch underlying overlay dirs, we need to check
+# them explicity after test.
+_require_scratch_nocheck
 # We need to require all features together, because nfs_export cannot
 # be enabled when index is disabled
 _require_scratch_overlay_features index nfs_export redirect_dir
@@ -124,6 +126,13 @@ unmount_dirs()
 	$UMOUNT_PROG $SCRATCH_MNT
 }
 
+# Check underlying dirs of overlayfs
+check_dirs()
+{
+	_overlay_check_scratch_dirs $middle:$lower $upper $work \
+				-o "index=on,nfs_export=on,redirect_dir=on"
+}
+
 # Check non-stale file handles of lower/upper moved files
 create_dirs
 create_test_files $upper/uppertestdir -w
@@ -147,6 +156,7 @@ mv $SCRATCH_MNT/uppertestdir.lo/* $SCRATCH_MNT/lowertestdir.lo/
 test_file_handles $SCRATCH_MNT -r -i $tmp.upper_file_handles
 test_file_handles $SCRATCH_MNT -r -i $tmp.lower_file_handles
 unmount_dirs
+check_dirs
 
 # Check non-stale file handles of lower/upper renamed dirs
 create_dirs
@@ -176,6 +186,7 @@ test_file_handles $SCRATCH_MNT -rp -i $tmp.lower_subdir_file_handles
 # (providing renamed testdir argument pins the indexed testdir to dcache)
 test_file_handles $SCRATCH_MNT/lowertestdir.new -rp -i $tmp.lower_subdir_file_handles
 unmount_dirs
+check_dirs
 
 # Check encode/decode/read of lower file handles on lower layers only r/o overlay.
 # For non-upper overlay mount, nfs_export requires disabling redirect_dir.
@@ -184,6 +195,7 @@ $MOUNT_PROG -t overlay $OVL_BASE_SCRATCH_MNT $SCRATCH_MNT \
 test_file_handles $SCRATCH_MNT/lowertestdir -rp
 test_file_handles $SCRATCH_MNT/lowertestdir/subdir -rp
 unmount_dirs
+check_dirs
 
 # Check encode/decode/read of lower file handles on lower layers only r/o overlay
 # with non-indexed redirects from top lower layer to bottom lower layer.
@@ -195,6 +207,7 @@ $MOUNT_PROG -t overlay $OVL_BASE_SCRATCH_MNT $SCRATCH_MNT \
 test_file_handles $SCRATCH_MNT -r
 test_file_handles $SCRATCH_MNT/subdir -rp
 unmount_dirs
+check_dirs
 
 status=0
 exit
diff --git a/tests/overlay/055 b/tests/overlay/055
index 70fe6ac..0e1c6d2 100755
--- a/tests/overlay/055
+++ b/tests/overlay/055
@@ -67,8 +67,10 @@ _cleanup()
 _supported_fs overlay
 _supported_os Linux
 _require_test
-_require_scratch
 _require_test_program "open_by_handle"
+# Use non-default scratch underlying overlay dirs, we need to check
+# them explicity after test.
+_require_scratch_nocheck
 # We need to require all features together, because nfs_export cannot
 # be enabled when index is disabled
 _require_scratch_overlay_features index nfs_export redirect_dir
@@ -128,6 +130,13 @@ unmount_dirs()
 	$UMOUNT_PROG $SCRATCH_MNT
 }
 
+# Check underlying dirs of overlayfs
+check_dirs()
+{
+	_overlay_check_scratch_dirs $middle:$lower $upper $work \
+				-o "index=on,nfs_export=on,redirect_dir=on"
+}
+
 # Check file handles of dir with ancestor under lower redirect
 create_dirs
 create_test_files $lower/origin -w
@@ -165,6 +174,7 @@ test_file_handles $SCRATCH_MNT/merged.new/dir -rp -i $tmp.dir_file_handles
 test_file_handles $SCRATCH_MNT/merged.new/dir -rp -i $tmp.subdir_file_handles
 test_file_handles $SCRATCH_MNT/merged.new/parent -rp -i $tmp.child_file_handles
 unmount_dirs
+check_dirs
 
 status=0
 exit
-- 
2.5.0

[xfstests PATCH v3 6/6] overlay: correct test mount options

[zhangyi (F)]

Current overlay's _test_mount functions mount base test filesystem
with TEST_FS_MOUNT_OPTS and mount test overlayfs with MOUNT_OPTIONS
instead of TEST_FS_MOUNT_OPTS. The TEST_FS_MOUNT_OPTS variable
should be used for test overlayfs like MOUNT_OPTIONS use for scratch
overlayfs.

This patch rename OVL_BASE_MOUNT_OPTIONS to
OVL_BASE_SCRATCH_MOUNT_OPTIONS and introduce an counterpart variable
OVL_BASE_TEST_MOUNT_OPTIONS for test base filesystem, and then use
TEST_FS_MOUNT_OPTS for test overlayfs. We also modify overlay mount
helpers to adapt mount options.

Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
---
 common/config     | 17 +++++++++++++----
 common/overlay    | 31 +++++++++++++++++++++++--------
 tests/overlay/022 |  2 +-
 tests/overlay/025 |  2 +-
 tests/overlay/029 |  6 +++---
 tests/overlay/036 | 20 ++++++++++----------
 6 files changed, 51 insertions(+), 27 deletions(-)

diff --git a/common/config b/common/config
index 20f0e5f..fd04a16 100644
--- a/common/config
+++ b/common/config
@@ -346,6 +346,9 @@ _test_mount_opts()
 	glusterfs)
 		export TEST_FS_MOUNT_OPTS=$GLUSTERFS_MOUNT_OPTIONS
 		;;
+	overlay)
+		export TEST_FS_MOUNT_OPTS=$OVERLAY_MOUNT_OPTIONS
+		;;
 	ext2|ext3|ext4|ext4dev)
 		# acls & xattrs aren't turned on by default on older ext$FOO
 		export TEST_FS_MOUNT_OPTS="-o acl,user_xattr $EXT_MOUNT_OPTIONS"
@@ -546,16 +549,19 @@ _overlay_config_override()
 	# Store original base fs vars
 	export OVL_BASE_TEST_DEV="$TEST_DEV"
 	export OVL_BASE_TEST_DIR="$TEST_DIR"
-	# If config does not set MOUNT_OPTIONS, its value may be
-	# leftover from previous _overlay_config_override, so
+	# If config does not set MOUNT_OPTIONS and TEST_FS_MOUNT_OPTS, its
+	# value may be leftover from previous _overlay_config_override, so
 	# don't use that value for base fs mount
 	[ "$MOUNT_OPTIONS" != "$OVERLAY_MOUNT_OPTIONS" ] || unset MOUNT_OPTIONS
-	export OVL_BASE_MOUNT_OPTIONS="$MOUNT_OPTIONS"
+	export OVL_BASE_SCRATCH_MOUNT_OPTIONS="$MOUNT_OPTIONS"
+	[ "$TEST_FS_MOUNT_OPTS" != "$OVERLAY_MOUNT_OPTIONS" ] || unset TEST_FS_MOUNT_OPTS
+	export OVL_BASE_TEST_MOUNT_OPTIONS="$TEST_FS_MOUNT_OPTS"
 
 	# 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"
+	export TEST_FS_MOUNT_OPTS="$OVERLAY_MOUNT_OPTIONS"
 
 	[ -b "$SCRATCH_DEV" ] || return 0
 
@@ -580,7 +586,10 @@ _overlay_config_restore()
 	[ -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
-	[ -z "$OVL_BASE_MOUNT_OPTIONS" ] || export MOUNT_OPTIONS=$OVL_BASE_MOUNT_OPTIONS
+	[ -z "$OVL_BASE_SCRATCH_MOUNT_OPTIONS" ] || \
+		export MOUNT_OPTIONS=$OVL_BASE_SCRATCH_MOUNT_OPTIONS
+	[ -z "$OVL_BASE_TEST_MOUNT_OPTIONS" ] || \
+		export TEST_FS_MOUNT_OPTS=$OVL_BASE_TEST_MOUNT_OPTIONS
 }
 
 # Parse config section options. This function will parse all the configuration
diff --git a/common/overlay b/common/overlay
index 29f9bf8..058b025 100644
--- a/common/overlay
+++ b/common/overlay
@@ -20,7 +20,19 @@ _overlay_mount_dirs()
 	shift 3
 
 	$MOUNT_PROG -t overlay -o lowerdir=$lowerdir -o upperdir=$upperdir \
-		    -o workdir=$workdir `_common_dev_mount_options $*`
+		    -o workdir=$workdir $*
+}
+
+# Mount overlayfs with optional dirs and mount point
+_overlay_mount_optional_dirs()
+{
+	local lowerdir=$1
+	local upperdir=$2
+	local workdir=$3
+	shift 3
+
+	_overlay_mount_dirs $lowerdir $upperdir $workdir \
+			    `_common_dev_mount_options $*`
 }
 
 # Mount with same options/mnt/dev of scratch mount, but optionally
@@ -33,7 +45,8 @@ _overlay_scratch_mount_dirs()
 	shift 3
 
 	_overlay_mount_dirs $lowerdir $upperdir $workdir \
-				$* $OVL_BASE_SCRATCH_MNT $SCRATCH_MNT
+			    `_common_dev_mount_options $*` \
+			    $OVL_BASE_SCRATCH_MNT $SCRATCH_MNT
 }
 
 _overlay_mkdirs()
@@ -86,26 +99,28 @@ _overlay_base_test_mount()
 {
 	_overlay_base_mount OVL_BASE_TEST_DEV OVL_BASE_TEST_DIR \
 			"$OVL_BASE_TEST_DEV" "$OVL_BASE_TEST_DIR" \
-			$TEST_FS_MOUNT_OPTS $SELINUX_MOUNT_OPTIONS
+			$OVL_BASE_TEST_MOUNT_OPTIONS $SELINUX_MOUNT_OPTIONS
 }
 
 _overlay_test_mount()
 {
 	_overlay_base_test_mount && \
-		_overlay_mount $OVL_BASE_TEST_DIR $TEST_DIR $*
+		_overlay_mount $OVL_BASE_TEST_DIR $TEST_DIR \
+			       $TEST_FS_MOUNT_OPTS $SELINUX_MOUNT_OPTIONS $*
 }
 
 _overlay_base_scratch_mount()
 {
 	_overlay_base_mount OVL_BASE_SCRATCH_DEV OVL_BASE_SCRATCH_MNT \
 			"$OVL_BASE_SCRATCH_DEV" "$OVL_BASE_SCRATCH_MNT" \
-			$OVL_BASE_MOUNT_OPTIONS $SELINUX_MOUNT_OPTIONS
+			$OVL_BASE_SCRATCH_MOUNT_OPTIONS $SELINUX_MOUNT_OPTIONS
 }
 
 _overlay_scratch_mount()
 {
 	_overlay_base_scratch_mount && \
-		_overlay_mount $OVL_BASE_SCRATCH_MNT $SCRATCH_MNT $*
+		_overlay_mount $OVL_BASE_SCRATCH_MNT $SCRATCH_MNT \
+			       `_common_dev_mount_options $*`
 }
 
 _overlay_base_unmount()
@@ -305,7 +320,7 @@ _check_overlay_test_fs()
 	_overlay_check_fs "$TEST_DIR" \
 		OVL_BASE_TEST_DEV OVL_BASE_TEST_DIR \
 		"$OVL_BASE_TEST_DEV" "$OVL_BASE_TEST_DIR" \
-		$TEST_FS_MOUNT_OPTS $SELINUX_MOUNT_OPTIONS
+		$OVL_BASE_TEST_MOUNT_OPTIONS $SELINUX_MOUNT_OPTIONS
 }
 
 _check_overlay_scratch_fs()
@@ -313,5 +328,5 @@ _check_overlay_scratch_fs()
 	_overlay_check_fs "$SCRATCH_MNT" \
 		OVL_BASE_SCRATCH_DEV OVL_BASE_SCRATCH_MNT \
 		"$OVL_BASE_SCRATCH_DEV" "$OVL_BASE_SCRATCH_MNT" \
-		$OVL_BASE_MOUNT_OPTIONS $SELINUX_MOUNT_OPTIONS
+		$OVL_BASE_SCRATCH_MOUNT_OPTIONS $SELINUX_MOUNT_OPTIONS
 }
diff --git a/tests/overlay/022 b/tests/overlay/022
index ef4d73f..58683fb 100755
--- a/tests/overlay/022
+++ b/tests/overlay/022
@@ -68,7 +68,7 @@ _scratch_mount
 mkdir -p $tmp/{lower,mnt}
 # mount overlay using upper from another overlay upper
 # should fail
-_overlay_mount_dirs $tmp/lower $SCRATCH_MNT/upper \
+_overlay_mount_optional_dirs $tmp/lower $SCRATCH_MNT/upper \
   $SCRATCH_MNT/work overlay $tmp/mnt > /dev/null 2>&1
 if [ $? -ne 0 ] ; then
 	echo "Silence is golden"
diff --git a/tests/overlay/025 b/tests/overlay/025
index bdbb4e5..f82ff4c 100755
--- a/tests/overlay/025
+++ b/tests/overlay/025
@@ -71,7 +71,7 @@ mkdir -p -m 0 $tmpfsdir/upper/testd
 setfacl -m u:$qa_user:rx $tmpfsdir/upper/testd
 
 # mount overlay using dirs in tmpfs
-_overlay_mount_dirs $tmpfsdir/{lower,upper,work,overlay,mnt}
+_overlay_mount_optional_dirs $tmpfsdir/{lower,upper,work,overlay,mnt}
 
 # user accessing test dir, should be OKay
 _user_do "ls $tmpfsdir/mnt/testd"
diff --git a/tests/overlay/029 b/tests/overlay/029
index 2ac674f..ca04ea8 100755
--- a/tests/overlay/029
+++ b/tests/overlay/029
@@ -77,20 +77,20 @@ _scratch_mount
 
 mkdir -p $tmp/{upper,mnt,work}
 # mount overlay again using upper dir from SCRATCH_MNT dir
-_overlay_mount_dirs $SCRATCH_MNT/up $tmp/{upper,work} \
+_overlay_mount_optional_dirs $SCRATCH_MNT/up $tmp/{upper,work} \
   overlay $tmp/mnt
 # accessing file in the second mount
 cat $tmp/mnt/foo
 $UMOUNT_PROG $tmp/mnt
 
 # mount overlay again using lower dir from SCRATCH_MNT dir
-_overlay_mount_dirs $SCRATCH_MNT/low $tmp/{upper,work} \
+_overlay_mount_optional_dirs $SCRATCH_MNT/low $tmp/{upper,work} \
   overlay $tmp/mnt
 cat $tmp/mnt/bar
 $UMOUNT_PROG $tmp/mnt
 
 # mount overlay again using SCRATCH_MNT dir
-_overlay_mount_dirs $SCRATCH_MNT/ $tmp/{upper,work} \
+_overlay_mount_optional_dirs $SCRATCH_MNT/ $tmp/{upper,work} \
   overlay $tmp/mnt
 cat $tmp/mnt/up/foo
 cat $tmp/mnt/low/bar
diff --git a/tests/overlay/036 b/tests/overlay/036
index 8d3ccf4..2a79ef2 100755
--- a/tests/overlay/036
+++ b/tests/overlay/036
@@ -87,29 +87,29 @@ workdir2=$OVL_BASE_SCRATCH_MNT/workdir2
 mkdir -p $lowerdir $lowerdir2 $upperdir $upperdir2 $workdir $workdir2
 
 # Mount overlay with lowerdir, upperdir, workdir
-_overlay_mount_dirs $lowerdir $upperdir $workdir \
-		    overlay $SCRATCH_MNT
+_overlay_mount_optional_dirs $lowerdir $upperdir $workdir \
+			     overlay $SCRATCH_MNT
 
 # Try to mount another overlay with the same upperdir
 # with index=off - expect success
-_overlay_mount_dirs $lowerdir $upperdir $workdir2 \
-		    overlay0 $SCRATCH_MNT -oindex=off && \
-		    $UMOUNT_PROG $SCRATCH_MNT
+_overlay_mount_optional_dirs $lowerdir $upperdir $workdir2 \
+			     overlay0 $SCRATCH_MNT -oindex=off && \
+			     $UMOUNT_PROG $SCRATCH_MNT
 
 # Try to mount another overlay with the same workdir
 # with index=off - expect success
-_overlay_mount_dirs $lowerdir2 $upperdir2 $workdir \
-		    overlay1 $SCRATCH_MNT -oindex=off && \
-		    $UMOUNT_PROG $SCRATCH_MNT
+_overlay_mount_optional_dirs $lowerdir2 $upperdir2 $workdir \
+			     overlay1 $SCRATCH_MNT -oindex=off && \
+			     $UMOUNT_PROG $SCRATCH_MNT
 
 # Try to mount another overlay with the same upperdir
 # with index=on - expect EBUSY
-_overlay_mount_dirs $lowerdir $upperdir $workdir2 \
+_overlay_mount_optional_dirs $lowerdir $upperdir $workdir2 \
 	    overlay2 $SCRATCH_MNT -oindex=on 2>&1 | _filter_busy_mount
 
 # Try to mount another overlay with the same workdir
 # with index=on - expect EBUSY
-_overlay_mount_dirs $lowerdir2 $upperdir2 $workdir \
+_overlay_mount_optional_dirs $lowerdir2 $upperdir2 $workdir \
 	    overlay3 $SCRATCH_MNT -oindex=on 2>&1 | _filter_busy_mount
 
 # check overlayfs
-- 
2.5.0

Re: [xfstests PATCH v3 5/6] overlay: correct scratch dirs check

[Amir Goldstein]

On Tue, Feb 13, 2018 at 9:08 AM, zhangyi (F) <yi.zhang@huawei.com> wrote:
> Tests that use _overlay_scratch_mount_dirs instead of _scratch_mount
> should use _require_overlay_scratch_dirs instead of _require_scratch
> because these tests are either mounting with multiple lower dirs or
> mounting with non-default lower/upper/work dir, so
> _check_overlay_scratch_fs won't handle these cases correctly, we need
> to call _overlay_check_scratch_dirs with the correct arguments.
>
> This patch modify these tests to optionally call
> _overlay_check_scratch_dirs at the end of the test or before
> _scratch_umount to umount $SCRATCH_MNT and run the checker.
>
> Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
> Reviewed-by: Amir Goldstein <amir73il@gmail.com>

Usually, you should remove Reviewed-by when patch has changed considerably.
Anyway, you may restore Reviewed-by after addressing minor comments below.

[...]

> --- a/tests/overlay/038
> +++ b/tests/overlay/038
> @@ -46,7 +46,9 @@ _cleanup()
>  # real QA test starts here
>  _supported_fs overlay
>  _supported_os Linux
> -_require_scratch
> +# Use non-default scratch underlying overlay dirs, we need to check
> +# them explicity after test.
> +_require_scratch_nocheck
>  _require_attrs
>  _require_test_program "t_dir_type"
>
> @@ -161,6 +163,9 @@ subdir_d=$($here/src/t_dir_type $pure_lower_dir $pure_lower_subdir_st_ino)
>  [[ $subdir_d == "subdir d" ]] || \
>         echo "Merged dir: Invalid d_ino reported for subdir"
>
> +# check overlayfs
> +_check_scratch_fs
> +
>  _scratch_unmount
>

Every time this pattern repeats, it is better to _check after _unmount.
Otherwise _check would need to unmount and mount for no reason.

>  # Verify pure lower residing in dir which has another lower layer
> @@ -202,6 +207,9 @@ subdir_d=$($here/src/t_dir_type $pure_lower_dir $pure_lower_subdir_st_ino)
>  [[ $subdir_d == "subdir d" ]] || \
>         echo "Pure lower in dir which has another lower layer: Invalid d_ino reported for subdir"
>
> +# check overlayfs
> +_overlay_check_scratch_dirs "$middir:$lowerdir" $upperdir $workdir
> +
>  echo "Silence is golden"
>  status=0
>  exit
> diff --git a/tests/overlay/041 b/tests/overlay/041
> index 11efacb..da50e0b 100755
> --- a/tests/overlay/041
> +++ b/tests/overlay/041
> @@ -48,7 +48,9 @@ _cleanup()
>  # real QA test starts here
>  _supported_fs overlay
>  _supported_os Linux
> -_require_scratch
> +# Use non-default scratch underlying overlay dirs, we need to check
> +# them explicity after test.
> +_require_scratch_nocheck
>  _require_test
>  _require_attrs
>  _require_test_program "t_dir_type"
> @@ -166,6 +168,9 @@ subdir_d=$($here/src/t_dir_type $pure_lower_dir $pure_lower_subdir_st_ino)
>  [[ $subdir_d == "subdir d" ]] || \
>         echo "Merged dir: Invalid d_ino reported for subdir"
>
> +# check overlayfs
> +_overlay_check_scratch_dirs $lowerdir $upperdir $workdir
> +

Here as well.

>  _scratch_unmount
>
>  # Verify pure lower residing in dir which has another lower layer
> @@ -206,6 +211,9 @@ subdir_d=$($here/src/t_dir_type $pure_lower_dir $pure_lower_subdir_st_ino)
>  [[ $subdir_d == "subdir d" ]] || \
>         echo "Pure lower in dir which has another lower layer: Invalid d_ino reported for subdir"
>
> +# check overlayfs
> +_overlay_check_scratch_dirs "$middir:$lowerdir" $upperdir $workdir
> +
>  echo "Silence is golden"
>  status=0
>  exit
> diff --git a/tests/overlay/043 b/tests/overlay/043
> index 699c4e1..46df686 100755
> --- a/tests/overlay/043
> +++ b/tests/overlay/043
> @@ -56,7 +56,9 @@ _cleanup()
>  # real QA test starts here
>  _supported_fs overlay
>  _supported_os Linux
> -_require_scratch
> +# Use non-default scratch underlying overlay dirs, we need to check
> +# them explicity after test.
> +_require_scratch_nocheck
>  _require_test
>  _require_test_program "af_unix"
>  _require_test_program "t_dir_type"
> @@ -153,6 +155,9 @@ _overlay_scratch_mount_dirs $lowerdir $upperdir $workdir
>  # Compare inode numbers before/after mount cycle
>  check_inode_numbers $testdir $tmp.after_move $tmp.after_cycle
>
> +# check overlayfs
> +_overlay_check_scratch_dirs $lowerdir $upperdir $workdir
> +
>  echo "Silence is golden"
>  status=0
>  exit
> diff --git a/tests/overlay/044 b/tests/overlay/044
> index e57f6f7..2ab3035 100755
> --- a/tests/overlay/044
> +++ b/tests/overlay/044
> @@ -49,7 +49,9 @@ _cleanup()
>  # real QA test starts here
>  _supported_fs overlay
>  _supported_os Linux
> -_require_scratch
> +# Use non-default scratch underlying overlay dirs, we need to check
> +# them explicity after test.
> +_require_scratch_nocheck
>  _require_test
>  _require_scratch_feature index
>  _require_test_program "t_dir_type"
> @@ -122,6 +124,7 @@ check_ino_nlink $SCRATCH_MNT $tmp.before $tmp.after_one
>
>  # Verify that the hardlinks survive a mount cycle
>  $UMOUNT_PROG $SCRATCH_MNT
> +_overlay_check_scratch_dirs $lowerdir $upperdir $workdir -o index=on
>  _overlay_scratch_mount_dirs $lowerdir $upperdir $workdir -o index=on
>
>  echo "== After mount cycle =="
> @@ -138,5 +141,8 @@ echo "== After write two =="
>  cat $FILES
>  check_ino_nlink $SCRATCH_MNT $tmp.after_one $tmp.after_two
>
> +# check overlayfs
> +_overlay_check_scratch_dirs $lowerdir $upperdir $workdir -o index=on
> +
>  status=0
>  exit
> diff --git a/tests/overlay/051 b/tests/overlay/051
> index ae7844d..b888f1e 100755
> --- a/tests/overlay/051
> +++ b/tests/overlay/051
> @@ -61,8 +61,10 @@ _cleanup()
>  _supported_fs overlay
>  _supported_os Linux
>  _require_test
> -_require_scratch
>  _require_test_program "open_by_handle"
> +# Use non-default scratch underlying overlay dirs, we need to check
> +# them explicity after test.
> +_require_scratch_nocheck
>  # We need to require both features together, because nfs_export cannot
>  # be enabled when index is disabled
>  _require_scratch_overlay_features index nfs_export
> @@ -128,6 +130,13 @@ unmount_dirs()
>         $UMOUNT_PROG $SCRATCH_MNT
>  }
>
> +# Check underlying dirs of overlayfs
> +check_dirs()
> +{
> +       _overlay_check_scratch_dirs $middle:$lower $upper $work \
> +                               -o "index=on,nfs_export=on"

Please embed this check inside unmount_dirs() instead of adding
check_dirs calls - the reason - I tried to keep diff between
05${i} and 05${i+1} to the implementation of the helpers, so that if
one test is updated  it is easier to update its ^samefs flavor.

Thanks,
Amir.

Re: [xfstests PATCH v3 6/6] overlay: correct test mount options

[Amir Goldstein]

On Tue, Feb 13, 2018 at 9:08 AM, zhangyi (F) <yi.zhang@huawei.com> wrote:
> Current overlay's _test_mount functions mount base test filesystem
> with TEST_FS_MOUNT_OPTS and mount test overlayfs with MOUNT_OPTIONS
> instead of TEST_FS_MOUNT_OPTS. The TEST_FS_MOUNT_OPTS variable
> should be used for test overlayfs like MOUNT_OPTIONS use for scratch
> overlayfs.
>
> This patch rename OVL_BASE_MOUNT_OPTIONS to
> OVL_BASE_SCRATCH_MOUNT_OPTIONS and introduce an counterpart variable
> OVL_BASE_TEST_MOUNT_OPTIONS for test base filesystem, and then use
> TEST_FS_MOUNT_OPTS for test overlayfs. We also modify overlay mount
> helpers to adapt mount options.
>
> Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
> ---
>  common/config     | 17 +++++++++++++----
>  common/overlay    | 31 +++++++++++++++++++++++--------
>  tests/overlay/022 |  2 +-
>  tests/overlay/025 |  2 +-
>  tests/overlay/029 |  6 +++---
>  tests/overlay/036 | 20 ++++++++++----------
>  6 files changed, 51 insertions(+), 27 deletions(-)
>
> diff --git a/common/config b/common/config
> index 20f0e5f..fd04a16 100644
> --- a/common/config
> +++ b/common/config
> @@ -346,6 +346,9 @@ _test_mount_opts()
>         glusterfs)
>                 export TEST_FS_MOUNT_OPTS=$GLUSTERFS_MOUNT_OPTIONS
>                 ;;
> +       overlay)
> +               export TEST_FS_MOUNT_OPTS=$OVERLAY_MOUNT_OPTIONS
> +               ;;
>         ext2|ext3|ext4|ext4dev)
>                 # acls & xattrs aren't turned on by default on older ext$FOO
>                 export TEST_FS_MOUNT_OPTS="-o acl,user_xattr $EXT_MOUNT_OPTIONS"
> @@ -546,16 +549,19 @@ _overlay_config_override()
>         # Store original base fs vars
>         export OVL_BASE_TEST_DEV="$TEST_DEV"
>         export OVL_BASE_TEST_DIR="$TEST_DIR"
> -       # If config does not set MOUNT_OPTIONS, its value may be
> -       # leftover from previous _overlay_config_override, so
> +       # If config does not set MOUNT_OPTIONS and TEST_FS_MOUNT_OPTS, its
> +       # value may be leftover from previous _overlay_config_override, so
>         # don't use that value for base fs mount
>         [ "$MOUNT_OPTIONS" != "$OVERLAY_MOUNT_OPTIONS" ] || unset MOUNT_OPTIONS
> -       export OVL_BASE_MOUNT_OPTIONS="$MOUNT_OPTIONS"
> +       export OVL_BASE_SCRATCH_MOUNT_OPTIONS="$MOUNT_OPTIONS"
> +       [ "$TEST_FS_MOUNT_OPTS" != "$OVERLAY_MOUNT_OPTIONS" ] || unset TEST_FS_MOUNT_OPTS
> +       export OVL_BASE_TEST_MOUNT_OPTIONS="$TEST_FS_MOUNT_OPTS"
>
>         # 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"
> +       export TEST_FS_MOUNT_OPTS="$OVERLAY_MOUNT_OPTIONS"
>
>         [ -b "$SCRATCH_DEV" ] || return 0
>
> @@ -580,7 +586,10 @@ _overlay_config_restore()
>         [ -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
> -       [ -z "$OVL_BASE_MOUNT_OPTIONS" ] || export MOUNT_OPTIONS=$OVL_BASE_MOUNT_OPTIONS
> +       [ -z "$OVL_BASE_SCRATCH_MOUNT_OPTIONS" ] || \
> +               export MOUNT_OPTIONS=$OVL_BASE_SCRATCH_MOUNT_OPTIONS
> +       [ -z "$OVL_BASE_TEST_MOUNT_OPTIONS" ] || \
> +               export TEST_FS_MOUNT_OPTS=$OVL_BASE_TEST_MOUNT_OPTIONS
>  }
>
>  # Parse config section options. This function will parse all the configuration
> diff --git a/common/overlay b/common/overlay
> index 29f9bf8..058b025 100644
> --- a/common/overlay
> +++ b/common/overlay
> @@ -20,7 +20,19 @@ _overlay_mount_dirs()
>         shift 3
>
>         $MOUNT_PROG -t overlay -o lowerdir=$lowerdir -o upperdir=$upperdir \
> -                   -o workdir=$workdir `_common_dev_mount_options $*`
> +                   -o workdir=$workdir $*
> +}
> +
> +# Mount overlayfs with optional dirs and mount point
> +_overlay_mount_optional_dirs()
> +{
> +       local lowerdir=$1
> +       local upperdir=$2
> +       local workdir=$3
> +       shift 3
> +
> +       _overlay_mount_dirs $lowerdir $upperdir $workdir \
> +                           `_common_dev_mount_options $*`
>  }
>

Instead of changing the name of the helper and change all the tests
that call it,
factor out a "local" helper __overlay_mount_dirs_o() that takes all options
from argument and keep the helper name _overlay_mount_dirs(), which the
tests use, unchanged.

Then, the only user of the "local" helper __overlay_mount_dirs_o() will be
the "local" helper _overlay_mount(), which you should also rename to
__overlay_mount_o(), as it should not be used directly by tests.

> # Mount with same options/mnt/dev of scratch mount, but optionally
> @@ -33,7 +45,8 @@ _overlay_scratch_mount_dirs()
>        shift 3
>
>         _overlay_mount_dirs $lowerdir $upperdir $workdir \
> -                               $* $OVL_BASE_SCRATCH_MNT $SCRATCH_MNT
> +                           `_common_dev_mount_options $*` \
> +                           $OVL_BASE_SCRATCH_MNT $SCRATCH_MNT
> }

This doesn't need to change either.

Thanks,
Amir.

Re: [xfstests PATCH v3 6/6] overlay: correct test mount options

[Eryu Guan]

On Tue, Feb 13, 2018 at 11:26:30AM +0200, Amir Goldstein wrote:
> On Tue, Feb 13, 2018 at 9:08 AM, zhangyi (F) <yi.zhang@huawei.com> wrote:
> > Current overlay's _test_mount functions mount base test filesystem
> > with TEST_FS_MOUNT_OPTS and mount test overlayfs with MOUNT_OPTIONS
> > instead of TEST_FS_MOUNT_OPTS. The TEST_FS_MOUNT_OPTS variable
> > should be used for test overlayfs like MOUNT_OPTIONS use for scratch
> > overlayfs.
> >
> > This patch rename OVL_BASE_MOUNT_OPTIONS to
> > OVL_BASE_SCRATCH_MOUNT_OPTIONS and introduce an counterpart variable
> > OVL_BASE_TEST_MOUNT_OPTIONS for test base filesystem, and then use
> > TEST_FS_MOUNT_OPTS for test overlayfs. We also modify overlay mount
> > helpers to adapt mount options.
> >
> > Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
> > ---
> >  common/config     | 17 +++++++++++++----
> >  common/overlay    | 31 +++++++++++++++++++++++--------
> >  tests/overlay/022 |  2 +-
> >  tests/overlay/025 |  2 +-
> >  tests/overlay/029 |  6 +++---
> >  tests/overlay/036 | 20 ++++++++++----------
> >  6 files changed, 51 insertions(+), 27 deletions(-)
> >
> > diff --git a/common/config b/common/config
> > index 20f0e5f..fd04a16 100644
> > --- a/common/config
> > +++ b/common/config
> > @@ -346,6 +346,9 @@ _test_mount_opts()
> >         glusterfs)
> >                 export TEST_FS_MOUNT_OPTS=$GLUSTERFS_MOUNT_OPTIONS
> >                 ;;
> > +       overlay)
> > +               export TEST_FS_MOUNT_OPTS=$OVERLAY_MOUNT_OPTIONS
> > +               ;;
> >         ext2|ext3|ext4|ext4dev)
> >                 # acls & xattrs aren't turned on by default on older ext$FOO
> >                 export TEST_FS_MOUNT_OPTS="-o acl,user_xattr $EXT_MOUNT_OPTIONS"
> > @@ -546,16 +549,19 @@ _overlay_config_override()
> >         # Store original base fs vars
> >         export OVL_BASE_TEST_DEV="$TEST_DEV"
> >         export OVL_BASE_TEST_DIR="$TEST_DIR"
> > -       # If config does not set MOUNT_OPTIONS, its value may be
> > -       # leftover from previous _overlay_config_override, so
> > +       # If config does not set MOUNT_OPTIONS and TEST_FS_MOUNT_OPTS, its
> > +       # value may be leftover from previous _overlay_config_override, so
> >         # don't use that value for base fs mount
> >         [ "$MOUNT_OPTIONS" != "$OVERLAY_MOUNT_OPTIONS" ] || unset MOUNT_OPTIONS
> > -       export OVL_BASE_MOUNT_OPTIONS="$MOUNT_OPTIONS"
> > +       export OVL_BASE_SCRATCH_MOUNT_OPTIONS="$MOUNT_OPTIONS"
> > +       [ "$TEST_FS_MOUNT_OPTS" != "$OVERLAY_MOUNT_OPTIONS" ] || unset TEST_FS_MOUNT_OPTS
> > +       export OVL_BASE_TEST_MOUNT_OPTIONS="$TEST_FS_MOUNT_OPTS"
> >
> >         # 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"
> > +       export TEST_FS_MOUNT_OPTS="$OVERLAY_MOUNT_OPTIONS"
> >
> >         [ -b "$SCRATCH_DEV" ] || return 0
> >
> > @@ -580,7 +586,10 @@ _overlay_config_restore()
> >         [ -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
> > -       [ -z "$OVL_BASE_MOUNT_OPTIONS" ] || export MOUNT_OPTIONS=$OVL_BASE_MOUNT_OPTIONS
> > +       [ -z "$OVL_BASE_SCRATCH_MOUNT_OPTIONS" ] || \
> > +               export MOUNT_OPTIONS=$OVL_BASE_SCRATCH_MOUNT_OPTIONS
> > +       [ -z "$OVL_BASE_TEST_MOUNT_OPTIONS" ] || \
> > +               export TEST_FS_MOUNT_OPTS=$OVL_BASE_TEST_MOUNT_OPTIONS
> >  }
> >
> >  # Parse config section options. This function will parse all the configuration
> > diff --git a/common/overlay b/common/overlay
> > index 29f9bf8..058b025 100644
> > --- a/common/overlay
> > +++ b/common/overlay
> > @@ -20,7 +20,19 @@ _overlay_mount_dirs()
> >         shift 3
> >
> >         $MOUNT_PROG -t overlay -o lowerdir=$lowerdir -o upperdir=$upperdir \
> > -                   -o workdir=$workdir `_common_dev_mount_options $*`
> > +                   -o workdir=$workdir $*
> > +}
> > +
> > +# Mount overlayfs with optional dirs and mount point
> > +_overlay_mount_optional_dirs()
> > +{
> > +       local lowerdir=$1
> > +       local upperdir=$2
> > +       local workdir=$3
> > +       shift 3
> > +
> > +       _overlay_mount_dirs $lowerdir $upperdir $workdir \
> > +                           `_common_dev_mount_options $*`
> >  }
> >
> 
> Instead of changing the name of the helper and change all the tests
> that call it,
> factor out a "local" helper __overlay_mount_dirs_o() that takes all options
> from argument and keep the helper name _overlay_mount_dirs(), which the
> tests use, unchanged.

Agreed, I just don't like the name __overlay_mount_dirs_o that much :)
How about something like _do_overlay_mount_dirs?

Thanks,
Eryu

> 
> Then, the only user of the "local" helper __overlay_mount_dirs_o() will be
> the "local" helper _overlay_mount(), which you should also rename to
> __overlay_mount_o(), as it should not be used directly by tests.
> 
> > # Mount with same options/mnt/dev of scratch mount, but optionally
> > @@ -33,7 +45,8 @@ _overlay_scratch_mount_dirs()
> >        shift 3
> >
> >         _overlay_mount_dirs $lowerdir $upperdir $workdir \
> > -                               $* $OVL_BASE_SCRATCH_MNT $SCRATCH_MNT
> > +                           `_common_dev_mount_options $*` \
> > +                           $OVL_BASE_SCRATCH_MNT $SCRATCH_MNT
> > }
> 
> This doesn't need to change either.
> 
> Thanks,
> Amir.

Re: [xfstests PATCH v3 6/6] overlay: correct test mount options

[Amir Goldstein]

On Tue, Feb 13, 2018 at 11:48 AM, Eryu Guan <eguan@redhat.com> wrote:
> On Tue, Feb 13, 2018 at 11:26:30AM +0200, Amir Goldstein wrote:
>> On Tue, Feb 13, 2018 at 9:08 AM, zhangyi (F) <yi.zhang@huawei.com> wrote:
>> > Current overlay's _test_mount functions mount base test filesystem
>> > with TEST_FS_MOUNT_OPTS and mount test overlayfs with MOUNT_OPTIONS
>> > instead of TEST_FS_MOUNT_OPTS. The TEST_FS_MOUNT_OPTS variable
>> > should be used for test overlayfs like MOUNT_OPTIONS use for scratch
>> > overlayfs.
>> >
>> > This patch rename OVL_BASE_MOUNT_OPTIONS to
>> > OVL_BASE_SCRATCH_MOUNT_OPTIONS and introduce an counterpart variable
>> > OVL_BASE_TEST_MOUNT_OPTIONS for test base filesystem, and then use
>> > TEST_FS_MOUNT_OPTS for test overlayfs. We also modify overlay mount
>> > helpers to adapt mount options.
>> >
>> > Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
>> > ---
>> >  common/config     | 17 +++++++++++++----
>> >  common/overlay    | 31 +++++++++++++++++++++++--------
>> >  tests/overlay/022 |  2 +-
>> >  tests/overlay/025 |  2 +-
>> >  tests/overlay/029 |  6 +++---
>> >  tests/overlay/036 | 20 ++++++++++----------
>> >  6 files changed, 51 insertions(+), 27 deletions(-)
>> >
>> > diff --git a/common/config b/common/config
>> > index 20f0e5f..fd04a16 100644
>> > --- a/common/config
>> > +++ b/common/config
>> > @@ -346,6 +346,9 @@ _test_mount_opts()
>> >         glusterfs)
>> >                 export TEST_FS_MOUNT_OPTS=$GLUSTERFS_MOUNT_OPTIONS
>> >                 ;;
>> > +       overlay)
>> > +               export TEST_FS_MOUNT_OPTS=$OVERLAY_MOUNT_OPTIONS
>> > +               ;;
>> >         ext2|ext3|ext4|ext4dev)
>> >                 # acls & xattrs aren't turned on by default on older ext$FOO
>> >                 export TEST_FS_MOUNT_OPTS="-o acl,user_xattr $EXT_MOUNT_OPTIONS"
>> > @@ -546,16 +549,19 @@ _overlay_config_override()
>> >         # Store original base fs vars
>> >         export OVL_BASE_TEST_DEV="$TEST_DEV"
>> >         export OVL_BASE_TEST_DIR="$TEST_DIR"
>> > -       # If config does not set MOUNT_OPTIONS, its value may be
>> > -       # leftover from previous _overlay_config_override, so
>> > +       # If config does not set MOUNT_OPTIONS and TEST_FS_MOUNT_OPTS, its
>> > +       # value may be leftover from previous _overlay_config_override, so
>> >         # don't use that value for base fs mount
>> >         [ "$MOUNT_OPTIONS" != "$OVERLAY_MOUNT_OPTIONS" ] || unset MOUNT_OPTIONS
>> > -       export OVL_BASE_MOUNT_OPTIONS="$MOUNT_OPTIONS"
>> > +       export OVL_BASE_SCRATCH_MOUNT_OPTIONS="$MOUNT_OPTIONS"
>> > +       [ "$TEST_FS_MOUNT_OPTS" != "$OVERLAY_MOUNT_OPTIONS" ] || unset TEST_FS_MOUNT_OPTS
>> > +       export OVL_BASE_TEST_MOUNT_OPTIONS="$TEST_FS_MOUNT_OPTS"
>> >
>> >         # 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"
>> > +       export TEST_FS_MOUNT_OPTS="$OVERLAY_MOUNT_OPTIONS"
>> >
>> >         [ -b "$SCRATCH_DEV" ] || return 0
>> >
>> > @@ -580,7 +586,10 @@ _overlay_config_restore()
>> >         [ -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
>> > -       [ -z "$OVL_BASE_MOUNT_OPTIONS" ] || export MOUNT_OPTIONS=$OVL_BASE_MOUNT_OPTIONS
>> > +       [ -z "$OVL_BASE_SCRATCH_MOUNT_OPTIONS" ] || \
>> > +               export MOUNT_OPTIONS=$OVL_BASE_SCRATCH_MOUNT_OPTIONS
>> > +       [ -z "$OVL_BASE_TEST_MOUNT_OPTIONS" ] || \
>> > +               export TEST_FS_MOUNT_OPTS=$OVL_BASE_TEST_MOUNT_OPTIONS
>> >  }
>> >
>> >  # Parse config section options. This function will parse all the configuration
>> > diff --git a/common/overlay b/common/overlay
>> > index 29f9bf8..058b025 100644
>> > --- a/common/overlay
>> > +++ b/common/overlay
>> > @@ -20,7 +20,19 @@ _overlay_mount_dirs()
>> >         shift 3
>> >
>> >         $MOUNT_PROG -t overlay -o lowerdir=$lowerdir -o upperdir=$upperdir \
>> > -                   -o workdir=$workdir `_common_dev_mount_options $*`
>> > +                   -o workdir=$workdir $*
>> > +}
>> > +
>> > +# Mount overlayfs with optional dirs and mount point
>> > +_overlay_mount_optional_dirs()
>> > +{
>> > +       local lowerdir=$1
>> > +       local upperdir=$2
>> > +       local workdir=$3
>> > +       shift 3
>> > +
>> > +       _overlay_mount_dirs $lowerdir $upperdir $workdir \
>> > +                           `_common_dev_mount_options $*`
>> >  }
>> >
>>
>> Instead of changing the name of the helper and change all the tests
>> that call it,
>> factor out a "local" helper __overlay_mount_dirs_o() that takes all options
>> from argument and keep the helper name _overlay_mount_dirs(), which the
>> tests use, unchanged.
>
> Agreed, I just don't like the name __overlay_mount_dirs_o that much :)
> How about something like _do_overlay_mount_dirs?
>

Yeh, didn't like it either. __do is fine by me

Amir.

Re: [xfstests PATCH v3 0/6] overlay: add overlay filesystem dirs check

[Eryu Guan]

On Tue, Feb 13, 2018 at 03:08:26PM +0800, zhangyi (F) wrote:
> Hi, Eryu:
> 
> I have handle all comments from last iterations and fix some mistakes, please
> review.
> 
> Patch 1-5: hook overlay filesystem check and correct current test cases.
> Patch 6: handle the test mount options issue, it add the counterpart of the
>          scratch mount options.

Thanks for the new revision! It looks good overall, I tested with both
legacy and new overlay configurations and both results look fine.

We only need a new version of patch 5 to address Amir's comments. Patch
6 needs some work too, but I think that can be done separately if
necessary.

Thanks,
Eryu

> 
> Thanks,
> Yi.
> 
> Changes since v2:
> - Rename _is_mounted to _is_dev_mounted and use findmnt instead of mount.
> - Fix mistakes in patch 2/6 and introduce _is_dir_mountpoint helper, also
>   introduce fsck options use for overlayfs.
> - Add overlay/049 into patch 4/6 to skip fs check.
> - Add nfs export cases into patch 5/6 to do correct dirs check.
> - Add the "missing feature" of test mount options.
> 
> Changes since v1:
> - Details comments in patch 1/5.
> - Improve _overlay_check_scratch_dirs to accept extra options.
> - Fix basic filesystem mounted check in _overlay_check_fs.
> - Improve overlay/044 in patch 5/5 to add index=on option.
> 
> ----------
> 
> Background:
> 
> This patchset implement filesystem check for overlay filesystem, base on
> my "overlay: add fsck.overlay basic tests" patchset and works only if
> fsck.overlay[1] exists (otherwise not run).
> 
> [1] https://github.com/hisilicon/overlayfs-progs
> 
> zhangyi (F) (6):
>   common/rc: improve dev mounted check helper
>   overlay: hook filesystem check helper
>   overlay/003: fix fs check failure
>   overlay: skip check for tests finished with corrupt filesystem
>   overlay: correct scratch dirs check
>   overlay: correct test mount options
> 
>  README.overlay    |  10 +++-
>  common/config     |  21 ++++++--
>  common/overlay    | 152 +++++++++++++++++++++++++++++++++++++++++++++++++++---
>  common/rc         |  47 +++++++++--------
>  common/xfs        |   2 +-
>  tests/overlay/003 |   1 -
>  tests/overlay/005 |   7 ++-
>  tests/overlay/010 |   7 ++-
>  tests/overlay/014 |  10 +++-
>  tests/overlay/019 |   2 +-
>  tests/overlay/022 |   2 +-
>  tests/overlay/025 |   2 +-
>  tests/overlay/029 |   6 +--
>  tests/overlay/031 |   2 +-
>  tests/overlay/035 |   7 ++-
>  tests/overlay/036 |  26 ++++++----
>  tests/overlay/037 |   7 ++-
>  tests/overlay/038 |  10 +++-
>  tests/overlay/041 |  10 +++-
>  tests/overlay/043 |   7 ++-
>  tests/overlay/044 |   8 ++-
>  tests/overlay/049 |   2 +-
>  tests/overlay/051 |  19 ++++++-
>  tests/overlay/053 |  15 +++++-
>  tests/overlay/055 |  12 ++++-
>  25 files changed, 328 insertions(+), 66 deletions(-)
> 
> -- 
> 2.5.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html