Change in vdsm[master]: v2v: Add convertExternalVm for XmlRpc

nsoffer at redhat.com nsoffer at redhat.com
Fri May 1 13:20:22 UTC 2015


Nir Soffer has posted comments on this change.

Change subject: v2v: Add convertExternalVm for XmlRpc
......................................................................


Patch Set 4:

(4 comments)

https://gerrit.ovirt.org/#/c/40003/4/client/vdsClient.py
File client/vdsClient.py:

Line 173:         return getPassword(args['auth'])
Line 174:     return default
Line 175: 
Line 176: 
Line 177: def parsePassword(string):
Having both parsePassword and getPassword is little bit confusing.
Line 178:     if string.startswith('auth='):
Line 179:         return getPassword(string.split('=')[1])
Line 180:     return string
Line 181: 


Line 1921:     def convertExternalVm(self, args):
Line 1922:         validateArgTypes(args, [str, str, str, parse_dict, str],
Line 1923:                          requiredArgsNumber=5)
Line 1924:         uri, username, auth, vminfo, jobid = args
Line 1925:         passwd = parsePassword(auth)
Instead of adding new confusing parsePassword function, you can use the existing infrastructure:

    d = parseArgs(auth)
    password = getAuthFromArgs(d, auth)

It is very good infrastructure, but we should fix all places parsing auth=, not add multiple incompatible ways to do the same thing.
Line 1926:         response = self.s.convertExternalVm(uri, username, passwd, vminfo,
Line 1927:                                             jobid)
Line 1928:         if response['status']['code'] != 0:
Line 1929:             return response['status']['code'], response['status']['message']


Line 2804:                 '  uri:      uri of external system (vmware etc)',
Line 2805:                 '  username: login name for given uri',
Line 2806:                 '  auth:     password for given uri, can be: ',
Line 2807:                 '            plain text password or auth=file:path '
Line 2808:                 'or auth=env:name or auth=pass:password',
Should be indented so it aligns with "plain text..."
Line 2809:                 '  vminfo:   Python dictionary, parameter for import:',
Line 2810:                 '            {"vmName": "name",',
Line 2811:                 '             "poolId":, "<UUID>",',
Line 2812:                 '             "domainId": "<UUID",',


Line 2805:                 '  username: login name for given uri',
Line 2806:                 '  auth:     password for given uri, can be: ',
Line 2807:                 '            plain text password or auth=file:path '
Line 2808:                 'or auth=env:name or auth=pass:password',
Line 2809:                 '  vminfo:   Python dictionary, parameter for import:',
This must be quoted, so it will parse as single command line argument.
Line 2810:                 '            {"vmName": "name",',
Line 2811:                 '             "poolId":, "<UUID>",',
Line 2812:                 '             "domainId": "<UUID",',
Line 2813:                 '             "disks": [{"volumeId": "<UUID>",',


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I94d7886e1295f5e98d0fafc8837567ff383a6c7a
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shahar Havivi <shavivi at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Shahar Havivi <shavivi at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list