diff mbox series

fuzzy: skip online scrub and health checks if not supported

Message ID 20230522212712.97846-1-ailiop@suse.com (mailing list archive)
State New, archived
Headers show
Series fuzzy: skip online scrub and health checks if not supported | expand

Commit Message

Anthony Iliopoulos May 22, 2023, 9:27 p.m. UTC
Commit a27e6e6f4c18 introduced xfs health checking on no-repair fuzz,
which in turn requires scrub to be run before that. The health checks
are done only if scrub returns with an error (which is expected as an
indication that fuzzed metadata errors were picked up), but the code
does not discern between xfs_scrub returning an error because of
uncorrected metadata vs failing because the kernel does not support
scrub at all.

This causes all tests that do fuzzing with no-repair strategy to fail on
kernels compiled without online scrub support (CONFIG_XFS_ONLINE_SCRUB).

Skip scrub and health checks altogether, if the kernel does not support
it, since the tests are still valuable.

Fixes: a27e6e6f4c18 ("common: check xfs health after doing an online scrub")
Signed-off-by: Anthony Iliopoulos <ailiop@suse.com>
---
 common/fuzzy | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Darrick J. Wong May 22, 2023, 11:45 p.m. UTC | #1
On Mon, May 22, 2023 at 11:27:12PM +0200, Anthony Iliopoulos wrote:
> Commit a27e6e6f4c18 introduced xfs health checking on no-repair fuzz,
> which in turn requires scrub to be run before that. The health checks
> are done only if scrub returns with an error (which is expected as an
> indication that fuzzed metadata errors were picked up), but the code
> does not discern between xfs_scrub returning an error because of
> uncorrected metadata vs failing because the kernel does not support
> scrub at all.
> 
> This causes all tests that do fuzzing with no-repair strategy to fail on
> kernels compiled without online scrub support (CONFIG_XFS_ONLINE_SCRUB).
> 
> Skip scrub and health checks altogether, if the kernel does not support
> it, since the tests are still valuable.
> 
> Fixes: a27e6e6f4c18 ("common: check xfs health after doing an online scrub")
> Signed-off-by: Anthony Iliopoulos <ailiop@suse.com>

At first I thought "Doesn't the _require_scrub in (say) xfs/466 prevent
this test from running at all?"  But then I remembered that helper only
checks for the existence of the xfs_scrub program; it doesn't actually
verify that the kernel works.

I probably need to do more work making sure that the
online/offline/bothrepair tests actually go away if the kernel support
isn't there.  In the meantime, I think this is correct so

Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D


> ---
>  common/fuzzy | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/common/fuzzy b/common/fuzzy
> index 4365e7343e92..a78a354142df 100644
> --- a/common/fuzzy
> +++ b/common/fuzzy
> @@ -422,6 +422,12 @@ __scratch_xfs_fuzz_field_norepair() {
>  		return 1
>  	fi
>  
> +	# Skip scrub and health check if scrub is not supported
> +	if ! _supports_xfs_scrub $SCRATCH_MNT $SCRATCH_DEV; then
> +		__scratch_xfs_fuzz_unmount
> +		return 0
> +	fi
> +
>  	# Make sure online scrub will catch whatever we fuzzed
>  	__fuzz_notify "++ Detect fuzzed field (online)"
>  	_scratch_scrub -n -a 1 -e continue 2>&1
> -- 
> 2.40.0
>
Zorro Lang May 23, 2023, 4:16 a.m. UTC | #2
On Mon, May 22, 2023 at 11:27:12PM +0200, Anthony Iliopoulos wrote:
> Commit a27e6e6f4c18 introduced xfs health checking on no-repair fuzz,
> which in turn requires scrub to be run before that. The health checks
> are done only if scrub returns with an error (which is expected as an
> indication that fuzzed metadata errors were picked up), but the code
> does not discern between xfs_scrub returning an error because of
> uncorrected metadata vs failing because the kernel does not support
> scrub at all.
> 
> This causes all tests that do fuzzing with no-repair strategy to fail on
> kernels compiled without online scrub support (CONFIG_XFS_ONLINE_SCRUB).

Great, thanks for fixing this issue!

Reviewed-by: Zorro Lang <zlang@redhat.com>

> 
> Skip scrub and health checks altogether, if the kernel does not support
> it, since the tests are still valuable.
> 
> Fixes: a27e6e6f4c18 ("common: check xfs health after doing an online scrub")
> Signed-off-by: Anthony Iliopoulos <ailiop@suse.com>
> ---
>  common/fuzzy | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/common/fuzzy b/common/fuzzy
> index 4365e7343e92..a78a354142df 100644
> --- a/common/fuzzy
> +++ b/common/fuzzy
> @@ -422,6 +422,12 @@ __scratch_xfs_fuzz_field_norepair() {
>  		return 1
>  	fi
>  
> +	# Skip scrub and health check if scrub is not supported
> +	if ! _supports_xfs_scrub $SCRATCH_MNT $SCRATCH_DEV; then
> +		__scratch_xfs_fuzz_unmount
> +		return 0
> +	fi
> +
>  	# Make sure online scrub will catch whatever we fuzzed
>  	__fuzz_notify "++ Detect fuzzed field (online)"
>  	_scratch_scrub -n -a 1 -e continue 2>&1
> -- 
> 2.40.0
>
diff mbox series

Patch

diff --git a/common/fuzzy b/common/fuzzy
index 4365e7343e92..a78a354142df 100644
--- a/common/fuzzy
+++ b/common/fuzzy
@@ -422,6 +422,12 @@  __scratch_xfs_fuzz_field_norepair() {
 		return 1
 	fi
 
+	# Skip scrub and health check if scrub is not supported
+	if ! _supports_xfs_scrub $SCRATCH_MNT $SCRATCH_DEV; then
+		__scratch_xfs_fuzz_unmount
+		return 0
+	fi
+
 	# Make sure online scrub will catch whatever we fuzzed
 	__fuzz_notify "++ Detect fuzzed field (online)"
 	_scratch_scrub -n -a 1 -e continue 2>&1