[PATCH] fstests: btrfs/220: fix the test failure due to new default mount option

[PATCH] fstests: btrfs/220: fix the test failure due to new default mount option

[Qu Wenruo]

[BUG]
The latest misc-next tree will make test case btrfs/220 fail with the
following error messages:

btrfs/220 15s ... - output mismatch (see ~/xfstests/results//btrfs/220.out.bad)
    --- tests/btrfs/220.out	2022-05-11 09:55:30.749999997 +0800
    +++ ~/xfstests/results//btrfs/220.out.bad	2022-12-09 13:57:23.706666671 +0800
    @@ -1,2 +1,5 @@
     QA output created by 220
    +Unexepcted mount options, checking for 'rw,relatime,discard=async,space_cache=v2,subvolid=5,subvol=/' in 'rw,relatime,space_cache=v2,subvolid=5,subvol=/' using 'nodiscard'
    +Unexepcted mount options, checking for 'rw,relatime,discard=async,space_cache=v2,subvolid=5,subvol=/' in 'rw,relatime,space_cache=v2,subvolid=5,subvol=/' using 'nodiscard'
    +Unexepcted mount options, checking for 'rw,relatime,discard=async,space_cache=v2,subvolid=5,subvol=/' in 'rw,relatime,space_cache=v2,subvolid=5,subvol=/' using 'nodiscard'
     Silence is golden
    ...
    (Run 'diff -u ~/xfstests/tests/btrfs/220.out ~/xfstests/results//btrfs/220.out.bad'  to see the entire diff)
Ran: btrfs/220
Failures: btrfs/220
Failed 1 of 1 tests

[CAUSE]
Since patch "btrfs: auto enable discard=async when possible", which is
already in the maintainer's tree for next merge window, btrfs will
automatically enable asynchronous discard for devices which supports
discard.

This makes our $DEFAULT_OPTS to have "discard=async" in it.

While for "nodiscard" mount option, we will completely disable all
discard, causing the above mismatch.

[FIX]
Fix it by introducing $DEFAULT_NODISCARD_OPTS specifically for
"nodiscard" mount option.

If async discard is not enabled by default, $DEFAULT_NODISCARD_OPTS will
be the same as $DEFAULT_OPTS, thus everything would work as usual.

If async discard is enabled by default, $DEFAULT_NODISCARD_OPTS will
have that removed, so for "nodiscard" we can use $DEFAULT_NODISCARD_OPTS
as expected mount options.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 tests/btrfs/220 | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/tests/btrfs/220 b/tests/btrfs/220
index 4d94ccd6..30ca06f6 100755
--- a/tests/btrfs/220
+++ b/tests/btrfs/220
@@ -280,10 +280,10 @@ test_revertible_options()
 	if [ "$enable_discard_sync" = true ]; then
 		test_roundtrip_mount "discard" "discard" "discard=sync" "discard"
 		test_roundtrip_mount "discard=async" "discard=async" "discard=sync" "discard"
-		test_roundtrip_mount "discard=sync" "discard" "nodiscard" "$DEFAULT_OPTS"
+		test_roundtrip_mount "discard=sync" "discard" "nodiscard" "$DEFAULT_NODISCARD_OPTS"
 	else
 		test_roundtrip_mount "discard" "discard" "discard" "discard"
-		test_roundtrip_mount "discard" "discard" "nodiscard" "$DEFAULT_OPTS"
+		test_roundtrip_mount "discard" "discard" "nodiscard" "$DEFAULT_NODISCARD_OPTS"
 	fi
 
 	test_roundtrip_mount "enospc_debug" "enospc_debug" "noenospc_debug" "$DEFAULT_OPTS"
@@ -344,6 +344,12 @@ _scratch_mount
 DEFAULT_OPTS=$(cat /proc/self/mounts | grep $SCRATCH_MNT | \
 		$AWK_PROG '{ print $4 }')
 
+# Since 63a7cb130718 ("btrfs: auto enable discard=async when possible"),
+# "discard=async" will be automatically enabled if the device supports.
+# This can screw up our test against nodiscard options, thus remove the
+# default "discard=async" mount option for "nodiscard" tests.
+DEFAULT_NODISCARD_OPTS=$(echo -n "$DEFAULT_OPTS" | $SED_PROG 's/,discard=async//')
+
 $BTRFS_UTIL_PROG subvolume create "$SCRATCH_MNT/vol1" > /dev/null
 touch "$SCRATCH_MNT/vol1/file.txt"
 _scratch_unmount
-- 
2.38.0

Re: [PATCH] fstests: btrfs/220: fix the test failure due to new default mount option

[David Disseldorp]

Hi Qu,

On Fri,  9 Dec 2022 14:05:10 +0800, Qu Wenruo wrote:

> [BUG]
> The latest misc-next tree will make test case btrfs/220 fail with the
> following error messages:
> 
> btrfs/220 15s ... - output mismatch (see ~/xfstests/results//btrfs/220.out.bad)
>     --- tests/btrfs/220.out	2022-05-11 09:55:30.749999997 +0800
>     +++ ~/xfstests/results//btrfs/220.out.bad	2022-12-09 13:57:23.706666671 +0800
>     @@ -1,2 +1,5 @@
>      QA output created by 220
>     +Unexepcted mount options, checking for 'rw,relatime,discard=async,space_cache=v2,subvolid=5,subvol=/' in 'rw,relatime,space_cache=v2,subvolid=5,subvol=/' using 'nodiscard'
>     +Unexepcted mount options, checking for 'rw,relatime,discard=async,space_cache=v2,subvolid=5,subvol=/' in 'rw,relatime,space_cache=v2,subvolid=5,subvol=/' using 'nodiscard'
>     +Unexepcted mount options, checking for 'rw,relatime,discard=async,space_cache=v2,subvolid=5,subvol=/' in 'rw,relatime,space_cache=v2,subvolid=5,subvol=/' using 'nodiscard'
>      Silence is golden
>     ...
>     (Run 'diff -u ~/xfstests/tests/btrfs/220.out ~/xfstests/results//btrfs/220.out.bad'  to see the entire diff)
> Ran: btrfs/220
> Failures: btrfs/220
> Failed 1 of 1 tests
> 
> [CAUSE]
> Since patch "btrfs: auto enable discard=async when possible", which is
> already in the maintainer's tree for next merge window, btrfs will
> automatically enable asynchronous discard for devices which supports
> discard.
> 
> This makes our $DEFAULT_OPTS to have "discard=async" in it.
> 
> While for "nodiscard" mount option, we will completely disable all
> discard, causing the above mismatch.
> 
> [FIX]
> Fix it by introducing $DEFAULT_NODISCARD_OPTS specifically for
> "nodiscard" mount option.
> 
> If async discard is not enabled by default, $DEFAULT_NODISCARD_OPTS will
> be the same as $DEFAULT_OPTS, thus everything would work as usual.
> 
> If async discard is enabled by default, $DEFAULT_NODISCARD_OPTS will
> have that removed, so for "nodiscard" we can use $DEFAULT_NODISCARD_OPTS
> as expected mount options.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  tests/btrfs/220 | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/btrfs/220 b/tests/btrfs/220
> index 4d94ccd6..30ca06f6 100755
> --- a/tests/btrfs/220
> +++ b/tests/btrfs/220
> @@ -280,10 +280,10 @@ test_revertible_options()
>  	if [ "$enable_discard_sync" = true ]; then
>  		test_roundtrip_mount "discard" "discard" "discard=sync" "discard"
>  		test_roundtrip_mount "discard=async" "discard=async" "discard=sync" "discard"
> -		test_roundtrip_mount "discard=sync" "discard" "nodiscard" "$DEFAULT_OPTS"
> +		test_roundtrip_mount "discard=sync" "discard" "nodiscard" "$DEFAULT_NODISCARD_OPTS"
>  	else
>  		test_roundtrip_mount "discard" "discard" "discard" "discard"
> -		test_roundtrip_mount "discard" "discard" "nodiscard" "$DEFAULT_OPTS"
> +		test_roundtrip_mount "discard" "discard" "nodiscard" "$DEFAULT_NODISCARD_OPTS"
>  	fi
>  
>  	test_roundtrip_mount "enospc_debug" "enospc_debug" "noenospc_debug" "$DEFAULT_OPTS"
> @@ -344,6 +344,12 @@ _scratch_mount
>  DEFAULT_OPTS=$(cat /proc/self/mounts | grep $SCRATCH_MNT | \
>  		$AWK_PROG '{ print $4 }')
>  
> +# Since 63a7cb130718 ("btrfs: auto enable discard=async when possible"),
> +# "discard=async" will be automatically enabled if the device supports.
> +# This can screw up our test against nodiscard options, thus remove the
> +# default "discard=async" mount option for "nodiscard" tests.
> +DEFAULT_NODISCARD_OPTS=$(echo -n "$DEFAULT_OPTS" | $SED_PROG 's/,discard=async//')

Comma placement here looks a little fragile, but I suppose discard won't
normally be listed first.
FWIW, substring removal is also possible in plain bash e.g.:
  DEFAULT_NODISCARD_OPTS="${DEFAULT_OPTS/,discard=async/}"
Either way...

Reviewed-by: David Disseldorp <ddiss@suse.de>

> +
>  $BTRFS_UTIL_PROG subvolume create "$SCRATCH_MNT/vol1" > /dev/null
>  touch "$SCRATCH_MNT/vol1/file.txt"
>  _scratch_unmount

Re: [PATCH] fstests: btrfs/220: fix the test failure due to new default mount option

[Qu Wenruo]

Ping?

I didn't see this merged thus it would still fail with non-zoned devices.

Thanks,
Qu

On 2022/12/9 14:05, Qu Wenruo wrote:
> [BUG]
> The latest misc-next tree will make test case btrfs/220 fail with the
> following error messages:
> 
> btrfs/220 15s ... - output mismatch (see ~/xfstests/results//btrfs/220.out.bad)
>      --- tests/btrfs/220.out	2022-05-11 09:55:30.749999997 +0800
>      +++ ~/xfstests/results//btrfs/220.out.bad	2022-12-09 13:57:23.706666671 +0800
>      @@ -1,2 +1,5 @@
>       QA output created by 220
>      +Unexepcted mount options, checking for 'rw,relatime,discard=async,space_cache=v2,subvolid=5,subvol=/' in 'rw,relatime,space_cache=v2,subvolid=5,subvol=/' using 'nodiscard'
>      +Unexepcted mount options, checking for 'rw,relatime,discard=async,space_cache=v2,subvolid=5,subvol=/' in 'rw,relatime,space_cache=v2,subvolid=5,subvol=/' using 'nodiscard'
>      +Unexepcted mount options, checking for 'rw,relatime,discard=async,space_cache=v2,subvolid=5,subvol=/' in 'rw,relatime,space_cache=v2,subvolid=5,subvol=/' using 'nodiscard'
>       Silence is golden
>      ...
>      (Run 'diff -u ~/xfstests/tests/btrfs/220.out ~/xfstests/results//btrfs/220.out.bad'  to see the entire diff)
> Ran: btrfs/220
> Failures: btrfs/220
> Failed 1 of 1 tests
> 
> [CAUSE]
> Since patch "btrfs: auto enable discard=async when possible", which is
> already in the maintainer's tree for next merge window, btrfs will
> automatically enable asynchronous discard for devices which supports
> discard.
> 
> This makes our $DEFAULT_OPTS to have "discard=async" in it.
> 
> While for "nodiscard" mount option, we will completely disable all
> discard, causing the above mismatch.
> 
> [FIX]
> Fix it by introducing $DEFAULT_NODISCARD_OPTS specifically for
> "nodiscard" mount option.
> 
> If async discard is not enabled by default, $DEFAULT_NODISCARD_OPTS will
> be the same as $DEFAULT_OPTS, thus everything would work as usual.
> 
> If async discard is enabled by default, $DEFAULT_NODISCARD_OPTS will
> have that removed, so for "nodiscard" we can use $DEFAULT_NODISCARD_OPTS
> as expected mount options.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>   tests/btrfs/220 | 10 ++++++++--
>   1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/btrfs/220 b/tests/btrfs/220
> index 4d94ccd6..30ca06f6 100755
> --- a/tests/btrfs/220
> +++ b/tests/btrfs/220
> @@ -280,10 +280,10 @@ test_revertible_options()
>   	if [ "$enable_discard_sync" = true ]; then
>   		test_roundtrip_mount "discard" "discard" "discard=sync" "discard"
>   		test_roundtrip_mount "discard=async" "discard=async" "discard=sync" "discard"
> -		test_roundtrip_mount "discard=sync" "discard" "nodiscard" "$DEFAULT_OPTS"
> +		test_roundtrip_mount "discard=sync" "discard" "nodiscard" "$DEFAULT_NODISCARD_OPTS"
>   	else
>   		test_roundtrip_mount "discard" "discard" "discard" "discard"
> -		test_roundtrip_mount "discard" "discard" "nodiscard" "$DEFAULT_OPTS"
> +		test_roundtrip_mount "discard" "discard" "nodiscard" "$DEFAULT_NODISCARD_OPTS"
>   	fi
>   
>   	test_roundtrip_mount "enospc_debug" "enospc_debug" "noenospc_debug" "$DEFAULT_OPTS"
> @@ -344,6 +344,12 @@ _scratch_mount
>   DEFAULT_OPTS=$(cat /proc/self/mounts | grep $SCRATCH_MNT | \
>   		$AWK_PROG '{ print $4 }')
>   
> +# Since 63a7cb130718 ("btrfs: auto enable discard=async when possible"),
> +# "discard=async" will be automatically enabled if the device supports.
> +# This can screw up our test against nodiscard options, thus remove the
> +# default "discard=async" mount option for "nodiscard" tests.
> +DEFAULT_NODISCARD_OPTS=$(echo -n "$DEFAULT_OPTS" | $SED_PROG 's/,discard=async//')
> +
>   $BTRFS_UTIL_PROG subvolume create "$SCRATCH_MNT/vol1" > /dev/null
>   touch "$SCRATCH_MNT/vol1/file.txt"
>   _scratch_unmount