Bug 1427482

the ifcfg-plugin cannot correctly write/read the team slave connection.
Reboot is not relevant to reproduce



nmcli con add type team con-name team142 ifname team142 ip4 10.106.142.57/24 ipv6.method ignore autoconnect no
nmcli con add type vlan slave-type team con-name team-slave-enp31s0f1-142 ifname enp31s0f1-142 dev enp31s0f1 id 142 master team142 autoconnect no

nmcli connection show team-slave-enp31s0f1-142 > c-1
nmcli connection reload
nmcli connection show team-slave-enp31s0f1-142 > c-2

diff c-?




# cat /etc/sysconfig/network-scripts/ifcfg-team-slave-enp31s0f1-142 
VLAN=yes
DEVICE=enp31s0f1-142
PHYSDEV=enp31s0f1
VLAN_ID=142
REORDER_HDR=yes
GVRP=no
MVRP=no
NAME=team-slave-enp31s0f1-142
UUID=bffcb0c3-a6dd-4ceb-8c84-1bc569bca556
ONBOOT=no
TEAM_MASTER=team142
DEVICETYPE=TeamPort

https://cgit.freedesktop.org/NetworkManager/NetworkManager/log/?h=th/ifcfg-reread-rh1427482

as workaround you could use the keyfile plugin.

NM doesn't allow you to move the connection from ifcfg-rh settings plugin to keyfile plugin, but you can just create a file like



cat <<EOF > /etc/NetworkManager/system-connections/team-slave-enp31s0f1-142
[vlan]
parent=foo
EOF
chmod 600 /etc/NetworkManager/system-connections/team-slave-enp31s0f1-142
nmcli connection reload



and from then on, modify the connection as you like:

  nmcli con modify id team-slave-enp31s0f1-142 slave-type team con-name \
        team-slave-enp31s0f1-142 ifname enp31s0f1-142 dev enp31s0f1 id 142 \
        master team142

> ifcfg-rh: change "goto error" pattern to return early and nm_auto*

 write_route_file_legacy (const char *filename, NMSettingIPConfig *s_ip4, GError **error)
 {
 [...]
        if (!g_file_set_contents (filename, route_contents, -1, NULL)) {
                g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED,
                             "Writing route file '%s' failed", filename);
-               goto error;
+               return TRUE;

return FALSE?


> ifcfg-rh: re-read connection after write and compare the result

Perhaps the check should only be performed when NM is compiled with
more assertions? In other words, the question is: do we want users to
see such warnings in the logs and report them (even if they might be
harmless from user point of view, like the one about missing wired
setting on reread)?

The rest LGTM.

LGTM (apart from what Beniamino already pointed out)

(In reply to Beniamino Galvani from comment #5)

> > ifcfg-rh: re-read connection after write and compare the result
> 
> Perhaps the check should only be performed when NM is compiled with
> more assertions? In other words, the question is: do we want users to
> see such warnings in the logs and report them (even if they might be
> harmless from user point of view, like the one about missing wired
> setting on reread)?

there are two reasons why the patch of always warning is wrong:

 (1) during write, certificate blobs are converted to paths, and (as ugly
   it is to do that), it is a valid situation where a re-read connection
   differs
 (2) contrary to the keyfile implementation, for ifcfg we re-read the
   connection from file. Thus, there is a race where somebody might
   modify the connection after we write it and before reading it back.

The other cases, I consider a bug. Thus, they are not *harmless* at all. However, logging a warning about a bug isn't helpful to the user either, the bug should be fixed instead.

TL;DR: yes, now no warnings are logged.


Btw, (2) is the reason why I don't do "keyfile: updated connection when writing keyfile" for ifcfg-rh. Although, I really wish we would, but that would require that the writer first creates an in-memory-representation of the files and parses that back in, without accessing the file. Like keyfile writer does.


How about now?

LGTM now.

thanks. Merged: https://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?id=9d39569287f6770b951e30f68d88e14f9ec68ac7

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHSA-2017:2299