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

danken at redhat.com danken at redhat.com
Wed Feb 10 15:24:03 UTC 2016


Dan Kenigsberg has posted comments on this change.

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


Patch Set 7: Code-Review-1

(4 comments)

https://gerrit.ovirt.org/#/c/53100/7/lib/vdsm/netinfo/bonding.py
File lib/vdsm/netinfo/bonding.py:

Line 73: "
nit: we try to be consistent and use only single quotes for strings in most modules.


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

Line 18
Line 19
Line 20
Line 21
Line 22
functools comes before logging, alphabetically. Please do not move.


Line 28
Line 29
Line 30
Line 31
Line 32
no need to change whitespace in this patch


Line 79:         ad_partner_mac as well.
Line 80:     """
Line 81:     sys_nic_path = os.path.join(NET_PATH, nic_name)
Line 82:     ad_aggregator_id = ad_partner_mac = None
Line 83:     if bond:
these are basically two different functions that are coerced to live together.

I'd rather have 

 _get_slave_agg_id(nic_name)

and

 _get_bond_agg_id(bond_name)

The latter may be renamed and return a tuple of (agg_id, partner_mac)
Line 84:         agg_id_path = os.path.join(sys_nic_path, 'bonding', 'ad_aggregator')
Line 85:         ad_mac_path = os.path.join(sys_nic_path, 'bonding', 'ad_partner_mac')
Line 86:     else:
Line 87:         agg_id_path = os.path.join(sys_nic_path, 'bonding_slave',


-- 
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: 7
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