Re: [PATCH 3/5] btrfs: introduce rescue=ignorebadroots

[Josef Bacik <josef@toxicpanda.com>]

On 9/24/20 8:47 PM, Qu Wenruo wrote:
> 
> 
> On 2020/9/24 下午11:32, Josef Bacik wrote:
>> In the face of extent root corruption, or any other core fs wide root
>> corruption we will fail to mount the file system.  This makes recovery
>> kind of a pain, because you need to fall back to userspace tools to
>> scrape off data.  Instead provide a mechanism to gracefully handle bad
>> roots, so we can at least mount read-only and possibly recover data from
>> the file system.
>>
>> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> 
> Mostly OK, but still a small problem inlined below.
> [...]
>> index 46f4efd58652..08b3ca60f3df 100644
>> --- a/fs/btrfs/volumes.c
>> +++ b/fs/btrfs/volumes.c
>> @@ -7656,6 +7656,13 @@ int btrfs_verify_dev_extents(struct btrfs_fs_info *fs_info)
>>   	u64 prev_dev_ext_end = 0;
>>   	int ret = 0;
>>   
>> +	/*
>> +	 * We don't have a dev_root because we mounted with ignorebadroots and
>> +	 * failed to load the root, so skip the verification.
>> +	 */
>> +	if (!root)
>> +		return 0;
>> +
> 
> The check itself is mostly for write, to ensure we won't have
> missing/unnecessary dev extents to mess up chunk allocation.
> 
> For RO operations, the check makes little sense, and can be safely
> ignored for ignorebadroots.
> 
> Furthermore this only handles the case where the device tree root is
> corrupted.
> But if only part of the device tree is corrupted, we still continue
> checking and fail to mount.
> 
> It's better to skip the whole check for dev extents if we're using
> ignorebadroots rescue option.
> No matter if the root is corrupted or not.
> 

Yeah good point, I'll fix this up.  Thanks,

Josef