All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH for xfstests 0/4] overlay: add fsck.overlay basic tests
@ 2017-12-14  6:48 zhangyi (F)
  2017-12-14  6:48 ` [PATCH for xfstests 1/4] overlay: add filesystem check helper zhangyi (F)
                   ` (3 more replies)
  0 siblings, 4 replies; 15+ messages in thread
From: zhangyi (F) @ 2017-12-14  6:48 UTC (permalink / raw)
  To: linux-unionfs, fstests
  Cc: miklos, amir73il, eguan, darrick.wong, yi.zhang, miaoxie

Hi all:

Here is the test cases for fsck.overlay split from fsck.overlay program.
Add fsck helper and hook to _check_test_fs/_check_scratch_fs as Amir
suggested, it will work only if fsck.overlay exists. Implement three basic
cases to test fsck how to deal with whiteouts/opaque xattr/redirect xattr.

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

 common/config         |   1 +
 common/overlay        |  78 +++++++++++++++++++++
 common/rc             |   4 +-
 tests/overlay/201     |  98 ++++++++++++++++++++++++++
 tests/overlay/201.out |   3 +
 tests/overlay/202     | 141 ++++++++++++++++++++++++++++++++++++++
 tests/overlay/202.out |   5 ++
 tests/overlay/203     | 185 ++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/overlay/203.out |   8 +++
 tests/overlay/group   |   3 +
 10 files changed, 524 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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH for xfstests 1/4] overlay: add filesystem check helper
  2017-12-14  6:48 [PATCH for xfstests 0/4] overlay: add fsck.overlay basic tests zhangyi (F)
@ 2017-12-14  6:48 ` zhangyi (F)
  2017-12-14  9:05   ` Amir Goldstein
  2017-12-14  6:48 ` [PATCH for xfstests 2/4] overlay: add fsck.overlay whiteout test zhangyi (F)
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 15+ messages in thread
From: zhangyi (F) @ 2017-12-14  6:48 UTC (permalink / raw)
  To: linux-unionfs, fstests
  Cc: miklos, amir73il, eguan, darrick.wong, yi.zhang, miaoxie

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 | 78 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 common/rc      |  4 +--
 3 files changed, 81 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..689799c 100644
--- a/common/overlay
+++ b/common/overlay
@@ -151,3 +151,81 @@ _require_scratch_overlay_feature()
 	        _notrun "${FSTYP} feature '${feature}' cannot be enabled on ${SCRATCH_DEV}"
 	_scratch_unmount
 }
+
+_overlay_fsck_dirs()
+{
+	local options=$1
+	local lowerdir=$2
+	local upperdir=$3
+	local workdir=$4
+
+	[[ ! -x "$FSCK_OVERLAY_PROG" ]] && return 0
+
+	$FSCK_OVERLAY_PROG $options -l $lowerdir -u $upperdir -w $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
+
+	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..0837637 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_test_fs
 	;;
     pvfs2)
 	;;
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH for xfstests 2/4] overlay: add fsck.overlay whiteout test
  2017-12-14  6:48 [PATCH for xfstests 0/4] overlay: add fsck.overlay basic tests zhangyi (F)
  2017-12-14  6:48 ` [PATCH for xfstests 1/4] overlay: add filesystem check helper zhangyi (F)
@ 2017-12-14  6:48 ` zhangyi (F)
  2017-12-14 13:13   ` Amir Goldstein
  2017-12-14  6:48 ` [PATCH for xfstests 3/4] overlay: add fsck.overlay opaque directory test zhangyi (F)
  2017-12-14  6:48 ` [PATCH for xfstests 4/4] overlay: add fsck.overlay redirect " zhangyi (F)
  3 siblings, 1 reply; 15+ messages in thread
From: zhangyi (F) @ 2017-12-14  6:48 UTC (permalink / raw)
  To: linux-unionfs, fstests
  Cc: miklos, amir73il, eguan, darrick.wong, yi.zhang, miaoxie

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     | 98 +++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/overlay/201.out |  3 ++
 tests/overlay/group   |  1 +
 3 files changed, 102 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..1052baa
--- /dev/null
+++ b/tests/overlay/201
@@ -0,0 +1,98 @@
+#! /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
+
+# 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 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"
+mknod $lowerdir/foo c 0 0
+mknod $upperdir/foo c 0 0
+mknod $upperdir/bar c 0 0
+_overlay_fsck_dirs -a $lowerdir $upperdir $workdir >> $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
+mknod $upperdir/foo c 0 0
+mknod $lowerdir/bar c 0 0
+_overlay_fsck_dirs -a "$lowerdir:$lowerdir2" $upperdir $workdir >> \
+	 $seqres.full 2>&1 || _fail "fsck should not fail"
+check_whiteout $upperdir/foo
+check_whiteout $lowerdir/bar
+
+# success, all done
+status=0
+exit
diff --git a/tests/overlay/201.out b/tests/overlay/201.out
new file mode 100644
index 0000000..20338db
--- /dev/null
+++ b/tests/overlay/201.out
@@ -0,0 +1,3 @@
+QA output created by 201
++ Orphan whiteout
++ Valid whiteout
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

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH for xfstests 3/4] overlay: add fsck.overlay opaque directory test
  2017-12-14  6:48 [PATCH for xfstests 0/4] overlay: add fsck.overlay basic tests zhangyi (F)
  2017-12-14  6:48 ` [PATCH for xfstests 1/4] overlay: add filesystem check helper zhangyi (F)
  2017-12-14  6:48 ` [PATCH for xfstests 2/4] overlay: add fsck.overlay whiteout test zhangyi (F)
@ 2017-12-14  6:48 ` zhangyi (F)
  2017-12-14 13:25   ` Amir Goldstein
  2017-12-14  6:48 ` [PATCH for xfstests 4/4] overlay: add fsck.overlay redirect " zhangyi (F)
  3 siblings, 1 reply; 15+ messages in thread
From: zhangyi (F) @ 2017-12-14  6:48 UTC (permalink / raw)
  To: linux-unionfs, fstests
  Cc: miklos, amir73il, eguan, darrick.wong, yi.zhang, miaoxie

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

Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
---
 tests/overlay/202     | 141 ++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/overlay/202.out |   5 ++
 tests/overlay/group   |   1 +
 3 files changed, 147 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..3f217ad
--- /dev/null
+++ b/tests/overlay/202
@@ -0,0 +1,141 @@
+#! /bin/bash
+# FS QA Test 202
+#
+# Test fsck.overlay how to deal with opaque 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_OPAQUE_XATTR="trusted.overlay.opaque"
+OVL_OPAQUE_XATTR_VAL="y"
+
+# Set opaque xattr to a directory
+set_opaque()
+{
+	local target=$1
+
+	$SETFATTR_PROG -n $OVL_OPAQUE_XATTR -v $OVL_OPAQUE_XATTR_VAL $target
+}
+
+# Check opaque xattr
+check_opaque()
+{
+	local dir=$1
+
+	opaque=$($GETFATTR_PROG --absolute-names --only-values -n \
+		$OVL_OPAQUE_XATTR $dir)
+
+	[[ $opaque == $OVL_OPAQUE_XATTR_VAL ]] || \
+		echo "Opaque xattr removed incorrectly"
+}
+
+# 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 opaque xattr covering a nonexistent lower target, should remove
+echo "+ Invalid opaque"
+mkdir $lowerdir/testdir1
+mkdir -p $upperdir/testdir/testdir2
+set_opaque $lowerdir/testdir1
+set_opaque $upperdir/testdir/testdir2
+_overlay_fsck_dirs -a $lowerdir $upperdir $workdir >> $seqres.full 2>&1 || \
+	_fail "fsck should not fail"
+$GETFATTR_PROG --absolute-names -d -m $OVL_OPAQUE_XATTR_VAL $lowerdir/testdir1
+$GETFATTR_PROG --absolute-names -d -m $OVL_OPAQUE_XATTR_VAL $upperdir/testdir/testdir2
+rm -rf $lowerdir/* $upperdir/*
+
+# Test valid opaque xattr in upper covering lower target, should not remove
+echo "+ Valid opaque"
+mkdir $lowerdir/testdir
+touch $lowerdir/foo
+mkdir $upperdir/testdir
+mkdir $upperdir/foo
+set_opaque $upperdir/testdir
+set_opaque $upperdir/foo
+_overlay_fsck_dirs -a $lowerdir $upperdir $workdir >> $seqres.full 2>&1 || \
+	_fail "fsck should not fail"
+check_opaque $upperdir/testdir
+check_opaque $upperdir/foo
+rm -rf $lowerdir/* $upperdir/*
+
+# Test valid opaque xattr in middle layer covering lower target, should not remove
+echo "+ Valid opaque(2)"
+mkdir $lowerdir2/testdir
+touch $lowerdir2/foo
+mkdir $lowerdir/testdir
+mkdir $lowerdir/foo
+set_opaque $lowerdir/testdir
+set_opaque $lowerdir/foo
+_overlay_fsck_dirs -a "$lowerdir:$lowerdir2" $upperdir $workdir >> \
+	$seqres.full 2>&1 || _fail "fsck should not fail"
+check_opaque $lowerdir/testdir
+check_opaque $lowerdir/foo
+rm -rf $lowerdir2/* $lowerdir/*
+
+# Test valid opaque in merged directory, should not remove
+echo "+ Valid opaque(3)"
+mkdir $lowerdir/testdir1
+mkdir -p $upperdir/testdir1/testdir2
+set_opaque $upperdir/testdir1/testdir2
+_overlay_fsck_dirs -a $lowerdir $upperdir $workdir >> $seqres.full 2>&1 || \
+	_fail "fsck should not fail"
+check_opaque $upperdir/testdir1/testdir2
+rm -rf $lowerdir/* $upperdir/*
+
+# success, all done
+status=0
+exit
diff --git a/tests/overlay/202.out b/tests/overlay/202.out
new file mode 100644
index 0000000..1aadc4d
--- /dev/null
+++ b/tests/overlay/202.out
@@ -0,0 +1,5 @@
+QA output created by 202
++ Invalid opaque
++ Valid opaque
++ Valid opaque(2)
++ Valid opaque(3)
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

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH for xfstests 4/4] overlay: add fsck.overlay redirect directory test
  2017-12-14  6:48 [PATCH for xfstests 0/4] overlay: add fsck.overlay basic tests zhangyi (F)
                   ` (2 preceding siblings ...)
  2017-12-14  6:48 ` [PATCH for xfstests 3/4] overlay: add fsck.overlay opaque directory test zhangyi (F)
@ 2017-12-14  6:48 ` zhangyi (F)
  2017-12-14 13:44   ` Amir Goldstein
  3 siblings, 1 reply; 15+ messages in thread
From: zhangyi (F) @ 2017-12-14  6:48 UTC (permalink / raw)
  To: linux-unionfs, fstests
  Cc: miklos, amir73il, eguan, darrick.wong, yi.zhang, miaoxie

Add fsck.overlay test case to test it how to deal with invalid/valid
redirect xattr in underlying directories of overlayfs. It should remove
the invalid redirect dir xattr automatically and keep the valid one.

Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
---
 tests/overlay/203     | 185 ++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/overlay/203.out |   8 +++
 tests/overlay/group   |   1 +
 3 files changed, 194 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..ccf1109
--- /dev/null
+++ b/tests/overlay/203
@@ -0,0 +1,185 @@
+#! /bin/bash
+# FS QA Test 203
+#
+# 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"
+
+# Set redirect xattr to a directory
+set_redirect()
+{
+	local target=$1
+	local value=$2
+
+	$SETFATTR_PROG -n $OVL_REDIRECT_XATTR -v $value $target
+}
+
+# Check xattr is correct or not
+check_xattr()
+{
+	local name=$1
+	local expect=$2
+	local target=$3
+
+	value=$($GETFATTR_PROG --absolute-names --only-values -n $name $target)
+
+	[[ $value == $expect ]] || echo "$name 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 "Valid whiteout removed incorrectly"
+}
+
+# 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"
+mkdir $upperdir/testdir
+set_redirect $upperdir/testdir "abc"
+_overlay_fsck_dirs -a $lowerdir $upperdir $workdir >> $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
+mkdir $upperdir/testdir
+set_redirect $upperdir/testdir "origin"
+_overlay_fsck_dirs -a $lowerdir $upperdir $workdir >> $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
+mkdir $upperdir/testdir
+set_redirect $upperdir/testdir "origin"
+_overlay_fsck_dirs -a $lowerdir $upperdir $workdir >> $seqres.full 2>&1 || \
+	_fail "fsck should not fail"
+check_xattr $OVL_REDIRECT_XATTR "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
+mkdir -p $upperdir/testdir1/testdir2
+set_redirect $upperdir/testdir1/testdir2 "/origin"
+_overlay_fsck_dirs -a $lowerdir $upperdir $workdir >> $seqres.full 2>&1 || \
+	_fail "fsck should not fail"
+check_xattr $OVL_REDIRECT_XATTR "/origin" $upperdir/testdir1/testdir2
+rm -rf $lowerdir/* $upperdir/*
+
+# Test missing whiteout with valid redirect directory, should fix whiteout
+echo "+ Missing whiteout"
+mkdir $lowerdir/origin
+mkdir $upperdir/testdir
+set_redirect $upperdir/testdir "origin"
+_overlay_fsck_dirs -a $lowerdir $upperdir $workdir >> $seqres.full 2>&1 || \
+	_fail "fsck should not fail"
+check_xattr $OVL_REDIRECT_XATTR "origin" $upperdir/testdir
+check_whiteout $upperdir/origin
+rm -rf $lowerdir/* $upperdir/*
+
+# Test missing opaque xattr with valid redirect directory, should fix opaque
+echo "+ Missing opaque"
+mkdir $lowerdir/origin
+mkdir $upperdir/origin
+mkdir $upperdir/testdir
+set_redirect $upperdir/testdir "origin"
+_overlay_fsck_dirs -a $lowerdir $upperdir $workdir >> $seqres.full 2>&1 || \
+	_fail "fsck should not fail"
+check_xattr $OVL_REDIRECT_XATTR "origin" $upperdir/testdir
+check_xattr $OVL_OPAQUE_XATTR $OVL_OPAQUE_XATTR_VAL $upperdir/origin
+rm -rf $lowerdir/* $upperdir/*
+
+# Test duplicate redirect directories point to one origin, should fail in
+# -a 'auto' mode, should remove the duplicate one in -y 'yes' mode
+echo "+ Duplicate redirect"
+mkdir $lowerdir/origin
+mknod $upperdir/origin c 0 0
+mkdir $upperdir/testdir1
+mkdir $upperdir/testdir2
+set_redirect $upperdir/testdir1 "origin"
+set_redirect $upperdir/testdir2 "origin"
+_overlay_fsck_dirs -a $lowerdir $upperdir $workdir >> $seqres.full 2>&1 && \
+	_fail "fsck should fail"
+_overlay_fsck_dirs -y $lowerdir $upperdir $workdir >> $seqres.full 2>&1 || \
+	_fail "fsck should not fail"
+check_xattr $OVL_REDIRECT_XATTR "origin" $upperdir/testdir2
+
+# success, all done
+status=0
+exit
diff --git a/tests/overlay/203.out b/tests/overlay/203.out
new file mode 100644
index 0000000..3f8155d
--- /dev/null
+++ b/tests/overlay/203.out
@@ -0,0 +1,8 @@
+QA output created by 203
++ Invalid redirect
++ Invalid redirect(2)
++ Valid redirect
++ Valid redirect(2)
++ Missing whiteout
++ Missing opaque
++ Duplicate redirect
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

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH for xfstests 1/4] overlay: add filesystem check helper
  2017-12-14  6:48 ` [PATCH for xfstests 1/4] overlay: add filesystem check helper zhangyi (F)
@ 2017-12-14  9:05   ` Amir Goldstein
  2017-12-14 12:40     ` zhangyi (F)
  0 siblings, 1 reply; 15+ messages in thread
From: Amir Goldstein @ 2017-12-14  9:05 UTC (permalink / raw)
  To: zhangyi (F)
  Cc: overlayfs, fstests, Miklos Szeredi, Eryu Guan, Darrick J. Wong, Miao Xie

On Thu, Dec 14, 2017 at 8:48 AM, 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>
> ---
>  common/config  |  1 +
>  common/overlay | 78 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  common/rc      |  4 +--
>  3 files changed, 81 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..689799c 100644
> --- a/common/overlay
> +++ b/common/overlay
> @@ -151,3 +151,81 @@ _require_scratch_overlay_feature()
>                 _notrun "${FSTYP} feature '${feature}' cannot be enabled on ${SCRATCH_DEV}"
>         _scratch_unmount
>  }
> +
> +_overlay_fsck_dirs()
> +{
> +       local options=$1
> +       local lowerdir=$2
> +       local upperdir=$3
> +       local workdir=$4
> +
> +       [[ ! -x "$FSCK_OVERLAY_PROG" ]] && return 0
> +
> +       $FSCK_OVERLAY_PROG $options -l $lowerdir -u $upperdir -w $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

Maybe the tmp.fsck output reporting to seqres.full should be done in
_overlay_fsck_dirs?
I think this output could be useful for understanding fsck tests failure.

> +       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..0837637 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_test_fs

_check_overlay_scratch_fs

Thanks,
Amir.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH for xfstests 1/4] overlay: add filesystem check helper
  2017-12-14  9:05   ` Amir Goldstein
@ 2017-12-14 12:40     ` zhangyi (F)
  2017-12-14 13:14       ` Amir Goldstein
  0 siblings, 1 reply; 15+ messages in thread
From: zhangyi (F) @ 2017-12-14 12:40 UTC (permalink / raw)
  To: Amir Goldstein
  Cc: overlayfs, fstests, Miklos Szeredi, Eryu Guan, Darrick J. Wong, Miao Xie

On 2017/12/14 17:05, Amir Goldstein Write:
> On Thu, Dec 14, 2017 at 8:48 AM, 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>
>> ---
>>  common/config  |  1 +
>>  common/overlay | 78 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  common/rc      |  4 +--
>>  3 files changed, 81 insertions(+), 2 deletions(-)
>>
[..]
>> +_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
> 
> Maybe the tmp.fsck output reporting to seqres.full should be done in
> _overlay_fsck_dirs?
> I think this output could be useful for understanding fsck tests failure.

If we do these in _overlay_fsck_dirs, we can get output only when fsck return
fail, but this output maybe useful for understanding fsck.overlay even through
fsck pass when we test it. So I call _overlay_fsck_dirs and put output to
seqres.full alone in each test case now, see 0002-0004 patches.
But it's also fine to put these into _overlay_fsck_dirs.

[..]
>> --- 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_test_fs
> 
> _check_overlay_scratch_fs
> 

will fix

Thanks,
Yi.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH for xfstests 2/4] overlay: add fsck.overlay whiteout test
  2017-12-14  6:48 ` [PATCH for xfstests 2/4] overlay: add fsck.overlay whiteout test zhangyi (F)
@ 2017-12-14 13:13   ` Amir Goldstein
  2017-12-15  5:35     ` zhangyi (F)
  0 siblings, 1 reply; 15+ messages in thread
From: Amir Goldstein @ 2017-12-14 13:13 UTC (permalink / raw)
  To: zhangyi (F)
  Cc: overlayfs, fstests, Miklos Szeredi, Eryu Guan, Darrick J. Wong, Miao Xie

On Thu, Dec 14, 2017 at 8:48 AM, 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>
> ---
>  tests/overlay/201     | 98 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/overlay/201.out |  3 ++
>  tests/overlay/group   |  1 +
>  3 files changed, 102 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..1052baa
> --- /dev/null
> +++ b/tests/overlay/201
> @@ -0,0 +1,98 @@
> +#! /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
> +
> +# 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 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"
> +mknod $lowerdir/foo c 0 0
> +mknod $upperdir/foo c 0 0
> +mknod $upperdir/bar c 0 0
> +_overlay_fsck_dirs -a $lowerdir $upperdir $workdir >> $seqres.full 2>&1 || \

Don'y you mean -p ?  ;-p


> +       _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
> +mknod $upperdir/foo c 0 0
> +mknod $lowerdir/bar c 0 0
> +_overlay_fsck_dirs -a "$lowerdir:$lowerdir2" $upperdir $workdir >> \
> +        $seqres.full 2>&1 || _fail "fsck should not fail"
> +check_whiteout $upperdir/foo
> +check_whiteout $lowerdir/bar
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/overlay/201.out b/tests/overlay/201.out
> new file mode 100644
> index 0000000..20338db
> --- /dev/null
> +++ b/tests/overlay/201.out
> @@ -0,0 +1,3 @@
> +QA output created by 201
> ++ Orphan whiteout
> ++ Valid whiteout
> 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
> --


Looks good

Amir.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH for xfstests 1/4] overlay: add filesystem check helper
  2017-12-14 12:40     ` zhangyi (F)
@ 2017-12-14 13:14       ` Amir Goldstein
  0 siblings, 0 replies; 15+ messages in thread
From: Amir Goldstein @ 2017-12-14 13:14 UTC (permalink / raw)
  To: zhangyi (F)
  Cc: overlayfs, fstests, Miklos Szeredi, Eryu Guan, Darrick J. Wong, Miao Xie

On Thu, Dec 14, 2017 at 2:40 PM, zhangyi (F) <yi.zhang@huawei.com> wrote:
> On 2017/12/14 17:05, Amir Goldstein Write:
>> On Thu, Dec 14, 2017 at 8:48 AM, 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>
>>> ---
>>>  common/config  |  1 +
>>>  common/overlay | 78 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>  common/rc      |  4 +--
>>>  3 files changed, 81 insertions(+), 2 deletions(-)
>>>
> [..]
>>> +_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
>>
>> Maybe the tmp.fsck output reporting to seqres.full should be done in
>> _overlay_fsck_dirs?
>> I think this output could be useful for understanding fsck tests failure.
>
> If we do these in _overlay_fsck_dirs, we can get output only when fsck return
> fail, but this output maybe useful for understanding fsck.overlay even through
> fsck pass when we test it. So I call _overlay_fsck_dirs and put output to
> seqres.full alone in each test case now, see 0002-0004 patches.
> But it's also fine to put these into _overlay_fsck_dirs.
>

I understand. Fine by me to keep this patch unchanged.
Besides the _check_overlay_scratch_fs typo,
Looks good.
Amir.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH for xfstests 3/4] overlay: add fsck.overlay opaque directory test
  2017-12-14  6:48 ` [PATCH for xfstests 3/4] overlay: add fsck.overlay opaque directory test zhangyi (F)
@ 2017-12-14 13:25   ` Amir Goldstein
  0 siblings, 0 replies; 15+ messages in thread
From: Amir Goldstein @ 2017-12-14 13:25 UTC (permalink / raw)
  To: zhangyi (F)
  Cc: overlayfs, fstests, Miklos Szeredi, Eryu Guan, Darrick J. Wong, Miao Xie

On Thu, Dec 14, 2017 at 8:48 AM, zhangyi (F) <yi.zhang@huawei.com> wrote:
> Add fsck.overlay test case to test it how to deal with invalid/valid
> opaque xattr in underlying directories of overlayfs.
>
> Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
> ---
>  tests/overlay/202     | 141 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/overlay/202.out |   5 ++
>  tests/overlay/group   |   1 +
>  3 files changed, 147 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..3f217ad
> --- /dev/null
> +++ b/tests/overlay/202
> @@ -0,0 +1,141 @@
> +#! /bin/bash
> +# FS QA Test 202
> +#
> +# Test fsck.overlay how to deal with opaque 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_OPAQUE_XATTR="trusted.overlay.opaque"
> +OVL_OPAQUE_XATTR_VAL="y"
> +
> +# Set opaque xattr to a directory
> +set_opaque()
> +{
> +       local target=$1
> +
> +       $SETFATTR_PROG -n $OVL_OPAQUE_XATTR -v $OVL_OPAQUE_XATTR_VAL $target
> +}
> +
> +# Check opaque xattr
> +check_opaque()
> +{
> +       local dir=$1
> +
> +       opaque=$($GETFATTR_PROG --absolute-names --only-values -n \
> +               $OVL_OPAQUE_XATTR $dir)
> +
> +       [[ $opaque == $OVL_OPAQUE_XATTR_VAL ]] || \
> +               echo "Opaque xattr removed incorrectly"
> +}
> +
> +# 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 opaque xattr covering a nonexistent lower target, should remove
> +echo "+ Invalid opaque"
> +mkdir $lowerdir/testdir1
> +mkdir -p $upperdir/testdir/testdir2
> +set_opaque $lowerdir/testdir1
> +set_opaque $upperdir/testdir/testdir2
> +_overlay_fsck_dirs -a $lowerdir $upperdir $workdir >> $seqres.full 2>&1 || \
> +       _fail "fsck should not fail"
> +$GETFATTR_PROG --absolute-names -d -m $OVL_OPAQUE_XATTR_VAL $lowerdir/testdir1
> +$GETFATTR_PROG --absolute-names -d -m $OVL_OPAQUE_XATTR_VAL $upperdir/testdir/testdir2
> +rm -rf $lowerdir/* $upperdir/*
> +
> +# Test valid opaque xattr in upper covering lower target, should not remove
> +echo "+ Valid opaque"
> +mkdir $lowerdir/testdir
> +touch $lowerdir/foo
> +mkdir $upperdir/testdir
> +mkdir $upperdir/foo
> +set_opaque $upperdir/testdir
> +set_opaque $upperdir/foo
> +_overlay_fsck_dirs -a $lowerdir $upperdir $workdir >> $seqres.full 2>&1 || \

Looks good except ;-p

> +       _fail "fsck should not fail"
> +check_opaque $upperdir/testdir
> +check_opaque $upperdir/foo
> +rm -rf $lowerdir/* $upperdir/*
> +
> +# Test valid opaque xattr in middle layer covering lower target, should not remove
> +echo "+ Valid opaque(2)"
> +mkdir $lowerdir2/testdir
> +touch $lowerdir2/foo
> +mkdir $lowerdir/testdir
> +mkdir $lowerdir/foo
> +set_opaque $lowerdir/testdir
> +set_opaque $lowerdir/foo
> +_overlay_fsck_dirs -a "$lowerdir:$lowerdir2" $upperdir $workdir >> \
> +       $seqres.full 2>&1 || _fail "fsck should not fail"
> +check_opaque $lowerdir/testdir
> +check_opaque $lowerdir/foo
> +rm -rf $lowerdir2/* $lowerdir/*
> +
> +# Test valid opaque in merged directory, should not remove
> +echo "+ Valid opaque(3)"
> +mkdir $lowerdir/testdir1
> +mkdir -p $upperdir/testdir1/testdir2
> +set_opaque $upperdir/testdir1/testdir2
> +_overlay_fsck_dirs -a $lowerdir $upperdir $workdir >> $seqres.full 2>&1 || \
> +       _fail "fsck should not fail"
> +check_opaque $upperdir/testdir1/testdir2
> +rm -rf $lowerdir/* $upperdir/*
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/overlay/202.out b/tests/overlay/202.out
> new file mode 100644
> index 0000000..1aadc4d
> --- /dev/null
> +++ b/tests/overlay/202.out
> @@ -0,0 +1,5 @@
> +QA output created by 202
> ++ Invalid opaque
> ++ Valid opaque
> ++ Valid opaque(2)
> ++ Valid opaque(3)
> 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
>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH for xfstests 4/4] overlay: add fsck.overlay redirect directory test
  2017-12-14  6:48 ` [PATCH for xfstests 4/4] overlay: add fsck.overlay redirect " zhangyi (F)
@ 2017-12-14 13:44   ` Amir Goldstein
  2017-12-15  6:42     ` zhangyi (F)
  0 siblings, 1 reply; 15+ messages in thread
From: Amir Goldstein @ 2017-12-14 13:44 UTC (permalink / raw)
  To: zhangyi (F)
  Cc: overlayfs, fstests, Miklos Szeredi, Eryu Guan, Darrick J. Wong, Miao Xie

On Thu, Dec 14, 2017 at 8:48 AM, zhangyi (F) <yi.zhang@huawei.com> wrote:
> Add fsck.overlay test case to test it how to deal with invalid/valid
> redirect xattr in underlying directories of overlayfs. It should remove
> the invalid redirect dir xattr automatically and keep the valid one.
>
> Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
> ---
>  tests/overlay/203     | 185 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/overlay/203.out |   8 +++
>  tests/overlay/group   |   1 +
>  3 files changed, 194 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..ccf1109
> --- /dev/null
> +++ b/tests/overlay/203
> @@ -0,0 +1,185 @@
> +#! /bin/bash
> +# FS QA Test 203
> +#
> +# 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"
> +
> +# Set redirect xattr to a directory
> +set_redirect()
> +{
> +       local target=$1
> +       local value=$2
> +
> +       $SETFATTR_PROG -n $OVL_REDIRECT_XATTR -v $value $target
> +}
> +
> +# Check xattr is correct or not
> +check_xattr()
> +{
> +       local name=$1
> +       local expect=$2
> +       local target=$3
> +
> +       value=$($GETFATTR_PROG --absolute-names --only-values -n $name $target)
> +
> +       [[ $value == $expect ]] || echo "$name xattr incorrect"
> +}
> +

It would be nice if you added check_redirect() and check_opaque() mini helpers.

> +# 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 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"
> +mkdir $upperdir/testdir
> +set_redirect $upperdir/testdir "abc"
> +_overlay_fsck_dirs -a $lowerdir $upperdir $workdir >> $seqres.full 2>&1 || \
> +       _fail "fsck should not fail"

;-p

BTW, if you like -a, I don't mind if you keep it as well.
e2fsck seems to accept them both.

> +$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
> +mkdir $upperdir/testdir
> +set_redirect $upperdir/testdir "origin"
> +_overlay_fsck_dirs -a $lowerdir $upperdir $workdir >> $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
> +mkdir $upperdir/testdir
> +set_redirect $upperdir/testdir "origin"
> +_overlay_fsck_dirs -a $lowerdir $upperdir $workdir >> $seqres.full 2>&1 || \
> +       _fail "fsck should not fail"
> +check_xattr $OVL_REDIRECT_XATTR "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
> +mkdir -p $upperdir/testdir1/testdir2
> +set_redirect $upperdir/testdir1/testdir2 "/origin"
> +_overlay_fsck_dirs -a $lowerdir $upperdir $workdir >> $seqres.full 2>&1 || \
> +       _fail "fsck should not fail"
> +check_xattr $OVL_REDIRECT_XATTR "/origin" $upperdir/testdir1/testdir2
> +rm -rf $lowerdir/* $upperdir/*
> +
> +# Test missing whiteout with valid redirect directory, should fix whiteout
> +echo "+ Missing whiteout"
> +mkdir $lowerdir/origin
> +mkdir $upperdir/testdir
> +set_redirect $upperdir/testdir "origin"
> +_overlay_fsck_dirs -a $lowerdir $upperdir $workdir >> $seqres.full 2>&1 || \
> +       _fail "fsck should not fail"
> +check_xattr $OVL_REDIRECT_XATTR "origin" $upperdir/testdir
> +check_whiteout $upperdir/origin
> +rm -rf $lowerdir/* $upperdir/*
> +

Please test the Valid "rename exchange redirect" case:
set_redirect $upperdir/testdir1 "testdir2"
set_redirect $upperdir/testdir2 "testdir1"
fsck should not remove those redirects.

> +# Test missing opaque xattr with valid redirect directory, should fix opaque

Why does fsck prefer redirect over non-redirect?
Why is this not a private case of duplicate redirect?

> +echo "+ Missing opaque"
> +mkdir $lowerdir/origin
> +mkdir $upperdir/origin
> +mkdir $upperdir/testdir
> +set_redirect $upperdir/testdir "origin"
> +_overlay_fsck_dirs -a $lowerdir $upperdir $workdir >> $seqres.full 2>&1 || \
> +       _fail "fsck should not fail"
> +check_xattr $OVL_REDIRECT_XATTR "origin" $upperdir/testdir
> +check_xattr $OVL_OPAQUE_XATTR $OVL_OPAQUE_XATTR_VAL $upperdir/origin
> +rm -rf $lowerdir/* $upperdir/*
> +
> +# Test duplicate redirect directories point to one origin, should fail in
> +# -a 'auto' mode, should remove the duplicate one in -y 'yes' mode
> +echo "+ Duplicate redirect"
> +mkdir $lowerdir/origin
> +mknod $upperdir/origin c 0 0
> +mkdir $upperdir/testdir1
> +mkdir $upperdir/testdir2
> +set_redirect $upperdir/testdir1 "origin"
> +set_redirect $upperdir/testdir2 "origin"
> +_overlay_fsck_dirs -a $lowerdir $upperdir $workdir >> $seqres.full 2>&1 && \
> +       _fail "fsck should fail"
> +_overlay_fsck_dirs -y $lowerdir $upperdir $workdir >> $seqres.full 2>&1 || \
> +       _fail "fsck should not fail"
> +check_xattr $OVL_REDIRECT_XATTR "origin" $upperdir/testdir2
> +

So what guaranty you have that fsck clears testdir1 and not testdir2?
Maybe check there is exactly one redirect left after fsck?
What does fsck do with -y to the duplicate dir anyway?
If copying from e2fsck behavior for duplicate referenced block, should
copy the content of lower dirs to upper dir and make it opaque, but that can
get nasty for a nested duplicate tree... OTOH, if user got to duplicate
redirect dir by cp -a of a redirected upper dir, then a full copy of the tree
is what user intended for.

Amir.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH for xfstests 2/4] overlay: add fsck.overlay whiteout test
  2017-12-14 13:13   ` Amir Goldstein
@ 2017-12-15  5:35     ` zhangyi (F)
  0 siblings, 0 replies; 15+ messages in thread
From: zhangyi (F) @ 2017-12-15  5:35 UTC (permalink / raw)
  To: Amir Goldstein
  Cc: overlayfs, fstests, Miklos Szeredi, Eryu Guan, Darrick J. Wong, Miao Xie

On 2017/12/14 21:13, Amir Goldstein Wrote:
> On Thu, Dec 14, 2017 at 8:48 AM, 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>
>> ---
>>  tests/overlay/201     | 98 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>  tests/overlay/201.out |  3 ++
>>  tests/overlay/group   |  1 +
>>  3 files changed, 102 insertions(+)
>>  create mode 100755 tests/overlay/201
>>  create mode 100644 tests/overlay/201.out
>>
[..]
>> +# 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"
>> +mknod $lowerdir/foo c 0 0
>> +mknod $upperdir/foo c 0 0
>> +mknod $upperdir/bar c 0 0
>> +_overlay_fsck_dirs -a $lowerdir $upperdir $workdir >> $seqres.full 2>&1 || \
> 
> Don'y you mean -p ?  ;-p
> 
Will change to use -p and elsewhere, it's easier to understand.

Thanks,
Yi.

>> +       _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
>> +mknod $upperdir/foo c 0 0
>> +mknod $lowerdir/bar c 0 0
>> +_overlay_fsck_dirs -a "$lowerdir:$lowerdir2" $upperdir $workdir >> \
>> +        $seqres.full 2>&1 || _fail "fsck should not fail"
>> +check_whiteout $upperdir/foo
>> +check_whiteout $lowerdir/bar
>> +
>> +# success, all done
>> +status=0
>> +exit
>> diff --git a/tests/overlay/201.out b/tests/overlay/201.out
>> new file mode 100644
>> index 0000000..20338db
>> --- /dev/null
>> +++ b/tests/overlay/201.out
>> @@ -0,0 +1,3 @@
>> +QA output created by 201
>> ++ Orphan whiteout
>> ++ Valid whiteout
>> 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
>> --
> 
> 
> Looks good
> 
> Amir.
> 
> .
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH for xfstests 4/4] overlay: add fsck.overlay redirect directory test
  2017-12-14 13:44   ` Amir Goldstein
@ 2017-12-15  6:42     ` zhangyi (F)
  2017-12-15  7:23       ` Amir Goldstein
  0 siblings, 1 reply; 15+ messages in thread
From: zhangyi (F) @ 2017-12-15  6:42 UTC (permalink / raw)
  To: Amir Goldstein
  Cc: overlayfs, fstests, Miklos Szeredi, Eryu Guan, Darrick J. Wong, Miao Xie

On 2017/12/14 21:44, Amir Goldstein Wrote:
> On Thu, Dec 14, 2017 at 8:48 AM, zhangyi (F) <yi.zhang@huawei.com> wrote:
>> Add fsck.overlay test case to test it how to deal with invalid/valid
>> redirect xattr in underlying directories of overlayfs. It should remove
>> the invalid redirect dir xattr automatically and keep the valid one.
>>
>> Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
>> ---
>>  tests/overlay/203     | 185 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>  tests/overlay/203.out |   8 +++
>>  tests/overlay/group   |   1 +
>>  3 files changed, 194 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..ccf1109
>> --- /dev/null
>> +++ b/tests/overlay/203
>> @@ -0,0 +1,185 @@
>> +#! /bin/bash
>> +# FS QA Test 203
>> +#
>> +# 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"
>> +
>> +# Set redirect xattr to a directory
>> +set_redirect()
>> +{
>> +       local target=$1
>> +       local value=$2
>> +
>> +       $SETFATTR_PROG -n $OVL_REDIRECT_XATTR -v $value $target
>> +}
>> +
>> +# Check xattr is correct or not
>> +check_xattr()
>> +{
>> +       local name=$1
>> +       local expect=$2
>> +       local target=$3
>> +
>> +       value=$($GETFATTR_PROG --absolute-names --only-values -n $name $target)
>> +
>> +       [[ $value == $expect ]] || echo "$name xattr incorrect"
>> +}
>> +
> 
> It would be nice if you added check_redirect() and check_opaque() mini helpers.
> 
Will do.

>> +# 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 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"
>> +mkdir $upperdir/testdir
>> +set_redirect $upperdir/testdir "abc"
>> +_overlay_fsck_dirs -a $lowerdir $upperdir $workdir >> $seqres.full 2>&1 || \
>> +       _fail "fsck should not fail"
> 
> ;-p
> 
> BTW, if you like -a, I don't mind if you keep it as well.
> e2fsck seems to accept them both.
> 
Will change.

>> +$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
>> +mkdir $upperdir/testdir
>> +set_redirect $upperdir/testdir "origin"
>> +_overlay_fsck_dirs -a $lowerdir $upperdir $workdir >> $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
>> +mkdir $upperdir/testdir
>> +set_redirect $upperdir/testdir "origin"
>> +_overlay_fsck_dirs -a $lowerdir $upperdir $workdir >> $seqres.full 2>&1 || \
>> +       _fail "fsck should not fail"
>> +check_xattr $OVL_REDIRECT_XATTR "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
>> +mkdir -p $upperdir/testdir1/testdir2
>> +set_redirect $upperdir/testdir1/testdir2 "/origin"
>> +_overlay_fsck_dirs -a $lowerdir $upperdir $workdir >> $seqres.full 2>&1 || \
>> +       _fail "fsck should not fail"
>> +check_xattr $OVL_REDIRECT_XATTR "/origin" $upperdir/testdir1/testdir2
>> +rm -rf $lowerdir/* $upperdir/*
>> +
>> +# Test missing whiteout with valid redirect directory, should fix whiteout
>> +echo "+ Missing whiteout"
>> +mkdir $lowerdir/origin
>> +mkdir $upperdir/testdir
>> +set_redirect $upperdir/testdir "origin"
>> +_overlay_fsck_dirs -a $lowerdir $upperdir $workdir >> $seqres.full 2>&1 || \
>> +       _fail "fsck should not fail"
>> +check_xattr $OVL_REDIRECT_XATTR "origin" $upperdir/testdir
>> +check_whiteout $upperdir/origin
>> +rm -rf $lowerdir/* $upperdir/*
>> +
> 
> Please test the Valid "rename exchange redirect" case:
> set_redirect $upperdir/testdir1 "testdir2"
> set_redirect $upperdir/testdir2 "testdir1"
> fsck should not remove those redirects.
> 
I can add this case. I was wondering why add this special one? If I understand right,
these two xattrs will not remove after fsck if $lowerdir/testdir1 and $lowerdir/testdir2
exists, just like the second test case "Test valid redirect xattr point to a directory
origin in the same directory".

>> +# Test missing opaque xattr with valid redirect directory, should fix opaque
> 
> Why does fsck prefer redirect over non-redirect?
> Why is this not a private case of duplicate redirect?
> 
Sorry, I not follow.
This test, I simulate the case of the following:
1) mkdir -p lower/testdir upper/testdir
2) mount overlay
3) mv merge/testdir merge/dir2   (will create an whiteout 'testdir' in upper)
4) mkdir merge/testdir   (create opaque dir)
5) umount overlay
6) remove opaque xattr of upper/testdir (missing opaque xattr lead to lower/testdir/* expose)

Am I miss something?

>> +echo "+ Missing opaque"
>> +mkdir $lowerdir/origin
>> +mkdir $upperdir/origin
>> +mkdir $upperdir/testdir
>> +set_redirect $upperdir/testdir "origin"
>> +_overlay_fsck_dirs -a $lowerdir $upperdir $workdir >> $seqres.full 2>&1 || \
>> +       _fail "fsck should not fail"
>> +check_xattr $OVL_REDIRECT_XATTR "origin" $upperdir/testdir
>> +check_xattr $OVL_OPAQUE_XATTR $OVL_OPAQUE_XATTR_VAL $upperdir/origin
>> +rm -rf $lowerdir/* $upperdir/*
>> +
>> +# Test duplicate redirect directories point to one origin, should fail in
>> +# -a 'auto' mode, should remove the duplicate one in -y 'yes' mode
>> +echo "+ Duplicate redirect"
>> +mkdir $lowerdir/origin
>> +mknod $upperdir/origin c 0 0
>> +mkdir $upperdir/testdir1
>> +mkdir $upperdir/testdir2
>> +set_redirect $upperdir/testdir1 "origin"
>> +set_redirect $upperdir/testdir2 "origin"
>> +_overlay_fsck_dirs -a $lowerdir $upperdir $workdir >> $seqres.full 2>&1 && \
>> +       _fail "fsck should fail"
>> +_overlay_fsck_dirs -y $lowerdir $upperdir $workdir >> $seqres.full 2>&1 || \
>> +       _fail "fsck should not fail"
>> +check_xattr $OVL_REDIRECT_XATTR "origin" $upperdir/testdir2
>> +
> 
> So what guaranty you have that fsck clears testdir1 and not testdir2?
> Maybe check there is exactly one redirect left after fsck?
> What does fsck do with -y to the duplicate dir anyway?
> If copying from e2fsck behavior for duplicate referenced block, should
> copy the content of lower dirs to upper dir and make it opaque, but that can
> get nasty for a nested duplicate tree... OTOH, if user got to duplicate
> redirect dir by cp -a of a redirected upper dir, then a full copy of the tree
> is what user intended for.
> 
Let me see, now, fsck will remove the *later redirect xattr in 'yes' mode,
which means that we guess user want to copy the tree in upperdir if they
call cp -a when overlayfs is offline, not the whole tree expose in merge
layer. I think if we need to copy whole tree include lower dirs maybe too
complicated.

*) fsck just remove the later one when scaning, I understand it maybe cannot guaranty
fsck will clears testdir1 and not testdir2 for some base filesystem, so need to find
better way, thoughts?

Thanks,
Yi.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH for xfstests 4/4] overlay: add fsck.overlay redirect directory test
  2017-12-15  6:42     ` zhangyi (F)
@ 2017-12-15  7:23       ` Amir Goldstein
  2017-12-15  8:57         ` zhangyi (F)
  0 siblings, 1 reply; 15+ messages in thread
From: Amir Goldstein @ 2017-12-15  7:23 UTC (permalink / raw)
  To: zhangyi (F)
  Cc: overlayfs, fstests, Miklos Szeredi, Eryu Guan, Darrick J. Wong, Miao Xie

On Fri, Dec 15, 2017 at 8:42 AM, zhangyi (F) <yi.zhang@huawei.com> wrote:
> On 2017/12/14 21:44, Amir Goldstein Wrote:
>> On Thu, Dec 14, 2017 at 8:48 AM, zhangyi (F) <yi.zhang@huawei.com> wrote:
[...]
>> Please test the Valid "rename exchange redirect" case:
>> set_redirect $upperdir/testdir1 "testdir2"
>> set_redirect $upperdir/testdir2 "testdir1"
>> fsck should not remove those redirects.
>>
> I can add this case. I was wondering why add this special one? If I understand right,
> these two xattrs will not remove after fsck if $lowerdir/testdir1 and $lowerdir/testdir2
> exists, just like the second test case "Test valid redirect xattr point to a directory
> origin in the same directory".
>

IIUC, the first version of fsck you posted wanted to make sure that
origin is covered with either a whiteout or opaque dir and I commented
that it is also legal to be covered by a redirected dir.
So the test of exchanged redirect dirs checks that this condition is handled
correctly by fsck.

>>> +# Test missing opaque xattr with valid redirect directory, should fix opaque
>>
>> Why does fsck prefer redirect over non-redirect?
>> Why is this not a private case of duplicate redirect?
>>
> Sorry, I not follow.
> This test, I simulate the case of the following:
> 1) mkdir -p lower/testdir upper/testdir
> 2) mount overlay
> 3) mv merge/testdir merge/dir2   (will create an whiteout 'testdir' in upper)
> 4) mkdir merge/testdir   (create opaque dir)
> 5) umount overlay
> 6) remove opaque xattr of upper/testdir (missing opaque xattr lead to lower/testdir/* expose)
>
> Am I miss something?

That is one way of getting to that inconsistency which suggests that the correct
and automatic fix is to set opaque on uper/testdir, but what about:

1) extract pre-defined image to lower/testdir upper/testdir
2) mount overlay
3) mv merge/testdir merge/dir2   (will create an whiteout 'testdir' in upper)
4) mkdir merge/testdir   (create opaque dir)
5) umount overlay
6) re-extract pre-defined image to lower/testdir upper/testdir (*)

(*) Anything that *can* happen *will* happen...

Note that prior to redirect_dir feature, use could even get a way with this
partial restore/undo of testdir changes.

My point is that if you have 2 dirs pointing at the same lower, either
because they are both redirect or because one is not opaque, it is
not possible to automatically determine which should be the merge dir
and need to ask user to resolve the conflict.

>
>>> +echo "+ Missing opaque"
>>> +mkdir $lowerdir/origin
>>> +mkdir $upperdir/origin
>>> +mkdir $upperdir/testdir
>>> +set_redirect $upperdir/testdir "origin"
>>> +_overlay_fsck_dirs -a $lowerdir $upperdir $workdir >> $seqres.full 2>&1 || \
>>> +       _fail "fsck should not fail"
>>> +check_xattr $OVL_REDIRECT_XATTR "origin" $upperdir/testdir
>>> +check_xattr $OVL_OPAQUE_XATTR $OVL_OPAQUE_XATTR_VAL $upperdir/origin
>>> +rm -rf $lowerdir/* $upperdir/*
>>> +
>>> +# Test duplicate redirect directories point to one origin, should fail in
>>> +# -a 'auto' mode, should remove the duplicate one in -y 'yes' mode
>>> +echo "+ Duplicate redirect"
>>> +mkdir $lowerdir/origin
>>> +mknod $upperdir/origin c 0 0
>>> +mkdir $upperdir/testdir1
>>> +mkdir $upperdir/testdir2
>>> +set_redirect $upperdir/testdir1 "origin"
>>> +set_redirect $upperdir/testdir2 "origin"
>>> +_overlay_fsck_dirs -a $lowerdir $upperdir $workdir >> $seqres.full 2>&1 && \
>>> +       _fail "fsck should fail"
>>> +_overlay_fsck_dirs -y $lowerdir $upperdir $workdir >> $seqres.full 2>&1 || \
>>> +       _fail "fsck should not fail"
>>> +check_xattr $OVL_REDIRECT_XATTR "origin" $upperdir/testdir2
>>> +
>>
>> So what guaranty you have that fsck clears testdir1 and not testdir2?
>> Maybe check there is exactly one redirect left after fsck?
>> What does fsck do with -y to the duplicate dir anyway?
>> If copying from e2fsck behavior for duplicate referenced block, should
>> copy the content of lower dirs to upper dir and make it opaque, but that can
>> get nasty for a nested duplicate tree... OTOH, if user got to duplicate
>> redirect dir by cp -a of a redirected upper dir, then a full copy of the tree
>> is what user intended for.
>>
> Let me see, now, fsck will remove the *later redirect xattr in 'yes' mode,
> which means that we guess user want to copy the tree in upperdir if they
> call cp -a when overlayfs is offline, not the whole tree expose in merge
> layer. I think if we need to copy whole tree include lower dirs maybe too
> complicated.
>
> *) fsck just remove the later one when scaning, I understand it maybe cannot guaranty
> fsck will clears testdir1 and not testdir2 for some base filesystem, so need to find
> better way, thoughts?
>

quite simple I think. Instead of verifying that redirect was removed
from testdir1
and remained on testdir2, verify that redirect from removed from either and
left on exactly one of them. Less trivial scripting, but not impossible.

Amir.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH for xfstests 4/4] overlay: add fsck.overlay redirect directory test
  2017-12-15  7:23       ` Amir Goldstein
@ 2017-12-15  8:57         ` zhangyi (F)
  0 siblings, 0 replies; 15+ messages in thread
From: zhangyi (F) @ 2017-12-15  8:57 UTC (permalink / raw)
  To: Amir Goldstein
  Cc: overlayfs, fstests, Miklos Szeredi, Eryu Guan, Darrick J. Wong, Miao Xie

On 2017/12/15 15:23, Amir Goldstein Wrote:
> On Fri, Dec 15, 2017 at 8:42 AM, zhangyi (F) <yi.zhang@huawei.com> wrote:
>> On 2017/12/14 21:44, Amir Goldstein Wrote:
>>> On Thu, Dec 14, 2017 at 8:48 AM, zhangyi (F) <yi.zhang@huawei.com> wrote:
> [...]
>>> Please test the Valid "rename exchange redirect" case:
>>> set_redirect $upperdir/testdir1 "testdir2"
>>> set_redirect $upperdir/testdir2 "testdir1"
>>> fsck should not remove those redirects.
>>>
>> I can add this case. I was wondering why add this special one? If I understand right,
>> these two xattrs will not remove after fsck if $lowerdir/testdir1 and $lowerdir/testdir2
>> exists, just like the second test case "Test valid redirect xattr point to a directory
>> origin in the same directory".
>>
> 
> IIUC, the first version of fsck you posted wanted to make sure that
> origin is covered with either a whiteout or opaque dir and I commented
> that it is also legal to be covered by a redirected dir.
> So the test of exchanged redirect dirs checks that this condition is handled
> correctly by fsck.
> 
Understand, will add.

[..]
> That is one way of getting to that inconsistency which suggests that the correct
> and automatic fix is to set opaque on uper/testdir, but what about:
> 
> 1) extract pre-defined image to lower/testdir upper/testdir
> 2) mount overlay
> 3) mv merge/testdir merge/dir2   (will create an whiteout 'testdir' in upper)
> 4) mkdir merge/testdir   (create opaque dir)
> 5) umount overlay
> 6) re-extract pre-defined image to lower/testdir upper/testdir (*)
> 
> (*) Anything that *can* happen *will* happen...
> 
> Note that prior to redirect_dir feature, use could even get a way with this
> partial restore/undo of testdir changes.
> 
> My point is that if you have 2 dirs pointing at the same lower, either
> because they are both redirect or because one is not opaque, it is
> not possible to automatically determine which should be the merge dir
> and need to ask user to resolve the conflict.
> 
I think I follow you now, this does verly likly to happen. Will handle this
case to ask user in default mode, do nothing in auto and yes mode.

[..]
>> Let me see, now, fsck will remove the *later redirect xattr in 'yes' mode,
>> which means that we guess user want to copy the tree in upperdir if they
>> call cp -a when overlayfs is offline, not the whole tree expose in merge
>> layer. I think if we need to copy whole tree include lower dirs maybe too
>> complicated.
>>
>> *) fsck just remove the later one when scaning, I understand it maybe cannot guaranty
>> fsck will clears testdir1 and not testdir2 for some base filesystem, so need to find
>> better way, thoughts?
>>
> 
> quite simple I think. Instead of verifying that redirect was removed
> from testdir1
> and remained on testdir2, verify that redirect from removed from either and
> left on exactly one of them. Less trivial scripting, but not impossible.
> 
Yes, will do.

Thanks a lot,
Yi

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2017-12-15  8:57 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-14  6:48 [PATCH for xfstests 0/4] overlay: add fsck.overlay basic tests zhangyi (F)
2017-12-14  6:48 ` [PATCH for xfstests 1/4] overlay: add filesystem check helper zhangyi (F)
2017-12-14  9:05   ` Amir Goldstein
2017-12-14 12:40     ` zhangyi (F)
2017-12-14 13:14       ` Amir Goldstein
2017-12-14  6:48 ` [PATCH for xfstests 2/4] overlay: add fsck.overlay whiteout test zhangyi (F)
2017-12-14 13:13   ` Amir Goldstein
2017-12-15  5:35     ` zhangyi (F)
2017-12-14  6:48 ` [PATCH for xfstests 3/4] overlay: add fsck.overlay opaque directory test zhangyi (F)
2017-12-14 13:25   ` Amir Goldstein
2017-12-14  6:48 ` [PATCH for xfstests 4/4] overlay: add fsck.overlay redirect " zhangyi (F)
2017-12-14 13:44   ` Amir Goldstein
2017-12-15  6:42     ` zhangyi (F)
2017-12-15  7:23       ` Amir Goldstein
2017-12-15  8:57         ` zhangyi (F)

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.