Re: [PATCH u-boot] fs: btrfs: do not fail when offset of a ROOT_ITEM is not -1

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

On 2021/2/10 上午1:33, Marek Behún wrote:
> When the btrfs_read_fs_root() function is searching a ROOT_ITEM with
> location key offset other than -1, it currently fails via BUG_ON.
>
> The offset can have other value than -1, though. This can happen for
> example if a subvolume is renamed:
>
>    $ btrfs subvolume create X && sync
>    Create subvolume './X'
>    $ btrfs inspect-internal dump-tree /dev/root | grep -B 2 'name: X$
>          location key (270 ROOT_ITEM 18446744073709551615) type DIR
>          transid 283 data_len 0 name_len 1
>          name: X
>    $ mv X Y && sync
>    $ btrfs inspect-internal dump-tree /dev/root | grep -B 2 'name: Y$
>          location key (270 ROOT_ITEM 0) type DIR
>          transid 285 data_len 0 name_len 1
>          name: Y
>
> As can be seen the offset changed from -1ULL to 0.


Offset for subvolume ROOT_ITEM can be other values, especially for
snapshot that offset is the transid when it get created.

But the problem is, if we call btrfs_read_fs_root() for subvolume tree,
the offset of the key really doesn't matter, the only important thing is
the objectid.

Thus we use that BUG_ON() to catch careless callers.

Would you please provide a case where we wrongly call
btrfs_read_fs_root() with incorrect offset inside btrfs-progs/uboot?

I believe that would be the proper way to fix.

Thanks,
Qu
>
> Do not fail in this case.
>
> Signed-off-by: Marek Behún <marek.behun@nic.cz>
> Cc: David Sterba <dsterba@suse.com>
> Cc: Qu Wenruo <wqu@suse.com>
> Cc: Tom Rini <trini@konsulko.com>
> ---
>   fs/btrfs/disk-io.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index b332ecb796..c6fdec95c1 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -732,8 +732,7 @@ struct btrfs_root *btrfs_read_fs_root(struct btrfs_fs_info *fs_info,
>   		return fs_info->chunk_root;
>   	if (location->objectid == BTRFS_CSUM_TREE_OBJECTID)
>   		return fs_info->csum_root;
> -	BUG_ON(location->objectid == BTRFS_TREE_RELOC_OBJECTID ||
> -	       location->offset != (u64)-1);
> +	BUG_ON(location->objectid == BTRFS_TREE_RELOC_OBJECTID);
>
>   	node = rb_search(&fs_info->fs_root_tree, (void *)&objectid,
>   			 btrfs_fs_roots_compare_objectids, NULL);
>