Change in vdsm[ovirt-3.4]: ifcfg: stop leaving leftover bridges when removing bonded ne...

danken at redhat.com danken at redhat.com
Sun Mar 30 21:15:02 UTC 2014


Dan Kenigsberg has uploaded a new change for review.

Change subject: ifcfg: stop leaving leftover bridges when removing bonded networks
......................................................................

ifcfg: stop leaving leftover bridges when removing bonded networks

When having a network bridged non-vlanned bonded network, removing
the network while intending to remove the bond would remove the
network bridge and then create it again.

This patch fixes it by removing the 'BRIDGE' entry from the bond
before ifuppin it up again.

Change-Id: I3de692c7f089d6a9f404c3a2f6f568c12d8a985d
Bug-Url: https://bugzilla.redhat.com/1071398
Bug-Url: https://bugzilla.redhat.com/1082275
Signed-off-by: Antoni S. Puimedon <asegurap at redhat.com>
Reviewed-on: http://gerrit.ovirt.org/25203
Reviewed-by: Dan Kenigsberg <danken at redhat.com>
Tested-by: Dan Kenigsberg <danken at redhat.com>
---
M tests/functional/networkTests.py
M vdsm/netconf/ifcfg.py
2 files changed, 30 insertions(+), 4 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/24/26224/1

diff --git a/tests/functional/networkTests.py b/tests/functional/networkTests.py
index 0f7a618..b8028e8 100644
--- a/tests/functional/networkTests.py
+++ b/tests/functional/networkTests.py
@@ -40,7 +40,7 @@
                             getLinks)
 
 from vdsm.utils import RollbackContext
-from vdsm.netinfo import operstate, prefix2netmask, getRouteDeviceTo
+from vdsm.netinfo import bridges, operstate, prefix2netmask, getRouteDeviceTo
 from vdsm import ipwrapper
 
 
@@ -1741,3 +1741,24 @@
                                                       NOCHK)
             self.assertEqual(status, SUCCESS, msg)
             self.assertNetworkDoesntExist(NETWORK_NAME)
+
+    @RequireDummyMod
+    @ValidateRunningAsRoot
+    def testNoBridgeLeftovers(self):
+        """Test for https://bugzilla.redhat.com/1071398"""
+        with dummyIf(2) as nics:
+            network = {NETWORK_NAME: {'bonding': BONDING_NAME}}
+            bonds = {BONDING_NAME: {'nics': nics}}
+            status, msg = self.vdsm_net.setupNetworks(network, bonds, NOCHK)
+            self.assertEqual(status, SUCCESS, msg)
+            self.assertNetworkExists(NETWORK_NAME)
+
+            # Remove the network but not the bond
+            network[NETWORK_NAME] = {'remove': True}
+            status, msg = self.vdsm_net.setupNetworks(network, {}, NOCHK)
+            self.assertEqual(status, SUCCESS, msg)
+            self.assertNotIn(NETWORK_NAME, bridges())
+
+            bonds[BONDING_NAME] = {'remove': True}
+            status, msg = self.vdsm_net.setupNetworks({}, bonds, NOCHK)
+            self.assertEqual(status, SUCCESS, msg)
diff --git a/vdsm/netconf/ifcfg.py b/vdsm/netconf/ifcfg.py
index 16587d9..1f382dc 100644
--- a/vdsm/netconf/ifcfg.py
+++ b/vdsm/netconf/ifcfg.py
@@ -1,4 +1,4 @@
-# Copyright 2011-2013 Red Hat, Inc.
+# Copyright 2011-2014 Red Hat, Inc.
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -189,6 +189,9 @@
             else:
                 self.configApplier.setBondingMtu(bonding.name,
                                                  netinfo.DEFAULT_MTU)
+                if bonding.bridge is not None:
+                    self.configApplier._updateConfigValue(
+                        netinfo.NET_CONF_PREF + bonding.name, 'BRIDGE', None)
                 ifup(bonding.name)
         else:
             set_mtu = self._setNewMtu(
@@ -725,7 +728,8 @@
 
     def _updateConfigValue(self, conffile, entry, value):
         """
-        Set value for network configuration file
+        Set value for network configuration file. If value is None, remove the
+        entry from conffile.
 
         :param entry: entry to update (entry=value)
         :type entry: string
@@ -739,7 +743,8 @@
             entries = [line for line in f.readlines()
                        if not line.startswith(entry + '=')]
 
-        entries.append('\n' + entry + '=' + value)
+        if value is not None:
+            entries.append('\n' + entry + '=' + value)
         self._backup(conffile)
         with open(conffile, 'w') as f:
             f.writelines(entries)


-- 
To view, visit http://gerrit.ovirt.org/26224
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I3de692c7f089d6a9f404c3a2f6f568c12d8a985d
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.4
Gerrit-Owner: Dan Kenigsberg <danken at redhat.com>


More information about the vdsm-patches mailing list