Change in vdsm[master]: Fix nics and bonds bridgeless rule removal

asegurap at redhat.com asegurap at redhat.com
Wed Jun 26 01:34:12 UTC 2013


Antoni Segura Puimedon has uploaded a new change for review.

Change subject: Fix nics and bonds bridgeless rule removal
......................................................................

Fix nics and bonds bridgeless rule removal

The current approach to remove the route-ethX and rule-ethX from
configNetwork can't work for bridgeless nics and bonds because,
to solve a bug, in the past, we modified the nic/bond "remove"
step so that it would leave the nics and bonds up after it is
done.

The above means that from configNetwork, where the current code
was integrating with SourceRoute and calling for routes and rules
removal, we can't really remove them because we have no sane way
to inject the route/rule files removal between the ifdown and the
ifup steps done in the ifcfg configurator.

Change-Id: I09aec8fa9a2ef2d61609a0a807f4e7130978fbfb
Signed-off-by: Antoni S. Puimedon <asegurap at redhat.com>
---
M vdsm/configNetwork.py
M vdsm/netconf/ifcfg.py
M vdsm/sourceRoute.py
3 files changed, 33 insertions(+), 25 deletions(-)


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

diff --git a/vdsm/configNetwork.py b/vdsm/configNetwork.py
index d54049e..218c081 100755
--- a/vdsm/configNetwork.py
+++ b/vdsm/configNetwork.py
@@ -38,7 +38,6 @@
 from netmodels import IpConfig
 from netmodels import Nic
 from netmodels import Vlan
-from sourceRoute import StaticSourceRoute
 
 CONNECTIVITY_TIMEOUT_DEFAULT = 4
 
@@ -199,12 +198,6 @@
                                 gateway, bootproto, _netinfo, configurator,
                                 **options)
 
-    # bootproto returns None for both static and no bootproto
-    if bootproto != 'dhcp':
-        logging.debug("Adding source route %s, %s, %s, %s" %
-                      (netEnt.name, ipaddr, netmask, gateway))
-        StaticSourceRoute(netEnt.name, configurator).\
-            configure(ipaddr, netmask, gateway)
     configurator.configureLibvirtNetwork(network, netEnt)
     netEnt.configure(**options)
 
@@ -278,13 +271,6 @@
         assertBridgeClean(network, vlan, bonding, nics)
 
 
-def _removeSourceRoute(netEnt, configurator):
-    _, _, _, bootproto, _ = netEnt.getIpConfig()
-    if bootproto != 'dhcp':
-        logging.debug("Removing source route for device %s" % netEnt.name)
-        StaticSourceRoute(netEnt.name, configurator).remove()
-
-
 def _delNonVdsmNetwork(network, vlan, bonding, nics, _netinfo, configurator):
     if network in netinfo.bridges():
         netEnt = objectivizeNetwork(bridge=network, vlan=vlan, bonding=bonding,
@@ -292,7 +278,6 @@
                                     configurator=configurator,
                                     implicitBonding=False)
         netEnt.remove()
-        _removeSourceRoute(netEnt, configurator)
     else:
         raise ConfigNetworkError(ne.ERR_BAD_BRIDGE, "Cannot delete network"
                                  " %r: It doesn't exist in the system" %
@@ -329,9 +314,6 @@
                                 configurator=configurator,
                                 implicitBonding=implicitBonding)
     netEnt.remove()
-    # Can't remove the static rules and routes before netEnt.remove. Otherwise
-    # the routes/rules files would not be there for ifdown.
-    _removeSourceRoute(netEnt, configurator)
     configurator.removeLibvirtNetwork(network)
 
     # We need to gather NetInfo again to refresh networks info from libvirt.
diff --git a/vdsm/netconf/ifcfg.py b/vdsm/netconf/ifcfg.py
index c9521eb..4ccc15d 100644
--- a/vdsm/netconf/ifcfg.py
+++ b/vdsm/netconf/ifcfg.py
@@ -29,12 +29,13 @@
 import threading
 
 from neterrors import ConfigNetworkError
-import libvirtCfg
+from netmodels import Nic, Bond, Bridge
+from sourceRoute import StaticSourceRoute
 from storage.misc import execCmd
 from vdsm import constants
 from vdsm import netinfo
 from vdsm import utils
-from netmodels import Nic, Bond, Bridge
+import libvirtCfg
 import neterrors as ne
 
 
@@ -73,6 +74,7 @@
         ifdown(bridge.name)
         if bridge.port:
             bridge.port.configure(bridge=bridge.name, **opts)
+        self._addSourceRoute(bridge, ipaddr, netmask, gateway, bootproto)
         ifup(bridge.name, async)
 
     def configureVlan(self, vlan, bridge=None, **opts):
@@ -81,6 +83,7 @@
                                   ipaddr=ipaddr, netmask=netmask,
                                   gateway=gateway, bootproto=bootproto, **opts)
         vlan.device.configure(**opts)
+        self._addSourceRoute(vlan, ipaddr, netmask, gateway, bootproto)
         ifup(vlan.name, async)
 
     def configureBond(self, bond, bridge=None, **opts):
@@ -95,6 +98,7 @@
                 ifdown(slave.name)
         for slave in bond.slaves:
             slave.configure(bonding=bond.name, **opts)
+        self._addSourceRoute(bond, ipaddr, netmask, gateway, bootproto)
         ifup(bond.name, async)
 
     def editBonding(self, bond, _netinfo):
@@ -110,6 +114,7 @@
                                  mtu=nic.mtu, ipaddr=ipaddr,
                                  netmask=netmask, gateway=gateway,
                                  bootproto=bootproto, **opts)
+        self._addSourceRoute(nic, ipaddr, netmask, gateway, bootproto)
         if not bonding:
             if not netinfo.isVlanned(nic.name):
                 ifdown(nic.name)
@@ -126,6 +131,7 @@
 
     def removeBridge(self, bridge):
         ifdown(bridge.name)
+        self._removeSourceRoute(bridge)
         execCmd([constants.EXT_BRCTL, 'delbr', bridge.name])
         self.configWriter.removeBridge(bridge.name)
         if bridge.port:
@@ -133,12 +139,14 @@
 
     def removeVlan(self, vlan):
         ifdown(vlan.name)
+        self._removeSourceRoute(vlan)
         self.configWriter.removeVlan(vlan.name)
         vlan.device.remove()
 
     def _ifaceDownAndCleanup(self, iface, _netinfo):
         """Returns True iff the iface is to be removed."""
         ifdown(iface.name)
+        self._removeSourceRoute(iface)
         self.configWriter.removeIfaceCleanup(iface.name)
         return not _netinfo.ifaceUsers(iface.name)
 
@@ -166,6 +174,20 @@
         else:
             self._setNewMtu(nic, _netinfo)
         ifup(nic.name)
+
+    def _addSourceRoute(self, netEnt, ipaddr, netmask, gateway, bootproto):
+        # bootproto is None for both static and no bootproto
+        if bootproto != 'dhcp' and netEnt.master is None:
+            logging.debug("Adding source route %s, %s, %s, %s" %
+                          (netEnt.name, ipaddr, netmask, gateway))
+            StaticSourceRoute(netEnt.name, self).\
+                configure(ipaddr, netmask, gateway)
+
+    def _removeSourceRoute(self, netEnt):
+        _, _, _, bootproto, _ = netEnt.getIpConfig()
+        if bootproto != 'dhcp' and netEnt.master is None:
+            logging.debug("Removing source route for device %s" % netEnt.name)
+            StaticSourceRoute(netEnt.name, self).remove()
 
     def _setNewMtu(self, iface, _netinfo):
         """
@@ -534,7 +556,7 @@
 
         if netinfo.NetInfo().ifaceUsers(bonding):
             confParams = netinfo.getIfaceCfg(bonding)
-            if not ipaddr:
+            if not ipaddr and bootproto != 'dhcp':
                 ipaddr = confParams.get('IPADDR', None)
                 netmask = confParams.get('NETMASK', None)
                 gateway = confParams.get('GATEWAY', None)
@@ -567,7 +589,7 @@
 
         if _netinfo.ifaceUsers(nic):
             confParams = netinfo.getIfaceCfg(nic)
-            if not ipaddr:
+            if not ipaddr and bootproto != 'dhcp':
                 ipaddr = confParams.get('IPADDR', None)
                 netmask = confParams.get('NETMASK', None)
                 gateway = confParams.get('GATEWAY', None)
diff --git a/vdsm/sourceRoute.py b/vdsm/sourceRoute.py
index 90741aa..03edad4 100644
--- a/vdsm/sourceRoute.py
+++ b/vdsm/sourceRoute.py
@@ -23,9 +23,6 @@
 import logging
 import netaddr
 
-from netconf.ifcfg import ConfigWriter
-from netconf.ifcfg import Ifcfg
-from netconf.iproute2 import Iproute2
 from vdsm import netinfo
 from vdsm.ipwrapper import Route
 from vdsm.ipwrapper import routeLinkNetForDevice
@@ -223,4 +220,11 @@
 
 
 if __name__ == "__main__":
+    # This imports are here due to the fact that we only need to create
+    # configurators if being used as a standalone script and because otherwise
+    # when importing SourceRoute from the configurators, we'd get a circular
+    # dependency.
+    from netconf.ifcfg import ConfigWriter
+    from netconf.ifcfg import Ifcfg
+    from netconf.iproute2 import Iproute2
     main()


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I09aec8fa9a2ef2d61609a0a807f4e7130978fbfb
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Antoni Segura Puimedon <asegurap at redhat.com>


More information about the vdsm-patches mailing list