Change in vdsm[master]: [WIP] netwiring: [4/4] Add API definitions.

wudxw at linux.vnet.ibm.com wudxw at linux.vnet.ibm.com
Tue Dec 4 08:16:49 UTC 2012


Mark Wu has posted comments on this change.

Change subject: [WIP] netwiring: [4/4] Add API definitions.
......................................................................


Patch Set 14: (3 inline comments)

....................................................
File vdsm/libvirtvm.py
Line 1563:         netParam = params.get('network')
Line 1564:         if netParam is None:
Line 1565:             # If no network is specified we take the vnic to the dummy bridge
Line 1566:             # and set the link 'down' always.
Line 1567:             source.setAttribute('bridge', DUMMY_BRIDGE)
still don't understand why we need move it to dummy bridge. does it have difference compared with just set the link down?
Line 1568:             link.setAttribute('state', 'down')
Line 1569:         else:
Line 1570:             # There is a network defined. Thus, we either just modify the link
Line 1571:             # status and/or move between network backends.


Line 1570:             # There is a network defined. Thus, we either just modify the link
Line 1571:             # status and/or move between network backends.
Line 1572:             if network != netParam:
Line 1573:                 # If a different network is specified. First we take the link
Line 1574:                 # down and then update the device to connect to the new bridge.
you didn't answer my previous question on it. I suppose we don't need to change the link status, and just use one libvirt call to move it to the network.  The guest network stack could handle the missed packets.  I don't see the benefit of 'setting link down' at first.
Line 1575:                 link.setAttribute('state', 'down')
Line 1576:                 try:
Line 1577:                     self._dom.updateDeviceFlags(
Line 1578:                         vnicXML.toxml(encoding='utf-8'),


Line 1614:                 for mirrNet in params.get('portMirroring', []):
Line 1615:                     supervdsm.getProxy().setPortMirroring(mirrNet, netDev.name)
Line 1616:                     mirroredNetworks.append(mirrNet)
Line 1617:             except Exception, e:
Line 1618:                 # In case we fail, we rollback the whole updateIntefaceDevice.
How about using a dict to represent the result for different actions, then we don't need rollback all the change.
Line 1619:                 for mirrNet in mirroredNetworks:
Line 1620:                     supervdsm.getProxy().unsetPortMirroring(mirrNet,
Line 1621:                                                             netDev['name'])
Line 1622:                 # TODO: Rollback link and network.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3b9b4f49f80466a83e3e13f1042ac2a8866c6bcd
Gerrit-PatchSet: 14
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Antoni Segura Puimedon <asegurap at redhat.com>
Gerrit-Reviewer: Alona Kaplan <alkaplan at redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky at redhat.com>
Gerrit-Reviewer: Livnat Peer <lpeer at redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw at linux.vnet.ibm.com>
Gerrit-Reviewer: ShaoHe Feng <shaohef at linux.vnet.ibm.com>
Gerrit-Reviewer: oVirt Jenkins CI Server


More information about the vdsm-patches mailing list