Change in vdsm[master]: vdsClient.py: Allow specifying nested dicts for devices when...

ilvovsky at redhat.com ilvovsky at redhat.com
Sun May 6 08:21:57 UTC 2012


Igor Lvovsky has posted comments on this change.

Change subject: vdsClient.py: Allow specifying nested dicts for devices when creating a vm
......................................................................


Patch Set 1: (4 inline comments)

OK Hans,
I got it, you are right. Our implementation is very partial.
So, thanks for fixing it.
Just several comments inside.
In general it will be great to support nested dictionaries. 
About the proper value types, vdsClient actually try to behave like ovirt-engine and according to our agreement with engine they pass variables as strings and vdsm handles it inside (e.g. {'key1': 'lala', 'ke2': '5', 'key3': 'True'}).

....................................................
File vdsm_cli/vdsClient.py
Line 1394:                     d["index"] = int(d["index"])
According to our agreement with ovirt-engine we want our parameters  over xmlrpc (such as int, bool, ...) as strings. It's mean if you want to pass dict, you should do it like
{'key1': 'lala', 'ke2': '5', 'key3': 'True'}
vdsm will handle it inside properly.

Line 1399:                 raise Exception("missing name value separator ':' in '%s'" % spec)
Maybe I am missing something, but why do you need this one?.
What is spec format you handle here?
It's looks like very partial validation for me.

Line 1420:                         d[name] = False
As I wrote before, we want it strings over xmlrpc.

Line 1559:         'create'  :  ( serv.do_create,
Can you please add proper explanation here?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibe4eb528684c1d2053cb9574d8f63bbbf0a03c23
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Hans De Goede <hdegoede at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Hans De Goede <hdegoede at redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky at redhat.com>


More information about the vdsm-patches mailing list