Hide Forgot
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
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
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.
Created attachment 1225286 [details] Attempt at producing more compatible ifcfg files
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.
(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.
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.
(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.
Merged upstream in 80e43390310c32462912eb88da2a9a12c2f29dc7
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
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.
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