Change in vdsm[master]: Add iproute2 configurator
Dan Kenigsberg has submitted this change and it was merged. Change subject: Add iproute2 configurator .. Add iproute2 configurator This patch adds a new host network configurator based on iproute2. We could just reuse the ifcfg's configurator totally and add a new ConfigWriter based on iproute2. But it turns out this approach is not very suitable to the iprtoute2 model. The ifcfg configurator depends on the behavior of ifup/ifdown, so it imposes some unnecessary restriction on the new ConfigWriter. So this patch chooses to add a new configurator and reuse some code of ifcfg configurator. In future, we could move those common code to a base class after refactoring ifcfg configurator. This patch dosen't include the support for dhcp and ipv6, which will be added in following patches. Change-Id: I762cfa30f78c5a46507b86f53e98bcf79dfc5844 Signed-off-by: Mark Wu Reviewed-on: http://gerrit.ovirt.org/15301 Reviewed-by: Antoni Segura Puimedon Reviewed-by: Dan Kenigsberg --- M lib/vdsm/define.py M vdsm/netconf/__init__.py M vdsm/netconf/ifcfg.py M vdsm/netconf/iproute2.py M vdsm/neterrors.py 5 files changed, 247 insertions(+), 8 deletions(-) Approvals: Antoni Segura Puimedon: Looks good to me, but someone else must approve Mark Wu: Verified Dan Kenigsberg: Looks good to me, approved -- To view, visit http://gerrit.ovirt.org/15301 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: I762cfa30f78c5a46507b86f53e98bcf79dfc5844 Gerrit-PatchSet: 26 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Itamar Heim Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Petr Šebek Gerrit-Reviewer: oVirt Jenkins CI Server ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Add iproute2 configurator
Dan Kenigsberg has posted comments on this change. Change subject: Add iproute2 configurator .. Patch Set 25: Code-Review+2 -- 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: 25 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Itamar Heim Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Petr Šebek Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Add iproute2 configurator
oVirt Jenkins CI Server has posted comments on this change. Change subject: Add iproute2 configurator .. Patch Set 25: Build Successful http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/6180/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_network_functional_tests/1045/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/5393/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/6284/ : SUCCESS -- 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: 25 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Itamar Heim Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Petr Šebek Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Add iproute2 configurator
Antoni Segura Puimedon has posted comments on this change. Change subject: Add iproute2 configurator .. Patch Set 25: Code-Review+1 -- 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: 25 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Itamar Heim Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Petr Šebek Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Add iproute2 configurator
Mark Wu has posted comments on this change. Change subject: Add iproute2 configurator .. Patch Set 25: Verified+1 -- 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: 25 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Itamar Heim Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Petr Šebek Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Add iproute2 configurator
Dan Kenigsberg has posted comments on this change. Change subject: Add iproute2 configurator .. Patch Set 24: Code-Review-1 (1 comment) File vdsm/neterrors.py Line 27: ERR_BAD_VLAN = 26 Line 28: ERR_BAD_BRIDGE = 27 Line 29: ERR_USED_BRIDGE = 28 Line 30: ERR_FAILED_IFUP = 29 Line 31: ERR_FAILED_IFDOWN = 30 Please reserve more errcodes in define.py. Line 32: ERR_LOST_CONNECTION = 10# noConPeer Line 33: Line 34: Line 35: class ConfigNetworkError(Exception): -- 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: 24 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Itamar Heim Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Petr Šebek Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Add iproute2 configurator
oVirt Jenkins CI Server has posted comments on this change. Change subject: Add iproute2 configurator .. Patch Set 24: Verified-1 Build Failed http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/6083/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_network_functional_tests/1030/ : FAILURE http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/5296/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/6186/ : SUCCESS -- 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: 24 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Itamar Heim Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Petr Šebek Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Add iproute2 configurator
Antoni Segura Puimedon has posted comments on this change. Change subject: Add iproute2 configurator .. Patch Set 24: Code-Review+1 -- 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: 24 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Itamar Heim Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Petr Šebek Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Add iproute2 configurator
oVirt Jenkins CI Server has posted comments on this change. Change subject: Add iproute2 configurator .. Patch Set 23: Verified-1 Build Failed http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/6080/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_network_functional_tests/1027/ : FAILURE http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/5293/ : FAILURE http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/6183/ : FAILURE -- 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: 23 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Itamar Heim Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Petr Šebek Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Add iproute2 configurator
Mark Wu has posted comments on this change. Change subject: Add iproute2 configurator .. Patch Set 24: Verified+1 -- 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: 24 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Itamar Heim Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Petr Šebek Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Add iproute2 configurator
Antoni Segura Puimedon has posted comments on this change. Change subject: Add iproute2 configurator .. Patch Set 22: For the rest, it looks very good to me. Thanks Mark! -- 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 Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Itamar Heim Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Petr Šebek Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Add iproute2 configurator
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
Gerrit-Reviewer: Antoni Segura Puimedon
Gerrit-Reviewer: Assaf Muller
Gerrit-Reviewer: Ayal Baron
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Itamar Heim
Gerrit-Reviewer: Mark Wu
Gerrit-Reviewer: Petr Šebek
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
Change in vdsm[master]: Add iproute2 configurator
oVirt Jenkins CI Server has posted comments on this change. Change subject: Add iproute2 configurator .. Patch Set 22: Build Successful http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/6026/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_network_functional_tests/998/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/5239/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/6128/ : SUCCESS -- 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 Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Itamar Heim Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Petr Šebek Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Add iproute2 configurator
Mark Wu has posted comments on this change. Change subject: Add iproute2 configurator .. Patch Set 21: Verified+1 -- 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: 21 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Itamar Heim Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Petr Šebek Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Add iproute2 configurator
oVirt Jenkins CI Server has posted comments on this change. Change subject: Add iproute2 configurator .. Patch Set 21: Verified-1 Build Failed http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5848/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_network_functional_tests/940/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5940/ : FAILURE http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/5052/ : SUCCESS -- 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: 21 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Itamar Heim Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Petr Šebek Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Add iproute2 configurator
Antoni Segura Puimedon has posted comments on this change. Change subject: Add iproute2 configurator .. Patch Set 20: Verified-1 (1 comment) Fails for when configurator is ifcgs (see comment). File vdsm/netconf/ifcfg.py Line 48: super(Ifcfg, self).__init__(ConfigWriter(), inRollback) Line 49: Line 50: def begin(self): Line 51: if self.configApplier is None: Line 52: self.configApplier = ConfigWriter() Since you removed the unified persistence attribute, this will raise an AttributeError. Line 53: if self.unifiedPersistence and self.runningConfig is None: Line 54: self.runningConfig = RunningConfig() Line 55: Line 56: def rollback(self): -- 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: 20 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Itamar Heim Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Petr Šebek 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
Change in vdsm[master]: Add iproute2 configurator
oVirt Jenkins CI Server has posted comments on this change. Change subject: Add iproute2 configurator .. Patch Set 20: Build Failed http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5799/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_network_functional_tests/923/ : FAILURE http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5895/ : FAILURE http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/5008/ : FAILURE -- 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: 20 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Itamar Heim Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Petr Šebek Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Add iproute2 configurator
oVirt Jenkins CI Server has posted comments on this change. Change subject: Add iproute2 configurator .. Patch Set 20: Build Failed http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/5005/ : To avoid overloading the infrastructure, a whitelist for running gerrit triggered jobs has been set in place, if you feel like you should be in it, please contact infra at ovirt dot org. http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5799/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_network_functional_tests/923/ : FAILURE http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5895/ : FAILURE -- 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: 20 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Itamar Heim Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Petr Šebek Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Add iproute2 configurator
oVirt Jenkins CI Server has posted comments on this change. Change subject: Add iproute2 configurator .. Patch Set 20: Verified-1 Build Failed http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4999/ : To avoid overloading the infrastructure, a whitelist for running gerrit triggered jobs has been set in place, if you feel like you should be in it, please contact infra at ovirt dot org. http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5799/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5889/ : To avoid overloading the infrastructure, a whitelist for running gerrit triggered jobs has been set in place, if you feel like you should be in it, please contact infra at ovirt dot org. http://jenkins.ovirt.org/job/vdsm_network_functional_tests/923/ : FAILURE -- 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: 20 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Itamar Heim Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Petr Šebek Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Add iproute2 configurator
oVirt Jenkins CI Server has posted comments on this change. Change subject: Add iproute2 configurator .. Patch Set 19: Verified-1 Build Failed http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4673/ : To avoid overloading the infrastructure, a whitelist for running gerrit triggered jobs has been set in place, if you feel like you should be in it, please contact infra at ovirt dot org. http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5473/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_network_functional_tests/811/ : FAILURE http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5552/ : To avoid overloading the infrastructure, a whitelist for running gerrit triggered jobs has been set in place, if you feel like you should be in it, please contact infra at ovirt dot org. -- 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: 19 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Itamar Heim Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Petr Šebek Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Add iproute2 configurator
Mark Wu has posted comments on this change. Change subject: Add iproute2 configurator .. Patch Set 19: It can pass all the tests except followings: testAddVlanedBridgeless testAddVlanedBridgeless_oneCommand testIPv6ConfigNetwork testStaticSourceRouting(kwargs=False) testStaticSourceRouting(kwargs=True) The first two is caused by getIfaceCfg doesn't work with unified persistence. The third one is caused the lack of ipv6 support staticRouting support is not included this patch either. -- 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: 19 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Itamar Heim Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Petr Šebek Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Add iproute2 configurator
Mark Wu has posted comments on this change. Change subject: Add iproute2 configurator .. Patch Set 18: Sorry for no update on this patch in a long period. Actually, this patch is pending on two problems: 1, a race problem in the unified persistence patch merged in a few days before. Antoni and I are working on it now. 2. The live rollback support for unified persistence (http://gerrit.ovirt.org/#/c/20032/). I get a offline comment on patch from Antoni: using imp.load instead of pickle. I am going to update that patch in a couple of days. -- 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: 18 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Itamar Heim Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Petr Šebek Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Add iproute2 configurator
Itamar Heim has posted comments on this change. Change subject: Add iproute2 configurator .. Patch Set 18: ping? -- 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: 18 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Itamar Heim Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Petr Šebek Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Add iproute2 configurator
Mark Wu has posted comments on this change. Change subject: Add iproute2 configurator .. Patch Set 18: (1 comment) File vdsm/netconf/iproute2.py Line 121: if toBeRemoved: Line 122: if iface.master is None: Line 123: self.configApplier.removeIpConfig(iface) Line 124: Line 125: if destroy: Done Line 126: destroyAction(iface) Line 127: else: Line 128: self.configApplier.setIfaceMtu(iface.name, Line 129:netinfo.DEFAULT_MTU) -- 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: 18 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Petr Šebek Gerrit-Reviewer: Saggi Mizrahi 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
Change in vdsm[master]: Add iproute2 configurator
Dan Kenigsberg has posted comments on this change. Change subject: Add iproute2 configurator .. Patch Set 18: Code-Review-1 (1 comment) -1 mostly for visibility. File vdsm/netconf/iproute2.py Line 121: if toBeRemoved: Line 122: if iface.master is None: Line 123: self.configApplier.removeIpConfig(iface) Line 124: Line 125: if destroy: I share Ayal's opinion about the semantics of this function, which is too complex, particularly when you note that the it has only two usages. I cannot describe in few English words what this function does. So maybe we should suffer the partial code repetition, and inline the function into remoteBond() and removeNic(). Line 126: destroyAction(iface) Line 127: else: Line 128: self.configApplier.setIfaceMtu(iface.name, Line 129:netinfo.DEFAULT_MTU) -- 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: 18 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Petr Šebek Gerrit-Reviewer: Saggi Mizrahi 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
Change in vdsm[master]: Add iproute2 configurator
Mark Wu has posted comments on this change.
Change subject: Add iproute2 configurator
..
Patch Set 18:
(4 comments)
File vdsm/netconf/iproute2.py
Line 39: def __init__(self):
Line 40: super(Iproute2, self).__init__(ConfigApplier())
Line 41:
Line 42: def begin(self):
Line 43: # To be implemented in a following patch.
Currently, the network configuration code is executed in a context manager
implemented in class Configurator itself. It aims to implement the transaction
management. Please see Configurator.__enter__() in netconf/__init__.py
The transaction management of iproute2 depends on unified persistentence
(http://gerrit.ovirt.org/#/c/16699/)
That's why I leave the API here. I will rebase on the series of unified
persistence patches or resolve it in a separate patch.
Line 44: pass
Line 45:
Line 46: def commit(self):
Line 47: # To be implemented in a following patch.
Line 81: if bond.areOptionsApplied():
Line 82: nicsToAdd = nicsToSet - currentNics
Line 83: nicsToRemove = currentNics - nicsToSet
Line 84: else:
Line 85: nicsToAdd = nicsToSet
Done
Line 86: nicsToRemove = currentNics
Line 87:
Line 88: for nic in nicsToRemove:
Line 89: slave = Nic(nic, self, _netinfo=_netinfo)
Line 121: if toBeRemoved:
Line 122: if iface.master is None:
Line 123: self.configApplier.removeIpConfig(iface)
Line 124:
Line 125: if destroy:
it's not a public API, but an internal function in Iproute2 class. It's used by
the API removeBond and removeNic. Do you still think it could lead bugs?
Line 126: destroyAction(iface)
Line 127: else:
Line 128: self.configApplier.setIfaceMtu(iface.name,
Line 129:netinfo.DEFAULT_MTU)
Line 217:
Line 218: def removeVlan(self, vlan):
Line 219: ipwrapper.linkDel(vlan.name)
Line 220:
Line 221: def addBond(self, bond):
Yes, I agree with your comments. I aggregate them under a class just to make
the code organized the same as ifcfg configurator. Then the two configurators
can share the common code as much as possible.
Line 222: if bond.name not in netinfo.bondings():
Line 223: logging.debug('Add new bonding %s', bond)
Line 224: with open(netinfo.BONDING_MASTERS, 'w') as f:
Line 225: f.write('+%s' % bond.name)
--
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: 18
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu
Gerrit-Reviewer: Antoni Segura Puimedon
Gerrit-Reviewer: Assaf Muller
Gerrit-Reviewer: Ayal Baron
Gerrit-Reviewer: Mark Wu
Gerrit-Reviewer: Petr Šebek
Gerrit-Reviewer: Saggi Mizrahi
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
Change in vdsm[master]: Add iproute2 configurator
Ayal Baron has posted comments on this change.
Change subject: Add iproute2 configurator
..
Patch Set 18:
(4 comments)
File vdsm/netconf/iproute2.py
Line 39: def __init__(self):
Line 40: super(Iproute2, self).__init__(ConfigApplier())
Line 41:
Line 42: def begin(self):
Line 43: # To be implemented in a following patch.
why not introduce the API where it's used? (i.e. in the subsequent patch)
Line 44: pass
Line 45:
Line 46: def commit(self):
Line 47: # To be implemented in a following patch.
Line 81: if bond.areOptionsApplied():
Line 82: nicsToAdd = nicsToSet - currentNics
Line 83: nicsToRemove = currentNics - nicsToSet
Line 84: else:
Line 85: nicsToAdd = nicsToSet
nit:
you could move these 2 lines above the 'if' and then if optionsApplied:
nicsToAdd -= currentNics...
Line 86: nicsToRemove = currentNics
Line 87:
Line 88: for nic in nicsToRemove:
Line 89: slave = Nic(nic, self, _netinfo=_netinfo)
Line 121: if toBeRemoved:
Line 122: if iface.master is None:
Line 123: self.configApplier.removeIpConfig(iface)
Line 124:
Line 125: if destroy:
now the API has parameters (destroy, destroyAction) which are only honoured if
this nic has no users, otherwise these options are ignored.
imo this will lead to bugs
Line 126: destroyAction(iface)
Line 127: else:
Line 128: self.configApplier.setIfaceMtu(iface.name,
Line 129:netinfo.DEFAULT_MTU)
Line 217:
Line 218: def removeVlan(self, vlan):
Line 219: ipwrapper.linkDel(vlan.name)
Line 220:
Line 221: def addBond(self, bond):
all of these methods are static, why are they aggregated under a class? (even
the name of the class suggests that the aggregation is arbitrary)
Line 222: if bond.name not in netinfo.bondings():
Line 223: logging.debug('Add new bonding %s', bond)
Line 224: with open(netinfo.BONDING_MASTERS, 'w') as f:
Line 225: f.write('+%s' % bond.name)
--
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: 18
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu
Gerrit-Reviewer: Antoni Segura Puimedon
Gerrit-Reviewer: Assaf Muller
Gerrit-Reviewer: Ayal Baron
Gerrit-Reviewer: Mark Wu
Gerrit-Reviewer: Petr Šebek
Gerrit-Reviewer: Saggi Mizrahi
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
Change in vdsm[master]: Add iproute2 configurator
oVirt Jenkins CI Server has posted comments on this change. Change subject: Add iproute2 configurator .. Patch Set 18: Build Successful http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/4311/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/3414/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/4230/ : SUCCESS -- 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: 18 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Petr Šebek Gerrit-Reviewer: Saggi Mizrahi Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Add iproute2 configurator
Mark Wu has posted comments on this change. Change subject: Add iproute2 configurator .. Patch Set 18: Verified+1 -- 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: 18 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Petr Šebek Gerrit-Reviewer: Saggi Mizrahi Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Add iproute2 configurator
Mark Wu has posted comments on this change. Change subject: Add iproute2 configurator .. Patch Set 17: (7 comments) Ayal, Thanks a lot for the insightful review! Some problems doesn't exist in the patch 16, which should be the latest change, and the other problems will be fixed in next change. File vdsm/netconf/iproute2.py Line 84: @contextmanager Line 85: def _removeIface(self, iface): Line 86: _netinfo = netinfo.NetInfo() Line 87: toBeRemoved = not _netinfo.ifaceUsers(iface.name) Line 88: destroyed = yield not toBeRemoved this problem was already fixed in patch 16. Actually, patch 16 is the latest change of this patch. It's overrode by an old change unintentionally when Assaf updated his patch which is based on this one. Line 89: Line 90: if toBeRemoved: Line 91: if iface.master is None: Line 92: self.configApplier.removeIpConfig(iface) Line 90: if toBeRemoved: Line 91: if iface.master is None: Line 92: self.configApplier.removeIpConfig(iface) Line 93: Line 94: if not destroyed: Done Line 95: self.configApplier.setIfaceMtu(iface.name, Line 96:netinfo.DEFAULT_MTU) Line 97: self.configApplier.ifdown(iface) Line 98: else: Line 105: self.configApplier.removeBond(bonding) Line 106: for slave in bonding.slaves: Line 107: self.configApplier.removeBondSlave(bonding, slave) Line 108: slave.remove() Line 109: return True Done Line 110: Line 111: def removeNic(self, nic): Line 112: with self._removeIface(nic): Line 113: return False Line 140: ipConfig.netmask) Line 141: if ipConfig.gateway and ipConfig.defaultRoute: Line 142: ipwrapper.routeAdd(['default', 'via', ipConfig.gateway]) Line 143: except ipwrapper.IPRoute2Error as e: Line 144: if 'File exists' in e.message[0]: Done Line 145: pass Line 146: else: Line 147: raise Line 148: Line 149: def removeIpConfig(self, iface): Line 150: ipwrapper.addrFlush(iface.name) Line 151: Line 152: def setIfaceMtu(self, iface, mtu): Line 153: if mtu: in former implementation, setIfaceMtu use an instance of NetDevice as argument, and it just use iface.mtu as the mtu value if not extra mtu passed it. But we just use a plain str to represent the interface, so it doesn't make sense to call it without mtu. I am going to remove the statement 'if mtu:', the validation is already done in the netmodels layer. Line 154: ipwrapper.linkSet(iface, ['mtu', str(mtu)]) Line 155: Line 156: def setBondingMtu(self, iface, mtu): Line 157: self.setIfaceMtu(iface, mtu) Line 168: self.setIfaceMtu(iface.name, iface.mtu) Line 169: self.ifup(iface) Line 170: Line 171: def addBridge(self, bridge): Line 172: execCmd(['brctl', 'addbr', bridge.name]) Done Line 173: Line 174: def addBridgePort(self, bridge): Line 175: execCmd(['brctl', 'addif', bridge.name, bridge.port.name]) Line 176: Line 171: def addBridge(self, bridge): Line 172: execCmd(['brctl', 'addbr', bridge.name]) Line 173: Line 174: def addBridgePort(self, bridge): Line 175: execCmd(['brctl', 'addif', bridge.name, bridge.port.name]) Done Line 176: Line 177: def removeBridge(self, bridge): Line 178: execCmd([constants.EXT_BRCTL, 'delbr', bridge.name]) Line 179: -- 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: 17 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Petr Šebek Gerrit-Reviewer: Saggi Mizrahi 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
Change in vdsm[master]: Add iproute2 configurator
Ayal Baron has posted comments on this change. Change subject: Add iproute2 configurator .. Patch Set 17: Code-Review-1 (8 comments) Partial review, but thought it's worth to publish already since I don't have time to go on at the moment. File vdsm/netconf/iproute2.py Line 84: @contextmanager Line 85: def _removeIface(self, iface): Line 86: _netinfo = netinfo.NetInfo() Line 87: toBeRemoved = not _netinfo.ifaceUsers(iface.name) Line 88: destroyed = yield not toBeRemoved I may be missing something here, but I don't see how destroyed can be anything but 'None'. The only reason I can see to do such an assignment is if you expect to get the value of toBeRemoved back from caller, but I do not believe GeneratorContextManager supports that and regardless, from the usages you're not doing so. If this is simply meant to be 'not toBeRemoved' then it's redundant (and you don't need 2 booleans) Line 89: Line 90: if toBeRemoved: Line 91: if iface.master is None: Line 92: self.configApplier.removeIpConfig(iface) Line 90: if toBeRemoved: Line 91: if iface.master is None: Line 92: self.configApplier.removeIpConfig(iface) Line 93: Line 94: if not destroyed: Again, unless this is meant to be passed back in, then it's equal to not not toBeRemoved == toBeRemoved and we're already under 'if toBeRemoved' Line 95: self.configApplier.setIfaceMtu(iface.name, Line 96:netinfo.DEFAULT_MTU) Line 97: self.configApplier.ifdown(iface) Line 98: else: Line 105: self.configApplier.removeBond(bonding) Line 106: for slave in bonding.slaves: Line 107: self.configApplier.removeBondSlave(bonding, slave) Line 108: slave.remove() Line 109: return True return here is for removeBond, it does not propagate back into _removeIface Line 110: Line 111: def removeNic(self, nic): Line 112: with self._removeIface(nic): Line 113: return False Line 140: ipConfig.netmask) Line 141: if ipConfig.gateway and ipConfig.defaultRoute: Line 142: ipwrapper.routeAdd(['default', 'via', ipConfig.gateway]) Line 143: except ipwrapper.IPRoute2Error as e: Line 144: if 'File exists' in e.message[0]: same (EEXIST) Line 145: pass Line 146: else: Line 147: raise Line 148: Line 149: def removeIpConfig(self, iface): Line 150: ipwrapper.addrFlush(iface.name) Line 151: Line 152: def setIfaceMtu(self, iface, mtu): Line 153: if mtu: if not mtu: raise ValueError... what's the point of calling setIfaceMtu and not passing mtu? Line 154: ipwrapper.linkSet(iface, ['mtu', str(mtu)]) Line 155: Line 156: def setBondingMtu(self, iface, mtu): Line 157: self.setIfaceMtu(iface, mtu) Line 168: self.setIfaceMtu(iface.name, iface.mtu) Line 169: self.ifup(iface) Line 170: Line 171: def addBridge(self, bridge): Line 172: execCmd(['brctl', 'addbr', bridge.name]) Please wrap execCmd for brctl and handle brctl specific exceptions. Line 173: Line 174: def addBridgePort(self, bridge): Line 175: execCmd(['brctl', 'addif', bridge.name, bridge.port.name]) Line 176: Line 171: def addBridge(self, bridge): Line 172: execCmd(['brctl', 'addbr', bridge.name]) Line 173: Line 174: def addBridgePort(self, bridge): Line 175: execCmd(['brctl', 'addif', bridge.name, bridge.port.name]) your use of brctl and constants.EXT_BRCTL should be consistent (i.e. just the constant) Line 176: Line 177: def removeBridge(self, bridge): Line 178: execCmd([constants.EXT_BRCTL, 'delbr', bridge.name]) Line 179: Line 181: try: Line 182: ipwrapper.linkAdd(link=vlan.device.name, name=vlan.name, Line 183: type='vlan', linkArgs=['id', vlan.tag]) Line 184: except ipwrapper.IPRoute2Error as e: Line 185: if 'File exists' in e.message[0]: please see in fileUtils how to check for EEXIST. If ipwrapper doesn't throw the exception properly then that too should be fixed Line 186: pass Line 187: else: Line 188: raise Line 189: -- 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: 17 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Ayal Ba
Change in vdsm[master]: Add iproute2 configurator
oVirt Jenkins CI Server has posted comments on this change. Change subject: Add iproute2 configurator .. Patch Set 17: Build Successful http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/4174/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/3279/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/4095/ : SUCCESS -- 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: 17 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Petr Šebek Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Add iproute2 configurator
Assaf Muller has posted comments on this change. Change subject: Add iproute2 configurator .. Patch Set 17: I took out the new ipwrapper functions to another patch and rebased the iproute2 series on that patch. -- 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: 17 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Petr Šebek Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Add iproute2 configurator
oVirt Jenkins CI Server has posted comments on this change. Change subject: Add iproute2 configurator .. Patch Set 16: Build Successful http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/4082/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/3187/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/4003/ : SUCCESS -- 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: 16 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Petr Šebek Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Add iproute2 configurator
Mark Wu has posted comments on this change. Change subject: Add iproute2 configurator .. Patch Set 16: it can pass all network functional tests. -- 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: 16 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Petr Šebek Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Add iproute2 configurator
oVirt Jenkins CI Server has posted comments on this change. Change subject: Add iproute2 configurator .. Patch Set 15: Build Successful http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/4076/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/3181/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/3997/ : SUCCESS -- 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: 15 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Petr Šebek Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Add iproute2 configurator
oVirt Jenkins CI Server has posted comments on this change. Change subject: Add iproute2 configurator .. Patch Set 14: Build Successful http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/4072/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/3177/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/3993/ : SUCCESS -- 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: 14 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Petr Šebek Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Add iproute2 configurator
oVirt Jenkins CI Server has posted comments on this change. Change subject: Add iproute2 configurator .. Patch Set 13: Build Successful http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/4067/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/3172/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/3988/ : SUCCESS -- 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: 13 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Petr Šebek Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Add iproute2 configurator
oVirt Jenkins CI Server has posted comments on this change. Change subject: Add iproute2 configurator .. Patch Set 12: Build Successful http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/4023/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/3134/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/3941/ : SUCCESS -- 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: 12 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Petr Šebek Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Add iproute2 configurator
Assaf Muller has posted comments on this change. Change subject: Add iproute2 configurator .. Patch Set 12: I pushed a small change to make ipLinkAdd a bit more generic so it suits more needs. -- 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: 12 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Petr Šebek Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Add iproute2 configurator
oVirt Jenkins CI Server has posted comments on this change. Change subject: Add iproute2 configurator .. Patch Set 11: Build Successful http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/3127/ : SUCCESS -- 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: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Petr Šebek Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Add iproute2 configurator
oVirt Jenkins CI Server has posted comments on this change. Change subject: Add iproute2 configurator .. Patch Set 10: Verified-1 Build Failed http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/3126/ : ABORTED -- 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: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Petr Šebek Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Add iproute2 configurator
Assaf Muller has posted comments on this change. Change subject: Add iproute2 configurator .. Patch Set 11: Code-Review-1 (1 comment) File lib/vdsm/ipwrapper.py Line 259: def addrFlush(dev): Line 260: command = [_IP_BINARY.cmd, 'addr', 'flush', 'dev', dev] Line 261: _execCmd(command) Line 262: Line 263: I'm rebasing a functional test patch on this patch, and I need the ability to run this command: ip link add 'dummy_50' type dummy The 'link slave' and 'name dev' parts should be optional to allow this use case. Otherwise this is a very welcome change :) Line 264: def linkAdd(dev, slave, linkType, linkArgs): Line 265: command = [_IP_BINARY.cmd, 'link', 'add', 'link', slave, 'name', dev, Line 266:'type', linkType] Line 267: command += linkArgs -- 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: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Petr Šebek 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
Change in vdsm[master]: Add iproute2 configurator
oVirt Jenkins CI Server has posted comments on this change. Change subject: Add iproute2 configurator .. Patch Set 9: Build Successful http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/4013/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/3124/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/3931/ : SUCCESS -- 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: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Petr Šebek Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Add iproute2 configurator
Assaf Muller has posted comments on this change. Change subject: Add iproute2 configurator .. Patch Set 11: Ok sorry Mark - Patch set 11 is identical to 9. -- 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: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Petr Šebek Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Add iproute2 configurator
Assaf Muller has posted comments on this change. Change subject: Add iproute2 configurator .. Patch Set 10: Hah, well, that's a shame - You were working on patchset 9 exactly when I rebased a series of patches on this patch, and the rebase pushed patchset 10, revoking all of the changes in patchset 9. I'll revert patchset 10. -- 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: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Petr Šebek Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Add iproute2 configurator
Mark Wu has posted comments on this change. Change subject: Add iproute2 configurator .. Patch Set 8: (7 comments) Thanks a lot for the review! File lib/vdsm/ipwrapper.py Line 249: command += rule Line 250: _execCmd(command) Line 251: Line 252: Line 253: def addrAdd(addr): Done Line 254: command = [_IP_BINARY.cmd, 'addr', 'add'] + addr Line 255: _execCmd(command) Line 256: Line 257: Line 254: command = [_IP_BINARY.cmd, 'addr', 'add'] + addr Line 255: _execCmd(command) Line 256: Line 257: Line 258: def addrFlush(addr): Done Line 259: command = [_IP_BINARY.cmd, 'addr', 'flush'] + addr Line 260: _execCmd(command) Line 261: Line 262: Line 259: command = [_IP_BINARY.cmd, 'addr', 'flush'] + addr Line 260: _execCmd(command) Line 261: Line 262: Line 263: def linkAdd(link): Done Line 264: command = [_IP_BINARY.cmd, 'link', 'add'] + link Line 265: _execCmd(command) Line 266: Line 267: Line 264: command = [_IP_BINARY.cmd, 'link', 'add'] + link Line 265: _execCmd(command) Line 266: Line 267: Line 268: def linkSet(link): Done Line 269: command = [_IP_BINARY.cmd, 'link', 'set'] + link Line 270: _execCmd(command) Line 271: Line 272: Line 269: command = [_IP_BINARY.cmd, 'link', 'set'] + link Line 270: _execCmd(command) Line 271: Line 272: Line 273: def linkDel(link): Done Line 274: command = [_IP_BINARY.cmd, 'link', 'del'] + link File vdsm/netconf/iproute2.py Line 56: slave.configure(**opts) Line 57: Line 58: self.configApplier.setIfaceConfigAndUp(bond) Line 59: Line 60: def editBonding(self, bond, _netinfo): thanks! I also noticed this problem and planned to fix it after your patch is merged. Line 61: self.configApplier.ifdown(bond) Line 62: for nic in _netinfo.getNicsForBonding(bond.name): Line 63: slave = Nic(nic, self, _netinfo=_netinfo) Line 64: self.configApplier.removeBondSlave(bond, slave) Line 181: try: Line 182: ipwrapper.linkAdd(['link', vlan.device.name, 'name', vlan.name, Line 183:'type', 'vlan', 'id', vlan.tag]) Line 184: except ipwrapper.IPRoute2Error as e: Line 185: if 'File exists' in e.message[0]: if for some reason, the vlan device is still left on system after the network over it is remove. Then we can't add network over it again. That could make more robust. I agree with that it only helps for the case caused by the defect of our code. But I think it's harmless also because it doesn't swallow the exception of the real issue. Line 186: pass Line 187: else: Line 188: raise Line 189: -- 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: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Petr Šebek 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
Change in vdsm[master]: Add iproute2 configurator
Antoni Segura Puimedon has posted comments on this change. Change subject: Add iproute2 configurator .. Patch Set 8: Code-Review-1 (7 comments) -1 for visibility of the comments. File lib/vdsm/ipwrapper.py Line 249: command += rule Line 250: _execCmd(command) Line 251: Line 252: Line 253: def addrAdd(addr): We could add a first parameter 'dev'. Line 254: command = [_IP_BINARY.cmd, 'addr', 'add'] + addr Line 255: _execCmd(command) Line 256: Line 257: Line 254: command = [_IP_BINARY.cmd, 'addr', 'add'] + addr Line 255: _execCmd(command) Line 256: Line 257: Line 258: def addrFlush(addr): We could add a first parameter 'dev'. Line 259: command = [_IP_BINARY.cmd, 'addr', 'flush'] + addr Line 260: _execCmd(command) Line 261: Line 262: Line 259: command = [_IP_BINARY.cmd, 'addr', 'flush'] + addr Line 260: _execCmd(command) Line 261: Line 262: Line 263: def linkAdd(link): We could add a first parameter 'dev'. Line 264: command = [_IP_BINARY.cmd, 'link', 'add'] + link Line 265: _execCmd(command) Line 266: Line 267: Line 264: command = [_IP_BINARY.cmd, 'link', 'add'] + link Line 265: _execCmd(command) Line 266: Line 267: Line 268: def linkSet(link): We could add a first parameter 'dev'. Line 269: command = [_IP_BINARY.cmd, 'link', 'set'] + link Line 270: _execCmd(command) Line 271: Line 272: Line 269: command = [_IP_BINARY.cmd, 'link', 'set'] + link Line 270: _execCmd(command) Line 271: Line 272: Line 273: def linkDel(link): We could add a first parameter 'dev'. Line 274: command = [_IP_BINARY.cmd, 'link', 'del'] + link File vdsm/netconf/iproute2.py Line 56: slave.configure(**opts) Line 57: Line 58: self.configApplier.setIfaceConfigAndUp(bond) Line 59: Line 60: def editBonding(self, bond, _netinfo): After the editbondings patch I made I'll make a followup patch for this. Line 61: self.configApplier.ifdown(bond) Line 62: for nic in _netinfo.getNicsForBonding(bond.name): Line 63: slave = Nic(nic, self, _netinfo=_netinfo) Line 64: self.configApplier.removeBondSlave(bond, slave) Line 181: try: Line 182: ipwrapper.linkAdd(['link', vlan.device.name, 'name', vlan.name, Line 183:'type', 'vlan', 'id', vlan.tag]) Line 184: except ipwrapper.IPRoute2Error as e: Line 185: if 'File exists' in e.message[0]: Does this happen often? It could be a sign of us needlessly reconfiguring. Line 186: pass Line 187: else: Line 188: raise Line 189: -- 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: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Petr Šebek 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
Change in vdsm[master]: Add iproute2 configurator
oVirt Jenkins CI Server has posted comments on this change. Change subject: Add iproute2 configurator .. Patch Set 8: Build Successful http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/3977/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/3894/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/3088/ : SUCCESS -- 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: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Petr Šebek Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Add iproute2 configurator
oVirt Jenkins CI Server has posted comments on this change. Change subject: Add iproute2 configurator .. Patch Set 7: Build Successful http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/3976/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/3893/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/3087/ : SUCCESS -- 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: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Petr Šebek Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Add iproute2 configurator
Mark Wu has posted comments on this change.
Change subject: Add iproute2 configurator
..
Patch Set 7:
(2 comments)
File lib/vdsm/netinfo.py
Line 46: PROC_NET_VLAN = '/proc/net/vlan/'
Line 47: NET_PATH = '/sys/class/net'
Line 48: BONDING_MASTERS = '/sys/class/net/bonding_masters'
Line 49: BONDING_SLAVES = '/sys/class/net/%s/bonding/slaves'
Line 50: BONDING_OPTION = '/sys/class/net/{bond}/bonding/{option}'
need rebase on http://gerrit.ovirt.org/#/c/17491/, which defines BONDING_OPTIONS
Line 51:
Line 52: LIBVIRT_NET_PREFIX = 'vdsm-'
Line 53: DUMMY_BRIDGE = ';vdsmdummy;'
Line 54: DEFAULT_MTU = '1500'
File vdsm/netconf/iproute2.py
Line 58:
Line 59: self.configApplier.setIfaceConfigAndUp(bond)
Line 60:
Line 61: def editBonding(self, bond, _netinfo):
Line 62: self.configApplier.ifdown(bond)
The 'ifdown/ifup' issue will be fixed in a following up patch based on:
http://gerrit.ovirt.org/#/c/17491/ Don't down bonds and nics when adding vlan
or resizing
Line 63: for nic in _netinfo.getNicsForBonding(bond.name):
Line 64: slave = Nic(nic, self, _netinfo=_netinfo)
Line 65: self.configApplier.removeBondSlave(bond, slave)
Line 66: slave.remove()
--
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: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu
Gerrit-Reviewer: Antoni Segura Puimedon
Gerrit-Reviewer: Mark Wu
Gerrit-Reviewer: Petr Šebek
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
Change in vdsm[master]: Add iproute2 configurator
Mark Wu has posted comments on this change.
Change subject: Add iproute2 configurator
..
Patch Set 6:
(2 comments)
File lib/vdsm/netinfo.py
Line 46: PROC_NET_VLAN = '/proc/net/vlan/'
Line 47: NET_PATH = '/sys/class/net'
Line 48: BONDING_MASTERS = '/sys/class/net/bonding_masters'
Line 49: BONDING_SLAVES = '/sys/class/net/%s/bonding/slaves'
Line 50: BONDING_OPTION = '/sys/class/net/{bond}/bonding/{option}'
need rebase on http://gerrit.ovirt.org/#/c/17491/, which defines BONDING_OPTIONS
Line 51:
Line 52: LIBVIRT_NET_PREFIX = 'vdsm-'
Line 53: DUMMY_BRIDGE = ';vdsmdummy;'
Line 54: DEFAULT_MTU = '1500'
File vdsm/netconf/iproute2.py
Line 57: slave.configure(**opts)
Line 58:
Line 59: self.configApplier.setIfaceConfigAndUp(bond)
Line 60:
Line 61: def editBonding(self, bond, _netinfo):
The 'ifdown/ifup' issue will be fixed in a following up patch based on:
http://gerrit.ovirt.org/#/c/17491/
Don't down bonds and nics when adding vlan or resizing
Line 62: self.configApplier.ifdown(bond)
Line 63: for nic in _netinfo.getNicsForBonding(bond.name):
Line 64: slave = Nic(nic, self, _netinfo=_netinfo)
Line 65: self.configApplier.removeBondSlave(bond, slave)
--
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: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu
Gerrit-Reviewer: Antoni Segura Puimedon
Gerrit-Reviewer: Mark Wu
Gerrit-Reviewer: Petr Šebek
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
Change in vdsm[master]: Add iproute2 configurator
oVirt Jenkins CI Server has posted comments on this change. Change subject: Add iproute2 configurator .. Patch Set 6: Build Successful http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/3974/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/3891/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/3085/ : SUCCESS -- 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: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Petr Šebek Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Add iproute2 configurator
Mark Wu has posted comments on this change. Change subject: Add iproute2 configurator .. Patch Set 5: (1 comment) File vdsm/netconf/iproute2.py Line 34: Line 35: class Iproute2(Configurator): Line 36: def __init__(self): Line 37: self.configApplier = ConfigApplier() Line 38: self._libvirtAdded = set() Yes, you're right. But this patch is based on the patches of unified persistence. The change of Configurator.__init__ is not included. I am going to rebase the unified persistence patch. Line 39: Line 40: def configureBridge(self, bridge, **opts): Line 41: # Todo: check the stp option. Line 42: execCmd(['brctl', 'addbr', bridge.name]) -- 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: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Petr Šebek 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
Change in vdsm[master]: Add iproute2 configurator
Antoni Segura Puimedon has posted comments on this change. Change subject: Add iproute2 configurator .. Patch Set 5: Code-Review-1 (1 comment) File vdsm/netconf/iproute2.py Line 34: Line 35: class Iproute2(Configurator): Line 36: def __init__(self): Line 37: self.configApplier = ConfigApplier() Line 38: self._libvirtAdded = set() I believe we should call super's init here. Line 39: Line 40: def configureBridge(self, bridge, **opts): Line 41: # Todo: check the stp option. Line 42: execCmd(['brctl', 'addbr', bridge.name]) -- 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: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Petr Šebek 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
Change in vdsm[master]: Add iproute2 configurator
oVirt Jenkins CI Server has posted comments on this change. Change subject: Add iproute2 configurator .. Patch Set 5: Verified-1 Build Failed http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/3703/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/3619/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/2812/ : FAILURE -- 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: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Petr Šebek Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Add iproute2 configurator
Petr Šebek has posted comments on this change. Change subject: Add iproute2 configurator .. Patch Set 4: (1 inline comment) File vdsm/netconf/iproute2.py Line 103: ipwrapper.linkDel(['dev', vlan.name]) Line 104: vlan.device.remove() Line 105: Line 106: def _ifaceDownAndCleanup(self, iface, _netinfo): Line 107: """Returns True iff the iface is to be removed.""" Ah okay, my bad. Thanks for clarification. Line 108: self.configApplier.ifdown(iface) Line 109: if iface.master is None: Line 110: self.configApplier.removeIpConfig(iface) Line 111: return not _netinfo.ifaceUsers(iface.name) -- 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: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Petr Šebek ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Add iproute2 configurator
Antoni Segura Puimedon has posted comments on this change. Change subject: Add iproute2 configurator .. Patch Set 4: (1 inline comment) File vdsm/netconf/iproute2.py Line 103: ipwrapper.linkDel(['dev', vlan.name]) Line 104: vlan.device.remove() Line 105: Line 106: def _ifaceDownAndCleanup(self, iface, _netinfo): Line 107: """Returns True iff the iface is to be removed.""" No, that's a compsci term meaning "if and only if". Line 108: self.configApplier.ifdown(iface) Line 109: if iface.master is None: Line 110: self.configApplier.removeIpConfig(iface) Line 111: return not _netinfo.ifaceUsers(iface.name) -- 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: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Petr Šebek ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Add iproute2 configurator
Petr Šebek has posted comments on this change. Change subject: Add iproute2 configurator .. Patch Set 4: (1 inline comment) Waiting to see what will happen with that vlan issue. File vdsm/netconf/iproute2.py Line 103: ipwrapper.linkDel(['dev', vlan.name]) Line 104: vlan.device.remove() Line 105: Line 106: def _ifaceDownAndCleanup(self, iface, _netinfo): Line 107: """Returns True iff the iface is to be removed.""" minor typo? iff -> if Line 108: self.configApplier.ifdown(iface) Line 109: if iface.master is None: Line 110: self.configApplier.removeIpConfig(iface) Line 111: return not _netinfo.ifaceUsers(iface.name) -- 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: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Petr Šebek ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Add iproute2 configurator
Antoni Segura Puimedon has posted comments on this change. Change subject: Add iproute2 configurator .. Patch Set 4: I would prefer that you didn't submit this (1 inline comment) I found a potential issue that exists also in ifcfg. File vdsm/netconf/iproute2.py Line 46: self.configApplier.setIfaceMtu(bridge.name, bridge.mtu) Line 47: self.configApplier.ifup(bridge) Line 48: Line 49: def configureVlan(self, vlan, **opts): Line 50: vlan.device.configure(**opts) Note that there is a patch from Hunt Xu that solves a problem that we currently have in the ifcfg configurator where we reconfigure vlan underlying devices even when they were pre-exisiting (which was bad). We should do it well from the start from the start in the iproute2 configurator. You can see the patch and my comments to it in: http://gerrit.ovirt.org/#/c/16474/2 Line 51: self.configApplier.addVlan(vlan) Line 52: if vlan.ip: Line 53: self.configApplier.setIpConfig(vlan) Line 54: self.configApplier.setIfaceMtu(vlan.name, vlan.mtu) -- 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: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: oVirt Jenkins CI Server ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Add iproute2 configurator
Antoni Segura Puimedon has posted comments on this change. Change subject: Add iproute2 configurator .. Patch Set 4: Looks good to me, but someone else must approve Thanks Mark! -- 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: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: oVirt Jenkins CI Server ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Add iproute2 configurator
oVirt Jenkins CI Server has posted comments on this change. Change subject: Add iproute2 configurator .. Patch Set 4: Fails Build Failed http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/3074/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/3153/ : FAILURE http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/2261/ : FAILURE -- 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: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: oVirt Jenkins CI Server ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Add iproute2 configurator
Antoni Segura Puimedon has posted comments on this change. Change subject: Add iproute2 configurator .. Patch Set 3: I would prefer that you didn't submit this (1 inline comment) Apart from the comment, it is probable that it will need a rebase after the patch that simplifies the configurator parameters for configureNic, configureBond, etc. File lib/vdsm/ipwrapper.py Line 245: command += rule Line 246: _execCmd(command) Line 247: Line 248: Line 249: def addrAdd(addr): All these methods perform actions rather than get information. Thus, we do not care about the result and shouldn't return anything. Line 250: command = [_IP_BINARY.cmd, 'addr', 'add'] + addr Line 251: return _execCmd(command) Line 252: Line 253: -- 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: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: oVirt Jenkins CI Server ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Add iproute2 configurator
oVirt Jenkins CI Server has posted comments on this change. Change subject: Add iproute2 configurator .. Patch Set 3: Build Successful http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/2977/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/3055/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/2166/ : SUCCESS -- 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: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: oVirt Jenkins CI Server ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Add iproute2 configurator
Mark Wu has posted comments on this change. Change subject: Add iproute2 configurator .. Patch Set 2: Verified verified with the same test cases as ifcfg configurator. add bridged network over vlaned bond or nic add bridged network over bond or nic add bridgeless network over vlaned bond or nic add bridgeless network over bond or nic remove various kinds of networks add bond, edit bond and remove bond -- 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: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: oVirt Jenkins CI Server ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Add iproute2 configurator
oVirt Jenkins CI Server has posted comments on this change. Change subject: Add iproute2 configurator .. Patch Set 2: Fails Build Failed http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/2714/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/1902/ : FAILURE http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/2788/ : SUCCESS -- 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: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: oVirt Jenkins CI Server ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
