bug#47681: Reloading udev rules requires a system restart

bug#47681: Reloading udev rules requires a system restart

[Maxim Cournoyer]

Hello Guix!

Using Guix System, after adding a new rule to the configuration of their
udev-service-type service, the only ways to get the new rule into effect
are to either:

1. restart udev

(which is almost the same as a reboot, bringing down your graphical session)

2. restart the operating system

Both of which are sub-optimal.

This is caused by the configuration file/rules being made known to udev
via environment variables:

$ sudo cat /proc/$(pgrep udev)/environ | xargs -0 -n1 echo
UDEV_CONFIG_FILE=/gnu/store/7yfpf8acjy884xbwaq5kn9z21irchfaj-udev.conf
EUDEV_RULES_DIRECTORY=/gnu/store/yv58b7rg7dm3191cj6sma896550wgy4v-udev-rules/lib/udev/rules.d
LINUX_MODULE_DIRECTORY=/run/booted-system/kernel/lib/modules
PATH=/run/current-system/profile/bin

For convenience, we should probably have the udev-service-type create a
union of what it needs under /etc/udev/ as on other distributions.  udev
could then rely on a fixed location to look things and use its inotify
based mechanism to detect changes and reload automatically when needed.

This could probably fix things such as 'udevadm test' only reading rule
files from under
/gnu/store/svplp9wl0g2ahlv5rf6bhmq3xvp4zzh3-eudev-3.2.9/lib/udev/rules.d,
for example.

Thank you,

Maxim

bug#47681: [PATCH] services: udev: Use a fixed location for the rules directory and config.

[Maxim Cournoyer]

Fixes <https://issues.guix.gnu.org/47681>.

This change adjusts the location of the udev configuration file and rules
directory to a fixed location.  Since udev relies on inotify to discover
change to its rules directory (/etc/udev/rules.d), by using a fixed directory
layout, new udev rules can be automatically picked up without restarting the
service.

* gnu/services/base.scm (udev-rules-union): Build rules output directly
in #$output.
(udev-shepherd-service)[start]: Adjust the UDEV_CONFIG_FILE and
EUDEV_RULES_DIRECTORY environment variables.
[actions]: Remove field.  The 'rules' action is no longer useful.
(udev.conf): New variable.
(udev-etc): New procedure.
(udev-service-type): Extend the etc-service-type with it.
---
 gnu/services/base.scm | 210 +++++++++++++++++++++---------------------
 1 file changed, 104 insertions(+), 106 deletions(-)

diff --git a/gnu/services/base.scm b/gnu/services/base.scm
index fbd01e84d6..4c8a840156 100644
--- a/gnu/services/base.scm
+++ b/gnu/services/base.scm
@@ -15,7 +15,7 @@
 ;;; Copyright © 2020, 2021 Brice Waegeneire <brice@waegenei.re>
 ;;; Copyright © 2021 qblade <qblade@protonmail.com>
 ;;; Copyright © 2021 Hui Lu <luhuins@163.com>
-;;; Copyright © 2021 Maxim Cournoyer <maxim.cournoyer@gmail.com>
+;;; Copyright © 2021, 2022 Maxim Cournoyer <maxim.cournoyer@gmail.com>
 ;;; Copyright © 2022 Guillaume Le Vaillant <glv@posteo.net>
 ;;;
 ;;; This file is part of GNU Guix.
@@ -1995,8 +1995,7 @@ (define (rules-sub-directory directory)
             (find directory-exists?
                   (map (cut string-append directory <>) %standard-locations)))
 
-          (mkdir-p (string-append #$output "/lib/udev"))
-          (union-build (string-append #$output "/lib/udev/rules.d")
+          (union-build #$output
                        (filter-map rules-sub-directory '#$packages)))))
 
   (computed-file "udev-rules" build))
@@ -2045,116 +2044,115 @@ (define kvm-udev-rule
 
 (define udev-shepherd-service
   ;; Return a <shepherd-service> for UDEV with RULES.
+  (match-lambda
+    (($ <udev-configuration> udev)
+     (list
+      (shepherd-service
+       (provision '(udev))
+
+       ;; Udev needs /dev to be a 'devtmpfs' mount so that new device nodes can
+       ;; be added: see
+       ;; <http://www.linuxfromscratch.org/lfs/view/development/chapter07/udev.html>.
+       (requirement '(root-file-system))
+
+       (documentation "Populate the /dev directory, dynamically.")
+       (start
+        (with-imported-modules (source-module-closure
+                                '((gnu build linux-boot)))
+          #~(lambda ()
+              (define udevd
+                ;; 'udevd' from eudev.
+                #$(file-append udev "/sbin/udevd"))
+
+              (define (wait-for-udevd)
+                ;; Wait until someone's listening on udevd's control
+                ;; socket.
+                (let ((sock (socket AF_UNIX SOCK_SEQPACKET 0)))
+                  (let try ()
+                    (catch 'system-error
+                      (lambda ()
+                        (connect sock PF_UNIX "/run/udev/control")
+                        (close-port sock))
+                      (lambda args
+                        (format #t "waiting for udevd...~%")
+                        (usleep 500000)
+                        (try))))))
+
+              ;; Allow udev to find the modules.
+              (setenv "LINUX_MODULE_DIRECTORY"
+                      "/run/booted-system/kernel/lib/modules")
+
+              (let* ((kernel-release
+                      (utsname:release (uname)))
+                     (linux-module-directory
+                      (getenv "LINUX_MODULE_DIRECTORY"))
+                     (directory
+                      (string-append linux-module-directory "/"
+                                     kernel-release))
+                     (old-umask (umask #o022)))
+                ;; If we're in a container, DIRECTORY might not exist,
+                ;; for instance because the host runs a different
+                ;; kernel.  In that case, skip it; we'll just miss a few
+                ;; nodes like /dev/fuse.
+                (when (file-exists? directory)
+                  (make-static-device-nodes directory))
+                (umask old-umask))
+
+              (let ((pid (fork+exec-command
+                          (list udevd)
+                          #:environment-variables
+                          (cons*
+                           ;; The first one is for udev, the second one for
+                           ;; eudev.
+                           "UDEV_CONFIG_FILE=/etc/udev/udev.conf"
+                           "EUDEV_RULES_DIRECTORY=/etc/udev/rules.d"
+                           (string-append "LINUX_MODULE_DIRECTORY="
+                                          (getenv "LINUX_MODULE_DIRECTORY"))
+                           (default-environment-variables)))))
+                ;; Wait until udevd is up and running.  This appears to
+                ;; be needed so that the events triggered below are
+                ;; actually handled.
+                (wait-for-udevd)
+
+                ;; Trigger device node creation.
+                (system* #$(file-append udev "/bin/udevadm")
+                         "trigger" "--action=add")
+
+                ;; Wait for things to settle down.
+                (system* #$(file-append udev "/bin/udevadm")
+                         "settle")
+                pid))))
+       (stop #~(make-kill-destructor))
+
+       ;; 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)
+       ;; We need additional modules.
+       (modules `((gnu build linux-boot)        ;'make-static-device-nodes'
+                  ,@%default-modules)))))))
+
+(define udev.conf
+  (computed-file "udev.conf"
+                 #~(call-with-output-file #$output
+                     (lambda (port)
+                       (format port "udev_rules=\"/etc/udev/rules.d\"~%")))))
+
+(define udev-etc
   (match-lambda
     (($ <udev-configuration> udev rules)
-     (let* ((rules     (udev-rules-union (cons* udev kvm-udev-rule rules)))
-            (udev.conf (computed-file "udev.conf"
-                                      #~(call-with-output-file #$output
-                                          (lambda (port)
-                                            (format port
-                                                    "udev_rules=\"~a/lib/udev/rules.d\"\n"
-                                                    #$rules))))))
-       (list
-        (shepherd-service
-         (provision '(udev))
-
-         ;; Udev needs /dev to be a 'devtmpfs' mount so that new device nodes can
-         ;; be added: see
-         ;; <http://www.linuxfromscratch.org/lfs/view/development/chapter07/udev.html>.
-         (requirement '(root-file-system))
-
-         (documentation "Populate the /dev directory, dynamically.")
-         (start
-          (with-imported-modules (source-module-closure
-                                  '((gnu build linux-boot)))
-            #~(lambda ()
-                (define udevd
-                  ;; 'udevd' from eudev.
-                  #$(file-append udev "/sbin/udevd"))
-
-                (define (wait-for-udevd)
-                  ;; Wait until someone's listening on udevd's control
-                  ;; socket.
-                  (let ((sock (socket AF_UNIX SOCK_SEQPACKET 0)))
-                    (let try ()
-                      (catch 'system-error
-                        (lambda ()
-                          (connect sock PF_UNIX "/run/udev/control")
-                          (close-port sock))
-                        (lambda args
-                          (format #t "waiting for udevd...~%")
-                          (usleep 500000)
-                          (try))))))
-
-                ;; Allow udev to find the modules.
-                (setenv "LINUX_MODULE_DIRECTORY"
-                        "/run/booted-system/kernel/lib/modules")
-
-                (let* ((kernel-release
-                        (utsname:release (uname)))
-                       (linux-module-directory
-                        (getenv "LINUX_MODULE_DIRECTORY"))
-                       (directory
-                        (string-append linux-module-directory "/"
-                                       kernel-release))
-                       (old-umask (umask #o022)))
-                  ;; If we're in a container, DIRECTORY might not exist,
-                  ;; for instance because the host runs a different
-                  ;; kernel.  In that case, skip it; we'll just miss a few
-                  ;; nodes like /dev/fuse.
-                  (when (file-exists? directory)
-                    (make-static-device-nodes directory))
-                  (umask old-umask))
-
-                (let ((pid (fork+exec-command (list udevd)
-                            #:environment-variables
-                            (cons*
-                             ;; The first one is for udev, the second one for
-                             ;; eudev.
-                             (string-append "UDEV_CONFIG_FILE=" #$udev.conf)
-                             (string-append "EUDEV_RULES_DIRECTORY="
-                                            #$(file-append
-                                               rules "/lib/udev/rules.d"))
-                             (string-append "LINUX_MODULE_DIRECTORY="
-                                            (getenv "LINUX_MODULE_DIRECTORY"))
-                             (default-environment-variables)))))
-                  ;; Wait until udevd is up and running.  This appears to
-                  ;; be needed so that the events triggered below are
-                  ;; actually handled.
-                  (wait-for-udevd)
-
-                  ;; Trigger device node creation.
-                  (system* #$(file-append udev "/bin/udevadm")
-                           "trigger" "--action=add")
-
-                  ;; Wait for things to settle down.
-                  (system* #$(file-append udev "/bin/udevadm")
-                           "settle")
-                  pid))))
-         (stop #~(make-kill-destructor))
-
-         ;; 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)
-         ;; We need additional modules.
-         (modules `((gnu build linux-boot)        ;'make-static-device-nodes'
-                    ,@%default-modules))
-
-         (actions (list (shepherd-action
-                         (name 'rules)
-                         (documentation "Display the directory containing
-the udev rules in use.")
-                         (procedure #~(lambda (_)
-                                        (display #$rules)
-                                        (newline))))))))))))
+     `(("udev"
+        ,(file-union
+          "udev" `(("udev.conf" ,udev.conf)
+                   ("rules.d" ,(udev-rules-union (cons* udev kvm-udev-rule
+                                                        rules))))))))))
 
 (define udev-service-type
   (service-type (name 'udev)
                 (extensions
                  (list (service-extension shepherd-root-service-type
-                                          udev-shepherd-service)))
-
+                                          udev-shepherd-service)
+                       (service-extension etc-service-type udev-etc)))
                 (compose concatenate)           ;concatenate the list of rules
                 (extend (lambda (config rules)
                           (match config
-- 
2.34.0

bug#47681: Reloading udev rules requires a system restart

[Maxim Cournoyer]

Hello,

Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:

> Fixes <https://issues.guix.gnu.org/47681>.
>
> This change adjusts the location of the udev configuration file and rules
> directory to a fixed location.  Since udev relies on inotify to discover
> change to its rules directory (/etc/udev/rules.d), by using a fixed directory
> layout, new udev rules can be automatically picked up without restarting the
> service.
>
> * gnu/services/base.scm (udev-rules-union): Build rules output directly
> in #$output.
> (udev-shepherd-service)[start]: Adjust the UDEV_CONFIG_FILE and
> EUDEV_RULES_DIRECTORY environment variables.
> [actions]: Remove field.  The 'rules' action is no longer useful.
> (udev.conf): New variable.
> (udev-etc): New procedure.
> (udev-service-type): Extend the etc-service-type with it.

Pushed with commit e9fa17eb98efbd6211ab44ab49b8c078d4b73e04.

Closing.

Maxim