[xfstests PATCH v4 0/5] overlay: add overlay filesystem dirs check

[xfstests PATCH v4 0/5] overlay: add overlay filesystem dirs check

[zhangyi (F)]

Hi Eryu and Amir:

Sorry for the late, I have handle comments from last iteration in patch
{5,6}/6, patch 6/6 will post separately later, please review.

Thanks,
Yi.

Changes since v3:
- Move overlay checker after _unmount in overlay/{038,041}, and embed
  checker into unmount_dirs() in overlay/{051,053,055}.
- split out patch "overlay: correct test mount options"

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

 README.overlay    |  10 ++++-
 common/config     |   4 ++
 common/overlay    | 125 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 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/031 |   2 +-
 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/049 |   2 +-
 tests/overlay/051 |  10 ++++-
 tests/overlay/053 |  10 ++++-
 tests/overlay/055 |  10 ++++-
 22 files changed, 260 insertions(+), 44 deletions(-)

-- 
2.5.0

[xfstests PATCH v4 1/5] 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 a6caca9..17ff7b2 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 v4 2/5] 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 17ff7b2..cadbdf9 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 v4 3/5] 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 v4 4/5] 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 v4 5/5] 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 after
_scratch_umount to mount base filesystem only 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 | 10 ++++++++--
 tests/overlay/053 | 10 ++++++++--
 tests/overlay/055 | 10 ++++++++--
 13 files changed, 93 insertions(+), 16 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..972a16e 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"
 
@@ -163,6 +165,9 @@ subdir_d=$($here/src/t_dir_type $pure_lower_dir $pure_lower_subdir_st_ino)
 
 _scratch_unmount
 
+# check overlayfs
+_check_scratch_fs
+
 # Verify pure lower residing in dir which has another lower layer
 _scratch_mkfs
 
@@ -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..4e72348 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"
@@ -168,6 +170,9 @@ subdir_d=$($here/src/t_dir_type $pure_lower_dir $pure_lower_subdir_st_ino)
 
 _scratch_unmount
 
+# check overlayfs
+_overlay_check_scratch_dirs $lowerdir $upperdir $workdir
+
 # Verify pure lower residing in dir which has another lower layer
 middir=$OVL_BASE_TEST_DIR/$seq-ovl-mid
 lowerdir=$OVL_BASE_TEST_DIR/$seq-ovl-lower
@@ -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..04bd133 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
@@ -122,10 +124,14 @@ mount_dirs()
 				-o "index=on,nfs_export=on"
 }
 
-# Unmount the overlay without unmounting base fs
+# Unmount the overlay without unmounting base fs and check the
+# underlying dirs
 unmount_dirs()
 {
 	$UMOUNT_PROG $SCRATCH_MNT
+
+	_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
diff --git a/tests/overlay/053 b/tests/overlay/053
index d4fdcde..d766cde 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
@@ -118,10 +120,14 @@ mount_dirs()
 				-o "index=on,nfs_export=on,redirect_dir=on"
 }
 
-# Unmount the overlay without unmounting base fs
+# Unmount the overlay without unmounting base fs and check the
+# underlying dirs
 unmount_dirs()
 {
 	$UMOUNT_PROG $SCRATCH_MNT
+
+	_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
diff --git a/tests/overlay/055 b/tests/overlay/055
index 70fe6ac..02da6ef 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
@@ -122,10 +124,14 @@ mount_dirs()
 				-o "index=on,nfs_export=on,redirect_dir=on"
 }
 
-# Unmount the overlay without unmounting base fs
+# Unmount the overlay without unmounting base fs and check the
+# underlying dirs
 unmount_dirs()
 {
 	$UMOUNT_PROG $SCRATCH_MNT
+
+	_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
-- 
2.5.0

Re: [xfstests PATCH v4 2/5] overlay: hook filesystem check helper

[zhangyi (F)]

On 2018/2/22 11:26, Amir Goldstein Wrote:
> Zhangyi,
> 
> Sorry for dropping lists because replying from mobile. Feel free to add back.
> 
> 
> 
> On Feb 22, 2018 3:09 AM, "zhangyi (F)" <yi.zhang@huawei.com <mailto:yi.zhang@huawei.com>> wrote:
> 
>     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 <mailto: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"
> 
> 
> I may have missed this before but was under the impression that -n was the fsck option by default for post test fs check.
> 
> I believe that's how it usually is with fstests.
>
Yes, you are right, -n is the default option. Current OVERLAY_FSCK_OPTIONS is
used to avoid getting the base filesystem's FSCK_OPTIONS in multi sections config
case, becasue we cannot distinguish between leftover FSCK_OPTIONS and user specified
FSCK_OPTIONS, so introduce OVERLAY_FSCK_OPTIONS and set -n to default as example.

> In this example you can set fsck options same as mount options to indicate that for tests that use scratch_mount and test_mount with no extra arguments, fsck should verify there are no redirects in overlay.
> 
> Of course that would mean that you will have to mark more tests with require_scratch_nocheck, but you can do that later when fsck.overlay actually supports the -o arguments.
> 
Agree, we can do it after fsck.overlay can handle extra -o options.

Thanks,
Yi.

Re: [xfstests PATCH v4 2/5] overlay: hook filesystem check helper

[Amir Goldstein]

On Thu, Feb 22, 2018 at 8:27 AM, zhangyi (F) <yi.zhang@huawei.com> wrote:
> On 2018/2/22 11:26, Amir Goldstein Wrote:
>> Zhangyi,
>>
>> Sorry for dropping lists because replying from mobile. Feel free to add back.
>>
>>
>>
>> On Feb 22, 2018 3:09 AM, "zhangyi (F)" <yi.zhang@huawei.com <mailto:yi.zhang@huawei.com>> wrote:
>>
>>     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 <mailto: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"
>>
>>
>> I may have missed this before but was under the impression that -n was the fsck option by default for post test fs check.
>>
>> I believe that's how it usually is with fstests.
>>
> Yes, you are right, -n is the default option. Current OVERLAY_FSCK_OPTIONS is
> used to avoid getting the base filesystem's FSCK_OPTIONS in multi sections config
> case, becasue we cannot distinguish between leftover FSCK_OPTIONS and user specified
> FSCK_OPTIONS, so introduce OVERLAY_FSCK_OPTIONS and set -n to default as example.
>

I don't follow. How are FSCK_OPTIONS differ from MOUNT_OPTIONS in that sense?
When restoring base fs options, we check if MOUNT_OPTIONS differ from
OVERLAY_MOUNT_OPTIONS to distinguish between leftover MOUNT_OPTIONS.
My point is that specifying OVERLAY_FSCK_OPTIONS="-n" is the example is quite
confusing, because it suggests that use needs to set OVERLAY_FSCK_OPTIONS="-n"
in order to get the expected default behavior.
If that is indeed the case then it should be fixed.
And the example should be changed IMO to something like this, to be more
relevant to current code base and near (?) future fsck options:

OVERLAY_MOUNT_OPTIONS="-o index=on"
OVERLAY_FSCK_OPTIONS="-o index=on"

Thanks,
Amir.

Re: [xfstests PATCH v4 2/5] overlay: hook filesystem check helper

[zhangyi (F)]

On 2018/2/22 15:06, Amir Goldstein Wrote:
> On Thu, Feb 22, 2018 at 8:27 AM, zhangyi (F) <yi.zhang@huawei.com> wrote:
>> On 2018/2/22 11:26, Amir Goldstein Wrote:
>>> Zhangyi,
>>>
>>> Sorry for dropping lists because replying from mobile. Feel free to add back.
>>>
>>>
>>>
>>> On Feb 22, 2018 3:09 AM, "zhangyi (F)" <yi.zhang@huawei.com <mailto:yi.zhang@huawei.com>> wrote:
>>>
>>>     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 <mailto: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"
>>>
>>>
>>> I may have missed this before but was under the impression that -n was the fsck option by default for post test fs check.
>>>
>>> I believe that's how it usually is with fstests.
>>>
>> Yes, you are right, -n is the default option. Current OVERLAY_FSCK_OPTIONS is
>> used to avoid getting the base filesystem's FSCK_OPTIONS in multi sections config
>> case, becasue we cannot distinguish between leftover FSCK_OPTIONS and user specified
>> FSCK_OPTIONS, so introduce OVERLAY_FSCK_OPTIONS and set -n to default as example.
>>
> 
> I don't follow. How are FSCK_OPTIONS differ from MOUNT_OPTIONS in that sense?
> When restoring base fs options, we check if MOUNT_OPTIONS differ from
> OVERLAY_MOUNT_OPTIONS to distinguish between leftover MOUNT_OPTIONS.
> My point is that specifying OVERLAY_FSCK_OPTIONS="-n" is the example is quite
> confusing, because it suggests that use needs to set OVERLAY_FSCK_OPTIONS="-n"
> in order to get the expected default behavior.
> If that is indeed the case then it should be fixed.
> And the example should be changed IMO to something like this, to be more
> relevant to current code base and near (?) future fsck options:
> 
> OVERLAY_MOUNT_OPTIONS="-o index=on"
> OVERLAY_FSCK_OPTIONS="-o index=on"
> 

I understand what you mean, but the option arguments we pass to mount and
fsck.overlay were not quite the same, we only reuse overlay's special part
of MOUNT_OPTIONS(e.g. index=xx,redirect_dir=xx...), and we still need to pass
the fsck.overlay's own options(e.g. -n, maybe -f in future) through
OVERLAY_FSCK_OPTIONS (and I think there is no reason to hide this if user
want to given options directly). Like this:

OVERLAY_MOUNT_OPTIONS="-o index=on"
OVERLAY_FSCK_OPTIONS="-n -o index=on"

Now specifying OVERLAY_FSCK_OPTIONS="-n" is not necessary, I can modify to
OVERLAY_MOUNT_OPTIONS="-n, -o redirect_dir=off" after fsck.overlay can actually
handle this. If you think it's confusing, modify it now is also not harmful.

Thanks.
Yi.

Re: [xfstests PATCH v4 2/5] overlay: hook filesystem check helper

[zhangyi (F)]

On 2018/2/22 15:51, zhangyi (F) Wrote:
> On 2018/2/22 15:06, Amir Goldstein Wrote:
>> On Thu, Feb 22, 2018 at 8:27 AM, zhangyi (F) <yi.zhang@huawei.com> wrote:
>>> On 2018/2/22 11:26, Amir Goldstein Wrote:
>>>> Zhangyi,
>>>>
>>>> Sorry for dropping lists because replying from mobile. Feel free to add back.
>>>>
>>>>
>>>>
>>>> On Feb 22, 2018 3:09 AM, "zhangyi (F)" <yi.zhang@huawei.com <mailto:yi.zhang@huawei.com>> wrote:
>>>>
>>>>     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 <mailto: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"
>>>>
>>>>
>>>> I may have missed this before but was under the impression that -n was the fsck option by default for post test fs check.
>>>>
>>>> I believe that's how it usually is with fstests.
>>>>
>>> Yes, you are right, -n is the default option. Current OVERLAY_FSCK_OPTIONS is
>>> used to avoid getting the base filesystem's FSCK_OPTIONS in multi sections config
>>> case, becasue we cannot distinguish between leftover FSCK_OPTIONS and user specified
>>> FSCK_OPTIONS, so introduce OVERLAY_FSCK_OPTIONS and set -n to default as example.
>>>
>>
>> I don't follow. How are FSCK_OPTIONS differ from MOUNT_OPTIONS in that sense?
>> When restoring base fs options, we check if MOUNT_OPTIONS differ from
>> OVERLAY_MOUNT_OPTIONS to distinguish between leftover MOUNT_OPTIONS.
>> My point is that specifying OVERLAY_FSCK_OPTIONS="-n" is the example is quite
>> confusing, because it suggests that use needs to set OVERLAY_FSCK_OPTIONS="-n"
>> in order to get the expected default behavior.
>> If that is indeed the case then it should be fixed.
>> And the example should be changed IMO to something like this, to be more
>> relevant to current code base and near (?) future fsck options:
>>
>> OVERLAY_MOUNT_OPTIONS="-o index=on"
>> OVERLAY_FSCK_OPTIONS="-o index=on"
>>
> 
> I understand what you mean, but the option arguments we pass to mount and
> fsck.overlay were not quite the same, we only reuse overlay's special part
> of MOUNT_OPTIONS(e.g. index=xx,redirect_dir=xx...), and we still need to pass
> the fsck.overlay's own options(e.g. -n, maybe -f in future) through
> OVERLAY_FSCK_OPTIONS (and I think there is no reason to hide this if user
> want to given options directly). Like this:
> 
> OVERLAY_MOUNT_OPTIONS="-o index=on"
> OVERLAY_FSCK_OPTIONS="-n -o index=on"
> 
> Now specifying OVERLAY_FSCK_OPTIONS="-n" is not necessary, I can modify to
> OVERLAY_MOUNT_OPTIONS="-n, -o redirect_dir=off" after fsck.overlay can actually
  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  OVERLAY_FSCK_OPTIONS="-n -o redirect_dir=off", sorry
> handle this. If you think it's confusing, modify it now is also not harmful.
> 
> Thanks.
> Yi.
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-unionfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> .
> 

Re: [xfstests PATCH v4 2/5] overlay: hook filesystem check helper

[Eryu Guan]

Hi zhangyi,

I was testing v4 patchset with various overlay configs and results all
look good, thanks! But I found more minor issues that can be improved.

On Thu, Feb 22, 2018 at 10:18:31AM +0800, zhangyi (F) wrote:
> Hook filesystem check helper to _check_test_fs and _check_scratch_fs for
> constants underlying dirs of overlay filesystem, and introduce scratch

"for constants underlying..." looks confusing, do you mean

... for checking consistency of underlying dirs of overlay filesystem

?

> check helpers for optionally lower/upper/work dirs. These helpers works

I'm not sure I understand what "optionally" means here..

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

These comments look confusing to me too. I think you mean
_overlay_check_scratch_dirs is similar to _check_overlay_scratch_fs, but
it needs lower/upper/work dirs provided as arguments, it's useful for
non-default setups such as multiple lower layers.

> +_overlay_check_scratch_dirs()

I don't think we need it either (sorry I missed it in previous reviews),
just call _overlay_check_dirs directly with proper arguments in patch
5/5 (and umount first if necessary). We already have many helpers for
checking overlayfs, this one just makes it more confusing.

(At least this helper should be introduced in patch 5/5, along with its
first usage.)

Thanks,
Eryu

Re: [xfstests PATCH v4 2/5] overlay: hook filesystem check helper

[zhangyi (F)]

On 2018/2/26 22:51, Eryu Guan Wrote:
> Hi zhangyi,
> 
> I was testing v4 patchset with various overlay configs and results all
> look good, thanks! But I found more minor issues that can be improved.
> 
> On Thu, Feb 22, 2018 at 10:18:31AM +0800, zhangyi (F) wrote:
>> Hook filesystem check helper to _check_test_fs and _check_scratch_fs for
>> constants underlying dirs of overlay filesystem, and introduce scratch
> 
> "for constants underlying..." looks confusing, do you mean
> 
> ... for checking consistency of underlying dirs of overlay filesystem
> 
> ?
> 
>> check helpers for optionally lower/upper/work dirs. These helpers works
> 
> I'm not sure I understand what "optionally" means here..
> 
>> 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

Will to change it to:

_overlay_fsck_dirs $lowerdir $upperdir $workdir $FSCK_OPTIONS $* >>$tmp.fsck 2>&1

Becasue we need to override default extra overlay options(e.g. redirect_dir=xx)
defined in config for some tests.

>> +	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.
> 
> These comments look confusing to me too. I think you mean
> _overlay_check_scratch_dirs is similar to _check_overlay_scratch_fs, but
> it needs lower/upper/work dirs provided as arguments, it's useful for
> non-default setups such as multiple lower layers.
> 
>> +_overlay_check_scratch_dirs()
> 
> I don't think we need it either (sorry I missed it in previous reviews),
> just call _overlay_check_dirs directly with proper arguments in patch
> 5/5 (and umount first if necessary). We already have many helpers for
> checking overlayfs, this one just makes it more confusing.
> 
> (At least this helper should be introduced in patch 5/5, along with its
> first usage.)

Hi Eryu,

Thanks for your suggestions! This helper is use for checking non-default
lower/upper/work scratch dirs.We can use it corresponding to
_overlay_scratch_mount_dirs, so I think wrap this helper is not bad.
I can move it to patch 5/5 in next reversion.

Thanks,
Yi.