[xfstests PATCH 0/3] overlay: enhance fsck.overlay test cases

[xfstests PATCH 0/3] overlay: enhance fsck.overlay test cases

[zhangyi (F)]

Hi All,

These are fsck.overlay program related patch set. The first patch correct
the exit code of fsck.overlay in the existing 3 test cases, the second
patch introduce a stress case to test stability, and the final patch add
some exception cases to test exception handling process of fsck.overlay.

The fsck.overlay program also was undate at:

https://github.com/hisilicon/overlayfs-progs/tree/next

All these three patches have been tested with the new fsck.overlay,
please review.

Thanks!
Yi.

zhangyi (F) (3):
  overlay: correct fsck.overlay exit code
  overlay: add fsck.overlay stress test
  overlay: add fsck.overlay exception tests

 common/config         |  10 +++
 tests/overlay/045     |  59 ++++++++-----
 tests/overlay/046     |  62 ++++++++-----
 tests/overlay/056     |  22 +++--
 tests/overlay/061     |  81 +++++++++++++++++
 tests/overlay/061.out |   2 +
 tests/overlay/062     | 236 ++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/overlay/062.out |  10 +++
 tests/overlay/group   |   2 +
 9 files changed, 433 insertions(+), 51 deletions(-)
 create mode 100755 tests/overlay/061
 create mode 100644 tests/overlay/061.out
 create mode 100755 tests/overlay/062
 create mode 100644 tests/overlay/062.out

-- 
2.5.0

[xfstests PATCH 1/3] overlay: correct fsck.overlay exit code

[zhangyi (F)]

fsck.overlay should return correct exit code to show the file system
status after fsck, instead of return 0 means consistency and !0 means
inconsistency or something bad happened.

Fix the following three exit code after running fsck.overlay:

- Return FSCK_OK if the input file system is consistent,
- Return FSCK_NONDESTRUCT if the file system inconsistent errors
  corrected,
- Return FSCK_UNCORRECTED if the file system still have inconsistent
  errors.

This patch also correct the input underlying dirs for some "valid" test
cases, avoid return unexpected exit code which caused by other unrelated
inconsistency.

Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
---
 common/config     | 10 +++++++++
 tests/overlay/045 | 59 +++++++++++++++++++++++++++++++++-------------------
 tests/overlay/046 | 62 ++++++++++++++++++++++++++++++++++---------------------
 tests/overlay/056 | 22 ++++++++++++++------
 4 files changed, 102 insertions(+), 51 deletions(-)

diff --git a/common/config b/common/config
index 2f1f272..6e83fca 100644
--- a/common/config
+++ b/common/config
@@ -194,6 +194,16 @@ export CHECKBASHISMS_PROG="$(type -P checkbashisms)"
 export XFS_INFO_PROG="$(type -P xfs_info)"
 export DUPEREMOVE_PROG="$(type -P duperemove)"
 
+# Export exit code used by fsck-type programs
+export FSCK_OK=0
+export FSCK_NONDESTRUCT=1
+export FSCK_REBOOT=2
+export FSCK_UNCORRECTED=4
+export FSCK_ERROR=8
+export FSCK_USAGE=16
+export FSCK_CANCELED=32
+export FSCK_LIBRARY=128
+
 # use 'udevadm settle' or 'udevsettle' to wait for lv to be settled.
 # newer systems have udevadm command but older systems like RHEL5 don't.
 # But if neither one is available, just set it to "sleep 1" to wait for lv to
diff --git a/tests/overlay/045 b/tests/overlay/045
index acc7087..04db626 100755
--- a/tests/overlay/045
+++ b/tests/overlay/045
@@ -37,6 +37,7 @@ _require_attrs
 _require_command "$FSCK_OVERLAY_PROG" fsck.overlay
 
 OVL_XATTR_OPAQUE_VAL=y
+OVL_XATTR_IMPURE_VAL=y
 
 # remove all files from previous tests
 _scratch_mkfs
@@ -69,6 +70,15 @@ make_opaque_dir()
 	$SETFATTR_PROG -n $OVL_XATTR_OPAQUE -v $OVL_XATTR_OPAQUE_VAL $target
 }
 
+# Create impure directories
+make_impure_dir()
+{
+	for dir in $*; do
+		mkdir -p $dir
+		$SETFATTR_PROG -n $OVL_XATTR_IMPURE -v $OVL_XATTR_IMPURE_VAL $dir
+	done
+}
+
 # Create a redirect directory
 make_redirect_dir()
 {
@@ -79,6 +89,19 @@ make_redirect_dir()
 	$SETFATTR_PROG -n $OVL_XATTR_REDIRECT -v $value $target
 }
 
+# Run fsck.overlay and check return value
+run_fsck()
+{
+	local expect=$1
+	shift 1
+
+	_overlay_fsck_dirs $* >> $seqres.full 2>&1
+	fsck_ret=$?
+
+	[[ "$fsck_ret" == "$expect" ]] || \
+		echo "fsck return unexpected:$expect,$fsck_ret"
+}
+
 # Create test directories
 lowerdir=$OVL_BASE_SCRATCH_MNT/lower
 lowerdir2=$OVL_BASE_SCRATCH_MNT/lower2
@@ -96,8 +119,7 @@ echo "+ Orphan whiteout"
 make_test_dirs
 make_whiteout $lowerdir/foo $upperdir/{foo,bar}
 
-_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
-	echo "fsck should not fail"
+run_fsck $FSCK_NONDESTRUCT $lowerdir $upperdir $workdir -p
 ls $lowerdir
 ls $upperdir
 
@@ -107,8 +129,7 @@ make_test_dirs
 touch $lowerdir2/{foo,bar}
 make_whiteout $upperdir/foo $lowerdir/bar
 
-_overlay_fsck_dirs "$lowerdir:$lowerdir2" $upperdir $workdir -p >> \
-	 $seqres.full 2>&1 || echo "fsck should not fail"
+run_fsck $FSCK_OK "$lowerdir:$lowerdir2" $upperdir $workdir -p
 check_whiteout $upperdir/foo $lowerdir/bar
 
 # Test orphan whiteout in opaque directory, should remove
@@ -119,8 +140,7 @@ 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 || \
-	echo "fsck should not fail"
+run_fsck $FSCK_NONDESTRUCT $lowerdir $upperdir $workdir -p
 ls $upperdir/testdir
 
 # Test orphan whiteout whose parent path is not an merged directory,
@@ -135,8 +155,7 @@ make_whiteout $lowerdir/testdir2
 make_opaque_dir $lowerdir/testdir3
 make_whiteout $upperdir/{testdir1/foo,/testdir2/foo,testdir3/foo,testdir4/foo}
 
-_overlay_fsck_dirs "$lowerdir:$lowerdir2" $upperdir $workdir -p >> \
-	$seqres.full 2>&1 || echo "fsck should not fail"
+run_fsck $FSCK_NONDESTRUCT "$lowerdir:$lowerdir2" $upperdir $workdir -p
 ls $upperdir/testdir1
 ls $upperdir/testdir2
 ls $upperdir/testdir3
@@ -150,8 +169,7 @@ 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 || \
-	echo "fsck should not fail"
+run_fsck $FSCK_NONDESTRUCT $lowerdir $upperdir $workdir -p
 ls $upperdir/testdir
 
 # Test valid whiteout in redirect directory cover file in lower
@@ -160,11 +178,11 @@ echo "+ Valid whiteout(2)"
 make_test_dirs
 mkdir $lowerdir/origin
 touch $lowerdir/origin/foo
+make_impure_dir $upperdir
 make_redirect_dir $upperdir/testdir "origin"
-make_whiteout $upperdir/testdir/foo
+make_whiteout $upperdir/origin $upperdir/testdir/foo
 
-_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
-	echo "fsck should not fail"
+run_fsck $FSCK_OK $lowerdir $upperdir $workdir -p
 check_whiteout $upperdir/testdir/foo
 
 # Test valid whiteout covering lower target whose parent directory
@@ -175,10 +193,10 @@ 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
+make_whiteout $lowerdir/origin $upperdir/testdir/subdir/foo
+make_impure_dir $upperdir/testdir $upperdir
 
-_overlay_fsck_dirs "$lowerdir:$lowerdir2" $upperdir $workdir -p \
-	>> $seqres.full 2>&1 || echo "fsck should not fail"
+run_fsck $FSCK_OK "$lowerdir:$lowerdir2" $upperdir $workdir -p
 check_whiteout $upperdir/testdir/subdir/foo
 
 # Test invalid whiteout in opaque subdirectory in a redirect directory,
@@ -191,8 +209,7 @@ 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 || \
-	echo "fsck should not fail"
+run_fsck $FSCK_NONDESTRUCT $lowerdir $upperdir $workdir -p
 ls $upperdir/testdir/subdir
 
 # Test valid whiteout in reidrect subdirectory in a opaque directory
@@ -203,10 +220,10 @@ 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
+make_whiteout $upperdir/origin $upperdir/testdir/subdir/foo
+make_impure_dir $upperdir/testdir
 
-_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
-        echo "fsck should not fail"
+run_fsck $FSCK_OK $lowerdir $upperdir $workdir -p
 check_whiteout $upperdir/testdir/subdir/foo
 
 # success, all done
diff --git a/tests/overlay/046 b/tests/overlay/046
index 1c09609..a87aaa6 100755
--- a/tests/overlay/046
+++ b/tests/overlay/046
@@ -40,6 +40,16 @@ _require_command "$FSCK_OVERLAY_PROG" fsck.overlay
 _scratch_mkfs
 
 OVL_XATTR_OPAQUE_VAL=y
+OVL_XATTR_IMPURE_VAL=y
+
+# Create impure directories
+make_impure_dir()
+{
+	for dir in $*; do
+		mkdir -p $dir
+		$SETFATTR_PROG -n $OVL_XATTR_IMPURE -v $OVL_XATTR_IMPURE_VAL $dir
+	done
+}
 
 # Create a redirect directory
 make_redirect_dir()
@@ -104,6 +114,19 @@ check_whiteout()
 	done
 }
 
+# Run fsck.overlay and check return value
+run_fsck()
+{
+	local expect=$1
+	shift 1
+
+	_overlay_fsck_dirs $* >> $seqres.full 2>&1
+	fsck_ret=$?
+
+	[[ "$fsck_ret" == "$expect" ]] || \
+		echo "fsck return unexpected:$expect,$fsck_ret"
+}
+
 # Create test directories
 lowerdir=$OVL_BASE_SCRATCH_MNT/lower
 lowerdir2=$OVL_BASE_SCRATCH_MNT/lower2
@@ -121,8 +144,7 @@ echo "+ Invalid redirect"
 make_test_dirs
 make_redirect_dir $upperdir/testdir "invalid"
 
-_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
-	echo "fsck should not fail"
+run_fsck $FSCK_NONDESTRUCT $lowerdir $upperdir $workdir -p
 check_no_redirect $upperdir/testdir
 
 # Test invalid redirect xattr point to a file origin, should remove
@@ -131,8 +153,7 @@ make_test_dirs
 touch $lowerdir/origin
 make_redirect_dir $upperdir/testdir "origin"
 
-_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
-	echo "fsck should not fail"
+run_fsck $FSCK_NONDESTRUCT $lowerdir $upperdir $workdir -p
 check_no_redirect $upperdir/testdir
 
 # Test valid redirect xattr point to a directory origin in the same directory,
@@ -142,9 +163,9 @@ make_test_dirs
 mkdir $lowerdir/origin
 make_whiteout $upperdir/origin
 make_redirect_dir $upperdir/testdir "origin"
+make_impure_dir $upperdir
 
-_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
-	echo "fsck should not fail"
+run_fsck $FSCK_OK $lowerdir $upperdir $workdir -p
 check_redirect $upperdir/testdir "origin"
 
 # Test valid redirect xattr point to a directory origin in different directories
@@ -154,9 +175,9 @@ make_test_dirs
 mkdir $lowerdir/origin
 make_whiteout $upperdir/origin
 make_redirect_dir $upperdir/testdir1/testdir2 "/origin"
+make_impure_dir $upperdir/testdir1
 
-_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
-	echo "fsck should not fail"
+run_fsck $FSCK_OK $lowerdir $upperdir $workdir -p
 check_redirect $upperdir/testdir1/testdir2 "/origin"
 
 # Test valid redirect xattr but missing whiteout to cover lower target,
@@ -166,8 +187,7 @@ make_test_dirs
 mkdir $lowerdir/origin
 make_redirect_dir $upperdir/testdir "origin"
 
-_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
-	echo "fsck should not fail"
+run_fsck $FSCK_NONDESTRUCT $lowerdir $upperdir $workdir -p
 check_redirect $upperdir/testdir "origin"
 check_whiteout $upperdir/origin
 
@@ -177,9 +197,9 @@ make_test_dirs
 mkdir $lowerdir/{testdir1,testdir2}
 make_redirect_dir $upperdir/testdir1 "testdir2"
 make_redirect_dir $upperdir/testdir2 "testdir1"
+make_impure_dir $upperdir
 
-_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
-	echo "fsck should not fail"
+run_fsck $FSCK_OK $lowerdir $upperdir $workdir -p
 check_redirect $upperdir/testdir1 "testdir2"
 check_redirect $upperdir/testdir2 "testdir1"
 
@@ -191,8 +211,7 @@ mkdir $lowerdir/testdir
 make_redirect_dir $upperdir/testdir "invalid"
 
 # Question get yes answer: Should set opaque dir ?
-_overlay_fsck_dirs $lowerdir $upperdir $workdir -y >> $seqres.full 2>&1 || \
-	echo "fsck should not fail"
+run_fsck $FSCK_NONDESTRUCT $lowerdir $upperdir $workdir -y
 check_no_redirect $upperdir/testdir
 check_opaque $upperdir/testdir
 
@@ -205,12 +224,10 @@ make_redirect_dir $lowerdir/testdir1 "origin"
 make_redirect_dir $lowerdir/testdir2 "origin"
 make_redirect_dir $upperdir/testdir3 "origin"
 
-_overlay_fsck_dirs "$lowerdir:$lowerdir2" $upperdir $workdir -p >> \
-	$seqres.full 2>&1 && echo "fsck should fail"
+run_fsck $FSCK_UNCORRECTED "$lowerdir:$lowerdir2" $upperdir $workdir -p
 
 # Question get yes answer: Duplicate redirect directory, remove xattr ?
-_overlay_fsck_dirs "$lowerdir:$lowerdir2" $upperdir $workdir -y >> \
-	$seqres.full 2>&1 || echo "fsck should not fail"
+run_fsck $FSCK_NONDESTRUCT "$lowerdir:$lowerdir2" $upperdir $workdir -y
 redirect_1=`check_redirect $lowerdir/testdir1 "origin" 2>/dev/null`
 redirect_2=`check_redirect $lowerdir/testdir2 "origin" 2>/dev/null`
 [[ $redirect_1 == $redirect_2 ]] && echo "Redirect xattr incorrect"
@@ -223,12 +240,10 @@ make_test_dirs
 mkdir $lowerdir/origin $upperdir/origin
 make_redirect_dir $upperdir/testdir "origin"
 
-_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 && \
-	echo "fsck should fail"
+run_fsck $FSCK_UNCORRECTED $lowerdir $upperdir $workdir -p
 
 # Question get yes answer: Duplicate redirect directory, remove xattr ?
-_overlay_fsck_dirs $lowerdir $upperdir $workdir -y >> $seqres.full 2>&1 || \
-	echo "fsck should not fail"
+run_fsck $FSCK_NONDESTRUCT $lowerdir $upperdir $workdir -y
 check_no_redirect $upperdir/testdir
 
 # Test duplicate redirect xattr with lower same name directory exists,
@@ -240,8 +255,7 @@ make_redirect_dir $upperdir/testdir "invalid"
 
 # Question one get yes answer: Duplicate redirect directory, remove xattr?
 # Question two get yes answer: Should set opaque dir ?
-_overlay_fsck_dirs $lowerdir $upperdir $workdir -y >> $seqres.full 2>&1 || \
-	echo "fsck should not fail"
+run_fsck $FSCK_NONDESTRUCT $lowerdir $upperdir $workdir -y
 check_no_redirect $upperdir/testdir
 check_opaque $upperdir/testdir
 
diff --git a/tests/overlay/056 b/tests/overlay/056
index 778e1f6..9fca632 100755
--- a/tests/overlay/056
+++ b/tests/overlay/056
@@ -70,6 +70,19 @@ check_impure()
 	[[ "$value" == "$OVL_XATTR_IMPURE_VAL" ]] || echo "Missing impure xattr"
 }
 
+# Run fsck.overlay and check return value
+run_fsck()
+{
+	local expect=$1
+	shift 1
+
+	_overlay_fsck_dirs $* >> $seqres.full 2>&1
+	fsck_ret=$?
+
+	[[ "$fsck_ret" == "$expect" ]] || \
+		echo "fsck return unexpected:$expect,$fsck_ret"
+}
+
 # Create test directories
 lowerdir=$OVL_BASE_SCRATCH_MNT/lower
 lowerdir2=$OVL_BASE_SCRATCH_MNT/lower2
@@ -96,8 +109,7 @@ $UMOUNT_PROG $SCRATCH_MNT
 remove_impure $upperdir/testdir1
 remove_impure $upperdir/testdir2
 
-_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
-	echo "fsck should not fail"
+run_fsck $FSCK_NONDESTRUCT $lowerdir $upperdir $workdir -p
 check_impure $upperdir/testdir1
 check_impure $upperdir/testdir2
 
@@ -108,8 +120,7 @@ make_test_dirs
 mkdir $lowerdir/origin
 make_redirect_dir $upperdir/testdir/subdir "/origin"
 
-_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
-	echo "fsck should not fail"
+run_fsck $FSCK_NONDESTRUCT $lowerdir $upperdir $workdir -p
 check_impure $upperdir/testdir
 
 # Test missing impure xattr in directory which has merge directories,
@@ -118,8 +129,7 @@ echo "+ Missing impure(3)"
 make_test_dirs
 mkdir $lowerdir/testdir $upperdir/testdir
 
-_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
-	echo "fsck should not fail"
+run_fsck $FSCK_NONDESTRUCT $lowerdir $upperdir $workdir -p
 check_impure $upperdir
 
 # success, all done
-- 
2.5.0

Re: [xfstests PATCH 1/3] overlay: correct fsck.overlay exit code

[Amir Goldstein]

On Sat, Jul 28, 2018 at 11:42 AM, zhangyi (F) <yi.zhang@huawei.com> wrote:
> fsck.overlay should return correct exit code to show the file system
> status after fsck, instead of return 0 means consistency and !0 means
> inconsistency or something bad happened.
>
> Fix the following three exit code after running fsck.overlay:
>
> - Return FSCK_OK if the input file system is consistent,
> - Return FSCK_NONDESTRUCT if the file system inconsistent errors
>   corrected,
> - Return FSCK_UNCORRECTED if the file system still have inconsistent
>   errors.
>
> This patch also correct the input underlying dirs for some "valid" test
> cases, avoid return unexpected exit code which caused by other unrelated
> inconsistency.

... and also adds more test coverage (impure dir) ..
too many changes at once.
Please separate the exit code change from the rest of the changes.

Usually, Eryu doesn't like adding coverage to existing test, but since
there are "not so many" testers of fsck.overlay, maybe he can make an
exception, but you do need to declare this change in commit message.

>
> Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
> ---
>  common/config     | 10 +++++++++
>  tests/overlay/045 | 59 +++++++++++++++++++++++++++++++++-------------------
>  tests/overlay/046 | 62 ++++++++++++++++++++++++++++++++++---------------------
>  tests/overlay/056 | 22 ++++++++++++++------
>  4 files changed, 102 insertions(+), 51 deletions(-)
>
> diff --git a/common/config b/common/config
> index 2f1f272..6e83fca 100644
> --- a/common/config
> +++ b/common/config
> @@ -194,6 +194,16 @@ export CHECKBASHISMS_PROG="$(type -P checkbashisms)"
>  export XFS_INFO_PROG="$(type -P xfs_info)"
>  export DUPEREMOVE_PROG="$(type -P duperemove)"
>
> +# Export exit code used by fsck-type programs
> +export FSCK_OK=0
> +export FSCK_NONDESTRUCT=1
> +export FSCK_REBOOT=2
> +export FSCK_UNCORRECTED=4
> +export FSCK_ERROR=8
> +export FSCK_USAGE=16
> +export FSCK_CANCELED=32
> +export FSCK_LIBRARY=128
> +

While this is common to e2fsck, I don't think it is "common" enough,
so maybe keep those values in a less central file (i.e. common/fsck)
and name e2fsprogs and fsck.overlayfs (other?) specifically.

>  # use 'udevadm settle' or 'udevsettle' to wait for lv to be settled.
>  # newer systems have udevadm command but older systems like RHEL5 don't.
>  # But if neither one is available, just set it to "sleep 1" to wait for lv to
> diff --git a/tests/overlay/045 b/tests/overlay/045
> index acc7087..04db626 100755
> --- a/tests/overlay/045
> +++ b/tests/overlay/045
> @@ -37,6 +37,7 @@ _require_attrs
>  _require_command "$FSCK_OVERLAY_PROG" fsck.overlay
>
>  OVL_XATTR_OPAQUE_VAL=y
> +OVL_XATTR_IMPURE_VAL=y
>
>  # remove all files from previous tests
>  _scratch_mkfs
> @@ -69,6 +70,15 @@ make_opaque_dir()
>         $SETFATTR_PROG -n $OVL_XATTR_OPAQUE -v $OVL_XATTR_OPAQUE_VAL $target
>  }
>
> +# Create impure directories
> +make_impure_dir()
> +{
> +       for dir in $*; do
> +               mkdir -p $dir
> +               $SETFATTR_PROG -n $OVL_XATTR_IMPURE -v $OVL_XATTR_IMPURE_VAL $dir
> +       done
> +}
> +
>  # Create a redirect directory
>  make_redirect_dir()
>  {
> @@ -79,6 +89,19 @@ make_redirect_dir()
>         $SETFATTR_PROG -n $OVL_XATTR_REDIRECT -v $value $target
>  }
>
> +# Run fsck.overlay and check return value
> +run_fsck()
> +{
> +       local expect=$1
> +       shift 1
> +
> +       _overlay_fsck_dirs $* >> $seqres.full 2>&1
> +       fsck_ret=$?
> +
> +       [[ "$fsck_ret" == "$expect" ]] || \
> +               echo "fsck return unexpected:$expect,$fsck_ret"
> +}
> +

This helper repeats in every overlay/fsck test.
Move it to be a common function?

Thanks,
Amir.

Re: [xfstests PATCH 1/3] overlay: correct fsck.overlay exit code

[zhangyi (F)]

On 2018/7/30 13:38, Amir Goldstein Wrote:
> On Sat, Jul 28, 2018 at 11:42 AM, zhangyi (F) <yi.zhang@huawei.com> wrote:
>> fsck.overlay should return correct exit code to show the file system
>> status after fsck, instead of return 0 means consistency and !0 means
>> inconsistency or something bad happened.
>>
>> Fix the following three exit code after running fsck.overlay:
>>
>> - Return FSCK_OK if the input file system is consistent,
>> - Return FSCK_NONDESTRUCT if the file system inconsistent errors
>>   corrected,
>> - Return FSCK_UNCORRECTED if the file system still have inconsistent
>>   errors.
>>
>> This patch also correct the input underlying dirs for some "valid" test
>> cases, avoid return unexpected exit code which caused by other unrelated
>> inconsistency.
> 
> ... and also adds more test coverage (impure dir) ..
> too many changes at once.
> Please separate the exit code change from the rest of the changes.
> 

This patch doesn't add impure dir test coverage, it just set impure xattr
to the parent dir of similuated redirect dir to prevent fsck.overlay return
FSCK_NONDESTRUCT instead of FSCK_OK, because the previous "valid" test
case is not "valid" enough, it lose the impure xattr which will be fixed
by fsck.overlay (that is not the case want to cover).

I can still separate it from the exit code change if you want.

> Usually, Eryu doesn't like adding coverage to existing test, but since
> there are "not so many" testers of fsck.overlay, maybe he can make an
> exception, but you do need to declare this change in commit message.
> 
>>
>> Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
>> ---
>>  common/config     | 10 +++++++++
>>  tests/overlay/045 | 59 +++++++++++++++++++++++++++++++++-------------------
>>  tests/overlay/046 | 62 ++++++++++++++++++++++++++++++++++---------------------
>>  tests/overlay/056 | 22 ++++++++++++++------
>>  4 files changed, 102 insertions(+), 51 deletions(-)
>>
>> diff --git a/common/config b/common/config
>> index 2f1f272..6e83fca 100644
>> --- a/common/config
>> +++ b/common/config
>> @@ -194,6 +194,16 @@ export CHECKBASHISMS_PROG="$(type -P checkbashisms)"
>>  export XFS_INFO_PROG="$(type -P xfs_info)"
>>  export DUPEREMOVE_PROG="$(type -P duperemove)"
>>
>> +# Export exit code used by fsck-type programs
>> +export FSCK_OK=0
>> +export FSCK_NONDESTRUCT=1
>> +export FSCK_REBOOT=2
>> +export FSCK_UNCORRECTED=4
>> +export FSCK_ERROR=8
>> +export FSCK_USAGE=16
>> +export FSCK_CANCELED=32
>> +export FSCK_LIBRARY=128
>> +
> 
> While this is common to e2fsck, I don't think it is "common" enough,
> so maybe keep those values in a less central file (i.e. common/fsck)
> and name e2fsprogs and fsck.overlayfs (other?) specifically.
> 

IIUC, current e2fsprogs related codes in xfstests do not use these exit
code at all. Keep these into common/overlay now and put them into
common/fsck if e2fsprogs want to use it the future ?

>>  # use 'udevadm settle' or 'udevsettle' to wait for lv to be settled.
>>  # newer systems have udevadm command but older systems like RHEL5 don't.
>>  # But if neither one is available, just set it to "sleep 1" to wait for lv to
>> diff --git a/tests/overlay/045 b/tests/overlay/045
>> index acc7087..04db626 100755
>> --- a/tests/overlay/045
>> +++ b/tests/overlay/045
>> @@ -37,6 +37,7 @@ _require_attrs
>>  _require_command "$FSCK_OVERLAY_PROG" fsck.overlay
>>
>>  OVL_XATTR_OPAQUE_VAL=y
>> +OVL_XATTR_IMPURE_VAL=y
>>
>>  # remove all files from previous tests
>>  _scratch_mkfs
>> @@ -69,6 +70,15 @@ make_opaque_dir()
>>         $SETFATTR_PROG -n $OVL_XATTR_OPAQUE -v $OVL_XATTR_OPAQUE_VAL $target
>>  }
>>
>> +# Create impure directories
>> +make_impure_dir()
>> +{
>> +       for dir in $*; do
>> +               mkdir -p $dir
>> +               $SETFATTR_PROG -n $OVL_XATTR_IMPURE -v $OVL_XATTR_IMPURE_VAL $dir
>> +       done
>> +}
>> +
>>  # Create a redirect directory
>>  make_redirect_dir()
>>  {
>> @@ -79,6 +89,19 @@ make_redirect_dir()
>>         $SETFATTR_PROG -n $OVL_XATTR_REDIRECT -v $value $target
>>  }
>>
>> +# Run fsck.overlay and check return value
>> +run_fsck()
>> +{
>> +       local expect=$1
>> +       shift 1
>> +
>> +       _overlay_fsck_dirs $* >> $seqres.full 2>&1
>> +       fsck_ret=$?
>> +
>> +       [[ "$fsck_ret" == "$expect" ]] || \
>> +               echo "fsck return unexpected:$expect,$fsck_ret"
>> +}
>> +
> 
> This helper repeats in every overlay/fsck test.
> Move it to be a common function?
> 
Will do.

Thanks,
Yi.

[xfstests PATCH 2/3] overlay: add fsck.overlay stress test

[zhangyi (F)]

Introduce a test case for fsck.overlay which runs on the underlying
directories created by fsstress (contain a lot of fs objects) to
find potential stability issue.

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

diff --git a/tests/overlay/061 b/tests/overlay/061
new file mode 100755
index 0000000..b66b36a
--- /dev/null
+++ b/tests/overlay/061
@@ -0,0 +1,81 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2018 Huawei.  All Rights Reserved.
+#
+# FS QA Test No. 061
+#
+# Stress test: test fsck.overlay running on the underlying dirs
+# which were created by fsstress.
+#
+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_nocheck
+_require_command "$FSCK_OVERLAY_PROG" fsck.overlay
+
+# remove all files from previous tests
+_scratch_mkfs
+
+# Create an underlying layer which contain a lot of random objects
+create_layer()
+{
+	for dir in $*; do
+		$FSSTRESS_PROG -s 42 -d $dir -z \
+			-f creat=20 -f link=10 -f mkdir=20 -f mknod=10 \
+			-f rename=10 -f setxattr=10 -f symlink=10 -f write=10 \
+			-p 4 -n 500 -l50 > /dev/null 2>&1 &
+	done
+}
+
+
+# Create test directories
+lowerdir=$OVL_BASE_SCRATCH_MNT/$seq-ovl-lower
+lowerdir2=$OVL_BASE_SCRATCH_MNT/$seq-ovl-lower2
+upperdir=$OVL_BASE_SCRATCH_MNT/$seq-ovl-upper
+workdir=$OVL_BASE_SCRATCH_MNT/$seq-ovl-workdir
+
+make_test_dirs()
+{
+	rm -rf $lowerdir $lowerdir2 $upperdir $workdir
+	mkdir -p $lowerdir $lowerdir2 $upperdir $workdir
+}
+
+
+# Test stability, should not crash and should not fail on "yes" mode.
+make_test_dirs
+create_layer $lowerdir2 $lowerdir $upperdir
+wait %1 %2 %3
+
+_overlay_fsck_dirs "$lowerdir:$lowerdir2" $upperdir $workdir -y >> \
+	$seqres.full 2>&1
+
+[[ "$?" == "$FSCK_OK" || "$?" == "$FSCK_NONDESTRUCT" ]] || \
+	echo "fsck return unexpected $?"
+
+# success, all done
+echo "Silence is golden"
+status=0
+exit
diff --git a/tests/overlay/061.out b/tests/overlay/061.out
new file mode 100644
index 0000000..273be9e
--- /dev/null
+++ b/tests/overlay/061.out
@@ -0,0 +1,2 @@
+QA output created by 061
+Silence is golden
diff --git a/tests/overlay/group b/tests/overlay/group
index b73d7e9..7d13271 100644
--- a/tests/overlay/group
+++ b/tests/overlay/group
@@ -63,3 +63,4 @@
 058 auto quick exportfs
 059 auto quick copyup
 060 auto quick metacopy
+061 auto stress fsck
-- 
2.5.0

Re: [xfstests PATCH 2/3] overlay: add fsck.overlay stress test

[Amir Goldstein]

On Sat, Jul 28, 2018 at 11:42 AM, zhangyi (F) <yi.zhang@huawei.com> wrote:
> Introduce a test case for fsck.overlay which runs on the underlying
> directories created by fsstress (contain a lot of fs objects) to
> find potential stability issue.
>
> Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
> ---
>  tests/overlay/061     | 81 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/overlay/061.out |  2 ++
>  tests/overlay/group   |  1 +
>  3 files changed, 84 insertions(+)
>  create mode 100755 tests/overlay/061
>  create mode 100644 tests/overlay/061.out
>
> diff --git a/tests/overlay/061 b/tests/overlay/061
> new file mode 100755
> index 0000000..b66b36a
> --- /dev/null
> +++ b/tests/overlay/061
> @@ -0,0 +1,81 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2018 Huawei.  All Rights Reserved.
> +#
> +# FS QA Test No. 061
> +#
> +# Stress test: test fsck.overlay running on the underlying dirs
> +# which were created by fsstress.
> +#
> +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_nocheck
> +_require_command "$FSCK_OVERLAY_PROG" fsck.overlay
> +
> +# remove all files from previous tests
> +_scratch_mkfs
> +
> +# Create an underlying layer which contain a lot of random objects
> +create_layer()
> +{
> +       for dir in $*; do
> +               $FSSTRESS_PROG -s 42 -d $dir -z \
> +                       -f creat=20 -f link=10 -f mkdir=20 -f mknod=10 \
> +                       -f rename=10 -f setxattr=10 -f symlink=10 -f write=10 \
> +                       -p 4 -n 500 -l50 > /dev/null 2>&1 &
> +       done
> +}

There is one main difference between this test and overlay/019 -
this test is not sensitive to timing issues and therefore:
1. You gain nothing from running several fsstress processes in parallel
2. You loose something from using a constant ransom seed (-s 42)
current code gives you all layers exactly the same and also
the same for every invocation of test run, which is not what you want.
I think what you want is to use a random seed and record that seed along
with the output of fsstress in $seqres.full so that when test test does catch
a problem with fsck with some random files, you will be able to reproduce
the same layers from the full test output.

Thanks,
Amir.

Re: [xfstests PATCH 2/3] overlay: add fsck.overlay stress test

[zhangyi (F)]

On 2018/7/30 14:24, Amir Goldstein Wrote:
> On Sat, Jul 28, 2018 at 11:42 AM, zhangyi (F) <yi.zhang@huawei.com> wrote:
>> Introduce a test case for fsck.overlay which runs on the underlying
>> directories created by fsstress (contain a lot of fs objects) to
>> find potential stability issue.
>>
>> Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
>> ---
>>  tests/overlay/061     | 81 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>  tests/overlay/061.out |  2 ++
>>  tests/overlay/group   |  1 +
>>  3 files changed, 84 insertions(+)
>>  create mode 100755 tests/overlay/061
>>  create mode 100644 tests/overlay/061.out
>>
>> diff --git a/tests/overlay/061 b/tests/overlay/061
>> new file mode 100755
>> index 0000000..b66b36a
>> --- /dev/null
>> +++ b/tests/overlay/061
>> @@ -0,0 +1,81 @@
>> +#! /bin/bash
>> +# SPDX-License-Identifier: GPL-2.0
>> +# Copyright (c) 2018 Huawei.  All Rights Reserved.
>> +#
>> +# FS QA Test No. 061
>> +#
>> +# Stress test: test fsck.overlay running on the underlying dirs
>> +# which were created by fsstress.
>> +#
>> +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_nocheck
>> +_require_command "$FSCK_OVERLAY_PROG" fsck.overlay
>> +
>> +# remove all files from previous tests
>> +_scratch_mkfs
>> +
>> +# Create an underlying layer which contain a lot of random objects
>> +create_layer()
>> +{
>> +       for dir in $*; do
>> +               $FSSTRESS_PROG -s 42 -d $dir -z \
>> +                       -f creat=20 -f link=10 -f mkdir=20 -f mknod=10 \
>> +                       -f rename=10 -f setxattr=10 -f symlink=10 -f write=10 \
>> +                       -p 4 -n 500 -l50 > /dev/null 2>&1 &
>> +       done
>> +}
> 
> There is one main difference between this test and overlay/019 -
> this test is not sensitive to timing issues and therefore:
> 1. You gain nothing from running several fsstress processes in parallel

Sorry, I don't get it, running in parallel couldn't speed up fs objests creation ?

> 2. You loose something from using a constant ransom seed (-s 42)
> current code gives you all layers exactly the same and also
> the same for every invocation of test run, which is not what you want.
> I think what you want is to use a random seed and record that seed along
> with the output of fsstress in $seqres.full so that when test test does catch
> a problem with fsck with some random files, you will be able to reproduce
> the same layers from the full test output.

You are right, Thanks for point it out, I missed this totally, will fix.

Thanks,
Yi.

[xfstests PATCH 3/3] overlay: add fsck.overlay exception tests

[zhangyi (F)]

Introduce some exception test cases for fsck.overlay which runs on
invalid/unnormal underlying dirs or invalid input parameters to test
exception handling processes.

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

diff --git a/tests/overlay/062 b/tests/overlay/062
new file mode 100755
index 0000000..31f856d
--- /dev/null
+++ b/tests/overlay/062
@@ -0,0 +1,236 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2018 Huawei.  All Rights Reserved.
+#
+# FS QA Test No. 062
+#
+# Exception test: test fsck.overlay runs on invalid underlying
+# dirs or with invalid input options.
+#
+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_nocheck
+_require_attrs
+_require_command "$FSCK_OVERLAY_PROG" fsck.overlay
+_require_chattr i
+
+# remove all files from previous tests
+_scratch_mkfs
+
+# Create a whiteout
+make_whiteout()
+{
+	for arg in $*; do
+		mknod $arg c 0 0
+	done
+}
+
+# Create a redirect directory
+make_redirect_dir()
+{
+	local target=$1
+	local value=$2
+
+	mkdir -p $target
+	$SETFATTR_PROG -n $OVL_XATTR_REDIRECT -v $value $target
+}
+
+# Run fsck.overlay and check return value
+run_fsck()
+{
+	local expect=$1
+	local lowerdir=$2
+	local upperdir=$3
+	local workdir=$4
+	shift 4
+
+	[[ -n $lowerdir ]] && idirs=$(echo -n "-o lowerdir=$lowerdir")
+	[[ -n $upperdir ]] && idirs=$(echo -n "$idirs -o upperdir=$upperdir")
+	[[ -n $workdir ]] && idirs=$(echo -n "$idirs -o workdir=$workdir")
+
+	$FSCK_OVERLAY_PROG $idirs $* >> $seqres.full 2>&1
+	fsck_ret=$?
+
+	[[ "$fsck_ret" == "$expect" ]] || \
+		echo "fsck return unexpected:$expect,$fsck_ret"
+}
+
+# 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
+
+make_test_dirs()
+{
+	rm -rf $lowerdir $lowerdir2 $upperdir $workdir
+	mkdir -p $lowerdir $lowerdir2 $upperdir $workdir
+}
+
+# Test incomplete underlying dirs, should fail
+echo "+ Invalid input"
+make_test_dirs
+
+none_dir=""
+
+run_fsck $FSCK_USAGE $lowerdir $none_dir $none_dir
+run_fsck $FSCK_USAGE $lowerdir $upperdir $none_dir
+run_fsck $FSCK_USAGE $none_dir $upperdir $workdir
+
+
+# Test invalid underlying dirs, should fail
+echo "+ Invalid input(2)"
+make_test_dirs
+
+invalid_dir=$OVL_BASE_SCRATCH_MNT/invalid_dir
+
+run_fsck $FSCK_ERROR $lowerdir $upperdir $invalid_dir
+run_fsck $FSCK_ERROR $lowerdir $invalid_dir $workdir
+run_fsck $FSCK_ERROR $invalid_dir $upperdir $workdir
+
+
+# Test conflict input options, should fail
+echo "+ Invalid input(3)"
+make_test_dirs
+
+run_fsck $FSCK_USAGE $lowerdir $upperdir $workdir -pn
+run_fsck $FSCK_USAGE $lowerdir $upperdir $workdir -yn
+run_fsck $FSCK_USAGE $lowerdir $upperdir $workdir -py
+run_fsck $FSCK_USAGE $lowerdir $upperdir $workdir -pyn
+
+
+# Test upperdir and lowerdir belong to different base filesystems, should fail
+echo "+ Invalid workdir"
+make_test_dirs
+
+test_workdir=$OVL_BASE_TEST_DIR/work
+mkdir -p $test_workdir
+run_fsck $FSCK_ERROR $lowerdir $upperdir $test_workdir
+rm -rf $test_workdir
+
+
+# Test upperdir is subdir of workdir and vice versa, should fail
+echo "+ Invalid workdir(2)"
+make_test_dirs
+
+test_workdir=$upperdir/work
+test_upperdir=$workdir/upper
+mkdir -p $test_workdir $test_upperdir
+
+run_fsck $FSCK_ERROR $lowerdir $upperdir $test_workdir
+run_fsck $FSCK_ERROR $lowerdir $test_upperdir $workdir
+
+
+# Test upper layer is read-only, should fail in "!no" mode, and should
+# return the real consistent result in "no" mode.
+echo "+ Upper read-only"
+make_test_dirs
+
+test_lowerdir=$OVL_BASE_TEST_DIR/lower
+mkdir -p $test_lowerdir
+
+# Let upper layer read-only
+$MOUNT_PROG -o remount,ro $OVL_BASE_SCRATCH_MNT > /dev/null 2>&1
+# Refuse to check read-only upper layer
+run_fsck $FSCK_ERROR $test_lowerdir $upperdir $workdir
+run_fsck $FSCK_ERROR $test_lowerdir $upperdir $workdir -p
+run_fsck $FSCK_ERROR $test_lowerdir $upperdir $workdir -y
+# Allow to use "no" mode scan read-only upper layer
+run_fsck $FSCK_OK $test_lowerdir $upperdir $workdir -n
+$MOUNT_PROG -o remount,rw $OVL_BASE_SCRATCH_MNT > /dev/null 2>&1
+
+# Make a simple inconsistency on the upper layer and expect return failure
+make_whiteout $upperdir/invalid
+$MOUNT_PROG -o remount,ro $OVL_BASE_SCRATCH_MNT > /dev/null 2>&1
+run_fsck $FSCK_UNCORRECTED $test_lowerdir $upperdir $workdir -n
+$MOUNT_PROG -o remount,rw $OVL_BASE_SCRATCH_MNT > /dev/null 2>&1
+rm -rf $test_lowerdir
+
+
+# Test lower layer is read-only, should sacn each layer and return
+# the real consistent result.
+echo "+ Lower read-only"
+make_test_dirs
+
+test_lowerdir=$OVL_BASE_TEST_DIR/lower
+mkdir -p $test_lowerdir
+
+# Let lower layer read-only
+$MOUNT_PROG -o remount,ro $OVL_BASE_TEST_DIR > /dev/null 2>&1
+# Allow check read-only lower layers in all modes
+run_fsck $FSCK_OK $test_lowerdir $upperdir $workdir -p
+$MOUNT_PROG -o remount,rw $OVL_BASE_TEST_DIR > /dev/null 2>&1
+
+# Make a simple inconsistency on the read-only lower layer and expect
+# return failure.
+make_whiteout $test_lowerdir/invalid
+$MOUNT_PROG -o remount,ro $OVL_BASE_TEST_DIR > /dev/null 2>&1
+run_fsck $FSCK_UNCORRECTED $test_lowerdir $upperdir $workdir -p
+$MOUNT_PROG -o remount,rw $OVL_BASE_TEST_DIR > /dev/null 2>&1
+rm -rf $test_lowerdir
+
+
+# Test one of the lower layers is read-only, should sacn each layer and
+# return the real consistent result.
+echo "+ Lower read-only(2)"
+make_test_dirs
+
+test_lowerdir=$OVL_BASE_TEST_DIR/lower
+mkdir -p $test_lowerdir
+
+# Let lower layer read-only
+$MOUNT_PROG -o remount,ro $OVL_BASE_TEST_DIR > /dev/null 2>&1
+# Make a simple inconsistency on the bottom read-write lower layer
+# and expect return success (consistent middle read-only layer should
+# not affect the result).
+make_whiteout $lowerdir2/invalid
+run_fsck $FSCK_NONDESTRUCT $test_lowerdir:$lowerdir2 $upperdir $workdir -p
+$MOUNT_PROG -o remount,rw $OVL_BASE_TEST_DIR > /dev/null 2>&1
+
+# Make a simple inconsistency on the middle read-only lower layer
+# and expect return failure.
+make_whiteout $test_lowerdir/invalid
+$MOUNT_PROG -o remount,ro $OVL_BASE_TEST_DIR > /dev/null 2>&1
+run_fsck $FSCK_UNCORRECTED $test_lowerdir:$lowerdir2 $upperdir $workdir -p
+$MOUNT_PROG -o remount,rw $OVL_BASE_TEST_DIR > /dev/null 2>&1
+rm -rf $test_lowerdir
+
+
+# Test encounter error when try to fix some inconsistency, should fail
+echo "+ Encounter error"
+make_test_dirs
+
+# Make a simple inconsistency and set immutable flag to simulate fix error
+make_redirect_dir $upperdir/testdir "invalid"
+
+$CHATTR_PROG +i $upperdir/testdir
+run_fsck $(($FSCK_UNCORRECTED+$FSCK_ERROR)) $lowerdir $upperdir $workdir -p
+$CHATTR_PROG -i $upperdir/testdir
+
+# success, all done
+status=0
+exit
diff --git a/tests/overlay/062.out b/tests/overlay/062.out
new file mode 100644
index 0000000..021b74f
--- /dev/null
+++ b/tests/overlay/062.out
@@ -0,0 +1,10 @@
+QA output created by 062
++ Invalid input
++ Invalid input(2)
++ Invalid input(3)
++ Invalid workdir
++ Invalid workdir(2)
++ Upper read-only
++ Lower read-only
++ Lower read-only(2)
++ Encounter error
diff --git a/tests/overlay/group b/tests/overlay/group
index 7d13271..37c0b03 100644
--- a/tests/overlay/group
+++ b/tests/overlay/group
@@ -64,3 +64,4 @@
 059 auto quick copyup
 060 auto quick metacopy
 061 auto stress fsck
+062 auto quick fsck
-- 
2.5.0

Re: [xfstests PATCH 3/3] overlay: add fsck.overlay exception tests

[Amir Goldstein]

On Sat, Jul 28, 2018 at 11:42 AM, zhangyi (F) <yi.zhang@huawei.com> wrote:
> Introduce some exception test cases for fsck.overlay which runs on
> invalid/unnormal underlying dirs or invalid input parameters to test
> exception handling processes.
>
> Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
> ---
>  tests/overlay/062     | 236 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/overlay/062.out |  10 +++
>  tests/overlay/group   |   1 +
>  3 files changed, 247 insertions(+)
>  create mode 100755 tests/overlay/062
>  create mode 100644 tests/overlay/062.out
>
> diff --git a/tests/overlay/062 b/tests/overlay/062
> new file mode 100755
> index 0000000..31f856d
> --- /dev/null
> +++ b/tests/overlay/062
> @@ -0,0 +1,236 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2018 Huawei.  All Rights Reserved.
> +#
> +# FS QA Test No. 062
> +#
> +# Exception test: test fsck.overlay runs on invalid underlying
> +# dirs or with invalid input options.
> +#
> +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_nocheck
> +_require_attrs
> +_require_command "$FSCK_OVERLAY_PROG" fsck.overlay
> +_require_chattr i
> +
> +# remove all files from previous tests
> +_scratch_mkfs
> +
> +# Create a whiteout
> +make_whiteout()
> +{
> +       for arg in $*; do
> +               mknod $arg c 0 0
> +       done
> +}
> +
> +# Create a redirect directory
> +make_redirect_dir()
> +{
> +       local target=$1
> +       local value=$2
> +
> +       mkdir -p $target
> +       $SETFATTR_PROG -n $OVL_XATTR_REDIRECT -v $value $target
> +}
> +
> +# Run fsck.overlay and check return value
> +run_fsck()
> +{
> +       local expect=$1
> +       local lowerdir=$2
> +       local upperdir=$3
> +       local workdir=$4
> +       shift 4
> +
> +       [[ -n $lowerdir ]] && idirs=$(echo -n "-o lowerdir=$lowerdir")
> +       [[ -n $upperdir ]] && idirs=$(echo -n "$idirs -o upperdir=$upperdir")
> +       [[ -n $workdir ]] && idirs=$(echo -n "$idirs -o workdir=$workdir")
> +
> +       $FSCK_OVERLAY_PROG $idirs $* >> $seqres.full 2>&1
> +       fsck_ret=$?
> +
> +       [[ "$fsck_ret" == "$expect" ]] || \
> +               echo "fsck return unexpected:$expect,$fsck_ret"
> +}
> +
> +# 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
> +
> +make_test_dirs()
> +{
> +       rm -rf $lowerdir $lowerdir2 $upperdir $workdir
> +       mkdir -p $lowerdir $lowerdir2 $upperdir $workdir
> +}
> +
> +# Test incomplete underlying dirs, should fail
> +echo "+ Invalid input"
> +make_test_dirs
> +
> +none_dir=""
> +
> +run_fsck $FSCK_USAGE $lowerdir $none_dir $none_dir
> +run_fsck $FSCK_USAGE $lowerdir $upperdir $none_dir
> +run_fsck $FSCK_USAGE $none_dir $upperdir $workdir

Unless you use "$none_dir", 2 cases above are the same.

> +
> +
> +# Test invalid underlying dirs, should fail
> +echo "+ Invalid input(2)"
> +make_test_dirs
> +
> +invalid_dir=$OVL_BASE_SCRATCH_MNT/invalid_dir
> +
> +run_fsck $FSCK_ERROR $lowerdir $upperdir $invalid_dir
> +run_fsck $FSCK_ERROR $lowerdir $invalid_dir $workdir
> +run_fsck $FSCK_ERROR $invalid_dir $upperdir $workdir
> +
> +
> +# Test conflict input options, should fail
> +echo "+ Invalid input(3)"
> +make_test_dirs
> +
> +run_fsck $FSCK_USAGE $lowerdir $upperdir $workdir -pn
> +run_fsck $FSCK_USAGE $lowerdir $upperdir $workdir -yn
> +run_fsck $FSCK_USAGE $lowerdir $upperdir $workdir -py
> +run_fsck $FSCK_USAGE $lowerdir $upperdir $workdir -pyn
> +
> +
> +# Test upperdir and lowerdir belong to different base filesystems, should fail

You mean upper and work.

> +echo "+ Invalid workdir"
> +make_test_dirs
> +
> +test_workdir=$OVL_BASE_TEST_DIR/work
> +mkdir -p $test_workdir
> +run_fsck $FSCK_ERROR $lowerdir $upperdir $test_workdir
> +rm -rf $test_workdir
> +
> +
> +# Test upperdir is subdir of workdir and vice versa, should fail
> +echo "+ Invalid workdir(2)"
> +make_test_dirs
> +
> +test_workdir=$upperdir/work
> +test_upperdir=$workdir/upper
> +mkdir -p $test_workdir $test_upperdir
> +
> +run_fsck $FSCK_ERROR $lowerdir $upperdir $test_workdir
> +run_fsck $FSCK_ERROR $lowerdir $test_upperdir $workdir
> +
> +
> +# Test upper layer is read-only, should fail in "!no" mode, and should
> +# return the real consistent result in "no" mode.
> +echo "+ Upper read-only"
> +make_test_dirs
> +
> +test_lowerdir=$OVL_BASE_TEST_DIR/lower
> +mkdir -p $test_lowerdir
> +
> +# Let upper layer read-only
> +$MOUNT_PROG -o remount,ro $OVL_BASE_SCRATCH_MNT > /dev/null 2>&1
> +# Refuse to check read-only upper layer
> +run_fsck $FSCK_ERROR $test_lowerdir $upperdir $workdir
> +run_fsck $FSCK_ERROR $test_lowerdir $upperdir $workdir -p
> +run_fsck $FSCK_ERROR $test_lowerdir $upperdir $workdir -y
> +# Allow to use "no" mode scan read-only upper layer
> +run_fsck $FSCK_OK $test_lowerdir $upperdir $workdir -n
> +$MOUNT_PROG -o remount,rw $OVL_BASE_SCRATCH_MNT > /dev/null 2>&1
> +
> +# Make a simple inconsistency on the upper layer and expect return failure
> +make_whiteout $upperdir/invalid
> +$MOUNT_PROG -o remount,ro $OVL_BASE_SCRATCH_MNT > /dev/null 2>&1
> +run_fsck $FSCK_UNCORRECTED $test_lowerdir $upperdir $workdir -n
> +$MOUNT_PROG -o remount,rw $OVL_BASE_SCRATCH_MNT > /dev/null 2>&1
> +rm -rf $test_lowerdir
> +
> +
> +# Test lower layer is read-only, should sacn each layer and return
> +# the real consistent result.
> +echo "+ Lower read-only"
> +make_test_dirs
> +
> +test_lowerdir=$OVL_BASE_TEST_DIR/lower
> +mkdir -p $test_lowerdir
> +
> +# Let lower layer read-only
> +$MOUNT_PROG -o remount,ro $OVL_BASE_TEST_DIR > /dev/null 2>&1
> +# Allow check read-only lower layers in all modes
> +run_fsck $FSCK_OK $test_lowerdir $upperdir $workdir -p
> +$MOUNT_PROG -o remount,rw $OVL_BASE_TEST_DIR > /dev/null 2>&1
> +
> +# Make a simple inconsistency on the read-only lower layer and expect
> +# return failure.
> +make_whiteout $test_lowerdir/invalid
> +$MOUNT_PROG -o remount,ro $OVL_BASE_TEST_DIR > /dev/null 2>&1
> +run_fsck $FSCK_UNCORRECTED $test_lowerdir $upperdir $workdir -p
> +$MOUNT_PROG -o remount,rw $OVL_BASE_TEST_DIR > /dev/null 2>&1
> +rm -rf $test_lowerdir
> +
> +
> +# Test one of the lower layers is read-only, should sacn each layer and
> +# return the real consistent result.
> +echo "+ Lower read-only(2)"
> +make_test_dirs
> +
> +test_lowerdir=$OVL_BASE_TEST_DIR/lower
> +mkdir -p $test_lowerdir
> +
> +# Let lower layer read-only
> +$MOUNT_PROG -o remount,ro $OVL_BASE_TEST_DIR > /dev/null 2>&1
> +# Make a simple inconsistency on the bottom read-write lower layer
> +# and expect return success (consistent middle read-only layer should
> +# not affect the result).
> +make_whiteout $lowerdir2/invalid
> +run_fsck $FSCK_NONDESTRUCT $test_lowerdir:$lowerdir2 $upperdir $workdir -p
> +$MOUNT_PROG -o remount,rw $OVL_BASE_TEST_DIR > /dev/null 2>&1
> +
> +# Make a simple inconsistency on the middle read-only lower layer
> +# and expect return failure.
> +make_whiteout $test_lowerdir/invalid
> +$MOUNT_PROG -o remount,ro $OVL_BASE_TEST_DIR > /dev/null 2>&1
> +run_fsck $FSCK_UNCORRECTED $test_lowerdir:$lowerdir2 $upperdir $workdir -p
> +$MOUNT_PROG -o remount,rw $OVL_BASE_TEST_DIR > /dev/null 2>&1
> +rm -rf $test_lowerdir
> +
> +
> +# Test encounter error when try to fix some inconsistency, should fail
> +echo "+ Encounter error"
> +make_test_dirs
> +
> +# Make a simple inconsistency and set immutable flag to simulate fix error
> +make_redirect_dir $upperdir/testdir "invalid"
> +
> +$CHATTR_PROG +i $upperdir/testdir
> +run_fsck $(($FSCK_UNCORRECTED+$FSCK_ERROR)) $lowerdir $upperdir $workdir -p
> +$CHATTR_PROG -i $upperdir/testdir
> +

Please clear immutable flag also in cleanup().

Thanks,
Amir.

Re: [xfstests PATCH 0/3] overlay: enhance fsck.overlay test cases

[Amir Goldstein]

On Sat, Jul 28, 2018 at 11:42 AM, zhangyi (F) <yi.zhang@huawei.com> wrote:
> Hi All,
>
> These are fsck.overlay program related patch set. The first patch correct
> the exit code of fsck.overlay in the existing 3 test cases, the second
> patch introduce a stress case to test stability, and the final patch add
> some exception cases to test exception handling process of fsck.overlay.
>
> The fsck.overlay program also was undate at:
>
> https://github.com/hisilicon/overlayfs-progs/tree/next
>
> All these three patches have been tested with the new fsck.overlay,
> please review.
>

So if Eryu tests your changes without updating to new fsck.overlay,
tests will start to fail. Is that the expected behavior?

Right now it may not be a big nuisance, but you should declare which
is "the official" version of fsck.overlay to test with xfstest -
is it next? master?
Probably best if you ask Eryu to hold back on merging changes not
compatible with current fsck.overlay master, until the master branch
is updated and would be useful to ANNOUNCE the overlayfs-progs
updates to the list with the gist of changes.

Thanks,
Amir.

Re: [xfstests PATCH 0/3] overlay: enhance fsck.overlay test cases

[zhangyi (F)]

On 2018/7/30 13:56, Amir Goldstein Wrote:
> On Sat, Jul 28, 2018 at 11:42 AM, zhangyi (F) <yi.zhang@huawei.com> wrote:
>> Hi All,
>>
>> These are fsck.overlay program related patch set. The first patch correct
>> the exit code of fsck.overlay in the existing 3 test cases, the second
>> patch introduce a stress case to test stability, and the final patch add
>> some exception cases to test exception handling process of fsck.overlay.
>>
>> The fsck.overlay program also was undate at:
>>
>> https://github.com/hisilicon/overlayfs-progs/tree/next
>>
>> All these three patches have been tested with the new fsck.overlay,
>> please review.
>>
> 
> So if Eryu tests your changes without updating to new fsck.overlay,
> tests will start to fail. Is that the expected behavior?
> 
> Right now it may not be a big nuisance, but you should declare which
> is "the official" version of fsck.overlay to test with xfstest -
> is it next? master?
> Probably best if you ask Eryu to hold back on merging changes not
> compatible with current fsck.overlay master, until the master branch
> is updated and would be useful to ANNOUNCE the overlayfs-progs
> updates to the list with the gist of changes.
> 

Hi Amir,
Thanks for your suggestion. I will update these fsck.overlay changes to
master branch and post an ANNOUNCE email.

I originally wanted to wait for some comments, if any, before I update
these changes to fsck.overlay's master branch. I will follow your
suggested steps next time, and will post patch set before update to master
branch if it deserve to review.

Thanks,
Yi.