Re: [PATCH v3] btrfs: Do super block verification before writing it to disk

[Qu Wenruo <wqu@suse.com>]

[-- Attachment #1.1: Type: text/plain, Size: 1588 bytes --]



On 2018年04月19日 06:04, David Sterba wrote:
> On Tue, Apr 17, 2018 at 09:47:19AM +0800, Qu Wenruo wrote:
>> @@ -2680,7 +2681,7 @@ int open_ctree(struct super_block *sb,
>>  
>>  	memcpy(fs_info->fsid, fs_info->super_copy->fsid, BTRFS_FSID_SIZE);
>>  
>> -	ret = btrfs_check_super_valid(fs_info);
>> +	ret = btrfs_check_super_valid(fs_info, fs_info->super_copy);
>>  	if (ret) {
>>  		btrfs_err(fs_info, "superblock contains fatal errors");
>>  		err = -EINVAL;
>> @@ -3310,6 +3311,27 @@ static int write_dev_supers(struct btrfs_device *device,
> 
> This is in write_dev_supers, so the superblock is checked
> number-of-devices times. The caller write_all_supers rewrites the device
> item so it matches the device it's going to write to. But,
> btrfs_check_super_valid does not validate the dev_item so all the
> validation does not bring much benefit, as it repeatedly checks the same
> data.
> 
> So, what if the validation is done only once in write_all_supers? Lock
> the devices, validate, if it fails, report that and unlock devices and
> go readonly.

Makes sense.

I'll update btrfs_check_super_valid() to cooperate with that in next update.

Thanks,
Qu

> 
> There's a differnce to what you implemented: if the in-memory superblock
> corruption happens between writing to the devices, there are some left
> with the new superblock and some with the old.
> 
> Although this sounds quite improbable, I think that doing the check in
> advance would save some trouble if that happens. The superblocks on all
> devices will match.
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]