Change in vdsm[master]: setupNetworks: moving nics between bonds

fromani at redhat.com fromani at redhat.com
Fri Jan 29 10:18:04 UTC 2016


Francesco Romani has posted comments on this change.

Change subject: setupNetworks: moving nics between bonds
......................................................................


Patch Set 9: Code-Review+1

(3 comments)

only minor nits in case you need to resubmit.

https://gerrit.ovirt.org/#/c/50596/9/lib/vdsm/network/api.py
File lib/vdsm/network/api.py:

Line 694:         logger.debug("Creating bond %r with options %s", bond, bond.options)
Line 695:         configurator.configureBond(bond)
Line 696: 
Line 697: 
Line 698: def _calculate_all_removed_slaves(edited_bonds, _netinfo, configurator):
nit: are we calculating removed (-> action done in the past) slaves or all the slave we need to remove (-> action about to be done)? see also below
Line 699:     all_removed_slaves = []
Line 700:     for name, attrs in edited_bonds:
Line 701:         slaves_removed_from_bond = (set(_netinfo.bondings[name]['slaves']) -
Line 702:                                     set(attrs.get('nics')))


Line 703:         all_removed_slaves.extend(slaves_removed_from_bond)
Line 704:     return all_removed_slaves
Line 705: 
Line 706: 
Line 707: def _remove_slaves(removed_slaves, configurator, _netinfo):
same nit: feels weird to remove already removed slaves :)
Line 708:     for name in removed_slaves:
Line 709:         slave = Nic(name, configurator, _netinfo=_netinfo)
Line 710:         slave.remove(remove_even_if_used=True)
Line 711: 


https://gerrit.ovirt.org/#/c/50596/9/lib/vdsm/network/configurators/iproute2.py
File lib/vdsm/network/configurators/iproute2.py:

Line 192:         else:
Line 193:             self._setNewMtu(bonding, vlans.vlan_devs_for_iface(bonding.name))
Line 194: 
Line 195:     def removeNic(self, nic, remove_even_if_used=False):
Line 196:         """ The nic is not removed from a bond, if slave of any.
I don't know about the style in network code, but
we usually prefer
"""
to write the docstrings
this way
"""
Line 197:         This needs to be done explicitly.
Line 198:         """
Line 199:         toBeRemoved = not ifaceUsed(nic.name) or remove_even_if_used
Line 200: 


-- 
To view, visit https://gerrit.ovirt.org/50596
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia43fbfe90d1335ed4a740a1bc4f09610da2ecdce
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Marcin Mirecki <mmirecki at redhat.com>
Gerrit-Reviewer: Alona Kaplan <alkaplan at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Edward Haas <edwardh at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Ido Barkan <ibarkan at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Marcin Mirecki <mmirecki at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list