GNU bug report logs - #40274
[PATCH] gnu: Add kernel-module-loader-service.

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 40274 in the body.
You can then email your comments to 40274 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: Brice Waegeneire <brice <at> waegenei.re>
To: guix-patches <at> gnu.org
Subject: [PATCH] gnu: Add kernel-module-loader-service.
Date: Sat, 28 Mar 2020 14:59:08 +0100

* doc/guix.texi (Linux Services): Add a new subsection and document the
new service and its configuration.
* gnu/services/linux.scm (kernel-module-loader-service): New procedure.
(kernel-module-loader-service-type, kernel-module-loader-configuration):
New types.
---

This patch is related to the following thread:
https://lists.gnu.org/archive/html/guix-devel/2020-03/msg00381.html

 doc/guix.texi          | 48 ++++++++++++++++++++++++++++++++++
 gnu/services/linux.scm | 58 +++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 105 insertions(+), 1 deletion(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 77a4b8ed71..a297ec249f 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -75,6 +75,7 @@ Copyright @copyright{} 2020 Wiktor Żelazny@*
 Copyright @copyright{} 2020 Damien Cassou@*
 Copyright @copyright{} 2020 Jakub Kądziołka@*
 Copyright @copyright{} 2020 Jack Hill@*
+Copyright @copyright{} 2020 Brice Waegeneire@*
 
 Permission is granted to copy, distribute and/or modify this document
 under the terms of the GNU Free Documentation License, Version 1.3 or
@@ -25382,6 +25383,53 @@ notifications.
 @end table
 @end deftp
 
+@cindex modprobe
+@cindex kernel module loader
+@subsubsection Kernel Module Loader Service
+
+The @code{kernel-module-loader-service} provides a service to load
+kernel modules at boot.  This is especially usefull for modules that
+don't autoload and need to be manually loaded, like it's the case with
+@code{ddcci} for example.
+
+@deffn {Scheme Procedure} kernel-module-loader-service @var{modules}
+Return a service that run @command{modprobe} with arguments
+@var{modules} at boot.  For example loading the drivers provided by
+@code{ddci-driver-linux} can be done as follow:
+
+@lisp
+(use-modules (gnu))
+(use-package-modules linux)
+(use-service-modules linux)
+(operating-system
+  ...
+  (services (cons* (kernel-module-loader-service
+                     '("ddcci""ddcci_backlight"))
+                   %base-services))
+  (kernel-loadable-modules (list ddcci-driver-linux)))
+@end lisp
+@end defvr
+
+@deffn {Scheme Variable} kernel-module-loader-service-type
+The service type for loading kernel modules, that can't autoload, at
+boot, @command{modprobe}, on.  Its value must be a
+@code{kernel-module-loader-configuration} object, described below.
+
+@deftp {Data Type} kernel-module-loader-configuration
+The data type representing the configuration of @command{sysctl}.
+
+@deftp {Data Type} kernel-module-loader-configuration
+This is the configuration record for the
+@code{kernel-module-loader-service-type}.
+
+@table @asis
+@item @code{modprobe} (default: @code{(file-append kmod "/bin/modprobe"})
+The @command{modprobe} executable to use.
+
+@item @code{modules} (default: @code{'()})
+A list specifying the modules to load.
+@end table
+@end deftp
 
 @node Miscellaneous Services
 @subsection Miscellaneous Services
diff --git a/gnu/services/linux.scm b/gnu/services/linux.scm
index caa0326c31..99ab4a3329 100644
--- a/gnu/services/linux.scm
+++ b/gnu/services/linux.scm
@@ -1,5 +1,6 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2020 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
+;;; Copyright © 2020 Brice Waegeneire <brice <at> waegenei.re>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -37,7 +38,14 @@
             earlyoom-configuration-ignore-positive-oom-score-adj?
             earlyoom-configuration-show-debug-messages?
             earlyoom-configuration-send-notification-command
-            earlyoom-service-type))
+            earlyoom-service-type
+
+            kernel-module-loader-configuration
+            kernel-module-loader-configuration?
+            kernel-module-loader-configuration-modprobe
+            kernel-module-loader-configuration-modules
+            kernel-module-loader-service-type
+            kernel-module-loader-service))
 
 
 ;;;
@@ -123,3 +131,51 @@ representation."
     (list (service-extension shepherd-root-service-type
                              (compose list earlyoom-shepherd-service))))
    (description "Run @command{earlyoom}, the Early OOM daemon.")))
+
+;;;
+;;; Kernel module loader.
+;;;
+
+(define-record-type* <kernel-module-loader-configuration>
+  kernel-module-loader-configuration make-kernel-module-loader-configuration
+  kernel-module-loader-configuration?
+  (modprobe kernel-module-loader-configuration-modprobe ; path of the 'modprobe' command
+            (default (file-append kmod "/bin/modprobe")))
+  (modules kernel-module-loader-configuration-modules ; list of strings
+           (default '())))
+
+(define kernel-module-loader-shepherd-service
+  (match-lambda
+    (($ <kernel-module-loader-configuration> modprobe modules)
+     (list
+      (shepherd-service
+       (documentation "Load kernel modules.")
+       (provision '(kernel-module-loader))
+       (respawn? #f)
+       (one-shot? #t)
+       (start
+        #~(lambda _
+            (zero? (system* #$modprobe #$@modules)))))))))
+
+(define kernel-module-loader-service-type
+  (service-type
+   (name 'kernel-module-loader)
+   (description "Load kernel modules.")
+   (extensions
+    (list
+     (service-extension shepherd-root-service-type
+                        kernel-module-loader-shepherd-service)))
+   (compose concatenate)
+   (extend (lambda (config modules)
+             (kernel-module-loader-configuration
+              (inherit config)
+              (modules (append
+                        (kernel-module-loader-configuration-modules config)
+                        modules)))))
+   (default-value (kernel-module-loader-configuration))))
+
+(define* (kernel-module-loader-service modules)
+  "Return a service that loads kernel MODULES."
+  (service kernel-module-loader-service-type
+           (kernel-module-loader-configuration
+            (modules modules))))
-- 
2.25.1

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

From: Mathieu Othacehe <m.othacehe <at> gmail.com>
To: guix-patches <at> gnu.org
Cc: 40274 <at> debbugs.gnu.org
Subject: Re: [bug#40274] [PATCH] gnu: Add kernel-module-loader-service.
Date: Sat, 28 Mar 2020 21:51:09 +0100

Hello Brice,

Thanks for this patch! I think you could add this service to (gnu tests
linux-modules). This way we would have a full-coverage of the mechanism.

Mathieu

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

From: Brice Waegeneire <brice <at> waegenei.re>
To: guix-patches <at> gnu.org
Subject: [PATCH v2] gnu: Add kernel-module-loader-service.
Date: Tue, 31 Mar 2020 19:19:00 +0200

* doc/guix.texi (Linux Services): Add a new subsection and document the
new service and its configuration.
* gnu/services/linux.scm (kernel-module-loader-service): New procedure.
(kernel-module-loader-service-type, kernel-module-loader-configuration):
New types.
* gnu/tests/linux-modules.scm (module-loader-program): Procedure
removed.
(modules-loaded?-program): New procedure.
(run-loadable-kernel-modules-test): 'module-loader-program' procedure
replaced by the new one. New format for last arguments
[os]: Use 'kernel-module-loader-service'.
(%test-loadable-kernel-modules-0, %test-loadable-kernel-modules-1,
%test-loadable-kernel-modules-2): Use the new argument format.
---
 doc/guix.texi               | 52 +++++++++++++++++++++++++++++++
 gnu/services/linux.scm      | 61 ++++++++++++++++++++++++++++++++++++-
 gnu/tests/linux-modules.scm | 45 ++++++++++++++++-----------
 3 files changed, 140 insertions(+), 18 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 77a4b8ed71..e8ad830102 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -75,6 +75,7 @@ Copyright @copyright{} 2020 Wiktor Żelazny@*
 Copyright @copyright{} 2020 Damien Cassou@*
 Copyright @copyright{} 2020 Jakub Kądziołka@*
 Copyright @copyright{} 2020 Jack Hill@*
+Copyright @copyright{} 2020 Brice Waegeneire@*
 
 Permission is granted to copy, distribute and/or modify this document
 under the terms of the GNU Free Documentation License, Version 1.3 or
@@ -25382,6 +25383,57 @@ notifications.
 @end table
 @end deftp
 
+@cindex modprobe
+@cindex kernel module loader
+@subsubsection Kernel Module Loader Service
+
+The @code{kernel-module-loader-service} provides a service to load
+kernel modules at boot.  This is especially usefull for modules that
+don't autoload and need to be manually loaded, like it's the case with
+@code{ddcci} for example.
+
+@deffn {Scheme Procedure} kernel-module-loader-service @var{modules}
+Return a service that run @command{modprobe} with arguments
+@var{modules} at boot.  For example loading the drivers provided by
+@code{ddci-driver-linux} with @code{ddcci} in debugging mode can be done
+as follow:
+
+@lisp
+(use-modules (gnu))
+(use-package-modules linux)
+(use-service-modules linux)
+(operating-system
+  ...
+  (services (cons* (kernel-module-loader-service
+                     '(("ddcci" "dyndbg")
+                       ("ddcci_backlight")))
+                   %base-services))
+  (kernel-loadable-modules (list ddcci-driver-linux)))
+@end lisp
+@end defvr
+
+@deffn {Scheme Variable} kernel-module-loader-service-type
+The service type for loading kernel modules at boot with
+@command{modprobe}, for module that can't autoload.  Its value must be a
+@code{kernel-module-loader-configuration} object, described below.
+
+@deftp {Data Type} kernel-module-loader-configuration
+The data type representing the modules to be loaded by
+@command{modprobe}.
+
+@deftp {Data Type} kernel-module-loader-configuration
+This is the configuration record for the
+@code{kernel-module-loader-service-type}.
+
+@table @asis
+@item @code{modprobe} (default: @code{(file-append kmod "/bin/modprobe"})
+The @command{modprobe} executable to use.
+
+@item @code{modules} (default: @code{'()})
+A list of list of strings specifying the modules to load with their
+optional parameters.
+@end table
+@end deftp
 
 @node Miscellaneous Services
 @subsection Miscellaneous Services
diff --git a/gnu/services/linux.scm b/gnu/services/linux.scm
index caa0326c31..da71b275f1 100644
--- a/gnu/services/linux.scm
+++ b/gnu/services/linux.scm
@@ -1,5 +1,6 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2020 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
+;;; Copyright © 2020 Brice Waegeneire <brice <at> waegenei.re>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -37,7 +38,14 @@
             earlyoom-configuration-ignore-positive-oom-score-adj?
             earlyoom-configuration-show-debug-messages?
             earlyoom-configuration-send-notification-command
-            earlyoom-service-type))
+            earlyoom-service-type
+
+            kernel-module-loader-configuration
+            kernel-module-loader-configuration?
+            kernel-module-loader-configuration-modprobe
+            kernel-module-loader-configuration-modules
+            kernel-module-loader-service-type
+            kernel-module-loader-service))
 
 
 ;;;
@@ -123,3 +131,54 @@ representation."
     (list (service-extension shepherd-root-service-type
                              (compose list earlyoom-shepherd-service))))
    (description "Run @command{earlyoom}, the Early OOM daemon.")))
+
+;;;
+;;; Kernel module loader.
+;;;
+
+(define-record-type* <kernel-module-loader-configuration>
+  kernel-module-loader-configuration make-kernel-module-loader-configuration
+  kernel-module-loader-configuration?
+  (modprobe kernel-module-loader-configuration-modprobe ; path of the 'modprobe' command
+            (default (file-append kmod "/bin/modprobe")))
+  (modules kernel-module-loader-configuration-modules ; list of lists of strings
+           (default '())))
+
+(define kernel-module-loader-shepherd-service
+  (match-lambda
+    (($ <kernel-module-loader-configuration> modprobe modules)
+     (list
+      (shepherd-service
+       (documentation "Load kernel modules.")
+       (provision '(kernel-module-loader))
+       (respawn? #f)
+       (one-shot? #t)
+       (start
+        #~(lambda _
+            (and-map (lambda (module-with-parameters)
+                   (zero? (apply system* #$modprobe "--"
+                                 module-with-parameters)))
+                 '#$modules))))))))
+
+(define kernel-module-loader-service-type
+  (service-type
+   (name 'kernel-module-loader)
+   (description "Load kernel modules.")
+   (extensions
+    (list
+     (service-extension shepherd-root-service-type
+                        kernel-module-loader-shepherd-service)))
+   (compose concatenate)
+   (extend (lambda (config modules)
+             (kernel-module-loader-configuration
+              (inherit config)
+              (modules (append
+                        (kernel-module-loader-configuration-modules config)
+                        modules)))))
+   (default-value (kernel-module-loader-configuration))))
+
+(define* (kernel-module-loader-service modules)
+  "Return a service that loads kernel MODULES."
+  (service kernel-module-loader-service-type
+           (kernel-module-loader-configuration
+            (modules modules))))
diff --git a/gnu/tests/linux-modules.scm b/gnu/tests/linux-modules.scm
index 39e11587c6..7b820a1269 100644
--- a/gnu/tests/linux-modules.scm
+++ b/gnu/tests/linux-modules.scm
@@ -1,6 +1,7 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2019 Jakob L. Kreuze <zerodaysfordays <at> sdf.org>
 ;;; Copyright © 2020 Danny Milosavljevic <dannym <at> scratchpost.org>
+;;; Copyright © 2020 Brice Waegeneire <brice <at> waegenei.re>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -19,6 +20,7 @@
 
 (define-module (gnu tests linux-modules)
   #:use-module (gnu packages linux)
+  #:use-module (gnu services linux)
   #:use-module (gnu system)
   #:use-module (gnu system vm)
   #:use-module (gnu tests)
@@ -37,25 +39,31 @@
 ;;;
 ;;; Code:
 
-(define* (module-loader-program os modules)
-  "Return an executable store item that, upon being evaluated, will dry-run
-load MODULES."
+(define* (modules-loaded?-program os modules)
+  "Return an executable store item that, upon being evaluated, will verify
+that MODULES are actually loaded."
   (program-file
    "load-kernel-modules.scm"
-   (with-imported-modules (source-module-closure '((guix build utils)))
-     #~(begin
-         (use-modules (guix build utils))
-         (for-each (lambda (module)
-                     (invoke (string-append #$kmod "/bin/modprobe") "-n" "--"
-                             module))
-                   '#$modules)))))
+   #~(begin
+     (use-modules (ice-9 rdelim)
+                  (ice-9 popen))
+     (let* ((port (open-input-pipe (string-append #$kmod "/bin/lsmod")))
+            (output (read-string port))
+            (status (close-pipe port)))
+       (and-map (lambda (module) (string-contains output module))
+                '#$modules)))))
 
-(define* (run-loadable-kernel-modules-test module-packages module-names)
-  "Run a test of an OS having MODULE-PACKAGES, and modprobe MODULE-NAMES."
+(define* (run-loadable-kernel-modules-test module-packages
+                                           modules-with-parameters)
+  "Run a test of an OS having MODULE-PACKAGES, and verify that
+MODULES-WITH-PARAMETERS are loaded in memory."
   (define os
     (marionette-operating-system
      (operating-system
       (inherit (simple-operating-system))
+      (services (cons (kernel-module-loader-service modules-with-parameters)
+                      (operating-system-user-services
+                       (simple-operating-system))))
       (kernel-loadable-modules module-packages))
      #:imported-modules '((guix combinators))))
   (define vm (virtual-machine os))
@@ -75,7 +83,9 @@ load MODULES."
              marionette))
           (test-end)
           (exit (= (test-runner-fail-count (test-runner-current)) 0)))))
-  (gexp->derivation "loadable-kernel-modules" (test (module-loader-program os module-names))))
+  (gexp->derivation
+   "loadable-kernel-modules"
+   (test (modules-loaded?-program os (map car modules-with-parameters)))))
 
 (define %test-loadable-kernel-modules-0
   (system-test
@@ -88,16 +98,17 @@ with no extra modules.")
   (system-test
    (name "loadable-kernel-modules-1")
    (description "Tests loadable kernel modules facility of <operating-system>
-with one extra module.")
+with one extra module with some parameters.")
    (value (run-loadable-kernel-modules-test
            (list ddcci-driver-linux)
-           '("ddcci")))))
+           '(("ddcci" "dyndbg" "delay:120"))))))
 
 (define %test-loadable-kernel-modules-2
   (system-test
    (name "loadable-kernel-modules-2")
    (description "Tests loadable kernel modules facility of <operating-system>
-with two extra modules.")
+with two extra modules without any parameters.")
    (value (run-loadable-kernel-modules-test
            (list acpi-call-linux-module ddcci-driver-linux)
-           '("acpi_call" "ddcci")))))
+           '(("acpi_call")
+             ("ddcci"))))))
-- 
2.25.1

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

From: Brice Waegeneire <brice <at> waegenei.re>
To: Mathieu Othacehe <m.othacehe <at> gmail.com>
Cc: 40274 <at> debbugs.gnu.org,
 Guix-patches <guix-patches-bounces+brice+lists=waegenei.re <at> gnu.org>
Subject: Re: [bug#40274] [PATCH] gnu: Add kernel-module-loader-service.
Date: Tue, 31 Mar 2020 17:22:01 +0000

Hello Mathieu,

On 2020-03-28 20:51, Mathieu Othacehe wrote:
> Hello Brice,
> 
> Thanks for this patch! I think you could add this service to (gnu tests
> linux-modules). This way we would have a full-coverage of the 
> mechanism.
> 
> Mathieu

I have done what you suggested in v2. It also fix some issues and add 
the
ability to optionally specify parameters for each module.

- Brice

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

From: Brice Waegeneire <brice <at> waegenei.re>
To: guix-patches <at> gnu.org
Subject: [PATCH v3] gnu: Add kernel-module-loader-service.
Date: Tue, 31 Mar 2020 22:00:37 +0200

* doc/guix.texi (Linux Services): Add a new subsection and document the
new service and its configuration.
* gnu/services/linux.scm (kernel-module-loader-service): New procedure.
(kernel-module-loader-service-type, kernel-module-loader-configuration):
New types.
* gnu/tests/linux-modules.scm (module-loader-program): Procedure
removed.
(modules-loaded?-program): New procedure.
(run-loadable-kernel-modules-test): 'module-loader-program' procedure
replaced by the new one. New format for last arguments
[os]: Use 'kernel-module-loader-service'.
(%test-loadable-kernel-modules-0, %test-loadable-kernel-modules-1,
%test-loadable-kernel-modules-2): Use the new argument format.
---

This version fix some markup issue in doc/guix.texi.

 doc/guix.texi               | 54 ++++++++++++++++++++++++++++++++
 gnu/services/linux.scm      | 61 ++++++++++++++++++++++++++++++++++++-
 gnu/tests/linux-modules.scm | 45 ++++++++++++++++-----------
 3 files changed, 142 insertions(+), 18 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 77a4b8ed71..83c893ddb9 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -75,6 +75,7 @@ Copyright @copyright{} 2020 Wiktor Żelazny@*
 Copyright @copyright{} 2020 Damien Cassou@*
 Copyright @copyright{} 2020 Jakub Kądziołka@*
 Copyright @copyright{} 2020 Jack Hill@*
+Copyright @copyright{} 2020 Brice Waegeneire@*
 
 Permission is granted to copy, distribute and/or modify this document
 under the terms of the GNU Free Documentation License, Version 1.3 or
@@ -25382,6 +25383,59 @@ notifications.
 @end table
 @end deftp
 
+@cindex modprobe
+@cindex kernel module loader
+@subsubsection Kernel Module Loader Service
+
+The @code{kernel-module-loader-service} provides a service to load
+kernel modules at boot.  This is especially usefull for modules that
+don't autoload and need to be manually loaded, like it's the case with
+@code{ddcci} for example.
+
+@deffn {Scheme Procedure} kernel-module-loader-service @var{modules}
+Return a service that run @command{modprobe} with arguments
+@var{modules} at boot.  For example loading the drivers provided by
+@code{ddci-driver-linux} with @code{ddcci} in debugging mode can be done
+as follow:
+
+@lisp
+(use-modules (gnu))
+(use-package-modules linux)
+(use-service-modules linux)
+(operating-system
+  ...
+  (services (cons* (kernel-module-loader-service
+                     '(("ddcci" "dyndbg")
+                       ("ddcci_backlight")))
+                   %base-services))
+  (kernel-loadable-modules (list ddcci-driver-linux)))
+@end lisp
+@end deffn
+
+@deffn {Scheme Variable} kernel-module-loader-service-type
+The service type for loading kernel modules at boot with
+@command{modprobe}, for module that can't autoload.  Its value must be a
+@code{kernel-module-loader-configuration} object, described below.
+@end deffn
+
+@deftp {Data Type} kernel-module-loader-configuration
+The data type representing the modules to be loaded by
+@command{modprobe}.
+@end deftp
+
+@deftp {Data Type} kernel-module-loader-configuration
+This is the configuration record for the
+@code{kernel-module-loader-service-type}.
+
+@table @asis
+@item @code{modprobe} (default: @code{(file-append kmod "/bin/modprobe"})
+The @command{modprobe} executable to use.
+
+@item @code{modules} (default: @code{'()})
+A list of list of strings specifying the modules to load with their
+optional parameters.
+@end table
+@end deftp
 
 @node Miscellaneous Services
 @subsection Miscellaneous Services
diff --git a/gnu/services/linux.scm b/gnu/services/linux.scm
index caa0326c31..da71b275f1 100644
--- a/gnu/services/linux.scm
+++ b/gnu/services/linux.scm
@@ -1,5 +1,6 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2020 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
+;;; Copyright © 2020 Brice Waegeneire <brice <at> waegenei.re>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -37,7 +38,14 @@
             earlyoom-configuration-ignore-positive-oom-score-adj?
             earlyoom-configuration-show-debug-messages?
             earlyoom-configuration-send-notification-command
-            earlyoom-service-type))
+            earlyoom-service-type
+
+            kernel-module-loader-configuration
+            kernel-module-loader-configuration?
+            kernel-module-loader-configuration-modprobe
+            kernel-module-loader-configuration-modules
+            kernel-module-loader-service-type
+            kernel-module-loader-service))
 
 
 ;;;
@@ -123,3 +131,54 @@ representation."
     (list (service-extension shepherd-root-service-type
                              (compose list earlyoom-shepherd-service))))
    (description "Run @command{earlyoom}, the Early OOM daemon.")))
+
+;;;
+;;; Kernel module loader.
+;;;
+
+(define-record-type* <kernel-module-loader-configuration>
+  kernel-module-loader-configuration make-kernel-module-loader-configuration
+  kernel-module-loader-configuration?
+  (modprobe kernel-module-loader-configuration-modprobe ; path of the 'modprobe' command
+            (default (file-append kmod "/bin/modprobe")))
+  (modules kernel-module-loader-configuration-modules ; list of lists of strings
+           (default '())))
+
+(define kernel-module-loader-shepherd-service
+  (match-lambda
+    (($ <kernel-module-loader-configuration> modprobe modules)
+     (list
+      (shepherd-service
+       (documentation "Load kernel modules.")
+       (provision '(kernel-module-loader))
+       (respawn? #f)
+       (one-shot? #t)
+       (start
+        #~(lambda _
+            (and-map (lambda (module-with-parameters)
+                   (zero? (apply system* #$modprobe "--"
+                                 module-with-parameters)))
+                 '#$modules))))))))
+
+(define kernel-module-loader-service-type
+  (service-type
+   (name 'kernel-module-loader)
+   (description "Load kernel modules.")
+   (extensions
+    (list
+     (service-extension shepherd-root-service-type
+                        kernel-module-loader-shepherd-service)))
+   (compose concatenate)
+   (extend (lambda (config modules)
+             (kernel-module-loader-configuration
+              (inherit config)
+              (modules (append
+                        (kernel-module-loader-configuration-modules config)
+                        modules)))))
+   (default-value (kernel-module-loader-configuration))))
+
+(define* (kernel-module-loader-service modules)
+  "Return a service that loads kernel MODULES."
+  (service kernel-module-loader-service-type
+           (kernel-module-loader-configuration
+            (modules modules))))
diff --git a/gnu/tests/linux-modules.scm b/gnu/tests/linux-modules.scm
index 39e11587c6..7b820a1269 100644
--- a/gnu/tests/linux-modules.scm
+++ b/gnu/tests/linux-modules.scm
@@ -1,6 +1,7 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2019 Jakob L. Kreuze <zerodaysfordays <at> sdf.org>
 ;;; Copyright © 2020 Danny Milosavljevic <dannym <at> scratchpost.org>
+;;; Copyright © 2020 Brice Waegeneire <brice <at> waegenei.re>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -19,6 +20,7 @@
 
 (define-module (gnu tests linux-modules)
   #:use-module (gnu packages linux)
+  #:use-module (gnu services linux)
   #:use-module (gnu system)
   #:use-module (gnu system vm)
   #:use-module (gnu tests)
@@ -37,25 +39,31 @@
 ;;;
 ;;; Code:
 
-(define* (module-loader-program os modules)
-  "Return an executable store item that, upon being evaluated, will dry-run
-load MODULES."
+(define* (modules-loaded?-program os modules)
+  "Return an executable store item that, upon being evaluated, will verify
+that MODULES are actually loaded."
   (program-file
    "load-kernel-modules.scm"
-   (with-imported-modules (source-module-closure '((guix build utils)))
-     #~(begin
-         (use-modules (guix build utils))
-         (for-each (lambda (module)
-                     (invoke (string-append #$kmod "/bin/modprobe") "-n" "--"
-                             module))
-                   '#$modules)))))
+   #~(begin
+     (use-modules (ice-9 rdelim)
+                  (ice-9 popen))
+     (let* ((port (open-input-pipe (string-append #$kmod "/bin/lsmod")))
+            (output (read-string port))
+            (status (close-pipe port)))
+       (and-map (lambda (module) (string-contains output module))
+                '#$modules)))))
 
-(define* (run-loadable-kernel-modules-test module-packages module-names)
-  "Run a test of an OS having MODULE-PACKAGES, and modprobe MODULE-NAMES."
+(define* (run-loadable-kernel-modules-test module-packages
+                                           modules-with-parameters)
+  "Run a test of an OS having MODULE-PACKAGES, and verify that
+MODULES-WITH-PARAMETERS are loaded in memory."
   (define os
     (marionette-operating-system
      (operating-system
       (inherit (simple-operating-system))
+      (services (cons (kernel-module-loader-service modules-with-parameters)
+                      (operating-system-user-services
+                       (simple-operating-system))))
       (kernel-loadable-modules module-packages))
      #:imported-modules '((guix combinators))))
   (define vm (virtual-machine os))
@@ -75,7 +83,9 @@ load MODULES."
              marionette))
           (test-end)
           (exit (= (test-runner-fail-count (test-runner-current)) 0)))))
-  (gexp->derivation "loadable-kernel-modules" (test (module-loader-program os module-names))))
+  (gexp->derivation
+   "loadable-kernel-modules"
+   (test (modules-loaded?-program os (map car modules-with-parameters)))))
 
 (define %test-loadable-kernel-modules-0
   (system-test
@@ -88,16 +98,17 @@ with no extra modules.")
   (system-test
    (name "loadable-kernel-modules-1")
    (description "Tests loadable kernel modules facility of <operating-system>
-with one extra module.")
+with one extra module with some parameters.")
    (value (run-loadable-kernel-modules-test
            (list ddcci-driver-linux)
-           '("ddcci")))))
+           '(("ddcci" "dyndbg" "delay:120"))))))
 
 (define %test-loadable-kernel-modules-2
   (system-test
    (name "loadable-kernel-modules-2")
    (description "Tests loadable kernel modules facility of <operating-system>
-with two extra modules.")
+with two extra modules without any parameters.")
    (value (run-loadable-kernel-modules-test
            (list acpi-call-linux-module ddcci-driver-linux)
-           '("acpi_call" "ddcci")))))
+           '(("acpi_call")
+             ("ddcci"))))))
-- 
2.25.1

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

From: Nicolò Balzarotti <anothersms <at> gmail.com>
To: Brice Waegeneire <brice <at> waegenei.re>, 40274 <at> debbugs.gnu.org
Subject: Re: [bug#40274] [PATCH] gnu: Add kernel-module-loader-service.
Date: Tue, 31 Mar 2020 22:10:30 +0200

Hi, I found a typo

/usefull/useful/

Nicolò

Brice Waegeneire <brice <at> waegenei.re> writes:

> * doc/guix.texi (Linux Services): Add a new subsection and document the
> new service and its configuration.
> * gnu/services/linux.scm (kernel-module-loader-service): New procedure.
> (kernel-module-loader-service-type, kernel-module-loader-configuration):
> New types.
> ---
>
> This patch is related to the following thread:
> https://lists.gnu.org/archive/html/guix-devel/2020-03/msg00381.html
>
>  doc/guix.texi          | 48 ++++++++++++++++++++++++++++++++++
>  gnu/services/linux.scm | 58 +++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 105 insertions(+), 1 deletion(-)
>
> diff --git a/doc/guix.texi b/doc/guix.texi
> index 77a4b8ed71..a297ec249f 100644
> --- a/doc/guix.texi
> +++ b/doc/guix.texi
> @@ -75,6 +75,7 @@ Copyright @copyright{} 2020 Wiktor Żelazny@*
>  Copyright @copyright{} 2020 Damien Cassou@*
>  Copyright @copyright{} 2020 Jakub Kądziołka@*
>  Copyright @copyright{} 2020 Jack Hill@*
> +Copyright @copyright{} 2020 Brice Waegeneire@*
>  
>  Permission is granted to copy, distribute and/or modify this document
>  under the terms of the GNU Free Documentation License, Version 1.3 or
> @@ -25382,6 +25383,53 @@ notifications.
>  @end table
>  @end deftp
>  
> +@cindex modprobe
> +@cindex kernel module loader
> +@subsubsection Kernel Module Loader Service
> +
> +The @code{kernel-module-loader-service} provides a service to load
> +kernel modules at boot.  This is especially usefull for modules that
> +don't autoload and need to be manually loaded, like it's the case with
> +@code{ddcci} for example.
> +
> +@deffn {Scheme Procedure} kernel-module-loader-service @var{modules}
> +Return a service that run @command{modprobe} with arguments
> +@var{modules} at boot.  For example loading the drivers provided by
> +@code{ddci-driver-linux} can be done as follow:
> +
> +@lisp
> +(use-modules (gnu))
> +(use-package-modules linux)
> +(use-service-modules linux)
> +(operating-system
> +  ...
> +  (services (cons* (kernel-module-loader-service
> +                     '("ddcci""ddcci_backlight"))
> +                   %base-services))
> +  (kernel-loadable-modules (list ddcci-driver-linux)))
> +@end lisp
> +@end defvr
> +
> +@deffn {Scheme Variable} kernel-module-loader-service-type
> +The service type for loading kernel modules, that can't autoload, at
> +boot, @command{modprobe}, on.  Its value must be a
> +@code{kernel-module-loader-configuration} object, described below.
> +
> +@deftp {Data Type} kernel-module-loader-configuration
> +The data type representing the configuration of @command{sysctl}.
> +
> +@deftp {Data Type} kernel-module-loader-configuration
> +This is the configuration record for the
> +@code{kernel-module-loader-service-type}.
> +
> +@table @asis
> +@item @code{modprobe} (default: @code{(file-append kmod "/bin/modprobe"})
> +The @command{modprobe} executable to use.
> +
> +@item @code{modules} (default: @code{'()})
> +A list specifying the modules to load.
> +@end table
> +@end deftp
>  
>  @node Miscellaneous Services
>  @subsection Miscellaneous Services
> diff --git a/gnu/services/linux.scm b/gnu/services/linux.scm
> index caa0326c31..99ab4a3329 100644
> --- a/gnu/services/linux.scm
> +++ b/gnu/services/linux.scm
> @@ -1,5 +1,6 @@
>  ;;; GNU Guix --- Functional package management for GNU
>  ;;; Copyright © 2020 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
> +;;; Copyright © 2020 Brice Waegeneire <brice <at> waegenei.re>
>  ;;;
>  ;;; This file is part of GNU Guix.
>  ;;;
> @@ -37,7 +38,14 @@
>              earlyoom-configuration-ignore-positive-oom-score-adj?
>              earlyoom-configuration-show-debug-messages?
>              earlyoom-configuration-send-notification-command
> -            earlyoom-service-type))
> +            earlyoom-service-type
> +
> +            kernel-module-loader-configuration
> +            kernel-module-loader-configuration?
> +            kernel-module-loader-configuration-modprobe
> +            kernel-module-loader-configuration-modules
> +            kernel-module-loader-service-type
> +            kernel-module-loader-service))
>  
>  
>  ;;;
> @@ -123,3 +131,51 @@ representation."
>      (list (service-extension shepherd-root-service-type
>                               (compose list earlyoom-shepherd-service))))
>     (description "Run @command{earlyoom}, the Early OOM daemon.")))
> +
> +;;;
> +;;; Kernel module loader.
> +;;;
> +
> +(define-record-type* <kernel-module-loader-configuration>
> +  kernel-module-loader-configuration make-kernel-module-loader-configuration
> +  kernel-module-loader-configuration?
> +  (modprobe kernel-module-loader-configuration-modprobe ; path of the 'modprobe' command
> +            (default (file-append kmod "/bin/modprobe")))
> +  (modules kernel-module-loader-configuration-modules ; list of strings
> +           (default '())))
> +
> +(define kernel-module-loader-shepherd-service
> +  (match-lambda
> +    (($ <kernel-module-loader-configuration> modprobe modules)
> +     (list
> +      (shepherd-service
> +       (documentation "Load kernel modules.")
> +       (provision '(kernel-module-loader))
> +       (respawn? #f)
> +       (one-shot? #t)
> +       (start
> +        #~(lambda _
> +            (zero? (system* #$modprobe #$@modules)))))))))
> +
> +(define kernel-module-loader-service-type
> +  (service-type
> +   (name 'kernel-module-loader)
> +   (description "Load kernel modules.")
> +   (extensions
> +    (list
> +     (service-extension shepherd-root-service-type
> +                        kernel-module-loader-shepherd-service)))
> +   (compose concatenate)
> +   (extend (lambda (config modules)
> +             (kernel-module-loader-configuration
> +              (inherit config)
> +              (modules (append
> +                        (kernel-module-loader-configuration-modules config)
> +                        modules)))))
> +   (default-value (kernel-module-loader-configuration))))
> +
> +(define* (kernel-module-loader-service modules)
> +  "Return a service that loads kernel MODULES."
> +  (service kernel-module-loader-service-type
> +           (kernel-module-loader-configuration
> +            (modules modules))))
> -- 
> 2.25.1

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

From: Brice Waegeneire <brice <at> waegenei.re>
To: guix-patches <at> gnu.org
Subject: [PATCH v4] gnu: Add kernel-module-loader-service.
Date: Tue, 31 Mar 2020 22:23:40 +0200

* doc/guix.texi (Linux Services): Add a new subsection and document the
new service and its configuration.
* gnu/services/linux.scm (kernel-module-loader-service): New procedure.
(kernel-module-loader-service-type, kernel-module-loader-configuration):
New types.
* gnu/tests/linux-modules.scm (module-loader-program): Procedure
removed.
(modules-loaded?-program): New procedure.
(run-loadable-kernel-modules-test): 'module-loader-program' procedure
replaced by the new one. New format for last arguments
[os]: Use 'kernel-module-loader-service'.
(%test-loadable-kernel-modules-0, %test-loadable-kernel-modules-1,
%test-loadable-kernel-modules-2): Use the new argument format.
---

Fix typo in doc/guix.texi.

 doc/guix.texi               | 54 ++++++++++++++++++++++++++++++++
 gnu/services/linux.scm      | 61 ++++++++++++++++++++++++++++++++++++-
 gnu/tests/linux-modules.scm | 45 ++++++++++++++++-----------
 3 files changed, 142 insertions(+), 18 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 77a4b8ed71..125726a316 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -75,6 +75,7 @@ Copyright @copyright{} 2020 Wiktor Żelazny@*
 Copyright @copyright{} 2020 Damien Cassou@*
 Copyright @copyright{} 2020 Jakub Kądziołka@*
 Copyright @copyright{} 2020 Jack Hill@*
+Copyright @copyright{} 2020 Brice Waegeneire@*
 
 Permission is granted to copy, distribute and/or modify this document
 under the terms of the GNU Free Documentation License, Version 1.3 or
@@ -25382,6 +25383,59 @@ notifications.
 @end table
 @end deftp
 
+@cindex modprobe
+@cindex kernel module loader
+@subsubsection Kernel Module Loader Service
+
+The @code{kernel-module-loader-service} provides a service to load
+kernel modules at boot.  This is especially useful for modules that
+don't autoload and need to be manually loaded, like it's the case with
+@code{ddcci} for example.
+
+@deffn {Scheme Procedure} kernel-module-loader-service @var{modules}
+Return a service that run @command{modprobe} with arguments
+@var{modules} at boot.  For example loading the drivers provided by
+@code{ddci-driver-linux} with @code{ddcci} in debugging mode can be done
+as follow:
+
+@lisp
+(use-modules (gnu))
+(use-package-modules linux)
+(use-service-modules linux)
+(operating-system
+  ...
+  (services (cons* (kernel-module-loader-service
+                     '(("ddcci" "dyndbg")
+                       ("ddcci_backlight")))
+                   %base-services))
+  (kernel-loadable-modules (list ddcci-driver-linux)))
+@end lisp
+@end deffn
+
+@deffn {Scheme Variable} kernel-module-loader-service-type
+The service type for loading kernel modules at boot with
+@command{modprobe}, for module that can't autoload.  Its value must be a
+@code{kernel-module-loader-configuration} object, described below.
+@end deffn
+
+@deftp {Data Type} kernel-module-loader-configuration
+The data type representing the modules to be loaded by
+@command{modprobe}.
+@end deftp
+
+@deftp {Data Type} kernel-module-loader-configuration
+This is the configuration record for the
+@code{kernel-module-loader-service-type}.
+
+@table @asis
+@item @code{modprobe} (default: @code{(file-append kmod "/bin/modprobe"})
+The @command{modprobe} executable to use.
+
+@item @code{modules} (default: @code{'()})
+A list of list of strings specifying the modules to load with their
+optional parameters.
+@end table
+@end deftp
 
 @node Miscellaneous Services
 @subsection Miscellaneous Services
diff --git a/gnu/services/linux.scm b/gnu/services/linux.scm
index caa0326c31..da71b275f1 100644
--- a/gnu/services/linux.scm
+++ b/gnu/services/linux.scm
@@ -1,5 +1,6 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2020 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
+;;; Copyright © 2020 Brice Waegeneire <brice <at> waegenei.re>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -37,7 +38,14 @@
             earlyoom-configuration-ignore-positive-oom-score-adj?
             earlyoom-configuration-show-debug-messages?
             earlyoom-configuration-send-notification-command
-            earlyoom-service-type))
+            earlyoom-service-type
+
+            kernel-module-loader-configuration
+            kernel-module-loader-configuration?
+            kernel-module-loader-configuration-modprobe
+            kernel-module-loader-configuration-modules
+            kernel-module-loader-service-type
+            kernel-module-loader-service))
 
 
 ;;;
@@ -123,3 +131,54 @@ representation."
     (list (service-extension shepherd-root-service-type
                              (compose list earlyoom-shepherd-service))))
    (description "Run @command{earlyoom}, the Early OOM daemon.")))
+
+;;;
+;;; Kernel module loader.
+;;;
+
+(define-record-type* <kernel-module-loader-configuration>
+  kernel-module-loader-configuration make-kernel-module-loader-configuration
+  kernel-module-loader-configuration?
+  (modprobe kernel-module-loader-configuration-modprobe ; path of the 'modprobe' command
+            (default (file-append kmod "/bin/modprobe")))
+  (modules kernel-module-loader-configuration-modules ; list of lists of strings
+           (default '())))
+
+(define kernel-module-loader-shepherd-service
+  (match-lambda
+    (($ <kernel-module-loader-configuration> modprobe modules)
+     (list
+      (shepherd-service
+       (documentation "Load kernel modules.")
+       (provision '(kernel-module-loader))
+       (respawn? #f)
+       (one-shot? #t)
+       (start
+        #~(lambda _
+            (and-map (lambda (module-with-parameters)
+                   (zero? (apply system* #$modprobe "--"
+                                 module-with-parameters)))
+                 '#$modules))))))))
+
+(define kernel-module-loader-service-type
+  (service-type
+   (name 'kernel-module-loader)
+   (description "Load kernel modules.")
+   (extensions
+    (list
+     (service-extension shepherd-root-service-type
+                        kernel-module-loader-shepherd-service)))
+   (compose concatenate)
+   (extend (lambda (config modules)
+             (kernel-module-loader-configuration
+              (inherit config)
+              (modules (append
+                        (kernel-module-loader-configuration-modules config)
+                        modules)))))
+   (default-value (kernel-module-loader-configuration))))
+
+(define* (kernel-module-loader-service modules)
+  "Return a service that loads kernel MODULES."
+  (service kernel-module-loader-service-type
+           (kernel-module-loader-configuration
+            (modules modules))))
diff --git a/gnu/tests/linux-modules.scm b/gnu/tests/linux-modules.scm
index 39e11587c6..7b820a1269 100644
--- a/gnu/tests/linux-modules.scm
+++ b/gnu/tests/linux-modules.scm
@@ -1,6 +1,7 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2019 Jakob L. Kreuze <zerodaysfordays <at> sdf.org>
 ;;; Copyright © 2020 Danny Milosavljevic <dannym <at> scratchpost.org>
+;;; Copyright © 2020 Brice Waegeneire <brice <at> waegenei.re>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -19,6 +20,7 @@
 
 (define-module (gnu tests linux-modules)
   #:use-module (gnu packages linux)
+  #:use-module (gnu services linux)
   #:use-module (gnu system)
   #:use-module (gnu system vm)
   #:use-module (gnu tests)
@@ -37,25 +39,31 @@
 ;;;
 ;;; Code:
 
-(define* (module-loader-program os modules)
-  "Return an executable store item that, upon being evaluated, will dry-run
-load MODULES."
+(define* (modules-loaded?-program os modules)
+  "Return an executable store item that, upon being evaluated, will verify
+that MODULES are actually loaded."
   (program-file
    "load-kernel-modules.scm"
-   (with-imported-modules (source-module-closure '((guix build utils)))
-     #~(begin
-         (use-modules (guix build utils))
-         (for-each (lambda (module)
-                     (invoke (string-append #$kmod "/bin/modprobe") "-n" "--"
-                             module))
-                   '#$modules)))))
+   #~(begin
+     (use-modules (ice-9 rdelim)
+                  (ice-9 popen))
+     (let* ((port (open-input-pipe (string-append #$kmod "/bin/lsmod")))
+            (output (read-string port))
+            (status (close-pipe port)))
+       (and-map (lambda (module) (string-contains output module))
+                '#$modules)))))
 
-(define* (run-loadable-kernel-modules-test module-packages module-names)
-  "Run a test of an OS having MODULE-PACKAGES, and modprobe MODULE-NAMES."
+(define* (run-loadable-kernel-modules-test module-packages
+                                           modules-with-parameters)
+  "Run a test of an OS having MODULE-PACKAGES, and verify that
+MODULES-WITH-PARAMETERS are loaded in memory."
   (define os
     (marionette-operating-system
      (operating-system
       (inherit (simple-operating-system))
+      (services (cons (kernel-module-loader-service modules-with-parameters)
+                      (operating-system-user-services
+                       (simple-operating-system))))
       (kernel-loadable-modules module-packages))
      #:imported-modules '((guix combinators))))
   (define vm (virtual-machine os))
@@ -75,7 +83,9 @@ load MODULES."
              marionette))
           (test-end)
           (exit (= (test-runner-fail-count (test-runner-current)) 0)))))
-  (gexp->derivation "loadable-kernel-modules" (test (module-loader-program os module-names))))
+  (gexp->derivation
+   "loadable-kernel-modules"
+   (test (modules-loaded?-program os (map car modules-with-parameters)))))
 
 (define %test-loadable-kernel-modules-0
   (system-test
@@ -88,16 +98,17 @@ with no extra modules.")
   (system-test
    (name "loadable-kernel-modules-1")
    (description "Tests loadable kernel modules facility of <operating-system>
-with one extra module.")
+with one extra module with some parameters.")
    (value (run-loadable-kernel-modules-test
            (list ddcci-driver-linux)
-           '("ddcci")))))
+           '(("ddcci" "dyndbg" "delay:120"))))))
 
 (define %test-loadable-kernel-modules-2
   (system-test
    (name "loadable-kernel-modules-2")
    (description "Tests loadable kernel modules facility of <operating-system>
-with two extra modules.")
+with two extra modules without any parameters.")
    (value (run-loadable-kernel-modules-test
            (list acpi-call-linux-module ddcci-driver-linux)
-           '("acpi_call" "ddcci")))))
+           '(("acpi_call")
+             ("ddcci"))))))
-- 
2.25.1

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

From: Brice Waegeneire <brice <at> waegenei.re>
To: Nicolò Balzarotti <anothersms <at> gmail.com>
Cc: 40274 <at> debbugs.gnu.org,
 Guix-patches <guix-patches-bounces+brice+lists=waegenei.re <at> gnu.org>
Subject: Re: [bug#40274] [PATCH] gnu: Add kernel-module-loader-service.
Date: Tue, 31 Mar 2020 20:25:07 +0000

Hello,

On 2020-03-31 20:10, Nicolò Balzarotti wrote:
> Hi, I found a typo
> 
> /usefull/useful/
> 
> Nicolò

Thank you Nicolò I was missing flyspell in texinfo-mode, it's fixed in 
v4.

- Brice

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

From: Mathieu Othacehe <m.othacehe <at> gmail.com>
To: guix-patches <at> gnu.org
Cc: 40274 <at> debbugs.gnu.org
Subject: Re: [bug#40274] [PATCH v4] gnu: Add kernel-module-loader-service.
Date: Wed, 01 Apr 2020 15:30:25 +0200

Hello Brice,

> +@deffn {Scheme Procedure} kernel-module-loader-service @var{modules}
> +Return a service that run @command{modprobe} with arguments
                           ^
                           runs

> +@var{modules} at boot.  For example loading the drivers provided by
        ^
        What's modules?
        

> +@deffn {Scheme Variable} kernel-module-loader-service-type
> +The service type for loading kernel modules at boot with
> +@command{modprobe}, for module that can't autoload.  Its value must be a
                                ^
                                modules

You can add a separator here, C-q C-l with Emacs :)                                

> +;;;
> +;;; Kernel module loader.
> +;;;

> +  (modprobe kernel-module-loader-configuration-modprobe ; path of the 'modprobe' command
> +            (default (file-append kmod "/bin/modprobe")))
> +  (modules kernel-module-loader-configuration-modules ; list of lists of strings
> +           (default '())))

Those lines are above the 78 columns limit.

> +
> +(define kernel-module-loader-shepherd-service
> +  (match-lambda
> +    (($ <kernel-module-loader-configuration> modprobe modules)
> +     (list
> +      (shepherd-service
> +       (documentation "Load kernel modules.")
> +       (provision '(kernel-module-loader))
> +       (respawn? #f)
> +       (one-shot? #t)
> +       (start
> +        #~(lambda _
> +            (and-map (lambda (module-with-parameters)
> +                   (zero? (apply system* #$modprobe "--"
> +                                 module-with-parameters)))
> +                 '#$modules))))))))

I wonder if it would make sense to rmmod the modules on service
stop? Probably not.

>     (value (run-loadable-kernel-modules-test
>             (list acpi-call-linux-module ddcci-driver-linux)
> -           '("acpi_call" "ddcci")))))
> +           '(("acpi_call")
> +             ("ddcci"))))))

When using multiple modules without any configuration, I find the
notation '(("module-a") ("module-b")) a bit unclear. Maybe we could add
a special case handling '("module-a" "module-b") as two kernel modules
taking no arguments.

Or we could define a record with two fields: module-name and arguments.
Not sure what's better.

Anyway, this looks good to me :). But let's wait for Danny opinion
maybe.

Thanks,

Mathieu

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

From: Brice Waegeneire <brice <at> waegenei.re>
To: Mathieu Othacehe <m.othacehe <at> gmail.com>
Cc: 40274 <at> debbugs.gnu.org,
 Guix-patches <guix-patches-bounces+brice+lists=waegenei.re <at> gnu.org>
Subject: Re: [bug#40274] [PATCH v4] gnu: Add kernel-module-loader-service.
Date: Wed, 01 Apr 2020 14:38:57 +0000

Hello Mathieu,

On 2020-04-01 13:30, Mathieu Othacehe wrote:
> You can add a separator here, C-q C-l with Emacs :)

I never used pagination in Emacs, maybe I should start using it.

>> +(define kernel-module-loader-shepherd-service
>> +  (match-lambda
>> +    (($ <kernel-module-loader-configuration> modprobe modules)
>> +     (list
>> +      (shepherd-service
>> +       (documentation "Load kernel modules.")
>> +       (provision '(kernel-module-loader))
>> +       (respawn? #f)
>> +       (one-shot? #t)
>> +       (start
>> +        #~(lambda _
>> +            (and-map (lambda (module-with-parameters)
>> +                   (zero? (apply system* #$modprobe "--"
>> +                                 module-with-parameters)))
>> +                 '#$modules))))))))
> 
> I wonder if it would make sense to rmmod the modules on service
> stop? Probably not.

This is a one-shot service so no.

>>     (value (run-loadable-kernel-modules-test
>>             (list acpi-call-linux-module ddcci-driver-linux)
>> -           '("acpi_call" "ddcci")))))
>> +           '(("acpi_call")
>> +             ("ddcci"))))))
> 
> When using multiple modules without any configuration, I find the
> notation '(("module-a") ("module-b")) a bit unclear. Maybe we could add
> a special case handling '("module-a" "module-b") as two kernel modules
> taking no arguments.
> 
> Or we could define a record with two fields: module-name and arguments.
> Not sure what's better.

I asked about it on IRC before starting v2 but nobody responded so I 
went on
with this “list of lists of string” implementation. I'll be happy to use 
a
better format, the special case look nice but maybe confusing for the 
user.

I have fixed the typo you reported but I'll wait for further suggestion 
about
the format before sending the new version.

- Brice

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

From: "pelzflorian (Florian Pelz)" <pelzflorian <at> pelzflorian.de>
To: Brice Waegeneire <brice <at> waegenei.re>
Cc: 40274 <at> debbugs.gnu.org
Subject: Re: [bug#40274] [PATCH v4] gnu: Add kernel-module-loader-service.
Date: Wed, 1 Apr 2020 21:34:58 +0200

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

Thank you for your patch.

I tried with your patch to make the Guix install image load the
uvesafb kernel module, but it fails and I get no log.  Is it possible
to make the service log failures?

I just get:

failed to load service: kernel-module-loader

printed on the console (but not to /var/log/messages).

I believe I pass parameters wrongly.  I attach how I try to use your
kernel-module-loader.

Regards,
Florian

[uvesafb-fails.patch (text/plain, attachment)]

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

From: "pelzflorian (Florian Pelz)" <pelzflorian <at> pelzflorian.de>
To: Brice Waegeneire <brice <at> waegenei.re>
Cc: 40274 <at> debbugs.gnu.org
Subject: Re: [bug#40274] [PATCH v4] gnu: Add kernel-module-loader-service.
Date: Wed, 1 Apr 2020 21:43:58 +0200

On Wed, Apr 01, 2020 at 09:34:58PM +0200, pelzflorian (Florian Pelz) wrote:
> I believe I pass parameters wrongly.  I attach how I try to use your
> kernel-module-loader.

Of course my quoting was wrong.

Nonetheless could you add logging/errors if possible?

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

From: "pelzflorian (Florian Pelz)" <pelzflorian <at> pelzflorian.de>
To: Brice Waegeneire <brice <at> waegenei.re>
Cc: 40274 <at> debbugs.gnu.org
Subject: Re: [bug#40274] [PATCH v4] gnu: Add kernel-module-loader-service.
Date: Wed, 1 Apr 2020 21:48:26 +0200

On Wed, Apr 01, 2020 at 09:34:58PM +0200, pelzflorian (Florian Pelz) wrote:
> I believe I pass parameters wrongly.  I attach how I try to use your
> kernel-module-loader.

Also I cannot use gexps in module parameters.

Regards,
Florian

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

From: Brice Waegeneire <brice <at> waegenei.re>
To: guix-patches <at> gnu.org
Subject: [PATCH v5] gnu: Add kernel-module-loader-service.
Date: Thu,  2 Apr 2020 14:37:12 +0200

* doc/guix.texi (Linux Services): Add a new subsection and document the
new service and its configuration.
* gnu/services/linux.scm (kernel-module-loader-service): New procedure.
(kernel-module-loader-service-type, kernel-module-loader-configuration):
New types.
* gnu/tests/linux-modules.scm (module-loader-program): Procedure
removed.
(modules-loaded?-program): New procedure.
(run-loadable-kernel-modules-test): 'module-loader-program' procedure
replaced by the new one. New format for last arguments
[os]: Use 'kernel-module-loader-service'.
(%test-loadable-kernel-modules-0, %test-loadable-kernel-modules-1,
%test-loadable-kernel-modules-2): Use the new argument format.
---

In this version the typos reported by Mathieu have been fixed and if modprobe
fail the error is logged by shepherd.

 doc/guix.texi               | 53 +++++++++++++++++++++++++++
 gnu/services/linux.scm      | 72 ++++++++++++++++++++++++++++++++++++-
 gnu/tests/linux-modules.scm | 45 ++++++++++++++---------
 3 files changed, 152 insertions(+), 18 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 8cb85fe62c..ea1d363efc 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -76,6 +76,7 @@ Copyright @copyright{} 2020 Damien Cassou@*
 Copyright @copyright{} 2020 Jakub Kądziołka@*
 Copyright @copyright{} 2020 Jack Hill@*
 Copyright @copyright{} 2020 Naga Malleswari@*
+Copyright @copyright{} 2020 Brice Waegeneire@*
 
 Permission is granted to copy, distribute and/or modify this document
 under the terms of the GNU Free Documentation License, Version 1.3 or
@@ -25383,6 +25384,58 @@ notifications.
 @end table
 @end deftp
 
+@cindex modprobe
+@cindex kernel module loader
+@subsubsection Kernel Module Loader Service
+
+The @code{kernel-module-loader-service} provides a service to load
+kernel modules at boot.  This is especially useful for modules that
+don't autoload and need to be manually loaded, like it's the case with
+@code{ddcci} for example.
+
+@deffn {Scheme Procedure} kernel-module-loader-service @var{modules}
+Return a service that runs @command{modprobe} for each given module.
+For example loading the drivers provided by @code{ddci-driver-linux}
+with @code{ddcci} in debugging mode can be done as follow:
+
+@lisp
+(use-modules (gnu))
+(use-package-modules linux)
+(use-service-modules linux)
+(operating-system
+  ...
+  (services (cons* (kernel-module-loader-service
+                     '(("ddcci" "dyndbg")
+                       ("ddcci_backlight")))
+                   %base-services))
+  (kernel-loadable-modules (list ddcci-driver-linux)))
+@end lisp
+@end deffn
+
+@deffn {Scheme Variable} kernel-module-loader-service-type
+The service type for loading kernel modules at boot with
+@command{modprobe}, for modules that can't autoload.  Its value must be
+a @code{kernel-module-loader-configuration} object, described below.
+@end deffn
+
+@deftp {Data Type} kernel-module-loader-configuration
+The data type representing the modules to be loaded by
+@command{modprobe}.
+@end deftp
+
+@deftp {Data Type} kernel-module-loader-configuration
+This is the configuration record for the
+@code{kernel-module-loader-service-type}.
+
+@table @asis
+@item @code{modprobe} (default: @code{(file-append kmod "/bin/modprobe"})
+The @command{modprobe} executable to use.
+
+@item @code{modules} (default: @code{'()})
+A list of list of strings specifying the modules to load with their
+optional parameters.
+@end table
+@end deftp
 
 @node Miscellaneous Services
 @subsection Miscellaneous Services
diff --git a/gnu/services/linux.scm b/gnu/services/linux.scm
index caa0326c31..e3942f693d 100644
--- a/gnu/services/linux.scm
+++ b/gnu/services/linux.scm
@@ -1,5 +1,6 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2020 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
+;;; Copyright © 2020 Brice Waegeneire <brice <at> waegenei.re>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -25,6 +26,8 @@
   #:use-module (gnu packages linux)
   #:use-module (srfi srfi-1)
   #:use-module (srfi srfi-26)
+  #:use-module (srfi srfi-34)
+  #:use-module (srfi srfi-35)
   #:use-module (ice-9 match)
   #:export (earlyoom-configuration
             earlyoom-configuration?
@@ -37,7 +40,14 @@
             earlyoom-configuration-ignore-positive-oom-score-adj?
             earlyoom-configuration-show-debug-messages?
             earlyoom-configuration-send-notification-command
-            earlyoom-service-type))
+            earlyoom-service-type
+
+            kernel-module-loader-configuration
+            kernel-module-loader-configuration?
+            kernel-module-loader-configuration-modprobe
+            kernel-module-loader-configuration-modules
+            kernel-module-loader-service-type
+            kernel-module-loader-service))
 
 
 ;;;
@@ -123,3 +133,63 @@ representation."
     (list (service-extension shepherd-root-service-type
                              (compose list earlyoom-shepherd-service))))
    (description "Run @command{earlyoom}, the Early OOM daemon.")))
+
+
+;;;
+;;; Kernel module loader.
+;;;
+
+(define-record-type* <kernel-module-loader-configuration>
+  kernel-module-loader-configuration make-kernel-module-loader-configuration
+  kernel-module-loader-configuration?
+  ;; path of the 'modprobe' command
+  (modprobe kernel-module-loader-configuration-modprobe
+            (default (file-append kmod "/bin/modprobe")))
+  ;; list of lists of strings
+  (modules kernel-module-loader-configuration-modules
+           (default '())))
+
+(define kernel-module-loader-shepherd-service
+  (match-lambda
+    (($ <kernel-module-loader-configuration> modprobe kernel-modules)
+     (list
+      (shepherd-service
+       (documentation "Load kernel modules.")
+       (provision '(kernel-module-loader))
+       (respawn? #f)
+       (one-shot? #t)
+       (modules `((srfi srfi-34)
+                  (srfi srfi-35)
+                  ,@%default-modules))
+       (start
+        #~(lambda _
+            (guard (c ((message-condition? c)
+                       (display (condition-message c))
+                       #f))
+              (map (lambda (module-with-parameters)
+                     (apply invoke/quiet #$modprobe "--"
+                            module-with-parameters))
+                   '#$kernel-modules)))))))))
+
+(define kernel-module-loader-service-type
+  (service-type
+   (name 'kernel-module-loader)
+   (description "Load kernel modules.")
+   (extensions
+    (list
+     (service-extension shepherd-root-service-type
+                        kernel-module-loader-shepherd-service)))
+   (compose concatenate)
+   (extend (lambda (config modules)
+             (kernel-module-loader-configuration
+              (inherit config)
+              (modules (append
+                        (kernel-module-loader-configuration-modules config)
+                        modules)))))
+   (default-value (kernel-module-loader-configuration))))
+
+(define* (kernel-module-loader-service modules)
+  "Return a service that loads kernel MODULES."
+  (service kernel-module-loader-service-type
+           (kernel-module-loader-configuration
+            (modules modules))))
diff --git a/gnu/tests/linux-modules.scm b/gnu/tests/linux-modules.scm
index 39e11587c6..2532850d34 100644
--- a/gnu/tests/linux-modules.scm
+++ b/gnu/tests/linux-modules.scm
@@ -1,6 +1,7 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2019 Jakob L. Kreuze <zerodaysfordays <at> sdf.org>
 ;;; Copyright © 2020 Danny Milosavljevic <dannym <at> scratchpost.org>
+;;; Copyright © 2020 Brice Waegeneire <brice <at> waegenei.re>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -19,6 +20,7 @@
 
 (define-module (gnu tests linux-modules)
   #:use-module (gnu packages linux)
+  #:use-module (gnu services linux)
   #:use-module (gnu system)
   #:use-module (gnu system vm)
   #:use-module (gnu tests)
@@ -37,25 +39,31 @@
 ;;;
 ;;; Code:
 
-(define* (module-loader-program os modules)
-  "Return an executable store item that, upon being evaluated, will dry-run
-load MODULES."
+(define* (modules-loaded?-program os modules)
+  "Return an executable store item that, upon being evaluated, will verify
+that MODULES are actually loaded."
   (program-file
    "load-kernel-modules.scm"
-   (with-imported-modules (source-module-closure '((guix build utils)))
-     #~(begin
-         (use-modules (guix build utils))
-         (for-each (lambda (module)
-                     (invoke (string-append #$kmod "/bin/modprobe") "-n" "--"
-                             module))
-                   '#$modules)))))
+   #~(begin
+     (use-modules (ice-9 rdelim)
+                  (ice-9 popen))
+     (let* ((port (open-input-pipe (string-append #$kmod "/bin/lsmod")))
+            (output (read-string port))
+            (status (close-pipe port)))
+       (and-map (lambda (module) (string-contains output module))
+                '#$modules)))))
 
-(define* (run-loadable-kernel-modules-test module-packages module-names)
-  "Run a test of an OS having MODULE-PACKAGES, and modprobe MODULE-NAMES."
+(define* (run-loadable-kernel-modules-test module-packages
+                                           modules-with-parameters)
+  "Run a test of an OS having MODULE-PACKAGES, and verify that
+MODULES-WITH-PARAMETERS are loaded in memory."
   (define os
     (marionette-operating-system
      (operating-system
       (inherit (simple-operating-system))
+      (services (cons (kernel-module-loader-service modules-with-parameters)
+                      (operating-system-user-services
+                       (simple-operating-system))))
       (kernel-loadable-modules module-packages))
      #:imported-modules '((guix combinators))))
   (define vm (virtual-machine os))
@@ -75,7 +83,9 @@ load MODULES."
              marionette))
           (test-end)
           (exit (= (test-runner-fail-count (test-runner-current)) 0)))))
-  (gexp->derivation "loadable-kernel-modules" (test (module-loader-program os module-names))))
+  (gexp->derivation
+   "loadable-kernel-modules"
+   (test (modules-loaded?-program os (map car modules-with-parameters)))))
 
 (define %test-loadable-kernel-modules-0
   (system-test
@@ -88,16 +98,17 @@ with no extra modules.")
   (system-test
    (name "loadable-kernel-modules-1")
    (description "Tests loadable kernel modules facility of <operating-system>
-with one extra module.")
+with one extra module with some parameters.")
    (value (run-loadable-kernel-modules-test
            (list ddcci-driver-linux)
-           '("ddcci")))))
+           '(("ddcci" "dyndbg" "delay=120"))))))
 
 (define %test-loadable-kernel-modules-2
   (system-test
    (name "loadable-kernel-modules-2")
    (description "Tests loadable kernel modules facility of <operating-system>
-with two extra modules.")
+with two extra modules without any parameters.")
    (value (run-loadable-kernel-modules-test
            (list acpi-call-linux-module ddcci-driver-linux)
-           '("acpi_call" "ddcci")))))
+           '(("acpi_call")
+             ("ddcci"))))))
-- 
2.25.1

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

From: Brice Waegeneire <brice <at> waegenei.re>
To: "pelzflorian (Florian Pelz)" <pelzflorian <at> pelzflorian.de>
Cc: 40274 <at> debbugs.gnu.org
Subject: Re: [bug#40274] [PATCH v4] gnu: Add kernel-module-loader-service.
Date: Thu, 02 Apr 2020 12:40:00 +0000

Hello Florian,

Thank you for trying out this new service.

On 2020-04-01 19:34, pelzflorian (Florian Pelz) wrote:
> I tried with your patch to make the Guix install image load the
> uvesafb kernel module, but it fails and I get no log.  Is it possible
> to make the service log failures?
> 
> I just get:
> 
> failed to load service: kernel-module-loader
> 
> printed on the console (but not to /var/log/messages).

This a message saying it can't load that service, in this case because
it's parameters weren't formatted correctly, this is not specific to
this service that's how shepherd handle failing to load a service.
Please open a new issue about it if you think (as I do) that shepherd
should be more explicit when it fails to load a service.

Regarding this service, logging has been implemented in v5; the output
of modprobe will be logged if it fails, for example when trying to load
an invalid kernel module like “foo”.

- Brice

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

From: Danny Milosavljevic <dannym <at> scratchpost.org>
To: Brice Waegeneire <brice <at> waegenei.re>
Cc: ludo <at> gnu.org, 40274 <at> debbugs.gnu.org
Subject: Re: [bug#40274] [PATCH v5] gnu: Add kernel-module-loader-service.
Date: Thu, 2 Apr 2020 15:56:40 +0200

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

Hi Brice,

I wonder how common it is to pass arguments to the modules explicitly in normal
operation.

I haven't done it often even in other distributions--and I'm a kernel hacker.

See also https://linux.die.net/man/5/modprobe.d for an alternative.

I'm not necessarily against doing it like you do it, but just want to bring up
the possibility of just omitting the functionality and let it be
someone-else's-problem, possibly another guix service that prepares
/etc/modprobe.d with module options and other things (aliases, installation and
removal invocations).

That's also important because Linux tries to (lazy-)autoload modules whenever
possible (via invoking modprobe).  In that case, the argument handling would
be inconsistent between if it was lazy-autoloaded compared to if it was loaded
by your loader.

(I even wonder if it were better for kernel-module-loader-service to read the
modprobe to use from /proc/sys/kernel/modprobe in order to make the situations
a little more consistent)

For example let's say the following happened:

(1) Linux boots up.
(2) Someone accesses some device so "modprobe foo" is invoked by Linux.
(3) foo is loaded, gets options from /etc/modprobe.d (usually none).
[Time passes, other stuff happens]
(4) Your kernel-module-loader-service is started, invokes "modprobe foo x=2".
(5) x=2 is not passed to the Linux kernel module ever.

I'm just saying maybe not invite this kind of trouble in the first place.

I don't think it fits Guix's declarative configuration style to do that
either.

Also, when reconfiguring the Guix system, kernel-module-loader-service won't
unload the kernel modules and thus also wouldn't load it with new options.

Also, it could happen that two different guix services require the same module
but with different options.  That's an insane problem to have and I wouldn't
try to support it.

(I've reviewed your patch, otherwise OK!)

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

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

From: Mathieu Othacehe <m.othacehe <at> gmail.com>
To: Brice Waegeneire <brice <at> waegenei.re>
Cc: 40274 <at> debbugs.gnu.org
Subject: Re: [bug#40274] [PATCH v5] gnu: Add kernel-module-loader-service.
Date: Thu, 02 Apr 2020 16:22:19 +0200

Hello Brice,

Thanks for this new version.

> +            (guard (c ((message-condition? c)
> +                       (display (condition-message c))

Maybe `(format (current-error-port) "~a~%" (condition-message c))'
instead of display.

Thanks,

Mathieu

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

From: Brice Waegeneire <brice <at> waegenei.re>
To: Danny Milosavljevic <dannym <at> scratchpost.org>
Cc: ludo <at> gnu.org, 40274 <at> debbugs.gnu.org
Subject: Re: [bug#40274] [PATCH v5] gnu: Add kernel-module-loader-service.
Date: Thu, 02 Apr 2020 17:13:05 +0000

On 2020-04-02 13:56, Danny Milosavljevic wrote:
> Hi Brice,
> 
> I wonder how common it is to pass arguments to the modules explicitly 
> in normal
> operation.
> 
> I haven't done it often even in other distributions--and I'm a kernel 
> hacker.
> 
> See also https://linux.die.net/man/5/modprobe.d for an alternative.
> 
> I'm not necessarily against doing it like you do it, but just want to 
> bring up
> the possibility of just omitting the functionality and let it be
> someone-else's-problem, possibly another guix service that prepares
> /etc/modprobe.d with module options and other things (aliases, 
> installation and
> removal invocations).
> 
> That's also important because Linux tries to (lazy-)autoload modules 
> whenever
> possible (via invoking modprobe).  In that case, the argument handling 
> would
> be inconsistent between if it was lazy-autoloaded compared to if it was 
> loaded
> by your loader.
> 
> (I even wonder if it were better for kernel-module-loader-service to 
> read the
> modprobe to use from /proc/sys/kernel/modprobe in order to make the 
> situations
> a little more consistent)
> 
> For example let's say the following happened:
> 
> (1) Linux boots up.
> (2) Someone accesses some device so "modprobe foo" is invoked by Linux.
> (3) foo is loaded, gets options from /etc/modprobe.d (usually none).
> [Time passes, other stuff happens]
> (4) Your kernel-module-loader-service is started, invokes "modprobe foo 
> x=2".
> (5) x=2 is not passed to the Linux kernel module ever.
> 
> I'm just saying maybe not invite this kind of trouble in the first 
> place.
> 
> I don't think it fits Guix's declarative configuration style to do that
> either.
> 
> Also, when reconfiguring the Guix system, kernel-module-loader-service 
> won't
> unload the kernel modules and thus also wouldn't load it with new 
> options.
> 
> Also, it could happen that two different guix services require the same 
> module
> but with different options.  That's an insane problem to have and I 
> wouldn't
> try to support it.
> 
> (I've reviewed your patch, otherwise OK!)
Hello Danny,

Thank for taking the time to review this patch. Since I'm definitely 
*not*
a kernel hacker --just a casual user-- I wasn't aware of the uselessness 
of
specifying the module arguments to modprobe in such service. I wrote 
this
patch just to load this pesky non auto-loading `ddcci-backlight` module 
and
I have no current use of specifying module arguments. I just thought it
*could* be useful, to some, to pass arguments to modprobe since it is
present in its API; but the edge-cases you brought up show that it 
wasn't a
good idea after all.

Should I just go back to the first format, with just a list of module
names, and we merge this patch? Or would it be better, regarding the 
user
interface, to start this patch anew by using `modprobe.d` API as a base.
By that I mean defining a `modprobe-service-type` which populates
`/etc/modprobe.d/` and can manually load a module at boot if needed 
(like
kernel-module-loader does)? Would it be overkill? Following is an 
example of
what such service could look like:

#+begin_src scheme
(service modprobe-service-type
         (list (modprobe-entry
                (module "ddcci")
                (load? #t)
                (options '("dyndbg" "delay=120"))
                (alises '("ddc/ci"))
                (install "")           ; default
                (remove ""))           ; default
               (modprobe-entry
                (module "acpi-call")
                (blacklist? #t))))
#+end_src

- Brice

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

From: Danny Milosavljevic <dannym <at> scratchpost.org>
To: Brice Waegeneire <brice <at> waegenei.re>
Cc: ludo <at> gnu.org, 40274 <at> debbugs.gnu.org
Subject: Re: [bug#40274] [PATCH v5] gnu: Add kernel-module-loader-service.
Date: Fri, 3 Apr 2020 12:20:44 +0200

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

Hi Brice,

On Thu, 02 Apr 2020 17:13:05 +0000
Brice Waegeneire <brice <at> waegenei.re> wrote:

> *could* be useful, to some, to pass arguments to modprobe since it is
> present in its API; 

Definitely could be useful.  The question is whether the complexity justifies it
and whether it can be reliable.

> Should I just go back to the first format, with just a list of module
> names, and we merge this patch? 

I would like that more since it's very improbable to ever need changing.

>Or would it be better, regarding the 
> user
> interface, to start this patch anew by using `modprobe.d` API as a base.
> By that I mean defining a `modprobe-service-type` which populates
> `/etc/modprobe.d/` and can manually load a module at boot if needed 
> (like
> kernel-module-loader does)? Would it be overkill? Following is an 
> example of
> what such service could look like:

I think the two things (loading a module and configuring a module) can be
seperate, so I wouldn't mix it.

For example why would you load a module before the service that needs
it actually starts up (if ever)?  That just increases the attack surface 
for no reason.  On the other hand, module configuration will probably be
done as part of some guix service presence--used or not.

That said, maybe there are upsides to--among other things easier-understandable
configuration if it's central.

> #+begin_src scheme
> (service modprobe-service-type
>           (list (modprobe-entry
>                  (module "ddcci")
>                  (load? #t)
>                  (options '("dyndbg" "delay=120"))
>                  (alises '("ddc/ci"))
>                  (install "")           ; default
>                  (remove ""))           ; default
>                 (modprobe-entry
>                  (module "acpi-call")
>                  (blacklist? #t))))

A service to do that would still be nice.  But maybe
"kernel-module-configuration-service" ?

(Please don't think I have all the answers, let's see which way is better)

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

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

From: "pelzflorian (Florian Pelz)" <pelzflorian <at> pelzflorian.de>
To: Danny Milosavljevic <dannym <at> scratchpost.org>
Cc: 40274 <at> debbugs.gnu.org, ludo <at> gnu.org, Brice Waegeneire <brice <at> waegenei.re>
Subject: Re: [bug#40274] [PATCH v5] gnu: Add kernel-module-loader-service.
Date: Fri, 3 Apr 2020 14:58:39 +0200

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

On Fri, Apr 03, 2020 at 12:20:44PM +0200, Danny Milosavljevic wrote:
> On Thu, 02 Apr 2020 17:13:05 +0000 Brice Waegeneire <brice <at> waegenei.re> wrote:
> > *could* be useful, to some, to pass arguments to modprobe since it is
> > present in its API; 
> Definitely could be useful.  The question is whether the complexity justifies it
> and whether it can be reliable.

I need modprobe arguments in some way via some service to pass to the
uvesafb module to make graphics and kmscon work on machines without
Kernel Mode Setting (see attachment), but the (string-append …) is not
executed as a procedure, it seems.

Apr  3 14:54:45 localhost vmunix: [    9.860831] shepherd[1]: Service user-homes has been started.
Apr  3 14:54:45 localhost vmunix: [    9.880505] shepherd[1]: '/gnu/store/d064bv2b1hrb07j2zj78i608db7qldx9-kmod-26/bin/modprobe -- uvesafb (string-append v86d= /gnu/store/0m780n5q4cr7rqmjn5zzd6adr3042f97-v86d-0.1.10 /sbin/v86d) mode_option=1024x768' exited with status 127; output follows:
Apr  3 14:54:45 localhost vmunix: [    9.881867] shepherd[1]: 
Apr  3 14:54:45 localhost vmunix: [    9.883595] shepherd[1]: Service kernel-module-loader could not be started.

Regards,
Florian

[uvesafb-gexp.patch (text/plain, attachment)]

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

From: "pelzflorian (Florian Pelz)" <pelzflorian <at> pelzflorian.de>
To: Brice Waegeneire <brice <at> waegenei.re>
Cc: 40274 <at> debbugs.gnu.org
Subject: Re: [bug#40274] [PATCH v4] gnu: Add kernel-module-loader-service.
Date: Fri, 3 Apr 2020 15:03:16 +0200

On Thu, Apr 02, 2020 at 12:40:00PM +0000, Brice Waegeneire wrote:
> On 2020-04-01 19:34, pelzflorian (Florian Pelz) wrote:
> > failed to load service: kernel-module-loader
> This a message saying it can't load that service, in this case because
> it's parameters weren't formatted correctly, this is not specific to
> this service that's how shepherd handle failing to load a service.
> Please open a new issue about it if you think (as I do) that shepherd
> should be more explicit when it fails to load a service.

I meant compilation could fail if the parameters are incorrect.  I do
not know how else Shepherd should fail.  But this is not very
important and applies to many other services as well.

Thank you for adding logging.  All this works well (in testing, I have
not read the code).

Regards,
Florian

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

From: Danny Milosavljevic <dannym <at> scratchpost.org>
To: Brice Waegeneire <brice <at> waegenei.re>
Cc: ludo <at> gnu.org, 40274 <at> debbugs.gnu.org
Subject: Re: [bug#40274] [PATCH v5] gnu: Add kernel-module-loader-service.
Date: Sat, 4 Apr 2020 12:51:35 +0200

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

Furthermore, many modules could be built-into the kernel (instead of loadable), too.  Then the "module" options somehow have to make it into the kernel command line as "module_name.parameter_name=parameter_value".

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

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

From: Brice Waegeneire <brice <at> waegenei.re>
To: guix-patches <at> gnu.org
Subject: [PATCH v6 0/2] gnu: Add kernel-module-loader-service.
Date: Sat,  4 Apr 2020 17:17:30 +0200

Hello,

The following modification have been made since the previous patch version:
- The format to specify kernel modules has been reverted to a simple list of
  strings.
- The `modprobe` command used is retrieved from "/proc/sys/kernel/modprobe".
- Thanks to the previously listed modifications,
  `kernel-module-loader-service-type` has been removed together with the
  helper function `kernel-module-loader-service`. No need to bother with a
  record now that we only need a list of strings.
- A temporary crutch to force `modprobe` from loading "/etc/modprobe.d" has
  been added. The permanent fix has been submitted as #40422[0] but it needs
  to go trough the core-updates branch.
- The documentation has been updated accordingly.
- An example showing how to specify module arguments using "/etc/modprobe.d/"
  has been added to the documentation.

[0]: https://issues.guix.info/issue/40422

- Brice

Brice Waegeneire (2):
  services: Allow modprobe to use "/etc/modprobe.d".
  gnu: Add kernel-module-loader-service.

 doc/guix.texi               | 38 +++++++++++++++++++++++++++
 gnu/services.scm            |  4 +++
 gnu/services/linux.scm      | 52 ++++++++++++++++++++++++++++++++++++-
 gnu/tests/linux-modules.scm | 33 ++++++++++++++---------
 4 files changed, 114 insertions(+), 13 deletions(-)

-- 
2.26.0

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

From: Brice Waegeneire <brice <at> waegenei.re>
To: guix-patches <at> gnu.org
Subject: [PATCH v6 1/2] services: Allow modprobe to use "/etc/modprobe.d".
Date: Sat,  4 Apr 2020 17:17:31 +0200

This is a temporary crutch allowing 'modprobe' to load it's
configuration from "/etc/modprobe.d". The definitive fix is contained in
<https://issues.guix.info/issue/40422> which need to go trough the
core-upates branch as it rebuild over 2200 packages.

* gnu/services.scm (%modprobe-wrapper): Set 'MODPROBE_OPTIONS'
environment variable.
---
 gnu/services.scm | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/gnu/services.scm b/gnu/services.scm
index e7a3a95e43..1c39c779a7 100644
--- a/gnu/services.scm
+++ b/gnu/services.scm
@@ -579,6 +579,10 @@ ACTIVATION-SCRIPT-TYPE."
                   #~(begin
                       (setenv "LINUX_MODULE_DIRECTORY"
                               "/run/booted-system/kernel/lib/modules")
+                      ;; FIXME: Remove this crutch when the patch #40422,
+                      ;; updating to kmod 27 is merged
+                      (setenv "MODPROBE_OPTIONS"
+                              "-C /etc/modprobe.d")
                       (apply execl #$modprobe
                              (cons #$modprobe (cdr (command-line))))))))
 
-- 
2.26.0

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

From: Brice Waegeneire <brice <at> waegenei.re>
To: guix-patches <at> gnu.org
Subject: [PATCH v6 2/2] gnu: Add kernel-module-loader-service.
Date: Sat,  4 Apr 2020 17:17:32 +0200

* doc/guix.texi (Linux Services): Add a new subsection and document the
new service and its configuration.
* gnu/services/linux.scm (kernel-module-loader-service-type): New type.
(kernel-module-loader-shepherd-service): New procedure.
* gnu/tests/linux-modules.scm (module-loader-program): Procedure
removed.
(modules-loaded?-program): New procedure.
(run-loadable-kernel-modules-test): 'module-loader-program' procedure
replaced by the new one.
[os]: Use 'kernel-module-loader-service'.
---
 doc/guix.texi               | 38 +++++++++++++++++++++++++++
 gnu/services/linux.scm      | 52 ++++++++++++++++++++++++++++++++++++-
 gnu/tests/linux-modules.scm | 33 ++++++++++++++---------
 3 files changed, 110 insertions(+), 13 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 8cb85fe62c..b18413f685 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -76,6 +76,7 @@ Copyright @copyright{} 2020 Damien Cassou@*
 Copyright @copyright{} 2020 Jakub Kądziołka@*
 Copyright @copyright{} 2020 Jack Hill@*
 Copyright @copyright{} 2020 Naga Malleswari@*
+Copyright @copyright{} 2020 Brice Waegeneire@*
 
 Permission is granted to copy, distribute and/or modify this document
 under the terms of the GNU Free Documentation License, Version 1.3 or
@@ -25383,6 +25384,43 @@ notifications.
 @end table
 @end deftp
 
+@cindex modprobe
+@cindex kernel module loader
+@subsubsection Kernel Module Loader Service
+
+The kernel module loader service allows one to load, at boot, loadable
+kernel modules---in opposition to built-in kernel modules which should
+be added to the @code{kernel-arguments} field instead.  This is
+especially useful for modules that don't autoload and need to be
+manually loaded, as it's the case with @code{ddcci} for example.
+
+@deffn {Scheme Variable} kernel-module-loader-service-type
+The service type for loading loadable kernel modules at boot with
+@command{modprobe}.  Its value must be a list of strings representing
+module names.  For example loading the drivers provided by
+@code{ddci-driver-linux}, in debugging mode by passing some module
+parameters, can be done as follow:
+
+@lisp
+(use-modules (gnu) (gnu services))
+(use-package-modules linux)
+(use-service-modules linux)
+
+(define ddcci-config
+  (plain-file "ddcci.conf"
+              "options ddcci dyndbg delay=120"))
+
+(operating-system
+  ...
+  (services (cons* (service kernel-module-loader-service-type
+                            '("ddcci" "ddcci_backlight"))
+                   (simple-service 'ddcci-config etc-service-type
+                                   (list `("modprobe.d/ddcci.conf"
+                                           ,ddcci-config)))
+                   %base-services))
+  (kernel-loadable-modules (list ddcci-driver-linux)))
+@end lisp
+@end deffn
 
 @node Miscellaneous Services
 @subsection Miscellaneous Services
diff --git a/gnu/services/linux.scm b/gnu/services/linux.scm
index caa0326c31..621d26fcff 100644
--- a/gnu/services/linux.scm
+++ b/gnu/services/linux.scm
@@ -1,5 +1,6 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2020 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
+;;; Copyright © 2020 Brice Waegeneire <brice <at> waegenei.re>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -25,6 +26,8 @@
   #:use-module (gnu packages linux)
   #:use-module (srfi srfi-1)
   #:use-module (srfi srfi-26)
+  #:use-module (srfi srfi-34)
+  #:use-module (srfi srfi-35)
   #:use-module (ice-9 match)
   #:export (earlyoom-configuration
             earlyoom-configuration?
@@ -37,7 +40,9 @@
             earlyoom-configuration-ignore-positive-oom-score-adj?
             earlyoom-configuration-show-debug-messages?
             earlyoom-configuration-send-notification-command
-            earlyoom-service-type))
+            earlyoom-service-type
+
+            kernel-module-loader-service-type))
 
 
 ;;;
@@ -123,3 +128,48 @@ representation."
     (list (service-extension shepherd-root-service-type
                              (compose list earlyoom-shepherd-service))))
    (description "Run @command{earlyoom}, the Early OOM daemon.")))
+
+
+;;;
+;;; Kernel module loader.
+;;;
+
+(define kernel-module-loader-shepherd-service
+  (match-lambda
+    ((and (? list? kernel-modules) ((? string?) ...))
+     (list
+      (shepherd-service
+       (documentation "Load kernel modules.")
+       (provision '(kernel-module-loader))
+       (requirement '(file-systems))
+       (respawn? #f)
+       (one-shot? #t)
+       (modules `((srfi srfi-34)
+                  (srfi srfi-35)
+                  ,@%default-modules))
+       (start
+        #~(lambda _
+            (if (file-exists? "/proc/sys/kernel/modprobe")
+                (let ((modprobe (call-with-input-file
+                                    "/proc/sys/kernel/modprobe" get-line)))
+                  (guard (c ((message-condition? c)
+                             (format (current-error-port) "~a~%"
+                                     (condition-message c))
+                             #f))
+                    (map (lambda (module)
+                           (invoke/quiet modprobe "--" module))
+                         '#$kernel-modules)))
+                (begin (format (current-error-port) "error: ~a~%"
+                               "Kernel is missing loadable module support.")
+                      #f)))))))))
+
+(define kernel-module-loader-service-type
+  (service-type
+   (name 'kernel-module-loader)
+   (description "Load kernel modules.")
+   (extensions
+    (list (service-extension shepherd-root-service-type
+                             kernel-module-loader-shepherd-service)))
+   (compose concatenate)
+   (extend append)
+   (default-value '())))
diff --git a/gnu/tests/linux-modules.scm b/gnu/tests/linux-modules.scm
index 39e11587c6..f1c4feb7a0 100644
--- a/gnu/tests/linux-modules.scm
+++ b/gnu/tests/linux-modules.scm
@@ -1,6 +1,7 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2019 Jakob L. Kreuze <zerodaysfordays <at> sdf.org>
 ;;; Copyright © 2020 Danny Milosavljevic <dannym <at> scratchpost.org>
+;;; Copyright © 2020 Brice Waegeneire <brice <at> waegenei.re>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -19,6 +20,8 @@
 
 (define-module (gnu tests linux-modules)
   #:use-module (gnu packages linux)
+  #:use-module (gnu services)
+  #:use-module (gnu services linux)
   #:use-module (gnu system)
   #:use-module (gnu system vm)
   #:use-module (gnu tests)
@@ -37,25 +40,30 @@
 ;;;
 ;;; Code:
 
-(define* (module-loader-program os modules)
-  "Return an executable store item that, upon being evaluated, will dry-run
-load MODULES."
+(define* (modules-loaded?-program os modules)
+  "Return an executable store item that, upon being evaluated, will verify
+that MODULES are actually loaded."
   (program-file
    "load-kernel-modules.scm"
-   (with-imported-modules (source-module-closure '((guix build utils)))
-     #~(begin
-         (use-modules (guix build utils))
-         (for-each (lambda (module)
-                     (invoke (string-append #$kmod "/bin/modprobe") "-n" "--"
-                             module))
-                   '#$modules)))))
+   #~(begin
+     (use-modules (ice-9 rdelim)
+                  (ice-9 popen))
+     (let* ((port (open-input-pipe (string-append #$kmod "/bin/lsmod")))
+            (output (read-string port))
+            (status (close-pipe port)))
+       (and-map (lambda (module) (string-contains output module))
+                '#$modules)))))
 
 (define* (run-loadable-kernel-modules-test module-packages module-names)
-  "Run a test of an OS having MODULE-PACKAGES, and modprobe MODULE-NAMES."
+  "Run a test of an OS having MODULE-PACKAGES, and verify that MODULE-NAMES
+are loaded in memory."
   (define os
     (marionette-operating-system
      (operating-system
       (inherit (simple-operating-system))
+      (services (cons (service kernel-module-loader-service-type module-names)
+                      (operating-system-user-services
+                       (simple-operating-system))))
       (kernel-loadable-modules module-packages))
      #:imported-modules '((guix combinators))))
   (define vm (virtual-machine os))
@@ -75,7 +83,8 @@ load MODULES."
              marionette))
           (test-end)
           (exit (= (test-runner-fail-count (test-runner-current)) 0)))))
-  (gexp->derivation "loadable-kernel-modules" (test (module-loader-program os module-names))))
+  (gexp->derivation "loadable-kernel-modules"
+                    (test (modules-loaded?-program os module-names))))
 
 (define %test-loadable-kernel-modules-0
   (system-test
-- 
2.26.0

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

From: Mathieu Othacehe <m.othacehe <at> gmail.com>
To: Brice Waegeneire <brice <at> waegenei.re>
Cc: 40274 <at> debbugs.gnu.org
Subject: Re: [bug#40274] [PATCH v6 2/2] gnu: Add kernel-module-loader-service.
Date: Sat, 04 Apr 2020 17:53:47 +0200

Hello Brice,

I think what you discussed with Danny, separating module loading from
configuration is very wise.

> +The kernel module loader service allows one to load, at boot, loadable
> +kernel modules---in opposition to built-in kernel modules which should
> +be added to the @code{kernel-arguments} field instead.  This is

Using kernel-arguments you can configure built-in modules but you cannot
force them to show-up, at least for most device drivers. For those
device drivers, loading of built-in modules is based on hardware probing
at boot-time. 

So I would just stick with:

"The kernel module loader service allows one to load, at boot time,
specific kernel modules."

> +(define ddcci-config
> +  (plain-file "ddcci.conf"
> +              "options ddcci dyndbg delay=120"))

Having the modprobe-service-type you proposed earlier, would be super
nice to do that :)

> +
> +(operating-system
> +  ...
> +  (services (cons* (service kernel-module-loader-service-type
> +                            '("ddcci" "ddcci_backlight"))
> +                   (simple-service 'ddcci-config etc-service-type
> +                                   (list `("modprobe.d/ddcci.conf"
> +                                           ,ddcci-config)))

Great!

This seems really nice to me.

Mathieu

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

From: "pelzflorian (Florian Pelz)" <pelzflorian <at> pelzflorian.de>
To: Brice Waegeneire <brice <at> waegenei.re>
Cc: 40274 <at> debbugs.gnu.org
Subject: Re: [bug#40274] [PATCH v6 2/2] gnu: Add kernel-module-loader-service.
Date: Sat, 4 Apr 2020 18:09:54 +0200

On Sat, Apr 04, 2020 at 05:17:30PM +0200, Brice Waegeneire wrote:
> Hello,
> 
> The following modification have been made since the previous patch version:
> - The format to specify kernel modules has been reverted to a simple list of
>   strings.
> - The `modprobe` command used is retrieved from "/proc/sys/kernel/modprobe".
> - Thanks to the previously listed modifications,
>   `kernel-module-loader-service-type` has been removed together with the
>   helper function `kernel-module-loader-service`. No need to bother with a
>   record now that we only need a list of strings.

You mean you removed kernel-module-loader-configuration.

Regarding patch 2/2:

On Sat, Apr 04, 2020 at 05:17:32PM +0200, Brice Waegeneire wrote:
> +module names.  For example loading the drivers provided by
> +@code{ddci-driver-linux}, in debugging mode by passing some module

This is a typo.  I can only find ddcci-driver-linux.


Thank you for this patch.  I do not know enough to judge the code.  I
will try again if it helps me load uvesafb.

Regards,
Florian

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

From: Brice Waegeneire <brice <at> waegenei.re>
To: "pelzflorian (Florian Pelz)" <pelzflorian <at> pelzflorian.de>
Cc: 40274 <at> debbugs.gnu.org,
 Guix-patches <guix-patches-bounces+brice+lists=waegenei.re <at> gnu.org>
Subject: Re: [bug#40274] [PATCH v6 2/2] gnu: Add kernel-module-loader-service.
Date: Sat, 04 Apr 2020 16:49:16 +0000

Hello Florian,

Thank you for the interest you have in this patch series.

On 2020-04-04 16:09, pelzflorian (Florian Pelz) wrote:
> On Sat, Apr 04, 2020 at 05:17:30PM +0200, Brice Waegeneire wrote:
>> - Thanks to the previously listed modifications,
>>   `kernel-module-loader-service-type` has been removed together with 
>> the
>>   helper function `kernel-module-loader-service`. No need to bother 
>> with a
>>   record now that we only need a list of strings.
> 
> You mean you removed kernel-module-loader-configuration.

Yes.

> Regarding patch 2/2:
> 
> On Sat, Apr 04, 2020 at 05:17:32PM +0200, Brice Waegeneire wrote:
>> +module names.  For example loading the drivers provided by
>> +@code{ddci-driver-linux}, in debugging mode by passing some module
> 
> This is a typo.  I can only find ddcci-driver-linux.

Good catch! DDC/CI what a silly name...

> Thank you for this patch.  I do not know enough to judge the code.  I
> will try again if it helps me load uvesafb.

Please do send feedback on using it, especially since you are the only 
one
in need of specifying module parameters ATM. I kept your usage in mind 
when
writing this patch version.

- Brice

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

From: "pelzflorian (Florian Pelz)" <pelzflorian <at> pelzflorian.de>
To: Brice Waegeneire <brice <at> waegenei.re>
Cc: 40274 <at> debbugs.gnu.org,
 Guix-patches <guix-patches-bounces+brice+lists=waegenei.re <at> gnu.org>
Subject: Re: [bug#40274] [PATCH v6 2/2] gnu: Add kernel-module-loader-service.
Date: Sat, 4 Apr 2020 19:46:34 +0200

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

On Sat, Apr 04, 2020 at 04:49:16PM +0000, Brice Waegeneire wrote:
> Please do send feedback on using it, especially since you are the only one
> in need of specifying module parameters ATM. I kept your usage in mind when
> writing this patch version.
> 
> - Brice

It does not seem to work.  There is nothing in /var/log/messages but
when I do

root <at> gnu.org # herd start kernel-module-loader
herd: exception caught while executing 'start' on service 'kernel-module-loader':
In procedure module-lookup: Unbound variable: get-line

I attach my changes.

Regards,
Florian

[uvesafb-3.patch (text/plain, attachment)]

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

From: Brice Waegeneire <brice <at> waegenei.re>
To: Danny Milosavljevic <dannym <at> scratchpost.org>
Cc: ludo <at> gnu.org, 40274 <at> debbugs.gnu.org
Subject: Re: [bug#40274] [PATCH v5] gnu: Add kernel-module-loader-service.
Date: Sat, 04 Apr 2020 17:58:09 +0000

Hello,

On 2020-04-03 10:20, Danny Milosavljevic wrote:
> On Thu, 02 Apr 2020 17:13:05 +0000
> Brice Waegeneire <brice <at> waegenei.re> wrote:
>> #+begin_src scheme
>> (service modprobe-service-type
>>           (list (modprobe-entry
>>                  (module "ddcci")
>>                  (load? #t)
>>                  (options '("dyndbg" "delay=120"))
>>                  (alises '("ddc/ci"))
>>                  (install "")           ; default
>>                  (remove ""))           ; default
>>                 (modprobe-entry
>>                  (module "acpi-call")
>>                  (blacklist? #t))))
> 
> A service to do that would still be nice.  But maybe
> "kernel-module-configuration-service" ?

A lengthy one but a better one for sure.

> (Please don't think I have all the answers, let's see which way is 
> better)

I don't but you seems to have more answers than I have on this specific 
subject.
We are on the same page here, the best interface should be the one that 
get
merged!

As you suggested, I choose to go back to the list of string format. It 
simplified
the boilerplate a lot: no record and no helper function. Yet it still 
allows passing
arguments to modules by extending `etc-service-type` to create
"/etc/modprobe.d/*.conf" files, and avoids all the pitfalls of the 
previous method
you made me aware off.

As `kernel-module-loader-service-type` is extensible it would be 
possible, in an
other patch series, if the need arises, to create a 
`kernel-module-configuration-service`
centralizing loadable modules configuration. But like you noted it would 
be specific
to loadable modules and not built-in ones since `kernel-arguments` is 
just an OS
field, which AFAIK can't be influenced by a service.

Thank you for your help Danny.

- Brice

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

From: Danny Milosavljevic <dannym <at> scratchpost.org>
To: Brice Waegeneire <brice <at> waegenei.re>
Cc: ludo <at> gnu.org, 40274 <at> debbugs.gnu.org
Subject: Re: [bug#40274] [PATCH v5] gnu: Add kernel-module-loader-service.
Date: Sat, 4 Apr 2020 20:31:28 +0200

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

Hi Brice,

>But like you noted it would 
> be specific
> to loadable modules and not built-in ones since `kernel-arguments` is 
> just an OS
> field, which AFAIK can't be influenced by a service.

There's no one stopping us from extending operating-system-kernel-arguments,
which already exists as a helper procedure, to also call
operating-system-services and check those for the command line arguments
to add (see gnu/system.scm).

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

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

From: Danny Milosavljevic <dannym <at> scratchpost.org>
To: Brice Waegeneire <brice <at> waegenei.re>
Cc: 40274 <at> debbugs.gnu.org
Subject: Re: [bug#40274] [PATCH v6 2/2] gnu: Add kernel-module-loader-service.
Date: Sat, 4 Apr 2020 23:10:04 +0200

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

On Sat,  4 Apr 2020 17:17:32 +0200
Brice Waegeneire <brice <at> waegenei.re> wrote:

> get-line

Missing (ice-9 rdelim) ?

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

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

From: Danny Milosavljevic <dannym <at> scratchpost.org>
To: Brice Waegeneire <brice <at> waegenei.re>
Cc: 40274 <at> debbugs.gnu.org
Subject: Re: [bug#40274] [PATCH v6 2/2] gnu: Add kernel-module-loader-service.
Date: Sat, 4 Apr 2020 23:11:44 +0200

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

On Sat, 4 Apr 2020 23:10:04 +0200
Danny Milosavljevic <dannym <at> scratchpost.org> wrote:

> On Sat,  4 Apr 2020 17:17:32 +0200
> Brice Waegeneire <brice <at> waegenei.re> wrote:
> 
> > get-line  
> 
> Missing (ice-9 rdelim) ?

I meant (ice-9 textual-ports)

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

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

From: Danny Milosavljevic <dannym <at> scratchpost.org>
To: Brice Waegeneire <brice <at> waegenei.re>
Cc: 40274 <at> debbugs.gnu.org
Subject: Re: [bug#40274] [PATCH v6 2/2] gnu: Add kernel-module-loader-service.
Date: Sat, 4 Apr 2020 23:16:54 +0200

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

What happens in the case that kernel-module-loader-service is used but no
modules to load are specified?  IMO that should be succeeding even in the
presence of a Linux kernel with no loadable module support.

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

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

From: "pelzflorian (Florian Pelz)" <pelzflorian <at> pelzflorian.de>
To: Danny Milosavljevic <dannym <at> scratchpost.org>
Cc: 40274 <at> debbugs.gnu.org, Brice Waegeneire <brice <at> waegenei.re>
Subject: Re: [bug#40274] [PATCH v6 2/2] gnu: Add kernel-module-loader-service.
Date: Sun, 5 Apr 2020 01:06:10 +0200

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

On Sat, Apr 04, 2020 at 11:11:44PM +0200, Danny Milosavljevic wrote:
> I meant (ice-9 textual-ports)

Yes.  I change Brice’s kernel-module-loader-shepherd-service (see
attachment) and everything works perfectly.

Thank you!

Regards,
Florian

[get-line.patch (text/plain, attachment)]

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

From: Brice Waegeneire <brice <at> waegenei.re>
To: guix-patches <at> gnu.org
Subject: [PATCH v7 0/2]  gnu: Add kernel-module-loader-service.
Date: Sun,  5 Apr 2020 07:28:01 +0200

This new revision:
- fix a typo in the documentation
- remove the part about built-in modules in the documentation
- add a missing Guile module to `kernel-module-loader-shepherd-service`
- `kernel-module-loader-shepherd-service` doesn't fail when the kernel don't
  support loadable module and there if no module are to be loaded

Brice Waegeneire (2):
  services: Allow modprobe to use "/etc/modprobe.d".
  gnu: Add kernel-module-loader-service.

 doc/guix.texi               | 37 +++++++++++++++++++++++++
 gnu/services.scm            |  4 +++
 gnu/services/linux.scm      | 54 ++++++++++++++++++++++++++++++++++++-
 gnu/tests/linux-modules.scm | 33 ++++++++++++++---------
 4 files changed, 115 insertions(+), 13 deletions(-)

-- 
2.26.0

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

From: Brice Waegeneire <brice <at> waegenei.re>
To: guix-patches <at> gnu.org
Subject: [PATCH v7 1/2] services: Allow modprobe to use "/etc/modprobe.d".
Date: Sun,  5 Apr 2020 07:28:02 +0200

This is a temporary crutch allowing 'modprobe' to load it's
configuration from "/etc/modprobe.d". The definitive fix is contained in
<https://issues.guix.info/issue/40422> which need to go trough the
core-upates branch as it rebuild over 2200 packages.

* gnu/services.scm (%modprobe-wrapper): Set 'MODPROBE_OPTIONS'
environment variable.
---
 gnu/services.scm | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/gnu/services.scm b/gnu/services.scm
index e7a3a95e43..1c39c779a7 100644
--- a/gnu/services.scm
+++ b/gnu/services.scm
@@ -579,6 +579,10 @@ ACTIVATION-SCRIPT-TYPE."
                   #~(begin
                       (setenv "LINUX_MODULE_DIRECTORY"
                               "/run/booted-system/kernel/lib/modules")
+                      ;; FIXME: Remove this crutch when the patch #40422,
+                      ;; updating to kmod 27 is merged
+                      (setenv "MODPROBE_OPTIONS"
+                              "-C /etc/modprobe.d")
                       (apply execl #$modprobe
                              (cons #$modprobe (cdr (command-line))))))))
 
-- 
2.26.0

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

From: Brice Waegeneire <brice <at> waegenei.re>
To: guix-patches <at> gnu.org
Subject: [PATCH v7 2/2] gnu: Add kernel-module-loader-service.
Date: Sun,  5 Apr 2020 07:28:03 +0200

* doc/guix.texi (Linux Services): Add a new subsection and document the
new service and its configuration.
* gnu/services/linux.scm (kernel-module-loader-service-type): New type.
(kernel-module-loader-shepherd-service): New procedure.
* gnu/tests/linux-modules.scm (module-loader-program): Procedure
removed.
(modules-loaded?-program): New procedure.
(run-loadable-kernel-modules-test): 'module-loader-program' procedure
replaced by the new one.
[os]: Use 'kernel-module-loader-service'.
---
 doc/guix.texi               | 37 +++++++++++++++++++++++++
 gnu/services/linux.scm      | 54 ++++++++++++++++++++++++++++++++++++-
 gnu/tests/linux-modules.scm | 33 ++++++++++++++---------
 3 files changed, 111 insertions(+), 13 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 8cb85fe62c..bc5602474e 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -76,6 +76,7 @@ Copyright @copyright{} 2020 Damien Cassou@*
 Copyright @copyright{} 2020 Jakub Kądziołka@*
 Copyright @copyright{} 2020 Jack Hill@*
 Copyright @copyright{} 2020 Naga Malleswari@*
+Copyright @copyright{} 2020 Brice Waegeneire@*
 
 Permission is granted to copy, distribute and/or modify this document
 under the terms of the GNU Free Documentation License, Version 1.3 or
@@ -25383,6 +25384,42 @@ notifications.
 @end table
 @end deftp
 
+@cindex modprobe
+@cindex kernel module loader
+@subsubsection Kernel Module Loader Service
+
+The kernel module loader service allows one to load loadable kernel
+modules at boot.  This is especially useful for modules that don't
+autoload and need to be manually loaded, as it's the case with
+@code{ddcci}.
+
+@deffn {Scheme Variable} kernel-module-loader-service-type
+The service type for loading loadable kernel modules at boot with
+@command{modprobe}.  Its value must be a list of strings representing
+module names.  For example loading the drivers provided by
+@code{ddcci-driver-linux}, in debugging mode by passing some module
+parameters, can be done as follow:
+
+@lisp
+(use-modules (gnu) (gnu services))
+(use-package-modules linux)
+(use-service-modules linux)
+
+(define ddcci-config
+  (plain-file "ddcci.conf"
+              "options ddcci dyndbg delay=120"))
+
+(operating-system
+  ...
+  (services (cons* (service kernel-module-loader-service-type
+                            '("ddcci" "ddcci_backlight"))
+                   (simple-service 'ddcci-config etc-service-type
+                                   (list `("modprobe.d/ddcci.conf"
+                                           ,ddcci-config)))
+                   %base-services))
+  (kernel-loadable-modules (list ddcci-driver-linux)))
+@end lisp
+@end deffn
 
 @node Miscellaneous Services
 @subsection Miscellaneous Services
diff --git a/gnu/services/linux.scm b/gnu/services/linux.scm
index caa0326c31..536d5f8c76 100644
--- a/gnu/services/linux.scm
+++ b/gnu/services/linux.scm
@@ -1,5 +1,6 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2020 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
+;;; Copyright © 2020 Brice Waegeneire <brice <at> waegenei.re>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -25,6 +26,8 @@
   #:use-module (gnu packages linux)
   #:use-module (srfi srfi-1)
   #:use-module (srfi srfi-26)
+  #:use-module (srfi srfi-34)
+  #:use-module (srfi srfi-35)
   #:use-module (ice-9 match)
   #:export (earlyoom-configuration
             earlyoom-configuration?
@@ -37,7 +40,9 @@
             earlyoom-configuration-ignore-positive-oom-score-adj?
             earlyoom-configuration-show-debug-messages?
             earlyoom-configuration-send-notification-command
-            earlyoom-service-type))
+            earlyoom-service-type
+
+            kernel-module-loader-service-type))
 
 
 ;;;
@@ -123,3 +128,50 @@ representation."
     (list (service-extension shepherd-root-service-type
                              (compose list earlyoom-shepherd-service))))
    (description "Run @command{earlyoom}, the Early OOM daemon.")))
+
+
+;;;
+;;; Kernel module loader.
+;;;
+
+(define kernel-module-loader-shepherd-service
+  (match-lambda
+    ((and (? list? kernel-modules) ((? string?) ...))
+     (list
+      (shepherd-service
+       (documentation "Load kernel modules.")
+       (provision '(kernel-module-loader))
+       (requirement '(file-systems))
+       (respawn? #f)
+       (one-shot? #t)
+       (modules `((srfi srfi-34)
+                  (srfi srfi-35)
+                  (rnrs io ports)
+                  ,@%default-modules))
+       (start
+        #~(lambda _
+            (unless (null? '#$kernel-modules)
+              (if (file-exists? "/proc/sys/kernel/modprobe")
+                  (let ((modprobe (call-with-input-file
+                                      "/proc/sys/kernel/modprobe" get-line)))
+                    (guard (c ((message-condition? c)
+                               (format (current-error-port) "~a~%"
+                                       (condition-message c))
+                               #f))
+                      (map (lambda (module)
+                             (invoke/quiet modprobe "--" module))
+                           '#$kernel-modules)))
+                  (begin (format (current-error-port) "error: ~a~%"
+                                 "Kernel is missing loadable module support.")
+                         #f))))))))))
+
+(define kernel-module-loader-service-type
+  (service-type
+   (name 'kernel-module-loader)
+   (description "Load kernel modules.")
+   (extensions
+    (list (service-extension shepherd-root-service-type
+                             kernel-module-loader-shepherd-service)))
+   (compose concatenate)
+   (extend append)
+   (default-value '())))
diff --git a/gnu/tests/linux-modules.scm b/gnu/tests/linux-modules.scm
index 39e11587c6..f1c4feb7a0 100644
--- a/gnu/tests/linux-modules.scm
+++ b/gnu/tests/linux-modules.scm
@@ -1,6 +1,7 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2019 Jakob L. Kreuze <zerodaysfordays <at> sdf.org>
 ;;; Copyright © 2020 Danny Milosavljevic <dannym <at> scratchpost.org>
+;;; Copyright © 2020 Brice Waegeneire <brice <at> waegenei.re>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -19,6 +20,8 @@
 
 (define-module (gnu tests linux-modules)
   #:use-module (gnu packages linux)
+  #:use-module (gnu services)
+  #:use-module (gnu services linux)
   #:use-module (gnu system)
   #:use-module (gnu system vm)
   #:use-module (gnu tests)
@@ -37,25 +40,30 @@
 ;;;
 ;;; Code:
 
-(define* (module-loader-program os modules)
-  "Return an executable store item that, upon being evaluated, will dry-run
-load MODULES."
+(define* (modules-loaded?-program os modules)
+  "Return an executable store item that, upon being evaluated, will verify
+that MODULES are actually loaded."
   (program-file
    "load-kernel-modules.scm"
-   (with-imported-modules (source-module-closure '((guix build utils)))
-     #~(begin
-         (use-modules (guix build utils))
-         (for-each (lambda (module)
-                     (invoke (string-append #$kmod "/bin/modprobe") "-n" "--"
-                             module))
-                   '#$modules)))))
+   #~(begin
+     (use-modules (ice-9 rdelim)
+                  (ice-9 popen))
+     (let* ((port (open-input-pipe (string-append #$kmod "/bin/lsmod")))
+            (output (read-string port))
+            (status (close-pipe port)))
+       (and-map (lambda (module) (string-contains output module))
+                '#$modules)))))
 
 (define* (run-loadable-kernel-modules-test module-packages module-names)
-  "Run a test of an OS having MODULE-PACKAGES, and modprobe MODULE-NAMES."
+  "Run a test of an OS having MODULE-PACKAGES, and verify that MODULE-NAMES
+are loaded in memory."
   (define os
     (marionette-operating-system
      (operating-system
       (inherit (simple-operating-system))
+      (services (cons (service kernel-module-loader-service-type module-names)
+                      (operating-system-user-services
+                       (simple-operating-system))))
       (kernel-loadable-modules module-packages))
      #:imported-modules '((guix combinators))))
   (define vm (virtual-machine os))
@@ -75,7 +83,8 @@ load MODULES."
              marionette))
           (test-end)
           (exit (= (test-runner-fail-count (test-runner-current)) 0)))))
-  (gexp->derivation "loadable-kernel-modules" (test (module-loader-program os module-names))))
+  (gexp->derivation "loadable-kernel-modules"
+                    (test (modules-loaded?-program os module-names))))
 
 (define %test-loadable-kernel-modules-0
   (system-test
-- 
2.26.0

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

From: Brice Waegeneire <brice <at> waegenei.re>
To: Danny Milosavljevic <dannym <at> scratchpost.org>
Cc: 40274 <at> debbugs.gnu.org,
 Guix-patches <guix-patches-bounces+brice+lists=waegenei.re <at> gnu.org>
Subject: Re: [bug#40274] [PATCH v6 2/2] gnu: Add kernel-module-loader-service.
Date: Sun, 05 Apr 2020 07:06:00 +0000

On 2020-04-04 21:11, Danny Milosavljevic wrote:
> On Sat, 4 Apr 2020 23:10:04 +0200
> Danny Milosavljevic <dannym <at> scratchpost.org> wrote:
> 
>> On Sat,  4 Apr 2020 17:17:32 +0200
>> Brice Waegeneire <brice <at> waegenei.re> wrote:
>> 
>> > get-line
>> 
>> Missing (ice-9 rdelim) ?
> 
> I meant (ice-9 textual-ports)

The tests reported a warning when (ice-9 textual-ports) was imported,
using (rnrs io ports) instead didn't.

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

From: Brice Waegeneire <brice <at> waegenei.re>
To: Danny Milosavljevic <dannym <at> scratchpost.org>
Cc: ludo <at> gnu.org, 40274 <at> debbugs.gnu.org,
 Guix-patches <guix-patches-bounces+brice+lists=waegenei.re <at> gnu.org>
Subject: Re: [bug#40274] [PATCH v5] gnu: Add kernel-module-loader-service.
Date: Sun, 05 Apr 2020 07:19:50 +0000

On 2020-04-04 18:31, Danny Milosavljevic wrote:
>> But like you noted it would
>> be specific
>> to loadable modules and not built-in ones since `kernel-arguments` is
>> just an OS
>> field, which AFAIK can't be influenced by a service.
> 
> There's no one stopping us from extending 
> operating-system-kernel-arguments,
> which already exists as a helper procedure, to also call
> operating-system-services and check those for the command line 
> arguments
> to add (see gnu/system.scm).

Thank you for pointing this to me, I didn't thought it was working this 
way.
So writing such a service is totally doable, interesting.

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

From: "pelzflorian (Florian Pelz)" <pelzflorian <at> pelzflorian.de>
To: Brice Waegeneire <brice <at> waegenei.re>
Cc: 40274 <at> debbugs.gnu.org
Subject: Re: [bug#40274] [PATCH v7 2/2] gnu: Add kernel-module-loader-service.
Date: Sun, 5 Apr 2020 12:06:46 +0200

On Sun, Apr 05, 2020 at 07:28:03AM +0200, Brice Waegeneire wrote:
> […]

I can use kernel-module-loader v6 without modifications just fine for
uvesafb.  Thank you!

Regards,
Florian

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

From: Danny Milosavljevic <dannym <at> scratchpost.org>
To: Brice Waegeneire <brice <at> waegenei.re>
Cc: 40274-done <at> debbugs.gnu.org
Subject: Re: [bug#40274] [PATCH v7 2/2] gnu: Add kernel-module-loader-service.
Date: Sun, 5 Apr 2020 13:10:16 +0200

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

Hi Brice,

I've pushed a variant of this to guix master as
commit 044d1478c9a63a64547c9cc320008f8d8fbf6791 with the following changes:

* Massively over-engineered the tests
* Made the shepherd conditional more readable

Thanks!

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

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

From: Danny Milosavljevic <dannym <at> scratchpost.org>
To: Brice Waegeneire <brice <at> waegenei.re>
Cc: 40274 <at> debbugs.gnu.org
Subject: Re: [bug#40274] [PATCH v7 1/2] services: Allow modprobe to use
 "/etc/modprobe.d".
Date: Sun, 5 Apr 2020 13:11:48 +0200

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

Pushed to guix master as commit 8c88e242292db9b35b4ae6ad788a0f8f3c94bb53.

I think core-updates will be merged to master pretty soon anyway, but the
question is whether we want to update it in core-updates before that, given
how many dependencies that has (WTF! What's up with that?).

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

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

From: Brice Waegeneire <brice <at> waegenei.re>
To: Danny Milosavljevic <dannym <at> scratchpost.org>
Cc: 40274 <at> debbugs.gnu.org
Subject: Re: [bug#40274] [PATCH v7 1/2] services: Allow modprobe to use
 "/etc/modprobe.d".
Date: Sun, 05 Apr 2020 12:38:14 +0000

On 2020-04-05 11:11, Danny Milosavljevic wrote:
> Pushed to guix master as commit 
> 8c88e242292db9b35b4ae6ad788a0f8f3c94bb53.
> 
> I think core-updates will be merged to master pretty soon anyway, but 
> the
> question is whether we want to update it in core-updates before that, 
> given
> how many dependencies that has (WTF! What's up with that?).

Thank you for helping with that!

I thought that core-updates was frozen which would delay #40422 for some 
time.
I wrote the crutch patch to avoid having to wait before supporting 
module
arguments, especially since Florian already had a need for it.

Previous Next

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