Change in vdsm[master]: v2v: Import VM from OVA file
Dan Kenigsberg has submitted this change and it was merged. Change subject: v2v: Import VM from OVA file .. v2v: Import VM from OVA file OVA is a tar file which contain a VM with its Ovf file and its disks as well. Change-Id: I0e440748ecc503f4d61e8f4f61bb0c7387589354 Signed-off-by: Shahar Havivi shah...@redhat.com Reviewed-on: https://gerrit.ovirt.org/43367 Reviewed-by: Francesco Romani from...@redhat.com Tested-by: Shahar Havivi shav...@redhat.com Continuous-Integration: Jenkins CI --- M vdsm/API.py M vdsm/rpc/bindingxmlrpc.py M vdsm/rpc/vdsmapi-schema.json M vdsm/v2v.py 4 files changed, 82 insertions(+), 18 deletions(-) Approvals: Shahar Havivi: Verified Jenkins CI: Passed CI tests Francesco Romani: Looks good to me, approved -- To view, visit https://gerrit.ovirt.org/43367 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: I0e440748ecc503f4d61e8f4f61bb0c7387589354 Gerrit-PatchSet: 12 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Shahar Havivi shav...@redhat.com Gerrit-Reviewer: Adam Litke ali...@redhat.com Gerrit-Reviewer: Arik Hadas aha...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek mskri...@redhat.com Gerrit-Reviewer: Shahar Havivi shav...@redhat.com Gerrit-Reviewer: automat...@ovirt.org ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Import VM from OVA file
automat...@ovirt.org has posted comments on this change. Change subject: v2v: Import VM from OVA file .. Patch Set 12: * Update tracker::IGNORE, no Bug-Url found * Set MODIFIED::IGNORE, no Bug-Url found. -- To view, visit https://gerrit.ovirt.org/43367 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0e440748ecc503f4d61e8f4f61bb0c7387589354 Gerrit-PatchSet: 12 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Shahar Havivi shav...@redhat.com Gerrit-Reviewer: Adam Litke ali...@redhat.com Gerrit-Reviewer: Arik Hadas aha...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek mskri...@redhat.com Gerrit-Reviewer: Shahar Havivi shav...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Import VM from OVA file
automat...@ovirt.org has posted comments on this change. Change subject: v2v: Import VM from OVA file .. Patch Set 11: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/43367 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0e440748ecc503f4d61e8f4f61bb0c7387589354 Gerrit-PatchSet: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Shahar Havivi shav...@redhat.com Gerrit-Reviewer: Adam Litke ali...@redhat.com Gerrit-Reviewer: Arik Hadas aha...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek mskri...@redhat.com Gerrit-Reviewer: Shahar Havivi shav...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Import VM from OVA file
automat...@ovirt.org has posted comments on this change. Change subject: v2v: Import VM from OVA file .. Patch Set 10: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/43367 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0e440748ecc503f4d61e8f4f61bb0c7387589354 Gerrit-PatchSet: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Shahar Havivi shav...@redhat.com Gerrit-Reviewer: Adam Litke ali...@redhat.com Gerrit-Reviewer: Arik Hadas aha...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek mskri...@redhat.com Gerrit-Reviewer: Shahar Havivi shav...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Import VM from OVA file
Shahar Havivi has posted comments on this change. Change subject: v2v: Import VM from OVA file .. Patch Set 9: Verified+1 -- To view, visit https://gerrit.ovirt.org/43367 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0e440748ecc503f4d61e8f4f61bb0c7387589354 Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Shahar Havivi shav...@redhat.com Gerrit-Reviewer: Arik Hadas aha...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek mskri...@redhat.com Gerrit-Reviewer: Shahar Havivi shav...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Import VM from OVA file
automat...@ovirt.org has posted comments on this change. Change subject: v2v: Import VM from OVA file .. Patch Set 9: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/43367 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0e440748ecc503f4d61e8f4f61bb0c7387589354 Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Shahar Havivi shav...@redhat.com Gerrit-Reviewer: Arik Hadas aha...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek mskri...@redhat.com Gerrit-Reviewer: Shahar Havivi shav...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Import VM from OVA file
Shahar Havivi has posted comments on this change.
Change subject: v2v: Import VM from OVA file
..
Patch Set 6:
(2 comments)
https://gerrit.ovirt.org/#/c/43367/6/vdsm/v2v.py
File vdsm/v2v.py:
Line 164:
Line 165:
Line 166: def convert_external_vm(uri, username, password, vminfo, job_id, irs):
Line 167: job = ImportVm.from_libvirt(uri, username, password, vminfo,
job_id, irs)
Line 168: job.start_libvirt()
I don't really like this. It partially defeats the purpose of factory funct
Done
Line 169: _add_job(job_id, job)
Line 170: return {'status': doneCode}
Line 171:
Line 172:
Line 294: @contextmanager
Line 295: def password_file(job_id, file_name, password):
Line 296: if file_name is None:
Line 297: yield
Line 298: return
still needed?
no...
Line 299: fd = os.open(file_name, os.O_WRONLY | os.O_CREAT, 0o600)
Line 300: try:
Line 301: os.write(fd, password.value)
Line 302: finally:
--
To view, visit https://gerrit.ovirt.org/43367
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0e440748ecc503f4d61e8f4f61bb0c7387589354
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shahar Havivi shav...@redhat.com
Gerrit-Reviewer: Arik Hadas aha...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek mskri...@redhat.com
Gerrit-Reviewer: Shahar Havivi shav...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Import VM from OVA file
automat...@ovirt.org has posted comments on this change. Change subject: v2v: Import VM from OVA file .. Patch Set 8: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/43367 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0e440748ecc503f4d61e8f4f61bb0c7387589354 Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Shahar Havivi shav...@redhat.com Gerrit-Reviewer: Arik Hadas aha...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek mskri...@redhat.com Gerrit-Reviewer: Shahar Havivi shav...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Import VM from OVA file
automat...@ovirt.org has posted comments on this change. Change subject: v2v: Import VM from OVA file .. Patch Set 7: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/43367 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0e440748ecc503f4d61e8f4f61bb0c7387589354 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Shahar Havivi shav...@redhat.com Gerrit-Reviewer: Arik Hadas aha...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek mskri...@redhat.com Gerrit-Reviewer: Shahar Havivi shav...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Import VM from OVA file
Francesco Romani has posted comments on this change. Change subject: v2v: Import VM from OVA file .. Patch Set 8: Code-Review+2 thanks, looks better now. I believe that _run_command is not the best name, but not good enough reason to delay this patch. -- To view, visit https://gerrit.ovirt.org/43367 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0e440748ecc503f4d61e8f4f61bb0c7387589354 Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Shahar Havivi shav...@redhat.com Gerrit-Reviewer: Arik Hadas aha...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek mskri...@redhat.com Gerrit-Reviewer: Shahar Havivi shav...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Import VM from OVA file
automat...@ovirt.org has posted comments on this change. Change subject: v2v: Import VM from OVA file .. Patch Set 6: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/43367 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0e440748ecc503f4d61e8f4f61bb0c7387589354 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Shahar Havivi shav...@redhat.com Gerrit-Reviewer: Arik Hadas aha...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek mskri...@redhat.com Gerrit-Reviewer: Shahar Havivi shav...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Import VM from OVA file
Francesco Romani has posted comments on this change.
Change subject: v2v: Import VM from OVA file
..
Patch Set 6: Code-Review-1
(2 comments)
On overall, looks good. But a couple of questions inside. -1 for visibility
https://gerrit.ovirt.org/#/c/43367/6/vdsm/v2v.py
File vdsm/v2v.py:
Line 164:
Line 165:
Line 166: def convert_external_vm(uri, username, password, vminfo, job_id, irs):
Line 167: job = ImportVm.from_libvirt(uri, username, password, vminfo,
job_id, irs)
Line 168: job.start_libvirt()
I don't really like this. It partially defeats the purpose of factory functions
(ImportVm.from_*)
Can't we hide the indirection inside the class? so from the outside we see
job = ImportVm.from_*(parameters)
job.start()
_add_job(job_id, job)
return response.success()
Line 169: _add_job(job_id, job)
Line 170: return {'status': doneCode}
Line 171:
Line 172:
Line 294: @contextmanager
Line 295: def password_file(job_id, file_name, password):
Line 296: if file_name is None:
Line 297: yield
Line 298: return
still needed?
Line 299: fd = os.open(file_name, os.O_WRONLY | os.O_CREAT, 0o600)
Line 300: try:
Line 301: os.write(fd, password.value)
Line 302: finally:
--
To view, visit https://gerrit.ovirt.org/43367
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0e440748ecc503f4d61e8f4f61bb0c7387589354
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shahar Havivi shav...@redhat.com
Gerrit-Reviewer: Arik Hadas aha...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek mskri...@redhat.com
Gerrit-Reviewer: Shahar Havivi shav...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Import VM from OVA file
automat...@ovirt.org has posted comments on this change. Change subject: v2v: Import VM from OVA file .. Patch Set 5: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/43367 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0e440748ecc503f4d61e8f4f61bb0c7387589354 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Shahar Havivi shav...@redhat.com Gerrit-Reviewer: Arik Hadas aha...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek mskri...@redhat.com Gerrit-Reviewer: Shahar Havivi shav...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Import VM from OVA file
Shahar Havivi has posted comments on this change. Change subject: v2v: Import VM from OVA file .. Patch Set 5: Verified+1 -- To view, visit https://gerrit.ovirt.org/43367 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0e440748ecc503f4d61e8f4f61bb0c7387589354 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Shahar Havivi shav...@redhat.com Gerrit-Reviewer: Arik Hadas aha...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek mskri...@redhat.com Gerrit-Reviewer: Shahar Havivi shav...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Import VM from OVA file
Francesco Romani has posted comments on this change.
Change subject: v2v: Import VM from OVA file
..
Patch Set 4:
(4 comments)
big question(s) inside, -1 for visibility.
https://gerrit.ovirt.org/#/c/43367/4/vdsm/rpc/vdsmapi-schema.json
File vdsm/rpc/vdsmapi-schema.json:
Line 3967: 'data': {'ova_path': 'str'},
Line 3968: 'returns': ['ExternalVmInfo']}
Line 3969:
Line 3970: ##
Line 3971: # @Host.convertOva:
same comment as
https://gerrit.ovirt.org/#/c/43271/4/vdsm/rpc/vdsmapi-schema.json
Maybe
convertExternalVmFromOva
?
Line 3972: #
Line 3973: # Convert VM from external file (OVA) to data domain
Line 3974: #
Line 3975: # @ova_path: actual path to the ova file
https://gerrit.ovirt.org/#/c/43367/4/vdsm/v2v.py
File vdsm/v2v.py:
Line 171: def convert_ova(ova_path, vminfo, job_id, irs):
Line 172: job = ImportVm.from_ova(ova_path, vminfo, job_id, irs)
Line 173: job.start()
Line 174: _add_job(job_id, job)
Line 175: return {'status': doneCode}
what about using
response.success()
?
Line 176:
Line 177:
Line 178: def get_ova_info(ova_path):
Line 179: ns = {'ovf': _OVF_NS, 'rasd': _RASD_NS}
Line 293: @contextmanager
Line 294: def password_file(job_id, file_name, password):
Line 295: if file_name is None:
Line 296: yield
Line 297: return
return is unneeded here.
I see why you did this, but it is a little ugly.
I think it is a little nicer if
- we extract what we run in _run() into another helper method:
try:
self._import()
except Exception as ex:
if self._aborted:
logging.debug(Job %r was aborted, self._id)
else:
logging.exception(Job %r failed, self._id)
self._status = STATUS.FAILED
self._description = ex.message
try:
self._abort()
except Exception as e:
logging.exception('Job %r, error trying to abort: %r',
self._id, e)
finally:
self._teardown_volumes()
Let's call this
_run_import_vm
or whatever name is more fitting.
We can also rename existing _run like
_run_import_vm_with_passwd
which should now read like
@traceback(msg=Error importing vm)
def _run(self):
with password_file(self._id, self._passwd_file, self._password):
self._run_import_vm()
or, again, any other better fitting name.
Then, we can rebind _run like you did for _create_command
Indeed this is a little clumsier that I'd like, but seems also clearer and
conforming to the other refactoring you did.
What do you think?
Line 298: fd = os.open(file_name, os.O_WRONLY | os.O_CREAT, 0o600)
Line 299: try:
Line 300: os.write(fd, password.value)
Line 301: finally:
Line 479:
get_storage_domain_path(self._prepared_volumes[0]['path']),
Line 480: self._vminfo['vmName']])
Line 481: return cmd
Line 482:
Line 483: def _from_ova_command(self):
maybe?
_create_command_from_ova?
anyway, should be similar to whatever you choose for the libvirt counterpart
Line 484: cmd = [_VIRT_V2V.cmd,
Line 485:'-i', 'ova', self._ova_path,
Line 486:'-o', 'vdsm',
Line 487:'-of', self._get_disk_format(),
--
To view, visit https://gerrit.ovirt.org/43367
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0e440748ecc503f4d61e8f4f61bb0c7387589354
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shahar Havivi shav...@redhat.com
Gerrit-Reviewer: Arik Hadas aha...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek mskri...@redhat.com
Gerrit-Reviewer: Shahar Havivi shav...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Import VM from OVA file
Francesco Romani has posted comments on this change. Change subject: v2v: Import VM from OVA file .. Patch Set 4: Code-Review-1 furthermore: can you please point me where you addressed comments from Arik in V2? -- To view, visit https://gerrit.ovirt.org/43367 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0e440748ecc503f4d61e8f4f61bb0c7387589354 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Shahar Havivi shav...@redhat.com Gerrit-Reviewer: Arik Hadas aha...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek mskri...@redhat.com Gerrit-Reviewer: Shahar Havivi shav...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Import VM from OVA file
Shahar Havivi has posted comments on this change.
Change subject: v2v: Import VM from OVA file
..
Patch Set 4:
(4 comments)
https://gerrit.ovirt.org/#/c/43367/4/vdsm/rpc/vdsmapi-schema.json
File vdsm/rpc/vdsmapi-schema.json:
Line 3967: 'data': {'ova_path': 'str'},
Line 3968: 'returns': ['ExternalVmInfo']}
Line 3969:
Line 3970: ##
Line 3971: # @Host.convertOva:
same comment as https://gerrit.ovirt.org/#/c/43271/4/vdsm/rpc/vdsmapi-schem
Done
Line 3972: #
Line 3973: # Convert VM from external file (OVA) to data domain
Line 3974: #
Line 3975: # @ova_path: actual path to the ova file
https://gerrit.ovirt.org/#/c/43367/4/vdsm/v2v.py
File vdsm/v2v.py:
Line 171: def convert_ova(ova_path, vminfo, job_id, irs):
Line 172: job = ImportVm.from_ova(ova_path, vminfo, job_id, irs)
Line 173: job.start()
Line 174: _add_job(job_id, job)
Line 175: return {'status': doneCode}
what about using
Done
Line 176:
Line 177:
Line 178: def get_ova_info(ova_path):
Line 179: ns = {'ovf': _OVF_NS, 'rasd': _RASD_NS}
Line 293: @contextmanager
Line 294: def password_file(job_id, file_name, password):
Line 295: if file_name is None:
Line 296: yield
Line 297: return
return is unneeded here.
I will do something like that
Line 298: fd = os.open(file_name, os.O_WRONLY | os.O_CREAT, 0o600)
Line 299: try:
Line 300: os.write(fd, password.value)
Line 301: finally:
Line 479:
get_storage_domain_path(self._prepared_volumes[0]['path']),
Line 480: self._vminfo['vmName']])
Line 481: return cmd
Line 482:
Line 483: def _from_ova_command(self):
maybe?
I want to reserve the _command and the name looks good to me
Line 484: cmd = [_VIRT_V2V.cmd,
Line 485:'-i', 'ova', self._ova_path,
Line 486:'-o', 'vdsm',
Line 487:'-of', self._get_disk_format(),
--
To view, visit https://gerrit.ovirt.org/43367
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0e440748ecc503f4d61e8f4f61bb0c7387589354
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shahar Havivi shav...@redhat.com
Gerrit-Reviewer: Arik Hadas aha...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek mskri...@redhat.com
Gerrit-Reviewer: Shahar Havivi shav...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Import VM from OVA file
automat...@ovirt.org has posted comments on this change. Change subject: v2v: Import VM from OVA file .. Patch Set 3: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/43367 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0e440748ecc503f4d61e8f4f61bb0c7387589354 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Shahar Havivi shav...@redhat.com Gerrit-Reviewer: Arik Hadas aha...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek mskri...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Import VM from OVA file
Shahar Havivi has posted comments on this change. Change subject: v2v: Import VM from OVA file .. Patch Set 3: Verified+1 -- To view, visit https://gerrit.ovirt.org/43367 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0e440748ecc503f4d61e8f4f61bb0c7387589354 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Shahar Havivi shav...@redhat.com Gerrit-Reviewer: Arik Hadas aha...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek mskri...@redhat.com Gerrit-Reviewer: Shahar Havivi shav...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Import VM from OVA file
automat...@ovirt.org has posted comments on this change. Change subject: v2v: Import VM from OVA file .. Patch Set 4: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/43367 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0e440748ecc503f4d61e8f4f61bb0c7387589354 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Shahar Havivi shav...@redhat.com Gerrit-Reviewer: Arik Hadas aha...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek mskri...@redhat.com Gerrit-Reviewer: Shahar Havivi shav...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Import VM from OVA file
Shahar Havivi has posted comments on this change. Change subject: v2v: Import VM from OVA file .. Patch Set 4: Verified+1 -- To view, visit https://gerrit.ovirt.org/43367 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0e440748ecc503f4d61e8f4f61bb0c7387589354 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Shahar Havivi shav...@redhat.com Gerrit-Reviewer: Arik Hadas aha...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek mskri...@redhat.com Gerrit-Reviewer: Shahar Havivi shav...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Import VM from OVA file
Arik Hadas has posted comments on this change. Change subject: v2v: Import VM from OVA file .. Patch Set 2: Verified-1 (1 comment) https://gerrit.ovirt.org/#/c/43367/2/vdsm/v2v.py File vdsm/v2v.py: Line 349: def from_ova(cls, ova_path, vminfo, job_id, irs): Line 350: obj = cls(vminfo, job_id, irs) Line 351: Line 352: obj._ova_path = ova_path Line 353: obj._create_command = obj._from_ova_command need to specify somehow that the password file is not needed in this flow Line 354: return obj Line 355: Line 356: def start(self): Line 357: t = threading.Thread(target=self._run) -- To view, visit https://gerrit.ovirt.org/43367 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0e440748ecc503f4d61e8f4f61bb0c7387589354 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Shahar Havivi shav...@redhat.com Gerrit-Reviewer: Arik Hadas aha...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek mskri...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Import VM from OVA file
automat...@ovirt.org has posted comments on this change. Change subject: v2v: Import VM from OVA file .. Patch Set 2: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/43367 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0e440748ecc503f4d61e8f4f61bb0c7387589354 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: Jenkins CI Gerrit-Reviewer: Michal Skrivanek mskri...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Import VM from OVA file
automat...@ovirt.org has posted comments on this change. Change subject: v2v: Import VM from OVA file .. Patch Set 1: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/43367 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0e440748ecc503f4d61e8f4f61bb0c7387589354 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Shahar Havivi shav...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Import VM from OVA file
Shahar Havivi has uploaded a new change for review.
Change subject: v2v: Import VM from OVA file
..
v2v: Import VM from OVA file
OVA is a tar file which contain a VM with its Ovf file and its disks as
well.
Change-Id: I0e440748ecc503f4d61e8f4f61bb0c7387589354
Signed-off-by: Shahar Havivi shah...@redhat.com
---
M vdsm/API.py
M vdsm/rpc/bindingxmlrpc.py
M vdsm/rpc/vdsmapi-schema.json
M vdsm/v2v.py
4 files changed, 58 insertions(+), 0 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/67/43367/1
diff --git a/vdsm/API.py b/vdsm/API.py
index 4c3724e..704f37b 100644
--- a/vdsm/API.py
+++ b/vdsm/API.py
@@ -1440,6 +1440,9 @@
return v2v.convert_external_vm(uri, username, password, vminfo, jobid,
self._cif.irs)
+def convertOva(self, ova_path, vminfo, jobid):
+return v2v.convert_ova(ova_path, vminfo, jobid, self._cif.irs)
+
def getConvertedVm(self, jobid):
return v2v.get_converted_vm(jobid)
diff --git a/vdsm/rpc/bindingxmlrpc.py b/vdsm/rpc/bindingxmlrpc.py
index b9dd54e..bc74e22 100644
--- a/vdsm/rpc/bindingxmlrpc.py
+++ b/vdsm/rpc/bindingxmlrpc.py
@@ -381,6 +381,10 @@
api = API.Global()
return api.convertExternalVm(uri, username, password, vminfo, jobid)
+def convertOva(self, ova_path, vminfo, jobid):
+api = API.Global()
+return api.convert_ova(ova_path, vminfo, jobid)
+
def getConvertedVm(self, jobid):
api = API.Global()
return api.getConvertedVm(jobid)
@@ -1109,6 +1113,7 @@
(self.getExternalVMs, 'getExternalVMs'),
(self.getOvaInfo, 'getOvaInfo'),
(self.convertExternalVm, 'convertExternalVm'),
+(self.convertOva, 'convertOva'),
(self.getConvertedVm, 'getConvertedVm'),
(self.abortV2VJob, 'abortV2VJob'),
(self.deleteV2VJob, 'deleteV2VJob'),
diff --git a/vdsm/rpc/vdsmapi-schema.json b/vdsm/rpc/vdsmapi-schema.json
index 291aee5..b4c577b 100644
--- a/vdsm/rpc/vdsmapi-schema.json
+++ b/vdsm/rpc/vdsmapi-schema.json
@@ -3947,6 +3947,23 @@
'data': {'ova_path': 'str'},
'returns': 'ExternalVmInfo'}
+##
+# @Host.convertOva:
+#
+# Convert VM from external file (OVA) to data domain
+#
+# @ova_path: actual path to the ova file
+#
+# @vminfo: information of the VM such as name, id etc
+#
+# @jobid:Assign a UUID to this operation which can be used
+#to identify it in @HostStats
+#
+# Since: 4.17.0
+##
+{'command': {'class': 'Host', 'name': 'convertOva'},
+ 'data': {'ova_path': 'str', 'vminfo': 'ExternalVmInfo',
+ 'jobid': 'UUID'}}
##
# @Host.convertExternalVm:
diff --git a/vdsm/v2v.py b/vdsm/v2v.py
index e891894..28fb55e 100644
--- a/vdsm/v2v.py
+++ b/vdsm/v2v.py
@@ -167,6 +167,13 @@
return {'status': doneCode}
+def convert_ova(ova_path, vminfo, job_id, irs):
+job = ImportVm.from_ova(ova_path, vminfo, job_id, irs)
+job.start()
+_add_job(job_id, job)
+return {'status': doneCode}
+
+
def get_ova_info(ova_path):
ns = {'ovf': _OVF_NS, 'rasd': _RASD_NS}
@@ -325,6 +332,8 @@
self._passwd_file = None
self._create_command = None
+self._ova_path = None
+
@classmethod
def from_libvirt(cls, uri, username, password, vminfo, job_id, irs):
obj = cls(vminfo, job_id, irs)
@@ -334,6 +343,14 @@
obj._password = password
obj._passwd_file = os.path.join(_V2V_DIR, %s.tmp % job_id)
obj._create_command = cls._from_libvirt_command
+return obj
+
+@classmethod
+def from_ova(cls, ova_path, vminfo, job_id, irs):
+obj = cls(vminfo, job_id, irs)
+
+obj._ova_path = ova_path
+obj._create_command = cls._from_ova_command
return obj
def start(self):
@@ -453,6 +470,22 @@
self._vminfo['vmName']])
return cmd
+def _from_ova_command(self):
+cmd = [_VIRT_V2V.cmd,
+ '-i', 'ova', self._ova_path,
+ '-o', 'vdsm',
+ '-of', self._get_disk_format(),
+ '-oa', self._vminfo.get('allocation', 'sparse').lower(),
+ self._generate_disk_parameters(),
+ '--vdsm-vm-uuid',
+ self._id,
+ '--vdsm-ovf-output',
+ _V2V_DIR,
+ '--machine-readable',
+ '-os',
+ get_storage_domain_path(self._prepared_volumes[0]['path'])]
+return cmd
+
def abort(self):
self._status = STATUS.ABORTED
logging.info('Job %r aborting...', self._id)
--
To view, visit https://gerrit.ovirt.org/43367
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I0e440748ecc503f4d61e8f4f61bb0c7387589354
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shahar
