Change in vdsm[master]: netinfo: Read default bonding options

osvoboda at redhat.com osvoboda at redhat.com
Wed May 14 16:12:22 UTC 2014


Ondřej Svoboda has posted comments on this change.

Change subject: netinfo: Read default bonding options
......................................................................


Patch Set 1:

(4 comments)

Addressed in a follow-up iteration.

http://gerrit.ovirt.org/#/c/27680/1/lib/vdsm/netinfo.py
File lib/vdsm/netinfo.py:

Line 184:     return opts
Line 185: 
Line 186: 
Line 187: def realBondOpts(bond, keys=None):
Line 188:     '''
> s/'''/"""/ for docstrings
Done
Line 189:     Returns a dictionary in the same format as bondOpts(). Values that are not
Line 190:     actually options are excluded, e.g. 'ad_num_ports' or 'slaves'.
Line 191:     '''
Line 192:     EXCLUDED = frozenset(('slaves',  'active_slave', 'mii_status', 'queue_id',


Line 193:                           'ad_aggregator', 'ad_num_ports', 'ad_actor_key',
Line 194:                           'ad_partner_key', 'ad_partner_mac'))
Line 195: 
Line 196:     opts = ((opt, val) for (opt, val) in bondOpts(bond, keys).iteritems()
Line 197:             if opt not in EXCLUDED)
> I'd prefer:
Done
Line 198: 
Line 199:     return dict(opts)
Line 200: 
Line 201: 


Line 571: 
Line 572: 
Line 573: @memoized
Line 574: def _getAllDefaultBondingOptions():
Line 575:     '''
> again, s/'''/"""/
Done
Line 576:     Returns default options per mode, in a dictionary of dictionaries. All keys
Line 577:     are strings.
Line 578:     '''
Line 579:     teeCmd = _TEE_BINARY.cmd


Line 574: def _getAllDefaultBondingOptions():
Line 575:     '''
Line 576:     Returns default options per mode, in a dictionary of dictionaries. All keys
Line 577:     are strings.
Line 578:     '''
> what about to have the dictionary have integers as keys and only cast mode 
To stay in line with bondOpts() which never casts numerical values to integers I would keep this. The expression in the next function (defaults['0']['mode'][-1]) would become even more complicated, as well as_getBondingOptions() in a later patch (which reads the mode from bondOpts again as a string).

The dictionary is queried at most once per mode thanks to memoization.
Line 579:     teeCmd = _TEE_BINARY.cmd
Line 580:     MAX_MODE = 6
Line 581: 
Line 582:     bondName = _randomIfaceName()


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If8109feeef02b71c1d74dc6303839c6f45175915
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda <osvoboda at redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap at redhat.com>
Gerrit-Reviewer: Ondřej Svoboda <osvoboda 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