Change in vdsm[ovirt-3.5]: ifcfg: only determine and write HWADDR in removeNic if enabled

osvoboda at redhat.com osvoboda at redhat.com
Thu Sep 17 13:43:15 UTC 2015


Hello Ido Barkan, Dan Kenigsberg,

I'd like you to do a code review.  Please visit

    https://gerrit.ovirt.org/46303

to review the following change.

Change subject: ifcfg: only determine and write HWADDR in removeNic if enabled
......................................................................

ifcfg: only determine and write HWADDR in removeNic if enabled

Writing of HWADDR has been deprecated for long now but for compatibility
with pre-1.0 NetworkManager (that matches on HWADDR in order to accept
NM_CONTROLLED=no) we shall still write it if requested with
hwaddr_in_ifcfg=always in vdsm.conf. This patch makes removeNic follow
suit -- addNic was alone in respecting the condition.

For more information refer to http://gerrit.ovirt.org/22288 and
https://bugzilla.redhat.com/1044060

Change-Id: Ieb4e7cdae83998b4102e79b7cb548ed95f672433
Bug-Url: https://bugzilla.redhat.com/1256252
Signed-off-by: Ondřej Svoboda <osvoboda at redhat.com>
Reviewed-on: https://gerrit.ovirt.org/45983
Continuous-Integration: Jenkins CI
Reviewed-by: Ido Barkan <ibarkan at redhat.com>
Reviewed-by: Dan Kenigsberg <danken at redhat.com>
---
M vdsm/network/configurators/ifcfg.py
1 file changed, 16 insertions(+), 9 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/03/46303/1

diff --git a/vdsm/network/configurators/ifcfg.py b/vdsm/network/configurators/ifcfg.py
index a68a28d..ba0688e 100644
--- a/vdsm/network/configurators/ifcfg.py
+++ b/vdsm/network/configurators/ifcfg.py
@@ -654,24 +654,31 @@
                                      bootproto, async, ipv6autoconf, dhcpv6)
         return ipconfig, mtu
 
-    def removeNic(self, nic):
-        cf = netinfo.NET_CONF_PREF + nic
-        self._backup(cf)
+    @staticmethod
+    def _hwaddr_from_ifcfg_or_system(nic, ifcfg):
+        if not _hwaddr_required():
+            return []
         try:
-            with open(cf) as nicFile:
-                hwlines = [line for line in nicFile if line.startswith(
+            with open(ifcfg) as ifcfg_file:
+                return [line for line in ifcfg_file if line.startswith(
                     'HWADDR=')]
         except IOError as e:
             if e.errno != errno.ENOENT:
-                logging.error("%s couldn't be read (errno %s)", cf, e.errno)
+                logging.error("%s couldn't be read (errno %s)", ifcfg, e.errno)
                 raise
-            logging.warning("%s doesn't exist, reading HWADDR from system", cf)
+            logging.warning("%s doesn't exist, reading HWADDR from the system",
+                            ifcfg)
             try:
-                hwlines = ['HWADDR=%s\n' % netinfo.gethwaddr(nic)]
+                return ['HWADDR=%s\n' % netinfo.gethwaddr(nic)]
             except IOError as e:
                 logging.exception("couldn't determine hardware address of %s "
                                   "(errno %s)", nic, e.errno)
-                hwlines = []
+                return []
+
+    def removeNic(self, nic):
+        cf = netinfo.NET_CONF_PREF + nic
+        self._backup(cf)
+        hwlines = self._hwaddr_from_ifcfg_or_system(nic, cf)
         l = [self.CONFFILE_HEADER + '\n', 'DEVICE=%s\n' % nic, 'ONBOOT=yes\n',
              'MTU=%s\n' % netinfo.DEFAULT_MTU] + hwlines
         l.append('NM_CONTROLLED=no\n')


-- 
To view, visit https://gerrit.ovirt.org/46303
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ieb4e7cdae83998b4102e79b7cb548ed95f672433
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.5
Gerrit-Owner: Ondřej Svoboda <osvoboda at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Ido Barkan <ibarkan at redhat.com>


More information about the vdsm-patches mailing list