bug#22050: fuse.ko not automatically loaded on GuixSD

bug#22050: fuse.ko not automatically loaded on GuixSD

[Ludovic Courtès]

On GuixSD, fuse.ko is not automatically loaded when someone tries to
access /dev/fuse:

--8<---------------cut here---------------start------------->8---
$ sshfs SOMEHOST /tmp/x
fuse: device not found, try 'modprobe fuse' first
--8<---------------cut here---------------end--------------->8---

We do have FUSE’s udev rules installed by default, but they don’t seem
to do much:

--8<---------------cut here---------------start------------->8---
50-udev-default.rules:75:KERNEL=="fuse", MODE="0666", OPTIONS+="static_node=fuse"
99-fuse.rules:1:KERNEL=="fuse", MODE="0666"
--8<---------------cut here---------------end--------------->8---

Ludo’.

bug#22050: [PATCH 0/2] Create static device nodes before starting udev.

[Danny Milosavljevic]

udev expects the device names of some special devices to already exist
when udev is started.

So ask kmod which they are and create them.

Danny Milosavljevic (2):
  linux-boot: Add make-static-device-nodes.
  services: base: Use make-static-device-nodes.

 gnu/build/linux-boot.scm | 62 ++++++++++++++++++++++++++++++++++++++++++++++++
 gnu/services/base.scm    |  7 +++++-
 2 files changed, 68 insertions(+), 1 deletion(-)

bug#22050: [PATCH 1/2] linux-boot: Add make-static-device-nodes.

[Danny Milosavljevic]

* gnu/build/linux-boot.scm (make-static-device-nodes): New variable.
---
 gnu/build/linux-boot.scm | 62 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 62 insertions(+)

diff --git a/gnu/build/linux-boot.scm b/gnu/build/linux-boot.scm
index 2547f1e0a..65273c87a 100644
--- a/gnu/build/linux-boot.scm
+++ b/gnu/build/linux-boot.scm
@@ -24,6 +24,8 @@
   #:use-module (srfi srfi-1)
   #:use-module (srfi srfi-26)
   #:use-module (ice-9 match)
+  #:use-module (ice-9 rdelim)
+  #:use-module (ice-9 popen)
   #:use-module (ice-9 ftw)
   #:use-module (guix build utils)
   #:use-module ((guix build syscalls)
@@ -35,6 +37,7 @@
             linux-command-line
             find-long-option
             make-essential-device-nodes
+            make-static-device-nodes
             configure-qemu-networking
 
             bind-mount
@@ -105,6 +108,65 @@ with the given MAJOR number, starting with MINOR."
              'block-special #o644 (device-number major (+ minor i)))
       (loop (+ i 1)))))
 
+(define (tmpfiles-mknod name type mode-string device-number-string)
+  "Given a NAME, TYPE, MODE-STRING, DEVICE-NUMBER-STRING,
+   call mknod with the respective numbers."
+  (let* ((mode (string->number mode-string 8))
+         (device-number-parts (string-split device-number-string #\:)))
+    (match device-number-parts
+     ((major-device-number-string minor-device-number-string)
+      (let ((major-device-number (string->number major-device-number-string))
+            (minor-device-number (string->number minor-device-number-string)))
+        (mknod name type #o660 (device-number major-device-number
+                                              minor-device-number))))
+     (_ #f))))
+
+(define (log-static-device-system-error name callback)
+  "Call CALLBACK.  If it fails, print an error message."
+  (catch 'system-error
+    (lambda ()
+      (apply callback args))
+    (lambda args
+      (format #t "could not create node '~a'~%" name))))
+
+(define* (make-static-device-nodes kmod-executable-name)
+  "Invoke and handle 'kmod static-nodes' output."
+  ;; "kmod static-nodes --format=tmpfiles" output format:
+  ;;   c! /dev/fuse 0600 - - - 10:229
+  ;;   d /dev/vfio 0755 - - -
+  (let ((port (open-pipe* OPEN_READ
+                          kmod-executable-name
+                          "static-nodes"
+                          "--format=tmpfiles"
+                          "--output=/proc/self/fd/1")))
+    (dynamic-wind
+      (lambda ()
+        #t)
+      (lambda ()
+        (let loop ((line (read-line port)))
+          (if (not (eof-object? line))
+            (let ((fields (string-split line #\space)))
+              (match fields
+               (("d" name mode-string "-" "-" "-")
+                (let ((mode (string->number mode-string 8)))
+                  (log-static-device-system-error name
+                    (lambda ()
+                      (mkdir name mode)))))
+               (("c!" name mode-string "-" "-" "-" device-number-string)
+                (log-static-device-system-error name
+                  (lambda ()
+                    (tmpfiles-mknod name 'char-special mode-string
+                                    device-number-string))))
+               (("b!" name mode-string "-" "-" "-" device-number-string)
+                (log-static-device-system-error name
+                  (lambda ()
+                    (tmpfiles-mknod name 'block-special mode-string
+                                    device-number-string))))
+               (_ #f))
+              (loop (read-line port))))))
+      (lambda ()
+        (close-pipe port)))))
+
 (define* (make-essential-device-nodes #:key (root "/"))
   "Make essential device nodes under ROOT/dev."
   ;; The hand-made devtmpfs/udev!

bug#22050: [PATCH 2/2] services: base: Use make-static-device-nodes.

[Danny Milosavljevic]

* gnu/services/base.scm (udev-shepherd-service): Use make-static-device-nodes.
---
 gnu/services/base.scm | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/gnu/services/base.scm b/gnu/services/base.scm
index 291dd6325..e1098b6d8 100644
--- a/gnu/services/base.scm
+++ b/gnu/services/base.scm
@@ -1727,6 +1727,8 @@ item of @var{packages}."
                     (setenv "EUDEV_RULES_DIRECTORY"
                             #$(file-append rules "/lib/udev/rules.d"))
 
+                    (make-static-device-nodes (string-append #$kmod "/bin/kmod"))
+
                     (let ((pid (primitive-fork)))
                       (case pid
                         ((0)
@@ -1750,7 +1752,10 @@ item of @var{packages}."
          ;; When halting the system, 'udev' is actually killed by
          ;; 'user-processes', i.e., before its own 'stop' method was called.
          ;; Thus, make sure it is not respawned.
-         (respawn? #f)))))))
+         (respawn? #f)
+         ;; We need additional modules.
+         (modules `((gnu build linux-boot)
+                    ,@%default-modules))))))))
 
 (define udev-service-type
   (service-type (name 'udev)

bug#22050: [PATCH v2 0/2] Create static device nodes before starting udev.

[Danny Milosavljevic]

udev expects the device names of some special devices to already exist
when udev is started.

So ask kmod which they are and create them.

Danny Milosavljevic (2):
  linux-boot: Add make-static-device-nodes.
  services: base: Use make-static-device-nodes.

 gnu/build/linux-boot.scm | 62 ++++++++++++++++++++++++++++++++++++++++++++++++
 gnu/services/base.scm    |  7 +++++-
 2 files changed, 68 insertions(+), 1 deletion(-)

bug#22050: [PATCH v2 1/2] linux-boot: Add make-static-device-nodes.

[Danny Milosavljevic]

* gnu/build/linux-boot.scm (make-static-device-nodes): New variable.
---
 gnu/build/linux-boot.scm | 62 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 62 insertions(+)

diff --git a/gnu/build/linux-boot.scm b/gnu/build/linux-boot.scm
index 2547f1e0a..54e919fcb 100644
--- a/gnu/build/linux-boot.scm
+++ b/gnu/build/linux-boot.scm
@@ -24,6 +24,8 @@
   #:use-module (srfi srfi-1)
   #:use-module (srfi srfi-26)
   #:use-module (ice-9 match)
+  #:use-module (ice-9 rdelim)
+  #:use-module (ice-9 popen)
   #:use-module (ice-9 ftw)
   #:use-module (guix build utils)
   #:use-module ((guix build syscalls)
@@ -35,6 +37,7 @@
             linux-command-line
             find-long-option
             make-essential-device-nodes
+            make-static-device-nodes
             configure-qemu-networking
 
             bind-mount
@@ -105,6 +108,65 @@ with the given MAJOR number, starting with MINOR."
              'block-special #o644 (device-number major (+ minor i)))
       (loop (+ i 1)))))
 
+(define (tmpfiles-mknod name type mode-string device-number-string)
+  "Given a NAME, TYPE, MODE-STRING, DEVICE-NUMBER-STRING,
+   call mknod with the respective numbers."
+  (let* ((mode (string->number mode-string 8))
+         (device-number-parts (string-split device-number-string #\:)))
+    (match device-number-parts
+     ((major-device-number-string minor-device-number-string)
+      (let ((major-device-number (string->number major-device-number-string))
+            (minor-device-number (string->number minor-device-number-string)))
+        (mknod name type #o660 (device-number major-device-number
+                                              minor-device-number))))
+     (_ #f))))
+
+(define (log-static-device-system-error name callback)
+  "Call CALLBACK.  If it fails, print an error message."
+  (catch 'system-error
+    (lambda ()
+      (callback))
+    (lambda args
+      (format #t "could not create node '~a'~%" name))))
+
+(define* (make-static-device-nodes kmod-executable-name)
+  "Invoke and handle 'kmod static-nodes' output."
+  ;; "kmod static-nodes --format=tmpfiles" output format:
+  ;;   c! /dev/fuse 0600 - - - 10:229
+  ;;   d /dev/vfio 0755 - - -
+  (let ((port (open-pipe* OPEN_READ
+                          kmod-executable-name
+                          "static-nodes"
+                          "--format=tmpfiles"
+                          "--output=/proc/self/fd/1")))
+    (dynamic-wind
+      (lambda ()
+        #t)
+      (lambda ()
+        (let loop ((line (read-line port)))
+          (if (not (eof-object? line))
+            (let ((fields (string-split line #\space)))
+              (match fields
+               (("d" name mode-string "-" "-" "-")
+                (let ((mode (string->number mode-string 8)))
+                  (log-static-device-system-error name
+                    (lambda ()
+                      (mkdir name mode)))))
+               (("c!" name mode-string "-" "-" "-" device-number-string)
+                (log-static-device-system-error name
+                  (lambda ()
+                    (tmpfiles-mknod name 'char-special mode-string
+                                    device-number-string))))
+               (("b!" name mode-string "-" "-" "-" device-number-string)
+                (log-static-device-system-error name
+                  (lambda ()
+                    (tmpfiles-mknod name 'block-special mode-string
+                                    device-number-string))))
+               (_ #f))
+              (loop (read-line port))))))
+      (lambda ()
+        (close-pipe port)))))
+
 (define* (make-essential-device-nodes #:key (root "/"))
   "Make essential device nodes under ROOT/dev."
   ;; The hand-made devtmpfs/udev!

bug#22050: [PATCH v2 2/2] services: base: Use make-static-device-nodes.

[Danny Milosavljevic]

* gnu/services/base.scm (udev-shepherd-service): Use make-static-device-nodes.
---
 gnu/services/base.scm | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/gnu/services/base.scm b/gnu/services/base.scm
index 291dd6325..e1098b6d8 100644
--- a/gnu/services/base.scm
+++ b/gnu/services/base.scm
@@ -1727,6 +1727,8 @@ item of @var{packages}."
                     (setenv "EUDEV_RULES_DIRECTORY"
                             #$(file-append rules "/lib/udev/rules.d"))
 
+                    (make-static-device-nodes (string-append #$kmod "/bin/kmod"))
+
                     (let ((pid (primitive-fork)))
                       (case pid
                         ((0)
@@ -1750,7 +1752,10 @@ item of @var{packages}."
          ;; When halting the system, 'udev' is actually killed by
          ;; 'user-processes', i.e., before its own 'stop' method was called.
          ;; Thus, make sure it is not respawned.
-         (respawn? #f)))))))
+         (respawn? #f)
+         ;; We need additional modules.
+         (modules `((gnu build linux-boot)
+                    ,@%default-modules))))))))
 
 (define udev-service-type
   (service-type (name 'udev)

bug#22050: [PATCH v3 0/2] Create static device nodes before starting udev.

[Danny Milosavljevic]

udev expects the device names of some special devices to already exist
when udev is started.

So ask kmod which they are and create them.

Danny Milosavljevic (2):
  linux-boot: Add make-static-device-nodes.
  services: base: Use make-static-device-nodes.

 gnu/build/linux-boot.scm | 62 ++++++++++++++++++++++++++++++++++++++++++++++++
 gnu/services/base.scm    |  7 +++++-
 2 files changed, 68 insertions(+), 1 deletion(-)

bug#22050: [PATCH v3 1/2] linux-boot: Add make-static-device-nodes.

[Danny Milosavljevic]

* gnu/build/linux-boot.scm (make-static-device-nodes): New variable.
---
 gnu/build/linux-boot.scm | 63 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 63 insertions(+)

diff --git a/gnu/build/linux-boot.scm b/gnu/build/linux-boot.scm
index 2547f1e0a..f6eea96e3 100644
--- a/gnu/build/linux-boot.scm
+++ b/gnu/build/linux-boot.scm
@@ -24,6 +24,8 @@
   #:use-module (srfi srfi-1)
   #:use-module (srfi srfi-26)
   #:use-module (ice-9 match)
+  #:use-module (ice-9 rdelim)
+  #:use-module (ice-9 popen)
   #:use-module (ice-9 ftw)
   #:use-module (guix build utils)
   #:use-module ((guix build syscalls)
@@ -35,6 +37,7 @@
             linux-command-line
             find-long-option
             make-essential-device-nodes
+            make-static-device-nodes
             configure-qemu-networking
 
             bind-mount
@@ -105,6 +108,66 @@ with the given MAJOR number, starting with MINOR."
              'block-special #o644 (device-number major (+ minor i)))
       (loop (+ i 1)))))
 
+(define (tmpfiles-mknod name type mode-string device-number-string)
+  "Given a NAME, TYPE, MODE-STRING, DEVICE-NUMBER-STRING,
+   call mknod with the respective numbers."
+  (let* ((mode (string->number mode-string 8))
+         (device-number-parts (string-split device-number-string #\:)))
+    (match device-number-parts
+     ((major-device-number-string minor-device-number-string)
+      (let ((major-device-number (string->number major-device-number-string))
+            (minor-device-number (string->number minor-device-number-string)))
+        (mknod name type #o660 (device-number major-device-number
+                                              minor-device-number))))
+     (_ #f))))
+
+(define (log-static-device-system-error name callback)
+  "Call CALLBACK.  If it fails, print an error message."
+  (catch 'system-error
+    (lambda ()
+      (callback))
+    (lambda args
+      (format #t "could not create node '~a'~%" name))))
+
+(define* (make-static-device-nodes kmod-executable-name)
+  "Invoke and handle 'kmod static-nodes' output."
+  ;; "kmod static-nodes --format=tmpfiles" output format:
+  ;;   c! /dev/fuse 0600 - - - 10:229
+  ;;   d /dev/vfio 0755 - - -
+  (let ((port (open-pipe* OPEN_READ
+                          kmod-executable-name
+                          "static-nodes"
+                          "--format=tmpfiles"
+                          "--output=/proc/self/fd/1")))
+    (dynamic-wind
+      (lambda ()
+        #t)
+      (lambda ()
+        (let loop ((line (read-line port)))
+          (if (not (eof-object? line))
+            (let ((fields (string-split line #\space)))
+              (match fields
+               (("d" name mode-string "-" "-" "-")
+                (let ((mode (string->number mode-string 8)))
+                  (log-static-device-system-error name
+                    (lambda ()
+                      (unless (file-exists? name)
+                        (mkdir name mode))))))
+               (("c!" name mode-string "-" "-" "-" device-number-string)
+                (log-static-device-system-error name
+                  (lambda ()
+                    (tmpfiles-mknod name 'char-special mode-string
+                                    device-number-string))))
+               (("b!" name mode-string "-" "-" "-" device-number-string)
+                (log-static-device-system-error name
+                  (lambda ()
+                    (tmpfiles-mknod name 'block-special mode-string
+                                    device-number-string))))
+               (_ #f))
+              (loop (read-line port))))))
+      (lambda ()
+        (close-pipe port)))))
+
 (define* (make-essential-device-nodes #:key (root "/"))
   "Make essential device nodes under ROOT/dev."
   ;; The hand-made devtmpfs/udev!

bug#22050: [PATCH v3 2/2] services: base: Use make-static-device-nodes.

[Danny Milosavljevic]

* gnu/services/base.scm (udev-shepherd-service): Use make-static-device-nodes.
---
 gnu/services/base.scm | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/gnu/services/base.scm b/gnu/services/base.scm
index 291dd6325..e1098b6d8 100644
--- a/gnu/services/base.scm
+++ b/gnu/services/base.scm
@@ -1727,6 +1727,8 @@ item of @var{packages}."
                     (setenv "EUDEV_RULES_DIRECTORY"
                             #$(file-append rules "/lib/udev/rules.d"))
 
+                    (make-static-device-nodes (string-append #$kmod "/bin/kmod"))
+
                     (let ((pid (primitive-fork)))
                       (case pid
                         ((0)
@@ -1750,7 +1752,10 @@ item of @var{packages}."
          ;; When halting the system, 'udev' is actually killed by
          ;; 'user-processes', i.e., before its own 'stop' method was called.
          ;; Thus, make sure it is not respawned.
-         (respawn? #f)))))))
+         (respawn? #f)
+         ;; We need additional modules.
+         (modules `((gnu build linux-boot)
+                    ,@%default-modules))))))))
 
 (define udev-service-type
   (service-type (name 'udev)

bug#22050: [PATCH v3 1/2] linux-boot: Add make-static-device-nodes.

[Ludovic Courtès]

Hello!

AIUI this will solve lack of /dev/fuse at startup (among other things),
right?  I always wondered why it wasn’t showing up automatically.

Danny Milosavljevic <dannym@scratchpost.org> skribis:

> * gnu/build/linux-boot.scm (make-static-device-nodes): New variable.
> ---
>  gnu/build/linux-boot.scm | 63 ++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 63 insertions(+)
>
> diff --git a/gnu/build/linux-boot.scm b/gnu/build/linux-boot.scm
> index 2547f1e0a..f6eea96e3 100644
> --- a/gnu/build/linux-boot.scm
> +++ b/gnu/build/linux-boot.scm
> @@ -24,6 +24,8 @@
>    #:use-module (srfi srfi-1)
>    #:use-module (srfi srfi-26)
>    #:use-module (ice-9 match)
> +  #:use-module (ice-9 rdelim)
> +  #:use-module (ice-9 popen)
>    #:use-module (ice-9 ftw)
>    #:use-module (guix build utils)
>    #:use-module ((guix build syscalls)
> @@ -35,6 +37,7 @@
>              linux-command-line
>              find-long-option
>              make-essential-device-nodes
> +            make-static-device-nodes
>              configure-qemu-networking
>  
>              bind-mount
> @@ -105,6 +108,66 @@ with the given MAJOR number, starting with MINOR."
>               'block-special #o644 (device-number major (+ minor i)))
>        (loop (+ i 1)))))
>  
> +(define (tmpfiles-mknod name type mode-string device-number-string)
> +  "Given a NAME, TYPE, MODE-STRING, DEVICE-NUMBER-STRING,
> +   call mknod with the respective numbers."
> +  (let* ((mode (string->number mode-string 8))
> +         (device-number-parts (string-split device-number-string #\:)))
> +    (match device-number-parts
> +     ((major-device-number-string minor-device-number-string)
> +      (let ((major-device-number (string->number major-device-number-string))
> +            (minor-device-number (string->number minor-device-number-string)))
> +        (mknod name type #o660 (device-number major-device-number
> +                                              minor-device-number))))
> +     (_ #f))))

That’s a surprising name ;-), and I would suggest separating parsing
from node creation (see below).

> +(define (log-static-device-system-error name callback)
> +  "Call CALLBACK.  If it fails, print an error message."
> +  (catch 'system-error
> +    (lambda ()
> +      (callback))
> +    (lambda args
> +      (format #t "could not create node '~a'~%" name))))

Rather:

  (define (report-system-error . args)
    (let ((errno (system-error-errno args)))
      (format (current-error-port) "could not create…: ~a~%" (strerror errno))))

  (define-syntax-rule (catch-system-error exp)
    (catch 'system-error
      (lambda ()
        exp)
      report-system-error))

(The term ‘callback’ is never used in Scheme; we just write ‘proc’ or
‘thunk’.)

> +(define* (make-static-device-nodes kmod-executable-name)
> +  "Invoke and handle 'kmod static-nodes' output."
> +  ;; "kmod static-nodes --format=tmpfiles" output format:
> +  ;;   c! /dev/fuse 0600 - - - 10:229
> +  ;;   d /dev/vfio 0755 - - -

I checked what kmod does and in fact it just reads
$LINUX_MODULE_DIRECTORY/*/modules.devname, which has a format similar to
what you’re parsing here:

--8<---------------cut here---------------start------------->8---
$ cat $LINUX_MODULE_DIRECTORY/*/modules.devname
# Device nodes to trigger on-demand module loading.
autofs4 autofs c10:235
fuse fuse c10:229
cuse cuse c10:203
btrfs btrfs-control c10:234
userio userio c10:240
vfio vfio/vfio c10:196
hci_vhci vhci c10:137
uhid uhid c10:239
vhost_net vhost-net c10:238
vhost_vsock vhost-vsock c10:241
snd_timer snd/timer c116:33
snd_seq snd/seq c116:1
--8<---------------cut here---------------end--------------->8---

Could we read that directly instead of invoking ‘kmod’?

What about having a ‘static-device-nodes’ procedure that would parse
that and return a list of <device-node>, where:

  ;; TYPE is 'char or 'block, MAJOR and MINOR are integers.
  (define-record-type <device-node>
    (device-node name type major minor module)
    device-node?
    …)

and then:

  (define create-device-node
    (match-lambda
      (($ <device-node> name type major minor)
       (mknod …))))

finally:

  (for-each create-device-node (static-device-nodes))

?

Thanks for fixing this!

Ludo’.

bug#22050: [PATCH v3 1/2] linux-boot: Add make-static-device-nodes.

[Danny Milosavljevic]

Hi Ludo,

On Thu, 14 Dec 2017 09:52:07 +0100
ludo@gnu.org (Ludovic Courtès) wrote:

> Hello!
> 
> AIUI this will solve lack of /dev/fuse at startup (among other things),
> right?  I always wondered why it wasn’t showing up automatically.

Yep.

(The reason it works in other distros is because there's an executable "systemd-tmpfiles" which does what I do in this patch, before udevd is started)

> Rather:
> 
>   (define (report-system-error . args)
>     (let ((errno (system-error-errno args)))
>       (format (current-error-port) "could not create…: ~a~%" (strerror errno))))
> 
>   (define-syntax-rule (catch-system-error exp)
>     (catch 'system-error
>       (lambda ()
>         exp)
>       report-system-error))

I'd like this to include the file name in the error message.  I rather hate UNIXoid error messages like "mknod: Permission denied" where it doesn't say which arguments mknod had :P

> (The term ‘callback’ is never used in Scheme; we just write ‘proc’ or
> ‘thunk’.)

Oh okay.

> Could we read that directly instead of invoking ‘kmod’?

Probably, but it would mean that we'd duplicate kmod and take up maintenance of something extraneous.  We could do it - but what does it buy us?

> What about having a ‘static-device-nodes’ procedure that would parse
> that and return a list of <device-node>, where:
> 
>   ;; TYPE is 'char or 'block, MAJOR and MINOR are integers.
>   (define-record-type <device-node>
>     (device-node name type major minor module)
>     device-node?
>     …)
> 
> and then:
> 
>   (define create-device-node
>     (match-lambda
>       (($ <device-node> name type major minor)
>        (mknod …))))
> 
> finally:
> 
>   (for-each create-device-node (static-device-nodes))
> 
> ?

The kmod format has entries not only for mknod but also for mkdir (it actually mkdirs the same directory twice sometimes which is why there was another version of this patch...).

bug#22050: [PATCH v3 1/2] linux-boot: Add make-static-device-nodes.

[Ludovic Courtès]

Heya Danny,

Danny Milosavljevic <dannym@scratchpost.org> skribis:

> On Thu, 14 Dec 2017 09:52:07 +0100
> ludo@gnu.org (Ludovic Courtès) wrote:
>
>> Hello!
>> 
>> AIUI this will solve lack of /dev/fuse at startup (among other things),
>> right?  I always wondered why it wasn’t showing up automatically.
>
> Yep.
>
> (The reason it works in other distros is because there's an executable "systemd-tmpfiles" which does what I do in this patch, before udevd is started)

OK.

>> Rather:
>> 
>>   (define (report-system-error . args)
>>     (let ((errno (system-error-errno args)))
>>       (format (current-error-port) "could not create…: ~a~%" (strerror errno))))
>> 
>>   (define-syntax-rule (catch-system-error exp)
>>     (catch 'system-error
>>       (lambda ()
>>         exp)
>>       report-system-error))
>
> I'd like this to include the file name in the error message.  I rather hate UNIXoid error messages like "mknod: Permission denied" where it doesn't say which arguments mknod had :P

Good point, and I agree!  In (guix ui) there’s a hack for that, but here
we could simply add a ‘file’ parameter to ‘report-system-error’ and
‘catch-system-error’ I suppose.

>> Could we read that directly instead of invoking ‘kmod’?
>
> Probably, but it would mean that we'd duplicate kmod and take up maintenance of something extraneous.  We could do it - but what does it buy us?

The code would be slightly simpler (no pipe, etc.), and the .devname
file might be more stable than the kmod output, being a kernel
interface.

WDYT?

>> What about having a ‘static-device-nodes’ procedure that would parse
>> that and return a list of <device-node>, where:
>> 
>>   ;; TYPE is 'char or 'block, MAJOR and MINOR are integers.
>>   (define-record-type <device-node>
>>     (device-node name type major minor module)
>>     device-node?
>>     …)
>> 
>> and then:
>> 
>>   (define create-device-node
>>     (match-lambda
>>       (($ <device-node> name type major minor)
>>        (mknod …))))
>> 
>> finally:
>> 
>>   (for-each create-device-node (static-device-nodes))
>> 
>> ?
>
> The kmod format has entries not only for mknod but also for mkdir (it actually mkdirs the same directory twice sometimes which is why there was another version of this patch...).

OK, I had overlooked that.  Hopefully the general approach remains
valid?

Thank you,
Ludo’.

bug#22050: [PATCH v3 1/2] linux-boot: Add make-static-device-nodes.

[Danny Milosavljevic]

Hi Ludo,

On Thu, 14 Dec 2017 14:14:09 +0100
ludo@gnu.org (Ludovic Courtès) wrote:

> The code would be slightly simpler (no pipe, etc.), and the .devname
> file might be more stable than the kmod output, being a kernel
> interface.
> 
> WDYT?

That's true.  The kernel interface definitely has Linus breathe down the neck of anyone who even thinks of changing it :)

> >> What about having a ‘static-device-nodes’ procedure that would parse
> >> that and return a list of <device-node>, where:
> >> 
> >>   ;; TYPE is 'char or 'block, MAJOR and MINOR are integers.
> >>   (define-record-type <device-node>
> >>     (device-node name type major minor module)
> >>     device-node?
> >>     …)
> >> 
> >> and then:
> >> 
> >>   (define create-device-node
> >>     (match-lambda
> >>       (($ <device-node> name type major minor)
> >>        (mknod …))))
> >> 
> >> finally:
> >> 
> >>   (for-each create-device-node (static-device-nodes))
> >> 
> >> ?  
> >
> > The kmod format has entries not only for mknod but also for mkdir (it actually mkdirs the same directory twice sometimes which is why there was another version of this patch...).  
> 
> OK, I had overlooked that.  Hopefully the general approach remains
> valid?

Yeah.  Apparently kmod autogenerates those when there's a slash in the name it got from the devname file.  That's why it generates two mkdir entries for "snd" - there's a "snd/timer" and a "snd/seq" in the devname file...

bug#22050: [PATCH v4 0/2] Create static device nodes before starting udev.

[Danny Milosavljevic]

udev expects the device names of some special devices to already exist
when udev is started.

So ask kmod which they are and create them.

Danny Milosavljevic (2):
  linux-boot: Add make-static-device-nodes.
  services: base: Use make-static-device-nodes.

 gnu/build/linux-boot.scm | 62 ++++++++++++++++++++++++++++++++++++++++++++++++
 gnu/services/base.scm    |  7 +++++-
 2 files changed, 68 insertions(+), 1 deletion(-)

bug#22050: [PATCH v4 1/2] linux-boot: Add make-static-device-nodes.

[Danny Milosavljevic]

* gnu/build/linux-boot.scm (make-static-device-nodes): New variable.
<device-node>: New variable.
parse-static-nodes-from-devname-file: New variable.
not-slash: New variable.
report-system-error: New variable.
catch-system-error: New variable.
create-device-node: New variable.

Co-Authored-By: Ludovic Courtès <ludo@gnu.org>    
---
 gnu/build/linux-boot.scm | 81 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 81 insertions(+)

diff --git a/gnu/build/linux-boot.scm b/gnu/build/linux-boot.scm
index 2547f1e0a..c83bbcda4 100644
--- a/gnu/build/linux-boot.scm
+++ b/gnu/build/linux-boot.scm
@@ -22,8 +22,12 @@
   #:use-module (system repl error-handling)
   #:autoload   (system repl repl) (start-repl)
   #:use-module (srfi srfi-1)
+  #:use-module (srfi srfi-9)
   #:use-module (srfi srfi-26)
   #:use-module (ice-9 match)
+  #:use-module (ice-9 rdelim)
+  #:use-module (ice-9 regex)
+  #:use-module (ice-9 popen)
   #:use-module (ice-9 ftw)
   #:use-module (guix build utils)
   #:use-module ((guix build syscalls)
@@ -35,6 +39,7 @@
             linux-command-line
             find-long-option
             make-essential-device-nodes
+            make-static-device-nodes
             configure-qemu-networking
 
             bind-mount
@@ -105,6 +110,82 @@ with the given MAJOR number, starting with MINOR."
              'block-special #o644 (device-number major (+ minor i)))
       (loop (+ i 1)))))
 
+(define-record-type <device-node>
+  (device-node name type major minor module)
+  device-node?
+  (name device-node-name)
+  (type device-node-type)
+  (major device-node-major)
+  (minor device-node-minor)
+  (module device-node-module))
+
+(define (parse-static-nodes-from-devname-file devname-name)
+  (call-with-input-file devname-name
+    (lambda (input-file)
+      (let loop ((line (read-line input-file)))
+        (if (eof-object? line)
+          '()
+          (match (string-split line #\space)
+           (("#" _ ...)
+            (loop (read-line input-file)))
+           ((module-name device-name device-spec)
+            (let* ((device-parts (string-match "(.)([0-9][0-9]*):([0-9][0-9]*)" device-spec))
+                   (type-string (match:substring device-parts 1))
+                   (type (match type-string
+                          ("c" 'char-special)
+                          ("b" 'block-special)))
+                   (major-string (match:substring device-parts 2))
+                   (major (string->number major-string 10))
+                   (minor-string (match:substring device-parts 3))
+                   (minor (string->number minor-string 10)))
+              (cons (device-node device-name type major minor module-name)
+                    (loop (read-line input-file)))))
+           (_
+            (begin
+              (format (current-error-port) "~a: ignored devname line '~a'~%"
+                      devname-name line)
+              (loop (read-line input-file))))))))))
+
+(define not-slash
+  (char-set-complement (char-set #\/)))
+
+(define (report-system-error name . args)
+  (let ((errno (system-error-errno args)))
+        (format (current-error-port) "could not create '~a': ~a~%" name
+                (strerror errno))))
+
+(define-syntax-rule (catch-system-error name exp)
+  (catch 'system-error
+    (lambda ()
+      exp)
+    (lambda args
+      (apply report-system-error name args))))
+
+(define create-device-node
+  (match-lambda
+    (($ <device-node> name type major minor module)
+     (let ((name-parts (string-tokenize name not-slash)))
+       (let loop ((prefix "/dev")
+                  (name-parts name-parts))
+         (match name-parts
+          ((leaf)
+           (let ((prefix (string-append prefix "/" leaf)))
+             (catch-system-error prefix
+               (mknod prefix type #o600 (device-number major minor)))))
+          ((prefix-addition tails ...)
+           (let ((prefix (string-append prefix "/" prefix-addition)))
+             (unless (file-exists? prefix)
+               (mkdir prefix #o755))
+             (loop prefix tails)))))))))
+
+(define* (make-static-device-nodes linux-module-directory)
+  (let* ((kernel-release (utsname:release (uname)))
+         (devname-name (string-append linux-module-directory "/"
+                                      kernel-release "/"
+                                      "modules.devname")))
+       (for-each create-device-node
+                 (parse-static-nodes-from-devname-file devname-name))))
+
 (define* (make-essential-device-nodes #:key (root "/"))
   "Make essential device nodes under ROOT/dev."
   ;; The hand-made devtmpfs/udev!

bug#22050: [PATCH v4 2/2] services: base: Use make-static-device-nodes.

[Danny Milosavljevic]

* gnu/services/base.scm (udev-shepherd-service): Use make-static-device-nodes.
---
 gnu/services/base.scm | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/gnu/services/base.scm b/gnu/services/base.scm
index 291dd6325..d3aa3392c 100644
--- a/gnu/services/base.scm
+++ b/gnu/services/base.scm
@@ -1727,6 +1727,8 @@ item of @var{packages}."
                     (setenv "EUDEV_RULES_DIRECTORY"
                             #$(file-append rules "/lib/udev/rules.d"))
 
+                    (make-static-device-nodes (getenv "LINUX_MODULE_DIRECTORY"))
+
                     (let ((pid (primitive-fork)))
                       (case pid
                         ((0)
@@ -1750,7 +1752,10 @@ item of @var{packages}."
          ;; When halting the system, 'udev' is actually killed by
          ;; 'user-processes', i.e., before its own 'stop' method was called.
          ;; Thus, make sure it is not respawned.
-         (respawn? #f)))))))
+         (respawn? #f)
+         ;; We need additional modules.
+         (modules `((gnu build linux-boot)
+                    ,@%default-modules))))))))
 
 (define udev-service-type
   (service-type (name 'udev)

bug#22050: [PATCH v5 0/2] Create static device nodes before starting udev.

[Danny Milosavljevic]

udev expects the device names of some special devices to already exist
when udev is started.

So ask kmod which they are and create them.

Danny Milosavljevic (2):
  linux-boot: Add make-static-device-nodes.
  services: base: Use make-static-device-nodes.

 gnu/build/linux-boot.scm | 62 ++++++++++++++++++++++++++++++++++++++++++++++++
 gnu/services/base.scm    |  7 +++++-
 2 files changed, 68 insertions(+), 1 deletion(-)

bug#22050: [PATCH v5 1/2] linux-boot: Add make-static-device-nodes.

[Danny Milosavljevic]

* gnu/build/linux-boot.scm (make-static-device-nodes): New variable.
<device-node>: New variable.
parse-static-nodes-from-devname-file: New variable.
not-slash: New variable.
report-system-error: New variable.
catch-system-error: New variable.
create-device-node: New variable.

Co-Authored-By: Ludovic Courtès <ludo@gnu.org>
---
 gnu/build/linux-boot.scm | 81 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 81 insertions(+)

diff --git a/gnu/build/linux-boot.scm b/gnu/build/linux-boot.scm
index 2547f1e0a..d9ced187c 100644
--- a/gnu/build/linux-boot.scm
+++ b/gnu/build/linux-boot.scm
@@ -22,8 +22,12 @@
   #:use-module (system repl error-handling)
   #:autoload   (system repl repl) (start-repl)
   #:use-module (srfi srfi-1)
+  #:use-module (srfi srfi-9)
   #:use-module (srfi srfi-26)
   #:use-module (ice-9 match)
+  #:use-module (ice-9 rdelim)
+  #:use-module (ice-9 regex)
+  #:use-module (ice-9 popen)
   #:use-module (ice-9 ftw)
   #:use-module (guix build utils)
   #:use-module ((guix build syscalls)
@@ -35,6 +39,7 @@
             linux-command-line
             find-long-option
             make-essential-device-nodes
+            make-static-device-nodes
             configure-qemu-networking
 
             bind-mount
@@ -105,6 +110,82 @@ with the given MAJOR number, starting with MINOR."
              'block-special #o644 (device-number major (+ minor i)))
       (loop (+ i 1)))))
 
+(define-record-type <device-node>
+  (device-node name type major minor module)
+  device-node?
+  (name device-node-name)
+  (type device-node-type)
+  (major device-node-major)
+  (minor device-node-minor)
+  (module device-node-module))
+
+(define (parse-static-nodes-from-devname-file devname-name)
+  (call-with-input-file devname-name
+    (lambda (input-file)
+      (let loop ((line (read-line input-file)))
+        (if (eof-object? line)
+          '()
+          (match (string-split line #\space)
+           (("#" _ ...)
+            (loop (read-line input-file)))
+           ((module-name device-name device-spec)
+            (let* ((device-parts
+                   (string-match "([bc])([0-9][0-9]*):([0-9][0-9]*)"
+                                 device-spec))
+                   (type-string (match:substring device-parts 1))
+                   (type (match type-string
+                          ("c" 'char-special)
+                          ("b" 'block-special)))
+                   (major-string (match:substring device-parts 2))
+                   (major (string->number major-string 10))
+                   (minor-string (match:substring device-parts 3))
+                   (minor (string->number minor-string 10)))
+              (cons (device-node device-name type major minor module-name)
+                    (loop (read-line input-file)))))
+           (_
+            (begin
+              (format (current-error-port) "~a: ignored devname line '~a'~%"
+                      devname-name line)
+              (loop (read-line input-file))))))))))
+
+(define not-slash
+  (char-set-complement (char-set #\/)))
+
+(define (report-system-error name . args)
+  (let ((errno (system-error-errno args)))
+        (format (current-error-port) "could not create '~a': ~a~%" name
+                (strerror errno))))
+
+(define-syntax-rule (catch-system-error name exp)
+  (catch 'system-error
+    (lambda ()
+      exp)
+    (lambda args
+      (apply report-system-error name args))))
+
+(define create-device-node
+  (match-lambda
+    (($ <device-node> name type major minor module)
+     (let ((name-parts (string-tokenize name not-slash)))
+       (let loop ((prefix "/dev")
+                  (name-parts name-parts))
+         (match name-parts
+          ((leaf)
+           (let ((prefix (string-append prefix "/" leaf)))
+             (catch-system-error prefix
+               (mknod prefix type #o600 (device-number major minor)))))
+          ((prefix-addition tails ...)
+           (let ((prefix (string-append prefix "/" prefix-addition)))
+             (unless (file-exists? prefix)
+               (mkdir prefix #o755))
+             (loop prefix tails)))))))))
+
+(define* (make-static-device-nodes linux-release-module-directory)
+  (let ((devname-name (string-append linux-release-module-directory "/"
+                                     "modules.devname")))
+    (for-each create-device-node
+              (parse-static-nodes-from-devname-file devname-name))))
+
 (define* (make-essential-device-nodes #:key (root "/"))
   "Make essential device nodes under ROOT/dev."
   ;; The hand-made devtmpfs/udev!

bug#22050: [PATCH v5 2/2] services: base: Use make-static-device-nodes.

[Danny Milosavljevic]

* gnu/services/base.scm (udev-shepherd-service): Use make-static-device-nodes.
---
 gnu/services/base.scm | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/gnu/services/base.scm b/gnu/services/base.scm
index 291dd6325..34699cdcd 100644
--- a/gnu/services/base.scm
+++ b/gnu/services/base.scm
@@ -1727,6 +1727,14 @@ item of @var{packages}."
                     (setenv "EUDEV_RULES_DIRECTORY"
                             #$(file-append rules "/lib/udev/rules.d"))
 
+                    (let* ((kernel-release
+                           (utsname:release (uname)))
+                           (linux-module-directory
+                            (getenv "LINUX_MODULE_DIRECTORY"))
+                           (linux-release-module-directory
+                            (string-append linux-module-directory "/"
+                                           kernel-release)))
+                      (make-static-device-nodes linux-release-module-directory))
                     (let ((pid (primitive-fork)))
                       (case pid
                         ((0)
@@ -1750,7 +1758,10 @@ item of @var{packages}."
          ;; When halting the system, 'udev' is actually killed by
          ;; 'user-processes', i.e., before its own 'stop' method was called.
          ;; Thus, make sure it is not respawned.
-         (respawn? #f)))))))
+         (respawn? #f)
+         ;; We need additional modules.
+         (modules `((gnu build linux-boot)
+                    ,@%default-modules))))))))
 
 (define udev-service-type
   (service-type (name 'udev)

bug#22050: [PATCH v4 1/2] linux-boot: Add make-static-device-nodes.

[Ludovic Courtès]

Hi Danny,

Danny Milosavljevic <dannym@scratchpost.org> skribis:

> * gnu/build/linux-boot.scm (make-static-device-nodes): New variable.
> <device-node>: New variable.
> parse-static-nodes-from-devname-file: New variable.
> not-slash: New variable.
> report-system-error: New variable.
> catch-system-error: New variable.
> create-device-node: New variable.

Nitpick: please adjust the syntax.

> +(define-record-type <device-node>
> +  (device-node name type major minor module)

Please add a one-line comment above like:

  ;; Representation of a /dev node.

> +(define (parse-static-nodes-from-devname-file devname-name)
> +  (call-with-input-file devname-name
> +    (lambda (input-file)

It would be more idiomatic to take an input port, rather than a file
name.  Also I’d suggest ‘read-static-device-nodes’, with a docstring:

  (define (read-static-device-nodes port)
    "Read from PORT a list of <device-node> written in the format used
  by /lib/modules/*/*.devname files."
    …)

> +      (let loop ((line (read-line input-file)))
> +        (if (eof-object? line)
> +          '()
> +          (match (string-split line #\space)
> +           (("#" _ ...)
> +            (loop (read-line input-file)))

To make sure all comments are handled, change this clause to:

  (((? (cut string-prefix? "#" <>)) _ ...)
   (loop (read-line input-line)))

> +(define (report-system-error name . args)
> +  (let ((errno (system-error-errno args)))
> +        (format (current-error-port) "could not create '~a': ~a~%" name
> +                (strerror errno))))

Align “(format” with the ‘e’ of ‘let’.  :-)

> +(define create-device-node

Please add a comment saying what it does.

> +  (match-lambda
> +    (($ <device-node> name type major minor module)
> +     (let ((name-parts (string-tokenize name not-slash)))
> +       (let loop ((prefix "/dev")
> +                  (name-parts name-parts))
> +         (match name-parts
> +          ((leaf)
> +           (let ((prefix (string-append prefix "/" leaf)))
> +             (catch-system-error prefix
> +               (mknod prefix type #o600 (device-number major minor)))))
> +          ((prefix-addition tails ...)
> +           (let ((prefix (string-append prefix "/" prefix-addition)))
> +             (unless (file-exists? prefix)
> +               (mkdir prefix #o755))
> +             (loop prefix tails)))))))))

This looks good, but would it be enough to do:

  (mkdir-p (dirname (string-append "/dev/" name)))

?

> +(define* (make-static-device-nodes linux-module-directory)

Docstring please.  :-)  IMO it’s important also to mention why those
nodes need to be created by hand.

Thank you!

Ludo’.

bug#22050: [PATCH v5 2/2] services: base: Use make-static-device-nodes.

[Ludovic Courtès]

Danny Milosavljevic <dannym@scratchpost.org> skribis:

> * gnu/services/base.scm (udev-shepherd-service): Use make-static-device-nodes.
> ---
>  gnu/services/base.scm | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/gnu/services/base.scm b/gnu/services/base.scm
> index 291dd6325..34699cdcd 100644
> --- a/gnu/services/base.scm
> +++ b/gnu/services/base.scm
> @@ -1727,6 +1727,14 @@ item of @var{packages}."
>                      (setenv "EUDEV_RULES_DIRECTORY"
>                              #$(file-append rules "/lib/udev/rules.d"))
>  
> +                    (let* ((kernel-release
> +                           (utsname:release (uname)))
                              ^
Indentation.

> +                           (linux-module-directory
> +                            (getenv "LINUX_MODULE_DIRECTORY"))
> +                           (linux-release-module-directory
> +                            (string-append linux-module-directory "/"
> +                                           kernel-release)))
> +                      (make-static-device-nodes linux-release-module-directory))

Nitpick: I think ‘directory’ would be good enough as an identifier to
this local variable.

Otherwise LGTM!

bug#22050: [PATCH v4 1/2] linux-boot: Add make-static-device-nodes.

[Danny Milosavljevic]

Hi Ludo,

On Fri, 15 Dec 2017 10:41:37 +0100
ludo@gnu.org (Ludovic Courtès) wrote:

> This looks good, but would it be enough to do:
> 
>   (mkdir-p (dirname (string-append "/dev/" name)))

mkdir permission needs to be specified to be #o755.

I've seen some variants in guix - but they are all buried somewhere deep:

./gnu/services/cups.scm:        (define (mkdir-p/perms directory owner perms)
./gnu/services/dns.scm:      (define (mkdir-p/perms directory owner perms)
./gnu/services/mail.scm:      (define (mkdir-p/perms directory owner perms)
./guix/build/utils.scm:(define (mkdir-p dir)
./guix/build/graft.scm:(define* (mkdir-p* dir #:optional (mode #o755))

I don't feel good relying on magical unspecified permissions in code required to boot the system.  I think mkdir-p itself subtracts the umask or something.  Is the umask guaranteed to be 0 at the point of usage of make-static-device-nodes ?

bug#22050: [PATCH v4 1/2] linux-boot: Add make-static-device-nodes.

[Ludovic Courtès]

Danny Milosavljevic <dannym@scratchpost.org> skribis:

> Hi Ludo,
>
> On Fri, 15 Dec 2017 10:41:37 +0100
> ludo@gnu.org (Ludovic Courtès) wrote:
>
>> This looks good, but would it be enough to do:
>> 
>>   (mkdir-p (dirname (string-append "/dev/" name)))
>
> mkdir permission needs to be specified to be #o755.
>
> I've seen some variants in guix - but they are all buried somewhere deep:
>
> ./gnu/services/cups.scm:        (define (mkdir-p/perms directory owner perms)
> ./gnu/services/dns.scm:      (define (mkdir-p/perms directory owner perms)
> ./gnu/services/mail.scm:      (define (mkdir-p/perms directory owner perms)
> ./guix/build/utils.scm:(define (mkdir-p dir)
> ./guix/build/graft.scm:(define* (mkdir-p* dir #:optional (mode #o755))

Oh, I didn’t think we had this many variants.

> I don't feel good relying on magical unspecified permissions in code required to boot the system.  I think mkdir-p itself subtracts the umask or something.  Is the umask guaranteed to be 0 at the point of usage of make-static-device-nodes ?

I’m not 100% sure.  You could set ‘umask’ explicitly given that it’s
single-threaded and all at this point, but it’s probably clearer to
explicitly specify the mode like you did initially.

So I guess you can forget my comment and apply the original variant (or
perhaps explicitly copy ‘mkdir-p/perms’ so we can find it more easily
when we factorize!).

Thanks for explaining!

Ludo’.