Change in vdsm[master]: NetReload: netmodels for addNetwork
Mark Wu has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 19: I would prefer that you didn't submit this (1 inline comment) File vdsm/netmodels.py Line 167: slaves = [] Line 168: for nic in nics: Line 169: nicVlans = tuple(_netinfo.getVlansForIface(nic)) Line 170: nicNet = _netinfo.getNetworkForIface(nic) Line 171: nicBond = _netinfo.getBondingForNic(name) should be nicBond = _netinfo.getBondingForNic(nic). Sorry not finding this problem before. It also could be a mistake existing in my suggestions. Line 172: if nicVlans or nicNet or nicBond and nicBond != name: Line 173: raise ConfigNetworkError( Line 174: ne.ERR_USED_NIC, 'nic %s already used by %s' % Line 175: (nic, nicVlans or nicNet or nicBond)) -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 19 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Livnat Peer lp...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
oVirt Jenkins CI Server has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 21: Build Successful http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/2546/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/1722/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/2619/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 21 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Livnat Peer lp...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
Antoni Segura Puimedon has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 21: Verified -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 21 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Livnat Peer lp...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
Dan Kenigsberg has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 21: (1 inline comment) File lib/vdsm/netinfo.py Line 446: return d Line 447: Line 448: Line 449: def isVlanned(dev): Line 450: return len(glob.glob(PROC_NET_VLAN + dev + '.*')) != 0 I kinda prefer iglob (which avoids list allocation). But I wonder why is it a bit different to the implementation of the vlans() function. I'd prefer return any([vlan.startswith(dev + '.') for vlan in vlans()]) and if PROC_NET_VLAN is safer than /sys/class/net (I think that it is!), that should be fixed in vlans(). Line 451: Line 452: Line 453: def getVlanDevice(vlan): Line 454: Return the device of the given VLAN. -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 21 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Livnat Peer lp...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
Mark Wu has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 21: Looks good to me, but someone else must approve -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 21 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Livnat Peer lp...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
oVirt Jenkins CI Server has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 22: Build Failed http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/1724/ : FAILURE http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/2621/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/2550/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 22 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Livnat Peer lp...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
oVirt Jenkins CI Server has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 22: No score Build Successful http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/2550/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/2621/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/1726/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 22 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Livnat Peer lp...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
Dan Kenigsberg has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 22: Looks good to me, approved basically copying Mark's ack. Thanks! -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 22 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Livnat Peer lp...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
Dan Kenigsberg has submitted this change and it was merged. Change subject: NetReload: netmodels for addNetwork .. NetReload: netmodels for addNetwork This patch defines the netwok entities that we use in oVirt: - NetDevice, - Bridge, - Vlan, - Bond, - Nic These entities are now responsible for their underlying devices for the addNetwork step. The interaction is as follows: 1. addNetwork -(generates netmodels via)- objectivizeNetwork 2. configure(logical_net) is called on the top network device, 3. The top network device calls configure for its device type on the configurator, that in turn calls configure in any underlying device. In a following patch, they shall take responsibility as well for network deleting. Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Signed-off-by: Antoni S. Puimedon asegu...@redhat.com --- M lib/vdsm/netinfo.py M tests/Makefile.am M tests/configNetworkTests.py A tests/netmodelsTests.py M vdsm.spec.in M vdsm/API.py M vdsm/Makefile.am M vdsm/configNetwork.py M vdsm/netconf/ifcfg.py A vdsm/netmodels.py 10 files changed, 661 insertions(+), 447 deletions(-) Approvals: Antoni Segura Puimedon: Verified Dan Kenigsberg: Looks good to me, approved -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 22 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Livnat Peer lp...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
Antoni Segura Puimedon has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 16: (2 inline comments) File vdsm/netconf/ifcfg.py Line 95: self.configWriter.addBonding(bond.name, bridge=bridge, Line 96: bondingOptions=bond.options, Line 97: mtu=bond.mtu, ipaddr=ipaddr, Line 98: netmask=netmask, gateway=gateway, Line 99: bootproto=bootproto, **opts) I did some testing with a previous version and the master behavior is: Bond0 has eth2 and eth3 with ip 192.168.1.14/24 (done by using iproute2 cmdline). If I add a bridgeless network 'foo' on bond0 with eth2 and eth3 and with ip 172.16.0.2/16, the new ip conf gets written on /etc/sysconfig/network-scripts/ifcfg-bond0 but the the new IP is set besides the old one, not replacing it (just like in this patchset). For nics though we need to do the flushing. Line 100: for slave in bond.slaves: Line 101: slave.configure(bonding=bond.name, **opts) Line 102: ifup(bond.name, async) Line 103: if network: File vdsm/netmodels.py Line 140: Line 141: Line 142: class Bond(NetDevice): Line 143: def __init__(self, name, configurator, ipconfig=None, mtu=None, slaves=(), Line 144: options='mode=802.3ad miimon=150'): Done Line 145: self.validateName(name) Line 146: self.slaves = slaves Line 147: if options: Line 148: self.validateOptions(name, options) -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 16 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Livnat Peer lp...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
oVirt Jenkins CI Server has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 17: Fails Build Failed http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/2542/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/1718/ : FAILURE http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/2615/ : FAILURE -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 17 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Livnat Peer lp...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
Antoni Segura Puimedon has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 17: (1 inline comment) File vdsm/netconf/ifcfg.py Line 95: bondingOptions=bond.options, Line 96: mtu=bond.mtu, ipaddr=ipaddr, Line 97: netmask=netmask, gateway=gateway, Line 98: bootproto=bootproto, **opts) Line 99: if not netinfo.isVlanned(bond.name): This check is still necessary because Bond.objectivize can be used with implicit bonding. Line 100: for slave in bond.slaves: Line 101: if not netinfo.isVlanned(slave.name): Line 102: ifdown(slave.name) Line 103: for slave in bond.slaves: -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 17 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Livnat Peer lp...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
oVirt Jenkins CI Server has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 18: Build Successful http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/2543/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/1719/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/2616/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 18 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Livnat Peer lp...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
Antoni Segura Puimedon has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 19: Verified -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 19 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Livnat Peer lp...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
oVirt Jenkins CI Server has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 19: Build Successful http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/2544/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/1720/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/2617/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 19 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Livnat Peer lp...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
Mark Wu has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 19: I would prefer that you didn't submit this (2 inline comments) File lib/vdsm/netinfo.py Line 446: return d Line 447: Line 448: Line 449: def isVlanned(dev): Line 450: return os.path.exists(PROC_NET_VLAN + dev) it looks kernel only export vlan info using the file name of vlan itself, not the interface it sits on. the file name should read like /proc/net/vlan/eth1.10, so we can't tell if an infterface is used as a slave of vlan by this way. Actually, I tried to find out this kind of link from the info exported by kernel, but failed. So we have to parsing the files under ' /proc/net/vlan', that's what the function vlans() does. Probably we still need resort to netinfo.vlans: return dev in [v['iface'] for v in _netinfo.vlans.values()] Line 451: Line 452: Line 453: def getVlanDevice(vlan): Line 454: Return the device of the given VLAN. File vdsm/netconf/ifcfg.py Line 97: netmask=netmask, gateway=gateway, Line 98: bootproto=bootproto, **opts) Line 99: if not netinfo.isVlanned(bond.name): Line 100: for slave in bond.slaves: Line 101: if not netinfo.isVlanned(slave.name): anyway, the check here is not necessary. Isn't it done in Bond.objectivize(), so you could call ifdown(slave.name) unconditionally. Line 102: ifdown(slave.name) Line 103: for slave in bond.slaves: Line 104: slave.configure(bonding=bond.name, **opts) Line 105: ifup(bond.name, async) -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 19 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Livnat Peer lp...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
Mark Wu has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 19: No score (1 inline comment) File lib/vdsm/netinfo.py Line 446: return d Line 447: Line 448: Line 449: def isVlanned(dev): Line 450: return os.path.exists(PROC_NET_VLAN + dev) OR: len(list(_netinfo.getVlansForIface(dev))) 0 Line 451: Line 452: Line 453: def getVlanDevice(vlan): Line 454: Return the device of the given VLAN. -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 19 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Livnat Peer lp...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
Mark Wu has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 16: (1 inline comment) File vdsm/netconf/ifcfg.py Line 88: if network: Line 89: self.configWriter.createLibvirtNetwork(network, False, vlan.name) Line 90: self._libvirtAdded.add(network) Line 91: Line 92: def configureBond(self, bond, network=None, bridge=None, vlan=None, it looks the parameter vlan is useless here. Line 93: **opts): Line 94: ipaddr, netmask, gateway, bootproto, async = bond.getIpConfig() Line 95: self.configWriter.addBonding(bond.name, bridge=bridge, Line 96: bondingOptions=bond.options, -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 16 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Livnat Peer lp...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
Mark Wu has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 16: (1 inline comment) File vdsm/netmodels.py Line 140: Line 141: Line 142: class Bond(NetDevice): Line 143: def __init__(self, name, configurator, ipconfig=None, mtu=None, slaves=(), Line 144: options='mode=802.3ad miimon=150'): In objectivizeNetwork, Bond.objectivize is called with the parameter 'bondingOptions' passed from engine, and then Bond.objectivize just pass it to Bond.__init__ if it's creating a new bond. The default options 'mode=802.3ad miimon=150' used hear will be overridden by 'None'. It's not what we expect. So probably we need change the default value of options to 'None' in the signature, and assign 'mode=802.3ad miimon=150' to it if None is passed in. Line 145: self.validateName(name) Line 146: self.slaves = slaves Line 147: if options: Line 148: self.validateOptions(name, options) -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 16 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Livnat Peer lp...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
Mark Wu has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 16: I would prefer that you didn't submit this (2 inline comments) Glad to see it's becoming very close to be merged. File vdsm/configNetwork.py Line 49:bondingOptions=None, nics=None, mtu=None, ipaddr=None, Line 50:netmask=None, gateway=None, bootproto=None, Line 51:_netinfo=None, configurator=None, blockingdhcp=None, Line 52:**opts): Line 53: '''Returns object hierarchy that describes the network configuration.''' Why do you reduce it to oneline? I think multi-line is not bad. What you need change is to use three double quota. I would like get Dan's comments on it. Line 54: if configurator is None: Line 55: configurator = Ifcfg() Line 56: if _netinfo is None: Line 57: _netinfo = netinfo.NetInfo() File vdsm/netconf/ifcfg.py Line 95: self.configWriter.addBonding(bond.name, bridge=bridge, Line 96: bondingOptions=bond.options, Line 97: mtu=bond.mtu, ipaddr=ipaddr, Line 98: netmask=netmask, gateway=gateway, Line 99: bootproto=bootproto, **opts) From my test, if a nic is up with an address configured, and then you use vdsClient to create a bridgeless network with different ip address on it, it will not raise an error. The new address will be added as a secondary ip to the nic even though it throws a warning of 'RTNETLINK answers: File exists' That's the behavior of ifup, which also happens to bond and vlan. This case could only happen when the nic or bond has ip configured before the first time it's used by vdsm. So probably we still need run ifdown manually for nic and bond to flush the old ip address Line 100: for slave in bond.slaves: Line 101: slave.configure(bonding=bond.name, **opts) Line 102: ifup(bond.name, async) Line 103: if network: -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 16 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Livnat Peer lp...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
Antoni Segura Puimedon has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 12: (6 inline comments) File tests/configNetworkTests.py Line 38: return {'fakebridgenet': {'iface': 'fakebridge', 'bridged': True}, Line 39: 'fakenet': {'iface': 'fakeint', 'bridged': False}} Line 40: Line 41: Line 42: def raiseInvalidOpException(*args, **kwargs): Done Line 43: return Exception('Attempted to apply network configuration during unit' Line 44: 'testing.') Line 45: Line 46: File vdsm/netmodels.py Line 52: class Nic(NetDevice): Line 53: @classmethod Line 54: def objectivize(cls, name, configurator, mtu, _netinfo): Line 55: if name not in _netinfo.nics: Line 56: raise ConfigNetworkError(ne.ERR_BAD_NIC, 'unknown nic: %s' % name) Done Line 57: return cls(name, configurator, Line 58:mtu=max(mtu, int(netinfo.getMtu(name Line 59: Line 60: def configure(self, network=None, bridge=None, bonding=None, vlan=None, Line 106: Line 107: Line 108: class Bridge(NetDevice): Line 109: '''This class represents traditional kernel bridges.''' Line 110: MAX_BRIDGE_NAME_LEN = 15 Done Line 111: ILLEGAL_BRIDGE_CHARS = frozenset(':. \t') Line 112: Line 113: def __init__(self, name, configurator, ipconfig=None, mtu=None, port=None, Line 114: forwardDelay=0, stp=None): Line 118: self.stp = stp Line 119: super(Bridge, self).__init__(name, configurator, ipconfig, mtu) Line 120: Line 121: def __repr__(self): Line 122: return 'Bridge(%s: %r)' % (self.name, self.ports) Done Line 123: Line 124: def configure(self, network, **opts): Line 125: self.configurator.configureBridge(self, network=network, **opts) Line 126: Line 245: self.netmask = netmask Line 246: self.gateway = gateway Line 247: Line 248: def __str__(self): Line 249: return self.__repr__() Right, I didn't know that :P Line 250: Line 251: def __repr__(self): Line 252: return 'IPv4(%s, %s, %s)' % (self.address, self.netmask, self.gateway) Line 253: Line 298: def getConfig(self): Line 299: try: Line 300: ipaddr = self.ip.inet.address Line 301: netmask = self.ip.inet.netmask Line 302: gateway = self.ip.inet.gateway Done Line 303: except AttributeError: Line 304: ipaddr = netmask = gateway = None Line 305: bootproto = self.bootproto -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 12 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Livnat Peer lp...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
Antoni Segura Puimedon has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 12: (2 inline comments) File vdsm/netconf/ifcfg.py Line 73: ifdown(bridge.name) Line 74: if bridge.port: Line 75: bridge.port.configure(bridge=bridge.name, **opts) Line 76: ifup(bridge.name, bootproto == 'dhcp' and Line 77: not utils.tobool(opts.get('blockingdhcp'))) Right. I'll move it to the IpConfig. Line 78: self.configWriter.createLibvirtNetwork(network, True) Line 79: self._libvirtAdded.add(bridge.name) Line 80: Line 81: def configureVlan(self, vlan, network=None, bridge=None, **opts): Line 100: netmask=netmask, gateway=gateway, Line 101: bootproto=bootproto, **opts) Line 102: # take down nics that need to be changed Line 103: _netinfo = netinfo.NetInfo() Line 104: vlanedIfaces = [v['iface'] for v in _netinfo.vlans.values()] Done Line 105: if bond.name not in vlanedIfaces: Line 106: for slave in bond.slaves: Line 107: if slave.name not in vlanedIfaces: Line 108: ifdown(slave.name) -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 12 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Livnat Peer lp...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
Antoni Segura Puimedon has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 12: (1 inline comment) File vdsm/configNetwork.py Line 142: _netinfo = netinfo.NetInfo() Line 143: bridged = utils.tobool(bridged) Line 144: Line 145: if mtu: Line 146: mtu = int(mtu) Done Line 147: if prefix: Line 148: if netmask: Line 149: raise ConfigNetworkError(ne.ERR_BAD_PARAMS, Line 150: 'Both PREFIX and NETMASK supplied') -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 12 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Livnat Peer lp...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
Antoni Segura Puimedon has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 12: (1 inline comment) File vdsm/configNetwork.py Line 51: def objectivizeNetwork(bridge=None, vlan=None, bonding=None, Line 52:bondingOptions=None, nics=None, mtu=None, ipaddr=None, Line 53:netmask=None, gateway=None, bootproto=None, Line 54:_netinfo=None, configurator=None, **opts): Line 55: '''Takes a series of parameters that describe a network configuration and Done Line 56: generates an object oriented representation of it. The object are set in Line 57: a hierarchical way and the ip configuration parametes (if any) are set on Line 58: the network device object that constitutes the top of the hierarchy.''' Line 59: if configurator is None: -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 12 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Livnat Peer lp...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
oVirt Jenkins CI Server has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 13: Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/2473/ (3/3) -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 13 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Livnat Peer lp...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
oVirt Jenkins CI Server has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 13: Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/2543/ (2/3) -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 13 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Livnat Peer lp...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
oVirt Jenkins CI Server has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 13: Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/1642/ (1/3) -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 13 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Livnat Peer lp...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
oVirt Jenkins CI Server has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 13: Fails Build Failed http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/2473/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/1642/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/2543/ : FAILURE -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 13 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Livnat Peer lp...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
oVirt Jenkins CI Server has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 14: Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/2474/ (1/3) -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 14 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Livnat Peer lp...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
oVirt Jenkins CI Server has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 14: Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/2544/ (2/3) -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 14 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Livnat Peer lp...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
oVirt Jenkins CI Server has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 14: Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/1643/ (3/3) -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 14 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Livnat Peer lp...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
oVirt Jenkins CI Server has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 14: Fails Build Failed http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/2474/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/1643/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/2544/ : FAILURE -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 14 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Livnat Peer lp...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
oVirt Jenkins CI Server has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 15: Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/2545/ (2/3) -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 15 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Livnat Peer lp...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
oVirt Jenkins CI Server has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 15: Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/2475/ (1/3) -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 15 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Livnat Peer lp...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
oVirt Jenkins CI Server has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 15: Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/1644/ (3/3) -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 15 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Livnat Peer lp...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
oVirt Jenkins CI Server has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 15: Fails Build Failed http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/2475/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/1644/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/2545/ : FAILURE -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 15 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Livnat Peer lp...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
oVirt Jenkins CI Server has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 16: Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/2546/ (1/3) -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 16 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Livnat Peer lp...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
oVirt Jenkins CI Server has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 16: Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/1645/ (3/3) -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 16 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Livnat Peer lp...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
oVirt Jenkins CI Server has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 16: Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/2476/ (2/3) -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 16 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Livnat Peer lp...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
oVirt Jenkins CI Server has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 16: Build Successful http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/2476/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/1645/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/2546/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 16 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Livnat Peer lp...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
Antoni Segura Puimedon has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 12: (1 inline comment) File vdsm/configNetwork.py Line 73: except ValueError: Line 74: raise ConfigNetworkError(ne.ERR_BAD_BONDING, 'Multiple nics ' Line 75: 'require a bonding device') Line 76: else: Line 77: bond = _netinfo.getBondingForNic(nic) It's an interesting idea. We can evaluate it for a future patch. Line 78: if bond: Line 79: raise ConfigNetworkError(ne.ERR_USED_NIC, 'nic %s already ' Line 80: 'enslaved to %s' % (nic, bond)) Line 81: topNetDev = Nic.objectivize(nic, configurator, mtu, _netinfo) -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 12 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Livnat Peer lp...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
Antoni Segura Puimedon has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 16: (1 inline comment) File vdsm/netconf/ifcfg.py Line 95: self.configWriter.addBonding(bond.name, bridge=bridge, Line 96: bondingOptions=bond.options, Line 97: mtu=bond.mtu, ipaddr=ipaddr, Line 98: netmask=netmask, gateway=gateway, Line 99: bootproto=bootproto, **opts) With regard to the comments on line 107 of patchset 12. I realized that the bonding code in initscripts (relevant file ifup-eth) already has a check for setting the link down before adding the interface to the bond. Thus, that part is not needed. Line 100: for slave in bond.slaves: Line 101: slave.configure(bonding=bond.name, **opts) Line 102: ifup(bond.name, async) Line 103: if network: -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 16 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Livnat Peer lp...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
Dan Kenigsberg has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 12: I would prefer that you didn't submit this (1 inline comment) File vdsm/netmodels.py Line 109: '''This class represents traditional kernel bridges.''' Line 110: MAX_BRIDGE_NAME_LEN = 15 Line 111: ILLEGAL_BRIDGE_CHARS = frozenset(':. \t') Line 112: Line 113: def __init__(self, name, configurator, ipconfig=None, mtu=None, port=None, We should continue to support nicless bridges (even though Engine does not have them). Line 114: forwardDelay=0, stp=None): Line 115: self.validateName(name) Line 116: self.port = port Line 117: self.forwardDelay = forwardDelay -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 12 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Livnat Peer lp...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
Dan Kenigsberg has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 12: No score -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 12 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Livnat Peer lp...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
Mark Wu has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 12: (1 inline comment) File vdsm/netmodels.py Line 109: '''This class represents traditional kernel bridges.''' Line 110: MAX_BRIDGE_NAME_LEN = 15 Line 111: ILLEGAL_BRIDGE_CHARS = frozenset(':. \t') Line 112: Line 113: def __init__(self, name, configurator, ipconfig=None, mtu=None, port=None, what is nicless bridge used for? For libvirt isolated or NAT network, the bridge is managed by libvirt, not related to host network configuration. So why do we need nicless bridge? Line 114: forwardDelay=0, stp=None): Line 115: self.validateName(name) Line 116: self.port = port Line 117: self.forwardDelay = forwardDelay -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 12 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Livnat Peer lp...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
Mark Wu has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 12: I would prefer that you didn't submit this (1 inline comment) File vdsm/netmodels.py Line 298: def getConfig(self): Line 299: try: Line 300: ipaddr = self.ip.inet.address Line 301: netmask = self.ip.inet.netmask Line 302: gateway = self.ip.inet.gateway should be ipaddr = self.inet.address netmask = self.inet.netmask gateway = self.inet.gateway right? Line 303: except AttributeError: Line 304: ipaddr = netmask = gateway = None Line 305: bootproto = self.bootproto -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 12 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Livnat Peer lp...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
Dan Kenigsberg has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 12: (10 inline comments) mostly minor comments File tests/configNetworkTests.py Line 38: return {'fakebridgenet': {'iface': 'fakebridge', 'bridged': True}, Line 39: 'fakenet': {'iface': 'fakeint', 'bridged': False}} Line 40: Line 41: Line 42: def raiseInvalidOpException(*args, **kwargs): this has to be declared _private. Raising a RuntimeError is a bit nicer. also, add space between 'unit' and 'testing'. Line 43: return Exception('Attempted to apply network configuration during unit' Line 44: 'testing.') Line 45: Line 46: File vdsm/configNetwork.py Line 51: def objectivizeNetwork(bridge=None, vlan=None, bonding=None, Line 52:bondingOptions=None, nics=None, mtu=None, ipaddr=None, Line 53:netmask=None, gateway=None, bootproto=None, Line 54:_netinfo=None, configurator=None, **opts): Line 55: '''Takes a series of parameters that describe a network configuration and http://www.python.org/dev/peps/pep-0257/ Line 56: generates an object oriented representation of it. The object are set in Line 57: a hierarchical way and the ip configuration parametes (if any) are set on Line 58: the network device object that constitutes the top of the hierarchy.''' Line 59: if configurator is None: Line 142: _netinfo = netinfo.NetInfo() Line 143: bridged = utils.tobool(bridged) Line 144: Line 145: if mtu: Line 146: mtu = int(mtu) I actually liked the breath-bearing newline that you removed Line 147: if prefix: Line 148: if netmask: Line 149: raise ConfigNetworkError(ne.ERR_BAD_PARAMS, Line 150: 'Both PREFIX and NETMASK supplied') File vdsm/netconf/ifcfg.py Line 70: netmask=netmask, mtu=bridge.mtu, Line 71: gateway=gateway, bootproto=bootproto, Line 72: **opts) Line 73: ifdown(bridge.name) Line 74: if bridge.port: (we would always need to check for niclessness) Line 75: bridge.port.configure(bridge=bridge.name, **opts) Line 76: ifup(bridge.name, bootproto == 'dhcp' and Line 77: not utils.tobool(opts.get('blockingdhcp'))) Line 78: self.configWriter.createLibvirtNetwork(network, True) Line 73: ifdown(bridge.name) Line 74: if bridge.port: Line 75: bridge.port.configure(bridge=bridge.name, **opts) Line 76: ifup(bridge.name, bootproto == 'dhcp' and Line 77: not utils.tobool(opts.get('blockingdhcp'))) do we really want to push blockingdhcp down to the configurator level? it's not like iproute2 would be able to take a different policy for calculating async. Line 78: self.configWriter.createLibvirtNetwork(network, True) Line 79: self._libvirtAdded.add(bridge.name) Line 80: Line 81: def configureVlan(self, vlan, network=None, bridge=None, **opts): Line 100: netmask=netmask, gateway=gateway, Line 101: bootproto=bootproto, **opts) Line 102: # take down nics that need to be changed Line 103: _netinfo = netinfo.NetInfo() Line 104: vlanedIfaces = [v['iface'] for v in _netinfo.vlans.values()] ...vlans.itervalues() would avoid an intermediate list() Line 105: if bond.name not in vlanedIfaces: Line 106: for slave in bond.slaves: Line 107: if slave.name not in vlanedIfaces: Line 108: ifdown(slave.name) File vdsm/netmodels.py Line 106: Line 107: Line 108: class Bridge(NetDevice): Line 109: '''This class represents traditional kernel bridges.''' Line 110: MAX_BRIDGE_NAME_LEN = 15 this is currently defined twice - better drop the configNet.py definition. Within this class, a better name is MAX_NAME_LEN. Line 111: ILLEGAL_BRIDGE_CHARS = frozenset(':. \t') Line 112: Line 113: def __init__(self, name, configurator, ipconfig=None, mtu=None, port=None, Line 114: forwardDelay=0, stp=None): Line 109: '''This class represents traditional kernel bridges.''' Line 110: MAX_BRIDGE_NAME_LEN = 15 Line 111: ILLEGAL_BRIDGE_CHARS = frozenset(':. \t') Line 112: Line 113: def __init__(self, name, configurator, ipconfig=None, mtu=None, port=None, I need it for testing vdsm. Really. I do it all the time. And I see a use case for an ensemble of VMs which are disconnected from the outer world. Line
Change in vdsm[master]: NetReload: netmodels for addNetwork
Giuseppe Vallarelli has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 7: (3 inline comments) File vdsm/configNetwork.py Line 47: MAX_BRIDGE_NAME_LEN = 15 Line 48: ILLEGAL_BRIDGE_CHARS = frozenset(':. \t') Line 49: Line 50: Line 51: def objectivizeNetwork(bridge=None, vlan=None, bonding=None, Ok, great. I agree with you, makeObjetOrientedNetworkRepresentation is an ugly name but I'm still sure you can find a better name than objectivizeNetwork. What about createNetworkModel ? Choosing good names ain't easy but we should at least made an attempt. Line 52:bondingOptions=None, nics=None, mtu=None, ipaddr=None, Line 53:netmask=None, gateway=None, bootproto=None, Line 54:_netinfo=None, configurator=None, **opts): Line 55: if configurator is None: Line 76: if bridge: Line 77: topObj = Bridge(bridge, configurator, ports=[topObj] if topObj else [], Line 78: mtu=mtu, stp=opts.get('stp'), Line 79: forwardDelay=opts.get('forward_delay', 0)) Line 80: topObj.ip = IpConfig(inet=IPv4(ipaddr, netmask, gateway), My point of view is that It doesn't matter if that path is not reached today, code is going to evolve and used in unexpected ways. Function signature says that I can ignore those parameters, it's also a 'public' function. I'll leave the decision to fix it to your good judgement. Line 81: bootproto=bootproto) Line 82: return topObj Line 83: Line 84: Line 175: if bonding not in vlanedIfaces: Line 176: for nic in nics: Line 177: if nic not in vlanedIfaces: Line 178: ifdown(nic) Line 179: netEnt.configure(network=network, **options) Sure no worries. What I was suggesting is to create a Network entity (so objectiveNetwork would return a Network obj) where you can attach Network components addNetDevice (bonding, nics, vlan etc.). This Network entity would have a configure method which iterates among the list of attached devices and call on them configure (that's why I was suggesting to attach configure on the entities). You're already doing something similar to this idea except that's not so explicit. Line 180: Line 181: Line 182: def assertBridgeClean(bridge, vlan, bonding, nics): Line 183: ports = set(netinfo.ports(bridge)) -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
Antoni Segura Puimedon has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 7: (6 inline comments) File vdsm/netmodels.py Line 50: Line 51: Line 52: class Nic(NetDevice): Line 53: @classmethod Line 54: def objectivize(cls, name, configurator, mtu, _netinfo, bonding=None): Done Line 55: if name not in _netinfo.nics: Line 56: raise ConfigNetworkError(ne.ERR_BAD_NIC, 'unknown nic: %s' % name) Line 57: nicBond = _netinfo.getBondingForNic(name) Line 58: if not bonding and nicBond or \ Line 57: nicBond = _netinfo.getBondingForNic(name) Line 58: if not bonding and nicBond or \ Line 59: bonding and nicBond and nicBond != bonding: Line 60: raise ConfigNetworkError(ne.ERR_USED_NIC, 'nic %s already enslaved' Line 61: ' to %s' % (name, nicBond)) Done Line 62: return cls(name, configurator, mtu=mtu if bonding else Line 63:max(mtu, int(netinfo.getMtu(name Line 64: Line 65: def configure(self, network=None, bridge=None, bonding=None, vlan=None, Line 59: bonding and nicBond and nicBond != bonding: Line 60: raise ConfigNetworkError(ne.ERR_USED_NIC, 'nic %s already enslaved' Line 61: ' to %s' % (name, nicBond)) Line 62: return cls(name, configurator, mtu=mtu if bonding else Line 63:max(mtu, int(netinfo.getMtu(name Done Line 64: Line 65: def configure(self, network=None, bridge=None, bonding=None, vlan=None, Line 66: **opts): Line 67: self.configurator.configureNic(self, network=network, bridge=bridge, Line 167: if name and nics: Line 168: slaves = [] Line 169: for nic in nics: Line 170: nicVlans = tuple(_netinfo.getVlansForIface(nic)) Line 171: nicNet = _netinfo.getNetworkForIface(nic) Done Line 172: if nicVlans or nicNet: Line 173: raise ConfigNetworkError( Line 174: ne.ERR_USED_NIC, 'nic %s already used by %s %s' % Line 175: (nic, 'vlans' if nicVlans else 'network', Line 168: slaves = [] Line 169: for nic in nics: Line 170: nicVlans = tuple(_netinfo.getVlansForIface(nic)) Line 171: nicNet = _netinfo.getNetworkForIface(nic) Line 172: if nicVlans or nicNet: Moved the nicBond check here. Line 173: raise ConfigNetworkError( Line 174: ne.ERR_USED_NIC, 'nic %s already used by %s %s' % Line 175: (nic, 'vlans' if nicVlans else 'network', Line 176: nicVlans or nicNet)) Line 179: elif name in _netinfo.bondings: # Implicit bonding. Line 180: nics = [nic for nic in _netinfo.getNicsForBonding(name)] Line 181: mtu = max(int(netinfo.getMtu(name)), mtu) Line 182: slaves = [Nic.objectivize(nic, configurator, mtu, _netinfo, name) Line 183: for nic in nics] Done Line 184: options = _netinfo.bondings[name]['cfg'].get('BONDING_OPTS') Line 185: else: Line 186: raise ConfigNetworkError(ne.ERR_BAD_BONDING, Line 187: 'Bonding %s not specified and it is not ' -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
oVirt Jenkins CI Server has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 8: Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/1598/ (2/3) -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
oVirt Jenkins CI Server has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 8: Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/2430/ (1/3) -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
oVirt Jenkins CI Server has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 8: Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/2499/ (3/3) -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
oVirt Jenkins CI Server has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 8: Fails Build Failed http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/2430/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/1598/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/2499/ : FAILURE -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
Antoni Segura Puimedon has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 8: Verified I'll investigate the discrepancy in the unit tests. But it passes all the testing I put it through. -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
oVirt Jenkins CI Server has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 8: No score Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/2500/ (3/3) -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
oVirt Jenkins CI Server has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 8: Fails Build Failed http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/2430/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/1598/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/2500/ : FAILURE -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
oVirt Jenkins CI Server has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 9: Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/1600/ (1/3) -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
oVirt Jenkins CI Server has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 9: Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/2501/ (2/3) -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
oVirt Jenkins CI Server has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 9: Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/2431/ (3/3) -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
oVirt Jenkins CI Server has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 9: Fails Build Failed http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/2431/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/1600/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/2501/ : FAILURE -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
oVirt Jenkins CI Server has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 10: Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/1601/ (1/3) -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
oVirt Jenkins CI Server has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 10: Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/2502/ (2/3) -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
oVirt Jenkins CI Server has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 10: Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/2432/ (3/3) -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
oVirt Jenkins CI Server has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 10: Build Successful http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/2432/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/1601/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/2502/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
Antoni Segura Puimedon has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 10: (1 inline comment) File vdsm/configNetwork.py Line 73: except ValueError: Line 74: raise ConfigNetworkError(ne.ERR_BAD_BONDING, 'Multiple nics ' Line 75: 'require a bonding device') Line 76: else: Line 77: bond = _netinfo.getBondingForNic(nic) Lines 77-80 should be moved to Nic.objectivize after patch http://gerrit.ovirt.org/#/c/14870/ that adds a reference to a master device and will make this check belong there. Line 78: if bond: Line 79: raise ConfigNetworkError(ne.ERR_USED_NIC, 'nic %s already ' Line 80: 'enslaved to %s' % (nic, bond)) Line 81: topObj = Nic.objectivize(nic, configurator, mtu, _netinfo) -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
oVirt Jenkins CI Server has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 11: Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/2503/ (1/3) -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Livnat Peer lp...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
oVirt Jenkins CI Server has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 11: Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/2433/ (3/3) -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Livnat Peer lp...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
oVirt Jenkins CI Server has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 11: Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/1602/ (2/3) -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Livnat Peer lp...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
oVirt Jenkins CI Server has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 11: Fails; I would prefer that you didn't submit this Build Failed http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/2433/ : UNSTABLE http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/1602/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/2503/ : FAILURE -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Livnat Peer lp...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
oVirt Jenkins CI Server has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 12: Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/1603/ (1/3) -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 12 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Livnat Peer lp...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
oVirt Jenkins CI Server has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 12: Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/2504/ (3/3) -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 12 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Livnat Peer lp...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
oVirt Jenkins CI Server has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 12: Build Successful http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/2434/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/1603/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/2504/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 12 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Livnat Peer lp...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
Antoni Segura Puimedon has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 12: Verified (1 inline comment) File vdsm/configNetwork.py Line 73: except ValueError: Line 74: raise ConfigNetworkError(ne.ERR_BAD_BONDING, 'Multiple nics ' Line 75: 'require a bonding device') Line 76: else: Line 77: bond = _netinfo.getBondingForNic(nic) Lines 77-80 should be moved to Nic.objectivize after patch http://gerrit.ovirt.org/#/c/14870/ that adds a reference to a master device and will make this check belong there. Line 78: if bond: Line 79: raise ConfigNetworkError(ne.ERR_USED_NIC, 'nic %s already ' Line 80: 'enslaved to %s' % (nic, bond)) Line 81: topNetDev = Nic.objectivize(nic, configurator, mtu, _netinfo) -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 12 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Livnat Peer lp...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
Giuseppe Vallarelli has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 12: Looks good to me, but someone else must approve Work done so far looks good to me, we may add more refinements in later patches. -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 12 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Livnat Peer lp...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
oVirt Jenkins CI Server has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 12: Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/2434/ (2/3) -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 12 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Livnat Peer lp...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
Mark Wu has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 12: I would prefer that you didn't submit this (7 inline comments) Except the comment in ifcfg.py line 108, all other problems could be resolved in following up patches. File vdsm/configNetwork.py Line 73: except ValueError: Line 74: raise ConfigNetworkError(ne.ERR_BAD_BONDING, 'Multiple nics ' Line 75: 'require a bonding device') Line 76: else: Line 77: bond = _netinfo.getBondingForNic(nic) I can't figure out how can you do that via the reference to master because you don't have the object when you call Nic.objecivize(). Actually, as I what commented before, I don't like do the validation in low level object. Instead, I would like to propose the following idea for validation: 1. each network entity validate its attribute in its own __init__() 2. to validate if the slave is fine to be used for the new master, we could add some validation functions in each network entity. For example, in the Nic class, we could have the following validation functions: def isAvailableForBond(self, bond): ... def isAvailableForVlan(self, vlan): ... def isAvailableForBridge(self, bridge): def isAvailableForIpConfig(self): --- apply in the case of bridgeless network And when you construct a high level network entity, let's say vlan, you could just call: device.isAvailableForVlan(vlan) Add you also could add a function to NetDevice to set ipconfig, and call self.isAvaliableForIpConfig() in that function. Then you could using it in line 91 to replace setting the ipconfig directly. After we have the changes above, probably _validateInterNetworkCompatibility could be removed. Definitely, I am not requesting the change in this patch. Line 78: if bond: Line 79: raise ConfigNetworkError(ne.ERR_USED_NIC, 'nic %s already ' Line 80: 'enslaved to %s' % (nic, bond)) Line 81: topNetDev = Nic.objectivize(nic, configurator, mtu, _netinfo) File vdsm/netconf/ifcfg.py Line 70: netmask=netmask, mtu=bridge.mtu, Line 71: gateway=gateway, bootproto=bootproto, Line 72: **opts) Line 73: ifdown(bridge.name) Line 74: if bridge.port: If we check it in Bridge's __init__, we don't need the check here. Line 75: bridge.port.configure(bridge=bridge.name, **opts) Line 76: ifup(bridge.name, bootproto == 'dhcp' and Line 77: not utils.tobool(opts.get('blockingdhcp'))) Line 78: self.configWriter.createLibvirtNetwork(network, True) Line 103: _netinfo = netinfo.NetInfo() Line 104: vlanedIfaces = [v['iface'] for v in _netinfo.vlans.values()] Line 105: if bond.name not in vlanedIfaces: Line 106: for slave in bond.slaves: Line 107: if slave.name not in vlanedIfaces: It could not be in vlanedIfaces because it's already validated in Bond's objectivize() Line 108: ifdown(slave.name) Line 109: for slave in bond.slaves: Line 110: slave.configure(bonding=bond.name, **opts) Line 111: ifup(bond.name, bootproto == 'dhcp' and Line 104: vlanedIfaces = [v['iface'] for v in _netinfo.vlans.values()] Line 105: if bond.name not in vlanedIfaces: Line 106: for slave in bond.slaves: Line 107: if slave.name not in vlanedIfaces: Line 108: ifdown(slave.name) It seems you changed the behavior. The old code 'if bonding not in vlanedIfaces:' covers two situations: the bonding is not used as slave of vlan and no bonding (bonding is None). In the second case, it also run ifdown on the nic if the nic is not connected to vlan. I guess the second implicit case is also what we need. So probably you need add similar code to configureNic if you want to move it in this patch. Otherwise just leave it there for a future patch. Line 109: for slave in bond.slaves: Line 110: slave.configure(bonding=bond.name, **opts) Line 111: ifup(bond.name, bootproto == 'dhcp' and Line 112: not utils.tobool(opts.get('blockingdhcp'))) File vdsm/netmodels.py Line 52: class Nic(NetDevice): Line 53: @classmethod Line 54: def objectivize(cls, name, configurator, mtu, _netinfo): Line 55: if name not in _netinfo.nics: Line 56: raise ConfigNetworkError(ne.ERR_BAD_NIC, 'unknown nic: %s' % name) It looks the objectivize() of Nic is thin enough to put it into its __init__(). Line 57:
Change in vdsm[master]: NetReload: netmodels for addNetwork
Mark Wu has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 12: No score (1 inline comment) File vdsm/netmodels.py Line 169: raise ConfigNetworkError( Line 170: ne.ERR_USED_NIC, 'nic %s already used by %s' % Line 171: (nic, nicVlans or nicNet or nicBond)) Line 172: slaves.append(Nic.objectivize(nic, configurator, mtu, Line 173: _netinfo)) Please ignore this one. It's pep8 compliant. Line 174: elif name in _netinfo.bondings: # Implicit bonding. Line 175: mtu = max(int(netinfo.getMtu(name)), mtu) Line 176: slaves = [Nic.objectivize(nic, configurator, mtu, _netinfo) Line 177: for nic in _netinfo.getNicsForBonding(name)] -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 12 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Livnat Peer lp...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
Giuseppe Vallarelli has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 7: I would prefer that you didn't submit this (3 inline comments) See comments specific comments. File vdsm/configNetwork.py Line 47: MAX_BRIDGE_NAME_LEN = 15 Line 48: ILLEGAL_BRIDGE_CHARS = frozenset(':. \t') Line 49: Line 50: Line 51: def objectivizeNetwork(bridge=None, vlan=None, bonding=None, Instead of objectivizeSomething can you change it to buildSomething or createSomething (same applies to the different models). Adding a docstring would be cool and responsible, you should explain what the object returned represents, if I remember correctly is this a configuration that a user wants to apply? In this case also createNetwork is still a generic name which doesn't convey the real meaning. Line 52:bondingOptions=None, nics=None, mtu=None, ipaddr=None, Line 53:netmask=None, gateway=None, bootproto=None, Line 54:_netinfo=None, configurator=None, **opts): Line 55: if configurator is None: Line 76: if bridge: Line 77: topObj = Bridge(bridge, configurator, ports=[topObj] if topObj else [], Line 78: mtu=mtu, stp=opts.get('stp'), Line 79: forwardDelay=opts.get('forward_delay', 0)) Line 80: topObj.ip = IpConfig(inet=IPv4(ipaddr, netmask, gateway), topObj can still be None, if a user of this function doesn't specifiy bonding, nics, etc.. Can you add also a test ? Line 81: bootproto=bootproto) Line 82: return topObj Line 83: Line 84: File vdsm/netmodels.py Line 27: from vdsm import netinfo Line 28: import neterrors as ne Line 29: Line 30: Line 31: class NetDevice(object): I think you should move in NetDevice the configure method (not implemented) so you make clear that every subclass of NetDevice should implement a configure method. Line 32: def __init__(self, name, configurator, ipconfig=None, mtu=None): Line 33: self.name = name Line 34: self.ip = ipconfig Line 35: self.mtu = mtu -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
Giuseppe Vallarelli has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 7: No score (1 inline comment) File vdsm/configNetwork.py Line 175: if bonding not in vlanedIfaces: Line 176: for nic in nics: Line 177: if nic not in vlanedIfaces: Line 178: ifdown(nic) Line 179: netEnt.configure(network=network, **options) What about a general Network model with 2 methods, addNetDevice and configure which call configure on the list of netDevices created by adding them with addNetDevice ? In this way you can also move the configure methods to the respective entities. Line 180: Line 181: Line 182: def assertBridgeClean(bridge, vlan, bonding, nics): Line 183: ports = set(netinfo.ports(bridge)) -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
Antoni Segura Puimedon has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 7: (6 inline comments) File vdsm/configNetwork.py Line 47: MAX_BRIDGE_NAME_LEN = 15 Line 48: ILLEGAL_BRIDGE_CHARS = frozenset(':. \t') Line 49: Line 50: Line 51: def objectivizeNetwork(bridge=None, vlan=None, bonding=None, This method takes a few arguments that describe a network and makes an object oriented representation out of it. It does not create a network and I am not a fan of unnecessary long names like makeObjectOrientedNetworkRepresentation. Good point about the docstring. Line 52:bondingOptions=None, nics=None, mtu=None, ipaddr=None, Line 53:netmask=None, gateway=None, bootproto=None, Line 54:_netinfo=None, configurator=None, **opts): Line 55: if configurator is None: Line 76: if bridge: Line 77: topObj = Bridge(bridge, configurator, ports=[topObj] if topObj else [], Line 78: mtu=mtu, stp=opts.get('stp'), Line 79: forwardDelay=opts.get('forward_delay', 0)) Line 80: topObj.ip = IpConfig(inet=IPv4(ipaddr, netmask, gateway), AFAIK This code can't be reached by calls from vdsClient nor the engine without a topObj being set. I could raise a BAD_PARAMS error I guess... Line 81: bootproto=bootproto) Line 82: return topObj Line 83: Line 84: Line 81: bootproto=bootproto) Line 82: return topObj Line 83: Line 84: Line 85: def _validateInterNetworkCompatibility(ni, vlan, iface, bridged): I'd leave this for a follow-up patch. Line 86: Line 87: Verify network compatibility with other networks on iface (bond/nic). Line 88: Line 89: Only following combinations allowed: Line 175: if bonding not in vlanedIfaces: Line 176: for nic in nics: Line 177: if nic not in vlanedIfaces: Line 178: ifdown(nic) Line 179: netEnt.configure(network=network, **options) I'm afraid I don't understand this. Would you please rephrase? Line 180: Line 181: Line 182: def assertBridgeClean(bridge, vlan, bonding, nics): Line 183: ports = set(netinfo.ports(bridge)) File vdsm/netmodels.py Line 27: from vdsm import netinfo Line 28: import neterrors as ne Line 29: Line 30: Line 31: class NetDevice(object): Done Line 32: def __init__(self, name, configurator, ipconfig=None, mtu=None): Line 33: self.name = name Line 34: self.ip = ipconfig Line 35: self.mtu = mtu Line 114: '''This class represents traditional kernel bridges.''' Line 115: MAX_BRIDGE_NAME_LEN = 15 Line 116: ILLEGAL_BRIDGE_CHARS = frozenset(':. \t') Line 117: Line 118: def __init__(self, name, configurator, ipconfig=None, mtu=None, ports=(), You are right that we don't do it at the moment and there are no plans to do it. Thus, this premature extensibility point (it was here for netinfo) will be dropped. Line 119: forwardDelay=0, stp=None): Line 120: self.validateName(name) Line 121: self.ports = ports Line 122: self.forwardDelay = forwardDelay -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
Mark Wu has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 7: (1 inline comment) File vdsm/configNetwork.py Line 174: vlanedIfaces = [v['iface'] for v in _netinfo.vlans.values()] Line 175: if bonding not in vlanedIfaces: Line 176: for nic in nics: Line 177: if nic not in vlanedIfaces: Line 178: ifdown(nic) this code snippet should be moved to configurator too. what's the purpose of it? Just make it possible to run ifup after update configurations? Line 179: netEnt.configure(network=network, **options) Line 180: Line 181: Line 182: def assertBridgeClean(bridge, vlan, bonding, nics): -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
Antoni Segura Puimedon has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 7: (5 inline comments) File vdsm/configNetwork.py Line 174: vlanedIfaces = [v['iface'] for v in _netinfo.vlans.values()] Line 175: if bonding not in vlanedIfaces: Line 176: for nic in nics: Line 177: if nic not in vlanedIfaces: Line 178: ifdown(nic) Right. I wanted to leave it for a later patch. But since there are other changes to this one, I might get it in also. Line 179: netEnt.configure(network=network, **options) Line 180: Line 181: Line 182: def assertBridgeClean(bridge, vlan, bonding, nics): File vdsm/netmodels.py Line 1: # Copyright 2011-2013 Red Hat, Inc. I'm not that knowledgeable either about this matters, but since I moved code here that was in a 2011 Copyrighted file, I thought I'd put 2011-13. Let's hope that Dan chips in with a clear decision. Line 2: # Line 3: # This program is free software; you can redistribute it and/or modify Line 4: # it under the terms of the GNU General Public License as published by Line 5: # the Free Software Foundation; either version 2 of the License, or Line 42: try: Line 43: ipaddr = self.ip.inet.address Line 44: netmask = self.ip.inet.netmask Line 45: gateway = self.ip.inet.gateway Line 46: bootproto = self.ip.bootproto Done Line 47: except AttributeError: Line 48: ipaddr = netmask = gateway = bootproto = None Line 49: return ipaddr, netmask, gateway, bootproto Line 50: Line 70: def remove(self, network=None, bond=None): Line 71: self.configurator.removeNic(self, network=network, bond=bond) Line 72: Line 73: def __repr__(self): Line 74: return 'Nic(' + self.name + ')' Done Line 75: Line 76: Line 77: class Vlan(NetDevice): Line 78: MAX_VLAN_ID = 4094 Line 180: nics = [nic for nic in _netinfo.getNicsForBonding(name)] Line 181: mtu = max(int(netinfo.getMtu(name)), mtu) Line 182: slaves = [Nic.objectivize(nic, configurator, mtu, _netinfo, name) Line 183: for nic in nics] Line 184: options = _netinfo.bondings[name]['cfg'].get('BONDING_OPTS') The thing is that the engine relies on vdsCaps having a 'cfg' field for devices, so even with other configurators, netinfo will have to continue to provide a {'cfg': {'BONDING_OPTS': 'someopt=somevalue'}}. We could, however, in a future patch, change it into _netinfo.getBondingOpts(name). Line 185: else: Line 186: raise ConfigNetworkError(ne.ERR_BAD_BONDING, Line 187: 'Bonding %s not specified and it is not ' Line 188: 'already on the system' % name) -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
Mark Wu has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 7: I would prefer that you didn't submit this (12 inline comments) File vdsm/configNetwork.py Line 81: bootproto=bootproto) Line 82: return topObj Line 83: Line 84: Line 85: def _validateInterNetworkCompatibility(ni, vlan, iface, bridged): This validation should be moved to network entity's objectivize() too. I am fine to fix it in a following up patch Line 86: Line 87: Verify network compatibility with other networks on iface (bond/nic). Line 88: Line 89: Only following combinations allowed: File vdsm/netmodels.py Line 1: # Copyright 2011-2013 Red Hat, Inc. This is a new added file, so why from 2011. I am not very clear about the copyright stuff. Line 2: # Line 3: # This program is free software; you can redistribute it and/or modify Line 4: # it under the terms of the GNU General Public License as published by Line 5: # the Free Software Foundation; either version 2 of the License, or Line 42: try: Line 43: ipaddr = self.ip.inet.address Line 44: netmask = self.ip.inet.netmask Line 45: gateway = self.ip.inet.gateway Line 46: bootproto = self.ip.bootproto how do you like moving the details into class IpConfig? the code could look like: ipaddr, netmask, gateway, bootproto = self.ip.getConfig() Line 47: except AttributeError: Line 48: ipaddr = netmask = gateway = bootproto = None Line 49: return ipaddr, netmask, gateway, bootproto Line 50: Line 50: Line 51: Line 52: class Nic(NetDevice): Line 53: @classmethod Line 54: def objectivize(cls, name, configurator, mtu, _netinfo, bonding=None): the parameter 'bonding' could be removed. Line 55: if name not in _netinfo.nics: Line 56: raise ConfigNetworkError(ne.ERR_BAD_NIC, 'unknown nic: %s' % name) Line 57: nicBond = _netinfo.getBondingForNic(name) Line 58: if not bonding and nicBond or \ Line 57: nicBond = _netinfo.getBondingForNic(name) Line 58: if not bonding and nicBond or \ Line 59: bonding and nicBond and nicBond != bonding: Line 60: raise ConfigNetworkError(ne.ERR_USED_NIC, 'nic %s already enslaved' Line 61: ' to %s' % (name, nicBond)) I prefer to move the validation for bonding to line 172. I think it makes sense to validate if the slave is ok to be used for the master in the master's objectivize(). Otherwise, we need consider different cases in slave's objectivize. Line 62: return cls(name, configurator, mtu=mtu if bonding else Line 63:max(mtu, int(netinfo.getMtu(name Line 64: Line 65: def configure(self, network=None, bridge=None, bonding=None, vlan=None, Line 59: bonding and nicBond and nicBond != bonding: Line 60: raise ConfigNetworkError(ne.ERR_USED_NIC, 'nic %s already enslaved' Line 61: ' to %s' % (name, nicBond)) Line 62: return cls(name, configurator, mtu=mtu if bonding else Line 63:max(mtu, int(netinfo.getMtu(name you don't need consider the bonding case for mtu here. because we restore nic's mtu to default(1500) on removal, so max(mtu, int(netinfo.getMtu(name)) could cover all cases. Do you think so? Line 64: Line 65: def configure(self, network=None, bridge=None, bonding=None, vlan=None, Line 66: **opts): Line 67: self.configurator.configureNic(self, network=network, bridge=bridge, Line 70: def remove(self, network=None, bond=None): Line 71: self.configurator.removeNic(self, network=network, bond=bond) Line 72: Line 73: def __repr__(self): Line 74: return 'Nic(' + self.name + ')' style problem, is 'Nic( %s )' % self.name better? Line 75: Line 76: Line 77: class Vlan(NetDevice): Line 78: MAX_VLAN_ID = 4094 Line 114: '''This class represents traditional kernel bridges.''' Line 115: MAX_BRIDGE_NAME_LEN = 15 Line 116: ILLEGAL_BRIDGE_CHARS = frozenset(':. \t') Line 117: Line 118: def __init__(self, name, configurator, ipconfig=None, mtu=None, ports=(), currently, we only use netmodels to configure or remove network entities, so I think just one port should be enough. We never connect two interfaces into one bridge, right? If we use netmodels to represent netinfo in future, we could add a tuple to represent all ports connected to the bridge including vnic if necessary. Line 119: forwardDelay=0, stp=None): Line 120: self.validateName(name) Line 121: self.ports = ports Line 122: self.forwardDelay = forwardDelay Line 167:
Change in vdsm[master]: NetReload: netmodels for addNetwork
Antoni Segura Puimedon has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 4: (5 inline comments) File vdsm/configNetwork.py Line 47: MAX_BRIDGE_NAME_LEN = 15 Line 48: ILLEGAL_BRIDGE_CHARS = frozenset(':. \t') Line 49: Line 50: Line 51: def objectivizeNetwork(net, bridged=None, vlan=None, bonding=None, Done Line 52:bondingOptions=None, nics=None, mtu=None, ipaddr=None, Line 53:netmask=None, gateway=None, bootproto=None, Line 54:_netinfo=None, configurator=None, **opts): Line 55: if configurator is None: Line 71: if bridged: Line 72: topObj = Bridge(net, configurator, ports=[topObj] if topObj else [], Line 73: mtu=mtu, stp=opts.get('stp'), Line 74: forwardDelay=opts.get('forward_delay', 0)) Line 75: topObj.ip = IpConfig(inet=IPv4(ipaddr, netmask, gateway), Done Line 76: bootproto=bootproto) Line 77: return topObj Line 78: Line 79: Line 76: bootproto=bootproto) Line 77: return topObj Line 78: Line 79: Line 80: def objectivizeBond(bonding, bondingOptions, nics, mtu, _netinfo, Done Line 81: configurator): Line 82: if bonding and nics: Line 83: slaves = [] Line 84: bondMtu = netinfo.getMaxMtu(nics, mtu) Line 115: raise ConfigNetworkError(ne.ERR_BAD_BONDING, 'Multiple nics require a ' Line 116: 'bonding device') Line 117: nic = nics[0] Line 118: nicBond = _netinfo.getBondingForNic(nic) Line 119: if nic not in _netinfo.nics: Done Line 120: raise ConfigNetworkError(ne.ERR_BAD_NIC, 'unknown nic: %r' % nic) Line 121: elif nicBond and nicBond != bonding: Line 122: raise ConfigNetworkError(ne.ERR_USED_NIC, 'nic %s already enslaved to ' Line 123: '%s' % (nic, nicBond)) Line 119: if nic not in _netinfo.nics: Line 120: raise ConfigNetworkError(ne.ERR_BAD_NIC, 'unknown nic: %r' % nic) Line 121: elif nicBond and nicBond != bonding: Line 122: raise ConfigNetworkError(ne.ERR_USED_NIC, 'nic %s already enslaved to ' Line 123: '%s' % (nic, nicBond)) Done Line 124: return Nic(nic, configurator, mtu=mtu) Line 125: Line 126: Line 127: def _validateInterNetworkCompatibility(ni, vlan, iface, bridged): -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
oVirt Jenkins CI Server has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 5: Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/1564/ (1/3) -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
oVirt Jenkins CI Server has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 5: Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/2398/ (3/3) -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
oVirt Jenkins CI Server has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 5: Build Successful http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/2398/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/1564/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/2466/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
oVirt Jenkins CI Server has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 6: Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/1565/ (1/3) -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
oVirt Jenkins CI Server has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 6: Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/2467/ (3/3) -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
oVirt Jenkins CI Server has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 6: Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/2399/ (2/3) -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
oVirt Jenkins CI Server has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 6: Fails Build Failed http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/2399/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/1565/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/2467/ : FAILURE -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
oVirt Jenkins CI Server has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 7: Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/1566/ (1/3) -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
oVirt Jenkins CI Server has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 7: Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/2468/ (2/3) -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
oVirt Jenkins CI Server has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 7: Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/2400/ (3/3) -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
oVirt Jenkins CI Server has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 7: Build Successful http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/2400/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/1566/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/2468/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
Antoni Segura Puimedon has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 7: Verified -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
Antoni Segura Puimedon has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 4: (6 inline comments) File tests/configNetworkTests.py Line 52: self.assertEqual(cneContext.exception.errCode, errCode) Line 53: Line 54: # Monkey patch the real network detection from the netinfo module. Line 55: @MonkeyPatch(netinfo, 'networks', _fakeNetworks) Line 56: @MonkeyPatch(netinfo, 'getMaxMtu', lambda *x: 1500) The reason why I have to monkey patch them is because when something goes wrong, e.g., you made a change to the validation code that lets an erroneous case through to the configuration applying stage, you want the test to abort without affecting the testing machine (nothing would happen if the tests are not run as root, but if you do, you can mess up your dev machine networking). Line 57: @MonkeyPatch(netconf.ifcfg, 'ifdown', lambda *x: raiseInvalidOpException()) Line 58: @MonkeyPatch(netconf.ifcfg, 'ifup', lambda *x: raiseInvalidOpException()) Line 59: @MonkeyPatch(Bond, 'configure', lambda *x: raiseInvalidOpException()) Line 60: @MonkeyPatch(Bridge, 'configure', lambda *x: raiseInvalidOpException()) File vdsm/configNetwork.py Line 50: Line 51: def objectivizeNetwork(net, bridged=None, vlan=None, bonding=None, Line 52:bondingOptions=None, nics=None, mtu=None, ipaddr=None, Line 53:netmask=None, gateway=None, bootproto=None, Line 54:_netinfo=None, configurator=None, **opts): In Python the default arguments are not processed on each call, but on module loading. Thus, this change that you propose would make that all the calls would share an instance instead of getting a new one each time (like the current code does). Line 55: if configurator is None: Line 56: configurator = Ifcfg() Line 57: if _netinfo is None: Line 58: _netinfo = netinfo.NetInfo() File vdsm/netconf/ifcfg.py Line 63: if self.configWriter: Line 64: self.configWriter = None Line 65: self._libvirtAdded = set() Line 66: Line 67: def configureBridge(self, bridge, network=None, **opts): Different configurators are the reason for that. I will check other possibilities for this that maintain the ability to use multiple configurators. Line 68: try: Line 69: ipaddr = bridge.ip.inet.address Line 70: netmask = bridge.ip.inet.netmask Line 71: gateway = bridge.ip.inet.gateway Line 74: ipaddr = netmask = gateway = bootproto = None Line 75: self.configWriter.addBridge(bridge.name, ipaddr=ipaddr, Line 76: netmask=netmask, mtu=bridge.mtu, Line 77: gateway=gateway, bootproto=bootproto, Line 78: **opts) I think that is a good idea. I wanted to leave configWriter untouched, but this single change makes sense. Thanks for proposing it. Line 79: ifdown(bridge.name) Line 80: for port in bridge.ports: Line 81: port.configure(bridge=bridge.name, **opts) Line 82: ifup(bridge.name, bootproto == 'dhcp' and File vdsm/netmodels.py Line 118: def __repr__(self): Line 119: return 'Bridge(' + self.name + ')' + '\n|-' + '\n|-'.join( Line 120: [repr(port) for port in self.ports]) Line 121: Line 122: def configure(self, **opts): It is passed through as part of the opts, but since explicit is better than implicit I'll change that. Line 123: self.configurator.configureBridge(self, **opts) Line 124: Line 125: def remove(self, force=False): Line 126: logging.debug('Removing bridge %r with ports = %s', self.name, Line 165: logging.debug('Removing bond %r with nics = %s', self.name, Line 166: self.slaves) Line 167: self.configurator.removeBond(self.name, self.slaves) Line 168: Line 169: @staticmethod Just that it is not using class information for anything and thus the class explicit parameter would be useless. I'm not opposed to changing it to class method though... Line 170: def validateName(name): Line 171: if not re.match('^bond[0-9]+$', name): Line 172: raise ConfigNetworkError(ne.ERR_BAD_BONDING, Line 173: '%r is not a valid bonding device name' % -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master
Change in vdsm[master]: NetReload: netmodels for addNetwork
Antoni Segura Puimedon has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 4: (1 inline comment) File tests/configNetworkTests.py Line 54: # Monkey patch the real network detection from the netinfo module. Line 55: @MonkeyPatch(netinfo, 'networks', _fakeNetworks) Line 56: @MonkeyPatch(netinfo, 'getMaxMtu', lambda *x: 1500) Line 57: @MonkeyPatch(netconf.ifcfg, 'ifdown', lambda *x: raiseInvalidOpException()) Line 58: @MonkeyPatch(netconf.ifcfg, 'ifup', lambda *x: raiseInvalidOpException()) The references to ifdown and ifup should be taken out as soon as those two are not called outside of configurator code. Line 59: @MonkeyPatch(Bond, 'configure', lambda *x: raiseInvalidOpException()) Line 60: @MonkeyPatch(Bridge, 'configure', lambda *x: raiseInvalidOpException()) Line 61: @MonkeyPatch(Nic, 'configure', lambda *x: raiseInvalidOpException()) Line 62: @MonkeyPatch(Vlan, 'configure', lambda *x: raiseInvalidOpException()) -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
Antoni Segura Puimedon has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 4: (2 inline comments) File vdsm/configNetwork.py Line 80: def objectivizeBond(bonding, bondingOptions, nics, mtu, _netinfo, Line 81: configurator): Line 82: if bonding and nics: Line 83: slaves = [] Line 84: bondMtu = netinfo.getMaxMtu(nics, mtu) Good catch. I seem to remember that I kept it from old code, but it makes sense to drop it. Line 85: for nic in nics: Line 86: _validateNicUnused(nic, _netinfo) Line 87: slaves.append(Nic(nic, configurator, mtu=bondMtu)) Line 88: elif bonding in _netinfo.bondings: # Implicit bonding. Line 86: _validateNicUnused(nic, _netinfo) Line 87: slaves.append(Nic(nic, configurator, mtu=bondMtu)) Line 88: elif bonding in _netinfo.bondings: # Implicit bonding. Line 89: nics = [nic for nic in _netinfo.getNicsForBonding(bonding)] Line 90: bondMtu = netinfo.getMaxMtu(nics, mtu) Done Line 91: slaves = [Nic(nic, configurator, mtu=bondMtu) for nic in nics] Line 92: bondingOptions = _netinfo.bondings[bonding]['cfg'].get( Line 93: 'BONDING_OPTS') Line 94: else: -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com 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]: NetReload: netmodels for addNetwork
Mark Wu has posted comments on this change. Change subject: NetReload: netmodels for addNetwork .. Patch Set 4: (1 inline comment) File vdsm/configNetwork.py Line 71: if bridged: Line 72: topObj = Bridge(net, configurator, ports=[topObj] if topObj else [], Line 73: mtu=mtu, stp=opts.get('stp'), Line 74: forwardDelay=opts.get('forward_delay', 0)) Line 75: topObj.ip = IpConfig(inet=IPv4(ipaddr, netmask, gateway), It's also possible to just configure bonding from UI without network attached. So it's better to add IpConfig only if ipaddr is not None. Line 76: bootproto=bootproto) Line 77: return topObj Line 78: Line 79: -- To view, visit http://gerrit.ovirt.org/14303 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba5c5b84760e27245cbe34c3b290c54e51278e72 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches