diff mbox series

[v2,1/4] overlay: correct fsck.overlay exit code

Message ID 20181016074559.24728-2-yi.zhang@huawei.com (mailing list archive)
State New, archived
Headers show
Series overlay: enhance fsck.overlay test cases | expand

Commit Message

Zhang Yi Oct. 16, 2018, 7:45 a.m. UTC
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.

Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
---
 common/overlay    | 29 +++++++++++++++++++++++++++++
 tests/overlay/045 | 27 +++++++++------------------
 tests/overlay/046 | 36 ++++++++++++------------------------
 tests/overlay/056 |  9 +++------
 4 files changed, 53 insertions(+), 48 deletions(-)

Comments

Amir Goldstein Oct. 16, 2018, 9:45 a.m. UTC | #1
On Tue, Oct 16, 2018 at 10:32 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 add a helper function to run fsck.overlay and check
> the return value is expected or not.
>
> Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
> ---
>  common/overlay    | 29 +++++++++++++++++++++++++++++
>  tests/overlay/045 | 27 +++++++++------------------
>  tests/overlay/046 | 36 ++++++++++++------------------------
>  tests/overlay/056 |  9 +++------
>  4 files changed, 53 insertions(+), 48 deletions(-)
>
> diff --git a/common/overlay b/common/overlay
> index b526f24..4cc2829 100644
> --- a/common/overlay
> +++ b/common/overlay
> @@ -12,6 +12,16 @@ export OVL_XATTR_NLINK="trusted.overlay.nlink"
>  export OVL_XATTR_UPPER="trusted.overlay.upper"
>  export OVL_XATTR_METACOPY="trusted.overlay.metacopy"
>
> +# Export exit code used by fsck.overlay program
> +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
> +
>  # helper function to do the actual overlayfs mount operation
>  _overlay_mount_dirs()
>  {
> @@ -193,6 +203,25 @@ _overlay_fsck_dirs()
>                            -o workdir=$workdir $*
>  }
>
> +# Run fsck and check the return value
> +_run_check_fsck()
> +{
> +       # 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"
> +}

Looks good.

Please consider the name _overlay_repair_dirs() with reference to
_repair_scratch_fs(), which is used for roughly the same purpose.

BTW, the tests generic/330 generic/332 when run with -overlay
over xfs with reflink support seem to call _repair_scratch_fs() which does:
        # Let's hope fsck -y suffices...
        fsck -t $FSTYP -y $SCRATCH_DEV 2>&1
        local res=$?
        case $res in
        0|1|2)
                res=0
                ;;
        *)
                _dump_err2 "fsck.$FSTYP failed, err=$res"

How come fsck.overlay is not complaining about missing arguments??

The rest of the generic tests that call _repair_scratch_fs() have
_require_dm_target() which _require_block_device(), so don't run with overlay.

If you do sort this out and add overlay support to
_repair_scratch_fs() its probably
worth replacing 0|1|2) with FSCK_ constants.

Thanks,
Amir.
Zhang Yi Oct. 18, 2018, 2:37 a.m. UTC | #2
On 2018/10/16 17:45, Amir Goldstein Wrote:
> On Tue, Oct 16, 2018 at 10:32 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 add a helper function to run fsck.overlay and check
>> the return value is expected or not.
>>
>> Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
>> ---
>>  common/overlay    | 29 +++++++++++++++++++++++++++++
>>  tests/overlay/045 | 27 +++++++++------------------
>>  tests/overlay/046 | 36 ++++++++++++------------------------
>>  tests/overlay/056 |  9 +++------
>>  4 files changed, 53 insertions(+), 48 deletions(-)
>>
>> diff --git a/common/overlay b/common/overlay
>> index b526f24..4cc2829 100644
>> --- a/common/overlay
>> +++ b/common/overlay
>> @@ -12,6 +12,16 @@ export OVL_XATTR_NLINK="trusted.overlay.nlink"
>>  export OVL_XATTR_UPPER="trusted.overlay.upper"
>>  export OVL_XATTR_METACOPY="trusted.overlay.metacopy"
>>
>> +# Export exit code used by fsck.overlay program
>> +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
>> +
>>  # helper function to do the actual overlayfs mount operation
>>  _overlay_mount_dirs()
>>  {
>> @@ -193,6 +203,25 @@ _overlay_fsck_dirs()
>>                            -o workdir=$workdir $*
>>  }
>>
>> +# Run fsck and check the return value
>> +_run_check_fsck()
>> +{
>> +       # 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"
>> +}
> 
> Looks good.
> 
> Please consider the name _overlay_repair_dirs() with reference to
> _repair_scratch_fs(), which is used for roughly the same purpose.
> 

_run_check_fsck() is helper used to test fsck and may expect to return
"error" exit code when we do exception tests(see patch 4), but
_repair_scratch_fs() always want to return "correct" exit code.

> BTW, the tests generic/330 generic/332 when run with -overlay
> over xfs with reflink support seem to call _repair_scratch_fs() which does:
>         # Let's hope fsck -y suffices...
>         fsck -t $FSTYP -y $SCRATCH_DEV 2>&1
>         local res=$?
>         case $res in
>         0|1|2)
>                 res=0
>                 ;;
>         *)
>                 _dump_err2 "fsck.$FSTYP failed, err=$res"
> 
> How come fsck.overlay is not complaining about missing arguments??
> 
> The rest of the generic tests that call _repair_scratch_fs() have
> _require_dm_target() which _require_block_device(), so don't run with overlay.
> 
> If you do sort this out and add overlay support to
> _repair_scratch_fs() its probably
> worth replacing 0|1|2) with FSCK_ constants.
> 

Thanks for pointing this out, I think we do something like below can fix
this problem, what do you think?

diff --git a/common/overlay b/common/overlay
index 2896594..b9fe1ee 100644
--- a/common/overlay
+++ b/common/overlay
@@ -226,6 +226,14 @@ _run_check_fsck()
                echo "fsck return unexpected value:$expect_ret,$fsck_ret"
 }

+_scratch_overlay_repair()
+{
+       _overlay_fsck_dirs $OVL_BASE_SCRATCH_MNT/$OVL_LOWER \
+                          $OVL_BASE_SCRATCH_MNT/$OVL_UPPER \
+                          $OVL_BASE_SCRATCH_MNT/$OVL_WORK \
+                          -y
+}
+
 _overlay_check_dirs()
 {
        local lowerdir=$1
diff --git a/common/rc b/common/rc
index 38d9de7..7127539 100644
--- a/common/rc
+++ b/common/rc
@@ -1100,6 +1100,18 @@ _repair_scratch_fs()
        fi
        return $res
         ;;
+    overlay)
+       _scratch_overlay_repair 2>&1
+       local res=$?
+       case $res in
+       $FSCK_OK|$FSCK_NONDESTRUCT)
+               res=0
+               ;;
+       *)
+               _dump_err2 "fsck.overlay failed, err=$res"
+       esac
+       return $res
+       ;;
     *)
         # Let's hope fsck -y suffices...
         fsck -t $FSTYP -y $SCRATCH_DEV 2>&1

Thanks,
Yi.
Amir Goldstein Oct. 18, 2018, 4:37 a.m. UTC | #3
On Thu, Oct 18, 2018 at 5:37 AM zhangyi (F) <yi.zhang@huawei.com> wrote:
>
> On 2018/10/16 17:45, Amir Goldstein Wrote:
> > On Tue, Oct 16, 2018 at 10:32 AM zhangyi (F) <yi.zhang@huawei.com> wrote:
[...]
> >
> > Please consider the name _overlay_repair_dirs() with reference to
> > _repair_scratch_fs(), which is used for roughly the same purpose.
> >
>
> _run_check_fsck() is helper used to test fsck and may expect to return
> "error" exit code when we do exception tests(see patch 4), but
> _repair_scratch_fs() always want to return "correct" exit code.
>

Right. so perhaps we need _overlay_repair_dirs() as a convenience
helper for things like the stress test and also related to another
comment about expecting return 0 is too fragile.

> > BTW, the tests generic/330 generic/332 when run with -overlay
> > over xfs with reflink support seem to call _repair_scratch_fs() which does:
> >         # Let's hope fsck -y suffices...
> >         fsck -t $FSTYP -y $SCRATCH_DEV 2>&1
> >         local res=$?
> >         case $res in
> >         0|1|2)
> >                 res=0
> >                 ;;
> >         *)
> >                 _dump_err2 "fsck.$FSTYP failed, err=$res"
> >
> > How come fsck.overlay is not complaining about missing arguments??
> >
> > The rest of the generic tests that call _repair_scratch_fs() have
> > _require_dm_target() which _require_block_device(), so don't run with overlay.
> >
> > If you do sort this out and add overlay support to
> > _repair_scratch_fs() its probably
> > worth replacing 0|1|2) with FSCK_ constants.
> >
>
> Thanks for pointing this out, I think we do something like below can fix
> this problem, what do you think?

I am puzzled about why those tests do NOT fail when fsck.overlay is given
incorrect args??

>
> diff --git a/common/overlay b/common/overlay
> index 2896594..b9fe1ee 100644
> --- a/common/overlay
> +++ b/common/overlay
> @@ -226,6 +226,14 @@ _run_check_fsck()
>                 echo "fsck return unexpected value:$expect_ret,$fsck_ret"
>  }
>
> +_scratch_overlay_repair()
> +{
> +       _overlay_fsck_dirs $OVL_BASE_SCRATCH_MNT/$OVL_LOWER \
> +                          $OVL_BASE_SCRATCH_MNT/$OVL_UPPER \
> +                          $OVL_BASE_SCRATCH_MNT/$OVL_WORK \
> +                          -y
> +}
> +

All overlay "global" helpers should be prefixed _overlay_XXX
unless there is a good reason to break that rule (even though we did
not always abide by this rule in the past).

But anyway, I think that a more specific _overlay_repair_scratch_fs()
should be used to hide the details of "correct" error code from common/rc.

And it would probably be useful if it used a helper _overlay_repair_dirs()
that can be used from overlay specific tests (like stress test).

Thanks,
Amir.
Amir Goldstein Oct. 18, 2018, 4:54 a.m. UTC | #4
On Tue, Oct 16, 2018 at 10:32 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 add a helper function to run fsck.overlay and check
> the return value is expected or not.
>
> Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
> ---
>  common/overlay    | 29 +++++++++++++++++++++++++++++
>  tests/overlay/045 | 27 +++++++++------------------
>  tests/overlay/046 | 36 ++++++++++++------------------------
>  tests/overlay/056 |  9 +++------
>  4 files changed, 53 insertions(+), 48 deletions(-)
>
> diff --git a/common/overlay b/common/overlay
> index b526f24..4cc2829 100644
> --- a/common/overlay
> +++ b/common/overlay
> @@ -12,6 +12,16 @@ export OVL_XATTR_NLINK="trusted.overlay.nlink"
>  export OVL_XATTR_UPPER="trusted.overlay.upper"
>  export OVL_XATTR_METACOPY="trusted.overlay.metacopy"
>
> +# Export exit code used by fsck.overlay program
> +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
> +
>  # helper function to do the actual overlayfs mount operation
>  _overlay_mount_dirs()
>  {
> @@ -193,6 +203,25 @@ _overlay_fsck_dirs()
>                            -o workdir=$workdir $*
>  }
>
> +# Run fsck and check the return value
> +_run_check_fsck()
> +{
> +       # 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"
> +}

Let's put some naming rules into effect here.
Helper should be prefixed _overlay.
_run_* to me implies run_check() wrapper.
So what's a better descriptive name for this helper? I donno, maybe
_overlay_fsck_expect() ?

Thanks,
Amir.
zhangyi Oct. 18, 2018, 4:22 p.m. UTC | #5
On 2018/10/18 12:37, Amir Goldstein Wrote:

> On Thu, Oct 18, 2018 at 5:37 AM zhangyi (F) <yi.zhang@huawei.com> wrote:
>> On 2018/10/16 17:45, Amir Goldstein Wrote:
>>> On Tue, Oct 16, 2018 at 10:32 AM zhangyi (F) <yi.zhang@huawei.com> wrote:
> [...]
>>> Please consider the name _overlay_repair_dirs() with reference to
>>> _repair_scratch_fs(), which is used for roughly the same purpose.
>>>
>> _run_check_fsck() is helper used to test fsck and may expect to return
>> "error" exit code when we do exception tests(see patch 4), but
>> _repair_scratch_fs() always want to return "correct" exit code.
>>
> Right. so perhaps we need _overlay_repair_dirs() as a convenience
> helper for things like the stress test and also related to another
> comment about expecting return 0 is too fragile.
>
>>> BTW, the tests generic/330 generic/332 when run with -overlay
>>> over xfs with reflink support seem to call _repair_scratch_fs() which does:
>>>          # Let's hope fsck -y suffices...
>>>          fsck -t $FSTYP -y $SCRATCH_DEV 2>&1
>>>          local res=$?
>>>          case $res in
>>>          0|1|2)
>>>                  res=0
>>>                  ;;
>>>          *)
>>>                  _dump_err2 "fsck.$FSTYP failed, err=$res"
>>>
>>> How come fsck.overlay is not complaining about missing arguments??
>>>
>>> The rest of the generic tests that call _repair_scratch_fs() have
>>> _require_dm_target() which _require_block_device(), so don't run with overlay.
>>>
>>> If you do sort this out and add overlay support to
>>> _repair_scratch_fs() its probably
>>> worth replacing 0|1|2) with FSCK_ constants.
>>>
>> Thanks for pointing this out, I think we do something like below can fix
>> this problem, what do you think?
> I am puzzled about why those tests do NOT fail when fsck.overlay is given
> incorrect args??
>
Oh, it's _dump_err2() helper's fault, it should be

--- a/common/rc
+++ b/common/rc
@@ -98,7 +98,7 @@ _dump_err_cont()
  _dump_err2()
  {
      _err_msg="$*"
-    >2& echo "$_err_msg"
+    >&2 echo "$_err_msg"
  }

Thanks,
Yi.
diff mbox series

Patch

diff --git a/common/overlay b/common/overlay
index b526f24..4cc2829 100644
--- a/common/overlay
+++ b/common/overlay
@@ -12,6 +12,16 @@  export OVL_XATTR_NLINK="trusted.overlay.nlink"
 export OVL_XATTR_UPPER="trusted.overlay.upper"
 export OVL_XATTR_METACOPY="trusted.overlay.metacopy"
 
+# Export exit code used by fsck.overlay program
+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
+
 # helper function to do the actual overlayfs mount operation
 _overlay_mount_dirs()
 {
@@ -193,6 +203,25 @@  _overlay_fsck_dirs()
 			   -o workdir=$workdir $*
 }
 
+# Run fsck and check the return value
+_run_check_fsck()
+{
+	# 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 acc7087..ed23258 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"
+_run_check_fsck $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"
+_run_check_fsck $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"
+_run_check_fsck $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"
+_run_check_fsck $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"
+_run_check_fsck $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"
+_run_check_fsck $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"
+_run_check_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 +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"
+_run_check_fsck $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"
+_run_check_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 6338a38..21645c1 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"
+_run_check_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 +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"
+_run_check_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,
@@ -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"
+_run_check_fsck $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"
+_run_check_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 +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"
+_run_check_fsck $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"
+_run_check_fsck $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"
+_run_check_fsck $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"
+_run_check_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_check_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 +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"
+_run_check_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_check_fsck $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"
+_run_check_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 44ffb54..8679b55 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"
+_run_check_fsck $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"
+_run_check_fsck $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"
+_run_check_fsck $FSCK_NONDESTRUCT $lowerdir $upperdir $workdir -p
 check_impure $upperdir
 
 # success, all done