[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[bug#68675] [PATCH v3 2/2] services: dhcp: Support the dhcpcd implementa
From: |
Ludovic Courtès |
Subject: |
[bug#68675] [PATCH v3 2/2] services: dhcp: Support the dhcpcd implementation. |
Date: |
Wed, 28 Feb 2024 21:53:51 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) |
Hello,
soeren@soeren-tempel.net skribis:
> * gnu/services/networking.scm (dhcp-client-shepherd-service): Add
> support for the dhcpcd client implementation.
> * gnu/services/networking.scm (dhcp-client-account-service): New
> procedure.
> * gnu/services/networking.scm (dhcp-client-service-type): Add optional
> account-service-type extensions (needed for dhcpcd).
> * gnu/system.scm (%base-packages-networking): Remove isc-dhcp from
> %base-packages (will be pulled in by dhcp-client-shepherd-service).
[...]
> + ;; Returns the execution configuration for the DHCP
> client
> + ;; selected by the package field of
> dhcp-client-configuration.
> + ;; The configuration is a pair of pidfile and
> execution command
> + ;; where the latter is a list.
> + (define exec-config
> + (case (string->symbol #$client-name)
> + ((isc-dhcp)
> + (let ((pid-file "/var/run/dhclient.pid"))
> + (cons
> + (cons* (string-append #$package
> "/sbin/dhclient")
> + "-nw" "-I" "-pf" pid-file ifaces)
> + pid-file)))
> + ((dhcpcd)
> + ;; For dhcpcd, the utilized pid-file depends
> on the
> + ;; command-line arguments. If multiple
> interfaces are
> + ;; given, a different pid-file is returned.
> Hence, we
> + ;; consult dhcpcd itself to determine the
> pid-file.
> + (let* ((cmd (string-append #$package
> "/sbin/dhcpcd"))
> + (arg (cons* cmd "-b" ifaces)))
> + (cons arg
> + (let* ((pipe (string-join (append arg
> '("-P")) " "))
> + (port (open-input-pipe pipe))
> + (path (read-line port)))
> + (close-pipe port)
> + path))))
> + (else
> + (display
> + "unknown 'package' value in
> dhcp-client-configuration"
> + (current-error-port))
> + (newline (current-error-port))
> + #f)))
> +
I would very much like to avoid the ‘open-input-pipe’ dance here. Maybe
there’s a way to ask it to not fork, in which case we don’t need to wait
for a PID file? (I’ll give up if this really really can’t be avoided. :-))
> + (and
> + exec-config
> + (let ((pid-file (cdr exec-config))
> + (exec-cmd (car exec-config)))
> + (false-if-exception (delete-file pid-file))
> + (let ((pid (fork+exec-command exec-cmd)))
> + (and (zero? (cdr (waitpid pid)))
> + (read-pid-file pid-file)))))))
Two notes: I would find it clearer to call ‘fork+exec-command’ above
instead of constructing ‘exec-config’.
Ideally, we’d use:
(start #~(if (file-exists? (string-append #$package "/bin/dhclient"))
(make-forkexec-constructor …) ;ISC version, with #:pid-file
(make-forkexec-constructor …))) ;dhcpcd, maybe without #:pid-file
Maybe that is too naive, but at least we should get as close as possible
to that, for instance by avoiding direct calls to ‘fork+exec-command’ +
‘waitpid’ + ‘read-pid-file’ as much as possible.
HTH!
Ludo’.