Re: [PATCH] fstests: btrfs/253: fix false alert due to _set_fs_sysfs_attr changes

[Naohiro Aota <Naohiro.Aota@wdc.com>]

On Sun Apr 20, 2025 at 5:38 PM JST, Qu Wenruo wrote:
> [FALSE FAILURE]
> Test btrfs/253 now fails like the following:
>
> btrfs/253 2s ... - output mismatch (see ~/xfstests/results//btrfs/253.out.bad)
>     --- tests/btrfs/253.out	2022-05-11 11:25:30.753333331 +0930
>     +++ ~/xfstests/results//btrfs/253.out.bad	2025-04-20 17:28:39.139180394 +0930
>     @@ -5,6 +5,7 @@
>      Calculate request size so last memory allocation cannot be completely fullfilled.
>      Third allocation.
>      Force allocation of system block type must fail.
>     +./common/rc: line 5213: echo: write error: No space left on device
>      Verify first allocation.
>      Verify second allocation.
>      Verify third allocation.
>     ...
>     (Run 'diff -u ~/xfstests/tests/btrfs/253.out ~/xfstests/results//btrfs/253.out.bad'  to see the entire diff)
>
> [CAUSE]
> Since commit 0a9011ae6a36 ("fstests: common/rc: set_fs_sysfs_attr:
> redirect errors to stdout") the function _set_fs_sysfs_attr() always
> output everything into stdout, thus the stderr redirection makes no
> sense anymore.
>
> And the expected failure will cause output difference and fail the test.
>
> [FIX]
> Instead of the useless re-direct of stderr, save the stdout and check if
> it contains the word "error" to determine if it failed.
>
> Fixes: 0a9011ae6a36 ("fstests: common/rc: set_fs_sysfs_attr: redirect errors to stdout")
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  tests/btrfs/253 | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/tests/btrfs/253 b/tests/btrfs/253
> index adbc6bfb..ad69dfe1 100755
> --- a/tests/btrfs/253
> +++ b/tests/btrfs/253
> @@ -228,7 +228,12 @@ alloc_size "Data" FOURTH_DATA_SIZE_MB
>  # Force chunk allocation of system block type must fail.
>  #
>  echo "Force allocation of system block type must fail."
> -_set_fs_sysfs_attr ${SCRATCH_BDEV} allocation/system/force_chunk_alloc 1 2>/dev/null
> +output=$(_set_fs_sysfs_attr ${SCRATCH_BDEV} allocation/system/force_chunk_alloc 1)
> +
> +if ! echo "$output" | grep -q "error" ; then
> +	echo "Force allocation succeeded unexpectedly."
> +	echo "$output" >> $seqres.full
> +fi

Can't we use _set_sysfs_policy_must_fail() instead? Apparently, that
function does not looks only for "policy" setting. So, renaming the
function would also be appreciated.

>  
>  #
>  # Verification of initial allocation.