Re: [bug#75145] [PATCH v2 0/1] services: network-manager: Add extra-configuration-files field.

[45mg]

Hi Arnaud!

Arnaud Daby-Seesaram <ds-ac@nanein.fr> writes:

> Adding a default configuration could one day be interesting.  However,
> I would rather (personal opinion) see the default value in an exported
> variable %default-networkmanager-files (non-contractual name) if needed.
> This variable could become the default value of the field.
>
> This way, it would be more transparent to users, and would enable them
> to easily opt-out.  Adding their files could be done with the following
> configuration:
>
> --8<---------------cut here---------------start------------->8---
> (extra-configuration-files
>   (cons* `("file1" ,(plain-file ...))
>          ...
>          %default-networkmanager-files))
> --8<---------------cut here---------------end--------------->8---

Yes, I think I agree. Of course, it's a moot point for now since we
don't have any use for such a variable yet, but it seems like a good
approach.

> As you said, you are doing something similar to `etc-service-type'.  Is
> there a reason not to extend [2] it directly (e.g. like
> `greetd-service-type' does in `(gnu services base)')?
>
> You could, for example, prepend "NetworkManager/conf.d/" to file names
> and pass the value to `etc-service-type'.  WDYT?

That's a good suggestion! I don't know why it didn't occur to me.

So I tried it; the problem is that if I pass something like this:
--8<---------------cut here---------------start------------->8---
(service-extension etc-service-type
                   (lambda (config)
                     `(("NetworkManager/conf.d"
                        ,(network-manager-configuration-directory config)))))
--8<---------------cut here---------------end--------------->8---

...then I get this error when creating a container:
--8<---------------cut here---------------start------------->8---
ERROR: In procedure primitive-load:
In procedure mkdir: Read-only file system
--8<---------------cut here---------------end--------------->8---

It looks like this is because of this line in `network-manager-activation`:
--8<---------------cut here---------------start------------->8---
        (mkdir-p "/etc/NetworkManager/system-connections")
--8<---------------cut here---------------end--------------->8---

When we use `etc-service-type`, "/etc/NetworkManager" becomes a symlink
to "/etc/static/NetworkManager"; and "/etc/static" is a symlink to the
result of building the derivation returned by `etc-entry` (see (gnu
services)). And derivations are in the store, which is read-only. So we
can't create "/etc/NetworkManager/system-connections/". And this won't
do, since NetworkManager itself needs to be able to write to that
directory to manage saved connections.

So it looks like that won't work.

> Would it make sense to allow the NetworkManager service to be extended?
> (E.g to allow another service to add a configuration file.)
>
> Note: this is not a blocker for your patch and could be done in a later
>       patch.  It is simply a thought that I wanted to share.

Yeah, this was another reason for this revision. Again, it's not
actually something we need right now; it's just worth leaving open as a
possibility.