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 <osvob...@redhat.com> Gerrit-Reviewer: Antoni Segura Puimedon <asegu...@redhat.com> Gerrit-Reviewer: Assaf Muller <amul...@redhat.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: Ondřej Svoboda <osvob...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
