Nir Soffer has posted comments on this change.

Change subject: v2v: add support for importing kvm based vms from libvirt
......................................................................


Patch Set 1: Code-Review+1

(3 comments)

Nice, needs some minor fixes.

https://gerrit.ovirt.org/#/c/56459/1/lib/vdsm/v2v.py
File lib/vdsm/v2v.py:

Line 345
Line 346
Line 347
Line 348
Line 349
Please remove this blank line.


Line 157:     if uri.startswith(_XEN_SSH_PROTOCOL):
Line 158:         command = XenCommand(uri, vminfo, job_id, irs)
Line 159:     elif uri.startswith(_VMWARE_PROTOCOL):
Line 160:         command = LibvirtCommand(uri, username, password, vminfo, 
job_id, irs)
Line 161:     else:
Please add constant for _KVM_PROTOCOL and do not assume that this is the only 
possible option. If we get another protocol, fail with ClientError.
Line 162:         command = KVMCommand(uri, username, password, vminfo, job_id, 
irs)
Line 163:     job = ImportVm(job_id, command)
Line 164:     job.start()
Line 165:     _add_job(job_id, job)


Line 572: 
Line 573:     @contextmanager
Line 574:     def execute(self):
Line 575:         with self._volumes(), self._password_file():
Line 576:             yield self._start_virt_v2v()
_start_virt_v2v is not correct now, since it may run kvm2ovirt or virt-v2v. 
Lets use a more generic name like start_helper
Line 577: 
Line 578:     def _source_images(self):
Line 579:         con = libvirtconnection.open_connection(uri=self._uri,
Line 580:                                                 
username=self._username,


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0ddbab700f2e0c54d53ed02ec61477b92c9c7960
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shahar Havivi <[email protected]>
Gerrit-Reviewer: Francesco Romani <[email protected]>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <[email protected]>
Gerrit-Reviewer: gerrit-hooks <[email protected]>
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to