Change in vdsm[master]: Advertise aggregator ID in bonding interfaces

edwardh at redhat.com edwardh at redhat.com
Wed Feb 10 14:59:08 UTC 2016


Edward Haas has posted comments on this change.

Change subject: Advertise aggregator ID in bonding interfaces
......................................................................


Patch Set 6:

(2 comments)

https://gerrit.ovirt.org/#/c/53100/6/lib/vdsm/netinfo/nics.py
File lib/vdsm/netinfo/nics.py:

Line 73:     except IOError:
Line 74:         return 0
Line 75: 
Line 76: 
Line 77: def _get_aggregator_id(nic_name, bond=False):
> Like I wrote to @danken in prev. revision: this is a pure NIC attribute, al
My recommendation is to report this nic information through the bond slaves, in the bond context.
Under the bond module, we know all slaves have these attributes, from a nic perspective we need to ask it for each nic... and more or less say that nics module needs to use bonding logic (although not directly imported).
Line 78:     """Read ad_aggregator_id of NIC. If it's bond, read
Line 79:         ad_partner_mac as well.
Line 80:     """
Line 81:     sys_nic_path = NET_CONFIG.format(nic=nic_name)


https://gerrit.ovirt.org/#/c/53100/6/tests/functional/networkTests.py
File tests/functional/networkTests.py:

Line 2827: 
Line 2828:     @permutations([[{}], [{'options': 'mode=4 lacp_rate=1 miimon=100'}]])
Line 2829:     @cleanupNet
Line 2830:     @ValidateRunningAsRoot
Line 2831:     def test_bond_mode4_caps_aggregator_id(self, mode4):
> What do you mean "everything is none"? If bond mode is default, the aggrega
I mean, if you add this test and the new production code does not exist, it will still pass (at least, most of the asserts will pass)... Making the test just document usage, but not checking anything.
Line 2832:         with dummyIf(3) as nics:
Line 2833:             bonding = {'nics': nics[:2]}
Line 2834:             bonding.update(mode4)
Line 2835:             status, msg = self.setupNetworks(


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I85267967c9cb1b0a626d91cb1953361ed4de727a
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Sagi Shnaidman <sshnaidm at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Edward Haas <edwardh at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horáček <phoracek at redhat.com>
Gerrit-Reviewer: Sagi Shnaidman <sshnaidm at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list