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

[David Sterba <dsterba@suse.cz>]

On Fri, Jun 14, 2024 at 06:58:20AM +0930, Qu Wenruo wrote:
> 
> 
> 在 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'.

Changing would break backward compatibility, now it's part of user
visible interface. It's not an ABI or API but at least we should do such
changes without considering the consequences.

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

The idea of the descriptors is to make it visible that the filesystem is
in some unusual state, skipping checksum verification can make a
difference when reading blocks that would normally not pass the check.