On Sun, Jun 19, 2022 at 04:46:55PM +0300, Amir Goldstein wrote:
> Almost all of the tests that _require_freeze() fail to unfreeze
> scratch mount in case the test is interrupted while fs is frozen.
>
> Move the handling of unfreeze to generic check code.
> For now, tests only freeze scratch fs, but to be more robust, unfreeze
> both test and scratch fs following a call to _require_freeze().
>
> Tests could still hang if thier private _cleanup() routine tries
> to modify the frozen fs or wait for a blocked process. Fix the
> _cleanup() routine of xfs/011 to avoid that.
>
> Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
> ---
> check | 14 ++++++++------
> common/rc | 5 +++--
> tests/generic/390 | 2 --
> tests/xfs/011 | 2 --
> tests/xfs/517 | 1 -
> 5 files changed, 11 insertions(+), 13 deletions(-)
>
> diff --git a/check b/check
> index de11b37e..d6ee71aa 100755
> --- a/check
> +++ b/check
> @@ -527,17 +527,21 @@ _check_filesystems()
> {
> local ret=0
>
> + # Make sure both test and scratch are unfrozen post _require_freeze()
> + if [ -f ${RESULT_DIR}/require_freeze ]; then
> + xfs_freeze -u "$TEST_DIR" >/dev/null 2>&1
> + xfs_freeze -u "$SCRATCH_MNT" >/dev/null 2>&1
> + fi
Hi Amir,
I'm wondering why not let each freeze related test cases take care of "unfreeze"
by itself? If a case freeze a filesystem which isn't on TEST_DEV or SCRATCH_DEV
(e.g. a loop device base on a file in $TEST_DIR), or other situations, then the
case still can call _require_freeze, but "unfreeze TEST_DIR or SCRATCH_MNT" can't
help.
Thanks,
Zorro
> if [ -f ${RESULT_DIR}/require_test ]; then
> _check_test_fs || ret=1
> - rm -f ${RESULT_DIR}/require_test*
> else
> _test_unmount 2> /dev/null
> fi
> if [ -f ${RESULT_DIR}/require_scratch ]; then
> _check_scratch_fs || ret=1
> - rm -f ${RESULT_DIR}/require_scratch*
> fi
> _scratch_unmount 2> /dev/null
> + rm -f ${RESULT_DIR}/require_*
> return $ret
> }
>
> @@ -783,8 +787,7 @@ function run_section()
> seqres="$REPORT_DIR/$seqnum"
>
> mkdir -p $RESULT_DIR
> - rm -f ${RESULT_DIR}/require_scratch*
> - rm -f ${RESULT_DIR}/require_test*
> + rm -f ${RESULT_DIR}/require_*
> echo -n "$seqnum"
>
> if $showme; then
> @@ -882,8 +885,7 @@ function run_section()
> _dump_err_cont "[failed, exit status $sts]"
> _test_unmount 2> /dev/null
> _scratch_unmount 2> /dev/null
> - rm -f ${RESULT_DIR}/require_test*
> - rm -f ${RESULT_DIR}/require_scratch*
> + rm -f ${RESULT_DIR}/require_*
> err=true
> else
> # The test apparently passed, so check for corruption
> diff --git a/common/rc b/common/rc
> index 3c072c16..b87dfe05 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -1540,8 +1540,7 @@ _notrun()
> {
> echo "$*" > $seqres.notrun
> echo "$seq not run: $*"
> - rm -f ${RESULT_DIR}/require_test*
> - rm -f ${RESULT_DIR}/require_scratch*
> + rm -f ${RESULT_DIR}/require_*
>
> status=0
> exit
> @@ -3648,6 +3647,8 @@ _require_freeze()
> local result=$?
> xfs_freeze -u "$TEST_DIR" >/dev/null 2>&1
> [ $result -eq 0 ] || _notrun "$FSTYP does not support freezing"
> + # Make sure both test and scratch are unfrozen at the end of the test
> + touch ${RESULT_DIR}/require_freeze
> }
>
> # Does NFS export work on this fs?
> diff --git a/tests/generic/390 b/tests/generic/390
> index 20c66e22..0f2b86fa 100755
> --- a/tests/generic/390
> +++ b/tests/generic/390
> @@ -14,8 +14,6 @@ _begin_fstest auto freeze stress
> _cleanup()
> {
> cd /
> - # Make sure $SCRATCH_MNT is unfreezed
> - xfs_freeze -u $SCRATCH_MNT 2>/dev/null
> rm -f $tmp.*
> }
>
> diff --git a/tests/xfs/011 b/tests/xfs/011
> index d6e9099e..351a574e 100755
> --- a/tests/xfs/011
> +++ b/tests/xfs/011
> @@ -17,9 +17,7 @@ _begin_fstest auto freeze log metadata quick
> _cleanup()
> {
> $KILLALL_PROG -9 fsstress 2>/dev/null
> - wait
> cd /
> - _scratch_unmount 2>/dev/null
> rm -f $tmp.*
> }
>
> diff --git a/tests/xfs/517 b/tests/xfs/517
> index f7f9a8a2..961668e3 100755
> --- a/tests/xfs/517
> +++ b/tests/xfs/517
> @@ -15,7 +15,6 @@ _register_cleanup "_cleanup" BUS
> _cleanup()
> {
> cd /
> - $XFS_IO_PROG -x -c 'thaw' $SCRATCH_MNT > /dev/null 2>&1
> rm -rf $tmp.*
> }
>
> --
> 2.25.1
>
