Change in vdsm[master]: network/api.py: delete network's orphans

asegurap at redhat.com asegurap at redhat.com
Fri Aug 8 08:26:07 UTC 2014


Antoni Segura Puimedon has posted comments on this change.

Change subject: network/api.py: delete network's orphans
......................................................................


Patch Set 4: Code-Review-1

(4 comments)

http://gerrit.ovirt.org/#/c/31018/4/vdsm/network/api.py
File vdsm/network/api.py:

Line 588: 
Line 589: 
Line 590: def _emergencyNetworkCleanup(network, networkAttrs, configurator):
Line 591:     """Remove all leftovers after failed setupNetwork"""
Line 592:     d = dict(networkAttrs)
isn't networkAttrs already a dictionary? Why the casting/renaming?
Line 593:     _netinfo = netinfo.NetInfo()
Line 594: 
Line 595:     def _used(devName):
Line 596:         masters = _netinfo.ifaceUsers(devName)


Line 609:                 topNetDev = Bond.objectivize(d['bonding'], configurator, None,
Line 610:                                              None, None, _netinfo, True)
Line 611:     elif 'nic' in d:
Line 612:         if d['nic'] in _netinfo.nics:
Line 613:             if not _used(d['nic']):
Will this not be checked already by the configurator removeNic code?
Line 614:                 topNetDev = Nic(d['nic'], configurator, _netinfo=_netinfo)
Line 615:     if 'vlan' in d:
Line 616:         vlan_name = '%s.%s' % (topNetDev.name, d['vlan'])
Line 617:         if vlan_name in _netinfo.vlans:


Line 611:     elif 'nic' in d:
Line 612:         if d['nic'] in _netinfo.nics:
Line 613:             if not _used(d['nic']):
Line 614:                 topNetDev = Nic(d['nic'], configurator, _netinfo=_netinfo)
Line 615:     if 'vlan' in d:
Note that vlan will only happen if 'bonding' in d or 'nic' in d.
Line 616:         vlan_name = '%s.%s' % (topNetDev.name, d['vlan'])
Line 617:         if vlan_name in _netinfo.vlans:
Line 618:             topNetDev = Vlan(topNetDev, d['vlan'], configurator)
Line 619:     if d['bridged']:


Line 738:                 addNetwork(network, configurator=configurator,
Line 739:                            implicitBonding=True, _netinfo=_netinfo, **d)
Line 740:             except ConfigNetworkError:
Line 741:                 logger.debug("Adding network %r failed. Running "
Line 742:                              "orphan-devices cleanup", network)
If I understand correctly _emergencyNetworkCleanup was designed for when ConfigNetworkError is raised with the network/error FAILED_IFUP. Here you are calling it for any ConfigNetworkErrror.

It would probably be best if you check error codes that might need it and error codes that don't.
Line 743:                 _emergencyNetworkCleanup(network, networkAttrs, configurator)
Line 744:                 raise
Line 745: 
Line 746:             _netinfo.updateDevices()  # Things like a bond mtu can change


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id14d109d67c5906bc85a2fcf98dfabd61d96da13
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Petr Horáček <phoracek at redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list