unofficial mirror of guix-patches@gnu.org 
 help / color / mirror / code / Atom feed
* [bug#40274] [PATCH] gnu: Add kernel-module-loader-service.
@ 2020-03-28 13:59 Brice Waegeneire
  2020-03-28 20:51 ` Mathieu Othacehe
                   ` (7 more replies)
  0 siblings, 8 replies; 44+ messages in thread
From: Brice Waegeneire @ 2020-03-28 13:59 UTC (permalink / raw)
  To: 40274

* 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@gmail.com>
+;;; Copyright © 2020 Brice Waegeneire <brice@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))
 
 \f
 ;;;
@@ -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

^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [bug#40274] [PATCH] gnu: Add kernel-module-loader-service.
  2020-03-28 13:59 [bug#40274] [PATCH] gnu: Add kernel-module-loader-service Brice Waegeneire
@ 2020-03-28 20:51 ` Mathieu Othacehe
  2020-03-31 17:22   ` Brice Waegeneire
  2020-03-31 17:19 ` [bug#40274] [PATCH v2] " Brice Waegeneire
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 44+ messages in thread
From: Mathieu Othacehe @ 2020-03-28 20:51 UTC (permalink / raw)
  To: 40274


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

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [bug#40274] [PATCH v2] gnu: Add kernel-module-loader-service.
  2020-03-28 13:59 [bug#40274] [PATCH] gnu: Add kernel-module-loader-service Brice Waegeneire
  2020-03-28 20:51 ` Mathieu Othacehe
@ 2020-03-31 17:19 ` Brice Waegeneire
  2020-03-31 20:00 ` [bug#40274] [PATCH v3] " Brice Waegeneire
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 44+ messages in thread
From: Brice Waegeneire @ 2020-03-31 17:19 UTC (permalink / raw)
  To: 40274

* 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@gmail.com>
+;;; Copyright © 2020 Brice Waegeneire <brice@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))
 
 \f
 ;;;
@@ -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@sdf.org>
 ;;; Copyright © 2020 Danny Milosavljevic <dannym@scratchpost.org>
+;;; Copyright © 2020 Brice Waegeneire <brice@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

^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [bug#40274] [PATCH] gnu: Add kernel-module-loader-service.
  2020-03-28 20:51 ` Mathieu Othacehe
@ 2020-03-31 17:22   ` Brice Waegeneire
  0 siblings, 0 replies; 44+ messages in thread
From: Brice Waegeneire @ 2020-03-31 17:22 UTC (permalink / raw)
  To: Mathieu Othacehe; +Cc: 40274, Guix-patches

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

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [bug#40274] [PATCH v3] gnu: Add kernel-module-loader-service.
  2020-03-28 13:59 [bug#40274] [PATCH] gnu: Add kernel-module-loader-service Brice Waegeneire
  2020-03-28 20:51 ` Mathieu Othacehe
  2020-03-31 17:19 ` [bug#40274] [PATCH v2] " Brice Waegeneire
@ 2020-03-31 20:00 ` Brice Waegeneire
  2020-03-31 20:10 ` [bug#40274] [PATCH] " Nicolò Balzarotti
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 44+ messages in thread
From: Brice Waegeneire @ 2020-03-31 20:00 UTC (permalink / raw)
  To: 40274

* 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@gmail.com>
+;;; Copyright © 2020 Brice Waegeneire <brice@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))
 
 \f
 ;;;
@@ -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@sdf.org>
 ;;; Copyright © 2020 Danny Milosavljevic <dannym@scratchpost.org>
+;;; Copyright © 2020 Brice Waegeneire <brice@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

^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [bug#40274] [PATCH] gnu: Add kernel-module-loader-service.
  2020-03-28 13:59 [bug#40274] [PATCH] gnu: Add kernel-module-loader-service Brice Waegeneire
                   ` (2 preceding siblings ...)
  2020-03-31 20:00 ` [bug#40274] [PATCH v3] " Brice Waegeneire
@ 2020-03-31 20:10 ` Nicolò Balzarotti
  2020-03-31 20:25   ` Brice Waegeneire
  2020-03-31 20:23 ` [bug#40274] [PATCH v4] " Brice Waegeneire
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 44+ messages in thread
From: Nicolò Balzarotti @ 2020-03-31 20:10 UTC (permalink / raw)
  To: Brice Waegeneire, 40274

Hi, I found a typo

/usefull/useful/

Nicolò

Brice Waegeneire <brice@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@gmail.com>
> +;;; Copyright © 2020 Brice Waegeneire <brice@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))
>  
>  \f
>  ;;;
> @@ -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

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [bug#40274] [PATCH v4] gnu: Add kernel-module-loader-service.
  2020-03-28 13:59 [bug#40274] [PATCH] gnu: Add kernel-module-loader-service Brice Waegeneire
                   ` (3 preceding siblings ...)
  2020-03-31 20:10 ` [bug#40274] [PATCH] " Nicolò Balzarotti
@ 2020-03-31 20:23 ` Brice Waegeneire
  2020-04-01 13:30   ` Mathieu Othacehe
  2020-04-01 19:34   ` pelzflorian (Florian Pelz)
  2020-04-02 12:37 ` [bug#40274] [PATCH v5] " Brice Waegeneire
                   ` (2 subsequent siblings)
  7 siblings, 2 replies; 44+ messages in thread
From: Brice Waegeneire @ 2020-03-31 20:23 UTC (permalink / raw)
  To: 40274

* 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@gmail.com>
+;;; Copyright © 2020 Brice Waegeneire <brice@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))
 
 \f
 ;;;
@@ -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@sdf.org>
 ;;; Copyright © 2020 Danny Milosavljevic <dannym@scratchpost.org>
+;;; Copyright © 2020 Brice Waegeneire <brice@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

^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [bug#40274] [PATCH] gnu: Add kernel-module-loader-service.
  2020-03-31 20:10 ` [bug#40274] [PATCH] " Nicolò Balzarotti
@ 2020-03-31 20:25   ` Brice Waegeneire
  0 siblings, 0 replies; 44+ messages in thread
From: Brice Waegeneire @ 2020-03-31 20:25 UTC (permalink / raw)
  To: Nicolò Balzarotti; +Cc: 40274, Guix-patches

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

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [bug#40274] [PATCH v4] gnu: Add kernel-module-loader-service.
  2020-03-31 20:23 ` [bug#40274] [PATCH v4] " Brice Waegeneire
@ 2020-04-01 13:30   ` Mathieu Othacehe
  2020-04-01 14:38     ` Brice Waegeneire
  2020-04-01 19:34   ` pelzflorian (Florian Pelz)
  1 sibling, 1 reply; 44+ messages in thread
From: Mathieu Othacehe @ 2020-04-01 13:30 UTC (permalink / raw)
  To: 40274


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

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [bug#40274] [PATCH v4] gnu: Add kernel-module-loader-service.
  2020-04-01 13:30   ` Mathieu Othacehe
@ 2020-04-01 14:38     ` Brice Waegeneire
  0 siblings, 0 replies; 44+ messages in thread
From: Brice Waegeneire @ 2020-04-01 14:38 UTC (permalink / raw)
  To: Mathieu Othacehe; +Cc: 40274, Guix-patches

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

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [bug#40274] [PATCH v4] gnu: Add kernel-module-loader-service.
  2020-03-31 20:23 ` [bug#40274] [PATCH v4] " Brice Waegeneire
  2020-04-01 13:30   ` Mathieu Othacehe
@ 2020-04-01 19:34   ` pelzflorian (Florian Pelz)
  2020-04-01 19:43     ` pelzflorian (Florian Pelz)
  2020-04-02 12:40     ` Brice Waegeneire
  1 sibling, 2 replies; 44+ messages in thread
From: pelzflorian (Florian Pelz) @ 2020-04-01 19:34 UTC (permalink / raw)
  To: Brice Waegeneire; +Cc: 40274

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

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

[-- Attachment #2: uvesafb-fails.patch --]
[-- Type: text/plain, Size: 1977 bytes --]

diff --git a/gnu/system/install.scm b/gnu/system/install.scm
index c15c2c7814..aa42449098 100644
--- a/gnu/system/install.scm
+++ b/gnu/system/install.scm
@@ -26,11 +26,13 @@
   #:use-module (gnu bootloader u-boot)
   #:use-module (guix gexp)
   #:use-module (guix store)
+  #:use-module (guix utils)
   #:use-module (guix monads)
   #:use-module ((guix packages) #:select (package-version))
   #:use-module ((guix store) #:select (%store-prefix))
   #:use-module (gnu installer)
   #:use-module (gnu services dbus)
+  #:use-module (gnu services linux)
   #:use-module (gnu services networking)
   #:use-module (gnu services shepherd)
   #:use-module (gnu services ssh)
@@ -50,6 +52,7 @@
   #:use-module (gnu packages texinfo)
   #:use-module (gnu packages compression)
   #:use-module (gnu packages nvi)
+  #:use-module (gnu packages xorg)
   #:use-module (ice-9 match)
   #:use-module (srfi srfi-26)
   #:export (installation-os
@@ -304,7 +307,8 @@ Access documentation at any time by pressing Alt-F2.\x1b[0m
     (define bare-bones-os
       (load "examples/bare-bones.tmpl"))
 
-    (list (service virtual-terminal-service-type)
+    (cons*
+          (service virtual-terminal-service-type)
 
           (service kmscon-service-type
                    (kmscon-configuration
@@ -408,7 +412,13 @@ Access documentation at any time by pressing Alt-F2.\x1b[0m
                    (list bare-bones-os
                          glibc-utf8-locales
                          texinfo
-                         (canonical-package guile-2.2))))))
+                         (canonical-package guile-2.2)))
+          (if (member (%current-system) '("x86_64-linux" "i686-linux"))
+              ;; Load uvesafb to show installer when no KMS is available.
+              `(,(kernel-module-loader-service
+                  '(("uvesafb" (string-append "v86d=" #$v86d "/sbin/v86d")
+                     "mode_option=1024x768"))))
+              '()))))
 
 (define %issue
   ;; Greeting.

^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [bug#40274] [PATCH v4] gnu: Add kernel-module-loader-service.
  2020-04-01 19:34   ` pelzflorian (Florian Pelz)
@ 2020-04-01 19:43     ` pelzflorian (Florian Pelz)
  2020-04-01 19:48       ` pelzflorian (Florian Pelz)
  2020-04-02 12:40     ` Brice Waegeneire
  1 sibling, 1 reply; 44+ messages in thread
From: pelzflorian (Florian Pelz) @ 2020-04-01 19:43 UTC (permalink / raw)
  To: Brice Waegeneire; +Cc: 40274

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?

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [bug#40274] [PATCH v4] gnu: Add kernel-module-loader-service.
  2020-04-01 19:43     ` pelzflorian (Florian Pelz)
@ 2020-04-01 19:48       ` pelzflorian (Florian Pelz)
  0 siblings, 0 replies; 44+ messages in thread
From: pelzflorian (Florian Pelz) @ 2020-04-01 19:48 UTC (permalink / raw)
  To: Brice Waegeneire; +Cc: 40274

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

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [bug#40274] [PATCH v5] gnu: Add kernel-module-loader-service.
  2020-03-28 13:59 [bug#40274] [PATCH] gnu: Add kernel-module-loader-service Brice Waegeneire
                   ` (4 preceding siblings ...)
  2020-03-31 20:23 ` [bug#40274] [PATCH v4] " Brice Waegeneire
@ 2020-04-02 12:37 ` Brice Waegeneire
  2020-04-02 13:56   ` Danny Milosavljevic
  2020-04-02 14:22   ` Mathieu Othacehe
  2020-04-04 15:17 ` [bug#40274] [PATCH v6 0/2] " Brice Waegeneire
  2020-04-05  5:28 ` [bug#40274] [PATCH v7 0/2] " Brice Waegeneire
  7 siblings, 2 replies; 44+ messages in thread
From: Brice Waegeneire @ 2020-04-02 12:37 UTC (permalink / raw)
  To: 40274

* 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@gmail.com>
+;;; Copyright © 2020 Brice Waegeneire <brice@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))
 
 \f
 ;;;
@@ -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.")))
+
+\f
+;;;
+;;; 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@sdf.org>
 ;;; Copyright © 2020 Danny Milosavljevic <dannym@scratchpost.org>
+;;; Copyright © 2020 Brice Waegeneire <brice@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

^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [bug#40274] [PATCH v4] gnu: Add kernel-module-loader-service.
  2020-04-01 19:34   ` pelzflorian (Florian Pelz)
  2020-04-01 19:43     ` pelzflorian (Florian Pelz)
@ 2020-04-02 12:40     ` Brice Waegeneire
  2020-04-03 13:03       ` pelzflorian (Florian Pelz)
  1 sibling, 1 reply; 44+ messages in thread
From: Brice Waegeneire @ 2020-04-02 12:40 UTC (permalink / raw)
  To: pelzflorian (Florian Pelz); +Cc: 40274

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

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [bug#40274] [PATCH v5] gnu: Add kernel-module-loader-service.
  2020-04-02 12:37 ` [bug#40274] [PATCH v5] " Brice Waegeneire
@ 2020-04-02 13:56   ` Danny Milosavljevic
  2020-04-02 17:13     ` Brice Waegeneire
  2020-04-02 14:22   ` Mathieu Othacehe
  1 sibling, 1 reply; 44+ messages in thread
From: Danny Milosavljevic @ 2020-04-02 13:56 UTC (permalink / raw)
  To: Brice Waegeneire; +Cc: ludo, 40274

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

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!)

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

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [bug#40274] [PATCH v5] gnu: Add kernel-module-loader-service.
  2020-04-02 12:37 ` [bug#40274] [PATCH v5] " Brice Waegeneire
  2020-04-02 13:56   ` Danny Milosavljevic
@ 2020-04-02 14:22   ` Mathieu Othacehe
  1 sibling, 0 replies; 44+ messages in thread
From: Mathieu Othacehe @ 2020-04-02 14:22 UTC (permalink / raw)
  To: Brice Waegeneire; +Cc: 40274


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

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [bug#40274] [PATCH v5] gnu: Add kernel-module-loader-service.
  2020-04-02 13:56   ` Danny Milosavljevic
@ 2020-04-02 17:13     ` Brice Waegeneire
  2020-04-03 10:20       ` Danny Milosavljevic
  0 siblings, 1 reply; 44+ messages in thread
From: Brice Waegeneire @ 2020-04-02 17:13 UTC (permalink / raw)
  To: Danny Milosavljevic; +Cc: ludo, 40274

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

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [bug#40274] [PATCH v5] gnu: Add kernel-module-loader-service.
  2020-04-02 17:13     ` Brice Waegeneire
@ 2020-04-03 10:20       ` Danny Milosavljevic
  2020-04-03 12:58         ` pelzflorian (Florian Pelz)
                           ` (2 more replies)
  0 siblings, 3 replies; 44+ messages in thread
From: Danny Milosavljevic @ 2020-04-03 10:20 UTC (permalink / raw)
  To: Brice Waegeneire; +Cc: ludo, 40274

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

Hi Brice,

On Thu, 02 Apr 2020 17:13:05 +0000
Brice Waegeneire <brice@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)

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

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [bug#40274] [PATCH v5] gnu: Add kernel-module-loader-service.
  2020-04-03 10:20       ` Danny Milosavljevic
@ 2020-04-03 12:58         ` pelzflorian (Florian Pelz)
  2020-04-04 10:51         ` Danny Milosavljevic
  2020-04-04 17:58         ` Brice Waegeneire
  2 siblings, 0 replies; 44+ messages in thread
From: pelzflorian (Florian Pelz) @ 2020-04-03 12:58 UTC (permalink / raw)
  To: Danny Milosavljevic; +Cc: 40274, ludo, Brice Waegeneire

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

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@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

[-- Attachment #2: uvesafb-gexp.patch --]
[-- Type: text/plain, Size: 2028 bytes --]

diff --git a/gnu/system/install.scm b/gnu/system/install.scm
index c15c2c7814..7327efe195 100644
--- a/gnu/system/install.scm
+++ b/gnu/system/install.scm
@@ -26,11 +26,13 @@
   #:use-module (gnu bootloader u-boot)
   #:use-module (guix gexp)
   #:use-module (guix store)
+  #:use-module (guix utils)
   #:use-module (guix monads)
   #:use-module ((guix packages) #:select (package-version))
   #:use-module ((guix store) #:select (%store-prefix))
   #:use-module (gnu installer)
   #:use-module (gnu services dbus)
+  #:use-module (gnu services linux)
   #:use-module (gnu services networking)
   #:use-module (gnu services shepherd)
   #:use-module (gnu services ssh)
@@ -50,6 +52,7 @@
   #:use-module (gnu packages texinfo)
   #:use-module (gnu packages compression)
   #:use-module (gnu packages nvi)
+  #:use-module (gnu packages xorg)
   #:use-module (ice-9 match)
   #:use-module (srfi srfi-26)
   #:export (installation-os
@@ -304,7 +307,8 @@ Access documentation at any time by pressing Alt-F2.\x1b[0m
     (define bare-bones-os
       (load "examples/bare-bones.tmpl"))
 
-    (list (service virtual-terminal-service-type)
+    (cons*
+          (service virtual-terminal-service-type)
 
           (service kmscon-service-type
                    (kmscon-configuration
@@ -408,7 +412,14 @@ Access documentation at any time by pressing Alt-F2.\x1b[0m
                    (list bare-bones-os
                          glibc-utf8-locales
                          texinfo
-                         (canonical-package guile-2.2))))))
+                         (canonical-package guile-2.2)))
+          (if (member (%current-system) '("x86_64-linux" "i686-linux"))
+              ;; Load uvesafb to show installer when no KMS is available.
+              `(,(kernel-module-loader-service
+                  (list (list "uvesafb"
+                              #~(string-append "v86d=" #$v86d "/sbin/v86d")
+                              "mode_option=1024x768"))))
+              '()))))
 
 (define %issue
   ;; Greeting.

^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [bug#40274] [PATCH v4] gnu: Add kernel-module-loader-service.
  2020-04-02 12:40     ` Brice Waegeneire
@ 2020-04-03 13:03       ` pelzflorian (Florian Pelz)
  0 siblings, 0 replies; 44+ messages in thread
From: pelzflorian (Florian Pelz) @ 2020-04-03 13:03 UTC (permalink / raw)
  To: Brice Waegeneire; +Cc: 40274

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

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [bug#40274] [PATCH v5] gnu: Add kernel-module-loader-service.
  2020-04-03 10:20       ` Danny Milosavljevic
  2020-04-03 12:58         ` pelzflorian (Florian Pelz)
@ 2020-04-04 10:51         ` Danny Milosavljevic
  2020-04-04 17:58         ` Brice Waegeneire
  2 siblings, 0 replies; 44+ messages in thread
From: Danny Milosavljevic @ 2020-04-04 10:51 UTC (permalink / raw)
  To: Brice Waegeneire; +Cc: ludo, 40274

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

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".

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

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [bug#40274] [PATCH v6 0/2] gnu: Add kernel-module-loader-service.
  2020-03-28 13:59 [bug#40274] [PATCH] gnu: Add kernel-module-loader-service Brice Waegeneire
                   ` (5 preceding siblings ...)
  2020-04-02 12:37 ` [bug#40274] [PATCH v5] " Brice Waegeneire
@ 2020-04-04 15:17 ` Brice Waegeneire
  2020-04-04 15:17   ` [bug#40274] [PATCH v6 1/2] services: Allow modprobe to use "/etc/modprobe.d" Brice Waegeneire
  2020-04-04 15:17   ` [bug#40274] [PATCH v6 2/2] gnu: Add kernel-module-loader-service Brice Waegeneire
  2020-04-05  5:28 ` [bug#40274] [PATCH v7 0/2] " Brice Waegeneire
  7 siblings, 2 replies; 44+ messages in thread
From: Brice Waegeneire @ 2020-04-04 15:17 UTC (permalink / raw)
  To: 40274

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

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [bug#40274] [PATCH v6 1/2] services: Allow modprobe to use "/etc/modprobe.d".
  2020-04-04 15:17 ` [bug#40274] [PATCH v6 0/2] " Brice Waegeneire
@ 2020-04-04 15:17   ` Brice Waegeneire
  2020-04-04 15:17   ` [bug#40274] [PATCH v6 2/2] gnu: Add kernel-module-loader-service Brice Waegeneire
  1 sibling, 0 replies; 44+ messages in thread
From: Brice Waegeneire @ 2020-04-04 15:17 UTC (permalink / raw)
  To: 40274

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

^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [bug#40274] [PATCH v6 2/2] gnu: Add kernel-module-loader-service.
  2020-04-04 15:17 ` [bug#40274] [PATCH v6 0/2] " Brice Waegeneire
  2020-04-04 15:17   ` [bug#40274] [PATCH v6 1/2] services: Allow modprobe to use "/etc/modprobe.d" Brice Waegeneire
@ 2020-04-04 15:17   ` Brice Waegeneire
  2020-04-04 15:53     ` Mathieu Othacehe
                       ` (3 more replies)
  1 sibling, 4 replies; 44+ messages in thread
From: Brice Waegeneire @ 2020-04-04 15:17 UTC (permalink / raw)
  To: 40274

* 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@gmail.com>
+;;; Copyright © 2020 Brice Waegeneire <brice@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))
 
 \f
 ;;;
@@ -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.")))
+
+\f
+;;;
+;;; 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@sdf.org>
 ;;; Copyright © 2020 Danny Milosavljevic <dannym@scratchpost.org>
+;;; Copyright © 2020 Brice Waegeneire <brice@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

^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [bug#40274] [PATCH v6 2/2] gnu: Add kernel-module-loader-service.
  2020-04-04 15:17   ` [bug#40274] [PATCH v6 2/2] gnu: Add kernel-module-loader-service Brice Waegeneire
@ 2020-04-04 15:53     ` Mathieu Othacehe
  2020-04-04 16:09     ` pelzflorian (Florian Pelz)
                       ` (2 subsequent siblings)
  3 siblings, 0 replies; 44+ messages in thread
From: Mathieu Othacehe @ 2020-04-04 15:53 UTC (permalink / raw)
  To: Brice Waegeneire; +Cc: 40274


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

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [bug#40274] [PATCH v6 2/2] gnu: Add kernel-module-loader-service.
  2020-04-04 15:17   ` [bug#40274] [PATCH v6 2/2] gnu: Add kernel-module-loader-service Brice Waegeneire
  2020-04-04 15:53     ` Mathieu Othacehe
@ 2020-04-04 16:09     ` pelzflorian (Florian Pelz)
  2020-04-04 16:49       ` Brice Waegeneire
  2020-04-04 21:10     ` Danny Milosavljevic
  2020-04-04 21:16     ` Danny Milosavljevic
  3 siblings, 1 reply; 44+ messages in thread
From: pelzflorian (Florian Pelz) @ 2020-04-04 16:09 UTC (permalink / raw)
  To: Brice Waegeneire; +Cc: 40274

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

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [bug#40274] [PATCH v6 2/2] gnu: Add kernel-module-loader-service.
  2020-04-04 16:09     ` pelzflorian (Florian Pelz)
@ 2020-04-04 16:49       ` Brice Waegeneire
  2020-04-04 17:46         ` pelzflorian (Florian Pelz)
  0 siblings, 1 reply; 44+ messages in thread
From: Brice Waegeneire @ 2020-04-04 16:49 UTC (permalink / raw)
  To: pelzflorian (Florian Pelz); +Cc: 40274, Guix-patches

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

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [bug#40274] [PATCH v6 2/2] gnu: Add kernel-module-loader-service.
  2020-04-04 16:49       ` Brice Waegeneire
@ 2020-04-04 17:46         ` pelzflorian (Florian Pelz)
  0 siblings, 0 replies; 44+ messages in thread
From: pelzflorian (Florian Pelz) @ 2020-04-04 17:46 UTC (permalink / raw)
  To: Brice Waegeneire; +Cc: 40274, Guix-patches

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

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@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

[-- Attachment #2: uvesafb-3.patch --]
[-- Type: text/plain, Size: 2667 bytes --]

diff --git a/gnu/system/install.scm b/gnu/system/install.scm
index c15c2c7814..1348c63f25 100644
--- a/gnu/system/install.scm
+++ b/gnu/system/install.scm
@@ -26,11 +26,13 @@
   #:use-module (gnu bootloader u-boot)
   #:use-module (guix gexp)
   #:use-module (guix store)
+  #:use-module (guix utils)
   #:use-module (guix monads)
   #:use-module ((guix packages) #:select (package-version))
   #:use-module ((guix store) #:select (%store-prefix))
   #:use-module (gnu installer)
   #:use-module (gnu services dbus)
+  #:use-module (gnu services linux)
   #:use-module (gnu services networking)
   #:use-module (gnu services shepherd)
   #:use-module (gnu services ssh)
@@ -50,6 +52,7 @@
   #:use-module (gnu packages texinfo)
   #:use-module (gnu packages compression)
   #:use-module (gnu packages nvi)
+  #:use-module (gnu packages xorg)
   #:use-module (ice-9 match)
   #:use-module (srfi srfi-26)
   #:export (installation-os
@@ -304,7 +307,8 @@ Access documentation at any time by pressing Alt-F2.\x1b[0m
     (define bare-bones-os
       (load "examples/bare-bones.tmpl"))
 
-    (list (service virtual-terminal-service-type)
+    (cons*
+          (service virtual-terminal-service-type)
 
           (service kmscon-service-type
                    (kmscon-configuration
@@ -408,7 +412,23 @@ Access documentation at any time by pressing Alt-F2.\x1b[0m
                    (list bare-bones-os
                          glibc-utf8-locales
                          texinfo
-                         (canonical-package guile-2.2))))))
+                         (canonical-package guile-2.2)))
+          (if (member (%current-system) '("x86_64-linux" "i686-linux"))
+              ;; Load uvesafb to show installer when no KMS is available.
+              `(,(service kernel-module-loader-service-type '("uvesafb"))
+                ,(let ((uvesafb-options
+                        #~(with-output-to-file #$output
+                            (lambda _
+                              (format #t
+                                (string-join `("options" "uvesafb"
+                                               ,(string-append "v86d=" #$v86d
+                                                               "/sbin/v86d")
+                                               "mode_option=1024x768")))))))
+                   (simple-service 'uvesafb-configuration etc-service-type
+                                   (list `("modprobe.d/uvesafb.conf"
+                                           ,(computed-file "uvesafb-options"
+                                                           uvesafb-options))))))
+              '()))))
 
 (define %issue
   ;; Greeting.

^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [bug#40274] [PATCH v5] gnu: Add kernel-module-loader-service.
  2020-04-03 10:20       ` Danny Milosavljevic
  2020-04-03 12:58         ` pelzflorian (Florian Pelz)
  2020-04-04 10:51         ` Danny Milosavljevic
@ 2020-04-04 17:58         ` Brice Waegeneire
  2020-04-04 18:31           ` Danny Milosavljevic
  2 siblings, 1 reply; 44+ messages in thread
From: Brice Waegeneire @ 2020-04-04 17:58 UTC (permalink / raw)
  To: Danny Milosavljevic; +Cc: ludo, 40274

Hello,

On 2020-04-03 10:20, Danny Milosavljevic wrote:
> On Thu, 02 Apr 2020 17:13:05 +0000
> Brice Waegeneire <brice@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

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [bug#40274] [PATCH v5] gnu: Add kernel-module-loader-service.
  2020-04-04 17:58         ` Brice Waegeneire
@ 2020-04-04 18:31           ` Danny Milosavljevic
  2020-04-05  7:19             ` Brice Waegeneire
  0 siblings, 1 reply; 44+ messages in thread
From: Danny Milosavljevic @ 2020-04-04 18:31 UTC (permalink / raw)
  To: Brice Waegeneire; +Cc: ludo, 40274

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

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).

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

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [bug#40274] [PATCH v6 2/2] gnu: Add kernel-module-loader-service.
  2020-04-04 15:17   ` [bug#40274] [PATCH v6 2/2] gnu: Add kernel-module-loader-service Brice Waegeneire
  2020-04-04 15:53     ` Mathieu Othacehe
  2020-04-04 16:09     ` pelzflorian (Florian Pelz)
@ 2020-04-04 21:10     ` Danny Milosavljevic
  2020-04-04 21:11       ` Danny Milosavljevic
  2020-04-04 21:16     ` Danny Milosavljevic
  3 siblings, 1 reply; 44+ messages in thread
From: Danny Milosavljevic @ 2020-04-04 21:10 UTC (permalink / raw)
  To: Brice Waegeneire; +Cc: 40274

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

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

> get-line

Missing (ice-9 rdelim) ?

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

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [bug#40274] [PATCH v6 2/2] gnu: Add kernel-module-loader-service.
  2020-04-04 21:10     ` Danny Milosavljevic
@ 2020-04-04 21:11       ` Danny Milosavljevic
  2020-04-04 23:06         ` pelzflorian (Florian Pelz)
  2020-04-05  7:06         ` Brice Waegeneire
  0 siblings, 2 replies; 44+ messages in thread
From: Danny Milosavljevic @ 2020-04-04 21:11 UTC (permalink / raw)
  To: Brice Waegeneire; +Cc: 40274

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

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

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

I meant (ice-9 textual-ports)

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

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [bug#40274] [PATCH v6 2/2] gnu: Add kernel-module-loader-service.
  2020-04-04 15:17   ` [bug#40274] [PATCH v6 2/2] gnu: Add kernel-module-loader-service Brice Waegeneire
                       ` (2 preceding siblings ...)
  2020-04-04 21:10     ` Danny Milosavljevic
@ 2020-04-04 21:16     ` Danny Milosavljevic
  3 siblings, 0 replies; 44+ messages in thread
From: Danny Milosavljevic @ 2020-04-04 21:16 UTC (permalink / raw)
  To: Brice Waegeneire; +Cc: 40274

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

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.

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

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [bug#40274] [PATCH v6 2/2] gnu: Add kernel-module-loader-service.
  2020-04-04 21:11       ` Danny Milosavljevic
@ 2020-04-04 23:06         ` pelzflorian (Florian Pelz)
  2020-04-05  7:06         ` Brice Waegeneire
  1 sibling, 0 replies; 44+ messages in thread
From: pelzflorian (Florian Pelz) @ 2020-04-04 23:06 UTC (permalink / raw)
  To: Danny Milosavljevic; +Cc: 40274, Brice Waegeneire

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

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

[-- Attachment #2: get-line.patch --]
[-- Type: text/plain, Size: 723 bytes --]

diff --git a/gnu/services/linux.scm b/gnu/services/linux.scm
index 621d26fcff..61a7dbb9a1 100644
--- a/gnu/services/linux.scm
+++ b/gnu/services/linux.scm
@@ -151,7 +151,8 @@ representation."
         #~(lambda _
             (if (file-exists? "/proc/sys/kernel/modprobe")
                 (let ((modprobe (call-with-input-file
-                                    "/proc/sys/kernel/modprobe" get-line)))
+                                    "/proc/sys/kernel/modprobe"
+                                  (@ (ice-9 textual-ports) get-line))))
                   (guard (c ((message-condition? c)
                              (format (current-error-port) "~a~%"
                                      (condition-message c))

^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [bug#40274] [PATCH v7 0/2]  gnu: Add kernel-module-loader-service.
  2020-03-28 13:59 [bug#40274] [PATCH] gnu: Add kernel-module-loader-service Brice Waegeneire
                   ` (6 preceding siblings ...)
  2020-04-04 15:17 ` [bug#40274] [PATCH v6 0/2] " Brice Waegeneire
@ 2020-04-05  5:28 ` Brice Waegeneire
  2020-04-05  5:28   ` [bug#40274] [PATCH v7 1/2] services: Allow modprobe to use "/etc/modprobe.d" Brice Waegeneire
  2020-04-05  5:28   ` [bug#40274] [PATCH v7 2/2] gnu: Add kernel-module-loader-service Brice Waegeneire
  7 siblings, 2 replies; 44+ messages in thread
From: Brice Waegeneire @ 2020-04-05  5:28 UTC (permalink / raw)
  To: 40274

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

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [bug#40274] [PATCH v7 1/2] services: Allow modprobe to use "/etc/modprobe.d".
  2020-04-05  5:28 ` [bug#40274] [PATCH v7 0/2] " Brice Waegeneire
@ 2020-04-05  5:28   ` Brice Waegeneire
  2020-04-05 11:11     ` Danny Milosavljevic
  2020-04-05  5:28   ` [bug#40274] [PATCH v7 2/2] gnu: Add kernel-module-loader-service Brice Waegeneire
  1 sibling, 1 reply; 44+ messages in thread
From: Brice Waegeneire @ 2020-04-05  5:28 UTC (permalink / raw)
  To: 40274

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

^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [bug#40274] [PATCH v7 2/2] gnu: Add kernel-module-loader-service.
  2020-04-05  5:28 ` [bug#40274] [PATCH v7 0/2] " Brice Waegeneire
  2020-04-05  5:28   ` [bug#40274] [PATCH v7 1/2] services: Allow modprobe to use "/etc/modprobe.d" Brice Waegeneire
@ 2020-04-05  5:28   ` Brice Waegeneire
  2020-04-05 10:06     ` pelzflorian (Florian Pelz)
  2020-04-05 11:10     ` bug#40274: " Danny Milosavljevic
  1 sibling, 2 replies; 44+ messages in thread
From: Brice Waegeneire @ 2020-04-05  5:28 UTC (permalink / raw)
  To: 40274

* 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@gmail.com>
+;;; Copyright © 2020 Brice Waegeneire <brice@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))
 
 \f
 ;;;
@@ -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.")))
+
+\f
+;;;
+;;; 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@sdf.org>
 ;;; Copyright © 2020 Danny Milosavljevic <dannym@scratchpost.org>
+;;; Copyright © 2020 Brice Waegeneire <brice@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

^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [bug#40274] [PATCH v6 2/2] gnu: Add kernel-module-loader-service.
  2020-04-04 21:11       ` Danny Milosavljevic
  2020-04-04 23:06         ` pelzflorian (Florian Pelz)
@ 2020-04-05  7:06         ` Brice Waegeneire
  1 sibling, 0 replies; 44+ messages in thread
From: Brice Waegeneire @ 2020-04-05  7:06 UTC (permalink / raw)
  To: Danny Milosavljevic; +Cc: 40274, Guix-patches

On 2020-04-04 21:11, Danny Milosavljevic wrote:
> On Sat, 4 Apr 2020 23:10:04 +0200
> Danny Milosavljevic <dannym@scratchpost.org> wrote:
> 
>> On Sat,  4 Apr 2020 17:17:32 +0200
>> Brice Waegeneire <brice@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.

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [bug#40274] [PATCH v5] gnu: Add kernel-module-loader-service.
  2020-04-04 18:31           ` Danny Milosavljevic
@ 2020-04-05  7:19             ` Brice Waegeneire
  0 siblings, 0 replies; 44+ messages in thread
From: Brice Waegeneire @ 2020-04-05  7:19 UTC (permalink / raw)
  To: Danny Milosavljevic; +Cc: ludo, 40274, Guix-patches

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.

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [bug#40274] [PATCH v7 2/2] gnu: Add kernel-module-loader-service.
  2020-04-05  5:28   ` [bug#40274] [PATCH v7 2/2] gnu: Add kernel-module-loader-service Brice Waegeneire
@ 2020-04-05 10:06     ` pelzflorian (Florian Pelz)
  2020-04-05 11:10     ` bug#40274: " Danny Milosavljevic
  1 sibling, 0 replies; 44+ messages in thread
From: pelzflorian (Florian Pelz) @ 2020-04-05 10:06 UTC (permalink / raw)
  To: Brice Waegeneire; +Cc: 40274

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

^ permalink raw reply	[flat|nested] 44+ messages in thread

* bug#40274: [PATCH v7 2/2] gnu: Add kernel-module-loader-service.
  2020-04-05  5:28   ` [bug#40274] [PATCH v7 2/2] gnu: Add kernel-module-loader-service Brice Waegeneire
  2020-04-05 10:06     ` pelzflorian (Florian Pelz)
@ 2020-04-05 11:10     ` Danny Milosavljevic
  1 sibling, 0 replies; 44+ messages in thread
From: Danny Milosavljevic @ 2020-04-05 11:10 UTC (permalink / raw)
  To: Brice Waegeneire; +Cc: 40274-done

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

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!

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

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [bug#40274] [PATCH v7 1/2] services: Allow modprobe to use "/etc/modprobe.d".
  2020-04-05  5:28   ` [bug#40274] [PATCH v7 1/2] services: Allow modprobe to use "/etc/modprobe.d" Brice Waegeneire
@ 2020-04-05 11:11     ` Danny Milosavljevic
  2020-04-05 12:38       ` Brice Waegeneire
  0 siblings, 1 reply; 44+ messages in thread
From: Danny Milosavljevic @ 2020-04-05 11:11 UTC (permalink / raw)
  To: Brice Waegeneire; +Cc: 40274

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

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?).

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

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [bug#40274] [PATCH v7 1/2] services: Allow modprobe to use "/etc/modprobe.d".
  2020-04-05 11:11     ` Danny Milosavljevic
@ 2020-04-05 12:38       ` Brice Waegeneire
  0 siblings, 0 replies; 44+ messages in thread
From: Brice Waegeneire @ 2020-04-05 12:38 UTC (permalink / raw)
  To: Danny Milosavljevic; +Cc: 40274

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.

^ permalink raw reply	[flat|nested] 44+ messages in thread

end of thread, other threads:[~2020-04-05 12:39 UTC | newest]

Thread overview: 44+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-28 13:59 [bug#40274] [PATCH] gnu: Add kernel-module-loader-service Brice Waegeneire
2020-03-28 20:51 ` Mathieu Othacehe
2020-03-31 17:22   ` Brice Waegeneire
2020-03-31 17:19 ` [bug#40274] [PATCH v2] " Brice Waegeneire
2020-03-31 20:00 ` [bug#40274] [PATCH v3] " Brice Waegeneire
2020-03-31 20:10 ` [bug#40274] [PATCH] " Nicolò Balzarotti
2020-03-31 20:25   ` Brice Waegeneire
2020-03-31 20:23 ` [bug#40274] [PATCH v4] " Brice Waegeneire
2020-04-01 13:30   ` Mathieu Othacehe
2020-04-01 14:38     ` Brice Waegeneire
2020-04-01 19:34   ` pelzflorian (Florian Pelz)
2020-04-01 19:43     ` pelzflorian (Florian Pelz)
2020-04-01 19:48       ` pelzflorian (Florian Pelz)
2020-04-02 12:40     ` Brice Waegeneire
2020-04-03 13:03       ` pelzflorian (Florian Pelz)
2020-04-02 12:37 ` [bug#40274] [PATCH v5] " Brice Waegeneire
2020-04-02 13:56   ` Danny Milosavljevic
2020-04-02 17:13     ` Brice Waegeneire
2020-04-03 10:20       ` Danny Milosavljevic
2020-04-03 12:58         ` pelzflorian (Florian Pelz)
2020-04-04 10:51         ` Danny Milosavljevic
2020-04-04 17:58         ` Brice Waegeneire
2020-04-04 18:31           ` Danny Milosavljevic
2020-04-05  7:19             ` Brice Waegeneire
2020-04-02 14:22   ` Mathieu Othacehe
2020-04-04 15:17 ` [bug#40274] [PATCH v6 0/2] " Brice Waegeneire
2020-04-04 15:17   ` [bug#40274] [PATCH v6 1/2] services: Allow modprobe to use "/etc/modprobe.d" Brice Waegeneire
2020-04-04 15:17   ` [bug#40274] [PATCH v6 2/2] gnu: Add kernel-module-loader-service Brice Waegeneire
2020-04-04 15:53     ` Mathieu Othacehe
2020-04-04 16:09     ` pelzflorian (Florian Pelz)
2020-04-04 16:49       ` Brice Waegeneire
2020-04-04 17:46         ` pelzflorian (Florian Pelz)
2020-04-04 21:10     ` Danny Milosavljevic
2020-04-04 21:11       ` Danny Milosavljevic
2020-04-04 23:06         ` pelzflorian (Florian Pelz)
2020-04-05  7:06         ` Brice Waegeneire
2020-04-04 21:16     ` Danny Milosavljevic
2020-04-05  5:28 ` [bug#40274] [PATCH v7 0/2] " Brice Waegeneire
2020-04-05  5:28   ` [bug#40274] [PATCH v7 1/2] services: Allow modprobe to use "/etc/modprobe.d" Brice Waegeneire
2020-04-05 11:11     ` Danny Milosavljevic
2020-04-05 12:38       ` Brice Waegeneire
2020-04-05  5:28   ` [bug#40274] [PATCH v7 2/2] gnu: Add kernel-module-loader-service Brice Waegeneire
2020-04-05 10:06     ` pelzflorian (Florian Pelz)
2020-04-05 11:10     ` bug#40274: " Danny Milosavljevic

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).