Ondřej Svoboda has posted comments on this change.
Change subject: netinfo: Retrieve bonding options differing from defaults
......................................................................
Patch Set 20:
(2 comments)
The functional tests are being solved by making sure only the expected options (reverting
any divergence) are applied when adding/changing a bond.
http://gerrit.ovirt.org/#/c/24456/20/lib/vdsm/netinfo.py
File lib/vdsm/netinfo.py:
Line 534: return ''.join(random.choice(CHARS) for _ in range(MAX_LENGTH))
Line 535:
Line 536:
Line 537: @memoized
Line 538: def _getDefaultBondingOptions():
please document the two-level dictionary returned here
Actually, the nature of the bonding options is that only the 802.3ad (4) mode uses a
few specific, extra options whose values are empty for other modes. The rest of the
options is all the same, both key-wise and value-wise.
The simplicity of the design makes great sense here and I suppose the concept of modes
sharing the defaults is going to stay. Thus I propose to combine the defaults in a single
dictionary.
Line 539: teeCmd = _TEE_BINARY.cmd
Line 540: MAX_MODE = 6
Line 541:
Line 542: bondName = _randomIfaceName()
Line 569: def _getBondingOptions(bond):
Line 570: '''
Line 571: Returns non-empty options differing from defaults, excluding e.g.
'slaves'.
Line 572:
Line 573: A string key=value representation (as in ifcfg) is also returned for
A function should do one thing. Please split the dict-to-legacy
conversion
Agreed, locally, I have already moved the code out.
Line 574: backwards compatibility reasons.
Line 575: '''
Line 576: EXCLUDED = frozenset(('slaves', 'active_slave',
'mii_status', 'queue_id'))
Line 577:
--
To view, visit
http://gerrit.ovirt.org/24456
To unsubscribe, visit
http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ief6d366b1b761627c7203cf236b75ef538af3e26
Gerrit-PatchSet: 20
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda <osvoboda(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Assaf Muller <amuller(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Ondřej Svoboda <osvoboda(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes