[PATCH 0/4] btrfs-progs: map-logical: remove the extent check

[PATCH 0/4] btrfs-progs: map-logical: remove the extent check

[Qu Wenruo]

Commit eb81f8d263ce ("btrfs-progs: map-logical: Rework map-logical
logics") introduced a strict extent item check for btrfs-map-logical.

It not only makes it harder to use btrfs-map-logical, but also prevents
btrfs-map-logical to handle fs with corrupted extent tree.

This patchset will address the behavior by completely removing the
extent item check, and allow btrfs-map-logical to work on fs with
corrupted extent tree.

Since we're here, also change the default mapping length to sectorsize,
and reject any unaligned logical/bytes pair.

Qu Wenruo (4):
  btrfs-progs: map-logical: use sectorsize as default size
  btrfs-progs: map-logical: reject unaligned logical/bytes pair
  btrfs-progs: map-logical: loosen the required trees to open the
    filesystem
  btrfs-progs: map-logical: remove the extent item check

 btrfs-map-logical.c | 158 +++++---------------------------------------
 1 file changed, 17 insertions(+), 141 deletions(-)

-- 
2.32.0

[PATCH 1/4] btrfs-progs: map-logical: use sectorsize as default size

[Qu Wenruo]

Originally btrfs-map-logical is utilizing function debug_read_block(),
which pretents to read an extent buffer from disk, thus it uses nodesize
as default mapping size.

But after commit eb81f8d263ce ("btrfs-progs: map-logical: Rework
map-logical logics") it no longer requires an extent buffer to do the
mapping.

Thus the default mapping size is no longer needed to be nodesize.

Thus change the default size to sectorsize.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 btrfs-map-logical.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/btrfs-map-logical.c b/btrfs-map-logical.c
index b35677730374..eff0b89dbec6 100644
--- a/btrfs-map-logical.c
+++ b/btrfs-map-logical.c
@@ -288,7 +288,7 @@ int main(int argc, char **argv)
 	}
 
 	if (bytes == 0)
-		bytes = root->fs_info->nodesize;
+		bytes = root->fs_info->sectorsize;
 	cur_logical = logical;
 	cur_len = bytes;
 
-- 
2.32.0

[PATCH 2/4] btrfs-progs: map-logical: reject unaligned logical/bytes pair

[Qu Wenruo]

Btrfs filesystem is a block filesystem, there is no sense to support
unaligned logical/bytes pair.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 btrfs-map-logical.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/btrfs-map-logical.c b/btrfs-map-logical.c
index eff0b89dbec6..21f00fa20ce8 100644
--- a/btrfs-map-logical.c
+++ b/btrfs-map-logical.c
@@ -289,6 +289,16 @@ int main(int argc, char **argv)
 
 	if (bytes == 0)
 		bytes = root->fs_info->sectorsize;
+
+	if (!IS_ALIGNED(logical, root->fs_info->sectorsize) ||
+	    !IS_ALIGNED(bytes, root->fs_info->sectorsize)) {
+		ret = -EINVAL;
+		error(
+	"invalid logical/bytes, both need to aligned to %u, have %llu and %llu",
+			root->fs_info->sectorsize, logical, bytes);
+		goto close;
+	}
+
 	cur_logical = logical;
 	cur_len = bytes;
 
-- 
2.32.0

Re: [PATCH 2/4] btrfs-progs: map-logical: reject unaligned logical/bytes pair

[Nikolay Borisov]

On 12.08.21 г. 8:35, Qu Wenruo wrote:
> Btrfs filesystem is a block filesystem, there is no sense to support
> unaligned logical/bytes pair.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  btrfs-map-logical.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/btrfs-map-logical.c b/btrfs-map-logical.c
> index eff0b89dbec6..21f00fa20ce8 100644
> --- a/btrfs-map-logical.c
> +++ b/btrfs-map-logical.c
> @@ -289,6 +289,16 @@ int main(int argc, char **argv)
>  
>  	if (bytes == 0)
>  		bytes = root->fs_info->sectorsize;
> +
> +	if (!IS_ALIGNED(logical, root->fs_info->sectorsize) ||
> +	    !IS_ALIGNED(bytes, root->fs_info->sectorsize)) {
> +		ret = -EINVAL;
> +		error(
> +	"invalid logical/bytes, both need to aligned to %u, have %llu and %llu",
> +			root->fs_info->sectorsize, logical, bytes);

In order to be more graceful I'd say print this as a warning and do the
alignment automatically, informing the user this has been performed.

> +		goto close;
> +	}
> +
>  	cur_logical = logical;
>  	cur_len = bytes;
>  
> 

[PATCH 3/4] btrfs-progs: map-logical: loosen the required trees to open the filesystem

[Qu Wenruo]

For btrfs-map-logical, it only requires chunk tree to do the
logicla->physical mapping.

All othe trees are not really needed.

Loosen the required trees to make btrfs-map-logical to work better on
corrupted fs.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 btrfs-map-logical.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/btrfs-map-logical.c b/btrfs-map-logical.c
index 21f00fa20ce8..9f119d08bad8 100644
--- a/btrfs-map-logical.c
+++ b/btrfs-map-logical.c
@@ -261,7 +261,8 @@ int main(int argc, char **argv)
 	radix_tree_init();
 	cache_tree_init(&root_cache);
 
-	root = open_ctree(dev, 0, 0);
+	root = open_ctree(dev, 0, OPEN_CTREE_PARTIAL |
+				  OPEN_CTREE_NO_BLOCK_GROUPS);
 	if (!root) {
 		fprintf(stderr, "Open ctree failed\n");
 		free(output_file);
-- 
2.32.0

Re: [PATCH 3/4] btrfs-progs: map-logical: loosen the required trees to open the filesystem

[Nikolay Borisov]

On 12.08.21 г. 8:35, Qu Wenruo wrote:
> For btrfs-map-logical, it only requires chunk tree to do the
> logicla->physical mapping.
> 
> All othe trees are not really needed.
> 
> Loosen the required trees to make btrfs-map-logical to work better on
> corrupted fs.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  btrfs-map-logical.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/btrfs-map-logical.c b/btrfs-map-logical.c
> index 21f00fa20ce8..9f119d08bad8 100644
> --- a/btrfs-map-logical.c
> +++ b/btrfs-map-logical.c
> @@ -261,7 +261,8 @@ int main(int argc, char **argv)
>  	radix_tree_init();
>  	cache_tree_init(&root_cache);
>  
> -	root = open_ctree(dev, 0, 0);
> +	root = open_ctree(dev, 0, OPEN_CTREE_PARTIAL |
> +				  OPEN_CTREE_NO_BLOCK_GROUPS);

nit: OPEN_CTREE_PARTIAL implies ignoring error when opening the csum and
extent tress, since those trees are opened via
setup_root_or_create_block. This tells me that
OPEN_CTREE_NO_BLOCK_GROUPS is redundant and can be removed and replaced
with simply OPEN_CTREE_PARTIAL.

>  	if (!root) {
>  		fprintf(stderr, "Open ctree failed\n");
>  		free(output_file);
> 

[PATCH 4/4] btrfs-progs: map-logical: remove the extent item check

[Qu Wenruo]

Tool btrfs-map-logical is really only doing logical -> physical mapping,
mostly utilized by developers or experienced users.

There is really no need to check whether the specified range has an
extent or not.

In fact the extent check behavior is a big blockage for corrupted fs as
such fs can have corrupted extent tree, doing an extent item search can
lead to -EIO error.

This patch will just remove the extent item check, allowing users to do
any logical -> physical mapping lookup as long as there is a chunk.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 btrfs-map-logical.c | 145 ++------------------------------------------
 1 file changed, 5 insertions(+), 140 deletions(-)

diff --git a/btrfs-map-logical.c b/btrfs-map-logical.c
index 9f119d08bad8..96263013a5c6 100644
--- a/btrfs-map-logical.c
+++ b/btrfs-map-logical.c
@@ -38,65 +38,6 @@
  * */
 static FILE *info_file;
 
-static int map_one_extent(struct btrfs_fs_info *fs_info,
-			  u64 *logical_ret, u64 *len_ret, int search_forward)
-{
-	struct btrfs_path *path;
-	struct btrfs_key key;
-	u64 logical;
-	u64 len = 0;
-	int ret = 0;
-
-	BUG_ON(!logical_ret);
-	logical = *logical_ret;
-
-	path = btrfs_alloc_path();
-	if (!path)
-		return -ENOMEM;
-
-	key.objectid = logical;
-	key.type = 0;
-	key.offset = 0;
-
-	ret = btrfs_search_slot(NULL, fs_info->extent_root, &key, path,
-				0, 0);
-	if (ret < 0)
-		goto out;
-	BUG_ON(ret == 0);
-	ret = 0;
-
-again:
-	btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]);
-	if ((search_forward && key.objectid < logical) ||
-	    (!search_forward && key.objectid > logical) ||
-	    (key.type != BTRFS_EXTENT_ITEM_KEY &&
-	     key.type != BTRFS_METADATA_ITEM_KEY)) {
-		if (!search_forward)
-			ret = btrfs_previous_extent_item(fs_info->extent_root,
-							 path, 0);
-		else
-			ret = btrfs_next_extent_item(fs_info->extent_root,
-						     path, 0);
-		if (ret)
-			goto out;
-		goto again;
-	}
-	logical = key.objectid;
-	if (key.type == BTRFS_METADATA_ITEM_KEY)
-		len = fs_info->nodesize;
-	else
-		len = key.offset;
-
-out:
-	btrfs_free_path(path);
-	if (!ret) {
-		*logical_ret = logical;
-		if (len_ret)
-			*len_ret = len;
-	}
-	return ret;
-}
-
 static int __print_mapping_info(struct btrfs_fs_info *fs_info, u64 logical,
 				u64 len, int mirror_num)
 {
@@ -134,16 +75,6 @@ static int __print_mapping_info(struct btrfs_fs_info *fs_info, u64 logical,
 	return ret;
 }
 
-/*
- * Logical and len is the exact value of a extent.
- * And offset is the offset inside the extent. It's only used for case
- * where user only want to print part of the extent.
- *
- * Caller *MUST* ensure the range [logical,logical+len) are in one extent.
- * Or we can encounter the following case, causing a -ENOENT error:
- * |<-----given parameter------>|
- *		|<------ Extent A ----->|
- */
 static int print_mapping_info(struct btrfs_fs_info *fs_info, u64 logical,
 			      u64 len)
 {
@@ -213,10 +144,7 @@ int main(int argc, char **argv)
 	u64 copy = 0;
 	u64 logical = 0;
 	u64 bytes = 0;
-	u64 cur_logical = 0;
-	u64 cur_len = 0;
 	int out_fd = -1;
-	int found = 0;
 	int ret = 0;
 
 	while(1) {
@@ -300,76 +228,13 @@ int main(int argc, char **argv)
 		goto close;
 	}
 
-	cur_logical = logical;
-	cur_len = bytes;
-
-	/* First find the nearest extent */
-	ret = map_one_extent(root->fs_info, &cur_logical, &cur_len, 0);
-	if (ret < 0) {
-		errno = -ret;
-		fprintf(stderr, "Failed to find extent at [%llu,%llu): %m\n",
-			cur_logical, cur_logical + cur_len);
+	ret = print_mapping_info(root->fs_info, logical, bytes);
+	if (ret < 0)
 		goto out_close_fd;
-	}
-	/*
-	 * Normally, search backward should be OK, but for special case like
-	 * given logical is quite small where no extents are before it,
-	 * we need to search forward.
-	 */
-	if (ret > 0) {
-		ret = map_one_extent(root->fs_info, &cur_logical, &cur_len, 1);
-		if (ret < 0) {
-			errno = -ret;
-			fprintf(stderr,
-				"Failed to find extent at [%llu,%llu): %m\n",
-				cur_logical, cur_logical + cur_len);
-			goto out_close_fd;
-		}
-		if (ret > 0) {
-			fprintf(stderr,
-				"Failed to find any extent at [%llu,%llu)\n",
-				cur_logical, cur_logical + cur_len);
-			goto out_close_fd;
-		}
-	}
-
-	while (cur_logical + cur_len >= logical && cur_logical < logical +
-	       bytes) {
-		u64 real_logical;
-		u64 real_len;
+	if (output_file && out_fd != -1)
+		ret = write_extent_content(root->fs_info, out_fd, logical,
+					   bytes, copy);
 
-		found = 1;
-		ret = map_one_extent(root->fs_info, &cur_logical, &cur_len, 1);
-		if (ret < 0)
-			goto out_close_fd;
-		if (ret > 0)
-			break;
-		/* check again if there is overlap. */
-		if (cur_logical + cur_len < logical ||
-		    cur_logical >= logical + bytes)
-			break;
-
-		real_logical = max(logical, cur_logical);
-		real_len = min(logical + bytes, cur_logical + cur_len) -
-			   real_logical;
-
-		ret = print_mapping_info(root->fs_info, real_logical, real_len);
-		if (ret < 0)
-			goto out_close_fd;
-		if (output_file && out_fd != -1) {
-			ret = write_extent_content(root->fs_info, out_fd,
-					real_logical, real_len, copy);
-			if (ret < 0)
-				goto out_close_fd;
-		}
-
-		cur_logical += cur_len;
-	}
-
-	if (!found) {
-		fprintf(stderr, "No extent found at range [%llu,%llu)\n",
-			logical, logical + bytes);
-	}
 out_close_fd:
 	if (output_file && out_fd != 1)
 		close(out_fd);
-- 
2.32.0