Re: [PATCH 12/23] preamble: fix missing _kill_fsstress

Dave Chinner
2025-01-21T04:37:54Z

On Thu, Jan 16, 2025 at 03:28:18PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
>
> Commit 8973af00ec212f added a _kill_fsstress to the standard _cleanup
> function. However, if something breaks during test program
> initialization before it gets to sourcing common/rc, then you get
> failures that look like this:
>
> --- /tmp/fstests/tests/generic/556.out 2024-09-25 12:09:52.938797554 -0700
> +++ /var/tmp/fstests/generic/556.out.bad 2025-01-04 22:34:01.268327003 -0800
> @@ -1,16 +1,3 @@
> QA output created by 556
> -SCRATCH_MNT/basic Casefold
> -SCRATCH_MNT/basic
> -SCRATCH_MNT/casefold_flag_removal Casefold
> -SCRATCH_MNT/casefold_flag_removal Casefold
> -SCRATCH_MNT/flag_inheritance/d1/d2/d3 Casefold
> -SCRATCH_MNT/symlink/ind1/TARGET
> -mv: 'SCRATCH_MNT/rename/rename' and 'SCRATCH_MNT/rename/RENAME' are the same file
> -# file: SCRATCH_MNT/xattrs/x
> -user.foo="bar"
> -
> -# file: SCRATCH_MNT/xattrs/x/f1
> -user.foo="bar"
> -
> -touch: 'SCRATCH_MNT/strict/corac'$'\314\247\303': Invalid argument
> -touch: 'SCRATCH_MNT/strict/cora'$'\303\247\303': Invalid argument
> +./tests/generic/556: 108: common/config: Syntax error: "&" unexpected
> +./tests/generic/556: 10: _kill_fsstress: not found
>
> It's that last line that's unnecessary. Fix this by checking for the
> presence of a _kill_fsstress before invoking it.
>
> Cc: <fstests@vger.kernel.org> # v2024.12.08
> Fixes: 8973af00ec212f ("fstests: cleanup fsstress process management")
> Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
> ---
> common/preamble | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>
> diff --git a/common/preamble b/common/preamble
> index 78e45d522f482c..0c9ee2e0377dd5 100644
> --- a/common/preamble
> +++ b/common/preamble
> @@ -7,7 +7,7 @@
> # Standard cleanup function. Individual tests can override this.
> _cleanup()
> {
> - _kill_fsstress
> + command -v _kill_fsstress &>/dev/null && _kill_fsstress
> cd /
> rm -r -f $tmp.*
> }

Looks fine.

Reviewed-by: Dave Chinner <dchinner@redhat.com>
--
Dave Chinner
david@fromorbit.com