diff mbox series

[bug#55231,v2] initrd: Allow extra search paths with ‘initrd-extra-module-paths’

Message ID 2053c4ab42dfe2719cfc377934ac2fb9bcb500a9.1653160364.git.bjc@spork.org
State New
Headers show
Series [bug#55231,v2] initrd: Allow extra search paths with ‘initrd-extra-module-paths’ | expand

Checks

Context Check Description
cbaines/comparison success View comparision
cbaines/git branch success View Git branch
cbaines/applying patch success View Laminar job
cbaines/issue success View issue

Commit Message

Brian Cully May 21, 2022, 7:12 p.m. UTC 
From: Brian Cully <bjc@kublai.com>

This patch allows copying of out-of-tree kernel modules to the Linux
initrd.

For out-of-tree modules to found, an extra slot has been added to
‘operating-system’: ‘initrd-extra-module-paths’, which contains a list
containing items of either FILE-LIKE or (FILE-LIKE OUTPUT) which will be
searched for modules in addition to the standard Linux module path. The
required modules can then be added to ‘initrd-modules’ as normal and all paths
will be searched for it, including for any modules depended on.

* doc: Update documentation for ‘initrd-extra-module-paths’.
* gnu/build/linux-modules.scm (find-module-file): change DIRECTORY argument to
DIRECTORIES. Now takes a list of directories to search, rather than a single
one.
* gnu/system.scm <operating-system>: Add INITRD-EXTRA-MODULE-PATHS
field and accessor. Takes a list of file-like objects.
* gnu/system/linux-initrd.scm (flat-linux-module-directory): change LINUX
argument to PACKAGES. Now contains a list of file-likes to search for modules.
(raw-initrd): Add LINUX-EXTRA-MODULE-PATHS keyword argument. Pass it
to (flat-linux-extra-module-paths) along with the selected LINUX package.
(base-initrd): Add LINUX-EXTRA-MODULE-PATHS keyword argument. Pass it
to (raw-initrd).
---
This version includes both the previous patch and the documentation.

 doc/guix.texi               | 42 ++++++++++++++++++++++++------
 gnu/build/linux-modules.scm | 19 ++++++++------
 gnu/system.scm              |  5 ++++
 gnu/system/linux-initrd.scm | 52 +++++++++++++++++++++++++------------
 4 files changed, 85 insertions(+), 33 deletions(-)

Comments

Ludovic Courtès June 1, 2022, 3:54 p.m. UTC | #1 
Hello Brian,

Brian Cully <bjc@spork.org> skribis:

> From: Brian Cully <bjc@kublai.com>
>
> This patch allows copying of out-of-tree kernel modules to the Linux
> initrd.
>
> For out-of-tree modules to found, an extra slot has been added to
> ‘operating-system’: ‘initrd-extra-module-paths’, which contains a list
> containing items of either FILE-LIKE or (FILE-LIKE OUTPUT) which will be
> searched for modules in addition to the standard Linux module path. The
> required modules can then be added to ‘initrd-modules’ as normal and all paths
> will be searched for it, including for any modules depended on.
>
> * doc: Update documentation for ‘initrd-extra-module-paths’.
> * gnu/build/linux-modules.scm (find-module-file): change DIRECTORY argument to
> DIRECTORIES. Now takes a list of directories to search, rather than a single
> one.
> * gnu/system.scm <operating-system>: Add INITRD-EXTRA-MODULE-PATHS
> field and accessor. Takes a list of file-like objects.
> * gnu/system/linux-initrd.scm (flat-linux-module-directory): change LINUX
> argument to PACKAGES. Now contains a list of file-likes to search for modules.
> (raw-initrd): Add LINUX-EXTRA-MODULE-PATHS keyword argument. Pass it
> to (flat-linux-extra-module-paths) along with the selected LINUX package.
> (base-initrd): Add LINUX-EXTRA-MODULE-PATHS keyword argument. Pass it
> to (raw-initrd).

Nice, it looks like a welcome addition.

> +If a module listed in @code{initrd-modules} is not included in the
> +Linux-libre kernel, then the location to it must be added to the
> +@code{initrd-extra-module-paths} list.  For example, if your root file
> +system exists on a ZFS pool, then your configuration might look like the
> +following:
> +
> +@lisp
> +(operating-system
> +  ;; @dots{}
> +  (initrd-modules (cons "zfs" %base-initrd-modules))
> +  (initrd-extra-module-paths (list (list zfs "module"))))
> +@end lisp

I wonder if we could reuse the ‘kernel-loadable-modules’ field for this
purpose instead of introducing a new field.  We’d need to pass it to the
initrd procedures and have them search in there in addition to the
kernel package, pretty much like this patch already does actually.

WDYT?

Nitpick: the GNU convention is to use “path” to denote “search paths”,
and other “file”, “file name”, or similar.  In this case, that’d be
“kernel module” or “Linux module”.

Thank you!

Ludo’.
Brian Cully June 2, 2022, 8:40 p.m. UTC | #2 
Ludovic Courtès <ludo@gnu.org> writes:

> I wonder if we could reuse the ‘kernel-loadable-modules’ field 
> for this
> purpose instead of introducing a new field.  We’d need to pass 
> it to the
> initrd procedures and have them search in there in addition to 
> the
> kernel package, pretty much like this patch already does 
> actually.

This sounds like it could be made to work as you suggest. My 
feeling is that the two contexts are slightly different, though, 
as the Linux modules are a superset of the initrd modules, so I’d 
prefer not to mix them as it might be confusing to people who are 
used to other Linux distros where the initrd modules are called 
out separately. I admit I’m probably being silly here, and don’t 
have any serious objection in principle.

> Nitpick: the GNU convention is to use “path” to denote “search 
> paths”,
> and other “file”, “file name”, or similar.  In this case, that’d 
> be
> “kernel module” or “Linux module”.

I struggled with this a fair amount, actually. What these 
file-likes actually represent is an element of a search path, even 
if they come in the odd form of a file-like object, which is why I 
used ‘path’. ‘file’ seems wrong, as it implies (to me) that it’s 
the ‘initrd-extra-module-files’ option itself that would include 
the module, rather than the ‘initrd-modules’ option.

Of course, all this goes away if we just reuse 
‘kernel-loadable-modules’ as an additional input, rather than 
adding another option, so that’s a distinct mark in favor of doing 
that.

When I get some time (hopefully soon!) I’ll try to thread 
‘kernel-loadable-modules’ through instead and see how far I can 
get with that approach. Do you think the documentation for it will 
need to be updated to specify that it’s also used as a search path 
for initrd building? Or maybe the better option is to update the 
documentation for ‘initrd-modules’ to say that it uses 
‘kernel-loadable-modules’ as input?

-bjc
Ludovic Courtès June 3, 2022, 7:27 a.m. UTC | #3 
Hi,

Brian Cully <bjc@spork.org> skribis:

> Ludovic Courtès <ludo@gnu.org> writes:
>
>> I wonder if we could reuse the ‘kernel-loadable-modules’ field for
>> this
>> purpose instead of introducing a new field.  We’d need to pass it to
>> the
>> initrd procedures and have them search in there in addition to the
>> kernel package, pretty much like this patch already does actually.
>
> This sounds like it could be made to work as you suggest. My feeling
> is that the two contexts are slightly different, though, as the Linux
> modules are a superset of the initrd modules,

Right.  Here, we want the initrd machinery to search for modules in any
place where they can be found, which is either ‘kernel’ or
‘kernel-loadable-modules’.

One could define ‘initrd-extra-module-paths’ to be different from
‘kernel-loadable-modules’, for example if you can be sure the module
will be loaded from the initrd and not after, but in general both are
likely to have the same value, no?

>> Nitpick: the GNU convention is to use “path” to denote “search
>> paths”,
>> and other “file”, “file name”, or similar.  In this case, that’d be
>> “kernel module” or “Linux module”.
>
> I struggled with this a fair amount, actually. What these file-likes
> actually represent is an element of a search path, even if they come
> in the odd form of a file-like object, which is why I used
> ‘path’. ‘file’ seems wrong, as it implies (to me) that it’s the
> ‘initrd-extra-module-files’ option itself that would include the
> module, rather than the ‘initrd-modules’ option.

Hmm right, you have a point!  :-)

> When I get some time (hopefully soon!) I’ll try to thread
> ‘kernel-loadable-modules’ through instead and see how far I can get
> with that approach. Do you think the documentation for it will need to
> be updated to specify that it’s also used as a search path for initrd
> building? Or maybe the better option is to update the documentation
> for ‘initrd-modules’ to say that it uses ‘kernel-loadable-modules’ as
> input?

I think you should update the documentation in the commit that changes
things, so that the patch is self-contained.

It may be enough to state in the documentation of the ‘initrd-modules’
field that its value is a list of module names that are searched for in
‘kernel’ and ‘kernel-loadable-modules’.

WDYT?

Thanks,
Ludo’.
diff mbox series

Patch

diff --git a/doc/guix.texi b/doc/guix.texi
index faa35060ef..9fd45ea209 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -15490,6 +15490,16 @@  operating-system Reference
 The list of Linux kernel modules that need to be available in the
 initial RAM disk.  @xref{Initial RAM Disk}.
 
+@item @code{initrd-extra-module-paths} (default: @code{'()})
+@cindex initrd
+@cindex initial RAM disk
+A list of paths outside of the Linux kernel tree to search for Linux
+kernel modules.
+
+The items in this may be either file-like objects (usually packages), or
+a list where the first element is a package and the second is an
+output--e.g. @code{(list (list zfs "module"))}.
+
 @item @code{initrd} (default: @code{base-initrd})
 A procedure that returns an initial RAM disk for the Linux
 kernel.  This field is provided to support low-level customization and
@@ -35816,6 +35826,19 @@  Initial RAM Disk
   (initrd-modules (cons "megaraid_sas" %base-initrd-modules)))
 @end lisp
 
+If a module listed in @code{initrd-modules} is not included in the
+Linux-libre kernel, then the location to it must be added to the
+@code{initrd-extra-module-paths} list.  For example, if your root file
+system exists on a ZFS pool, then your configuration might look like the
+following:
+
+@lisp
+(operating-system
+  ;; @dots{}
+  (initrd-modules (cons "zfs" %base-initrd-modules))
+  (initrd-extra-module-paths (list (list zfs "module"))))
+@end lisp
+
 @defvr {Scheme Variable} %base-initrd-modules
 This is the list of kernel modules included in the initrd by default.
 @end defvr
@@ -35929,13 +35952,15 @@  Initial RAM Disk
 @cindex initrd
 @cindex initial RAM disk
 @deffn {Scheme Procedure} raw-initrd @var{file-systems} @
-       [#:linux-modules '()] [#:mapped-devices '()] @
-       [#:keyboard-layout #f] @
+       [#:linux-modules '()] [#:linux-extra-module-paths '()] @
+       [#:mapped-devices '()] [#:keyboard-layout #f] @
        [#:helper-packages '()] [#:qemu-networking? #f] [#:volatile-root? #f]
 Return a derivation that builds a raw initrd.  @var{file-systems} is
 a list of file systems to be mounted by the initrd, possibly in addition to
 the root file system specified on the kernel command line via @option{root}.
 @var{linux-modules} is a list of kernel modules to be loaded at boot time.
+@var{linux-extra-module-paths} is a list of file-like objects to be searched
+for kernel modules.
 @var{mapped-devices} is a list of device mappings to realize before
 @var{file-systems} are mounted (@pxref{Mapped Devices}).
 @var{helper-packages} is a list of packages to be copied in the initrd.
@@ -35960,12 +35985,13 @@  Initial RAM Disk
 @deffn {Scheme Procedure} base-initrd @var{file-systems} @
        [#:mapped-devices '()] [#:keyboard-layout #f] @
        [#:qemu-networking? #f] [#:volatile-root? #f] @
-       [#:linux-modules '()]
-Return as a file-like object a generic initrd, with kernel
-modules taken from @var{linux}.  @var{file-systems} is a list of file-systems to be
-mounted by the initrd, possibly in addition to the root file system specified
-on the kernel command line via @option{root}.  @var{mapped-devices} is a list of device
-mappings to realize before @var{file-systems} are mounted.
+       [#:linux-modules '()] [#:linux-extra-module-paths '()]
+Return as a file-like object a generic initrd, with kernel modules taken
+from @var{linux} and @var{linux-extra-module-paths}.  @var{file-systems}
+is a list of file-systems to be mounted by the initrd, possibly in
+addition to the root file system specified on the kernel command line
+via @option{root}.  @var{mapped-devices} is a list of device mappings to
+realize before @var{file-systems} are mounted.
 
 When true, @var{keyboard-layout} is a @code{<keyboard-layout>} record denoting
 the desired console keyboard layout.  This is done before @var{mapped-devices}
diff --git a/gnu/build/linux-modules.scm b/gnu/build/linux-modules.scm
index 053720574b..97b7e429ea 100644
--- a/gnu/build/linux-modules.scm
+++ b/gnu/build/linux-modules.scm
@@ -225,8 +225,8 @@  (define (file-name->module-name file)
 '.ko[.gz|.xz]' and normalizing it."
   (normalize-module-name (strip-extension (basename file))))
 
-(define (find-module-file directory module)
-  "Lookup module NAME under DIRECTORY, and return its absolute file name.
+(define (find-module-file directories module)
+  "Lookup module NAME under DIRECTORIES, and return its absolute file name.
 NAME can be a file name with or without '.ko', or it can be a module name.
 Raise an error if it could not be found.
 
@@ -247,16 +247,19 @@  (define (find-module-file directory module)
                            (else chr)))
                        module))))
 
-  (match (find-files directory
-                     (lambda (file stat)
-                       (member (strip-extension
-                                (basename file)) names)))
+  (match (append-map (lambda (directory)
+                       (find-files directory
+                                   (lambda (file _stat)
+                                     (member (strip-extension
+                                              (basename file))
+                                             names))))
+                       directories)
     ((file)
      file)
     (()
-     (error "kernel module not found" module directory))
+     (error "kernel module not found" module directories))
     ((_ ...)
-     (error "several modules by that name" module directory))))
+     (error "several modules by that name" module directories))))
 
 (define* (recursive-module-dependencies files
                                         #:key (lookup-module dot-ko))
diff --git a/gnu/system.scm b/gnu/system.scm
index ba3a1865d7..6c712e5cea 100644
--- a/gnu/system.scm
+++ b/gnu/system.scm
@@ -107,6 +107,7 @@  (define-module (gnu system)
             operating-system-label
             operating-system-default-label
             operating-system-initrd-modules
+            operating-system-initrd-extra-module-paths
             operating-system-initrd
             operating-system-users
             operating-system-groups
@@ -236,6 +237,8 @@  (define-record-type* <operating-system> operating-system
   (initrd-modules operating-system-initrd-modules ; list of strings
                   (thunked)                       ; it's system-dependent
                   (default %base-initrd-modules))
+  (initrd-extra-module-paths operating-system-initrd-extra-module-paths ; list of file-likes
+                             (default '()))
 
   (firmware operating-system-firmware             ; list of packages
             (default %base-firmware))
@@ -1312,6 +1315,8 @@  (define (operating-system-initrd-file os)
                #:linux (operating-system-kernel os)
                #:linux-modules
                (operating-system-initrd-modules os)
+               #:linux-extra-module-paths
+               (operating-system-initrd-extra-module-paths os)
                #:mapped-devices mapped-devices
                #:keyboard-layout (operating-system-keyboard-layout os)))
 
diff --git a/gnu/system/linux-initrd.scm b/gnu/system/linux-initrd.scm
index 4c4c78e444..50a182d7d5 100644
--- a/gnu/system/linux-initrd.scm
+++ b/gnu/system/linux-initrd.scm
@@ -119,13 +119,23 @@  (define* (expression->initrd exp
                               `(#:references-graphs (("closure" ,init))))
                "/initrd.cpio.gz"))
 
-(define (flat-linux-module-directory linux modules)
+(define (flat-linux-module-directory packages modules)
   "Return a flat directory containing the Linux kernel modules listed in
-MODULES and taken from LINUX."
+MODULES and taken from PACKAGES."
   (define imported-modules
     (source-module-closure '((gnu build linux-modules)
                              (guix build utils))))
 
+  (define (package+out->input package out)
+    (gexp-input package out))
+
+  (define package-inputs
+    (map (lambda (p)
+           (match p
+             ((p o) (package+out->input p o))
+             (p     (package+out->input p "out"))))
+         packages))
+
   (define build-exp
     (with-imported-modules imported-modules
       (with-extensions (list guile-zlib)
@@ -135,11 +145,12 @@  (define (flat-linux-module-directory linux modules)
                          (srfi srfi-1)
                          (srfi srfi-26))
 
-            (define module-dir
-              (string-append #$linux "/lib/modules"))
+            (define module-dirs
+              (map (cut string-append <> "/lib/modules")
+                   '#$package-inputs))
 
             (define modules
-              (let* ((lookup  (cut find-module-file module-dir <>))
+              (let* ((lookup  (cut find-module-file module-dirs <>))
                      (modules (map lookup '#$modules)))
                 (append modules
                         (recursive-module-dependencies
@@ -172,20 +183,23 @@  (define* (raw-initrd file-systems
                       #:key
                       (linux linux-libre)
                       (linux-modules '())
+                      (linux-extra-module-paths '())
                       (mapped-devices '())
                       (keyboard-layout #f)
                       (helper-packages '())
                       qemu-networking?
                       volatile-root?
                       (on-error 'debug))
-  "Return as a file-like object a raw initrd, with kernel
-modules taken from LINUX.  FILE-SYSTEMS is a list of file-systems to be
-mounted by the initrd, possibly in addition to the root file system specified
-on the kernel command line via 'root'.  LINUX-MODULES is a list of kernel
-modules to be loaded at boot time. MAPPED-DEVICES is a list of device
-mappings to realize before FILE-SYSTEMS are mounted.
-HELPER-PACKAGES is a list of packages to be copied in the initrd. It may include
-e2fsck/static or other packages needed by the initrd to check root partition.
+  "Return as a file-like object a raw initrd, with kernel modules taken from
+LINUX.  FILE-SYSTEMS is a list of file-systems to be mounted by the initrd,
+possibly in addition to the root file system specified on the kernel command
+line via 'root'.  LINUX-MODULES is a list of kernel modules to be loaded at
+boot time. LINUX-EXTRA-MODULE-PATHS is a list of file-like objects which will
+be searched for modules in addition to the linux kernel. MAPPED-DEVICES is a
+list of device mappings to realize before FILE-SYSTEMS are mounted.
+HELPER-PACKAGES is a list of packages to be copied in the initrd. It may
+include e2fsck/static or other packages needed by the initrd to check root
+partition.
 
 When true, KEYBOARD-LAYOUT is a <keyboard-layout> record denoting the desired
 console keyboard layout.  This is done before MAPPED-DEVICES are set up and
@@ -221,7 +235,8 @@  (define* (raw-initrd file-systems
           #~())))
 
   (define kodir
-    (flat-linux-module-directory linux linux-modules))
+    (flat-linux-module-directory (cons linux linux-extra-module-paths)
+                                 linux-modules))
 
   (expression->initrd
    (with-imported-modules (source-module-closure
@@ -366,6 +381,7 @@  (define* (base-initrd file-systems
                       #:key
                       (linux linux-libre)
                       (linux-modules '())
+                      (linux-extra-module-paths '())
                       (mapped-devices '())
                       (keyboard-layout #f)
                       qemu-networking?
@@ -386,9 +402,10 @@  (define* (base-initrd file-systems
 QEMU-NETWORKING? and VOLATILE-ROOT? behaves as in raw-initrd.
 
 The initrd is automatically populated with all the kernel modules necessary
-for FILE-SYSTEMS and for the given options.  Additional kernel
-modules can be listed in LINUX-MODULES.  They will be added to the initrd, and
-loaded at boot time in the order in which they appear."
+for FILE-SYSTEMS and for the given options.  Additional kernel modules can be
+listed in LINUX-MODULES.  Additional search paths for modules can be listed in
+LINUX-EXTRA-MODULE-PATHS.  They will be added to the initrd, and loaded at
+boot time in the order in which they appear."
   (define linux-modules*
     ;; Modules added to the initrd and loaded from the initrd.
     `(,@linux-modules
@@ -408,6 +425,7 @@  (define* (base-initrd file-systems
   (raw-initrd file-systems
               #:linux linux
               #:linux-modules linux-modules*
+              #:linux-extra-module-paths linux-extra-module-paths
               #:mapped-devices mapped-devices
               #:helper-packages helper-packages
               #:keyboard-layout keyboard-layout