[PATCH xfstest v2 0/4] overlay: add fsck.overlay basic tests

[PATCH xfstest v2 0/4] overlay: add fsck.overlay basic tests

[zhangyi (F)]

Hi all:

Here is the second version of test cases for the upcoming fsck.overlay,
corresponding to fsck.overlay v3. Test fsck.overlay check and fix
inconsistency of whiteout/redirect directory/impure xattr.

Changes since v1:

- Fix _check_scratch_fs hook.
- Remove valid/invalid opaque xattr test.
- Add whiteout test cases of valid/invalid whiteouts in opaque/redirect
  parent directory.
- Add impure xattr test.


zhangyi (F) (4):
  overlay: add filesystem check helper
  overlay: add fsck.overlay whiteout test
  overlay: add fsck.overlay redirect directory test
  overlay: add fsck.overlay impure xattr test

 common/config         |   1 +
 common/overlay        |  80 +++++++++++++++++
 common/rc             |   4 +-
 tests/overlay/201     | 232 +++++++++++++++++++++++++++++++++++++++++++++++++
 tests/overlay/201.out |  10 +++
 tests/overlay/202     | 234 ++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/overlay/202.out |  10 +++
 tests/overlay/203     | 124 ++++++++++++++++++++++++++
 tests/overlay/203.out |   3 +
 tests/overlay/group   |   3 +
 10 files changed, 699 insertions(+), 2 deletions(-)
 create mode 100755 tests/overlay/201
 create mode 100644 tests/overlay/201.out
 create mode 100755 tests/overlay/202
 create mode 100644 tests/overlay/202.out
 create mode 100755 tests/overlay/203
 create mode 100644 tests/overlay/203.out

-- 
2.5.0

[PATCH xfstest v2 1/4] overlay: add filesystem check helper

[zhangyi (F)]

Add filesystem check helpers for the upcoming fsck.overlay utility,
and hook them to _check_test_fs and _check_scratch_fs. This helper
works only if fsck.overlay exists.

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

Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
---
 common/config  |  1 +
 common/overlay | 80 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 common/rc      |  4 +--
 3 files changed, 83 insertions(+), 2 deletions(-)

diff --git a/common/config b/common/config
index d0fbfe5..0505b71 100644
--- a/common/config
+++ b/common/config
@@ -236,6 +236,7 @@ case "$HOSTOS" in
         export MKFS_REISER4_PROG="`set_prog_path mkfs.reiser4`"
 	export E2FSCK_PROG="`set_prog_path e2fsck`"
 	export TUNE2FS_PROG="`set_prog_path tune2fs`"
+	export FSCK_OVERLAY_PROG="`set_prog_path fsck.overlay`"
         ;;
 esac
 
diff --git a/common/overlay b/common/overlay
index 1da4ab1..e7f0e62 100644
--- a/common/overlay
+++ b/common/overlay
@@ -151,3 +151,83 @@ _require_scratch_overlay_feature()
 	        _notrun "${FSTYP} feature '${feature}' cannot be enabled on ${SCRATCH_DEV}"
 	_scratch_unmount
 }
+
+_overlay_fsck_dirs()
+{
+	local lowerdir=$1
+	local upperdir=$2
+	local workdir=$3
+	local options=$4
+
+	[[ ! -x "$FSCK_OVERLAY_PROG" ]] && return 0
+
+	$FSCK_OVERLAY_PROG -o lowerdir=$lowerdir -o upperdir=$upperdir \
+			   -o workdir=$workdir $options
+}
+
+_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
+
+	if [ $err != 0 ]; then
+		status=1
+		if [ "$iam" != "check" ]; then
+			exit 1
+		fi
+		return 1
+	fi
+
+	return 0
+}
+
+_overlay_check_fs()
+{
+	local base_dev=$3
+	local base_mnt=$4
+
+	[ "$FSTYP" = overlay ] || return 0
+
+	# Base fs needs to be mounted to check overlay dirs
+	local mounted=""
+	[ -z "$base_dev" ] || mounted=`_is_mounted $base_dev`
+	[ -n "$mounted" ] || _overlay_base_mount $*
+
+	# No need to umount overlay for dir checks with default -n option
+	_overlay_check_dirs $base_mnt/$OVL_LOWER $base_mnt/$OVL_UPPER \
+			    $base_mnt/$OVL_WORK
+	local ret=$?
+
+	[ -n "$mounted" ] || _overlay_base_unmount "$base_dev" "$base_mnt"
+	return $ret
+}
+
+_check_overlay_test_fs()
+{
+	_overlay_check_fs 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 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 4c053a5..79d5f25 100644
--- a/common/rc
+++ b/common/rc
@@ -2507,7 +2507,7 @@ _check_test_fs()
 	# no way to check consistency for GlusterFS
 	;;
     overlay)
-	# no way to check consistency for overlay
+	_check_overlay_test_fs
 	;;
     pvfs2)
 	;;
@@ -2562,7 +2562,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

[PATCH xfstest v2 2/4] overlay: add fsck.overlay whiteout test

[zhangyi (F)]

Add fsck.overlay test case to test it how to deal with orphan/valid
whiteouts in underlying directories of overlayfs.

Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
---
 tests/overlay/201     | 232 ++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/overlay/201.out |  10 +++
 tests/overlay/group   |   1 +
 3 files changed, 243 insertions(+)
 create mode 100755 tests/overlay/201
 create mode 100644 tests/overlay/201.out

diff --git a/tests/overlay/201 b/tests/overlay/201
new file mode 100755
index 0000000..6331b61
--- /dev/null
+++ b/tests/overlay/201
@@ -0,0 +1,232 @@
+#! /bin/bash
+# FS QA Test 201
+#
+# Test fsck.overlay how to deal with whiteouts in overlayfs.
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2017 Huawei.  All Rights Reserved.
+# Author: zhangyi (F) <yi.zhang@huawei.com>
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+#-----------------------------------------------------------------------
+#
+
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1        # failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	cd /
+	rm -f $tmp.*
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# remove previous $seqres.full before test
+rm -f $seqres.full
+
+# real QA test starts here
+_supported_fs overlay
+_supported_os Linux
+_require_scratch
+_require_command "$FSCK_OVERLAY_PROG" fsck.overlay
+
+# remove all files from previous tests
+_scratch_mkfs
+
+OVL_REDIRECT_XATTR="trusted.overlay.redirect"
+OVL_OPAQUE_XATTR="trusted.overlay.opaque"
+OVL_OPAQUE_XATTR_VAL="y"
+
+# Check whiteout
+check_whiteout()
+{
+	local target=$1
+
+	target_type=`stat -c "%F:%t,%T" $target`
+
+	[[ $target_type == "character special file:0,0" ]] || \
+		echo "Valid whiteout removed incorrectly"
+}
+
+# Create a whiteout
+make_whiteout()
+{
+	local target=$1
+
+	mknod $target c 0 0
+}
+
+# Create an opaque directory
+make_opaque_dir()
+{
+	local target=$1
+
+	mkdir -p $target
+	$SETFATTR_PROG -n $OVL_OPAQUE_XATTR -v $OVL_OPAQUE_XATTR_VAL $target
+}
+
+# Create a redirect directory
+make_redirect_dir()
+{
+	local target=$1
+	local value=$2
+
+	mkdir -p $target
+	$SETFATTR_PROG -n $OVL_REDIRECT_XATTR -v $value $target
+}
+
+# Create test directories
+lowerdir=$OVL_BASE_SCRATCH_MNT/lower
+lowerdir2=$OVL_BASE_SCRATCH_MNT/lower2
+upperdir=$OVL_BASE_SCRATCH_MNT/upper
+workdir=$OVL_BASE_SCRATCH_MNT/workdir
+
+mkdir -p $lowerdir $lowerdir2 $upperdir $workdir $workdir
+
+# Test orphan whiteout in lower and upper layer, should remove
+echo "+ Orphan whiteout"
+make_whiteout $lowerdir/foo
+make_whiteout $upperdir/foo
+make_whiteout $upperdir/bar
+
+_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
+	_fail "fsck should not fail"
+ls $lowerdir
+ls $upperdir
+rm -rf $lowerdir/* $upperdir/*
+
+# Test valid whiteout covering lower target, should not remove
+echo "+ Valid whiteout"
+touch $lowerdir2/foo $lowerdir2/bar
+make_whiteout $upperdir/foo
+make_whiteout $lowerdir/bar
+
+_overlay_fsck_dirs "$lowerdir:$lowerdir2" $upperdir $workdir -p >> \
+	 $seqres.full 2>&1 || _fail "fsck should not fail"
+check_whiteout $upperdir/foo
+check_whiteout $lowerdir/bar
+rm -rf $lowerdir/* $lowerdir2/* $upperdir/*
+
+# Test orphan whiteout in opaque directory, should remove
+echo "+ Orphan whiteout(2)"
+mkdir $lowerdir/testdir
+touch $lowerdir/testdir/foo
+make_opaque_dir $upperdir/testdir
+make_whiteout $upperdir/testdir/foo
+
+_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
+	_fail "fsck should not fail"
+ls $upperdir/testdir
+rm -rf $lowerdir/* $upperdir/*
+
+# Test orphan whiteout whose parent path is not an merged directory,
+# should remove
+echo "+ Orphan whiteout(3)"
+mkdir $lowerdir2/testdir1 $lowerdir2/testdir2 $lowerdir2/testdir3
+touch $lowerdir2/testdir1/foo $lowerdir2/testdir2/foo $lowerdir2/testdir3/foo
+mkdir $upperdir/testdir1 $upperdir/testdir2 $upperdir/testdir3
+touch $lowerdir/testdir1
+make_whiteout $lowerdir/testdir2
+make_opaque_dir $lowerdir/testdir3
+make_whiteout $upperdir/testdir1/foo
+make_whiteout $upperdir/testdir2/foo
+make_whiteout $upperdir/testdir3/foo
+
+_overlay_fsck_dirs "$lowerdir:$lowerdir2" $upperdir $workdir -p >> \
+	$seqres.full 2>&1 || _fail "fsck should not fail"
+ls $upperdir/testdir1
+ls $upperdir/testdir2
+ls $upperdir/testdir3
+rm -rf $lowerdir/* $lowerdir2/* $upperdir/*
+
+# Test orphan whiteout in redirect directory, should remove
+echo "+ Orphan whiteout(4)"
+mkdir $lowerdir/testdir
+mkdir $lowerdir/origin
+touch $lowerdir/testdir/foo
+make_redirect_dir $upperdir/testdir "origin"
+make_whiteout $upperdir/testdir/foo
+
+_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
+	_fail "fsck should not fail"
+ls $upperdir/testdir
+rm -rf $lowerdir/* $upperdir/*
+
+# Test valid whiteout in redirect directory cover file in lower
+# redirect origin directory, should not remove
+echo "+ Valid whiteout(2)"
+mkdir $lowerdir/origin
+touch $lowerdir/origin/foo
+make_redirect_dir $upperdir/testdir "origin"
+make_whiteout $upperdir/testdir/foo
+
+_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
+	_fail "fsck should not fail"
+check_whiteout $upperdir/testdir/foo
+rm -rf $lowerdir/* $upperdir/*
+
+# Test valid whiteout covering lower target whose parent directory
+# merge with a redirect directory in the middle layer, should not remove.
+echo "+ Valid whiteout(3)"
+mkdir -p $lowerdir2/origin/subdir
+touch $lowerdir2/origin/subdir/foo
+make_redirect_dir $lowerdir/testdir "origin"
+mkdir -p $upperdir/testdir/subdir
+make_whiteout $upperdir/testdir/subdir/foo
+
+_overlay_fsck_dirs "$lowerdir:$lowerdir2" $upperdir $workdir -p \
+	>> $seqres.full 2>&1 || _fail "fsck should not fail"
+check_whiteout $upperdir/testdir/subdir/foo
+rm -rf $lowerdir/* $lowerdir2/* $upperdir/*
+
+# Test invalid whiteout in opaque subdirectory in a redirect directory,
+# should remove
+echo "+ Orphan whiteout(5)"
+mkdir -p $lowerdir/origin/subdir
+touch $lowerdir/origin/subdir/foo
+make_redirect_dir $upperdir/testdir "origin"
+make_opaque_dir $upperdir/testdir/subdir
+make_whiteout $upperdir/testdir/subdir/foo
+
+_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
+	_fail "fsck should not fail"
+ls $upperdir/testdir/subdir
+rm -rf $lowerdir/* $upperdir/*
+
+# Test valid whiteout in reidrect subdirectory in a opaque directory
+# covering lower target, should not remove
+echo "+ Valid whiteout(4)"
+mkdir $lowerdir/origin
+touch $lowerdir/origin/foo
+make_opaque_dir $upperdir/testdir
+make_redirect_dir $upperdir/testdir/subdir "/origin"
+make_whiteout $upperdir/testdir/subdir/foo
+
+_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
+        _fail "fsck should not fail"
+check_whiteout $upperdir/testdir/subdir/foo
+rm -rf $lowerdir/* $upperdir/*
+
+# success, all done
+status=0
+exit
diff --git a/tests/overlay/201.out b/tests/overlay/201.out
new file mode 100644
index 0000000..157bb85
--- /dev/null
+++ b/tests/overlay/201.out
@@ -0,0 +1,10 @@
+QA output created by 201
++ Orphan whiteout
++ Valid whiteout
++ Orphan whiteout(2)
++ Orphan whiteout(3)
++ Orphan whiteout(4)
++ Valid whiteout(2)
++ Valid whiteout(3)
++ Orphan whiteout(5)
++ Valid whiteout(4)
diff --git a/tests/overlay/group b/tests/overlay/group
index 7e541e4..7c5fcbb 100644
--- a/tests/overlay/group
+++ b/tests/overlay/group
@@ -49,3 +49,4 @@
 044 auto quick copyup hardlink nonsamefs
 047 auto quick copyup hardlink
 048 auto quick copyup hardlink
+201 auto quick fsck
-- 
2.5.0

[PATCH xfstest v2 3/4] overlay: add fsck.overlay redirect directory test

[zhangyi (F)]

Add fsck.overlay test case to test it how to deal with invalid/valid/
duplicate redirect xattr in underlying directories of overlayfs.

Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
---
 tests/overlay/202     | 234 ++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/overlay/202.out |  10 +++
 tests/overlay/group   |   1 +
 3 files changed, 245 insertions(+)
 create mode 100755 tests/overlay/202
 create mode 100644 tests/overlay/202.out

diff --git a/tests/overlay/202 b/tests/overlay/202
new file mode 100755
index 0000000..91bff47
--- /dev/null
+++ b/tests/overlay/202
@@ -0,0 +1,234 @@
+#! /bin/bash
+# FS QA Test 202
+#
+# Test fsck.overlay how to deal with redirect xattr in overlayfs.
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2017 Huawei.  All Rights Reserved.
+# Author: zhangyi (F) <yi.zhang@huawei.com>
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+#-----------------------------------------------------------------------
+#
+
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1        # failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	cd /
+	rm -f $tmp.*
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+. ./common/attr
+
+# remove previous $seqres.full before test
+rm -f $seqres.full
+
+# real QA test starts here
+_supported_fs overlay
+_supported_os Linux
+_require_scratch
+_require_attrs
+_require_command "$FSCK_OVERLAY_PROG" fsck.overlay
+
+# remove all files from previous tests
+_scratch_mkfs
+
+OVL_REDIRECT_XATTR="trusted.overlay.redirect"
+OVL_OPAQUE_XATTR="trusted.overlay.opaque"
+OVL_OPAQUE_XATTR_VAL="y"
+
+# Create a redirect directory
+make_redirect_dir()
+{
+	local target=$1
+	local value=$2
+
+	mkdir -p $target
+	$SETFATTR_PROG -n $OVL_REDIRECT_XATTR -v $value $target
+}
+
+# Check redirect xattr
+check_redirect()
+{
+	local expect=$1
+	local target=$2
+
+	value=$($GETFATTR_PROG --absolute-names --only-values \
+		-n $OVL_REDIRECT_XATTR $target)
+
+	[[ $value == $expect ]]	|| echo "Redirect xattr incorrect"
+}
+
+# Check opaque xattr
+check_opaque()
+{
+	local target=$1
+
+	value=$($GETFATTR_PROG --absolute-names --only-values \
+		-n $OVL_OPAQUE_XATTR $target)
+
+	[[ $value == $OVL_OPAQUE_XATTR_VAL ]] || echo "Opaque xattr incorrect"
+}
+
+# Check whiteout
+check_whiteout()
+{
+	local target=$1
+
+	target_type=`stat -c "%F:%t,%T" $target`
+
+	[[ $target_type == "character special file:0,0" ]] || \
+		echo "Whiteout missing"
+}
+
+# Create test directories
+lowerdir=$OVL_BASE_SCRATCH_MNT/lower
+lowerdir2=$OVL_BASE_SCRATCH_MNT/lower2
+upperdir=$OVL_BASE_SCRATCH_MNT/upper
+workdir=$OVL_BASE_SCRATCH_MNT/workdir
+
+mkdir -p $lowerdir $lowerdir2 $upperdir $workdir $workdir
+
+# Test invalid redirect xattr point to a nonexistent origin, should remove
+echo "+ Invalid redirect"
+make_redirect_dir $upperdir/testdir "abc"
+
+_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
+	_fail "fsck should not fail"
+$GETFATTR_PROG --absolute-names -d -m $OVL_REDIRECT_XATTR $upperdir/testdir
+rm -rf $upperdir/*
+
+# Test invalid redirect xattr point to a file origin, should remove
+echo "+ Invalid redirect(2)"
+touch $lowerdir/origin
+make_redirect_dir $upperdir/testdir "origin"
+
+_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
+	_fail "fsck should not fail"
+$GETFATTR_PROG --absolute-names -d -m $OVL_REDIRECT_XATTR $upperdir/testdir
+rm -rf $lowerdir/* $upperdir/*
+
+# Test valid redirect xattr point to a directory origin in the same directory,
+# should not remove
+echo "+ Valid redirect"
+mkdir $lowerdir/origin
+mknod $upperdir/origin c 0 0
+make_redirect_dir $upperdir/testdir "origin"
+
+_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
+	_fail "fsck should not fail"
+check_redirect "origin" $upperdir/testdir
+rm -rf $lowerdir/* $upperdir/*
+
+# Test valid redirect xattr point to a directory origin in different directories
+# should not remove
+echo "+ Valid redirect(2)"
+mkdir $lowerdir/origin
+mknod $upperdir/origin c 0 0
+make_redirect_dir $upperdir/testdir1/testdir2 "/origin"
+
+_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
+	_fail "fsck should not fail"
+check_redirect "/origin" $upperdir/testdir1/testdir2
+rm -rf $lowerdir/* $upperdir/*
+
+# Test valid redirect xattr but missing whiteout to cover lower target,
+# should fix whiteout
+echo "+ Missing whiteout"
+mkdir $lowerdir/origin
+make_redirect_dir $upperdir/testdir "origin"
+
+_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
+	_fail "fsck should not fail"
+check_redirect "origin" $upperdir/testdir
+check_whiteout $upperdir/origin
+rm -rf $lowerdir/* $upperdir/*
+
+# Test valid redirect xattrs exchanged by rename, should not remove
+echo "+ Valid redirect(3)"
+mkdir $lowerdir/testdir1 $lowerdir/testdir2
+make_redirect_dir $upperdir/testdir1 "testdir2"
+make_redirect_dir $upperdir/testdir2 "testdir1"
+
+_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
+        _fail "fsck should not fail"
+check_redirect "testdir2" $upperdir/testdir1
+check_redirect "testdir1" $upperdir/testdir2
+rm -rf $lowerdir/* $upperdir/*
+
+# Test valid redirect xattr with general directory cover lower origin
+# target, should ask user this directory is merged or not by default
+# and do nothing in auto mode
+echo "+ Valid redirect(4)"
+mkdir $lowerdir/origin
+mkdir $upperdir/origin
+make_redirect_dir $upperdir/testdir "origin"
+
+_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
+	_fail "fsck should not fail"
+check_redirect "origin" $upperdir/testdir
+$GETFATTR_PROG --absolute-names -d -m $OVL_OPAQUE_XATTR $upperdir/testdir
+
+echo "n" > $tmp.input
+_overlay_fsck_dirs $lowerdir $upperdir $workdir <$tmp.input >> $seqres.full 2>&1 || \
+	_fail "fsck should not fail"
+check_redirect "origin" $upperdir/testdir
+check_opaque $upperdir/origin
+rm -rf $lowerdir/* $upperdir/*
+
+# Test invalid redirect xattr with lower same name directory exists,
+# should remove invalid xattr, and ask user directory merge or not
+# by default
+echo "+ Invalid redirect(3)"
+mkdir $lowerdir/testdir
+make_redirect_dir $upperdir/testdir "origin"
+
+echo "y" > $tmp.input
+echo "n" >> $tmp.input
+_overlay_fsck_dirs $lowerdir $upperdir $workdir <$tmp.input >> $seqres.full 2>&1 || \
+        _fail "fsck should not fail"
+$GETFATTR_PROG --absolute-names -d -m $OVL_REDIRECT_XATTR $upperdir/testdir
+check_opaque $upperdir/testdir
+rm -rf $lowerdir/* $upperdir/*
+
+# Test duplicate redirect directories point to one origin, should fail in
+# auto mode, should remove the duplicate one in yes mode
+echo "+ Duplicate redirect"
+mkdir $lowerdir/origin
+mknod $upperdir/origin c 0 0
+make_redirect_dir $upperdir/testdir1 "origin"
+make_redirect_dir $upperdir/testdir2 "origin"
+
+_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 && \
+	_fail "fsck should fail"
+_overlay_fsck_dirs $lowerdir $upperdir $workdir -y >> $seqres.full 2>&1 || \
+	_fail "fsck should not fail"
+redirect_1=`check_redirect "origin" $upperdir/testdir1 2>/dev/null`
+redirect_2=`check_redirect "origin" $upperdir/testdir2 2>/dev/null`
+[[ $redirect_1 == $redirect_2 ]] && echo "Redirect xattr incorrect"
+
+# success, all done
+status=0
+exit
diff --git a/tests/overlay/202.out b/tests/overlay/202.out
new file mode 100644
index 0000000..ae50d96
--- /dev/null
+++ b/tests/overlay/202.out
@@ -0,0 +1,10 @@
+QA output created by 202
++ Invalid redirect
++ Invalid redirect(2)
++ Valid redirect
++ Valid redirect(2)
++ Missing whiteout
++ Valid redirect(3)
++ Valid redirect(4)
++ Invalid redirect(3)
++ Duplicate redirect
diff --git a/tests/overlay/group b/tests/overlay/group
index 7c5fcbb..e39b5e0 100644
--- a/tests/overlay/group
+++ b/tests/overlay/group
@@ -50,3 +50,4 @@
 047 auto quick copyup hardlink
 048 auto quick copyup hardlink
 201 auto quick fsck
+202 auto quick fsck
-- 
2.5.0

[PATCH xfstest v2 4/4] overlay: add fsck.overlay impure xattr test

[zhangyi (F)]

Add fsck.overlay test case to test it how to deal with impure
xattr in underlying directories of overlayfs.

Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
---
 tests/overlay/203     | 124 ++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/overlay/203.out |   3 ++
 tests/overlay/group   |   1 +
 3 files changed, 128 insertions(+)
 create mode 100755 tests/overlay/203
 create mode 100644 tests/overlay/203.out

diff --git a/tests/overlay/203 b/tests/overlay/203
new file mode 100755
index 0000000..f21dc6d
--- /dev/null
+++ b/tests/overlay/203
@@ -0,0 +1,124 @@
+#! /bin/bash
+# FS QA Test 203
+#
+# Test fsck.overlay how to deal with impure xattr in overlayfs.
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2017 Huawei.  All Rights Reserved.
+# Author: zhangyi (F) <yi.zhang@huawei.com>
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+#-----------------------------------------------------------------------
+#
+
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1        # failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	cd /
+	rm -f $tmp.*
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# remove previous $seqres.full before test
+rm -f $seqres.full
+
+# real QA test starts here
+_supported_fs overlay
+_supported_os Linux
+_require_scratch
+_require_command "$FSCK_OVERLAY_PROG" fsck.overlay
+
+# remove all files from previous tests
+_scratch_mkfs
+
+OVL_REDIRECT_XATTR="trusted.overlay.redirect"
+OVL_IMPURE_XATTR="trusted.overlay.impure"
+OVL_IMPURE_XATTR_VAL="y"
+
+# Create a redirect directory
+make_redirect_dir()
+{
+	local target=$1
+	local value=$2
+
+	mkdir -p $target
+	$SETFATTR_PROG -n $OVL_REDIRECT_XATTR -v $value $target
+}
+
+# Remove impure xattr
+remove_impure()
+{
+	local target=$1
+
+	$SETFATTR_PROG -x $OVL_IMPURE_XATTR $target
+}
+
+# Check impure xattr
+check_impure()
+{
+	local target=$1
+
+        value=$($GETFATTR_PROG --absolute-names --only-values \
+                -n $OVL_IMPURE_XATTR $target)
+
+        [[ $value == $OVL_IMPURE_XATTR_VAL ]] || echo "Missing impure xattr"
+}
+
+# Create test directories
+lowerdir=$OVL_BASE_SCRATCH_MNT/lower
+lowerdir2=$OVL_BASE_SCRATCH_MNT/lower2
+upperdir=$OVL_BASE_SCRATCH_MNT/upper
+workdir=$OVL_BASE_SCRATCH_MNT/workdir
+
+mkdir -p $lowerdir $lowerdir2 $upperdir $workdir $workdir
+
+# Test missing impure xattr in directory which has origin files, should fix
+echo "+ Missing impure"
+mkdir $lowerdir/testdir $upperdir/testdir
+touch $lowerdir/testdir/foo
+_overlay_scratch_mount_dirs $lowerdir $upperdir $workdir
+touch $SCRATCH_MNT/testdir/foo
+$UMOUNT_PROG $SCRATCH_MNT
+remove_impure $upperdir/testdir
+
+_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
+	_fail "fsck should not fail"
+check_impure $upperdir/testdir
+rm -rf $lowerdir/* $upperdir/*
+
+# Test missing impure xattr in directory which has redirect directories,
+# should fix
+echo "+ Missing impure(2)"
+mkdir -p $lowerdir/origin
+make_redirect_dir $upperdir/testdir/subdir "/origin"
+
+_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
+        _fail "fsck should not fail"
+check_impure $upperdir/testdir
+rm -rf $lowerdir/* $upperdir/*
+
+# success, all done
+status=0
+exit
diff --git a/tests/overlay/203.out b/tests/overlay/203.out
new file mode 100644
index 0000000..3bedf2a
--- /dev/null
+++ b/tests/overlay/203.out
@@ -0,0 +1,3 @@
+QA output created by 203
++ Missing impure
++ Missing impure(2)
diff --git a/tests/overlay/group b/tests/overlay/group
index e39b5e0..f99d89e 100644
--- a/tests/overlay/group
+++ b/tests/overlay/group
@@ -51,3 +51,4 @@
 048 auto quick copyup hardlink
 201 auto quick fsck
 202 auto quick fsck
+203 auto quick fsck
-- 
2.5.0

Re: [PATCH xfstest v2 2/4] overlay: add fsck.overlay whiteout test

[Amir Goldstein]

On Thu, Dec 28, 2017 at 1:49 PM, zhangyi (F) <yi.zhang@huawei.com> wrote:
> Add fsck.overlay test case to test it how to deal with orphan/valid
> whiteouts in underlying directories of overlayfs.
>
> Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>

Very nice!
Reviewed-by: Amir Goldstein <amir73il@gmail.com>

minor suggestions below

> ---
>  tests/overlay/201     | 232 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/overlay/201.out |  10 +++
>  tests/overlay/group   |   1 +
>  3 files changed, 243 insertions(+)
>  create mode 100755 tests/overlay/201
>  create mode 100644 tests/overlay/201.out
>
> diff --git a/tests/overlay/201 b/tests/overlay/201
> new file mode 100755
> index 0000000..6331b61
> --- /dev/null
> +++ b/tests/overlay/201
> @@ -0,0 +1,232 @@
> +#! /bin/bash
> +# FS QA Test 201
> +#
> +# Test fsck.overlay how to deal with whiteouts in overlayfs.
> +#
> +#-----------------------------------------------------------------------
> +# Copyright (c) 2017 Huawei.  All Rights Reserved.
> +# Author: zhangyi (F) <yi.zhang@huawei.com>
> +#
> +# This program is free software; you can redistribute it and/or
> +# modify it under the terms of the GNU General Public License as
> +# published by the Free Software Foundation.
> +#
> +# This program is distributed in the hope that it would be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write the Free Software Foundation,
> +# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
> +#-----------------------------------------------------------------------
> +#
> +
> +seq=`basename $0`
> +seqres=$RESULT_DIR/$seq
> +echo "QA output created by $seq"
> +
> +here=`pwd`
> +tmp=/tmp/$$
> +status=1        # failure is the default!
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +_cleanup()
> +{
> +       cd /
> +       rm -f $tmp.*
> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +
> +# remove previous $seqres.full before test
> +rm -f $seqres.full
> +
> +# real QA test starts here
> +_supported_fs overlay
> +_supported_os Linux
> +_require_scratch
> +_require_command "$FSCK_OVERLAY_PROG" fsck.overlay
> +
> +# remove all files from previous tests
> +_scratch_mkfs
> +
> +OVL_REDIRECT_XATTR="trusted.overlay.redirect"
> +OVL_OPAQUE_XATTR="trusted.overlay.opaque"
> +OVL_OPAQUE_XATTR_VAL="y"
> +
> +# Check whiteout
> +check_whiteout()
> +{
> +       local target=$1
> +
> +       target_type=`stat -c "%F:%t,%T" $target`
> +
> +       [[ $target_type == "character special file:0,0" ]] || \
> +               echo "Valid whiteout removed incorrectly"
> +}
> +
> +# Create a whiteout
> +make_whiteout()
> +{
> +       local target=$1
> +
> +       mknod $target c 0 0
> +}
> +
> +# Create an opaque directory
> +make_opaque_dir()
> +{
> +       local target=$1
> +
> +       mkdir -p $target
> +       $SETFATTR_PROG -n $OVL_OPAQUE_XATTR -v $OVL_OPAQUE_XATTR_VAL $target
> +}
> +
> +# Create a redirect directory
> +make_redirect_dir()
> +{
> +       local target=$1
> +       local value=$2
> +
> +       mkdir -p $target
> +       $SETFATTR_PROG -n $OVL_REDIRECT_XATTR -v $value $target
> +}
> +
> +# Create test directories
> +lowerdir=$OVL_BASE_SCRATCH_MNT/lower
> +lowerdir2=$OVL_BASE_SCRATCH_MNT/lower2
> +upperdir=$OVL_BASE_SCRATCH_MNT/upper
> +workdir=$OVL_BASE_SCRATCH_MNT/workdir
> +
> +mkdir -p $lowerdir $lowerdir2 $upperdir $workdir $workdir

Would be nice to pack these mkdir after rm -rf as make_dirs helper
to run before every test case, instead of mkdir once and rm -rf after each test.

> +
> +# Test orphan whiteout in lower and upper layer, should remove
> +echo "+ Orphan whiteout"
> +make_whiteout $lowerdir/foo
> +make_whiteout $upperdir/foo
> +make_whiteout $upperdir/bar
> +
> +_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
> +       _fail "fsck should not fail"
> +ls $lowerdir
> +ls $upperdir
> +rm -rf $lowerdir/* $upperdir/*
> +
> +# Test valid whiteout covering lower target, should not remove
> +echo "+ Valid whiteout"
> +touch $lowerdir2/foo $lowerdir2/bar
> +make_whiteout $upperdir/foo
> +make_whiteout $lowerdir/bar
> +
> +_overlay_fsck_dirs "$lowerdir:$lowerdir2" $upperdir $workdir -p >> \
> +        $seqres.full 2>&1 || _fail "fsck should not fail"
> +check_whiteout $upperdir/foo
> +check_whiteout $lowerdir/bar
> +rm -rf $lowerdir/* $lowerdir2/* $upperdir/*
> +
> +# Test orphan whiteout in opaque directory, should remove
> +echo "+ Orphan whiteout(2)"
> +mkdir $lowerdir/testdir
> +touch $lowerdir/testdir/foo
> +make_opaque_dir $upperdir/testdir
> +make_whiteout $upperdir/testdir/foo
> +
> +_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
> +       _fail "fsck should not fail"
> +ls $upperdir/testdir
> +rm -rf $lowerdir/* $upperdir/*
> +
> +# Test orphan whiteout whose parent path is not an merged directory,
> +# should remove
> +echo "+ Orphan whiteout(3)"
> +mkdir $lowerdir2/testdir1 $lowerdir2/testdir2 $lowerdir2/testdir3
> +touch $lowerdir2/testdir1/foo $lowerdir2/testdir2/foo $lowerdir2/testdir3/foo
> +mkdir $upperdir/testdir1 $upperdir/testdir2 $upperdir/testdir3

How about mkdir $upperdir/testdir4 (case of parent is a pure upper dir)

> +touch $lowerdir/testdir1
> +make_whiteout $lowerdir/testdir2
> +make_opaque_dir $lowerdir/testdir3
> +make_whiteout $upperdir/testdir1/foo
> +make_whiteout $upperdir/testdir2/foo
> +make_whiteout $upperdir/testdir3/foo
> +
> +_overlay_fsck_dirs "$lowerdir:$lowerdir2" $upperdir $workdir -p >> \
> +       $seqres.full 2>&1 || _fail "fsck should not fail"
> +ls $upperdir/testdir1
> +ls $upperdir/testdir2
> +ls $upperdir/testdir3
> +rm -rf $lowerdir/* $lowerdir2/* $upperdir/*
> +
> +# Test orphan whiteout in redirect directory, should remove
> +echo "+ Orphan whiteout(4)"
> +mkdir $lowerdir/testdir
> +mkdir $lowerdir/origin
> +touch $lowerdir/testdir/foo
> +make_redirect_dir $upperdir/testdir "origin"
> +make_whiteout $upperdir/testdir/foo
> +
> +_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
> +       _fail "fsck should not fail"
> +ls $upperdir/testdir
> +rm -rf $lowerdir/* $upperdir/*
> +
> +# Test valid whiteout in redirect directory cover file in lower
> +# redirect origin directory, should not remove
> +echo "+ Valid whiteout(2)"
> +mkdir $lowerdir/origin
> +touch $lowerdir/origin/foo
> +make_redirect_dir $upperdir/testdir "origin"
> +make_whiteout $upperdir/testdir/foo
> +
> +_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
> +       _fail "fsck should not fail"
> +check_whiteout $upperdir/testdir/foo
> +rm -rf $lowerdir/* $upperdir/*
> +
> +# Test valid whiteout covering lower target whose parent directory
> +# merge with a redirect directory in the middle layer, should not remove.
> +echo "+ Valid whiteout(3)"
> +mkdir -p $lowerdir2/origin/subdir
> +touch $lowerdir2/origin/subdir/foo
> +make_redirect_dir $lowerdir/testdir "origin"
> +mkdir -p $upperdir/testdir/subdir
> +make_whiteout $upperdir/testdir/subdir/foo
> +
> +_overlay_fsck_dirs "$lowerdir:$lowerdir2" $upperdir $workdir -p \
> +       >> $seqres.full 2>&1 || _fail "fsck should not fail"
> +check_whiteout $upperdir/testdir/subdir/foo
> +rm -rf $lowerdir/* $lowerdir2/* $upperdir/*
> +
> +# Test invalid whiteout in opaque subdirectory in a redirect directory,
> +# should remove
> +echo "+ Orphan whiteout(5)"
> +mkdir -p $lowerdir/origin/subdir
> +touch $lowerdir/origin/subdir/foo
> +make_redirect_dir $upperdir/testdir "origin"
> +make_opaque_dir $upperdir/testdir/subdir
> +make_whiteout $upperdir/testdir/subdir/foo
> +
> +_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
> +       _fail "fsck should not fail"
> +ls $upperdir/testdir/subdir
> +rm -rf $lowerdir/* $upperdir/*
> +
> +# Test valid whiteout in reidrect subdirectory in a opaque directory
> +# covering lower target, should not remove
> +echo "+ Valid whiteout(4)"
> +mkdir $lowerdir/origin
> +touch $lowerdir/origin/foo
> +make_opaque_dir $upperdir/testdir
> +make_redirect_dir $upperdir/testdir/subdir "/origin"
> +make_whiteout $upperdir/testdir/subdir/foo
> +
> +_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
> +        _fail "fsck should not fail"
> +check_whiteout $upperdir/testdir/subdir/foo
> +rm -rf $lowerdir/* $upperdir/*
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/overlay/201.out b/tests/overlay/201.out
> new file mode 100644
> index 0000000..157bb85
> --- /dev/null
> +++ b/tests/overlay/201.out
> @@ -0,0 +1,10 @@
> +QA output created by 201
> ++ Orphan whiteout
> ++ Valid whiteout
> ++ Orphan whiteout(2)
> ++ Orphan whiteout(3)
> ++ Orphan whiteout(4)
> ++ Valid whiteout(2)
> ++ Valid whiteout(3)
> ++ Orphan whiteout(5)
> ++ Valid whiteout(4)
> diff --git a/tests/overlay/group b/tests/overlay/group
> index 7e541e4..7c5fcbb 100644
> --- a/tests/overlay/group
> +++ b/tests/overlay/group
> @@ -49,3 +49,4 @@
>  044 auto quick copyup hardlink nonsamefs
>  047 auto quick copyup hardlink
>  048 auto quick copyup hardlink
> +201 auto quick fsck
> --
> 2.5.0
>

Re: [PATCH xfstest v2 1/4] overlay: add filesystem check helper

[Amir Goldstein]

On Thu, Dec 28, 2017 at 1:49 PM, zhangyi (F) <yi.zhang@huawei.com> wrote:
> Add filesystem check helpers for the upcoming fsck.overlay utility,
> and hook them to _check_test_fs and _check_scratch_fs. This helper
> works only if fsck.overlay exists.
>
> [ _check_test_fs/_check_scratch_fs part picked from Amir's patch, thanks ]
>
> Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>

If this patch doesn't change in following posting, please add:

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

> ---
>  common/config  |  1 +
>  common/overlay | 80 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  common/rc      |  4 +--
>  3 files changed, 83 insertions(+), 2 deletions(-)
>
> diff --git a/common/config b/common/config
> index d0fbfe5..0505b71 100644
> --- a/common/config
> +++ b/common/config
> @@ -236,6 +236,7 @@ case "$HOSTOS" in
>          export MKFS_REISER4_PROG="`set_prog_path mkfs.reiser4`"
>         export E2FSCK_PROG="`set_prog_path e2fsck`"
>         export TUNE2FS_PROG="`set_prog_path tune2fs`"
> +       export FSCK_OVERLAY_PROG="`set_prog_path fsck.overlay`"
>          ;;
>  esac
>
> diff --git a/common/overlay b/common/overlay
> index 1da4ab1..e7f0e62 100644
> --- a/common/overlay
> +++ b/common/overlay
> @@ -151,3 +151,83 @@ _require_scratch_overlay_feature()
>                 _notrun "${FSTYP} feature '${feature}' cannot be enabled on ${SCRATCH_DEV}"
>         _scratch_unmount
>  }
> +
> +_overlay_fsck_dirs()
> +{
> +       local lowerdir=$1
> +       local upperdir=$2
> +       local workdir=$3
> +       local options=$4
> +
> +       [[ ! -x "$FSCK_OVERLAY_PROG" ]] && return 0
> +
> +       $FSCK_OVERLAY_PROG -o lowerdir=$lowerdir -o upperdir=$upperdir \
> +                          -o workdir=$workdir $options
> +}
> +
> +_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
> +
> +       if [ $err != 0 ]; then
> +               status=1
> +               if [ "$iam" != "check" ]; then
> +                       exit 1
> +               fi
> +               return 1
> +       fi
> +
> +       return 0
> +}
> +
> +_overlay_check_fs()
> +{
> +       local base_dev=$3
> +       local base_mnt=$4
> +
> +       [ "$FSTYP" = overlay ] || return 0
> +
> +       # Base fs needs to be mounted to check overlay dirs
> +       local mounted=""
> +       [ -z "$base_dev" ] || mounted=`_is_mounted $base_dev`
> +       [ -n "$mounted" ] || _overlay_base_mount $*
> +
> +       # No need to umount overlay for dir checks with default -n option
> +       _overlay_check_dirs $base_mnt/$OVL_LOWER $base_mnt/$OVL_UPPER \
> +                           $base_mnt/$OVL_WORK
> +       local ret=$?
> +
> +       [ -n "$mounted" ] || _overlay_base_unmount "$base_dev" "$base_mnt"
> +       return $ret
> +}
> +
> +_check_overlay_test_fs()
> +{
> +       _overlay_check_fs 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 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 4c053a5..79d5f25 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -2507,7 +2507,7 @@ _check_test_fs()
>         # no way to check consistency for GlusterFS
>         ;;
>      overlay)
> -       # no way to check consistency for overlay
> +       _check_overlay_test_fs
>         ;;
>      pvfs2)
>         ;;
> @@ -2562,7 +2562,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
>

Re: [PATCH xfstest v2 3/4] overlay: add fsck.overlay redirect directory test

[Amir Goldstein]

On Thu, Dec 28, 2017 at 1:49 PM, zhangyi (F) <yi.zhang@huawei.com> wrote:
> Add fsck.overlay test case to test it how to deal with invalid/valid/
> duplicate redirect xattr in underlying directories of overlayfs.
>
> Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>

Nice. See suggestions to improve.

> ---
>  tests/overlay/202     | 234 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/overlay/202.out |  10 +++
>  tests/overlay/group   |   1 +
>  3 files changed, 245 insertions(+)
>  create mode 100755 tests/overlay/202
>  create mode 100644 tests/overlay/202.out
>
> diff --git a/tests/overlay/202 b/tests/overlay/202
> new file mode 100755
> index 0000000..91bff47
> --- /dev/null
> +++ b/tests/overlay/202
> @@ -0,0 +1,234 @@
> +#! /bin/bash
> +# FS QA Test 202
> +#
> +# Test fsck.overlay how to deal with redirect xattr in overlayfs.
> +#
> +#-----------------------------------------------------------------------
> +# Copyright (c) 2017 Huawei.  All Rights Reserved.
> +# Author: zhangyi (F) <yi.zhang@huawei.com>
> +#
> +# This program is free software; you can redistribute it and/or
> +# modify it under the terms of the GNU General Public License as
> +# published by the Free Software Foundation.
> +#
> +# This program is distributed in the hope that it would be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write the Free Software Foundation,
> +# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
> +#-----------------------------------------------------------------------
> +#
> +
> +seq=`basename $0`
> +seqres=$RESULT_DIR/$seq
> +echo "QA output created by $seq"
> +
> +here=`pwd`
> +tmp=/tmp/$$
> +status=1        # failure is the default!
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +_cleanup()
> +{
> +       cd /
> +       rm -f $tmp.*
> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +. ./common/attr
> +
> +# remove previous $seqres.full before test
> +rm -f $seqres.full
> +
> +# real QA test starts here
> +_supported_fs overlay
> +_supported_os Linux
> +_require_scratch
> +_require_attrs
> +_require_command "$FSCK_OVERLAY_PROG" fsck.overlay
> +
> +# remove all files from previous tests
> +_scratch_mkfs
> +
> +OVL_REDIRECT_XATTR="trusted.overlay.redirect"
> +OVL_OPAQUE_XATTR="trusted.overlay.opaque"
> +OVL_OPAQUE_XATTR_VAL="y"
> +
> +# Create a redirect directory
> +make_redirect_dir()
> +{
> +       local target=$1
> +       local value=$2
> +
> +       mkdir -p $target
> +       $SETFATTR_PROG -n $OVL_REDIRECT_XATTR -v $value $target
> +}
> +
> +# Check redirect xattr
> +check_redirect()
> +{
> +       local expect=$1
> +       local target=$2

Suggest to flip target/expect order to match target/value order in
make_redirect_dir
Then for the tests that expect empty redirect call check_redirect with
empty expect,
e.g. check_redirect $upperdir/testdir ""
of course "" is not strictly needed, but may be more readable
Even better use helper check_not_redirect() to do that

> +
> +       value=$($GETFATTR_PROG --absolute-names --only-values \
> +               -n $OVL_REDIRECT_XATTR $target)
> +
> +       [[ $value == $expect ]] || echo "Redirect xattr incorrect"
> +}
> +
> +# Check opaque xattr
> +check_opaque()
> +{
> +       local target=$1

Here too, I suggest:
       local expect=${2-$OVL_OPAQUE_XATTR_VAL}

And then tests can either call check_opaque $target "" or add a helper
check_not_opaque() to do that

> +
> +       value=$($GETFATTR_PROG --absolute-names --only-values \
> +               -n $OVL_OPAQUE_XATTR $target)
> +
> +       [[ $value == $OVL_OPAQUE_XATTR_VAL ]] || echo "Opaque xattr incorrect"
> +}
> +
> +# Check whiteout
> +check_whiteout()
> +{
> +       local target=$1
> +
> +       target_type=`stat -c "%F:%t,%T" $target`
> +
> +       [[ $target_type == "character special file:0,0" ]] || \
> +               echo "Whiteout missing"
> +}
> +
> +# Create test directories
> +lowerdir=$OVL_BASE_SCRATCH_MNT/lower
> +lowerdir2=$OVL_BASE_SCRATCH_MNT/lower2
> +upperdir=$OVL_BASE_SCRATCH_MNT/upper
> +workdir=$OVL_BASE_SCRATCH_MNT/workdir
> +
> +mkdir -p $lowerdir $lowerdir2 $upperdir $workdir $workdir

Again, suggest make_dirs helper with rm -rf before and drop
all individual rm -rf after tests.

> +
> +# Test invalid redirect xattr point to a nonexistent origin, should remove
> +echo "+ Invalid redirect"
> +make_redirect_dir $upperdir/testdir "abc"
> +
> +_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
> +       _fail "fsck should not fail"
> +$GETFATTR_PROG --absolute-names -d -m $OVL_REDIRECT_XATTR $upperdir/testdir
> +rm -rf $upperdir/*
> +
> +# Test invalid redirect xattr point to a file origin, should remove
> +echo "+ Invalid redirect(2)"
> +touch $lowerdir/origin
> +make_redirect_dir $upperdir/testdir "origin"
> +
> +_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
> +       _fail "fsck should not fail"
> +$GETFATTR_PROG --absolute-names -d -m $OVL_REDIRECT_XATTR $upperdir/testdir
> +rm -rf $lowerdir/* $upperdir/*
> +
> +# Test valid redirect xattr point to a directory origin in the same directory,
> +# should not remove
> +echo "+ Valid redirect"
> +mkdir $lowerdir/origin
> +mknod $upperdir/origin c 0 0
> +make_redirect_dir $upperdir/testdir "origin"
> +
> +_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
> +       _fail "fsck should not fail"
> +check_redirect "origin" $upperdir/testdir
> +rm -rf $lowerdir/* $upperdir/*
> +
> +# Test valid redirect xattr point to a directory origin in different directories
> +# should not remove
> +echo "+ Valid redirect(2)"
> +mkdir $lowerdir/origin
> +mknod $upperdir/origin c 0 0
> +make_redirect_dir $upperdir/testdir1/testdir2 "/origin"
> +
> +_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
> +       _fail "fsck should not fail"
> +check_redirect "/origin" $upperdir/testdir1/testdir2
> +rm -rf $lowerdir/* $upperdir/*
> +
> +# Test valid redirect xattr but missing whiteout to cover lower target,
> +# should fix whiteout
> +echo "+ Missing whiteout"
> +mkdir $lowerdir/origin
> +make_redirect_dir $upperdir/testdir "origin"
> +
> +_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
> +       _fail "fsck should not fail"
> +check_redirect "origin" $upperdir/testdir
> +check_whiteout $upperdir/origin
> +rm -rf $lowerdir/* $upperdir/*
> +
> +# Test valid redirect xattrs exchanged by rename, should not remove
> +echo "+ Valid redirect(3)"
> +mkdir $lowerdir/testdir1 $lowerdir/testdir2
> +make_redirect_dir $upperdir/testdir1 "testdir2"
> +make_redirect_dir $upperdir/testdir2 "testdir1"
> +
> +_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
> +        _fail "fsck should not fail"
> +check_redirect "testdir2" $upperdir/testdir1
> +check_redirect "testdir1" $upperdir/testdir2
> +rm -rf $lowerdir/* $upperdir/*
> +
> +# Test valid redirect xattr with general directory cover lower origin
> +# target, should ask user this directory is merged or not by default
> +# and do nothing in auto mode
> +echo "+ Valid redirect(4)"
> +mkdir $lowerdir/origin
> +mkdir $upperdir/origin
> +make_redirect_dir $upperdir/testdir "origin"
> +
> +_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
> +       _fail "fsck should not fail"
> +check_redirect "origin" $upperdir/testdir
> +$GETFATTR_PROG --absolute-names -d -m $OVL_OPAQUE_XATTR $upperdir/testdir
> +
> +echo "n" > $tmp.input

This approach seems a bit fragile to me.
Maybe you will want to add some questions in the future.
To me it makes better sense to check behavior of fsck -y is automated test,
because some user who does not understand the questions is surely going
to run fsck -y to fix problems.

> +_overlay_fsck_dirs $lowerdir $upperdir $workdir <$tmp.input >> $seqres.full 2>&1 || \
> +       _fail "fsck should not fail"
> +check_redirect "origin" $upperdir/testdir
> +check_opaque $upperdir/origin
> +rm -rf $lowerdir/* $upperdir/*
> +
> +# Test invalid redirect xattr with lower same name directory exists,
> +# should remove invalid xattr, and ask user directory merge or not
> +# by default
> +echo "+ Invalid redirect(3)"
> +mkdir $lowerdir/testdir
> +make_redirect_dir $upperdir/testdir "origin"
> +
> +echo "y" > $tmp.input
> +echo "n" >> $tmp.input

Same here. suggest to validate results of fsck -y.
fsck -y to user means: I don't care how fix it, just fix it!
If user does not care, fsck can have its own arbitrary selections,
but they should be consistent, so we can test them.

> +_overlay_fsck_dirs $lowerdir $upperdir $workdir <$tmp.input >> $seqres.full 2>&1 || \
> +        _fail "fsck should not fail"
> +$GETFATTR_PROG --absolute-names -d -m $OVL_REDIRECT_XATTR $upperdir/testdir
> +check_opaque $upperdir/testdir
> +rm -rf $lowerdir/* $upperdir/*
> +
> +# Test duplicate redirect directories point to one origin, should fail in
> +# auto mode, should remove the duplicate one in yes mode
> +echo "+ Duplicate redirect"
> +mkdir $lowerdir/origin
> +mknod $upperdir/origin c 0 0
> +make_redirect_dir $upperdir/testdir1 "origin"
> +make_redirect_dir $upperdir/testdir2 "origin"
> +
> +_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 && \
> +       _fail "fsck should fail"
> +_overlay_fsck_dirs $lowerdir $upperdir $workdir -y >> $seqres.full 2>&1 || \
> +       _fail "fsck should not fail"
> +redirect_1=`check_redirect "origin" $upperdir/testdir1 2>/dev/null`
> +redirect_2=`check_redirect "origin" $upperdir/testdir2 2>/dev/null`
> +[[ $redirect_1 == $redirect_2 ]] && echo "Redirect xattr incorrect"
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/overlay/202.out b/tests/overlay/202.out
> new file mode 100644
> index 0000000..ae50d96
> --- /dev/null
> +++ b/tests/overlay/202.out
> @@ -0,0 +1,10 @@
> +QA output created by 202
> ++ Invalid redirect
> ++ Invalid redirect(2)
> ++ Valid redirect
> ++ Valid redirect(2)
> ++ Missing whiteout
> ++ Valid redirect(3)
> ++ Valid redirect(4)
> ++ Invalid redirect(3)
> ++ Duplicate redirect
> diff --git a/tests/overlay/group b/tests/overlay/group
> index 7c5fcbb..e39b5e0 100644
> --- a/tests/overlay/group
> +++ b/tests/overlay/group
> @@ -50,3 +50,4 @@
>  047 auto quick copyup hardlink
>  048 auto quick copyup hardlink
>  201 auto quick fsck
> +202 auto quick fsck
> --
> 2.5.0
>

Re: [PATCH xfstest v2 4/4] overlay: add fsck.overlay impure xattr test

[Amir Goldstein]

On Thu, Dec 28, 2017 at 1:49 PM, zhangyi (F) <yi.zhang@huawei.com> wrote:
> Add fsck.overlay test case to test it how to deal with impure
> xattr in underlying directories of overlayfs.
>
> Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>

Looks good, besides the usual make_dirs() suggestion.


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


> ---
>  tests/overlay/203     | 124 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/overlay/203.out |   3 ++
>  tests/overlay/group   |   1 +
>  3 files changed, 128 insertions(+)
>  create mode 100755 tests/overlay/203
>  create mode 100644 tests/overlay/203.out
>
> diff --git a/tests/overlay/203 b/tests/overlay/203
> new file mode 100755
> index 0000000..f21dc6d
> --- /dev/null
> +++ b/tests/overlay/203
> @@ -0,0 +1,124 @@
> +#! /bin/bash
> +# FS QA Test 203
> +#
> +# Test fsck.overlay how to deal with impure xattr in overlayfs.
> +#
> +#-----------------------------------------------------------------------
> +# Copyright (c) 2017 Huawei.  All Rights Reserved.
> +# Author: zhangyi (F) <yi.zhang@huawei.com>
> +#
> +# This program is free software; you can redistribute it and/or
> +# modify it under the terms of the GNU General Public License as
> +# published by the Free Software Foundation.
> +#
> +# This program is distributed in the hope that it would be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write the Free Software Foundation,
> +# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
> +#-----------------------------------------------------------------------
> +#
> +
> +seq=`basename $0`
> +seqres=$RESULT_DIR/$seq
> +echo "QA output created by $seq"
> +
> +here=`pwd`
> +tmp=/tmp/$$
> +status=1        # failure is the default!
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +_cleanup()
> +{
> +       cd /
> +       rm -f $tmp.*
> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +
> +# remove previous $seqres.full before test
> +rm -f $seqres.full
> +
> +# real QA test starts here
> +_supported_fs overlay
> +_supported_os Linux
> +_require_scratch
> +_require_command "$FSCK_OVERLAY_PROG" fsck.overlay
> +
> +# remove all files from previous tests
> +_scratch_mkfs
> +
> +OVL_REDIRECT_XATTR="trusted.overlay.redirect"
> +OVL_IMPURE_XATTR="trusted.overlay.impure"
> +OVL_IMPURE_XATTR_VAL="y"
> +
> +# Create a redirect directory
> +make_redirect_dir()
> +{
> +       local target=$1
> +       local value=$2
> +
> +       mkdir -p $target
> +       $SETFATTR_PROG -n $OVL_REDIRECT_XATTR -v $value $target
> +}
> +
> +# Remove impure xattr
> +remove_impure()
> +{
> +       local target=$1
> +
> +       $SETFATTR_PROG -x $OVL_IMPURE_XATTR $target
> +}
> +
> +# Check impure xattr
> +check_impure()
> +{
> +       local target=$1
> +
> +        value=$($GETFATTR_PROG --absolute-names --only-values \
> +                -n $OVL_IMPURE_XATTR $target)
> +
> +        [[ $value == $OVL_IMPURE_XATTR_VAL ]] || echo "Missing impure xattr"
> +}
> +
> +# Create test directories
> +lowerdir=$OVL_BASE_SCRATCH_MNT/lower
> +lowerdir2=$OVL_BASE_SCRATCH_MNT/lower2
> +upperdir=$OVL_BASE_SCRATCH_MNT/upper
> +workdir=$OVL_BASE_SCRATCH_MNT/workdir
> +
> +mkdir -p $lowerdir $lowerdir2 $upperdir $workdir $workdir
> +
> +# Test missing impure xattr in directory which has origin files, should fix
> +echo "+ Missing impure"
> +mkdir $lowerdir/testdir $upperdir/testdir
> +touch $lowerdir/testdir/foo
> +_overlay_scratch_mount_dirs $lowerdir $upperdir $workdir
> +touch $SCRATCH_MNT/testdir/foo
> +$UMOUNT_PROG $SCRATCH_MNT
> +remove_impure $upperdir/testdir
> +
> +_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
> +       _fail "fsck should not fail"
> +check_impure $upperdir/testdir
> +rm -rf $lowerdir/* $upperdir/*
> +
> +# Test missing impure xattr in directory which has redirect directories,
> +# should fix
> +echo "+ Missing impure(2)"
> +mkdir -p $lowerdir/origin
> +make_redirect_dir $upperdir/testdir/subdir "/origin"
> +
> +_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
> +        _fail "fsck should not fail"
> +check_impure $upperdir/testdir
> +rm -rf $lowerdir/* $upperdir/*
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/overlay/203.out b/tests/overlay/203.out
> new file mode 100644
> index 0000000..3bedf2a
> --- /dev/null
> +++ b/tests/overlay/203.out
> @@ -0,0 +1,3 @@
> +QA output created by 203
> ++ Missing impure
> ++ Missing impure(2)
> diff --git a/tests/overlay/group b/tests/overlay/group
> index e39b5e0..f99d89e 100644
> --- a/tests/overlay/group
> +++ b/tests/overlay/group
> @@ -51,3 +51,4 @@
>  048 auto quick copyup hardlink
>  201 auto quick fsck
>  202 auto quick fsck
> +203 auto quick fsck
> --
> 2.5.0
>

Re: [PATCH xfstest v2 2/4] overlay: add fsck.overlay whiteout test

[zhangyi (F)]

On 2017/12/28 20:11, Amir Goldstein Wrote:
> On Thu, Dec 28, 2017 at 1:49 PM, zhangyi (F) <yi.zhang@huawei.com> wrote:
>> Add fsck.overlay test case to test it how to deal with orphan/valid
>> whiteouts in underlying directories of overlayfs.
>>
>> Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
> 
> Very nice!
> Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> 
> minor suggestions below
> 

Thanks a lot for your suggestions!

[..]
>> +# Create test directories
>> +lowerdir=$OVL_BASE_SCRATCH_MNT/lower
>> +lowerdir2=$OVL_BASE_SCRATCH_MNT/lower2
>> +upperdir=$OVL_BASE_SCRATCH_MNT/upper
>> +workdir=$OVL_BASE_SCRATCH_MNT/workdir
>> +
>> +mkdir -p $lowerdir $lowerdir2 $upperdir $workdir $workdir
> 
> Would be nice to pack these mkdir after rm -rf as make_dirs helper
> to run before every test case, instead of mkdir once and rm -rf after each test.
> 
It looks better, will change it for all 3 test cases.

[..]
>> +# Test orphan whiteout whose parent path is not an merged directory,
>> +# should remove
>> +echo "+ Orphan whiteout(3)"
>> +mkdir $lowerdir2/testdir1 $lowerdir2/testdir2 $lowerdir2/testdir3
>> +touch $lowerdir2/testdir1/foo $lowerdir2/testdir2/foo $lowerdir2/testdir3/foo
>> +mkdir $upperdir/testdir1 $upperdir/testdir2 $upperdir/testdir3
> 
> How about mkdir $upperdir/testdir4 (case of parent is a pure upper dir)
> 
Yes, it's perfect, will add.

Thanks,
Yi.

Re: [PATCH xfstest v2 3/4] overlay: add fsck.overlay redirect directory test

[zhangyi (F)]

On 2017/12/28 20:37, Amir Goldstein Wrote:
> On Thu, Dec 28, 2017 at 1:49 PM, zhangyi (F) <yi.zhang@huawei.com> wrote:
>> Add fsck.overlay test case to test it how to deal with invalid/valid/
>> duplicate redirect xattr in underlying directories of overlayfs.
>>
>> Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
> 
> Nice. See suggestions to improve.

Thanks for your suggestions!

[..]
>> +# Check redirect xattr
>> +check_redirect()
>> +{
>> +       local expect=$1
>> +       local target=$2
> 
> Suggest to flip target/expect order to match target/value order in
> make_redirect_dir
> Then for the tests that expect empty redirect call check_redirect with
> empty expect,
> e.g. check_redirect $upperdir/testdir ""
> of course "" is not strictly needed, but may be more readable
> Even better use helper check_not_redirect() to do that

I prefer check_not_redirect(), will change it.

>> +
>> +       value=$($GETFATTR_PROG --absolute-names --only-values \
>> +               -n $OVL_REDIRECT_XATTR $target)
>> +
>> +       [[ $value == $expect ]] || echo "Redirect xattr incorrect"
>> +}
>> +
>> +# Check opaque xattr
>> +check_opaque()
>> +{
>> +       local target=$1
> 
> Here too, I suggest:
>        local expect=${2-$OVL_OPAQUE_XATTR_VAL}
> 
> And then tests can either call check_opaque $target "" or add a helper
> check_not_opaque() to do that
> 

[..]
>> +# Create test directories
>> +lowerdir=$OVL_BASE_SCRATCH_MNT/lower
>> +lowerdir2=$OVL_BASE_SCRATCH_MNT/lower2
>> +upperdir=$OVL_BASE_SCRATCH_MNT/upper
>> +workdir=$OVL_BASE_SCRATCH_MNT/workdir
>> +
>> +mkdir -p $lowerdir $lowerdir2 $upperdir $workdir $workdir
> 
> Again, suggest make_dirs helper with rm -rf before and drop
> all individual rm -rf after tests.
> 
Will change.

[..]
>> +# Test valid redirect xattr with general directory cover lower origin
>> +# target, should ask user this directory is merged or not by default
>> +# and do nothing in auto mode
>> +echo "+ Valid redirect(4)"
>> +mkdir $lowerdir/origin
>> +mkdir $upperdir/origin
>> +make_redirect_dir $upperdir/testdir "origin"
>> +
>> +_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
>> +       _fail "fsck should not fail"
>> +check_redirect "origin" $upperdir/testdir
>> +$GETFATTR_PROG --absolute-names -d -m $OVL_OPAQUE_XATTR $upperdir/testdir
>> +
>> +echo "n" > $tmp.input
> 
> This approach seems a bit fragile to me.
> Maybe you will want to add some questions in the future.
> To me it makes better sense to check behavior of fsck -y is automated test,
> because some user who does not understand the questions is surely going
> to run fsck -y to fix problems.
> 
Yes, for ordinary users, they are more likely to use -p or -y, this input
will also increase workload in future, is not really necessary, will remove.

>> +_overlay_fsck_dirs $lowerdir $upperdir $workdir <$tmp.input >> $seqres.full 2>&1 || \
>> +       _fail "fsck should not fail"
>> +check_redirect "origin" $upperdir/testdir
>> +check_opaque $upperdir/origin
>> +rm -rf $lowerdir/* $upperdir/*
>> +
>> +# Test invalid redirect xattr with lower same name directory exists,
>> +# should remove invalid xattr, and ask user directory merge or not
>> +# by default
>> +echo "+ Invalid redirect(3)"
>> +mkdir $lowerdir/testdir
>> +make_redirect_dir $upperdir/testdir "origin"
>> +
>> +echo "y" > $tmp.input
>> +echo "n" >> $tmp.input
> 
> Same here. suggest to validate results of fsck -y.
> fsck -y to user means: I don't care how fix it, just fix it!
> If user does not care, fsck can have its own arbitrary selections,
> but they should be consistent, so we can test them.
> 
Will remove, too.

Thanks,
Yi.

Re: [PATCH xfstest v2 3/4] overlay: add fsck.overlay redirect directory test

[zhangyi (F)]

On 2017/12/28 20:37, Amir Goldstein Wrote:

>
>> +# Test valid redirect xattr with general directory cover lower origin
>> +# target, should ask user this directory is merged or not by default
>> +# and do nothing in auto mode
>> +echo "+ Valid redirect(4)"
>> +mkdir $lowerdir/origin
>> +mkdir $upperdir/origin
>> +make_redirect_dir $upperdir/testdir "origin"
>> +
>> +_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
>> +       _fail "fsck should not fail"
>> +check_redirect "origin" $upperdir/testdir
>> +$GETFATTR_PROG --absolute-names -d -m $OVL_OPAQUE_XATTR $upperdir/testdir
>> +
>> +echo "n" > $tmp.input
> This approach seems a bit fragile to me.
> Maybe you will want to add some questions in the future.
> To me it makes better sense to check behavior of fsck -y is automated test,
> because some user who does not understand the questions is surely going
> to run fsck -y to fix problems.
>
>> +_overlay_fsck_dirs $lowerdir $upperdir $workdir <$tmp.input >> $seqres.full 2>&1 || \
>> +       _fail "fsck should not fail"
>> +check_redirect "origin" $upperdir/testdir
>> +check_opaque $upperdir/origin
>> +rm -rf $lowerdir/* $upperdir/*
>> +
Hi Amir：

Think about this again. IIUC, I think there is a consistency problem 
about this case.
If we allow user to create a new directory in upper layer merge with 
lower redirect origin target when overlay offline, we will see duplicate 
d_ino in overlay filesystem after next mount, becasue current 
ovl_getattr() will get the lower d_ino for both merged dir and redirect dir.

eg:
mnt:         dira(ino=x)    dirb(ino=x)
upper:      *dira(ino=y)   dirb(ino=z, redirect=dirb)
lower:       dira(ino=x)

*) User remove whiteout or opaque dir and then newly created when 
overlayfs is offline.

Furthermore, if user rename dira again, will lead to duplicate redirect 
xattr directly.
So I think we should forbid user do this and check by fsck.overlay. Or 
handle by overlay filesytem?  thought?

Thanks，
Yi.

Re: [PATCH xfstest v2 3/4] overlay: add fsck.overlay redirect directory test

[Amir Goldstein]

On Sat, Dec 30, 2017 at 6:06 AM, zhangyi (F) <yizhang089@gmail.com> wrote:
> On 2017/12/28 20:37, Amir Goldstein Wrote:
>
>>
>>> +# Test valid redirect xattr with general directory cover lower origin
>>> +# target, should ask user this directory is merged or not by default
>>> +# and do nothing in auto mode
>>> +echo "+ Valid redirect(4)"
>>> +mkdir $lowerdir/origin
>>> +mkdir $upperdir/origin
>>> +make_redirect_dir $upperdir/testdir "origin"
>>> +
>>> +_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1
>>> || \
>>> +       _fail "fsck should not fail"
>>> +check_redirect "origin" $upperdir/testdir
>>> +$GETFATTR_PROG --absolute-names -d -m $OVL_OPAQUE_XATTR
>>> $upperdir/testdir
>>> +
>>> +echo "n" > $tmp.input
>>
>> This approach seems a bit fragile to me.
>> Maybe you will want to add some questions in the future.
>> To me it makes better sense to check behavior of fsck -y is automated
>> test,
>> because some user who does not understand the questions is surely going
>> to run fsck -y to fix problems.
>>
>>> +_overlay_fsck_dirs $lowerdir $upperdir $workdir <$tmp.input >>
>>> $seqres.full 2>&1 || \
>>> +       _fail "fsck should not fail"
>>> +check_redirect "origin" $upperdir/testdir
>>> +check_opaque $upperdir/origin
>>> +rm -rf $lowerdir/* $upperdir/*
>>> +
>
> Hi Amir：
>
> Think about this again. IIUC, I think there is a consistency problem about
> this case.
> If we allow user to create a new directory in upper layer merge with lower
> redirect origin target when overlay offline, we will see duplicate d_ino in
> overlay filesystem after next mount, becasue current ovl_getattr() will get
> the lower d_ino for both merged dir and redirect dir.
>
> eg:
> mnt:         dira(ino=x)    dirb(ino=x)
> upper:      *dira(ino=y)   dirb(ino=z, redirect=dirb)
> lower:       dira(ino=x)
>
> *) User remove whiteout or opaque dir and then newly created when overlayfs
> is offline.
>
> Furthermore, if user rename dira again, will lead to duplicate redirect
> xattr directly.
> So I think we should forbid user do this and check by fsck.overlay. Or
> handle by overlay filesytem?  thought?
>

I am not sure I follow. Of course there is a consistency problem with
allowing merge
and redirect to same target. merge dir is just a private case of
"redirect to .".
merge+redirect is not different in any way than multiple redirect.
All our discussion w.r.t the "Is merge dir?" question was revolving around the
fact that either the merge dir or the redirected dir needs to be fixed.

And duplicate d_ino is the *least* of the problem.
With duplicate redirect, there can be many objects (all descendants)
in the file system
with the same st_dev;st_ino, which may have different data.

This work in my queue is verifying multiple redirect/merge at lookup time,
but *only* if directories where indexed to begin with.
fsck.overlay will be needed to find and correct duplicate redirects
and even merge
dirs that were copied up without an index and fix the index (if
-overify=on is given).

There is a limitation of verify=on that it cannot use index to verify
mulitple redirect
from mid layers. This is where only fsck.overlay can be used.

Cheers,
Amir.

Re: [PATCH xfstest v2 3/4] overlay: add fsck.overlay redirect directory test

[zhangyi]

On 2017/12/30 17:35, Amir Goldstein Wrote:
> On Sat, Dec 30, 2017 at 6:06 AM, zhangyi (F) <yizhang089@gmail.com> wrote:
>> On 2017/12/28 20:37, Amir Goldstein Wrote:
>>
>>>> +# Test valid redirect xattr with general directory cover lower origin
>>>> +# target, should ask user this directory is merged or not by default
>>>> +# and do nothing in auto mode
>>>> +echo "+ Valid redirect(4)"
>>>> +mkdir $lowerdir/origin
>>>> +mkdir $upperdir/origin
>>>> +make_redirect_dir $upperdir/testdir "origin"
>>>> +
>>>> +_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1
>>>> || \
>>>> +       _fail "fsck should not fail"
>>>> +check_redirect "origin" $upperdir/testdir
>>>> +$GETFATTR_PROG --absolute-names -d -m $OVL_OPAQUE_XATTR
>>>> $upperdir/testdir
>>>> +
>>>> +echo "n" > $tmp.input
>>> This approach seems a bit fragile to me.
>>> Maybe you will want to add some questions in the future.
>>> To me it makes better sense to check behavior of fsck -y is automated
>>> test,
>>> because some user who does not understand the questions is surely going
>>> to run fsck -y to fix problems.
>>>
>>>> +_overlay_fsck_dirs $lowerdir $upperdir $workdir <$tmp.input >>
>>>> $seqres.full 2>&1 || \
>>>> +       _fail "fsck should not fail"
>>>> +check_redirect "origin" $upperdir/testdir
>>>> +check_opaque $upperdir/origin
>>>> +rm -rf $lowerdir/* $upperdir/*
>>>> +
>> Hi Amir：
>>
>> Think about this again. IIUC, I think there is a consistency problem about
>> this case.
>> If we allow user to create a new directory in upper layer merge with lower
>> redirect origin target when overlay offline, we will see duplicate d_ino in
>> overlay filesystem after next mount, becasue current ovl_getattr() will get
>> the lower d_ino for both merged dir and redirect dir.
>>
>> eg:
>> mnt:         dira(ino=x)    dirb(ino=x)
>> upper:      *dira(ino=y)   dirb(ino=z, redirect=dirb)
>> lower:       dira(ino=x)
>>
>> *) User remove whiteout or opaque dir and then newly created when overlayfs
>> is offline.
>>
>> Furthermore, if user rename dira again, will lead to duplicate redirect
>> xattr directly.
>> So I think we should forbid user do this and check by fsck.overlay. Or
>> handle by overlay filesytem?  thought?
>>
> I am not sure I follow. Of course there is a consistency problem with
> allowing merge
> and redirect to same target. merge dir is just a private case of
> "redirect to .".
> merge+redirect is not different in any way than multiple redirect.
> All our discussion w.r.t the "Is merge dir?" question was revolving around the
> fact that either the merge dir or the redirected dir needs to be fixed.
Exactly，If user selsect the "merge" option(not opaque), we also should 
fix the
redirected dir which point to the same origin. Thanks!

> And duplicate d_ino is the *least* of the problem.
> With duplicate redirect, there can be many objects (all descendants)
> in the file system
> with the same st_dev;st_ino, which may have different data.
>
> This work in my queue is verifying multiple redirect/merge at lookup time,
> but *only* if directories where indexed to begin with.
> fsck.overlay will be needed to find and correct duplicate redirects
> and even merge
> dirs that were copied up without an index and fix the index (if
> -overify=on is given).
>
> There is a limitation of verify=on that it cannot use index to verify
> mulitple redirect
> from mid layers. This is where only fsck.overlay can be used.
OK, I can add this check and fix latter.  :）

Cheers,
Yi.

Re: [PATCH xfstest v2 2/4] overlay: add fsck.overlay whiteout test

[Vivek Goyal]

On Thu, Dec 28, 2017 at 07:49:31PM +0800, zhangyi (F) wrote:

[..]
> +# Create test directories
> +lowerdir=$OVL_BASE_SCRATCH_MNT/lower
> +lowerdir2=$OVL_BASE_SCRATCH_MNT/lower2
> +upperdir=$OVL_BASE_SCRATCH_MNT/upper
> +workdir=$OVL_BASE_SCRATCH_MNT/workdir
> +
> +mkdir -p $lowerdir $lowerdir2 $upperdir $workdir $workdir

Minor nit. Why $workdir has been specified twice. I saw it in another test
as well.

Vivek

Re: [PATCH xfstest v2 2/4] overlay: add fsck.overlay whiteout test

[zhangyi (F)]

On 2018/1/3 4:05, Vivek Goyal Wrote:
> On Thu, Dec 28, 2017 at 07:49:31PM +0800, zhangyi (F) wrote:
> 
> [..]
>> +# Create test directories
>> +lowerdir=$OVL_BASE_SCRATCH_MNT/lower
>> +lowerdir2=$OVL_BASE_SCRATCH_MNT/lower2
>> +upperdir=$OVL_BASE_SCRATCH_MNT/upper
>> +workdir=$OVL_BASE_SCRATCH_MNT/workdir
>> +
>> +mkdir -p $lowerdir $lowerdir2 $upperdir $workdir $workdir
> 
> Minor nit. Why $workdir has been specified twice. I saw it in another test
> as well.
> 
Will fix, thanks!

zhangyi.

Re: [PATCH xfstest v2 2/4] overlay: add fsck.overlay whiteout test

[Vivek Goyal]

On Thu, Dec 28, 2017 at 07:49:31PM +0800, zhangyi (F) wrote:

[..]
> +# Test orphan whiteout in lower and upper layer, should remove
> +echo "+ Orphan whiteout"
> +make_whiteout $lowerdir/foo
> +make_whiteout $upperdir/foo
> +make_whiteout $upperdir/bar
> +
> +_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
> +	_fail "fsck should not fail"

I ran fsck.overlay manually (V3 patches) with one orphan whiteout in
upper. I get following output.

fsck.overlay -o lowerdir=lower,upperdir=upper,workdir=work 
Orphan whiteout: /root/fsck-overlay-testing/upper/foo Remove ? [y]: 
y
fsck.overlay:[Error]: Cannot getxattr /root/fsck-overlay-testing/upper/foo trusted.overlay.origin: No such file or directory
Filesystem clean

This message about "overlay.origin"  not being there, should not be an
error. You have already figured out, its an orphan whiteout, and user
already asked you to remove it. So checking or origin after that can
calling it Error might not make much sense. Also on top, later I get
"Filesystem clean".

I think in this case, user should not see any message until and unless a
error actually has happened which prevents from cleaning orphan whiteout.

Vivek

Re: [PATCH xfstest v2 2/4] overlay: add fsck.overlay whiteout test

[zhangyi (F)]

On 2018/1/3 23:04, Vivek Goyal Wrote:
> On Thu, Dec 28, 2017 at 07:49:31PM +0800, zhangyi (F) wrote:
> 
> [..]
>> +# Test orphan whiteout in lower and upper layer, should remove
>> +echo "+ Orphan whiteout"
>> +make_whiteout $lowerdir/foo
>> +make_whiteout $upperdir/foo
>> +make_whiteout $upperdir/bar
>> +
>> +_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
>> +	_fail "fsck should not fail"
> 
> I ran fsck.overlay manually (V3 patches) with one orphan whiteout in
> upper. I get following output.
> 
> fsck.overlay -o lowerdir=lower,upperdir=upper,workdir=work 
> Orphan whiteout: /root/fsck-overlay-testing/upper/foo Remove ? [y]: 
> y
> fsck.overlay:[Error]: Cannot getxattr /root/fsck-overlay-testing/upper/foo trusted.overlay.origin: No such file or directory
> Filesystem clean
> 
> This message about "overlay.origin"  not being there, should not be an
> error. You have already figured out, its an orphan whiteout, and user
> already asked you to remove it. So checking or origin after that can
> calling it Error might not make much sense. Also on top, later I get
> "Filesystem clean".
> 
> I think in this case, user should not see any message until and unless a
> error actually has happened which prevents from cleaning orphan whiteout.
> 

Oh, this is a bug introduced by "fsck.overlay: add impure xattr check", thanks
for testing and pointing this out. I will fix in nest version.

Thanks,
zhangyi.