all messages for Guix-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: "Ludovic Courtès" <ludo@gnu.org>
To: soeren@soeren-tempel.net
Cc: 68675@debbugs.gnu.org
Subject: [bug#68675] [PATCH v3 2/2] services: dhcp: Support the dhcpcd implementation.
Date: Wed, 28 Feb 2024 21:53:51 +0100	[thread overview]
Message-ID: <87plwgfgyo.fsf@gnu.org> (raw)
In-Reply-To: <bfd99f266c3fd749c6b24b386a7ac90dc89ce338.1707828643.git.soeren@soeren-tempel.net> (soeren@soeren-tempel.net's message of "Tue, 13 Feb 2024 13:50:43 +0100")

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




  reply	other threads:[~2024-02-28 20:56 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-01-23 16:12 [bug#68675] [PATCH] Support dhcpcd in dhcp-client-service-type soeren
2024-01-23 16:14 ` [bug#68675] [PATCH] gnu: Add dhcpcd soeren
2024-01-23 16:14 ` [bug#68675] [PATCH] services: dhcp: Support the dhcpcd implementation soeren
2024-01-23 19:52 ` [bug#68675] [PATCH] Support dhcpcd in dhcp-client-service-type Sergey Trofimov
2024-01-24 19:11   ` Sören Tempel
2024-01-24 19:05 ` [bug#68675] [PATCH v2] gnu: Add dhcpcd soeren
2024-01-24 19:05   ` [bug#68675] [PATCH v2] services: dhcp: Support the dhcpcd implementation soeren
2024-02-12 21:41     ` Ludovic Courtès
2024-02-13 12:52       ` Sören Tempel
2024-02-28 20:46         ` Ludovic Courtès
2024-02-12 21:32   ` [bug#68675] [PATCH v2] gnu: Add dhcpcd Ludovic Courtès
2024-02-13 12:50 ` [bug#68675] [PATCH v3 1/2] " soeren
2024-02-13 12:50   ` [bug#68675] [PATCH v3 2/2] services: dhcp: Support the dhcpcd implementation soeren
2024-02-28 20:53     ` Ludovic Courtès [this message]
2024-03-11  9:10       ` Sören Tempel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87plwgfgyo.fsf@gnu.org \
    --to=ludo@gnu.org \
    --cc=68675@debbugs.gnu.org \
    --cc=soeren@soeren-tempel.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this external index

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

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.