[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: btrfs compression by default



On Fri, 2021-02-19 at 14:02 +0100, Martin Kolman wrote:
> On Tue, 2021-02-16 at 20:16 -0800, Michel Alexandre Salim wrote:
> > On Tue, 2021-02-16 at 15:05 +0100, Vendula Poncova wrote:
> > > On Tue, Feb 16, 2021 at 3:16 AM Michel Alexandre Salim
> > > <michel michel-slm name> wrote:
> > > > Hi Jiří, Vendula, all,
> > > > 
> > > > On Tue, 2021-02-09 at 12:12 +0100, Vendula Poncova wrote:
> > > > > On Tue, Feb 9, 2021 at 10:42 AM <jkonecny redhat com> wrote:
> > > > > > Hi everyone,
> > > > > > 
> > > > > > I see a few options for this. First is to add this directly
> > > > > > to
> > > > > > blivet
> > > > > > library as you pointed out already. However, I don't think
> > > > > > blivet
> > > > > > developers would be happy about that because they are trying
> > > > > > to
> > > > > > be as
> > > > > > much as possible general purpose library and this change is
> > > > > > really
> > > > > > just
> > > > > > about Anaconda.
> > > > > > 
> > > > > > Another option seems to be to modify Anaconda code directly.
> > > > > > Unfortunately, I can't tell from top of my mind how hard that
> > > > > > would
> > > > > > be
> > > > > > but still seems like the easiest solution.
> > > > > > 
> > > > > > However, the best approach which I can think of is to add
> > > > > > something
> > > > > > like that to our configuration files. Benefit would be that
> > > > > > another
> > > > > > modifications like that could be done easily in the future.
> > > > > > That is
> > > > > > of
> > > > > > course for a discussion for the Anaconda team because it
> > > > > > could
> > > > > > be
> > > > > > pretty hard to implemented this in some common form.
> > > > > > 
> > > > > > Vendy, Vojta, do you have some better ideas or could you tell
> > > > > > us
> > > > > > something more about my ideas above?
> > > > > > 
> > > > > > Best Regards,
> > > > > > Jirka
> > > > > > 
> > > > > > On Tue, 2021-02-09 at 01:18 -0500, Neal Gompa wrote:
> > > > > > > On Tue, Feb 9, 2021 at 1:09 AM Michel Alexandre Salim
> > > > > > > <michel michel-slm name> wrote:
> > > > > > > > Hi,
> > > > > > > > 
> > > > > > > > On Mon, 2021-02-08 at 21:02 -0700, Chris Murphy wrote:
> > > > > > > > > Hi,
> > > > > > > > > 
> > > > > > > > > This is in regards to this Fedora 34 change:
> > > > > > > > > https://fedoraproject.org/wiki/Changes/BtrfsTransparentCompression#Scope
> > > > > > > > > 
> > > > > > > > > The gist is to do 'mount -o compress=zstd:1' anytime
> > > > > > > > > Btrfs is
> > > > > > > > > used,
> > > > > > > > > whether Destination Installation>Automatic or Custom or
> > > > > > > > > Advanced-Custom. Apply it during the installation, and
> > > > > > > > > add it
> > > > > > > > > to
> > > > > > > > > /etc/fstab.
> > > > > > > > > 
> > > > > > > > > Somehow I got confused thinking that autopart supports
> > > > > > > > > --
> > > > > > > > > fsoptions,
> > > > > > > > > and that would be the way to do this. But (a) --
> > > > > > > > > fsoptions
> > > > > > > > > isn't
> > > > > > > > > supported with autopart, and (b) it wouldn't be a
> > > > > > > > > universal
> > > > > > > > > approach
> > > > > > > > > anyway. And we want this to be consistent. Now I'm
> > > > > > > > > thinking it
> > > > > > > > > needs
> > > > > > > > > to go somewhere in:
> > > > > > > > > 
> > > > > > > > > https://github.com/storaged-project/blivet/blob/3.4-devel/blivet/devices/btrfs.py
> > > > > > > > > 
> > > > > > > > > Am I on the right track, or does it need to go
> > > > > > > > > somewhere
> > > > > > > > > else,
> > > > > > > > > or
> > > > > > > > > in
> > > > > > > > > addition to that?
> > > > > > > > > 
> > > > > > > > (I'm one of the change owners, and trying to figure this
> > > > > > > > out with
> > > > > > > > Chris).
> > > > > > > > 
> > > > > > > > Ideally we have a solution that is configurable - i.e.
> > > > > > > > this
> > > > > > > > is
> > > > > > > > exposed
> > > > > > > > via a kickstart command (and probably entailing changes
> > > > > > > > in
> > > > > > > > Anaconda
> > > > > > > > and
> > > > > > > > pykickstart), but if that is not possible, or require a
> > > > > > > > lot
> > > > > > > > of
> > > > > > > > rework
> > > > > > > > (which we can try to work on), if we are willing to carry
> > > > > > > > a
> > > > > > > > patch
> > > > > > > > for
> > > > > > > > blivet or some other component to override the behavior
> > > > > > > > just for
> > > > > > > > Fedora
> > > > > > > > 34, what's the best way of achieving that?
> > > > > > > > 
> > > > > > > > Btrfs is probably the only filesystem with built-in
> > > > > > > > compression
> > > > > > > > that we
> > > > > > > > potentially care about, so designing an interface to
> > > > > > > > expose
> > > > > > > > this
> > > > > > > > functionality might be an overkill -- but we're not sure.
> > > > > > > > 
> > > > > > > 
> > > > > > > Eventually, VDO might get integrated into the mainline
> > > > > > > tree,
> > > > > > > so
> > > > > > > having
> > > > > > > the interface which could be used for LVM compression
> > > > > > > through
> > > > > > > VDO
> > > > > > > wouldn't be too bad.
> > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > 
> > > > > Hello!
> > > > > 
> > > > > The change says: "On variants using btrfs as the default
> > > > > filesystem,
> > > > > enable transparent compression using zstd." It means that the
> > > > > compression has to be configurable per product, so we need to
> > > > > add
> > > > > a
> > > > > new configuration option to the Anaconda configuration files
> > > > > (something like btrfs_compression_enabled = True). I guess you
> > > > > planned
> > > > > to use interactive-defaults.ks, but we are going to replace
> > > > > this
> > > > > file
> > > > > with the configuration files anyway.
> > > > > 
> > > > > I have checked the Blivet's code and we might be able to
> > > > > redefine
> > > > > the
> > > > > _mount_class attribute of the BTRFS class. The mount class
> > > > > defines
> > > > > default mount options. That should be enough to fix all
> > > > > partitioning
> > > > > methods. What do you think, Vojta?
> > > > > 
> > > > I'm trying to figure this out, and am currently a bit stuck:
> > > > 
> > > > - patching devicetree/fsset.py's `fstab` to ensure fstab contains
> > > > compress=zstd:1 works, but the change only affects fstab on the
> > > > installed system, so during installation compression is not
> > > > enabled
> > > > - patching devicetree/handler.py's `mount_options` to inject
> > > > compress=zstd:1 does not seem to do anything
> > > > https://github.com/rhinstaller/anaconda/compare/master...michel-slm:f34-34.23.1-btrfs-compression?expand=1
> > > > 
> > > > - patching devicetree/handler.py's `get_device_mount_options` to
> > > > ensure
> > > > btrfs filesystems area always mounted with compress=zstd:1 does
> > > > not
> > > > work as expected (patching `set_device_mount_options` does not do
> > > > anything either):
> > > > https://github.com/rhinstaller/anaconda/compare/master...michel-slm:f34-34.23.1-btrfs1?expand=1
> > > > 
> > > > 
> > > > Since I don't really want to hardcode this long-term (hardcoding
> > > > in
> > > > a
> > > > proof-of-concept, or for the F34 release only, is not too bad) --
> > > > is
> > > > Blivet the place to do this? What I had in mind is:
> > > > 
> > > > - Some product definition in data/product.d specifies compression
> > > > should be enabled
> > > > - When that is enabled, Anaconda overrides the _mount_class of
> > > > the
> > > > BTRFS class it gets from Blivet
> > > > 
> > > > ^ where in Anaconda's codebase can I do this? Trying to follow
> > > > this
> > > > in
> > > > the code but it's not easy especially since there does not seem
> > > > to
> > > > be
> > > > an example of another filesystem conditionally overriding the
> > > > default
> > > > mount options.
> > > 
> > > Hello,
> > > 
> > > I have talked to Vojta and we suggest the following solution:
> > > 
> > > 1. Open a pull request with [0] in the upstream.
> > > 2. Add a new flag to blivet.flags for enabling the compression of
> > > btrfs. It should be False by default. Modify [0] to respect the
> > > flag.
> > > 3. Open a pull request that sets up the new flag in Anaconda.
> > > Anaconda
> > > sets the Blivet's flags in [1].
> > > 4. If you want, add a new option to the [Storage] section of the
> > > Anaconda configuration file that will control the new flag. It
> > > should
> > > be False by default. Set this option in the relevant product
> > > configuration files located at [2].
> > > 
> > > [0] 
> > > https://src.fedoraproject.org/rpms/python-blivet/pull-request/8
> > > [1] 
> > > https://github.com/rhinstaller/anaconda/blob/master/pyanaconda/modules/storage/initialization.py#L50
> > > [2] 
> > >  https://github.com/rhinstaller/anaconda/tree/master/data/product.d
> > > 
> > > Vendy
> > 
> > Thanks Vendy and Vojta! This turns out to be way more straightforward
> > than I expected.
> > 
> > https://github.com/storaged-project/blivet/pull/932 for the new
> > Blivet
> > PR - I tested that with just this change, a normal installation still
> > works
> > https://github.com/rhinstaller/anaconda/pull/3177 for the Anaconda PR
> > -
> > with both blivet and Anaconda patched, on Fedora I ended up with a
> > Btrfs filesystem that's compressed both when the filesystem is
> > mounted
> > by Anaconda and when it's booted (the mount options end up in fstab).
> > 
> > Thanks all! I wish I could send everyone beers.
> Anaconda and Blivet with the needed changes have been built for F34
> (and Rawhide):
> https://koji.fedoraproject.org/koji/buildinfo?buildID=1711886
> https://koji.fedoraproject.org/koji/buildinfo?buildID=1711853
> 
> Both should be hitting Fedora composes near you shortly. :)
> 
Thank you! Sorry for the belated reply, life happened[^1] so I'm really
glad these got sorted just in time!

[^1]: https://michel-slm.name/posts/2021-02-26-software-wetware/

Best regards,

-- 
Michel Alexandre Salim
profile: https://keyoxide.org/michel michel-slm name

Attachment: signature.asc
Description: This is a digitally signed message part


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]