Re: [PATCH 3/4] btrfs: introduce new "rescue=ignoremetacsums" mount option

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

在 2024/6/13 05:08, David Sterba 写道:
> On Tue, Jun 11, 2024 at 02:51:37PM +0930, Qu Wenruo wrote:
>> This patch introduces "rescue=ignoremetacsums" to ignore metadata csums,
>> meanwhile all the other metadata sanity checks are still kept as is.
>>
>> This new mount option is mostly to allow the kernel to mount an
>> interrupted checksum conversion (at the metadata csum overwrite stage).
>>
>> And since the main part of metadata sanity checks is inside
>> tree-checker, we shouldn't lose much safety, and the new mount option is
>> rescue mount option it requires full read-only mount.
>>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>
>> --- a/fs/btrfs/disk-io.c
>> +++ b/fs/btrfs/disk-io.c
>> @@ -367,6 +367,7 @@ int btrfs_validate_extent_buffer(struct extent_buffer *eb,
>>   	u8 result[BTRFS_CSUM_SIZE];
>>   	const u8 *header_csum;
>>   	int ret = 0;
>> +	bool ignore_csum = btrfs_test_opt(fs_info, IGNOREMETACSUMS);
>
> const
>
>> --- a/fs/btrfs/messages.c
>> +++ b/fs/btrfs/messages.c
>> @@ -20,7 +20,7 @@ static const char fs_state_chars[] = {
>>   	[BTRFS_FS_STATE_TRANS_ABORTED]		= 'A',
>>   	[BTRFS_FS_STATE_DEV_REPLACING]		= 'R',
>>   	[BTRFS_FS_STATE_DUMMY_FS_INFO]		= 0,
>> -	[BTRFS_FS_STATE_NO_CSUMS]		= 'C',
>> +	[BTRFS_FS_STATE_NO_DATA_CSUMS]		= 'C',
>
> There should be the status also when the metadata checksums are not
> validated, the letters are arbitrary but should reflect the state if
> possible, I'd suggest to use 'S' here.

I'd prefer to change the NO_DATA_CSUMS one to use 'D' or 'd' (for data),
meanwhile for metadata we go 'M' or 'm'.

But on the other hand, I do not think data/meta csum ignoring really
deserves a dedicated state char.

It's not really that special compared to trans aborted or dummy fs.
(The same for dev-replacing)

I can add it for now, but I believe we need some better discussion on
the fs_state_chars.

>
> What I'm missing is the sysfs.c update, all options supported by rescue
> need to be listed in rescue_opts.
>
That would be updated in the next update.

Thanks,
Qu