* [PATCH v3 0/4] Misc. fsck.overlay test fixes @ 2019-05-28 15:17 Amir Goldstein 2019-05-28 15:17 ` [PATCH v3 1/4] fstests: define constants for fsck exit codes Amir Goldstein ` (3 more replies) 0 siblings, 4 replies; 10+ messages in thread From: Amir Goldstein @ 2019-05-28 15:17 UTC (permalink / raw) To: Eryu Guan; +Cc: zhangyi, Miklos Szeredi, linux-unionfs, fstests Eryu, This is a re-post for zhangyi's fsck.overlay test fixes from October [1]. Based on my own code review of those patches, I have made some minor modifications and added some more fixes. I have been carrying the two patches by zhangyi in my dev branch for a while. Without them running overlay tests with fsck.overlay installed is failing some tests. Zhangyi, Please review my changes, some based on your suggestions. Feel free to re-post "fsck.overlay stress test" and "fsck.overlay exception tests". My goal with this posting was just to fix failures of existing tests when fsck.overlay is installed. Thanks, Amir. Changes from v2: - Dropped patches of new tests - Move fsck exit code constants to common/config - Fix _repair_scratch_fs [1] https://marc.info/?l=fstests&m=153967515805435&w=2 Amir Goldstein (2): fstests: define constants for fsck exit codes overlay: fix _repair_scratch_fs zhangyi (F) (2): overlay: correct fsck.overlay exit code overlay: fix exit code for some fsck.overlay valid cases common/config | 11 +++++++++++ common/overlay | 36 ++++++++++++++++++++++++++++++++++ common/rc | 15 ++++++++++++--- tests/overlay/045 | 46 ++++++++++++++++++++++++-------------------- tests/overlay/046 | 49 ++++++++++++++++++++++++----------------------- tests/overlay/056 | 9 +++------ 6 files changed, 112 insertions(+), 54 deletions(-) -- 2.17.1 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v3 1/4] fstests: define constants for fsck exit codes 2019-05-28 15:17 [PATCH v3 0/4] Misc. fsck.overlay test fixes Amir Goldstein @ 2019-05-28 15:17 ` Amir Goldstein 2019-05-29 15:57 ` zhangyi (F) 2019-05-28 15:17 ` [PATCH v3 2/4] overlay: fix _repair_scratch_fs Amir Goldstein ` (2 subsequent siblings) 3 siblings, 1 reply; 10+ messages in thread From: Amir Goldstein @ 2019-05-28 15:17 UTC (permalink / raw) To: Eryu Guan; +Cc: zhangyi, Miklos Szeredi, linux-unionfs, fstests Define the constants for hard coded values used in _repair_scratch_fs() to check fsck exit code. Suggested-by: zhangyi (F) <yi.zhang@huawei.com> Signed-off-by: Amir Goldstein <amir73il@gmail.com> --- common/config | 11 +++++++++++ common/rc | 2 +- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/common/config b/common/config index 364432bb..bd64be62 100644 --- a/common/config +++ b/common/config @@ -69,6 +69,17 @@ export OVL_WORK="ovl-work" # overlay mount point parent must be the base fs root export OVL_MNT="ovl-mnt" +# From e2fsprogs/e2fsck/e2fsck.h: +# 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 + export PWD=`pwd` #export DEBUG=${DEBUG:=...} # arbitrary CFLAGS really. export MALLOCLIB=${MALLOCLIB:=/usr/lib/libefence.a} diff --git a/common/rc b/common/rc index e78e0920..cedc1cfa 100644 --- a/common/rc +++ b/common/rc @@ -1116,7 +1116,7 @@ _repair_scratch_fs() fsck -t $FSTYP -y $SCRATCH_DEV 2>&1 local res=$? case $res in - 0|1|2) + $FSCK_OK|$FSCK_NONDESTRUCT|$FSCK_REBOOT) res=0 ;; *) -- 2.17.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/4] fstests: define constants for fsck exit codes 2019-05-28 15:17 ` [PATCH v3 1/4] fstests: define constants for fsck exit codes Amir Goldstein @ 2019-05-29 15:57 ` zhangyi (F) 0 siblings, 0 replies; 10+ messages in thread From: zhangyi (F) @ 2019-05-29 15:57 UTC (permalink / raw) To: Amir Goldstein, Eryu Guan; +Cc: Miklos Szeredi, linux-unionfs, fstests On 2019/5/28 23:17, Amir Goldstein Wrote: > Define the constants for hard coded values used in _repair_scratch_fs() > to check fsck exit code. > > Suggested-by: zhangyi (F) <yi.zhang@huawei.com> > Signed-off-by: Amir Goldstein <amir73il@gmail.com> Looks good to me. Reviewed-by: zhangyi (F) <yi.zhang@huawei.com> Thanks, Yi. > --- > common/config | 11 +++++++++++ > common/rc | 2 +- > 2 files changed, 12 insertions(+), 1 deletion(-) > > diff --git a/common/config b/common/config > index 364432bb..bd64be62 100644 > --- a/common/config > +++ b/common/config > @@ -69,6 +69,17 @@ export OVL_WORK="ovl-work" > # overlay mount point parent must be the base fs root > export OVL_MNT="ovl-mnt" > > +# From e2fsprogs/e2fsck/e2fsck.h: > +# 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 > + > export PWD=`pwd` > #export DEBUG=${DEBUG:=...} # arbitrary CFLAGS really. > export MALLOCLIB=${MALLOCLIB:=/usr/lib/libefence.a} > diff --git a/common/rc b/common/rc > index e78e0920..cedc1cfa 100644 > --- a/common/rc > +++ b/common/rc > @@ -1116,7 +1116,7 @@ _repair_scratch_fs() > fsck -t $FSTYP -y $SCRATCH_DEV 2>&1 > local res=$? > case $res in > - 0|1|2) > + $FSCK_OK|$FSCK_NONDESTRUCT|$FSCK_REBOOT) > res=0 > ;; > *) > ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v3 2/4] overlay: fix _repair_scratch_fs 2019-05-28 15:17 [PATCH v3 0/4] Misc. fsck.overlay test fixes Amir Goldstein 2019-05-28 15:17 ` [PATCH v3 1/4] fstests: define constants for fsck exit codes Amir Goldstein @ 2019-05-28 15:17 ` Amir Goldstein 2019-05-29 16:10 ` zhangyi (F) 2019-05-28 15:17 ` [PATCH v3 3/4] overlay: correct fsck.overlay exit code Amir Goldstein 2019-05-28 15:17 ` [PATCH v3 4/4] overlay: fix exit code for some fsck.overlay valid cases Amir Goldstein 3 siblings, 1 reply; 10+ messages in thread From: Amir Goldstein @ 2019-05-28 15:17 UTC (permalink / raw) To: Eryu Guan; +Cc: zhangyi, Miklos Szeredi, linux-unionfs, fstests _repair_scratch_fs did not do the right thing for overlay. Implement and call _repair_overlay_scratch_fs to repair overlay filesystem and then fall through to repair base filesystem. The only tests currentrly calling _repair_scratch_fs on a ./check -overlay run are generic/330 generic/332 in case the base fs supports reflink. The rest of the tests calling _repair_scratch_fs require that $SCRATCH_DEV is a block device. Suggested-by: zhangyi (F) <yi.zhang@huawei.com> Signed-off-by: Amir Goldstein <amir73il@gmail.com> --- common/overlay | 17 +++++++++++++++++ common/rc | 13 +++++++++++-- 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/common/overlay b/common/overlay index b526f24d..a71c2035 100644 --- a/common/overlay +++ b/common/overlay @@ -320,3 +320,20 @@ _check_overlay_scratch_fs() "$OVL_BASE_SCRATCH_DEV" "$OVL_BASE_SCRATCH_MNT" \ $OVL_BASE_MOUNT_OPTIONS $SELINUX_MOUNT_OPTIONS } + +_repair_overlay_scratch_fs() +{ + _overlay_fsck_dirs $OVL_BASE_SCRATCH_MNT/$OVL_LOWER \ + $OVL_BASE_SCRATCH_MNT/$OVL_UPPER \ + $OVL_BASE_SCRATCH_MNT/$OVL_WORK -y + local res=$? + case $res in + $FSCK_OK|$FSCK_NONDESTRUCT) + res=0 + ;; + *) + _dump_err2 "fsck.overlay failed, err=$res" + ;; + esac + return $res +} diff --git a/common/rc b/common/rc index cedc1cfa..d0aa36a0 100644 --- a/common/rc +++ b/common/rc @@ -1112,8 +1112,17 @@ _repair_scratch_fs() return $res ;; *) - # Let's hope fsck -y suffices... - fsck -t $FSTYP -y $SCRATCH_DEV 2>&1 + local dev=$SCRATCH_DEV + local fstyp=$FSTYP + if [ $FSTYP = "overlay" -a -n "$OVL_BASE_SCRATCH_DEV" ]; then + _repair_overlay_scratch_fs + # Fall through to repair base fs + dev=$OVL_BASE_SCRATCH_DEV + fstyp=$OVL_BASE_FSTYP + $UMOUNT_PROG $OVL_BASE_SCRATCH_MNT + fi + # Let's hope fsck -y suffices... + fsck -t $fstyp -y $dev 2>&1 local res=$? case $res in $FSCK_OK|$FSCK_NONDESTRUCT|$FSCK_REBOOT) -- 2.17.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v3 2/4] overlay: fix _repair_scratch_fs 2019-05-28 15:17 ` [PATCH v3 2/4] overlay: fix _repair_scratch_fs Amir Goldstein @ 2019-05-29 16:10 ` zhangyi (F) 0 siblings, 0 replies; 10+ messages in thread From: zhangyi (F) @ 2019-05-29 16:10 UTC (permalink / raw) To: Amir Goldstein, Eryu Guan; +Cc: Miklos Szeredi, linux-unionfs, fstests On 2019/5/28 23:17, Amir Goldstein Wrote: > _repair_scratch_fs did not do the right thing for overlay. > Implement and call _repair_overlay_scratch_fs to repair > overlay filesystem and then fall through to repair base filesystem. > > The only tests currentrly calling _repair_scratch_fs on a > ./check -overlay run are generic/330 generic/332 in case the > base fs supports reflink. The rest of the tests calling > _repair_scratch_fs require that $SCRATCH_DEV is a block device. > > Suggested-by: zhangyi (F) <yi.zhang@huawei.com> > Signed-off-by: Amir Goldstein <amir73il@gmail.com> > --- > common/overlay | 17 +++++++++++++++++ > common/rc | 13 +++++++++++-- > 2 files changed, 28 insertions(+), 2 deletions(-) > > diff --git a/common/overlay b/common/overlay > index b526f24d..a71c2035 100644 > --- a/common/overlay > +++ b/common/overlay > @@ -320,3 +320,20 @@ _check_overlay_scratch_fs() > "$OVL_BASE_SCRATCH_DEV" "$OVL_BASE_SCRATCH_MNT" \ > $OVL_BASE_MOUNT_OPTIONS $SELINUX_MOUNT_OPTIONS > } > + > +_repair_overlay_scratch_fs() > +{ > + _overlay_fsck_dirs $OVL_BASE_SCRATCH_MNT/$OVL_LOWER \ > + $OVL_BASE_SCRATCH_MNT/$OVL_UPPER \ > + $OVL_BASE_SCRATCH_MNT/$OVL_WORK -y > + local res=$? > + case $res in > + $FSCK_OK|$FSCK_NONDESTRUCT) > + res=0 > + ;; > + *) > + _dump_err2 "fsck.overlay failed, err=$res" > + ;; > + esac > + return $res > +} > diff --git a/common/rc b/common/rc > index cedc1cfa..d0aa36a0 100644 > --- a/common/rc > +++ b/common/rc > @@ -1112,8 +1112,17 @@ _repair_scratch_fs() > return $res > ;; > *) > - # Let's hope fsck -y suffices... > - fsck -t $FSTYP -y $SCRATCH_DEV 2>&1 > + local dev=$SCRATCH_DEV > + local fstyp=$FSTYP > + if [ $FSTYP = "overlay" -a -n "$OVL_BASE_SCRATCH_DEV" ]; then > + _repair_overlay_scratch_fs > + # Fall through to repair base fs > + dev=$OVL_BASE_SCRATCH_DEV > + fstyp=$OVL_BASE_FSTYP > + $UMOUNT_PROG $OVL_BASE_SCRATCH_MNT > + fi > + # Let's hope fsck -y suffices... > + fsck -t $fstyp -y $dev 2>&1 > local res=$? > case $res in > $FSCK_OK|$FSCK_NONDESTRUCT|$FSCK_REBOOT) > It seems that maybe better to return the error code if one of the two repairs failed. But the $res is not used now, so it's not a big deal. Reviewed-by: zhangyi (F) <yi.zhang@huawei.com> ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v3 3/4] overlay: correct fsck.overlay exit code 2019-05-28 15:17 [PATCH v3 0/4] Misc. fsck.overlay test fixes Amir Goldstein 2019-05-28 15:17 ` [PATCH v3 1/4] fstests: define constants for fsck exit codes Amir Goldstein 2019-05-28 15:17 ` [PATCH v3 2/4] overlay: fix _repair_scratch_fs Amir Goldstein @ 2019-05-28 15:17 ` Amir Goldstein 2019-05-29 16:13 ` zhangyi (F) 2019-06-02 7:26 ` Eryu Guan 2019-05-28 15:17 ` [PATCH v3 4/4] overlay: fix exit code for some fsck.overlay valid cases Amir Goldstein 3 siblings, 2 replies; 10+ messages in thread From: Amir Goldstein @ 2019-05-28 15:17 UTC (permalink / raw) To: Eryu Guan; +Cc: zhangyi, Miklos Szeredi, linux-unionfs, fstests From: "zhangyi (F)" <yi.zhang@huawei.com> 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 add a helper function to run fsck.overlay and check the return value is expected or not. [amir] rename helper to _overlay_fsck_expect, split define of FSCK_* to a seprate path. Signed-off-by: zhangyi (F) <yi.zhang@huawei.com> Signed-off-by: Amir Goldstein <amir73il@gmail.com> --- common/overlay | 19 +++++++++++++++++++ tests/overlay/045 | 27 +++++++++------------------ tests/overlay/046 | 36 ++++++++++++------------------------ tests/overlay/056 | 9 +++------ 4 files changed, 43 insertions(+), 48 deletions(-) diff --git a/common/overlay b/common/overlay index a71c2035..53e35caf 100644 --- a/common/overlay +++ b/common/overlay @@ -193,6 +193,25 @@ _overlay_fsck_dirs() -o workdir=$workdir $* } +# Run fsck and check for expected return value +_overlay_fsck_expect() +{ + # The first arguments is the expected fsck program exit code, the + # remaining arguments are the input parameters of the fsck program. + local expect_ret=$1 + local lowerdir=$2 + local upperdir=$3 + local workdir=$4 + shift 4 + + _overlay_fsck_dirs $lowerdir $upperdir $workdir $* >> \ + $seqres.full 2>&1 + fsck_ret=$? + + [[ "$fsck_ret" == "$expect_ret" ]] || \ + echo "fsck return unexpected value:$expect_ret,$fsck_ret" +} + _overlay_check_dirs() { local lowerdir=$1 diff --git a/tests/overlay/045 b/tests/overlay/045 index acc70871..6b5e8ae4 100755 --- a/tests/overlay/045 +++ b/tests/overlay/045 @@ -96,8 +96,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" +_overlay_fsck_expect $FSCK_NONDESTRUCT $lowerdir $upperdir $workdir -p ls $lowerdir ls $upperdir @@ -107,8 +106,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" +_overlay_fsck_expect $FSCK_OK "$lowerdir:$lowerdir2" $upperdir $workdir -p check_whiteout $upperdir/foo $lowerdir/bar # Test orphan whiteout in opaque directory, should remove @@ -119,8 +117,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" +_overlay_fsck_expect $FSCK_NONDESTRUCT $lowerdir $upperdir $workdir -p ls $upperdir/testdir # Test orphan whiteout whose parent path is not an merged directory, @@ -135,8 +132,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" +_overlay_fsck_expect $FSCK_NONDESTRUCT "$lowerdir:$lowerdir2" $upperdir $workdir -p ls $upperdir/testdir1 ls $upperdir/testdir2 ls $upperdir/testdir3 @@ -150,8 +146,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" +_overlay_fsck_expect $FSCK_NONDESTRUCT $lowerdir $upperdir $workdir -p ls $upperdir/testdir # Test valid whiteout in redirect directory cover file in lower @@ -163,8 +158,7 @@ touch $lowerdir/origin/foo make_redirect_dir $upperdir/testdir "origin" make_whiteout $upperdir/testdir/foo -_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \ - echo "fsck should not fail" +_overlay_fsck_expect $FSCK_OK $lowerdir $upperdir $workdir -p check_whiteout $upperdir/testdir/foo # Test valid whiteout covering lower target whose parent directory @@ -177,8 +171,7 @@ make_redirect_dir $lowerdir/testdir "origin" mkdir -p $upperdir/testdir/subdir make_whiteout $upperdir/testdir/subdir/foo -_overlay_fsck_dirs "$lowerdir:$lowerdir2" $upperdir $workdir -p \ - >> $seqres.full 2>&1 || echo "fsck should not fail" +_overlay_fsck_expect $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 +184,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" +_overlay_fsck_expect $FSCK_NONDESTRUCT $lowerdir $upperdir $workdir -p ls $upperdir/testdir/subdir # Test valid whiteout in reidrect subdirectory in a opaque directory @@ -205,8 +197,7 @@ make_opaque_dir $upperdir/testdir make_redirect_dir $upperdir/testdir/subdir "/origin" make_whiteout $upperdir/testdir/subdir/foo -_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \ - echo "fsck should not fail" +_overlay_fsck_expect $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 6338a383..4a9ee68f 100755 --- a/tests/overlay/046 +++ b/tests/overlay/046 @@ -121,8 +121,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" +_overlay_fsck_expect $FSCK_NONDESTRUCT $lowerdir $upperdir $workdir -p check_no_redirect $upperdir/testdir # Test invalid redirect xattr point to a file origin, should remove @@ -131,8 +130,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" +_overlay_fsck_expect $FSCK_NONDESTRUCT $lowerdir $upperdir $workdir -p check_no_redirect $upperdir/testdir # Test valid redirect xattr point to a directory origin in the same directory, @@ -143,8 +141,7 @@ mkdir $lowerdir/origin make_whiteout $upperdir/origin make_redirect_dir $upperdir/testdir "origin" -_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \ - echo "fsck should not fail" +_overlay_fsck_expect $FSCK_OK $lowerdir $upperdir $workdir -p check_redirect $upperdir/testdir "origin" # Test valid redirect xattr point to a directory origin in different directories @@ -155,8 +152,7 @@ mkdir $lowerdir/origin make_whiteout $upperdir/origin make_redirect_dir $upperdir/testdir1/testdir2 "/origin" -_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \ - echo "fsck should not fail" +_overlay_fsck_expect $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 +162,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" +_overlay_fsck_expect $FSCK_NONDESTRUCT $lowerdir $upperdir $workdir -p check_redirect $upperdir/testdir "origin" check_whiteout $upperdir/origin @@ -178,8 +173,7 @@ mkdir $lowerdir/{testdir1,testdir2} make_redirect_dir $upperdir/testdir1 "testdir2" make_redirect_dir $upperdir/testdir2 "testdir1" -_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \ - echo "fsck should not fail" +_overlay_fsck_expect $FSCK_OK $lowerdir $upperdir $workdir -p check_redirect $upperdir/testdir1 "testdir2" check_redirect $upperdir/testdir2 "testdir1" @@ -191,8 +185,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" +_overlay_fsck_expect $FSCK_NONDESTRUCT $lowerdir $upperdir $workdir -y check_no_redirect $upperdir/testdir check_opaque $upperdir/testdir @@ -205,12 +198,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" +_overlay_fsck_expect $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" +_overlay_fsck_expect $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 +214,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" +_overlay_fsck_expect $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" +_overlay_fsck_expect $FSCK_NONDESTRUCT $lowerdir $upperdir $workdir -y check_no_redirect $upperdir/testdir # Test duplicate redirect xattr with lower same name directory exists, @@ -240,8 +229,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" +_overlay_fsck_expect $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 44ffb54a..dc7b98cb 100755 --- a/tests/overlay/056 +++ b/tests/overlay/056 @@ -96,8 +96,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" +_overlay_fsck_expect $FSCK_NONDESTRUCT $lowerdir $upperdir $workdir -p check_impure $upperdir/testdir1 check_impure $upperdir/testdir2 @@ -108,8 +107,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" +_overlay_fsck_expect $FSCK_NONDESTRUCT $lowerdir $upperdir $workdir -p check_impure $upperdir/testdir # Test missing impure xattr in directory which has merge directories, @@ -118,8 +116,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" +_overlay_fsck_expect $FSCK_NONDESTRUCT $lowerdir $upperdir $workdir -p check_impure $upperdir # success, all done -- 2.17.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v3 3/4] overlay: correct fsck.overlay exit code 2019-05-28 15:17 ` [PATCH v3 3/4] overlay: correct fsck.overlay exit code Amir Goldstein @ 2019-05-29 16:13 ` zhangyi (F) 2019-06-02 7:26 ` Eryu Guan 1 sibling, 0 replies; 10+ messages in thread From: zhangyi (F) @ 2019-05-29 16:13 UTC (permalink / raw) To: Amir Goldstein, Eryu Guan; +Cc: Miklos Szeredi, linux-unionfs, fstests On 2019/5/28 23:17, Amir Goldstein Wrote: > From: "zhangyi (F)" <yi.zhang@huawei.com> > > 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 add a helper function to run fsck.overlay and check > the return value is expected or not. > > [amir] rename helper to _overlay_fsck_expect, split define of FSCK_* > to a seprate path. > > Signed-off-by: zhangyi (F) <yi.zhang@huawei.com> > Signed-off-by: Amir Goldstein <amir73il@gmail.com> Thanks for improving the patch, looks good to me. Reviewed-by: zhangyi (F) <yi.zhang@huawei.com> Thanks, Yi. > --- > common/overlay | 19 +++++++++++++++++++ > tests/overlay/045 | 27 +++++++++------------------ > tests/overlay/046 | 36 ++++++++++++------------------------ > tests/overlay/056 | 9 +++------ > 4 files changed, 43 insertions(+), 48 deletions(-) > > diff --git a/common/overlay b/common/overlay > index a71c2035..53e35caf 100644 > --- a/common/overlay > +++ b/common/overlay > @@ -193,6 +193,25 @@ _overlay_fsck_dirs() > -o workdir=$workdir $* > } > > +# Run fsck and check for expected return value > +_overlay_fsck_expect() > +{ > + # The first arguments is the expected fsck program exit code, the > + # remaining arguments are the input parameters of the fsck program. > + local expect_ret=$1 > + local lowerdir=$2 > + local upperdir=$3 > + local workdir=$4 > + shift 4 > + > + _overlay_fsck_dirs $lowerdir $upperdir $workdir $* >> \ > + $seqres.full 2>&1 > + fsck_ret=$? > + > + [[ "$fsck_ret" == "$expect_ret" ]] || \ > + echo "fsck return unexpected value:$expect_ret,$fsck_ret" > +} > + > _overlay_check_dirs() > { > local lowerdir=$1 > diff --git a/tests/overlay/045 b/tests/overlay/045 > index acc70871..6b5e8ae4 100755 > --- a/tests/overlay/045 > +++ b/tests/overlay/045 > @@ -96,8 +96,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" > +_overlay_fsck_expect $FSCK_NONDESTRUCT $lowerdir $upperdir $workdir -p > ls $lowerdir > ls $upperdir > > @@ -107,8 +106,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" > +_overlay_fsck_expect $FSCK_OK "$lowerdir:$lowerdir2" $upperdir $workdir -p > check_whiteout $upperdir/foo $lowerdir/bar > > # Test orphan whiteout in opaque directory, should remove > @@ -119,8 +117,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" > +_overlay_fsck_expect $FSCK_NONDESTRUCT $lowerdir $upperdir $workdir -p > ls $upperdir/testdir > > # Test orphan whiteout whose parent path is not an merged directory, > @@ -135,8 +132,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" > +_overlay_fsck_expect $FSCK_NONDESTRUCT "$lowerdir:$lowerdir2" $upperdir $workdir -p > ls $upperdir/testdir1 > ls $upperdir/testdir2 > ls $upperdir/testdir3 > @@ -150,8 +146,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" > +_overlay_fsck_expect $FSCK_NONDESTRUCT $lowerdir $upperdir $workdir -p > ls $upperdir/testdir > > # Test valid whiteout in redirect directory cover file in lower > @@ -163,8 +158,7 @@ touch $lowerdir/origin/foo > make_redirect_dir $upperdir/testdir "origin" > make_whiteout $upperdir/testdir/foo > > -_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \ > - echo "fsck should not fail" > +_overlay_fsck_expect $FSCK_OK $lowerdir $upperdir $workdir -p > check_whiteout $upperdir/testdir/foo > > # Test valid whiteout covering lower target whose parent directory > @@ -177,8 +171,7 @@ make_redirect_dir $lowerdir/testdir "origin" > mkdir -p $upperdir/testdir/subdir > make_whiteout $upperdir/testdir/subdir/foo > > -_overlay_fsck_dirs "$lowerdir:$lowerdir2" $upperdir $workdir -p \ > - >> $seqres.full 2>&1 || echo "fsck should not fail" > +_overlay_fsck_expect $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 +184,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" > +_overlay_fsck_expect $FSCK_NONDESTRUCT $lowerdir $upperdir $workdir -p > ls $upperdir/testdir/subdir > > # Test valid whiteout in reidrect subdirectory in a opaque directory > @@ -205,8 +197,7 @@ make_opaque_dir $upperdir/testdir > make_redirect_dir $upperdir/testdir/subdir "/origin" > make_whiteout $upperdir/testdir/subdir/foo > > -_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \ > - echo "fsck should not fail" > +_overlay_fsck_expect $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 6338a383..4a9ee68f 100755 > --- a/tests/overlay/046 > +++ b/tests/overlay/046 > @@ -121,8 +121,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" > +_overlay_fsck_expect $FSCK_NONDESTRUCT $lowerdir $upperdir $workdir -p > check_no_redirect $upperdir/testdir > > # Test invalid redirect xattr point to a file origin, should remove > @@ -131,8 +130,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" > +_overlay_fsck_expect $FSCK_NONDESTRUCT $lowerdir $upperdir $workdir -p > check_no_redirect $upperdir/testdir > > # Test valid redirect xattr point to a directory origin in the same directory, > @@ -143,8 +141,7 @@ mkdir $lowerdir/origin > make_whiteout $upperdir/origin > make_redirect_dir $upperdir/testdir "origin" > > -_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \ > - echo "fsck should not fail" > +_overlay_fsck_expect $FSCK_OK $lowerdir $upperdir $workdir -p > check_redirect $upperdir/testdir "origin" > > # Test valid redirect xattr point to a directory origin in different directories > @@ -155,8 +152,7 @@ mkdir $lowerdir/origin > make_whiteout $upperdir/origin > make_redirect_dir $upperdir/testdir1/testdir2 "/origin" > > -_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \ > - echo "fsck should not fail" > +_overlay_fsck_expect $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 +162,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" > +_overlay_fsck_expect $FSCK_NONDESTRUCT $lowerdir $upperdir $workdir -p > check_redirect $upperdir/testdir "origin" > check_whiteout $upperdir/origin > > @@ -178,8 +173,7 @@ mkdir $lowerdir/{testdir1,testdir2} > make_redirect_dir $upperdir/testdir1 "testdir2" > make_redirect_dir $upperdir/testdir2 "testdir1" > > -_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \ > - echo "fsck should not fail" > +_overlay_fsck_expect $FSCK_OK $lowerdir $upperdir $workdir -p > check_redirect $upperdir/testdir1 "testdir2" > check_redirect $upperdir/testdir2 "testdir1" > > @@ -191,8 +185,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" > +_overlay_fsck_expect $FSCK_NONDESTRUCT $lowerdir $upperdir $workdir -y > check_no_redirect $upperdir/testdir > check_opaque $upperdir/testdir > > @@ -205,12 +198,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" > +_overlay_fsck_expect $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" > +_overlay_fsck_expect $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 +214,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" > +_overlay_fsck_expect $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" > +_overlay_fsck_expect $FSCK_NONDESTRUCT $lowerdir $upperdir $workdir -y > check_no_redirect $upperdir/testdir > > # Test duplicate redirect xattr with lower same name directory exists, > @@ -240,8 +229,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" > +_overlay_fsck_expect $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 44ffb54a..dc7b98cb 100755 > --- a/tests/overlay/056 > +++ b/tests/overlay/056 > @@ -96,8 +96,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" > +_overlay_fsck_expect $FSCK_NONDESTRUCT $lowerdir $upperdir $workdir -p > check_impure $upperdir/testdir1 > check_impure $upperdir/testdir2 > > @@ -108,8 +107,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" > +_overlay_fsck_expect $FSCK_NONDESTRUCT $lowerdir $upperdir $workdir -p > check_impure $upperdir/testdir > > # Test missing impure xattr in directory which has merge directories, > @@ -118,8 +116,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" > +_overlay_fsck_expect $FSCK_NONDESTRUCT $lowerdir $upperdir $workdir -p > check_impure $upperdir > > # success, all done > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 3/4] overlay: correct fsck.overlay exit code 2019-05-28 15:17 ` [PATCH v3 3/4] overlay: correct fsck.overlay exit code Amir Goldstein 2019-05-29 16:13 ` zhangyi (F) @ 2019-06-02 7:26 ` Eryu Guan 2019-06-02 8:07 ` Amir Goldstein 1 sibling, 1 reply; 10+ messages in thread From: Eryu Guan @ 2019-06-02 7:26 UTC (permalink / raw) To: Amir Goldstein; +Cc: zhangyi, Miklos Szeredi, linux-unionfs, fstests On Tue, May 28, 2019 at 06:17:22PM +0300, Amir Goldstein wrote: > From: "zhangyi (F)" <yi.zhang@huawei.com> > > 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 add a helper function to run fsck.overlay and check > the return value is expected or not. > > [amir] rename helper to _overlay_fsck_expect, split define of FSCK_* > to a seprate path. > > Signed-off-by: zhangyi (F) <yi.zhang@huawei.com> > Signed-off-by: Amir Goldstein <amir73il@gmail.com> > --- > common/overlay | 19 +++++++++++++++++++ > tests/overlay/045 | 27 +++++++++------------------ > tests/overlay/046 | 36 ++++++++++++------------------------ > tests/overlay/056 | 9 +++------ > 4 files changed, 43 insertions(+), 48 deletions(-) > > diff --git a/common/overlay b/common/overlay > index a71c2035..53e35caf 100644 > --- a/common/overlay > +++ b/common/overlay > @@ -193,6 +193,25 @@ _overlay_fsck_dirs() > -o workdir=$workdir $* > } > > +# Run fsck and check for expected return value > +_overlay_fsck_expect() > +{ > + # The first arguments is the expected fsck program exit code, the > + # remaining arguments are the input parameters of the fsck program. > + local expect_ret=$1 > + local lowerdir=$2 > + local upperdir=$3 > + local workdir=$4 > + shift 4 > + > + _overlay_fsck_dirs $lowerdir $upperdir $workdir $* >> \ > + $seqres.full 2>&1 > + fsck_ret=$? > + > + [[ "$fsck_ret" == "$expect_ret" ]] || \ > + echo "fsck return unexpected value:$expect_ret,$fsck_ret" This statement looks ambiguous, it's not that clear which return value is expected and which is unexpected. I'd like to change it to something like: "expect fsck.overlay to return $expect_ret, but got $fsck_ret" I can fix it on commit if you're OK with this change. Thanks, Eryu > +} > + > _overlay_check_dirs() > { > local lowerdir=$1 > diff --git a/tests/overlay/045 b/tests/overlay/045 > index acc70871..6b5e8ae4 100755 > --- a/tests/overlay/045 > +++ b/tests/overlay/045 > @@ -96,8 +96,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" > +_overlay_fsck_expect $FSCK_NONDESTRUCT $lowerdir $upperdir $workdir -p > ls $lowerdir > ls $upperdir > > @@ -107,8 +106,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" > +_overlay_fsck_expect $FSCK_OK "$lowerdir:$lowerdir2" $upperdir $workdir -p > check_whiteout $upperdir/foo $lowerdir/bar > > # Test orphan whiteout in opaque directory, should remove > @@ -119,8 +117,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" > +_overlay_fsck_expect $FSCK_NONDESTRUCT $lowerdir $upperdir $workdir -p > ls $upperdir/testdir > > # Test orphan whiteout whose parent path is not an merged directory, > @@ -135,8 +132,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" > +_overlay_fsck_expect $FSCK_NONDESTRUCT "$lowerdir:$lowerdir2" $upperdir $workdir -p > ls $upperdir/testdir1 > ls $upperdir/testdir2 > ls $upperdir/testdir3 > @@ -150,8 +146,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" > +_overlay_fsck_expect $FSCK_NONDESTRUCT $lowerdir $upperdir $workdir -p > ls $upperdir/testdir > > # Test valid whiteout in redirect directory cover file in lower > @@ -163,8 +158,7 @@ touch $lowerdir/origin/foo > make_redirect_dir $upperdir/testdir "origin" > make_whiteout $upperdir/testdir/foo > > -_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \ > - echo "fsck should not fail" > +_overlay_fsck_expect $FSCK_OK $lowerdir $upperdir $workdir -p > check_whiteout $upperdir/testdir/foo > > # Test valid whiteout covering lower target whose parent directory > @@ -177,8 +171,7 @@ make_redirect_dir $lowerdir/testdir "origin" > mkdir -p $upperdir/testdir/subdir > make_whiteout $upperdir/testdir/subdir/foo > > -_overlay_fsck_dirs "$lowerdir:$lowerdir2" $upperdir $workdir -p \ > - >> $seqres.full 2>&1 || echo "fsck should not fail" > +_overlay_fsck_expect $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 +184,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" > +_overlay_fsck_expect $FSCK_NONDESTRUCT $lowerdir $upperdir $workdir -p > ls $upperdir/testdir/subdir > > # Test valid whiteout in reidrect subdirectory in a opaque directory > @@ -205,8 +197,7 @@ make_opaque_dir $upperdir/testdir > make_redirect_dir $upperdir/testdir/subdir "/origin" > make_whiteout $upperdir/testdir/subdir/foo > > -_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \ > - echo "fsck should not fail" > +_overlay_fsck_expect $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 6338a383..4a9ee68f 100755 > --- a/tests/overlay/046 > +++ b/tests/overlay/046 > @@ -121,8 +121,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" > +_overlay_fsck_expect $FSCK_NONDESTRUCT $lowerdir $upperdir $workdir -p > check_no_redirect $upperdir/testdir > > # Test invalid redirect xattr point to a file origin, should remove > @@ -131,8 +130,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" > +_overlay_fsck_expect $FSCK_NONDESTRUCT $lowerdir $upperdir $workdir -p > check_no_redirect $upperdir/testdir > > # Test valid redirect xattr point to a directory origin in the same directory, > @@ -143,8 +141,7 @@ mkdir $lowerdir/origin > make_whiteout $upperdir/origin > make_redirect_dir $upperdir/testdir "origin" > > -_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \ > - echo "fsck should not fail" > +_overlay_fsck_expect $FSCK_OK $lowerdir $upperdir $workdir -p > check_redirect $upperdir/testdir "origin" > > # Test valid redirect xattr point to a directory origin in different directories > @@ -155,8 +152,7 @@ mkdir $lowerdir/origin > make_whiteout $upperdir/origin > make_redirect_dir $upperdir/testdir1/testdir2 "/origin" > > -_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \ > - echo "fsck should not fail" > +_overlay_fsck_expect $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 +162,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" > +_overlay_fsck_expect $FSCK_NONDESTRUCT $lowerdir $upperdir $workdir -p > check_redirect $upperdir/testdir "origin" > check_whiteout $upperdir/origin > > @@ -178,8 +173,7 @@ mkdir $lowerdir/{testdir1,testdir2} > make_redirect_dir $upperdir/testdir1 "testdir2" > make_redirect_dir $upperdir/testdir2 "testdir1" > > -_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \ > - echo "fsck should not fail" > +_overlay_fsck_expect $FSCK_OK $lowerdir $upperdir $workdir -p > check_redirect $upperdir/testdir1 "testdir2" > check_redirect $upperdir/testdir2 "testdir1" > > @@ -191,8 +185,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" > +_overlay_fsck_expect $FSCK_NONDESTRUCT $lowerdir $upperdir $workdir -y > check_no_redirect $upperdir/testdir > check_opaque $upperdir/testdir > > @@ -205,12 +198,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" > +_overlay_fsck_expect $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" > +_overlay_fsck_expect $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 +214,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" > +_overlay_fsck_expect $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" > +_overlay_fsck_expect $FSCK_NONDESTRUCT $lowerdir $upperdir $workdir -y > check_no_redirect $upperdir/testdir > > # Test duplicate redirect xattr with lower same name directory exists, > @@ -240,8 +229,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" > +_overlay_fsck_expect $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 44ffb54a..dc7b98cb 100755 > --- a/tests/overlay/056 > +++ b/tests/overlay/056 > @@ -96,8 +96,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" > +_overlay_fsck_expect $FSCK_NONDESTRUCT $lowerdir $upperdir $workdir -p > check_impure $upperdir/testdir1 > check_impure $upperdir/testdir2 > > @@ -108,8 +107,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" > +_overlay_fsck_expect $FSCK_NONDESTRUCT $lowerdir $upperdir $workdir -p > check_impure $upperdir/testdir > > # Test missing impure xattr in directory which has merge directories, > @@ -118,8 +116,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" > +_overlay_fsck_expect $FSCK_NONDESTRUCT $lowerdir $upperdir $workdir -p > check_impure $upperdir > > # success, all done > -- > 2.17.1 > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 3/4] overlay: correct fsck.overlay exit code 2019-06-02 7:26 ` Eryu Guan @ 2019-06-02 8:07 ` Amir Goldstein 0 siblings, 0 replies; 10+ messages in thread From: Amir Goldstein @ 2019-06-02 8:07 UTC (permalink / raw) To: Eryu Guan; +Cc: zhangyi, Miklos Szeredi, overlayfs, fstests On Sun, Jun 2, 2019 at 10:26 AM Eryu Guan <guaneryu@gmail.com> wrote: > > On Tue, May 28, 2019 at 06:17:22PM +0300, Amir Goldstein wrote: > > From: "zhangyi (F)" <yi.zhang@huawei.com> > > > > 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 add a helper function to run fsck.overlay and check > > the return value is expected or not. > > > > [amir] rename helper to _overlay_fsck_expect, split define of FSCK_* > > to a seprate path. > > > > Signed-off-by: zhangyi (F) <yi.zhang@huawei.com> > > Signed-off-by: Amir Goldstein <amir73il@gmail.com> > > --- > > common/overlay | 19 +++++++++++++++++++ > > tests/overlay/045 | 27 +++++++++------------------ > > tests/overlay/046 | 36 ++++++++++++------------------------ > > tests/overlay/056 | 9 +++------ > > 4 files changed, 43 insertions(+), 48 deletions(-) > > > > diff --git a/common/overlay b/common/overlay > > index a71c2035..53e35caf 100644 > > --- a/common/overlay > > +++ b/common/overlay > > @@ -193,6 +193,25 @@ _overlay_fsck_dirs() > > -o workdir=$workdir $* > > } > > > > +# Run fsck and check for expected return value > > +_overlay_fsck_expect() > > +{ > > + # The first arguments is the expected fsck program exit code, the > > + # remaining arguments are the input parameters of the fsck program. > > + local expect_ret=$1 > > + local lowerdir=$2 > > + local upperdir=$3 > > + local workdir=$4 > > + shift 4 > > + > > + _overlay_fsck_dirs $lowerdir $upperdir $workdir $* >> \ > > + $seqres.full 2>&1 > > + fsck_ret=$? > > + > > + [[ "$fsck_ret" == "$expect_ret" ]] || \ > > + echo "fsck return unexpected value:$expect_ret,$fsck_ret" > > This statement looks ambiguous, it's not that clear which return value > is expected and which is unexpected. I'd like to change it to something > like: > > "expect fsck.overlay to return $expect_ret, but got $fsck_ret" > > I can fix it on commit if you're OK with this change. > Fine by me. Thanks, Amir. ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v3 4/4] overlay: fix exit code for some fsck.overlay valid cases 2019-05-28 15:17 [PATCH v3 0/4] Misc. fsck.overlay test fixes Amir Goldstein ` (2 preceding siblings ...) 2019-05-28 15:17 ` [PATCH v3 3/4] overlay: correct fsck.overlay exit code Amir Goldstein @ 2019-05-28 15:17 ` Amir Goldstein 3 siblings, 0 replies; 10+ messages in thread From: Amir Goldstein @ 2019-05-28 15:17 UTC (permalink / raw) To: Eryu Guan; +Cc: zhangyi, Miklos Szeredi, linux-unionfs, fstests From: "zhangyi (F)" <yi.zhang@huawei.com> Some valid test cases about fsck.overlay may be not valid enough now, they lose the impure xattr on the parent directory of the simluated redirect directory, and lose the whiteout which use to cover the origin lower object. Then fsck.overlay will fix these two inconsistency which are not those test cases want to cover, thus it will lead to fsck.overlay return FSCK_NONDESTRUCT instead of FSCK_OK. Fix these by complement the missing overlay related features. Signed-off-by: zhangyi (F) <yi.zhang@huawei.com> --- tests/overlay/045 | 19 ++++++++++++++++--- tests/overlay/046 | 13 +++++++++++++ 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/tests/overlay/045 b/tests/overlay/045 index 6b5e8ae4..34b7ce4c 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() { @@ -155,8 +165,9 @@ 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_expect $FSCK_OK $lowerdir $upperdir $workdir -p check_whiteout $upperdir/testdir/foo @@ -169,7 +180,8 @@ 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_expect $FSCK_OK "$lowerdir:$lowerdir2" $upperdir $workdir -p check_whiteout $upperdir/testdir/subdir/foo @@ -195,7 +207,8 @@ 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_expect $FSCK_OK $lowerdir $upperdir $workdir -p check_whiteout $upperdir/testdir/subdir/foo diff --git a/tests/overlay/046 b/tests/overlay/046 index 4a9ee68f..36c74207 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() @@ -140,6 +150,7 @@ make_test_dirs mkdir $lowerdir/origin make_whiteout $upperdir/origin make_redirect_dir $upperdir/testdir "origin" +make_impure_dir $upperdir _overlay_fsck_expect $FSCK_OK $lowerdir $upperdir $workdir -p check_redirect $upperdir/testdir "origin" @@ -151,6 +162,7 @@ make_test_dirs mkdir $lowerdir/origin make_whiteout $upperdir/origin make_redirect_dir $upperdir/testdir1/testdir2 "/origin" +make_impure_dir $upperdir/testdir1 _overlay_fsck_expect $FSCK_OK $lowerdir $upperdir $workdir -p check_redirect $upperdir/testdir1/testdir2 "/origin" @@ -172,6 +184,7 @@ 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_expect $FSCK_OK $lowerdir $upperdir $workdir -p check_redirect $upperdir/testdir1 "testdir2" -- 2.17.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
end of thread, other threads:[~2019-06-02 8:07 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-05-28 15:17 [PATCH v3 0/4] Misc. fsck.overlay test fixes Amir Goldstein 2019-05-28 15:17 ` [PATCH v3 1/4] fstests: define constants for fsck exit codes Amir Goldstein 2019-05-29 15:57 ` zhangyi (F) 2019-05-28 15:17 ` [PATCH v3 2/4] overlay: fix _repair_scratch_fs Amir Goldstein 2019-05-29 16:10 ` zhangyi (F) 2019-05-28 15:17 ` [PATCH v3 3/4] overlay: correct fsck.overlay exit code Amir Goldstein 2019-05-29 16:13 ` zhangyi (F) 2019-06-02 7:26 ` Eryu Guan 2019-06-02 8:07 ` Amir Goldstein 2019-05-28 15:17 ` [PATCH v3 4/4] overlay: fix exit code for some fsck.overlay valid cases Amir Goldstein
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.