Issues and improvement for `kernel-loadable-modules'

Issues and improvement for `kernel-loadable-modules'

[Brice Waegeneire]

Hello Guix,

Thanks to Danny's work in[0] we have, since a few days, a way for 
packages
to provide Linux modules in the system profile. I have been waiting for
such a feature since I packaged `ddcci-driver-linux', which was kind of
useless without it. Using the new field `kernel-loadable-modules' I
stumbled upon three issues.

First I was expecting the packages in `kernel-loadable-modules' to use 
the
`kernel' field as their kernel input or to have a simple procedure to do
so. Otherwise you get a “Specified Linux kernel and Linux kernel modules
are not all of the same version”. It makes it more difficult that it 
needs
to be to write composable configurations; IOW why would I want to use a
module built with a different kernel that the one I'm specifying in my
`operating-system'.

Second issue, I can't manage to use an inferior as the kernel input for 
the
packages in `kernel-loadable-modules'. See the two following
snippets:

--8<---------------cut here---------------start------------->8---
LANGUAGE=C guix system vm kernel-loadable-modules-barbones.scm
Updating channel 'guix' from Git repository at 
'https://git.savannah.gnu.org/git/guix.git'...
Backtrace:
            1 (primitive-load "/home/bricewge/.config/guix/current/bi…")
In guix/ui.scm:
   1894:12  0 (run-guix-command _ . _)

guix/ui.scm:1894:12: In procedure run-guix-command:
In procedure %package-native-inputs-real: Wrong type argument: 
#<inferior-package linux-libre@5.4.27 7f90570fc600>
--8<---------------cut here---------------end--------------->8---

Following is `kernel-loadable-modules-barbones.scm':

--8<---------------cut here---------------start------------->8---
(use-modules (gnu)
              (gnu system)
              (srfi srfi-1)
              (guix inferior)
              (guix utils)
              (guix packages)
              (guix channels))
(use-package-modules linux)

(define channels
   (list (channel
          (name 'guix)
          (url "https://git.savannah.gnu.org/git/guix.git")
          (commit
           "591faabd8c93bfb6879910d8a424f0db835066c2"))))

(define my-linux
   (let ((inferior (inferior-for-channels channels)))
     (first (lookup-inferior-packages inferior "linux-libre" 
"4.4.217"))))

;; ;; NOTE It is already in master
;; (define operating-system-kernel-loadable-modules
;; (@@ (gnu system) operating-system-kernel-loadable-modules))

(define os
   (operating-system
     (host-name "komputilo")
     (timezone "Europe/Berlin")
     (locale "en_US.utf8")
     (bootloader (bootloader-configuration
                  (bootloader grub-bootloader)
                  (target "/dev/sdX")))
     (file-systems (cons (file-system
                           (device (file-system-label "my-root"))
                           (mount-point "/")
                           (type "ext4"))
                         %base-file-systems))

     (kernel-loadable-modules (list ddcci-driver-linux))))

(operating-system
   (inherit os)

   (kernel my-linux)
   (kernel-loadable-modules
    (map (lambda (module)
           (package/inherit module
                            (arguments
                             (ensure-keyword-arguments
                              (package-arguments module)
                              ;; `(#:linux ,(specification->package 
"linux@4.4.217")) ; NOTE It should works
                              `(#:linux ,my-linux) ; NOTE That's issue #2
                              ))))
         (operating-system-kernel-loadable-modules os))))
--8<---------------cut here---------------end--------------->8---

And last issue, we are missing a service to load module manually when 
they
can't be auto-loaded as it's the case with `ddcci`[1]. I have managed to
solve this one by writing my first service 
`load-kernel-modules-service'.
What can I improve before submitting it as a patch -- except the missing
documentation?

--8<---------------cut here---------------start------------->8---
(define-record-type* <load-kernel-modules-configuration>
load-kernel-modules-configuration make-load-kernel-modules-configuration
load-kernel-modules-configuration? (modules
load-kernel-modules-configuration-modules ; list of strings (default 
'())))

(define load-kernel-modules-shepherd-service
   (match-lambda
     (($ <load-kernel-modules-configuration> modules)
      (list
       (shepherd-service
        (documentation "Load kernel modules.")
        (provision '(load-kernel-modules))
        (respawn? #f)
        (one-shot? #t)
        (start
         #~(lambda _
             (zero? (system* #$(file-append kmod "/bin/modprobe")
                             #$@modules)))))))))

(define load-kernel-modules-service-type
   (service-type
    (name 'load-kernel-modules)
    (description "Load kernel modules.")
    (extensions
     (list
      (service-extension shepherd-root-service-type
                         load-kernel-modules-shepherd-service)))
    (compose concatenate)
    (extend (lambda (config modules)
              (load-kernel-modules-configuration
               (inherit config)
               (modules (append
                         (load-kernel-modules-configuration-modules 
config)
                         modules)))))
    (default-value (load-kernel-modules-configuration))))

(define* (load-kernel-modules-service modules)
   "Return a service that loads kernel MODULES."
   (service load-kernel-modules-service-type
            (load-kernel-modules-configuration
             (modules modules))))
--8<---------------cut here---------------end--------------->8---

[0]: https://issues.guix.info/issue/37868
[1]: 
https://gitlab.com/ddcci-driver-linux/ddcci-driver-linux#ddcci-bus-driver

Brice.

Re: Issues and improvement for `kernel-loadable-modules'

[Danny Milosavljevic]

[-- Attachment #1: Type: text/plain, Size: 1787 bytes --]

Hi Brice,

On Thu, 26 Mar 2020 14:34:03 +0000
Brice Waegeneire <brice@waegenei.re> wrote:

> First I was expecting the packages in `kernel-loadable-modules' to use 
> the
> `kernel' field as their kernel input or to have a simple procedure to do
> so. Otherwise you get a “Specified Linux kernel and Linux kernel modules
> are not all of the same version”. It makes it more difficult that it 
> needs
> to be to write composable configurations; IOW why would I want to use a
> module built with a different kernel that the one I'm specifying in my
> `operating-system'.

Because packages are modular, changing the build system can only be done
retroactively by a procedure.  We could totally do that but it would make
the kernel-loadable-modules field more magical and more difficult to debug.
Also, I guess it would hard-code linux-module-build-system for those
(right now you can use whatever build system you want).  Do we want it anyway?

> And last issue, we are missing a service to load module manually when 
> they
> can't be auto-loaded as it's the case with `ddcci`[1]. I have managed to
> solve this one by writing my first service 
> `load-kernel-modules-service'.
> What can I improve before submitting it as a patch -- except the missing
> documentation?

I think that things should be described by nouns and actions should be
described by verbs.

So "load-kernel-modules-service" sounds really wrong to me.
Maybe "kernel-module-loader-service"?

Others don't care so much because Scheme kinda erodes the boundary anyway.

Otherwise looks good to me.

I guess it could be nice to be able to extend this service from other services
using (service-extension load-kernel-modules-service-type '("module1"
"module2")) or something.

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: Issues and improvement for `kernel-loadable-modules'

[Brice Waegeneire]

On 2020-03-26 15:13, Danny Milosavljevic wrote:
> Hi Brice,
> 
> On Thu, 26 Mar 2020 14:34:03 +0000
> Brice Waegeneire <brice@waegenei.re> wrote:
> 
>> First I was expecting the packages in `kernel-loadable-modules' to use
>> the
>> `kernel' field as their kernel input or to have a simple procedure to 
>> do
>> so. Otherwise you get a “Specified Linux kernel and Linux kernel 
>> modules
>> are not all of the same version”. It makes it more difficult that it
>> needs
>> to be to write composable configurations; IOW why would I want to use 
>> a
>> module built with a different kernel that the one I'm specifying in my
>> `operating-system'.
> 
> Because packages are modular, changing the build system can only be 
> done
> retroactively by a procedure.  We could totally do that but it would 
> make
> the kernel-loadable-modules field more magical and more difficult to 
> debug.
> Also, I guess it would hard-code linux-module-build-system for those
> (right now you can use whatever build system you want).  Do we want it 
> anyway?
> 
>> And last issue, we are missing a service to load module manually when
>> they
>> can't be auto-loaded as it's the case with `ddcci`[1]. I have managed 
>> to
>> solve this one by writing my first service
>> `load-kernel-modules-service'.
>> What can I improve before submitting it as a patch -- except the 
>> missing
>> documentation?
> 
> I think that things should be described by nouns and actions should be
> described by verbs.
> 
> So "load-kernel-modules-service" sounds really wrong to me.
> Maybe "kernel-module-loader-service"?
> 
> Others don't care so much because Scheme kinda erodes the boundary 
> anyway.
> 
> Otherwise looks good to me.
> 
> I guess it could be nice to be able to extend this service from other 
> services
> using (service-extension load-kernel-modules-service-type '("module1"
> "module2")) or something.

Re: Issues and improvement for `kernel-loadable-modules'

[Brice Waegeneire]

Hello Danny,

Sorry for the empty email; cancel and send buttons were too close for
me...

On 2020-03-26 15:13, Danny Milosavljevic wrote:
> Hi Brice,
> 
> On Thu, 26 Mar 2020 14:34:03 +0000
> Brice Waegeneire <brice@waegenei.re> wrote:
> 
>> First I was expecting the packages in `kernel-loadable-modules' to use
>> the
>> `kernel' field as their kernel input or to have a simple procedure to 
>> do
>> so. Otherwise you get a “Specified Linux kernel and Linux kernel 
>> modules
>> are not all of the same version”. It makes it more difficult that it
>> needs
>> to be to write composable configurations; IOW why would I want to use 
>> a
>> module built with a different kernel that the one I'm specifying in my
>> `operating-system'.
> 
> Because packages are modular, changing the build system can only be 
> done
> retroactively by a procedure.  We could totally do that but it would 
> make
> the kernel-loadable-modules field more magical and more difficult to 
> debug.
> Also, I guess it would hard-code linux-module-build-system for those
> (right now you can use whatever build system you want).  Do we want it 
> anyway?

Magic isn't the way to go, for sure. I tried writting a procedure to 
abstract
it, but I wasn't successful: see my second issue with inferiors, the 
fact that
it only handle `linux-build-system' and that I need to use a direct 
reference to
a package. On that last point I would like to use something like
“(operating-system-kernel this-operating-system)” to reference the 
kernel
but my guile foo isn't good enough: “this-operating-system” has to be in 
scope
  of “operating-system”. Following is the procedure I'm talking about:

--8<---------------cut here---------------start------------->8---
   (kernel-loadable-modules
    (map (lambda (module)
           (package/inherit module
                            (arguments
                             (ensure-keyword-arguments
                              (package-arguments module)
                              `(#:linux ,my-linux)
                              ))))
--8<---------------cut here---------------end--------------->8---


>> And last issue, we are missing a service to load module manually when
>> they
>> can't be auto-loaded as it's the case with `ddcci`[1]. I have managed 
>> to
>> solve this one by writing my first service
>> `load-kernel-modules-service'.
>> What can I improve before submitting it as a patch -- except the 
>> missing
>> documentation?
> 
> I think that things should be described by nouns and actions should be
> described by verbs.
> 
> So "load-kernel-modules-service" sounds really wrong to me.
> Maybe "kernel-module-loader-service"?
> 
> Others don't care so much because Scheme kinda erodes the boundary 
> anyway.

That's definitely a better name.

> Otherwise looks good to me.
> 
> I guess it could be nice to be able to extend this service from other 
> services
> using (service-extension load-kernel-modules-service-type '("module1"
> "module2")) or something.

I think it's already the case, there are `compose' and `extend' fields 
in the
`service-type' and I tried it with
“(simple-service 'ddcci-module kernel-module-loader-service-type 
'("ddcci"))”.

Brice.