Dan Kenigsberg has uploaded a new change for review.

Change subject: restore_nets: during rollback, ignore removal of a missing bond
......................................................................

restore_nets: during rollback, ignore removal of a missing bond

If a bond is removed outside vdsm, attempting to restore networks to the
last known-good set of configuration currently fails.

With this patch, if vdsm is asked to remove an already-missing bond
device during an attempt to restore networking, this descripency is
ignored, and restore_nets would go on attempting to restore further
networks, hopefully restoring connectivity to the host.

The accompanying test verifies that restore_nets survive after such
removal. It also verifies that adding a network on top of an
already-existing bond ends with the bonding device being consumed into
the running (and later, persistent) config.

Change-Id: I7b863ffd829026f9ea3030ac77e57cf6521fe6ba
Signed-off-by: Dan Kenigsberg <[email protected]>
Reviewed-on: https://gerrit.ovirt.org/38211
---
M tests/functional/networkTests.py
M vdsm/network/api.py
2 files changed, 49 insertions(+), 8 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/12/38312/1

diff --git a/tests/functional/networkTests.py b/tests/functional/networkTests.py
index ccf78cd..3b3f474 100644
--- a/tests/functional/networkTests.py
+++ b/tests/functional/networkTests.py
@@ -43,7 +43,7 @@
 from vdsm.constants import EXT_BRCTL
 from vdsm.utils import RollbackContext, execCmd
 from vdsm.netinfo import (bridges, operstate, prefix2netmask, getRouteDeviceTo,
-                          getDhclientIfaces, BONDING_SLAVES)
+                          getDhclientIfaces, BONDING_SLAVES, BONDING_MASTERS)
 from vdsm import ipwrapper
 from vdsm.utils import pgrep
 
@@ -2150,3 +2150,37 @@
         self.assertEqual(status, SUCCESS, msg)
         self.assertNetworkDoesntExist(NETWORK_NAME)
         self.assertBondDoesntExist(BONDING_NAME, nics)
+
+    @cleanupNet
+    @ValidateRunningAsRoot
+    def test_setupNetworks_on_external_bond(self):
+        with dummyIf(1) as (nic, ):
+            with open(BONDING_MASTERS, 'w') as bonds:
+                bonds.write('+%s\n' % BONDING_NAME)
+            try:
+                with open(BONDING_SLAVES % BONDING_NAME, 'w') as f:
+                    f.write('+%s\n' % nic)
+                status, msg = self.vdsm_net.setupNetworks(
+                    {NETWORK_NAME:
+                        {'bonding': BONDING_NAME, 'bridged': False}},
+                    {BONDING_NAME: {'nics': [nic]}}, NOCHK)
+                self.assertEqual(status, SUCCESS, msg)
+                self.assertNetworkExists(NETWORK_NAME)
+                self.assertBondExists(BONDING_NAME, [nic])
+            finally:
+                with open(BONDING_MASTERS, 'w') as bonds:
+                    bonds.write('-%s\n' % BONDING_NAME)
+
+            self.vdsm_net.save_config()
+            self.vdsm_net.restoreNetConfig()
+
+            self.assertNetworkExists(NETWORK_NAME)
+            self.assertBondExists(BONDING_NAME, [nic])
+
+            status, msg = self.vdsm_net.setupNetworks(
+                {NETWORK_NAME: {'remove': True}},
+                {BONDING_NAME: {'remove': True}}, NOCHK)
+            self.assertEqual(status, SUCCESS, msg)
+            self.assertNetworkDoesntExist(NETWORK_NAME)
+            self.assertBondDoesntExist(BONDING_NAME, [nic])
+            self.vdsm_net.save_config()
diff --git a/vdsm/network/api.py b/vdsm/network/api.py
index bc48ff8..62e3d0d 100755
--- a/vdsm/network/api.py
+++ b/vdsm/network/api.py
@@ -479,7 +479,6 @@
                 raise ConfigNetworkError(ne.ERR_BAD_PARAMS, 'Cannot specify '
                                          'any attribute when removing')
 
-    currentBondings = netinfo.bondings()
     currentNicsSet = set(netinfo.nics())
     for bonding, bondingAttrs in bondings.iteritems():
         Bond.validateName(bonding)
@@ -487,9 +486,6 @@
             Bond.validateOptions(bonding, bondingAttrs['options'])
 
         if bondingAttrs.get('remove', False):
-            if bonding not in currentBondings:
-                raise ConfigNetworkError(ne.ERR_BAD_BONDING, "Cannot remove "
-                                         "bonding %s: Doesn't exist" % bonding)
             continue
 
         nics = bondingAttrs.get('nics', None)
@@ -501,7 +497,7 @@
                                      "Unknown nics in: %r" % list(nics))
 
 
-def _handleBondings(bondings, configurator):
+def _handleBondings(bondings, configurator, in_rollback):
     """ Add/Edit/Remove bond interface """
     logger = logging.getLogger("_handleBondings")
 
@@ -511,6 +507,16 @@
     addition = []
     for name, attrs in bondings.items():
         if 'remove' in attrs:
+            if name not in _netinfo.bondings:
+                if in_rollback:
+                    logger.error(
+                        'Cannot remove bonding %s during rollback: '
+                        'does not exist', name)
+                    continue
+                else:
+                    raise ConfigNetworkError(
+                        ne.ERR_BAD_BONDING,
+                        "Cannot remove bonding %s: does not exist" % name)
             bond = Bond.objectivize(name, configurator, attrs.get('options'),
                                     attrs.get('nics'), mtu=None,
                                     _netinfo=_netinfo,
@@ -639,7 +645,8 @@
     options = results['request']['options']
 
     logger.debug("Applying...")
-    with ConfiguratorClass(options.get('_inRollback', False)) as configurator:
+    in_rollback = options.get('_inRollback', False)
+    with ConfiguratorClass(in_rollback) as configurator:
         # Remove edited networks and networks with 'remove' attribute
         for network, networkAttrs in networks.items():
             if network in _netinfo.networks:
@@ -669,7 +676,7 @@
             else:
                 networksAdded.add(network)
 
-        _handleBondings(bondings, configurator)
+        _handleBondings(bondings, configurator, in_rollback)
 
         # We need to use the newest host info
         _netinfo.updateDevices()


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I7b863ffd829026f9ea3030ac77e57cf6521fe6ba
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.5
Gerrit-Owner: Dan Kenigsberg <[email protected]>
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to