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

From: Josef Bacik <josef@toxicpanda.com>
To: Qu Wenruo <quwenruo.btrfs@gmx.com>,
	linux-btrfs@vger.kernel.org, kernel-team@fb.com
Subject: Re: [PATCH 3/5] btrfs: introduce rescue=ignorebadroots
Date: Mon, 28 Sep 2020 14:24:41 -0400	[thread overview]
Message-ID: <355c3b88-1bb6-e79f-b06a-6aa684c3ed67@toxicpanda.com> (raw)
In-Reply-To: <b6710997-5150-d082-e260-9fbbaee74e4c@gmx.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