From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.90_1) id 1mO3OX-0005y4-3l for mharc-grub-devel@gnu.org; Wed, 08 Sep 2021 15:38:05 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:58082) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mO3OV-0005vJ-1b for grub-devel@gnu.org; Wed, 08 Sep 2021 15:38:03 -0400 Received: from dibed.net-space.pl ([84.10.22.86]:33615) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_3DES_EDE_CBC_SHA1:192) (Exim 4.90_1) (envelope-from ) id 1mO3OP-0005lk-T9 for grub-devel@gnu.org; Wed, 08 Sep 2021 15:38:02 -0400 Received: from router-fw.i.net-space.pl ([192.168.52.1]:48698 "EHLO tomti.i.net-space.pl") by router-fw-old.i.net-space.pl with ESMTP id S2098703AbhIHThy (ORCPT ); Wed, 8 Sep 2021 21:37:54 +0200 X-Comment: RFC 2476 MSA function at dibed.net-space.pl logged sender identity as: dkiper Date: Wed, 8 Sep 2021 21:37:52 +0200 From: Daniel Kiper To: Michael Chang Cc: grub-devel@gnu.org Subject: Re: [PATCH] i386-pc: build btrfs zstd support into separate module Message-ID: <20210908193752.u4pkfbkommbjkazq@tomti.i.net-space.pl> References: <20210831071228.8282-1-mchang@suse.com> <20210901163822.5vq766pc2wdjveef@tomti.i.net-space.pl> <20210902054830.GA9354@mazu> <20210902121252.fa3heoxb6afza7tq@tomti.i.net-space.pl> <20210903012139.GA6661@mazu> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210903012139.GA6661@mazu> User-Agent: NeoMutt/20170113 (1.7.2) Received-SPF: pass client-ip=84.10.22.86; envelope-from=dkiper@net-space.pl; helo=dibed.net-space.pl X-Spam_score_int: -18 X-Spam_score: -1.9 X-Spam_bar: - X-Spam_report: (-1.9 / 5.0 requ) BAYES_00=-1.9, SPF_PASS=-0.001, T_SPF_HELO_TEMPERROR=0.01 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 08 Sep 2021 19:38:03 -0000 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: > > > > > The zstd support in btrfs brings significant size increment to the > > > > > on-disk image that it can no longer fit into btrfs bootloader area and > > > > > short mbr gap. > > > > > > > > > > In order to support grub update on outstanding i386-pc setup with these > > > > > size constraints remain in place, here we build the zstd suppprt of > > > > > btrfs into a separate module, named btrfs_zstd, to alleviate the size > > > > > change. Please note this only makes it's way to i386-pc, other > > > > > architecture is not affected. > > > > > > > > I am OK with extracting zstd code from btrfs code. However, I want that be > > > > done for all architectures and platforms. No exceptions. > > > > > > May I ask for more background about this decision ? > > > > Yes, I want the same code and behavior for all architectures and > > platforms if possible. > > > > > Given that btrfs compression is per file property and can be > > > recompressed on the fly, there is no way to detect it beforehand thus > > > the safest bet is to assume that it is always needed. In other words if > > > the platform has no known limitation on the size of image, the zstd code > > > should be indivisible to btrfs.mo. > > > > Wait, you said this in the commit message too: > > > > Therefore if the system has enough space of embedding area for grub then > > zstd support for btrfs will be enabled automatically in the process of > > running grub-install through inserting btrfs_zstd module to the on-disk > > image, otherwise a warning will be logged on screen to indicate user > > that zstd support for btrfs is disabled due to the size limit. > > > > So, I thought this may work in the same way in all cases. If this is not > > true you have to fix this. Otherwise I cannot take this patch. > > I literally don't like to propagate this (somewhat awkward) fix to other > architectures that do not have sensible requirement to it, though it > looks possible to do so. > > If this doesn't meet the expection then I'm sorry. > > > TBH, I should reject this patch immediately because this is "small MBR > > gap stuff". And as I said a few months ago, I want to stop to pay > > attention to "small MBR gap stuff" in upstream. Sorry for being blunt > > but it is full can of worms, i.e., each patch like that one adds more > > cruft which we have to take care because a few folks in the wild could > > not spend a few hours to repartition their systems. I think distros > > should start putting more effort in educating their users WRT to small > > MBR gap and problems related to it instead of pushing again and again > > upstream patches which make the GRUB code more complicated. > > Well, I think the short mbr gap issue is all well received been > discussed several times. The reason I'm poking this beehive again is Cool! Thanks! > 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? Daniel