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

Qu Wenruo
2025-07-02T04:21:06Z

在 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
>>
>>
>
>