[PATCH] common: add proper btrfs log tree detection to _has_metadata_journaling

[PATCH] common: add proper btrfs log tree detection to _has_metadata_journaling

[Qu Wenruo]

[BUG]
With the incoming btrfs shutdown ioctl/remove_bdev callback support,
btrfs can be tested with the shutdown group.

But test case generic/050 still fails on btrfs with shutdown support:

generic/050 1s ... - output mismatch (see /home/adam/xfstests/results//generic/050.out.bad)
    --- tests/generic/050.out	2022-05-11 11:25:30.763333331 +0930
    +++ /home/adam/xfstests/results//generic/050.out.bad	2025-06-30 10:22:21.752068622 +0930
    @@ -13,9 +13,7 @@
     setting device read-only
     mounting filesystem that needs recovery on a read-only device:
     mount: device write-protected, mounting read-only
    -mount: cannot mount device read-only
     unmounting read-only filesystem
    -umount: SCRATCH_DEV: not mounted
     mounting filesystem with -o norecovery on a read-only device:
    ...
    (Run 'diff -u /home/adam/xfstests/tests/generic/050.out /home/adam/xfstests/results//generic/050.out.bad'  to see the entire diff)
Ran: generic/050

[CAUSE]
The test case generic/050 has several different golden output depending
on the fs features.

For fses which requires data write (e.g. replay the journal) during
mount, mounting a read-only block device should fail.
And that is the default golden output.

However there are cases that the fs doesn't need to write anything, e.g.
the fs has no metadata journal support at all, or the fs has no dirty
journal to replay.

In that case, there is the generic/050.nojournal golden output.

The test case is using the helper _has_metadata_journaling() to
determine the feature.

Unfortunately that helper doesn't support btrfs, thus the default
behavior is to assume the fs has journal to replay, thus for btrfs it
always assume there is a journal to replay and results the wrong golden
output to be used.

[FIX]
Add btrfs specific log tree check into _has_metadata_journaling(), so
that if there is no log tree to replay, expose the "nojournal" feature
correctly to pass generic/050.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 common/rc | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/common/rc b/common/rc
index 2d8e7167..a78b779a 100644
--- a/common/rc
+++ b/common/rc
@@ -4283,6 +4283,14 @@ _has_metadata_journaling()
 			return 1
 		fi
 		;;
+	btrfs)
+		_require_btrfs_command inspect-internal dump-super
+		if "$BTRFS_UTIL_PROG" inspect-internal dump-super $dev | \
+		   grep -q "log_root\s\+0" ; then
+			echo "$FSTYPE on $dev has empty log tree"
+			return 1
+		fi
+		;;
 	*)
 		# by default we pass; if you need to, add your fs above!
 		;;
-- 
2.50.0

Re: [PATCH] common: add proper btrfs log tree detection to _has_metadata_journaling

[Qu Wenruo]

在 2025/6/30 10:32, Qu Wenruo 写道:
> [BUG]
> With the incoming btrfs shutdown ioctl/remove_bdev callback support,
> btrfs can be tested with the shutdown group.

Forgot to mention, if someone is going to test btrfs with shutdown 
support, here is the branch:

https://github.com/adam900710/linux.git shutdown

Thanks,
Qu

> 
> But test case generic/050 still fails on btrfs with shutdown support:
> 
> generic/050 1s ... - output mismatch (see /home/adam/xfstests/results//generic/050.out.bad)
>      --- tests/generic/050.out	2022-05-11 11:25:30.763333331 +0930
>      +++ /home/adam/xfstests/results//generic/050.out.bad	2025-06-30 10:22:21.752068622 +0930
>      @@ -13,9 +13,7 @@
>       setting device read-only
>       mounting filesystem that needs recovery on a read-only device:
>       mount: device write-protected, mounting read-only
>      -mount: cannot mount device read-only
>       unmounting read-only filesystem
>      -umount: SCRATCH_DEV: not mounted
>       mounting filesystem with -o norecovery on a read-only device:
>      ...
>      (Run 'diff -u /home/adam/xfstests/tests/generic/050.out /home/adam/xfstests/results//generic/050.out.bad'  to see the entire diff)
> Ran: generic/050
> 
> [CAUSE]
> The test case generic/050 has several different golden output depending
> on the fs features.
> 
> For fses which requires data write (e.g. replay the journal) during
> mount, mounting a read-only block device should fail.
> And that is the default golden output.
> 
> However there are cases that the fs doesn't need to write anything, e.g.
> the fs has no metadata journal support at all, or the fs has no dirty
> journal to replay.
> 
> In that case, there is the generic/050.nojournal golden output.
> 
> The test case is using the helper _has_metadata_journaling() to
> determine the feature.
> 
> Unfortunately that helper doesn't support btrfs, thus the default
> behavior is to assume the fs has journal to replay, thus for btrfs it
> always assume there is a journal to replay and results the wrong golden
> output to be used.
> 
> [FIX]
> Add btrfs specific log tree check into _has_metadata_journaling(), so
> that if there is no log tree to replay, expose the "nojournal" feature
> correctly to pass generic/050.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>   common/rc | 8 ++++++++
>   1 file changed, 8 insertions(+)
> 
> diff --git a/common/rc b/common/rc
> index 2d8e7167..a78b779a 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -4283,6 +4283,14 @@ _has_metadata_journaling()
>   			return 1
>   		fi
>   		;;
> +	btrfs)
> +		_require_btrfs_command inspect-internal dump-super
> +		if "$BTRFS_UTIL_PROG" inspect-internal dump-super $dev | \
> +		   grep -q "log_root\s\+0" ; then
> +			echo "$FSTYPE on $dev has empty log tree"
> +			return 1
> +		fi
> +		;;
>   	*)
>   		# by default we pass; if you need to, add your fs above!
>   		;;

Re: [PATCH] common: add proper btrfs log tree detection to _has_metadata_journaling

[Zorro Lang]

On Mon, Jun 30, 2025 at 10:32:53AM +0930, Qu Wenruo wrote:
> [BUG]
> With the incoming btrfs shutdown ioctl/remove_bdev callback support,
> btrfs can be tested with the shutdown group.
> 
> But test case generic/050 still fails on btrfs with shutdown support:
> 
> generic/050 1s ... - output mismatch (see /home/adam/xfstests/results//generic/050.out.bad)
>     --- tests/generic/050.out	2022-05-11 11:25:30.763333331 +0930
>     +++ /home/adam/xfstests/results//generic/050.out.bad	2025-06-30 10:22:21.752068622 +0930
>     @@ -13,9 +13,7 @@
>      setting device read-only
>      mounting filesystem that needs recovery on a read-only device:
>      mount: device write-protected, mounting read-only
>     -mount: cannot mount device read-only
>      unmounting read-only filesystem
>     -umount: SCRATCH_DEV: not mounted
>      mounting filesystem with -o norecovery on a read-only device:
>     ...
>     (Run 'diff -u /home/adam/xfstests/tests/generic/050.out /home/adam/xfstests/results//generic/050.out.bad'  to see the entire diff)
> Ran: generic/050
> 
> [CAUSE]
> The test case generic/050 has several different golden output depending
> on the fs features.
> 
> For fses which requires data write (e.g. replay the journal) during
> mount, mounting a read-only block device should fail.
> And that is the default golden output.
> 
> However there are cases that the fs doesn't need to write anything, e.g.
> the fs has no metadata journal support at all, or the fs has no dirty
> journal to replay.
> 
> In that case, there is the generic/050.nojournal golden output.
> 
> The test case is using the helper _has_metadata_journaling() to
> determine the feature.
> 
> Unfortunately that helper doesn't support btrfs, thus the default
> behavior is to assume the fs has journal to replay, thus for btrfs it
> always assume there is a journal to replay and results the wrong golden
> output to be used.
> 
> [FIX]
> Add btrfs specific log tree check into _has_metadata_journaling(), so
> that if there is no log tree to replay, expose the "nojournal" feature
> correctly to pass generic/050.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  common/rc | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/common/rc b/common/rc
> index 2d8e7167..a78b779a 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -4283,6 +4283,14 @@ _has_metadata_journaling()
>  			return 1
>  		fi
>  		;;
> +	btrfs)
> +		_require_btrfs_command inspect-internal dump-super
> +		if "$BTRFS_UTIL_PROG" inspect-internal dump-super $dev | \
> +		   grep -q "log_root\s\+0" ; then
> +			echo "$FSTYPE on $dev has empty log tree"
                               ^^^^^^
                               FSTYP




> +			return 1
> +		fi
> +		;;
>  	*)
>  		# by default we pass; if you need to, add your fs above!
>  		;;
> -- 
> 2.50.0
> 
> 

Re: [PATCH] common: add proper btrfs log tree detection to _has_metadata_journaling

[Qu Wenruo]

在 2025/7/2 00:26, Zorro Lang 写道:
> On Mon, Jun 30, 2025 at 10:32:53AM +0930, Qu Wenruo wrote:
>> [BUG]
>> With the incoming btrfs shutdown ioctl/remove_bdev callback support,
>> btrfs can be tested with the shutdown group.
>>
>> But test case generic/050 still fails on btrfs with shutdown support:
>>
>> generic/050 1s ... - output mismatch (see /home/adam/xfstests/results//generic/050.out.bad)
>>      --- tests/generic/050.out	2022-05-11 11:25:30.763333331 +0930
>>      +++ /home/adam/xfstests/results//generic/050.out.bad	2025-06-30 10:22:21.752068622 +0930
>>      @@ -13,9 +13,7 @@
>>       setting device read-only
>>       mounting filesystem that needs recovery on a read-only device:
>>       mount: device write-protected, mounting read-only
>>      -mount: cannot mount device read-only
>>       unmounting read-only filesystem
>>      -umount: SCRATCH_DEV: not mounted
>>       mounting filesystem with -o norecovery on a read-only device:
>>      ...
>>      (Run 'diff -u /home/adam/xfstests/tests/generic/050.out /home/adam/xfstests/results//generic/050.out.bad'  to see the entire diff)
>> Ran: generic/050
>>
>> [CAUSE]
>> The test case generic/050 has several different golden output depending
>> on the fs features.
>>
>> For fses which requires data write (e.g. replay the journal) during
>> mount, mounting a read-only block device should fail.
>> And that is the default golden output.
>>
>> However there are cases that the fs doesn't need to write anything, e.g.
>> the fs has no metadata journal support at all, or the fs has no dirty
>> journal to replay.
>>
>> In that case, there is the generic/050.nojournal golden output.
>>
>> The test case is using the helper _has_metadata_journaling() to
>> determine the feature.
>>
>> Unfortunately that helper doesn't support btrfs, thus the default
>> behavior is to assume the fs has journal to replay, thus for btrfs it
>> always assume there is a journal to replay and results the wrong golden
>> output to be used.
>>
>> [FIX]
>> Add btrfs specific log tree check into _has_metadata_journaling(), so
>> that if there is no log tree to replay, expose the "nojournal" feature
>> correctly to pass generic/050.
>>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>>   common/rc | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/common/rc b/common/rc
>> index 2d8e7167..a78b779a 100644
>> --- a/common/rc
>> +++ b/common/rc
>> @@ -4283,6 +4283,14 @@ _has_metadata_journaling()
>>   			return 1
>>   		fi
>>   		;;
>> +	btrfs)
>> +		_require_btrfs_command inspect-internal dump-super
>> +		if "$BTRFS_UTIL_PROG" inspect-internal dump-super $dev | \
>> +		   grep -q "log_root\s\+0" ; then
>> +			echo "$FSTYPE on $dev has empty log tree"
>                                 ^^^^^^
>                                 FSTYP

Despite the typo, this change is causing other test cases to be skipped.

As _require_metadata_journaling() relies on _has_metadata_journaling() 
to check if the fs has metadata journal support.

I'll need to find a way to tell the test case that even a fs support 
metadata journaling, the fs can still be mounted RO when there is no 
dirty journal.

This will be more complex than just modifying _has_metadata_journaling().

Thanks,
Qu

> 
> 
> 
> 
>> +			return 1
>> +		fi
>> +		;;
>>   	*)
>>   		# by default we pass; if you need to, add your fs above!
>>   		;;
>> -- 
>> 2.50.0
>>
>>
> 
>