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 <[email protected]> --- 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 <[email protected]> _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
