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

From: Qu Wenruo <quwenruo.btrfs@gmx.com>
To: Josef Bacik <josef@toxicpanda.com>,
	linux-btrfs@vger.kernel.org, kernel-team@fb.com
Subject: Re: [PATCH 3/5] btrfs: introduce rescue=ignorebadroots
Date: Fri, 25 Sep 2020 08:47:07 +0800	[thread overview]
Message-ID: <b6710997-5150-d082-e260-9fbbaee74e4c@gmx.com> (raw)
In-Reply-To: <b7b5dfb5542c3eb965cd2d8a9baa2999b6bae638.1600961206.git.josef@toxicpanda.com>

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

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.

Thanks,
Qu

>  	key.objectid = 1;
>  	key.type = BTRFS_DEV_EXTENT_KEY;
>  	key.offset = 0;
> 

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