Re: [PATCH] fstests: generic/741: make cleanup to handle test failure properly

Zorro Lang
2025-06-05T17:10:55Z

On Thu, Jun 05, 2025 at 09:25:24AM +0930, Qu Wenruo wrote:
> [BUG]
> When I was tinkering the bdev open holder parameter, it caused a bug
> that it no longer rejects mounting the underlying device of a
> device-mapper.
>
> And the test case properly detects the regression:
>
> generic/741 1s ... umount: /mnt/test: target is busy.
> _check_btrfs_filesystem: filesystem on /dev/mapper/test-test is inconsistent
> (see /home/adam/xfstests/results//generic/741.full for details)
> Trying to repair broken TEST_DEV file system
> _check_btrfs_filesystem: filesystem on /dev/mapper/test-scratch1 is inconsistent
> (see /home/adam/xfstests/results//generic/741.full for details)
> - output mismatch (see /home/adam/xfstests/results//generic/741.out.bad)
> --- tests/generic/741.out 2024-04-06 08:10:44.773333344 +1030
> +++ /home/adam/xfstests/results//generic/741.out.bad 2025-06-05 09:18:03.675049206 +0930
> @@ -1,3 +1,2 @@
> QA output created by 741
> -mount: TEST_DIR/extra_mnt: SCRATCH_DEV already mounted or mount point busy
> -mount: TEST_DIR/extra_mnt: SCRATCH_DEV already mounted or mount point busy
> +rm: cannot remove '/mnt/test/extra_mnt': Device or resource busy
> ...
> (Run 'diff -u /home/adam/xfstests/tests/generic/741.out /home/adam/xfstests/results//generic/741.out.bad' to see the entire diff)
>
> The problem is, all later test will fail, because the $SCRATCH_DEV is
> still mounted at $extra_mnt:
>
> TEST_DEV=/dev/mapper/test-test is mounted but not on TEST_DIR=/mnt/test - aborting
> Already mounted result:
> /dev/mapper/test-test /mnt/test /dev/mapper/test-test /mnt/test
>
> [CAUSE]
> The test case itself is doing two expected-to-fail mounts, but the
> cleanup function is only doing unmount once, if the mount succeeded
> unexpectedly, the $SCRATCH_DEV will be mounted at $extra_mnt forever.
>
> [ENHANCEMENT]
> To avoid screwing up later test cases, do the $extra_mnt cleanup twice
> to handle the unexpected mount success.
>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
> tests/generic/741 | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/tests/generic/741 b/tests/generic/741
> index cac7045e..c15dc434 100755
> --- a/tests/generic/741
> +++ b/tests/generic/741
> @@ -13,6 +13,10 @@ _begin_fstest auto quick volume tempfsid
> # Override the default cleanup function.
> _cleanup()
> {
> + # If by somehow the fs mounted the underlying device (twice), we have
> + # to make sure $extra_mnt is not mounted, or TEST_DEV can not be
> + # unmounted for fsck.
> + _unmount $extra_mnt &> /dev/null

Hmm... I'm not sure a "double" (even treble) umount is good solution for
temporary "Device or resource busy" after umount. Many other cases might
hit this problem sometimes.
Maybe we can have a helper to do a certain umount which make sure the fs
is truely umounted before returning :)

Thanks,
Zorro

> _unmount $extra_mnt &> /dev/null
> rm -rf $extra_mnt
> _unmount_flakey
> --
> 2.49.0
>
>