From: Eryu Guan <guaneryu@gmail.com>
To: Amir Goldstein <amir73il@gmail.com>
Cc: zhangyi <yi.zhang@huawei.com>, Miklos Szeredi <miklos@szeredi.hu>,
linux-unionfs@vger.kernel.org, fstests@vger.kernel.org
Subject: Re: [PATCH v3 3/4] overlay: correct fsck.overlay exit code
Date: Sun, 2 Jun 2019 15:26:38 +0800 [thread overview]
Message-ID: <20190602070949.GS15846@desktop> (raw)
In-Reply-To: <20190528151723.12525-4-amir73il@gmail.com>
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
>
next prev parent reply other threads:[~2019-06-02 7:26 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190602070949.GS15846@desktop \
--to=guaneryu@gmail.com \
--cc=amir73il@gmail.com \
--cc=fstests@vger.kernel.org \
--cc=linux-unionfs@vger.kernel.org \
--cc=miklos@szeredi.hu \
--cc=yi.zhang@huawei.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.