Change in vdsm[master]: net: Mapping bonding option value names to numerics

edwardh at redhat.com edwardh at redhat.com
Sun Dec 20 07:18:23 UTC 2015


Edward Haas has posted comments on this change.

Change subject: net: Mapping bonding option value names to numerics
......................................................................


Patch Set 25:

(3 comments)

https://gerrit.ovirt.org/#/c/49390/25/lib/vdsm/netinfo/bonding.py
File lib/vdsm/netinfo/bonding.py:

Line 209: try:
        :         opt_num_val = bond_opts_map[mode_num][option_name][val_name]
        :     except KeyError:
        :         opt_num_val = None
> this can be also written as:
I understand, the idea was to protect all dics because it is a module api. On the other hand, it should not happen as far as I know.
Unit Tests cover unsupported options and the input of options is coming from the saved configuration.
Seems safer to handle all possible errors in one shot, but I'm not sure which is better, let me know if you still think I should change it.


https://gerrit.ovirt.org/#/c/49390/25/lib/vdsm/tool/dump_bonding_opts.py
File lib/vdsm/tool/dump_bonding_opts.py:

Line 82: with _bond_device(bond_name):
> you could just move the context manager to wrap only the statement that is 
_scan_modes uses _change_mode to set the bond mode.
_bond_device creates a bond.
Therefore, you cannot call _scan_mode outside the context (it expects the bond to exist).


Line 119: with open(BONDING_MASTERS, 'w') as bonds:
        :         bonds.write('-' + bond_name)
        :         bonds.flush()
        :         bonds.write('+' + bond_name)
> but this can be replaced with:
See previous response.


-- 
To view, visit https://gerrit.ovirt.org/49390
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1f4c16b1523822a2d53d4525841ff8741af6296c
Gerrit-PatchSet: 25
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Edward Haas <edwardh at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Edward Haas <edwardh at redhat.com>
Gerrit-Reviewer: Ido Barkan <ibarkan at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda <osvoboda at redhat.com>
Gerrit-Reviewer: Petr Horáček <phoracek at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list