GNU bug report logs - #45403
[PATCH] gnu: zfs: Split into packages specific for each of our major supported kernel versions.

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 45403 in the body.
You can then email your comments to 45403 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: raid5atemyhomework <raid5atemyhomework <at> protonmail.com>
To: "guix-patches <at> gnu.org" <guix-patches <at> gnu.org>
Subject: [PATCH] gnu: zfs: Split into packages specific for each of our major
 supported kernel versions.
Date: Thu, 24 Dec 2020 10:59:56 +0000

Fixes: https://issues.guix.gnu.org/45401

Untested --- no idea how to repoint my guix to a temporary commit on my local file repo, would appreciate any guidance.


From 242ee858b057f1c3ca4a80c9c449f710c5b1ff96 Mon Sep 17 00:00:00 2001
From: raid5atemyhomework <raid5atemyhomework <at> protonmail.com>
Date: Thu, 24 Dec 2020 10:54:17 +0000
Subject: [PATCH] gnu: zfs: Split into packages specific for each of our major
 supported kernel versions.

---
 gnu/packages/file-systems.scm | 31 ++++++++++++++++++++++++++++++-
 1 file changed, 30 insertions(+), 1 deletion(-)

diff --git a/gnu/packages/file-systems.scm b/gnu/packages/file-systems.scm
index af587f73fe..d3cdeda4fb 100644
--- a/gnu/packages/file-systems.scm
+++ b/gnu/packages/file-systems.scm
@@ -843,7 +843,7 @@ APFS.")
       (home-page "https://github.com/sgan81/apfs-fuse")
       (license license:gpl2+))))

-(define-public zfs
+(define zfs-base
   (package
     (name "zfs")
     (version "0.8.5")
@@ -954,6 +954,35 @@ originally developed for Solaris and is now maintained by the OpenZFS
 community.")
     (license license:cddl1.0)))

+(define-public zfs
+  (package
+    (inherit zfs-base)
+    (description
+     (string-append (package-description zfs-base)
+                    "
+
+This package is deprecated, use linux-VERSION-zfs packages instead."))))
+
+(define (make-linux-zfs linux-libre)
+  (package
+    (inherit zfs-base)
+    (arguments
+     `(#:linux ,linux-libre)
+       ,@(package-arguments zfs-base))
+    (description
+     (string-append (package-description zfs-base)
+                    "
+
+This package will be compiled to work with the linux-libre "
+                    (package-version linux-libre)
+                    " kernel."))))
+
+(define-public linux-5.9-zfs (make-linux-zfs linux-libre-5.9))
+(define-public linux-5.4-zfs (make-linux-zfs linux-libre-5.4))
+(define-public linux-4.19-zfs (make-linux-zfs linux-libre-4.19))
+(define-public linux-4.14-zfs (make-linux-zfs linux-libre-4.14))
+(define-public linux-4.4-zfs (make-linux-zfs linux-libre-4.4))
+
 (define-public mergerfs
   (package
     (name "mergerfs")
--
2.29.2

Message #8 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Leo Famulari <leo <at> famulari.name>
To: raid5atemyhomework via Guix-patches via <guix-patches <at> gnu.org>
Cc: 45403 <at> debbugs.gnu.org
Subject: Re: [bug#45403] [PATCH] gnu: zfs: Split into packages specific for
 each of our major supported kernel versions.
Date: Thu, 24 Dec 2020 15:41:33 -0500

On Thu, Dec 24, 2020 at 10:59:56AM +0000, raid5atemyhomework via Guix-patches via wrote:
> Fixes: https://issues.guix.gnu.org/45401
> 
> Untested --- no idea how to repoint my guix to a temporary commit on my local file repo, would appreciate any guidance.

There are primarily two ways to use Guix with a local Git repo.

First, you can follow the instructions in the manual section
Contributing, specifically the sections Building from Git and Running
Guix Before It Is Installed.

Or, you can use something like `guix pull --url=/path/to/my/repo
--commit=mycommit`.

Beyond that, does this patch create linux-libre packages with ZFS
support compiled in?

Message #14 received at 45403 <at> debbugs.gnu.org (full text, mbox):

From: raid5atemyhomework <raid5atemyhomework <at> protonmail.com>
To: "45403 <at> debbugs.gnu.org" <45403 <at> debbugs.gnu.org>
Subject: Re: [PATCH] gnu: zfs: Split into packages specific for each of our
 major supported kernel versions.
Date: Fri, 25 Dec 2020 01:12:12 +0000

> There are primarily two ways to use Guix with a local Git repo.
>
> First, you can follow the instructions in the manual section Contributing, specifically the sections Building from Git and RunningGuix Before It Is Installed.
>
> Or, you can use something like `guix pull --url=/path/to/my/repo --commit=mycommit`.

Thank you very much, will check.

> Beyond that, does this patch create linux-libre packages with ZFS support compiled in?

No, this patch continues to use the existing technique of downloading ZFS as source and compiling it as a kernel module you have to load in, as per licensing incompatibility.

What it changes is that it compiles kernel modules for all guix-provided kernel versions that ZFS currently is rated to support, rather than compile a kernel module for whatever `(default-linux)` is, which is always the latest and greatest kernel (currently 5.10) which ZFS is currently not rated for.

As Guix goes through the trouble of maintaining a `linux-libre-4.4` package I assume at least one user somewhere is using Linux 4.4, and did not want to prevent that user from using ZFS if they decide to do so someday.

I admit the naming scheme `linux-5.9-zfs` does make it seem like the package contains an entire Linux kernel.
The alternative is to call it `zfs-for-linux-5.9` but that makes the entire package name with version `zfs-for-linux-5.9-0.8.5` which I think is worse, because the version numbers are right next to each other.
What naming scheme would you suggest?

* `linux-5.9-zfs` --- makes it look like it contains a whole Linux kernel.
* `zfs-for-linux-5.9` --- versions are right next to each other: `zfs-for-linux-5.9-0.8.5`, this is bad since some versioning conventions include a `-` in the version naming, so at a glance this makes for a very confusing version.
* `kernelmodule-5.9-zfs` --- makes it clear that it is not an entire kernel, but this package *also* includes userspace utilities not just the kernel module.
* `zfs-for-linux-5.9-version` --- kind of a weird and absurdly lengthy name but maybe palatable?

I imagine a similar problem would occur for other kernel modules --- the internal kernel interfaces are *not* stable at all, so complex external kernel modules like ZFS need to know what kernel version it's being compiled for and include a bunch of `#ifdef`s to compensate.
So we need some decent convention for "how do we name kernel modules so that they are targeted for each kernel version".

Message #17 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Tobias Geerinckx-Rice <me <at> tobias.gr>
To: raid5atemyhomework <raid5atemyhomework <at> protonmail.com>,
 raid5atemyhomework via Guix-patches <guix-patches <at> gnu.org>
Cc: "45403 <at> debbugs.gnu.org" <45403 <at> debbugs.gnu.org>
Subject: Re: [bug#45403] [PATCH] gnu: zfs: Split into packages specific for
 each of our major supported kernel versions.
Date: Fri, 25 Dec 2020 05:09:44 +0100

[Message part 1 (text/plain, inline)]

Hullo RAIDperson!

Thanks for working to improve Guix's currently broken ZFS package!

raid5atemyhomework via Guix-patches via 写道：
> What it changes is that it compiles kernel modules for all 
> guix-provided kernel versions that ZFS currently is rated to 
> support,

However, I oppose this (and not because ‘rated’ sets off my 
corpo-speak alarms :-).  It makes your system.scm tedious to 
maintain, especially if we'd do this for all modules (and why 
not?).

Worse, there's no such thing as ‘a 5.10’ Linux module that loads 
on ‘a 5.10’ Linux kernel.  We still need an easy generic method to 
build modules against their chosen kernel and configuration.

As your patch illustrates, that already exists:

> (define (make-linux-zfs linux-libre)
>   (package
>     (inherit zfs-base)
>     (arguments
>      `(#:linux ,linux-libre)
>        ,@(package-arguments zfs-base))

It just belongs in your system.scm, not in Guix itself.

Kind regards,

T G-R

P.S.nitpick: unfortunately this breaks localisation and can't be 
done:

> +     (string-append (package-description zfs-base)
> +                    "
> +
> +This package is deprecated, use linux-VERSION-zfs packages 
> instead."))))

Descriptions need to be simple self-contained strings, even if it 
means duplicating entire paragraphs.

[signature.asc (application/pgp-signature, inline)]

Message #23 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Danny Milosavljevic <dannym <at> scratchpost.org>
To: Tobias Geerinckx-Rice via Guix-patches via <guix-patches <at> gnu.org>
Cc: raid5atemyhomework <at> protonmail.com, Tobias Geerinckx-Rice <me <at> tobias.gr>,
 45403 <at> debbugs.gnu.org
Subject: Re: [bug#45403] [PATCH] gnu: zfs: Split into packages specific for
 each of our major supported kernel versions.
Date: Mon, 4 Jan 2021 14:58:18 +0100

[Message part 1 (text/plain, inline)]

On Fri, 25 Dec 2020 05:09:44 +0100
Tobias Geerinckx-Rice via Guix-patches via <guix-patches <at> gnu.org> wrote:

> Worse, there's no such thing as ‘a 5.10’ Linux module that loads 
> on ‘a 5.10’ Linux kernel.  We still need an easy generic method to 
> build modules against their chosen kernel and configuration.
> 
> As your patch illustrates, that already exists:
> 
> > (define (make-linux-zfs linux-libre)
> >   (package
> >     (inherit zfs-base)
> >     (arguments
> >      `(#:linux ,linux-libre)
> >        ,@(package-arguments zfs-base))  
> 
> It just belongs in your system.scm, not in Guix itself.

Why is this needed?  KERNEL-LOADABLE-MODULES are already automatically
adapted in this way.

See this in gnu/system.scm :

(define* (operating-system-directory-base-entries os)
  "Return the basic entries of the 'system' directory of OS for use as the
value of the SYSTEM-SERVICE-TYPE service."
  (let* ((locale  (operating-system-locale-directory os))
         (kernel  (operating-system-kernel os))
         (hurd    (operating-system-hurd os))
         (modules (operating-system-kernel-loadable-modules os))
         (kernel  (if hurd
                      kernel
                      (profile
                       (content (packages->manifest
                                 (cons kernel
                                       (map (lambda (module)
                                              (if (package? module)
                                                  (package-for-kernel kernel    <-----
                                                                      module)
                                                  module))
                                            modules))))
                       (hooks (list linux-module-database)))))
         (initrd  (and (not hurd) (operating-system-initrd-file os)))
         (params  (operating-system-boot-parameters-file os)))
    `(("kernel" ,kernel)
      ,@(if hurd `(("hurd" ,hurd)) '())
      ("parameters" ,params)
      ,@(if initrd `(("initrd" ,initrd)) '())
      ("locale" ,locale))))   ;used by libc

[Message part 2 (application/pgp-signature, inline)]

Message #29 received at submit <at> debbugs.gnu.org (full text, mbox):

From: raid5atemyhomework <raid5atemyhomework <at> protonmail.com>
To: Danny Milosavljevic <dannym <at> scratchpost.org>
Cc: Tobias Geerinckx-Rice <me <at> tobias.gr>,
 "45403 <at> debbugs.gnu.org" <45403 <at> debbugs.gnu.org>,
 Tobias Geerinckx-Rice via Guix-patches via <guix-patches <at> gnu.org>
Subject: Re: [bug#45403] [PATCH] gnu: zfs: Split into packages specific for
 each of our major supported kernel versions.
Date: Mon, 04 Jan 2021 15:49:01 +0000

This doesn't work, for two reasons:

* The kernel-module is in the output "module" of the `zfs` package.  The code specifically checks for `(package? module)`, but we have to specify it as `(list system-zfs "module")`.
* ZFS is primarily managed via userspace tools, that's what ZFS users know how to use, and we should support that so that Guix users can ask non-Guix ZFS users for help with their ZFS problems.  The userspace tools are included in the package.  If the base `zfs` package cannot compile because it's incompatible with the default kernel, then the userspace tools are nonexistent and there is no way to manage ZFS.
  * So, for example we can't just remove the "module" output and have the `.ko` installed in the default "out" output, our core problem is we need to compile a ZFS module ***and*** userspace tools.
  * We can try to split the compilation to separate the ZFS module from the userspace tool, but that risks brittleness and subtle incompatibilities between a ZFS module compiled for your local kernel (which cannot be the latest and greatest kernel since the kernel does not maintain interface compatibility with kernel modules and the ZFS module is always having to catch up), and ZFS userspace tool compiled for the default latest-and-greatest kernel (even if we somehow manage to compile ***just*** the userspace tools with the latest kernel, there may be implicit assumptions about kernel constants and so on passed from the userspace tools to the kernel module, which may cause problems and birttleness).

So it's just safer to always override the `zfs` `#:linux` with the system kernel, and install it in `(packages ...)` and in `(kernel-loadable-modules ...)`.

* We get compileability as long as older kernel is used.
* We ensure that the userspace tool is built for the same kernel module for a compatible kernel, at all generations of the `guix system`.

Thanks
raid5atemyhomework

> Why is this needed? KERNEL-LOADABLE-MODULES are already automatically
> adapted in this way.
>
> See this in gnu/system.scm :
>
> (define* (operating-system-directory-base-entries os)
> "Return the basic entries of the 'system' directory of OS for use as the
> value of the SYSTEM-SERVICE-TYPE service."
> (let* ((locale (operating-system-locale-directory os))
> (kernel (operating-system-kernel os))
> (hurd (operating-system-hurd os))
> (modules (operating-system-kernel-loadable-modules os))
> (kernel (if hurd
> kernel
> (profile
> (content (packages->manifest
>
>                                  (cons kernel
>                                        (map (lambda (module)
>                                               (if (package? module)
>                                                   (package-for-kernel kernel    <-----
>                                                                       module)
>                                                   module))
>                                             modules))))
>                        (hooks (list linux-module-database)))))
>          (initrd  (and (not hurd) (operating-system-initrd-file os)))
>          (params  (operating-system-boot-parameters-file os)))
>     `(("kernel" ,kernel)
>       ,@(if hurd `(("hurd" ,hurd)) '())
>       ("parameters" ,params)
>       ,@(if initrd `(("initrd" ,initrd)) '())
>       ("locale" ,locale))))   ;used by libc

Message #35 received at submit <at> debbugs.gnu.org (full text, mbox):

From: raid5atemyhomework <raid5atemyhomework <at> protonmail.com>
To: Danny Milosavljevic <dannym <at> scratchpost.org>
Cc: Tobias Geerinckx-Rice <me <at> tobias.gr>,
 "45403 <at> debbugs.gnu.org" <45403 <at> debbugs.gnu.org>,
 Tobias Geerinckx-Rice via Guix-patches via <guix-patches <at> gnu.org>
Subject: Re: [bug#45403] [PATCH] gnu: zfs: Split into packages specific for
 each of our major supported kernel versions.
Date: Mon, 04 Jan 2021 16:07:08 +0000

Here's another alternative:

* Remove the separate "module" output and put the `.ko` file in the "out" output.
* Create a separate `zfs-guix-tools` which provides `zpool`, `zfs`, `zed`, `arc_summary`, `arcstat`, `dbufstat`, `raidz_test`, `zgenhostid`, `zvol_wait`, `fsck.zfs`, `mount.zfs`, `zdb`, `zhack`, `zinject`, `zstreamdump`, `ztest`.
  * What the provided binaries do is:
    * It looks at `/run/booted-system/kernel/manifest` and looks for a `"zfs"` package.
    * It `exec`s into the corresponding file in the `/gnu/store` for that package.

This way, we can have a separate package for ZFS userspace tools, which just calls the corresponding tools in the actual ZFS that gets installed.
It's somewhat magical, though.

Thanks
raid5atemyhomework

> This doesn't work, for two reasons:
>
> -   The kernel-module is in the output "module" of the `zfs` package. The code specifically checks for `(package? module)`, but we have to specify it as `(list system-zfs "module")`.
> -   ZFS is primarily managed via userspace tools, that's what ZFS users know how to use, and we should support that so that Guix users can ask non-Guix ZFS users for help with their ZFS problems. The userspace tools are included in the package. If the base `zfs` package cannot compile because it's incompatible with the default kernel, then the userspace tools are nonexistent and there is no way to manage ZFS.
>     -   So, for example we can't just remove the "module" output and have the `.ko` installed in the default "out" output, our core problem is we need to compile a ZFS module and userspace tools.
>     -   We can try to split the compilation to separate the ZFS module from the userspace tool, but that risks brittleness and subtle incompatibilities between a ZFS module compiled for your local kernel (which cannot be the latest and greatest kernel since the kernel does not maintain interface compatibility with kernel modules and the ZFS module is always having to catch up), and ZFS userspace tool compiled for the default latest-and-greatest kernel (even if we somehow manage to compile just the userspace tools with the latest kernel, there may be implicit assumptions about kernel constants and so on passed from the userspace tools to the kernel module, which may cause problems and birttleness).
>
>         So it's just safer to always override the `zfs` `#:linux` with the system kernel, and install it in `(packages ...)` and in `(kernel-loadable-modules ...)`.
>
> -   We get compileability as long as older kernel is used.
> -   We ensure that the userspace tool is built for the same kernel module for a compatible kernel, at all generations of the `guix system`.
>
>     Thanks
>     raid5atemyhomework
>
>
> > Why is this needed? KERNEL-LOADABLE-MODULES are already automatically
> > adapted in this way.
> > See this in gnu/system.scm :
> > (define* (operating-system-directory-base-entries os)
> > "Return the basic entries of the 'system' directory of OS for use as the
> > value of the SYSTEM-SERVICE-TYPE service."
> > (let* ((locale (operating-system-locale-directory os))
> > (kernel (operating-system-kernel os))
> > (hurd (operating-system-hurd os))
> > (modules (operating-system-kernel-loadable-modules os))
> > (kernel (if hurd
> > kernel
> > (profile
> > (content (packages->manifest
> >
> >                                  (cons kernel
> >                                        (map (lambda (module)
> >                                               (if (package? module)
> >                                                   (package-for-kernel kernel    <-----
> >                                                                       module)
> >                                                   module))
> >                                             modules))))
> >                        (hooks (list linux-module-database)))))
> >          (initrd  (and (not hurd) (operating-system-initrd-file os)))
> >          (params  (operating-system-boot-parameters-file os)))
> >     `(("kernel" ,kernel)
> >       ,@(if hurd `(("hurd" ,hurd)) '())
> >       ("parameters" ,params)
> >       ,@(if initrd `(("initrd" ,initrd)) '())
> >       ("locale" ,locale))))   ;used by libc
> >

Message #41 received at submit <at> debbugs.gnu.org (full text, mbox):

From: raid5atemyhomework <raid5atemyhomework <at> protonmail.com>
To: Danny Milosavljevic <dannym <at> scratchpost.org>
Cc: Tobias Geerinckx-Rice <me <at> tobias.gr>,
 "45403 <at> debbugs.gnu.org" <45403 <at> debbugs.gnu.org>,
 Tobias Geerinckx-Rice via Guix-patches via <guix-patches <at> gnu.org>
Subject: Re: [bug#45403] [PATCH] gnu: zfs: Split into packages specific for
 each of our major supported kernel versions.
Date: Mon, 04 Jan 2021 16:32:18 +0000

> Here's another alternative:
>
> -   Remove the separate "module" output and put the `.ko` file in the "out" output.
> -   Create a separate `zfs-guix-tools` which provides `zpool`, `zfs`, `zed`, `arc_summary`, `arcstat`, `dbufstat`, `raidz_test`, `zgenhostid`, `zvol_wait`, `fsck.zfs`, `mount.zfs`, `zdb`, `zhack`, `zinject`, `zstreamdump`, `ztest`.
>     -   What the provided binaries do is:
>         -   It looks at `/run/booted-system/kernel/manifest` and looks for a `"zfs"` package.
>         -   It `exec`s into the corresponding file in the `/gnu/store` for that package.
>
>             This way, we can have a separate package for ZFS userspace tools, which just calls the corresponding tools in the actual ZFS that gets installed.
>             It's somewhat magical, though.
>

Unfortunately, this confounds `bash-completion`.  We want the bash-completion from the actual compiled ZFS, not the shim `zfs-guix-tools`.  We could try digging for it too in the `kernel/manifest`, but that just adds more complexity and maintenance headaches.

It's also an extra layer of complexity and maintenance burden to create this shim wrapper that digs through your kernel manifest to look for the userland tools you need to manage ZFS in the same package that contains the actual ZFS kernel module.  I think it's better to just expose a `make-zfs-package` that overrides the `#:linux` and creates a new package that you can install in `(packages ...)` **and** in `(kernel-loadable-modules ...)`.

See https://issues.guix.gnu.org/45643 for my current latest patchset.

Thanks
raid5atemyhomework

Message #49 received at 45403-done <at> debbugs.gnu.org (full text, mbox):

From: raid5atemyhomework <raid5atemyhomework <at> protonmail.com>
To: "45403-done <at> debbugs.gnu.org" <45403-done <at> debbugs.gnu.org>
Subject: Re: [PATCH] gnu: zfs: Split into packages specific for each of our
 major supported kernel versions.
Date: Fri, 08 Jan 2021 15:50:59 +0000

Close in favor of 45692

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.