GNOME Bugzilla – Bug 773069
Support reading configuration from /run/NetworkManager/
Last modified: 2016-10-23 10:58:32 UTC
NM currently supports reading its config from /usr/lib/NetworkManager/conf.d/ (for distro packages) and /etc/NetworkManager/conf.d/ (for local/admin settings). I would like to add /run/NetworkManager/conf.d/ and /run/NetworkManager/system-connections/ to the search path. /run is useful if the files there are being auto-generated from another source like netplan [1], cloud-init, etc. For example, we have a systemd generator that creates a /run/NM/conf.d/ snippet with "unmanaged-devices+=..." for devices which are managed by systemd-networkd, so that the user does not accidentally interfere with those via nm-applet or nmcli. It would be awkward to put these auto-generated files into /etc as they are not primary configuration files, would need to be upgraded, admins could put in customizations, etc. This mirrors the structure of udev rules or systemd units, which can also be in /usr, /run/, or /etc/. [1] https://git.launchpad.net/~netplan-developers/netplan/tree/doc/netplan.md
Created attachment 337815 [details] [review] config: read /run/NetworkManager/conf.d files Patch for reading conf.d/ from /run, from Mathieu (added to CC:). I am porting our corresponding system-connections/ patch from 1.2.4, will attach that in a bit.
system-connections is quite a different thing from conf.d For the former I opened already bug https://bugzilla.gnome.org/show_bug.cgi?id=772414
Created attachment 337817 [details] [review] keyfile: read /run/NetworkManager/system-connections The second patch which extends keyfile to read connections from /run/NetworkManager/system-connections/.
@Thomas: Thanks, I didn't find bug 772414 in my bz search. What is connections.d/? Is that similar to/the successor of system-connections/ ?
Yes, I think /etc/NetworkManager/system-connections is a historic misnomer, because originally there were connections in the user-sessions (non-systemwide). Renaming the directory now is problematic, but the new alternate location should probably be named something like "/usr/lib/NetworkManager/connections.d", or maybe "/usr/lib/NetworkManager/keyfile.d"... no strong opinion there...
Created attachment 337818 [details] [review] keyfile: read /run/NetworkManager/system-connections Bah, forgot to re-run format-patch, the previous one was still missing a function rename (for porting to master).
Created attachment 337819 [details] [review] keyfile: read /run/NetworkManager/connections.d > /usr/lib/NetworkManager/connections.d WFM. Simple rename in the #define and the commit log.
I merged to addition for /var/run/NetworkManager/conf.d to master: https://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?id=3248b387a29ff35fedd9cdfb9f784c985298d428 Regarding the keyfile directory, I think that is more complicated. We also want to support a directory /usr/lib/NetworkManager/connection.d, so the change should be compatible with that (whatever the details). The keyfile path is configurable. Thus the code should make sure that nms_keyfile_utils_get_path() differs from NM_CONFIG_KEYFILE_PATH_RUNTIME. If the user runs NM with a keyfile path of "/var/run/NetworkManager/connection.d", it should mean that the hard-coded NM_CONFIG_KEYFILE_PATH_RUNTIME path is ignored. Basically, let's add a function nms_keyfile_utils_get_path_run() which returns NULL if user configured the paths to be the same. The ID (primary key) of a connection is their UUID (that is, you cannot have two connections with the same UUID). Currently, if /etc directory contains duplicates UUID, we prefer the file with the latest modification date (see _sort_paths() and note that update_connection() does not replace existing connections using the @protected_connections hash). I think with duplicates in /usr and /run directories, we want to shadow connections that are defined in later directories. That is, prefer /etc over /run over /usr. To achieve that, it's sufficient to properly adjust _sort_paths(). I think the file modification date should not matter for /usr and /run directories, instead here the filename should be used. Also, I think @paths should only be considered for files in /etc. load_connection() only accepts files from /etc. That seems right to me. If the user really wants to load a connection from /usr or /run, he needs to issue a full reload_connection() -- which properly handles duplicate UUIDs (see before). Nothing to do there. NetworkManager should never write to those locations /usr or /run. They should be read-only. So when a user tries to modify/delete a connection defined there, it should be rejected. Also, converting such a connection to in-memory-only should be rejected too. In a (later?) follow up, we can improve that: NM should put a file to /etc that shadows the file (or in case of deletion, it overwrites it). In case of modifying such a connection that is simple. Just create a new file in /etc. With the shadowing mechanism from before it will work out right. If the user deletes such a connection (or convert it to in-memory-only), we must persist a special marker to /etc. Maybe something like [connection] uuid=$UUID [.keyfile] deleted=1 Keyfile reader then must handle this properly to shadow files. It cannot be like systemd does symlinking files to /dev/null, because it's not the filename that is shadowed, but the UUID. The filename doesn't matter. I am closing this bug a fixed. Let's handle the keyfile dir as part of bug 772414