Antoni Segura Puimedon has posted comments on this change.
Change subject: Add iproute2 configurator
......................................................................
Patch Set 22: Code-Review-1
(5 comments)
Some minor things related to compatibility in error messages.
....................................................
File vdsm/netconf/iproute2.py
Line 32: from vdsm.netconfpersistence import RunningConfig
Line 33: from vdsm.utils import execCmd, CommandPath
Line 34:
Line 35:
Line 36: _BRCTL = CommandPath('brctl', '/usr/sbin/brctl')
The path is defined in constants.EXT_BRCTL
Line 37:
Line 38:
Line 39: class Iproute2(Configurator):
Line 40: def __init__(self, inRollback=False):
Line 77: self.runningConfig.setBonding(
Line 78: bond.name, {'options': bond.options,
Line 79: 'nics': [slave.name for slave in bond.slaves]})
Line 80:
Line 81: def editBonding(self, bond, _netinfo):
I would like to keep the same docstring as in ifcfg.py:
"""
Modifies the bond so that the bond in the system ends up with the
same slave and options configuration that are requested. Makes a
best effort not to interrupt connectivity.
"""
Line 82: nicsToSet = frozenset(nic.name for nic in bond.slaves)
Line 83: currentNics = frozenset(_netinfo.getNicsForBonding(bond.name))
Line 84: nicsToAdd = nicsToSet
Line 85: nicsToRemove = currentNics
Line 212:
Line 213: def addBridge(self, bridge):
Line 214: rc, _, err = execCmd([_BRCTL.cmd, 'addbr', bridge.name])
Line 215: if rc != 0:
Line 216: raise ConfigNetworkError(ERR_USED_BRIDGE, err)
I think the current neterror code that is closer to what happens here is:
ERR_FAILED_IFUP. In the future we should have more fine grained errors though.
Line 217:
Line 218: def addBridgePort(self, bridge):
Line 219: rc, _, err = execCmd([_BRCTL.cmd, 'addif', bridge.name,
Line 220: bridge.port.name])
Line 218: def addBridgePort(self, bridge):
Line 219: rc, _, err = execCmd([_BRCTL.cmd, 'addif', bridge.name,
Line 220: bridge.port.name])
Line 221: if rc != 0:
Line 222: raise ConfigNetworkError(ERR_USED_BRIDGE, err)
I think the current neterror code that is closer to what happens here is:
FAILED_IFUP. In the future we should have more fine grained errors though.
Line 223:
Line 224: def removeBridge(self, bridge):
Line 225: rc, _, err = execCmd([_BRCTL.cmd, 'delbr', bridge.name])
Line 226: if rc != 0:
Line 223:
Line 224: def removeBridge(self, bridge):
Line 225: rc, _, err = execCmd([_BRCTL.cmd, 'delbr', bridge.name])
Line 226: if rc != 0:
Line 227: raise ConfigNetworkError(ERR_USED_BRIDGE, err)
This doesn't raise right now with ifcfg. We simply swallow ifdown erroneous rc.
If we want to raise, and I tend to agree with it, we should add a
ERR_FAILED_IFDOWN
Line 228:
Line 229: def addVlan(self, vlan):
Line 230: ipwrapper.linkAdd(name=vlan.name, linkType='vlan',
Line 231: link=vlan.device.name, args=['id',
str(vlan.tag)])
--
To view, visit http://gerrit.ovirt.org/15301
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I762cfa30f78c5a46507b86f53e98bcf79dfc5844
Gerrit-PatchSet: 22
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu <wu...@linux.vnet.ibm.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegu...@redhat.com>
Gerrit-Reviewer: Assaf Muller <amul...@redhat.com>
Gerrit-Reviewer: Ayal Baron <aba...@redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com>
Gerrit-Reviewer: Itamar Heim <ih...@redhat.com>
Gerrit-Reviewer: Mark Wu <wu...@linux.vnet.ibm.com>
Gerrit-Reviewer: Petr Šebek <pse...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
