Bug 1369008 - Once NetworkManager is stopped, the ifcfg files it created via nmtui\cockpit are incompatible with initscripts, since MASTER=UUID instead of MASTER=device_name
Summary: Once NetworkManager is stopped, the ifcfg files it created via nmtui\cockpit ...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Red Hat Enterprise Linux 7
Classification: Red Hat
Component: NetworkManager
Version: 7.2
Hardware: x86_64
OS: Linux
high
high
Target Milestone: rc
: ---
Assignee: Lubomir Rintel
QA Contact: Desktop QE
Mirek Jahoda
URL:
Whiteboard:
Depends On:
Blocks: 1368650 1393481 1413048
TreeView+ depends on / blocked
 
Reported: 2016-08-22 10:33 UTC by Michael Burman
Modified: 2019-04-28 13:11 UTC (History)
15 users (show)

Fixed In Version: NetworkManager-1.8.0-0.2.git20170215.1d40c5f4.el7
Doc Type: Enhancement
Doc Text:
Previously, when creating a Team, Bridge, or Bond slave, NetworkManager wrote the Universally Unique Identifier (UUID) of the master connection to the ifcfg file. This was not compatible with the legacy network service that supports only using the device interface name. With this update, NetworkManager attempts to write a device name in addition to the connection UUID, and compatibility with the legacy network service has been improved. Note that only UUID is used in case NetworkManager is not able to resolve the master connection into a device name. That is currently the case when slave connections are added before the master connection is saved in the nmtui tool.
Clone Of:
: 1413048 (view as bug list)
Environment:
Last Closed: 2017-08-01 09:17:07 UTC


Attachments (Terms of Use)
Attempt at producing more compatible ifcfg files (9.34 KB, text/plain)
2016-11-28 13:24 UTC, Lubomir Rintel
no flags Details


Links
System ID Priority Status Summary Last Updated
Red Hat Product Errata RHSA-2017:2299 normal SHIPPED_LIVE Moderate: NetworkManager and libnl3 security, bug fix and enhancement update 2017-08-01 12:40:28 UTC

Description Michael Burman 2016-08-22 10:33:03 UTC
Description of problem:
[nmtui] - When creating a bond via nmtui it creates ifcfg files with MASTER=UUID instead of MASTER=device_name. 

cat /etc/sysconfig/network-scripts/ifcfg-bond1 
DEVICE=bond1
TYPE=Bond
BONDING_MASTER=yes
BOOTPROTO=dhcp
DEFROUTE=yes
IPV4_FAILURE_FATAL=no
IPV6INIT=yes
IPV6_AUTOCONF=yes
IPV6_DEFROUTE=yes
IPV6_FAILURE_FATAL=no
NAME=bond1
UUID=ba42c612-413e-4759-a74b-e9db8daff1b4
ONBOOT=yes
BONDING_OPTS="resend_igmp=1 updelay=0 use_carrier=1 miimon=100 downdelay=0 xmit_hash_policy=0 primary_reselect=0 fail_over_mac=0 arp_validate=0 mode=active-backup primary=enp4s0 lacp_rate=0 arp_interval=0 ad_select=0"
PEERDNS=yes
PEERROUTES=yes
IPV6_PEERDNS=yes
IPV6_PEERROUTES=yes


cat /etc/sysconfig/network-scripts/ifcfg-enp4s0-1 
TYPE=Ethernet
NAME=enp4s0
UUID=06cf9ac7-b3d6-4f10-959d-fc2c381046ac
DEVICE=enp4s0
ONBOOT=yes
MASTER=ba42c612-413e-4759-a74b-e9db8daff1b4
SLAVE=yes

cat /etc/sysconfig/network-scripts/ifcfg-enp6s0-1 
TYPE=Ethernet
NAME=enp6s0
UUID=e12157b6-5914-4556-8c24-7d1026c22dcd
DEVICE=enp6s0
ONBOOT=yes
MASTER=ba42c612-413e-4759-a74b-e9db8daff1b4
SLAVE=yes

Version-Release number of selected component (if applicable):
NetworkManager-1.0.6-30.el7_2.x86_64
rhel7.2

How reproducible:
100

Steps to Reproduce:
1. Create bond via nmtui

Actual results:
The slave's ifcfg files created with MASTER=UUID instead of MASTER=device_name

Expected results:
MASTER=device_name

Comment 2 Aniss 2016-08-22 21:00:46 UTC
confirmed with:
NetworkManager -V
1.0.6-27.el7
uname -r
3.10.0-327.el7.x86_64
where Master in slaves have the UUID of the bond master

Comment 3 Thomas Haller 2016-08-23 11:12:16 UTC
the master can be either specified to be the name of an interface or it can be the UUID of another connection. Both is valid from NetworkManager's point of view.

nmtui currently only supports the latter.

It's a valid feature request to also support specifying the interface name. But note that nmtui by design does not support all options to keep the UI simpler. In those cases, nmcli is the expert tool that gives access to every option.

Comment 4 Lubomir Rintel 2016-11-28 13:24:03 UTC
Created attachment 1225286 [details]
Attempt at producing more compatible ifcfg files

Comment 5 Thomas Haller 2016-11-28 13:38:49 UTC
there is svUnsetValue(,) for svSetValueString(,,NULL)


Also, pre-setting all values to NULL and only setting them later is not nice. It's better unset values if-and-only-if they are not to be set.

E.g.:

   #the master property
   MASTER=bond0

   TYPE=Ethernet

results on update in:

   #the master property

   TYPE=Ethernet
   MASTER=bond1

if you only reset the value without clearing it you get:


   #the master property
   MASTER=bond1

   TYPE=Ethernet


Also, unsetting the value unnecessarily, always marks the ifcfg-file as dirty, even if there is no change.

Comment 6 Thomas Haller 2016-11-28 18:07:25 UTC
(In reply to Thomas Haller from comment #5)
> 
> Also, pre-setting all values to NULL and only setting them later is not
> nice. It's better unset values if-and-only-if they are not to be set.
> 
> ...
>
> Also, unsetting the value unnecessarily, always marks the ifcfg-file as
> dirty, even if there is no change.

hm, rethinking this, shvarFile should become more intelligent and handle such cases automatically. It could do so by keeping a backup copy of the originally read values and then:

  - only actually store the file to disk if it really changed (maybe optionally,
    by having a always_write option).
  - it could avoid the reordering issue by merging the changes with the backup 
    copy.
  - it could even remove all keys that were not explicitly touched. Thus, making 
    the shUnsetValue() calls uneccessary because it would drop all keys that
    were not explicitly set. Preferably, not dropping them, but commenting them 
    out.

Comment 7 Lubomir Rintel 2016-12-05 12:41:53 UTC
https://cgit.freedesktop.org/NetworkManager/NetworkManager/log/?h=lr/ifcfg-device-rh1369008

Pushed some fixups.

Hoping you're *not* actually expecting me to refactor shfile.

Comment 8 Thomas Haller 2016-12-05 13:12:33 UTC
(In reply to Lubomir Rintel from comment #7)
> 
> Hoping you're *not* actually expecting me to refactor shfile.

"refactoring" is the wrong word. Extend and improve.

Similar issues are all over the place, e.g. https://cgit.freedesktop.org/NetworkManager/NetworkManager/tree/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c?id=a96c819f6f5d1c4faa21b242bea18096049c9674#n1318

~somebody~ should fix it and it's rather simple.

Comment 9 Lubomir Rintel 2016-12-15 17:10:22 UTC
Merged upstream in 80e43390310c32462912eb88da2a9a12c2f29dc7

Comment 14 Lubomir Rintel 2017-02-20 19:00:27 UTC
Turns out nmtui creates the slave connections first and then the master; thus the ifcfg plugin is unable to determine the master device name. This branch commits the slave connection only after the master editor is closed:

https://cgit.freedesktop.org/NetworkManager/NetworkManager/log/?h=lr/nmtui-slaves-after-master-rh1369008

Comment 15 Francesco Giudici 2017-02-21 17:33:30 UTC
I would just remove the comment here:

--- a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-connection.c
+++ b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-connection.c
@@ -340,10 +340,6 @@ commit_changes (NMSettingsConnection *connection,
        char *ifcfg_path = NULL;
        const char *filename;
 
-       /* To ensure we don't rewrite files that are only changed from other
-        * processes on-disk, read the existing connection back in and only rewrite
-        * it if it's really changed.
-        */
        filename = nm_settings_connection_get_filename (connection);
        if (filename) {
                success = writer_update_connection (NM_CONNECTION (connection),

as the connection is no more read back here.


Then found a minor typo in a comment:
--- a/clients/tui/nmt-editor-page.c
+++ b/clients/tui/nmt-editor-page.c
@@ -116,8 +116,8 @@ nmt_editor_page_add_section (NmtEditorPage *page,
  * nmt_editor_page_saved:
  * @page: the #NmtEditorPage
  *
- * This method is called when the user saves the connection. It givesa the
- * page a chance to do save its data outside the connections (such as
+ * This method is called when the user saves the connection. It gives
+ * the page a chance to save its data outside the connections (such as
  * recommit the slave connections).
  */

Branch lgtm.

Comment 16 errata-xmlrpc 2017-08-01 09:17:07 UTC
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


Note You need to log in before you can comment on or make changes to this bug.