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