[bug#30629] [PATCH 0/5] Detect missing modules in the initrd

[Ludovic Courtès]

Hi Danny!

Danny Milosavljevic <address@hidden> skribis:

>>   1. ‘device-module-aliases’ returns the empty list for /dev/dm-0, which
>>      is a LUKS device on my laptop.  I’m not sure what it would take to
>>      have it return “dm-crypt”, etc.  Same for RAID devices.
>
> Hmm...  I don't know either.

I browsed kmod in search of code that does that but couldn’t find it.
Do you know of another source for such things?

>>   2. Let’s assume you have: (initrd-modules '("a")).  ‘guix system’
>>      could report that module “b” is missing, even if “b” is actually a
>>      dependency of “a” and will therefore be automatically included in
>>      the initrd.  I think that’s an acceptable limitation (fixing it is
>>      non-trivial since we’d ideally need to build the target kernel so
>>      we can inspect its modules and determine their closure.)
>
> I think that's okay.

OK.

>> You’re welcome to give it a try.  In particular it’d be great if you
>> could check that ‘device-module-aliases’ returns the right thing on your
>> machine, as I shown in the example above.
>
> scheme@(guile-user)> (device-module-aliases "/dev/sda5")
> $1 = ("scsi:t-0x00" "pci:v00008086d00009C03sv000017AAsd00002214bc01sc06i01")
> scheme@(guile-user)> (device-module-aliases "/dev/sda1")
> $2 = ("scsi:t-0x00" "pci:v00008086d00009C03sv000017AAsd00002214bc01sc06i01")

Looks good.

> P.S. I just integrated my patchset (v5) and your patchset and have a working 
> system
> with pure guile initrd (modprobe is in guile, too).  I ran basic system tests 
> and
> also rebooted my machine with it :)

Damn it you’re too fast.  :-)  That’s good news though!

> Attached is the integration patch, so let's just review the patchsets
> as normal and then push both and then push the integration patch.

Do you think we could squash things to avoid the kmod-static
intermediate step when we push?

> I'm not sure about the module resolution order, first use the aliases or first
> use the real module files?

In what part?

> The Linux modules should be much more under control then...

Yes!

> From ffd464d540943e221636f7c63bcd22f4370803ae Mon Sep 17 00:00:00 2001
> From: Danny Milosavljevic <address@hidden>
> Date: Tue, 27 Feb 2018 21:25:27 +0100
> Subject: [FIXME 13/13] linux-initrd: Make modprobe pure-Guile.
> Tags: patch
>
> * gnu/build/linux-initrd.scm (build-initrd): Replace kmod by modprobe.
> * gnu/system/linux-initrd.scm (%modprobe-exp): New variable.
> (expression->initrd): Delete parameter "kmod".  Use the above.
> (raw-initrd): Replace kmod's default by "kmod".
> (base-initrd): Replace kmod's default by "kmod".
> Add LINUX-MODULES parameter again because it fell out before (?).

Awesome.  :-)

> +(define* (%modprobe-exp linux-module-directory)
> +  (with-imported-modules (source-module-closure
> +                          '((gnu build linux-modules)))
> +    #~(begin

I’d rather change that to ‘modprobe-program’ and have it return:

  (program-file "modprobe" (with-import-modules … #~(begin …)))

mostly because “file-like objects” compose better than arbitrary pieces
of code.

> +        (use-modules (gnu build linux-modules) (ice-9 getopt-long)
> +                     (ice-9 match) (srfi srfi-1))
> +        (define (lookup module)
> +          (let* ((name (ensure-dot-ko module))
> +                 (linux-release-module-directory
> +                  (string-append "/lib/modules/" (utsname:release (uname))
> +                                 "/"))

I think we can’t use ‘uname’ here because that returns info about the
build host, not about the machine and kernel we’re deploying.

> +                 (path (string-append linux-release-module-directory name)))

s/path/directory/ :-)

> +            (if (file-exists? path)
> +                path
> +                ;; FIXME: Make safe.
> +                (match (delete-duplicates (matching-modules module
> +                        (known-module-aliases
> +                         (string-append linux-release-module-directory
> +                                        "modules.alias"))))
> +                 (() #f)
> +                 ((x-name) (lookup x-name))
> +                 ((_ ...)
> +                  (error "several modules by that name"
> +                         name))))))
> +        (define option-spec
> +         '((quiet    (single-char #\q) (value #f))))
> +        (define options (getopt-long (command-line) option-spec))
> +        (for-each
> +          (lambda (option)
> +            (match option
> +             ((() modules ...)
> +              (for-each
> +                (lambda (module)
> +                  (let ((file-name (lookup module)))
> +                    (when file-name
> +                      (load-linux-module* file-name
> +                                          #:lookup-module lookup))))

Should it be an error when MODULE could not be found?

Also, indentation should be like:

  (for-each (lambda (option)
              …
              (for-each (lambda (module)
                          …)))
            …)

>  (define* (base-initrd file-systems
>                        #:key
>                        (linux linux-libre)
> +                      (linux-modules '())
>                        (kmod kmod-minimal/static)
>                        (mapped-devices '())
>                        qemu-networking?

We no longer need #:kmod here.

Thank you!

Ludo’.