Re: [PATCH] btrfs-progs: dump-tree: add extra chunk type checks

[Sun YangKai <sunk67188@gmail.com>]

On 2026/2/23 06:14, Qu Wenruo wrote:
> There is a report in the mailing list where a seemingly confused end
> user is passing a physical bytenr into "btrfs inspect dump-tree".
> 
> The best case is that bytenr has no corresponding chunk and will be
> rejected early.
> 
> But if there is a chunk cover that bytenr, and that chunk is a data
> chunk, "btrfs ins dump-tree" will mostly result a checksum mismatch,
> e.g.:
> 
>   $ btrfs ins dump-tree -b 13631488 ~/test.img
>   btrfs-progs v6.19
>   checksum verify failed on 13631488 wanted 0xcdcdcdcd found 0xf9d338a0
>   ERROR: failed to read tree block 13631488
> 
> Note that 13631488 is the logical bytenr of the first data chunk.
> 
> This can be confusing for end users, as they won't know if it's really
> some corrupted tree blocks, or they are just passing wrong numbers.
> 
> Enhance this by introducing a new check_metadata_logical() helper, to
> make sure a chunk exists and the type is also correct.
> 
> Now the error would be more obvious to end users:
> 
>   $ ./btrfs ins dump-tree -b 13631488 ~/test.img
>   btrfs-progs v6.19
>   ERROR: logical 13631488 is not in a metadata chunk, found chunk 13631488 len 8388608 flags 0x1

Would it make sense to use something like `type DATA` instead of
`flags 0x1` in the error message? I believe this would be more helpful 
for users.

Thanks,
Sun Yangkai

> 
> Link: https://lore.kernel.org/linux-btrfs/77535c20-da3e-4dfb-b3d7-9426c25b5da3@free.fr/
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>   cmds/inspect-dump-tree.c | 24 ++++++++++++++++++++++++
>   1 file changed, 24 insertions(+)
> 
> diff --git a/cmds/inspect-dump-tree.c b/cmds/inspect-dump-tree.c
> index 2bc3f68179a5..11eb950dbdcb 100644
> --- a/cmds/inspect-dump-tree.c
> +++ b/cmds/inspect-dump-tree.c
> @@ -33,6 +33,7 @@
>   #include "kernel-shared/print-tree.h"
>   #include "kernel-shared/extent_io.h"
>   #include "kernel-shared/tree-checker.h"
> +#include "kernel-shared/volumes.h"
>   #include "common/defs.h"
>   #include "common/extent-cache.h"
>   #include "common/messages.h"
> @@ -194,6 +195,26 @@ static int dump_add_tree_block(struct cache_tree *tree, u64 bytenr)
>   	return ret;
>   }
>   
> +static int check_metadata_logical(struct btrfs_fs_info *fs_info, u64 logical)
> +{
> +	struct cache_extent *ce;
> +	struct map_lookup *map;
> +
> +	ce = search_cache_extent(&fs_info->mapping_tree.cache_tree, logical);
> +	if (!ce || ce->start > logical) {
> +		error("no chunk map found for logical %llu", logical);
> +		return -ENOENT;
> +	}
> +	map = container_of(ce, struct map_lookup, ce);
> +	if (!(map->type & BTRFS_BLOCK_GROUP_METADATA)) {
> +		error(
> +"logical %llu is not in a metadata chunk, found chunk %llu len %llu flags 0x%llx",
> +		      logical, ce->start, ce->size, map->type);
> +		return -EINVAL;
> +	}
> +	return 0;
> +}
> +
>   /*
>    * Print all tree blocks recorded.
>    * All tree block bytenr record will also be freed in this function.
> @@ -228,6 +249,9 @@ static int dump_print_tree_blocks(struct btrfs_fs_info *fs_info,
>   			goto next;
>   		}
>   
> +		ret = check_metadata_logical(fs_info, bytenr);
> +		if (ret < 0)
> +			goto next;
>   		eb = read_tree_block(fs_info, bytenr, &check);
>   		if (!extent_buffer_uptodate(eb)) {
>   			error("failed to read tree block %llu", bytenr);