Nir Soffer has posted comments on this change. Change subject: v2v: Add convertExternalVm for XmlRpc ......................................................................
Patch Set 2: Code-Review-1 (4 comments) Need to fix password handling and and docs, and make names match the schema or the help. https://gerrit.ovirt.org/#/c/40003/2/client/vdsClient.py File client/vdsClient.py: Line 1926: Line 1927: def convertExternalVm(self, args): Line 1928: validateArgTypes(args, [str, str, str, parse_dict, str], Line 1929: requiredArgsNumber=5) Line 1930: uri, username, password, properties, job_id = args - password should be auth (match the help, this raw auth string) - properties should be vminfo (match the schema) - job_id should be jobid (match the schema) Line 1931: password = getAuthFromArgs(args, password) Line 1932: response = self.s.convertExternalVm(uri, username, password, Line 1933: properties, job_id) Line 1934: if response['status']['code'] != 0: Line 1927: def convertExternalVm(self, args): Line 1928: validateArgTypes(args, [str, str, str, parse_dict, str], Line 1929: requiredArgsNumber=5) Line 1930: uri, username, password, properties, job_id = args Line 1931: password = getAuthFromArgs(args, password) Should be: password = getPassword(auth) If we like to support also plain password: if auth.startswith('auth='): password = getPassword(auth) else: password = auth Not sure that we do want this - need to check what other verb support. Line 1932: response = self.s.convertExternalVm(uri, username, password, Line 1933: properties, job_id) Line 1934: if response['status']['code'] != 0: Line 1935: return response['status']['code'], response['status']['message'] Line 1929: requiredArgsNumber=5) Line 1930: uri, username, password, properties, job_id = args Line 1931: password = getAuthFromArgs(args, password) Line 1932: response = self.s.convertExternalVm(uri, username, password, Line 1933: properties, job_id) same Line 1934: if response['status']['code'] != 0: Line 1935: return response['status']['code'], response['status']['message'] Line 1936: return 0, 'Job started' Line 1937: Line 2818: 'Argumemnts:', Line 2819: ' uri: uri of external system (vmware etc)', Line 2820: ' username: login name for given uri', Line 2821: ' auth: password for given uri, can be: ', Line 2822: ' file:path or auth=env:name or auth=pass:password', Are you sure this is the expected input? I think it should be: auth=file:path or auth=env:name or auth=pass:password Do we want to support plain password also? Please check other code using this auth mechanism and be consistent. Line 2823: ' vminfo: string dictionary, parameter for import:', Line 2824: ' {"vmName": "name",', Line 2825: ' "poolId":, "<Guid>",', Line 2826: ' "domainId": "<Guid",', -- 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: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Shahar Havivi <shav...@redhat.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: Francesco Romani <from...@redhat.com> Gerrit-Reviewer: Michal Skrivanek <michal.skriva...@redhat.com> Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com> Gerrit-Reviewer: Shahar Havivi <shav...@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
