Re: [PATCH v2] fs/btrfs: Make extent item iteration to handle gaps

[Daniel Kiper]

On Thu, Oct 28, 2021 at 05:44:57PM +0800, Qu Wenruo via Grub-devel wrote:
> [BUG]
> Grub btrfs implementation can't handle two very basic btrfs file
> layouts:
>
> 1. Mixed inline/regualr extents
>    # mkfs.btrfs -f test.img
>    # mount test.img /mnt/btrfs
>    # xfs_io -f -c "pwrite 0 1k" -c "sync" -c "falloc 0 4k" \
>              -c "pwrite 4k 4k" /mnt/btrfs/file
>    # umount /mnt/btrfs
>    # ./grub-fstest ./grub-fstest --debug=btrfs ~/test.img hex "/file"
>
>    Such mixed inline/regular extents case is not recommended layout,
>    but all existing tools and kernel can handle it without problem
>
> 2. NO_HOLES feature
>    # mkfs.btrfs -f test.img -O no_holes
>    # mount test.img /mnt/btrfs
>    # xfs_io -f -c "pwrite 0 4k" -c "pwrite 8k 4k" /mnt/btrfs/file
>    # umount /mnt/btrfs
>    # ./grub-fstest ./grub-fstest --debug=btrfs ~/test.img hex "/file"
>
>    NO_HOLES feature is going to be the default mkfs feature in the incoming
>    v5.15 release, and kernel has support for it since v4.0.
>
> [CAUSE]
> The way GRUB btrfs code iterates through file extents relies on no gap
> between extents.
>
> If any gap is hit, then grub btrfs will error out, without any proper
> reason to help debug the bug.
>
> This is a bad assumption, since a long long time ago btrfs has a new
> feature called NO_HOLES to allow btrfs to skip the padding hole extent
> to reduce metadata usage.
>
> The NO_HOLES feature is already stable since kernel v4.0 and is going to
> be the default mkfs feature in the incoming v5.15 btrfs-progs release.
>
> [FIX]
> When there is a extent gap, instead of error out, just try next item.
>
> This is still not ideal, as kernel/progs/U-boot all do the iteration
> item by item, not relying on the file offset continuity.
>
> But it will be way more time consuming to correct the whole behavior
> than starting from scratch to build a proper designed btrfs module for GRUB.
>
> Signed-off-by: Qu Wenruo <wqu@suse.com>

Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Daniel