Package: guix-patches;
Reported by: Alexey Abramov <levenson <at> mmer.org>
Date: Sun, 23 Mar 2025 10:27:04 UTC
Severity: normal
Tags: patch
Done: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Bug is archived. No further changes may be made.
Message #53 received at 77204 <at> debbugs.gnu.org (full text, mbox):
From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com> To: Alexey Abramov <levenson <at> mmer.org> Cc: Ludovic Courtès <ludo <at> gnu.org>, 77204 <at> debbugs.gnu.org Subject: Re: [bug#77204] [PATCH v3 2/3] services: dnsmasq: Add pid-file, conf-file and conf-dir configuration fields. Date: Tue, 22 Apr 2025 14:35:30 +0900
Hi,
Alexey Abramov <levenson <at> mmer.org> writes:
> * gnu/services/dns.scm (<dnsmasq-configuration>)[pid-file]: New field to
> specify alternate path for dnsmasq PID.
> * gnu/services/dns.scm (<dnsmasq-configuration>)[conf-file]: New field to
> specify one or more configuration files.
> * gnu/services/dns.scm (<dnsmasq-configuration>)[conf-dir]: New field to
> read configuration files from a directory.
> * gnu/services/dns.scm (<dnsmasq-configuration>)[extra-options]: Move
> to the end of the definition as a last resort option.
> * gnu/services/dns.scm (dnsmasq-shepherd-service): Use the new fields
> instead of hardcoded values.
> * gnu/services/dns.scm: Export all record accessors.
> * doc/guix.texi: Document the new configuration options.
As mentioned earlier, file names should only appear once.
> ---
> doc/guix.texi | 14 ++++
> gnu/services/dns.scm | 157 ++++++++++++++++++++++++++++---------------
> 2 files changed, 115 insertions(+), 56 deletions(-)
>
> diff --git a/doc/guix.texi b/doc/guix.texi
> index 90fa6779657..1bbd1824a87 100644
> --- a/doc/guix.texi
> +++ b/doc/guix.texi
> @@ -35076,6 +35076,9 @@ DNS Services
> Likewise, @code{shepherd-requirement} is a list of Shepherd service names
> (symbols) that this service will depend on.
>
> +@item @code{pid-file} (default: @code{"/run/dnsmasq.pid"})
> +Specify an alternate path for dnsmasq to record its process-id in.
> +
> @item @code{no-hosts?} (default: @code{#f})
> When true, don't read the hostnames in /etc/hosts.
>
> @@ -35196,6 +35199,17 @@ DNS Services
> resolving MAC addresses is only possible if the client is in the local
> network or obtained a DHCP lease from dnsmasq.
>
> +@item @code{conf-file} (default: @code{'()})
> +Specify a configuration file or multiple. The given value should be a list of
> +string paths to the configuration files. File-like objects are also supported.
> +
> +@item @code{conf-dir} (default: @code{#f})
> +Read all the files in the given directory as configuration
> +files. @command{dnsmasq} also supports extensions for the field, but
> +here it is not implemented. It is more convenient to make
> +@code{computed-file} directory in the store and use that. Files are
> +loaded in alphabetical order of filename.
> +
> @item @code{extra-options} (default: @code{'()})
> This option provides an ``escape hatch'' for the user to provide arbitrary
> command-line arguments to @command{dnsmasq} as a list of strings.
> diff --git a/gnu/services/dns.scm b/gnu/services/dns.scm
> index f617f26891d..210fef4ece2 100644
> --- a/gnu/services/dns.scm
> +++ b/gnu/services/dns.scm
> @@ -55,6 +55,38 @@ (define-module (gnu services dns)
>
> dnsmasq-service-type
> dnsmasq-configuration
> + dnsmasq-configuration-package
> + dnsmasq-configuration-shepherd-provision
> + dnsmasq-configuration-shepherd-requirement
> + dnsmasq-configuration-pid-file
> + dnsmasq-configuration-no-hosts?
> + dnsmasq-configuration-port
> + dnsmasq-configuration-local-service?
> + dnsmasq-configuration-listen-address
> + dnsmasq-configuration-resolv-file
> + dnsmasq-configuration-no-resolv?
> + dnsmasq-configuration-forward-private-reverse-lookup?
> + dnsmasq-configuration-query-servers-in-order?
> + dnsmasq-configuration-servers
> + dnsmasq-configuration-servers-file
> + dnsmasq-configuration-addresses
> + dnsmasq-configuration-cache-size
> + dnsmasq-configuration-negative-cache?
> + dnsmasq-configuration-cpe-id
> + dnsmasq-configuration-tftp-enable?
> + dnsmasq-configuration-tftp-no-fail?
> + dnsmasq-configuration-tftp-single-port?
> + dnsmasq-tftp-secure?
> + dnsmasq-tftp-max
> + dnsmasq-tftp-mtu
> + dnsmasq-tftp-no-blocksize?
> + dnsmasq-tftp-lowercase?
> + dnsmasq-tftp-port-range
> + dnsmasq-tftp-root
> + dnsmasq-tftp-unique-root
> + dnsmasq-configuration-conf-file
> + dnsmasq-configuration-conf-dir
> + dnsmasq-configuration-extra-options
>
> unbound-service-type
> unbound-configuration
> @@ -750,60 +782,65 @@ (define-record-type* <dnsmasq-configuration>
> (default '(dnsmasq)))
> (shepherd-requirement dnsmasq-configuration-shepherd-requirement
> (default '(user-processes networking)))
> - (no-hosts? dnsmasq-configuration-no-hosts?
> - (default #f)) ;boolean
> - (port dnsmasq-configuration-port
> - (default 53)) ;integer
> - (local-service? dnsmasq-configuration-local-service?
> - (default #t)) ;boolean
> - (listen-addresses dnsmasq-configuration-listen-address
> - (default '())) ;list of string
> - (extra-options dnsmasq-configuration-extra-options
> - (default '())) ;list of string
> - (resolv-file dnsmasq-configuration-resolv-file
> - (default "/etc/resolv.conf")) ;string
> - (no-resolv? dnsmasq-configuration-no-resolv?
> - (default #f)) ;boolean
> + (pid-file dnsmasq-configuration-pid-file
> + (default "/run/dnsmasq.pid")) ;string
> + (no-hosts? dnsmasq-configuration-no-hosts?
> + (default #f)) ;boolean
> + (port dnsmasq-configuration-port
> + (default 53)) ;integer
> + (local-service? dnsmasq-configuration-local-service?
> + (default #t)) ;boolean
> + (listen-addresses dnsmasq-configuration-listen-address
> + (default '())) ;list of string
> + (resolv-file dnsmasq-configuration-resolv-file
> + (default "/etc/resolv.conf")) ;string
> + (no-resolv? dnsmasq-configuration-no-resolv?
> + (default #f)) ;boolean
> (forward-private-reverse-lookup?
> - dnsmasq-configuration-forward-private-reverse-lookup?
> - (default #t)) ;boolean
> - (query-servers-in-order?
> - dnsmasq-configuration-query-servers-in-order?
> - (default #f)) ;boolean
> - (servers dnsmasq-configuration-servers
> - (default '())) ;list of string
> - (servers-file dnsmasq-configuration-servers-file
> - (default #f)) ;string|file-like
> - (addresses dnsmasq-configuration-addresses
> - (default '())) ;list of string
> - (cache-size dnsmasq-configuration-cache-size
> - (default 150)) ;integer
> - (negative-cache? dnsmasq-configuration-negative-cache?
> - (default #t)) ;boolean
> - (cpe-id dnsmasq-configuration-cpe-id
> - (default #f)) ;string
> - (tftp-enable? dnsmasq-configuration-tftp-enable?
> - (default #f)) ;boolean
> - (tftp-no-fail? dnsmasq-configuration-tftp-no-fail?
> - (default #f)) ;boolean
> - (tftp-single-port? dnsmasq-configuration-tftp-single-port?
> - (default #f)) ;boolean
> - (tftp-secure? dnsmasq-tftp-secure?
> - (default #f)) ;boolean
> - (tftp-max dnsmasq-tftp-max
> - (default #f)) ;integer
> - (tftp-mtu dnsmasq-tftp-mtu
> - (default #f)) ;integer
> - (tftp-no-blocksize? dnsmasq-tftp-no-blocksize?
> - (default #f)) ;boolean
> - (tftp-lowercase? dnsmasq-tftp-lowercase?
> - (default #f)) ;boolean
> - (tftp-port-range dnsmasq-tftp-port-range
> - (default #f)) ;string
> - (tftp-root dnsmasq-tftp-root
> - (default "/var/empty,lo")) ;string
> - (tftp-unique-root dnsmasq-tftp-unique-root
> - (default #f))) ;"" or "ip" or "mac"
> + dnsmasq-configuration-forward-private-reverse-lookup?
> + (default #t)) ;boolean
> + (query-servers-in-order? dnsmasq-configuration-query-servers-in-order?
> + (default #f)) ;boolean
> + (servers dnsmasq-configuration-servers
> + (default '())) ;list of string
> + (servers-file dnsmasq-configuration-servers-file
> + (default #f)) ;string|file-like
> + (addresses dnsmasq-configuration-addresses
> + (default '())) ;list of string
> + (cache-size dnsmasq-configuration-cache-size
> + (default 150)) ;integer
> + (negative-cache? dnsmasq-configuration-negative-cache?
> + (default #t)) ;boolean
> + (cpe-id dnsmasq-configuration-cpe-id
> + (default #f)) ;string
> + (tftp-enable? dnsmasq-configuration-tftp-enable?
> + (default #f)) ;boolean
> + (tftp-no-fail? dnsmasq-configuration-tftp-no-fail?
> + (default #f)) ;boolean
> + (tftp-single-port? dnsmasq-configuration-tftp-single-port?
> + (default #f)) ;boolean
> + (tftp-secure? dnsmasq-tftp-secure?
> + (default #f)) ;boolean
> + (tftp-max dnsmasq-tftp-max
> + (default #f)) ;integer
> + (tftp-mtu dnsmasq-tftp-mtu
> + (default #f)) ;integer
> + (tftp-no-blocksize? dnsmasq-tftp-no-blocksize?
> + (default #f)) ;boolean
> + (tftp-lowercase? dnsmasq-tftp-lowercase?
> + (default #f)) ;boolean
> + (tftp-port-range dnsmasq-tftp-port-range
> + (default #f)) ;string
> + (tftp-root dnsmasq-tftp-root
> + (default "/var/empty,lo")) ;string
> + (tftp-unique-root dnsmasq-tftp-unique-root
> + (default #f)) ;"" or "ip" or "mac"
> + (conf-file dnsmasq-configuration-conf-file
> + (default '())) ;list of string|file-like
> + (conf-dir dnsmasq-configuration-conf-dir
> + (default #f)) ;string|file-like
> + (extra-options dnsmasq-configuration-extra-options
> + (default '())))
Please refrain from adjusting the indentation like this; it makes
reviewing difficult and here also breaks our max width of 80 columns.
> (define (warn-deprecated-dnsmasq-configuration-provision value)
> (when (pair? value)
> @@ -818,6 +855,7 @@ (define (dnsmasq-shepherd-service config)
> provision
> shepherd-provision
> shepherd-requirement
> + pid-file
> no-hosts?
> port local-service? listen-addresses
> resolv-file no-resolv?
> @@ -829,7 +867,8 @@ (define (dnsmasq-shepherd-service config)
> tftp-single-port? tftp-secure?
> tftp-max tftp-mtu tftp-no-blocksize?
> tftp-lowercase? tftp-port-range
> - tftp-root tftp-unique-root extra-options)
> + tftp-root tftp-unique-root
> + conf-file conf-dir extra-options)
> (shepherd-service
> (provision (or provision shepherd-provision))
> (requirement shepherd-requirement)
> @@ -838,7 +877,7 @@ (define (dnsmasq-shepherd-service config)
> (list
> #$(file-append package "/sbin/dnsmasq")
> "--keep-in-foreground"
> - "--pid-file=/run/dnsmasq.pid"
> + (string-append "--pid-file=" #$pid-file)
> #$@(if no-hosts?
> '("--no-hosts")
> '())
> @@ -909,8 +948,14 @@ (define (dnsmasq-shepherd-service config)
> (format #f "--tftp-unique-root=~a" tftp-unique-root)
> (format #f "--tftp-unique-root")))
> '())
> + #$@(map (lambda (conf-file)
> + #~(string-append "--conf-file=" #$conf-file))
> + conf-file)
> + #$@(if conf-dir
> + (list #~(string-append "--conf-dir=" #$conf-dir))
> + '())
> #$@extra-options)
> - #:pid-file "/run/dnsmasq.pid"))
> + #:pid-file #$pid-file))
> (stop #~(make-kill-destructor)))))
Other than that, it looks like a good change! Thank you.
--
Thanks,
Maxim
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.