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

danken at redhat.com danken at redhat.com
Tue May 20 14:28:39 UTC 2014


Dan Kenigsberg has posted comments on this change.

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


Patch Set 8: Code-Review-1

(2 comments)

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

Line 187:                 el for el in optFile.read().rstrip().split(' ') if el]
Line 188:     return opts
Line 189: 
Line 190: 
Line 191: def _realBondOpts(bond, keys=None):
> The optional parameter should be dropped. If the list of options to retriev
you are perfectly right. Please drop that arg!
Line 192:     """
Line 193:     Return a dictionary in the same format as bondOpts(). Exclude entries that
Line 194:     are not bonding options, e.g. 'ad_num_ports' or 'slaves'.
Line 195:     """


Line 623:     Return non-empty options differing from defaults, excluding not actual or
Line 624:     not applicable options, e.g. 'ad_num_ports' or 'slaves'.
Line 625:     """
Line 626:     opts = _realBondOpts(bond)
Line 627:     mode = opts['mode'][-1] if 'mode' in opts else None
> This function is called even before a bond exists hence the condition is ne
who calls _getBondingOptions() on a non-existing bond? I see no such caller in this patch, and I believe that no one should.
Line 628:     defaults = _getDefaultBondingOptions(mode)
Line 629: 
Line 630:     return dict(((opt, val) for (opt, val) in opts.iteritems()
Line 631:                  if val and val != defaults.get(opt)))


-- 
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: 8
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: Dan Kenigsberg <danken 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