Re: [PATCH] i386-pc: build btrfs zstd support into separate module

[Daniel Kiper <dkiper@net-space.pl>]

On Fri, Sep 10, 2021 at 05:22:22PM +0800, Michael Chang via Grub-devel wrote:
> On Wed, Sep 08, 2021 at 09:37:52PM +0200, Daniel Kiper wrote:
> > On Fri, Sep 03, 2021 at 09:21:39AM +0800, Michael Chang via Grub-devel wrote:
> > > On Thu, Sep 02, 2021 at 02:12:52PM +0200, Daniel Kiper wrote:
> > > > On Thu, Sep 02, 2021 at 01:48:30PM +0800, Michael Chang via Grub-devel wrote:
> > > > > On Wed, Sep 01, 2021 at 06:38:22PM +0200, Daniel Kiper wrote:
> > > > > > On Tue, Aug 31, 2021 at 03:12:28PM +0800, Michael Chang via Grub-devel wrote:
>
> [snip]
>
> > > just that I can't resist to fix problem from our users who opted to use
> > > "btrfs partition" which differs to "short mbr gap" with a lot more user
> > > base and sensible usecases.
> > >
> > > FWIW we want to address this problem, because mbr gap is adjustable via
> > > re-partitioning but btrfs bootloader area is not.
> >
> > Huh! The "btrfs partition" sounds like not resizable MBR gap! I am not
> > happy with it just at the beginning. Anyway, could you explain your use
> > case in more details including example commands and why core.img seems
> > landing in the "btrfs partition". I am especially curious about the
> > latter because I think the "btrfs partition" and things like that was
> > designed for something else, e.g., storing grubenv data. And why this
> > solution is i386 specific?
>
> Installing to btrfs partition is not something exotic to grub, it is
> actually a supported usecase by grub for a long time. Also zfs has
> similar design, see:
>
>   c7ba4f698     Support BtrFS embedding.
>   ba102053c     Support ZFS embedding.
>
> To make it happen, you just have to point the btrfs device to
> grub-install and it will setup and embed image there. That worked quite
> nicely until the zstd support came into play.
>
> On current git head, the command fails with core.img is too large.
>
> # ./grub-install -d ./grub-core /dev/vda2
>   Installing for i386-pc platform.
>   ./grub-install: warning: your core.img is unusually large.  It won't fit in the embedding area.
>   ./grub-install: error: filesystem `btrfs' doesn't support blocklists.
>
> With this patch, the embedding still works but warns the user zstd is
> disabled, also instructing them to use MBR if they need the zstd feature
> to work.
>
> # ./grub-install -d ./grub-core /dev/vda2
> Installing for i386-pc platform.
> ./grub-install: warning: btrfs zstd compression is disabled, please change install device to disk.
> Installation finished. No error reported.
>
> It is very i386-pc centric, as it is often used by legacy mbr boot code
> with which active partition is chainloaded. Some people still thinks
> this is the right thing to do and feels more comfortable this way to
> manage their multiple distrubution booting, compared with os-prober.

I do not like this solution because it is similar to all the patches
trying to solve small MBR problem. Though, after chatting with Btrfs
folks in Oracle, it seems to me there is better solution for this issue.
I think we should make the GRUB installer smarter. If it cannot put the
core image in first 64 KiB before primary superblock it should install
the core image behind the primary superblock, i.e. starting from
64 KiB + 4 KiB but below 1 MiB. Please take a look at [1] which nicely
explains the Btrfs bootloader support. Does it make sense for you?

Daniel

[1] https://btrfs.wiki.kernel.org/index.php/Manpage/btrfs(5)#BOOTLOADER_SUPPORT