Re: [PATCH] fstests: btrfs: new test case for leaking BTRFS_FS_STATE_REMOUNTING flag

在 2026/7/7 19:14, Anand Suveer Jain 写道:
> On 4/7/26 16:52, Qu Wenruo wrote:
>> This is a regression test for the patch "btrfs: fix leaking
>> BTRFS_FS_STATE_REMOUNTING flag", where conflicting mount options can
>> cause btrfs leave BTRFS_FS_STATE_REMOUNTING set.
>>
>> Such set BTRFS_FS_STATE_REMOUNTING will interrupt several features, and
>> the test case will use "btrfs qgroup rescan" to catch the
>> BTRFS_FS_STATE_REMOUNTING flag.
>>
>> Signed-off-by: Qu Wenruo <[email protected]>
>> ---
>>   tests/btrfs/352     | 53 +++++++++++++++++++++++++++++++++++++++++++++
>>   tests/btrfs/352.out |  2 ++
>>   2 files changed, 55 insertions(+)
>>   create mode 100755 tests/btrfs/352
>>   create mode 100644 tests/btrfs/352.out
>>
>> diff --git a/tests/btrfs/352 b/tests/btrfs/352
>> new file mode 100755
>> index 00000000..2ccf7fab
>> --- /dev/null
>> +++ b/tests/btrfs/352
>> @@ -0,0 +1,53 @@
>> +#! /bin/bash
>> +# SPDX-License-Identifier: GPL-2.0
>> +# Copyright (c) 2026 SUSE S.A.  All Rights Reserved.
>> +#
>> +# FS QA Test 352
>> +#
>> +# A regression test to make sure failed remount won't leave
>> +# BTRFS_FS_STATE_REMOUNTING flag set, which can interrupt several features,
>> +# one of them is btrfs qgroup rescan.
>> +#
>> +. ./common/preamble
>> +_begin_fstest auto quick remount
>> +
>> +_fixed_by_kernel_commit XXXXXXXXXXXX \
>> +	"btrfs: fix leaking BTRFS_FS_STATE_REMOUNTING flag"
>> +
> 
> 
> 
> We can replace this section with the existing helpers or,
> even better, with a new one as shown below. I am not too
> particular about creating a new helper, that is up to you.
> However, the use of existing helpers such as
> _require_btrfs_fs_sysfs and _require_btrfs_fs_feature
> will be better. Thanks
> 
> 
>> +rescue_opts="/sys/fs/btrfs/features/supported_rescue_options"
>> +if [ ! -f "$rescue_opts" ]; then
>> +	_notrun "No supported_rescue_options sysfs interface"
>> +fi
>> +
>> +if ! grep -q "ignorebadroots" "$rescue_opts"; then
>> +	_notrun "\"rescue=ignorebadroots\" mount option not supported"
>> +fi
>> +
> 
> 
> diff --git a/common/btrfs b/common/btrfs
> index 30288f07b61a..57453fb39893 100644
> --- a/common/btrfs
> +++ b/common/btrfs
> @@ -100,6 +100,29 @@ _require_btrfs_mkfs_uuid_option()
>   	fi
>   }
> 
> +# Check for the availability of a specific option within a btrfs
> feature set
> +# Arguments:
> +#   $1 - Feature attribute of /sys/fs/btrfs/features/
> +#   $2 - Option string to verify inside that file
> +_require_btrfs_fs_feature_option()
> +{

I'm fine to use the existing _require_btrfs_fs_feature() to detect that 
file.

But I'm not fine of a new _require_btrfs_fs_feature_option().

You have already mentioned the second parameter is optional, then why 
not merge it into the existing _require_btrfs_fs_feature().


Furthermore adding the second option can cause weird situations for the 
existing features.

E.g. just check /sys/fs/btrfs/features/rmdir_subvol, I believe it will 
be 0 on your environment too.

So if we use the optional parameter, there will be something like 
"_require_btrfs_fs_feature_option acl 0", which doesn't looks sane at all.

I'll convert to use _require_btrfs_fs_feature() for 
supported_rescue_options, but still manually test the content for the 
ibadroots option.

Thanks,
Qu
> +	local feat=$1
> +	local feat_type=$2
> +
> +	if [ -z "$feat" ] || [ -z "$feat_type" ]; then
> +		echo "Missing argument(s) for _require_btrfs_fs_feature_type"
> +		_exit 1
> +	fi
> +
> +	modprobe btrfs > /dev/null 2>&1
> +
> +	_require_btrfs_fs_feature "$feat"
> +
> +	if ! grep -qw "$feat_type" "/sys/fs/btrfs/features/$feat"; then
> +		_notrun "Feature type '$feat_type' not supported by the available
> btrfs version"
> +	fi
> +}
> +
>   _require_btrfs_fs_feature()
>   {
>   	if [ -z $1 ]; then
> diff --git a/tests/btrfs/352 b/tests/btrfs/352
> index 2ccf7fabdb71..14e357dbaa37 100755
> --- a/tests/btrfs/352
> +++ b/tests/btrfs/352
> @@ -14,15 +14,8 @@ _begin_fstest auto quick remount
>   _fixed_by_kernel_commit XXXXXXXXXXXX \
>   	"btrfs: fix leaking BTRFS_FS_STATE_REMOUNTING flag"
> 
> -rescue_opts="/sys/fs/btrfs/features/supported_rescue_options"
> -if [ ! -f "$rescue_opts" ]; then
> -	_notrun "No supported_rescue_options sysfs interface"
> -fi
> -
> -if ! grep -q "ignorebadroots" "$rescue_opts"; then
> -	_notrun "\"rescue=ignorebadroots\" mount option not supported"
> -fi
> -
> +_require_btrfs_fs_sysfs
> +_require_btrfs_fs_feature_option "supported_rescue_options"
> "ignorebadroots"
>   _require_btrfs_command quota rescan -w
>   _require_scratch
>   _scratch_mkfs >> $seqres.full
> 
> 
>