[bug#54352] [PATCH] services: dnsmasq: Add more options.

["Ludovic Courtès" <ludo@gnu.org>]

Hi,

Remco van 't Veer <remco@remworks.net> skribis:

> * gnu/services/dns.scm (<dnsmasq-configuration>): Add bogus-priv?,
> strict-order? and add-cpe-id options.
> (dnsmasq-shepherd-service): Pass bogus-priv, strict-order and add-cpe-id
> to the service.
> * doc/guix.texi (Guix Services): Document options added to dnsmasq.

I don’t use dnsmasq, but overall that LGTM, and a welcome addition.

Nitpick:

> +  (bogus-priv?      dnsmasq-configuration-bogus-priv?
> +                    (default #f))       ;boolean

I’d rather avoid the abbreviation and use something more descriptive,
even if that doesn’t match the name used in the config file.  Maybe
something like ‘forward-private-reverse-lookup?’ (I’m making it up based
on the doc you wrote.)  WDYT?

> +  (add-cpe-id       dnsmasq-configuration-add-cpe-id
> +                    (default #t))       ;string

Maybe ‘additional-cpe-id’ or ‘extra-cpe-id’ or ‘cpe-id’?

>      (($ <dnsmasq-configuration> package
>                                  no-hosts?
>                                  port local-service? listen-addresses
> -                                resolv-file no-resolv? servers
> -                                addresses cache-size negative-cache?
> +                                resolv-file no-resolv? bogus-priv?
> +                                strict-order? servers addresses cache-size
> +                                negative-cache? add-cpe-id

Not a blocker for this patch, but we should change that to
‘match-record’, which is less error-prone.

Let me know what you think; you can send an updated patch if the
suggestions make sense to you.

Thanks!

Ludo’.