Change in vdsm[ovirt-3.3]: Remove bonds before editing and adding

Hello Assaf Muller, Dan Kenigsberg,

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

    http://gerrit.ovirt.org/18587

to review the following change.

Change subject: Remove bonds before editing and adding
......................................................................

Remove bonds before editing and adding

The current implementation of handleBondings relies on the
non-deterministic ordering of the bondings dictionary for choosing
which bonds should be handled first.

The correct ordering is to first delete all the bonds to remove,
then edit and finally add new bonds. This ensures that slaves
of bonds to remove are properly freed before being added to existing
or new bonds.

Change-Id: I3ea081fe7760b52f6febb4959f69851b2cf4c2a1
Bug-Url: https://bugzilla.redhat.com/987525
Signed-off-by: Antoni S. Puimedon <asegurap(a)redhat.com&gt;
Reviewed-on: http://gerrit.ovirt.org/18500
Reviewed-by: Assaf Muller <amuller(a)redhat.com&gt;
Reviewed-by: Dan Kenigsberg <danken(a)redhat.com&gt;
---
M vdsm/configNetwork.py
1 file changed, 26 insertions(+), 19 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/87/18587/1

diff --git a/vdsm/configNetwork.py b/vdsm/configNetwork.py
index 155d22a..36edba7 100755
--- a/vdsm/configNetwork.py
+++ b/vdsm/configNetwork.py
@@ -398,26 +398,33 @@
 
     _netinfo = netinfo.NetInfo()
 
-    for bondName, bondAttrs in bondings.items():
-        bond = Bond.objectivize(bondName, configurator,
-                                bondAttrs.get('options'),
-                                bondAttrs.get('nics'), mtu=None,
-                                _netinfo=_netinfo,
-                                destroyOnMasterRemoval='remove' in bondAttrs)
-        if 'remove' in bondAttrs:
-            logger.debug("Removing bond %s with attributes %s", bondName,
-                         bondAttrs)
-            configurator.removeBond(bond)
-            del bondings[bondName]
-            del _netinfo.bondings[bondName]
-        elif bondName in _netinfo.bondings:
-            logger.debug("Editing bond %s with attributes %s", bondName,
-                         bondAttrs)
-            configurator.editBonding(bond, _netinfo)
+    edition = []
+    addition = []
+    for name, attrs in bondings.items():
+        if 'remove' in attrs:
+            bond = Bond.objectivize(name, configurator, attrs.get('options'),
+                                    attrs.get('nics'), mtu=None,
+                                    _netinfo=_netinfo,
+                                    destroyOnMasterRemoval='remove' in attrs)
+            bond.remove()
+            del _netinfo.bondings[name]
+        elif name in _netinfo.bondings:
+            edition.append((name, attrs))
         else:
-            logger.debug("Creating bond %s with attributes %s", bondName,
-                         bondAttrs)
-            configurator.configureBond(bond)
+            addition.append((name, attrs))
+
+    for name, attrs in edition:
+        bond = Bond.objectivize(name, configurator, attrs.get('options'),
+                                attrs.get('nics'), mtu=None, _netinfo=_netinfo,
+                                destroyOnMasterRemoval='remove' in attrs)
+        logger.debug("Editing bond %r with options %s", bond, bond.options)
+        configurator.editBonding(bond, _netinfo)
+    for name, attrs in addition:
+        bond = Bond.objectivize(name, configurator, attrs.get('options'),
+                                attrs.get('nics'), mtu=None, _netinfo=_netinfo,
+                                destroyOnMasterRemoval='remove' in attrs)
+        logger.debug("Creating bond %r with options %s", bond, bond.options)
+        configurator.configureBond(bond)
 
 
 def _buildBondOptions(bondName, bondings, _netinfo):


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I3ea081fe7760b52f6febb4959f69851b2cf4c2a1
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.3
Gerrit-Owner: Antoni Segura Puimedon <asegurap(a)redhat.com&gt;
Gerrit-Reviewer: Assaf Muller <amuller(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;