Change in vdsm[master]: addNetwork script: Prevent empty params from reaching Runnin...

danken at redhat.com danken at redhat.com
Mon Sep 29 22:21:51 UTC 2014


Dan Kenigsberg has posted comments on this change.

Change subject: addNetwork script: Prevent empty params from reaching RunningConfig
......................................................................


Patch Set 1: Code-Review-1

(2 comments)

Great catch of an ancient bug. I think that the implementation can be more precise.

http://gerrit.ovirt.org/#/c/33510/1//COMMIT_MSG
Commit Message:

Line 6: 
Line 7: addNetwork script: Prevent empty params from reaching RunningConfig
Line 8: 
Line 9: The consumers of the addNetwork script pass empty parameters, e.g. ""
Line 10: in order to fill the positional arguments that the script takes. The
The only positional argument where this is relevant to are vlan, bonding and nics. bridge is required to be non-empty, and anything past nics is a kwarg.

Let's be prudent and drop only these 3 if they have an empty value.
Line 11: problem with that is that those parameters were being passed to
Line 12: addNetwork without modification, and that includes the
Line 13: _alterNetworkConfig wrapper that makes addNetwork requests reach
Line 14: runningConfig (and eventually PersistentConfig).


http://gerrit.ovirt.org/#/c/33510/1/vdsm/network/api.py
File vdsm/network/api.py:

Line 821:         bridge = sys.argv[2]
Line 822:         kwargs = _parseKwargs(sys.argv[3:])
Line 823:         if 'nics' in kwargs:
Line 824:             kwargs['nics'] = kwargs['nics'].split(',')
Line 825:         # Remove empty keys so that they are not taken by _alterRunningConfig
after nics' conversion above, we should be worried only about vlan and bonding. Please drop only them if they are empty.
Line 826:         for key, value in kwargs.items():
Line 827:             if key != 'bondingOptions' and value == '':
Line 828:                 del kwargs[key]
Line 829:         addNetwork(bridge, **kwargs)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If6d56eefc05cdb7456f80b7ec13d0be8ad087aa3
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Antoni Segura Puimedon <asegurap at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsland 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