Change in vdsm[master]: v2v: get VM information from OVA file
automat...@ovirt.org has posted comments on this change. Change subject: v2v: get VM information 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/43271 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3c55c846f837bb5bf363717e05daabf5ee6631ca Gerrit-PatchSet: 12 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Shahar Havivi Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Arik Hadas Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Shahar Havivi 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: get VM information from OVA file
Dan Kenigsberg has submitted this change and it was merged. Change subject: v2v: get VM information from OVA file .. v2v: get VM information from OVA file In order to import a VM that exists in an OVA file the oVirt engine needs to get information regarding the VM, such as disks sizes, interfaces memory etc. Adding a new verb: getExternalVmFromOva Change-Id: I3c55c846f837bb5bf363717e05daabf5ee6631ca Signed-off-by: Shahar Havivi Reviewed-on: https://gerrit.ovirt.org/43271 Tested-by: Shahar Havivi Continuous-Integration: Jenkins CI Reviewed-by: Francesco Romani --- M vdsm/API.py M vdsm/rpc/Bridge.py M vdsm/rpc/bindingxmlrpc.py M vdsm/rpc/vdsmapi-schema.json M vdsm/v2v.py 5 files changed, 139 insertions(+), 1 deletion(-) Approvals: Shahar Havivi: Verified Jenkins CI: Passed CI tests Francesco Romani: Looks good to me, approved -- To view, visit https://gerrit.ovirt.org/43271 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: I3c55c846f837bb5bf363717e05daabf5ee6631ca Gerrit-PatchSet: 12 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Shahar Havivi Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Arik Hadas Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Shahar Havivi 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: get VM information from OVA file
Francesco Romani has posted comments on this change. Change subject: v2v: get VM information from OVA file .. Patch Set 11: Code-Review+2 raising score -- To view, visit https://gerrit.ovirt.org/43271 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3c55c846f837bb5bf363717e05daabf5ee6631ca Gerrit-PatchSet: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Shahar Havivi Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Arik Hadas Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Shahar Havivi 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: get VM information from OVA file
Francesco Romani has posted comments on this change. Change subject: v2v: get VM information from OVA file .. Patch Set 10: (1 comment) https://gerrit.ovirt.org/#/c/43271/10/vdsm/v2v.py File vdsm/v2v.py: Line 185: _add_general_ovf_info(vm, root, ns) Line 186: _add_disks_ovf_info(vm, root, ns) Line 187: _add_networks_ovf_info(vm, root, ns) Line 188: Line 189: return response.success(vmList=vm) > well if you look at the bridge.py you will see the all the verbs that retur Fair enough, let's keep this consistent. Line 190: Line 191: Line 192: def get_converted_vm(job_id): Line 193: try: -- To view, visit https://gerrit.ovirt.org/43271 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3c55c846f837bb5bf363717e05daabf5ee6631ca Gerrit-PatchSet: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Shahar Havivi Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Arik Hadas Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Shahar Havivi 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: get VM information from OVA file
Shahar Havivi has posted comments on this change. Change subject: v2v: get VM information from OVA file .. Patch Set 10: (1 comment) https://gerrit.ovirt.org/#/c/43271/10/vdsm/v2v.py File vdsm/v2v.py: Line 185: _add_general_ovf_info(vm, root, ns) Line 186: _add_disks_ovf_info(vm, root, ns) Line 187: _add_networks_ovf_info(vm, root, ns) Line 188: Line 189: return response.success(vmList=vm) > ok, but let's make a step further: now this is misleading, is not a list an well if you look at the bridge.py you will see the all the verbs that return a single vm (vm_create, vm_cont, change_cd, change_floppy,pause_vm etc) all return vmList key for a single VM. Line 190: Line 191: Line 192: def get_converted_vm(job_id): Line 193: try: -- To view, visit https://gerrit.ovirt.org/43271 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3c55c846f837bb5bf363717e05daabf5ee6631ca Gerrit-PatchSet: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Shahar Havivi Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Arik Hadas Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Shahar Havivi 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: get VM information from OVA file
automat...@ovirt.org has posted comments on this change. Change subject: v2v: get VM information 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/43271 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3c55c846f837bb5bf363717e05daabf5ee6631ca Gerrit-PatchSet: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Shahar Havivi Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Arik Hadas Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Shahar Havivi 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: get VM information from OVA file
Francesco Romani has posted comments on this change.
Change subject: v2v: get VM information from OVA file
..
Patch Set 10: Code-Review+1
(2 comments)
partial ACK.
Since we're cleaning up schema and return value, fuether comments on the topic.
https://gerrit.ovirt.org/#/c/43271/10/vdsm/rpc/Bridge.py
File vdsm/rpc/Bridge.py:
Line 409: 'Host_getConnectedStoragePools': {'ret': 'poollist'},
Line 410: 'Host_getDeviceList': {'ret': 'devList'},
Line 411: 'Host_getDevicesVisibility': {'ret': 'visible'},
Line 412: 'Host_getExternalVMs': {'ret': 'vmList'},
Line 413: 'Host_getExternalVmFromOva': {'ret': 'vmList'},
...here (see comment in v2v.py)
Line 414: 'Host_getConvertedVm': {'ret': 'ovf'},
Line 415: 'Host_getHardwareInfo': {'ret': 'info'},
Line 416: 'Host_getLVMVolumeGroups': {'ret': 'vglist'},
Line 417: 'Host_getStats': {'ret': 'info'},
https://gerrit.ovirt.org/#/c/43271/10/vdsm/v2v.py
File vdsm/v2v.py:
Line 185: _add_general_ovf_info(vm, root, ns)
Line 186: _add_disks_ovf_info(vm, root, ns)
Line 187: _add_networks_ovf_info(vm, root, ns)
Line 188:
Line 189: return response.success(vmList=vm)
ok, but let's make a step further: now this is misleading, is not a list
anymore rather a single value. I'd rename it to something like
info=vm
or
vmInfo=vm
let's not forget to check and, as likely needed, to update Bridge.py.
Line 190:
Line 191:
Line 192: def get_converted_vm(job_id):
Line 193: try:
--
To view, visit https://gerrit.ovirt.org/43271
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I3c55c846f837bb5bf363717e05daabf5ee6631ca
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shahar Havivi
Gerrit-Reviewer: Adam Litke
Gerrit-Reviewer: Arik Hadas
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Federico Simoncelli
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Shahar Havivi
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: get VM information from OVA file
Shahar Havivi has posted comments on this change. Change subject: v2v: get VM information from OVA file .. Patch Set 10: Verified+1 -- To view, visit https://gerrit.ovirt.org/43271 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3c55c846f837bb5bf363717e05daabf5ee6631ca Gerrit-PatchSet: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Shahar Havivi Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Arik Hadas Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Shahar Havivi 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: get VM information from OVA file
automat...@ovirt.org has posted comments on this change. Change subject: v2v: get VM information 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/43271 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3c55c846f837bb5bf363717e05daabf5ee6631ca Gerrit-PatchSet: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Shahar Havivi Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Arik Hadas Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Shahar Havivi 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: get VM information from OVA file
Shahar Havivi has posted comments on this change.
Change subject: v2v: get VM information from OVA file
..
Patch Set 9:
(2 comments)
https://gerrit.ovirt.org/#/c/43271/9/vdsm/rpc/vdsmapi-schema.json
File vdsm/rpc/vdsmapi-schema.json:
Line 3965: ##
Line 3966: {'command': {'class': 'Host', 'name': 'getExternalVmFromOva'},
Line 3967: 'data': {'ova_path': 'str'},
Line 3968: 'returns': ['ExternalVmInfo']}
Line 3969:
> Oops. You never define the 'ExternalVmInfo' type. Also, why are you retur
its not a new type its define in line 3900 and we use it for other verbs as
well.
For the VM array you are right this is an Engine constraint as Arik mention
here: https://gerrit.ovirt.org/#/c/43271/4/vdsm/v2v.py@179
I will change it to a single VM and ask Arik to fix it in the engine.
Line 3970:
Line 3971: ##
Line 3972: # @Host.convertExternalVm:
Line 3973: #
https://gerrit.ovirt.org/#/c/43271/9/vdsm/v2v.py
File vdsm/v2v.py:
Line 699: vm['vmName'] = vmName.text
Line 700: else:
Line 701: raise V2VError('Error parsing ovf information: no ovf:Name')
Line 702:
Line 703: memSize = node.find('.//ovf:Item[rasd:ResourceType="4"]/'
> ResouceType="4" <-- seems we should have some constants defined for these.
Done
Line 704: 'rasd:VirtualQuantity', ns)
Line 705: if memSize is not None:
Line 706: vm['memSize'] = int(memSize.text)
Line 707: else:
--
To view, visit https://gerrit.ovirt.org/43271
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I3c55c846f837bb5bf363717e05daabf5ee6631ca
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shahar Havivi
Gerrit-Reviewer: Adam Litke
Gerrit-Reviewer: Arik Hadas
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Federico Simoncelli
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Shahar Havivi
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: get VM information from OVA file
Adam Litke has posted comments on this change.
Change subject: v2v: get VM information from OVA file
..
Patch Set 9: Code-Review-1
(2 comments)
https://gerrit.ovirt.org/#/c/43271/9/vdsm/rpc/vdsmapi-schema.json
File vdsm/rpc/vdsmapi-schema.json:
Line 3965: ##
Line 3966: {'command': {'class': 'Host', 'name': 'getExternalVmFromOva'},
Line 3967: 'data': {'ova_path': 'str'},
Line 3968: 'returns': ['ExternalVmInfo']}
Line 3969:
Oops. You never define the 'ExternalVmInfo' type. Also, why are you returning
an array? You always pass in a single OVA path so it seems impossible that you
could ever have multiple ExternalVMInfo objects to return from this call.
Line 3970:
Line 3971: ##
Line 3972: # @Host.convertExternalVm:
Line 3973: #
https://gerrit.ovirt.org/#/c/43271/9/vdsm/v2v.py
File vdsm/v2v.py:
Line 699: vm['vmName'] = vmName.text
Line 700: else:
Line 701: raise V2VError('Error parsing ovf information: no ovf:Name')
Line 702:
Line 703: memSize = node.find('.//ovf:Item[rasd:ResourceType="4"]/'
ResouceType="4" <-- seems we should have some constants defined for these.
Line 704: 'rasd:VirtualQuantity', ns)
Line 705: if memSize is not None:
Line 706: vm['memSize'] = int(memSize.text)
Line 707: else:
--
To view, visit https://gerrit.ovirt.org/43271
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I3c55c846f837bb5bf363717e05daabf5ee6631ca
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shahar Havivi
Gerrit-Reviewer: Adam Litke
Gerrit-Reviewer: Arik Hadas
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Federico Simoncelli
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Shahar Havivi
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: get VM information from OVA file
Francesco Romani has posted comments on this change. Change subject: v2v: get VM information from OVA file .. Patch Set 9: Code-Review+2 thanks for the updates, looks good to me now. -- To view, visit https://gerrit.ovirt.org/43271 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3c55c846f837bb5bf363717e05daabf5ee6631ca Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Shahar Havivi Gerrit-Reviewer: Arik Hadas Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Shahar Havivi 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: get VM information from OVA file
Shahar Havivi has posted comments on this change. Change subject: v2v: get VM information from OVA file .. Patch Set 9: Verified+1 -- To view, visit https://gerrit.ovirt.org/43271 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3c55c846f837bb5bf363717e05daabf5ee6631ca Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Shahar Havivi Gerrit-Reviewer: Arik Hadas Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Shahar Havivi 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: get VM information from OVA file
automat...@ovirt.org has posted comments on this change. Change subject: v2v: get VM information 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/43271 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3c55c846f837bb5bf363717e05daabf5ee6631ca Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Shahar Havivi Gerrit-Reviewer: Arik Hadas Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Shahar Havivi 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: get VM information from OVA file
Francesco Romani has posted comments on this change.
Change subject: v2v: get VM information from OVA file
..
Patch Set 8: Code-Review+1
(2 comments)
minor doc comments inside. I see patch 44471 adds tests, hence preliminar ack
(+1)
https://gerrit.ovirt.org/#/c/43271/8/vdsm/API.py
File vdsm/API.py:
Line 1437:
Line 1438: def getExternalVmFromOva(self, ova_path):
Line 1439: """
Line 1440: return a dictionary, information regading a VM that is a part
Line 1441: of the ova
please make this docstring more alike getExternalVMs (of course wherever make
sense). As it is now, it is too generic.
Line 1442: """
Line 1443: return v2v.get_ova_info(ova_path)
Line 1444:
Line 1445: def convertExternalVm(self, uri, username, password, vminfo,
jobid):
https://gerrit.ovirt.org/#/c/43271/8/vdsm/rpc/vdsmapi-schema.json
File vdsm/rpc/vdsmapi-schema.json:
Line 3958: #
Line 3959: # @ova_path: path to the ova file
Line 3960: #
Line 3961: # Returns:
Line 3962: # A dictionary with information regarding the VM
Please change this line to state that the output is ExternalVmInfo, like
getExternalVMs returns.
Line 3963: #
Line 3964: # Since: 4.17.0
Line 3965: ##
Line 3966: {'command': {'class': 'Host', 'name': 'getExternalVmFromOva'},
--
To view, visit https://gerrit.ovirt.org/43271
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I3c55c846f837bb5bf363717e05daabf5ee6631ca
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shahar Havivi
Gerrit-Reviewer: Arik Hadas
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Shahar Havivi
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: get VM information from OVA file
automat...@ovirt.org has posted comments on this change. Change subject: v2v: get VM information 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/43271 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3c55c846f837bb5bf363717e05daabf5ee6631ca Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Shahar Havivi Gerrit-Reviewer: Arik Hadas Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Shahar Havivi 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: get VM information from OVA file
Shahar Havivi has posted comments on this change.
Change subject: v2v: get VM information from OVA file
..
Patch Set 6:
(3 comments)
testing patch: https://gerrit.ovirt.org/#/c/44471/
https://gerrit.ovirt.org/#/c/43271/6//COMMIT_MSG
Commit Message:
Line 8:
Line 9: In order to import a VM that exists in an OVA file the oVirt engine
Line 10: needs to get information regarding the VM, such as disks sizes,
Line 11: interfaces memory etc.
Line 12: Adding a new verb: getOvfInfo
> this needs to be updated to reflect the renamed verb.
Done
Line 13:
Line 14: Change-Id: I3c55c846f837bb5bf363717e05daabf5ee6631ca
https://gerrit.ovirt.org/#/c/43271/6/vdsm/v2v.py
File vdsm/v2v.py:
Line 50:
Line 51: _V2V_DIR = os.path.join(P_VDSM_RUN, 'v2v')
Line 52: _VIRT_V2V = CommandPath('virt-v2v', '/usr/bin/virt-v2v')
Line 53:
Line 54: # ovf sepcification:
> typo: specification
Done
Line 55: # https://www.iso.org/obp/ui/#iso:std:iso-iec:17203:ed-1:v1:en
Line 56: _OVF_NS = 'http://schemas.dmtf.org/ovf/envelope/1'
Line 57: _RASD_NS = 'http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/' \
Line 58:'CIM_ResourceAllocationSettingData'
Line 702:
Line 703: memSize = node.find('.//ovf:Item[rasd:ResourceType="4"]/'
Line 704: 'rasd:VirtualQuantity', ns)
Line 705: if memSize is not None:
Line 706: vm['memSize'] = int(memSize.text)
> this can raise ValueError. Is it OK if this bubbles up?
Yes, this is not suppose to happened since its part of the specification that
if there is value it should be a number.
Line 707: else:
Line 708: raise V2VError('Error parsing ovf information: no memory
size')
Line 709:
Line 710: smp = node.find('.//ovf:Item[rasd:ResourceType="3"]/'
--
To view, visit https://gerrit.ovirt.org/43271
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I3c55c846f837bb5bf363717e05daabf5ee6631ca
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shahar Havivi
Gerrit-Reviewer: Arik Hadas
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Shahar Havivi
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: get VM information from OVA file
automat...@ovirt.org has posted comments on this change. Change subject: v2v: get VM information 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/43271 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3c55c846f837bb5bf363717e05daabf5ee6631ca Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Shahar Havivi Gerrit-Reviewer: Arik Hadas Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Shahar Havivi 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: get VM information from OVA file
Francesco Romani has posted comments on this change. Change subject: v2v: get VM information from OVA file .. Patch Set 6: Code-Review-1 minor nits, -1 for visibility -- To view, visit https://gerrit.ovirt.org/43271 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3c55c846f837bb5bf363717e05daabf5ee6631ca Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Shahar Havivi Gerrit-Reviewer: Arik Hadas Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Shahar Havivi 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: get VM information from OVA file
Francesco Romani has posted comments on this change.
Change subject: v2v: get VM information from OVA file
..
Patch Set 6:
(4 comments)
minor comments. Looks OK so far.
I of course trust the verification, but this code begs for unit tests. Can you
please add them in a future patch (or point me to that patch? :) )
https://gerrit.ovirt.org/#/c/43271/6//COMMIT_MSG
Commit Message:
Line 8:
Line 9: In order to import a VM that exists in an OVA file the oVirt engine
Line 10: needs to get information regarding the VM, such as disks sizes,
Line 11: interfaces memory etc.
Line 12: Adding a new verb: getOvfInfo
this needs to be updated to reflect the renamed verb.
Line 13:
Line 14: Change-Id: I3c55c846f837bb5bf363717e05daabf5ee6631ca
https://gerrit.ovirt.org/#/c/43271/6/vdsm/v2v.py
File vdsm/v2v.py:
Line 50:
Line 51: _V2V_DIR = os.path.join(P_VDSM_RUN, 'v2v')
Line 52: _VIRT_V2V = CommandPath('virt-v2v', '/usr/bin/virt-v2v')
Line 53:
Line 54: # ovf sepcification:
typo: specification
Line 55: # https://www.iso.org/obp/ui/#iso:std:iso-iec:17203:ed-1:v1:en
Line 56: _OVF_NS = 'http://schemas.dmtf.org/ovf/envelope/1'
Line 57: _RASD_NS = 'http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/' \
Line 58:'CIM_ResourceAllocationSettingData'
Line 702:
Line 703: memSize = node.find('.//ovf:Item[rasd:ResourceType="4"]/'
Line 704: 'rasd:VirtualQuantity', ns)
Line 705: if memSize is not None:
Line 706: vm['memSize'] = int(memSize.text)
this can raise ValueError. Is it OK if this bubbles up?
Line 707: else:
Line 708: raise V2VError('Error parsing ovf information: no memory
size')
Line 709:
Line 710: smp = node.find('.//ovf:Item[rasd:ResourceType="3"]/'
Line 709:
Line 710: smp = node.find('.//ovf:Item[rasd:ResourceType="3"]/'
Line 711: 'rasd:VirtualQuantity', ns)
Line 712: if smp is not None:
Line 713: vm['smp'] = int(smp.text)
same comment as in line 706
Line 714: else:
Line 715: raise V2VError('Error parsing ovf information: no cpu info')
Line 716:
Line 717:
--
To view, visit https://gerrit.ovirt.org/43271
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I3c55c846f837bb5bf363717e05daabf5ee6631ca
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shahar Havivi
Gerrit-Reviewer: Arik Hadas
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Shahar Havivi
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: get VM information from OVA file
automat...@ovirt.org has posted comments on this change. Change subject: v2v: get VM information 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/43271 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3c55c846f837bb5bf363717e05daabf5ee6631ca Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Shahar Havivi Gerrit-Reviewer: Arik Hadas Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Shahar Havivi 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: get VM information from OVA file
Shahar Havivi has posted comments on this change. Change subject: v2v: get VM information from OVA file .. Patch Set 5: Verified+1 -- To view, visit https://gerrit.ovirt.org/43271 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3c55c846f837bb5bf363717e05daabf5ee6631ca Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Shahar Havivi Gerrit-Reviewer: Arik Hadas Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Shahar Havivi 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: get VM information from OVA file
automat...@ovirt.org has posted comments on this change. Change subject: v2v: get VM information 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/43271 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3c55c846f837bb5bf363717e05daabf5ee6631ca Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Shahar Havivi Gerrit-Reviewer: Arik Hadas Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Shahar Havivi 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: get VM information from OVA file
Shahar Havivi has posted comments on this change.
Change subject: v2v: get VM information from OVA file
..
Patch Set 4:
(9 comments)
https://gerrit.ovirt.org/#/c/43271/4/vdsm/rpc/vdsmapi-schema.json
File vdsm/rpc/vdsmapi-schema.json:
Line 3953:
Line 3954: ##
Line 3955: # @Host.getOvaInfo:
Line 3956: #
Line 3957: # Get VM information (disks, interfaces, memory etc) from OVA file
> We should spend a bit of time in choosing good names for public API, since
getExternalVMsFromOva sounds good
Line 3958: #
Line 3959: # @ova_path: path to the ova file
Line 3960: #
Line 3961: # Returns:
https://gerrit.ovirt.org/#/c/43271/4/vdsm/v2v.py
File vdsm/v2v.py:
Line 180: _add_general_ovf_info(vm, root, ns)
Line 181: _add_disks_ovf_info(vm, root, ns)
Line 182: _add_networks_ovf_info(vm, root, ns)
Line 183:
Line 184: return {'status': doneCode, 'vmList': [vm]}
> please consider using the response module:
sure, you right!
Line 185:
Line 186:
Line 187: def get_converted_vm(job_id):
Line 188: try:
Line 680: params['networks'].append(i)
Line 681:
Line 682:
Line 683: def _read_ovf_from_ova(ova_path):
Line 684: cmd = ['/usr/bin/tar', 'xf', ova_path, '*.ovf', '--to-stdout']
> question: can we make use of the tarfile package?
There is no way to use the --to-stdout in tarfile package,
I will add a comment in the code
Line 685: rc, output, error = execCmd(cmd)
Line 686: if rc:
Line 687: raise V2VError(error)
Line 688:
Line 688:
Line 689: return ''.join(output)
Line 690:
Line 691:
Line 692: def _add_general_ovf_info(vm, node, ns):
> this function seems to be easily unit-testable. Do we have tests? if not, c
Done
Line 693: try:
Line 694: vm['status'] = 'Down'
Line 695: vm['vmName'] = node.find('./ovf:VirtualSystem/ovf:Name',
ns).text
Line 696: vm['memSize'] =
int(node.find('.//ovf:Item[rasd:ResourceType="4"]/'
Line 692: def _add_general_ovf_info(vm, node, ns):
Line 693: try:
Line 694: vm['status'] = 'Down'
Line 695: vm['vmName'] = node.find('./ovf:VirtualSystem/ovf:Name',
ns).text
Line 696: vm['memSize'] =
int(node.find('.//ovf:Item[rasd:ResourceType="4"]/'
> a link to the doc - here or up when you define the _RASD_NS constant would
Done
Line 697: 'rasd:VirtualQuantity', ns).text)
Line 698: vm['smp'] =
int(node.find('.//ovf:Item[rasd:ResourceType="3"]/'
Line 699: 'rasd:VirtualQuantity', ns).text)
Line 700: except Exception as e:
Line 696: vm['memSize'] =
int(node.find('.//ovf:Item[rasd:ResourceType="4"]/'
Line 697: 'rasd:VirtualQuantity', ns).text)
Line 698: vm['smp'] =
int(node.find('.//ovf:Item[rasd:ResourceType="3"]/'
Line 699: 'rasd:VirtualQuantity', ns).text)
Line 700: except Exception as e:
> can we catch more specific exception?
yes, I will fix that
Line 701: raise V2VError('Error parsing ovf information: %s' %
e.message)
Line 702:
Line 703:
Line 704: def _add_disks_ovf_info(vm, node, ns):
Line 697: 'rasd:VirtualQuantity', ns).text)
Line 698: vm['smp'] =
int(node.find('.//ovf:Item[rasd:ResourceType="3"]/'
Line 699: 'rasd:VirtualQuantity', ns).text)
Line 700: except Exception as e:
Line 701: raise V2VError('Error parsing ovf information: %s' %
e.message)
> what's the benefit of having this Exception translation? I see no recovery
The reason is to know where the exception cam from (ie V2VError from v2v)
what do you suggest, not handling the error (ie it will be raised to the log?
Line 702:
Line 703:
Line 704: def _add_disks_ovf_info(vm, node, ns):
Line 705: try:
Line 713: alias =
node.find('.//ovf:References/ovf:File[@ovf:id="%s"]' %
Line 714: fileref, ns)
Line 715: disk['alias'] = alias.attrib.get('{%s}href' % _OVF_NS)
Line 716: vm['disks'].append(disk)
Line 717: except Exception as e:
> same as line 700
Done
Line 718: raise V2VError('Error parsing ovf disk information: %s' %
e.message)
Line 719:
Line 720:
Line 721: def _add_networks_ovf_info(vm, node, ns):
Line 731: net['type'] = 'bridge'
Line 732: else:
Line 733: net['type'] = 'interface'
Line 734: vm['networks'].append(net)
Line 735: except Exception as e:
> same as line 700
Done
--
To view, visit https://gerrit.ovirt.org/43271
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I3c55c846f837bb5bf363717e05daabf5ee6631ca
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shahar Havivi
Gerrit-Reviewer: Arik Hadas
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer:
Change in vdsm[master]: v2v: get VM information from OVA file
Arik Hadas has posted comments on this change.
Change subject: v2v: get VM information from OVA file
..
Patch Set 4:
(1 comment)
https://gerrit.ovirt.org/#/c/43271/4/vdsm/v2v.py
File vdsm/v2v.py:
Line 175: root = ET.fromstring(_read_ovf_from_ova(ova_path))
Line 176: except ET.ParseError as e:
Line 177: raise V2VError('Error reading ovf from ova, position: %r' %
e.position)
Line 178:
Line 179: vm = {}
> Should this contain exactly one VM?
right, we do it like that so we could reuse the response type of full-list
instead of presenting yet another struct in the engine
Line 180: _add_general_ovf_info(vm, root, ns)
Line 181: _add_disks_ovf_info(vm, root, ns)
Line 182: _add_networks_ovf_info(vm, root, ns)
Line 183:
--
To view, visit https://gerrit.ovirt.org/43271
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I3c55c846f837bb5bf363717e05daabf5ee6631ca
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shahar Havivi
Gerrit-Reviewer: Arik Hadas
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Shahar Havivi
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: get VM information from OVA file
Francesco Romani has posted comments on this change.
Change subject: v2v: get VM information from OVA file
..
Patch Set 4: Code-Review-1
(10 comments)
mostly questions, but quite some of them. -1 for visibility.
https://gerrit.ovirt.org/#/c/43271/4/vdsm/rpc/vdsmapi-schema.json
File vdsm/rpc/vdsmapi-schema.json:
Line 3953:
Line 3954: ##
Line 3955: # @Host.getOvaInfo:
Line 3956: #
Line 3957: # Get VM information (disks, interfaces, memory etc) from OVA file
We should spend a bit of time in choosing good names for public API, since we
need to support them for a long time.
Here I don't really like the 'getOvaInfo' name.
We should pick a more intention-revealing name, and it should also remind us
that this verb is related to getExternalVMs.
Maybe
getExternalVMsFromOva
?
Just a suggestion to kickstart more thought process
Line 3958: #
Line 3959: # @ova_path: path to the ova file
Line 3960: #
Line 3961: # Returns:
https://gerrit.ovirt.org/#/c/43271/4/vdsm/v2v.py
File vdsm/v2v.py:
Line 175: root = ET.fromstring(_read_ovf_from_ova(ova_path))
Line 176: except ET.ParseError as e:
Line 177: raise V2VError('Error reading ovf from ova, position: %r' %
e.position)
Line 178:
Line 179: vm = {}
Should this contain exactly one VM?
Line 180: _add_general_ovf_info(vm, root, ns)
Line 181: _add_disks_ovf_info(vm, root, ns)
Line 182: _add_networks_ovf_info(vm, root, ns)
Line 183:
Line 180: _add_general_ovf_info(vm, root, ns)
Line 181: _add_disks_ovf_info(vm, root, ns)
Line 182: _add_networks_ovf_info(vm, root, ns)
Line 183:
Line 184: return {'status': doneCode, 'vmList': [vm]}
please consider using the response module:
return response.success(vmList=[vm])
Line 185:
Line 186:
Line 187: def get_converted_vm(job_id):
Line 188: try:
Line 680: params['networks'].append(i)
Line 681:
Line 682:
Line 683: def _read_ovf_from_ova(ova_path):
Line 684: cmd = ['/usr/bin/tar', 'xf', ova_path, '*.ovf', '--to-stdout']
question: can we make use of the tarfile package?
https://docs.python.org/2/library/tarfile.html
If not, can you please add a quick comment reminding the future us why we did
like that?
Line 685: rc, output, error = execCmd(cmd)
Line 686: if rc:
Line 687: raise V2VError(error)
Line 688:
Line 688:
Line 689: return ''.join(output)
Line 690:
Line 691:
Line 692: def _add_general_ovf_info(vm, node, ns):
this function seems to be easily unit-testable. Do we have tests? if not, can
you please add them? (same for other _add*). It's OK to add them in a followup
patch.
Line 693: try:
Line 694: vm['status'] = 'Down'
Line 695: vm['vmName'] = node.find('./ovf:VirtualSystem/ovf:Name',
ns).text
Line 696: vm['memSize'] =
int(node.find('.//ovf:Item[rasd:ResourceType="4"]/'
Line 692: def _add_general_ovf_info(vm, node, ns):
Line 693: try:
Line 694: vm['status'] = 'Down'
Line 695: vm['vmName'] = node.find('./ovf:VirtualSystem/ovf:Name',
ns).text
Line 696: vm['memSize'] =
int(node.find('.//ovf:Item[rasd:ResourceType="4"]/'
a link to the doc - here or up when you define the _RASD_NS constant would be
nice
Line 697: 'rasd:VirtualQuantity', ns).text)
Line 698: vm['smp'] =
int(node.find('.//ovf:Item[rasd:ResourceType="3"]/'
Line 699: 'rasd:VirtualQuantity', ns).text)
Line 700: except Exception as e:
Line 696: vm['memSize'] =
int(node.find('.//ovf:Item[rasd:ResourceType="4"]/'
Line 697: 'rasd:VirtualQuantity', ns).text)
Line 698: vm['smp'] =
int(node.find('.//ovf:Item[rasd:ResourceType="3"]/'
Line 699: 'rasd:VirtualQuantity', ns).text)
Line 700: except Exception as e:
can we catch more specific exception?
Line 701: raise V2VError('Error parsing ovf information: %s' %
e.message)
Line 702:
Line 703:
Line 704: def _add_disks_ovf_info(vm, node, ns):
Line 697: 'rasd:VirtualQuantity', ns).text)
Line 698: vm['smp'] =
int(node.find('.//ovf:Item[rasd:ResourceType="3"]/'
Line 699: 'rasd:VirtualQuantity', ns).text)
Line 700: except Exception as e:
Line 701: raise V2VError('Error parsing ovf information: %s' %
e.message)
what's the benefit of having this Exception translation? I see no recovery code
in place, so it is really matters which group (general, disks, networks)
failed, granted there is enough context in the generic exception?
Line 702:
Line 703:
Line 704: def _add_disks_ovf_info(vm, node, ns):
Line 705: try:
Line 713: alias =
node.find('.//ovf:References/ovf:File[@ovf:id="%s"]' %
Line 714: fileref, ns)
Line 715: disk['alias'] = alias.attrib.get('{%s}href' % _OVF_NS)
Line 716: vm['disks'].append(disk
Change in vdsm[master]: v2v: get VM information from OVA file
Shahar Havivi has posted comments on this change. Change subject: v2v: get VM information from OVA file .. Patch Set 4: Verified+1 -- To view, visit https://gerrit.ovirt.org/43271 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3c55c846f837bb5bf363717e05daabf5ee6631ca Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Shahar Havivi Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Shahar Havivi 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: get VM information from OVA file
automat...@ovirt.org has posted comments on this change. Change subject: v2v: get VM information 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/43271 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3c55c846f837bb5bf363717e05daabf5ee6631ca Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Shahar Havivi Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Shahar Havivi 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: get VM information from OVA file
automat...@ovirt.org has posted comments on this change. Change subject: v2v: get VM information 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/43271 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3c55c846f837bb5bf363717e05daabf5ee6631ca Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Shahar Havivi Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Shahar Havivi 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: get VM information from OVA file
Shahar Havivi has posted comments on this change.
Change subject: v2v: get VM information from OVA file
..
Patch Set 2:
(1 comment)
https://gerrit.ovirt.org/#/c/43271/2/vdsm/v2v.py
File vdsm/v2v.py:
Line 49:
Line 50: _V2V_DIR = os.path.join(P_VDSM_RUN, 'v2v')
Line 51: _VIRT_V2V = CommandPath('virt-v2v', '/usr/bin/virt-v2v')
Line 52:
Line 53: _OVF_NS = 'http://schemas.dmtf.org/ovf/envelope/1'
> I'd like to have a glance at the OVA format. There is some overview doc you
OVA contains files used to describe a virtual machine,
includes an .OVF descriptor file, optional manifest (.MF) and other related
files saved in a single tar files.
There is a reference on OVA from here:
https://en.wikipedia.org/wiki/Open_Virtualization_Format
Line 54: _RASD_NS = 'http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/' \
Line 55:'CIM_ResourceAllocationSettingData'
Line 56:
Line 57: ImportProgress = namedtuple('ImportProgress',
--
To view, visit https://gerrit.ovirt.org/43271
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I3c55c846f837bb5bf363717e05daabf5ee6631ca
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shahar Havivi
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Shahar Havivi
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: get VM information from OVA file
Francesco Romani has posted comments on this change.
Change subject: v2v: get VM information from OVA file
..
Patch Set 2:
(1 comment)
https://gerrit.ovirt.org/#/c/43271/2/vdsm/v2v.py
File vdsm/v2v.py:
Line 49:
Line 50: _V2V_DIR = os.path.join(P_VDSM_RUN, 'v2v')
Line 51: _VIRT_V2V = CommandPath('virt-v2v', '/usr/bin/virt-v2v')
Line 52:
Line 53: _OVF_NS = 'http://schemas.dmtf.org/ovf/envelope/1'
I'd like to have a glance at the OVA format. There is some overview doc you can
recommend me to look at?
Line 54: _RASD_NS = 'http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/' \
Line 55:'CIM_ResourceAllocationSettingData'
Line 56:
Line 57: ImportProgress = namedtuple('ImportProgress',
--
To view, visit https://gerrit.ovirt.org/43271
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I3c55c846f837bb5bf363717e05daabf5ee6631ca
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shahar Havivi
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek
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: get VM information from OVA file
automat...@ovirt.org has posted comments on this change. Change subject: v2v: get VM information 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/43271 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3c55c846f837bb5bf363717e05daabf5ee6631ca Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Shahar Havivi Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek 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: get VM information from OVA file
automat...@ovirt.org has posted comments on this change. Change subject: v2v: get VM information 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/43271 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3c55c846f837bb5bf363717e05daabf5ee6631ca Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Shahar Havivi 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: get VM information from OVA file
Shahar Havivi has uploaded a new change for review.
Change subject: v2v: get VM information from OVA file
..
v2v: get VM information from OVA file
In order to import a VM that exists in an OVA file the oVirt engine
needs to get information regarding the VM, such as disks sizes,
interfaces memory etc.
Adding a new verb: getOvfInfo
Change-Id: I3c55c846f837bb5bf363717e05daabf5ee6631ca
Signed-off-by: Shahar Havivi
---
M vdsm/API.py
M vdsm/rpc/Bridge.py
M vdsm/rpc/bindingxmlrpc.py
M vdsm/rpc/vdsmapi-schema.json
M vdsm/v2v.py
5 files changed, 97 insertions(+), 0 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/71/43271/1
diff --git a/vdsm/API.py b/vdsm/API.py
index eafceb7..645b962 100644
--- a/vdsm/API.py
+++ b/vdsm/API.py
@@ -1418,6 +1418,13 @@
"""
return v2v.get_external_vms(uri, username, password)
+def getOvaInfo(self, ova_path):
+"""
+return a dictionary, information regading a VM that is a part
+of the ova
+"""
+return v2v.get_ova_info(ova_path)
+
def convertExternalVm(self, uri, username, password, vminfo, jobid):
return v2v.convert_external_vm(uri, username, password, vminfo, jobid,
self._cif.irs)
diff --git a/vdsm/rpc/Bridge.py b/vdsm/rpc/Bridge.py
index 4302baa..99396c1 100644
--- a/vdsm/rpc/Bridge.py
+++ b/vdsm/rpc/Bridge.py
@@ -410,6 +410,7 @@
'Host_getDeviceList': {'ret': 'devList'},
'Host_getDevicesVisibility': {'ret': 'visible'},
'Host_getExternalVMs': {'ret': 'vmList'},
+'Host_getOvaInfo': {'ret': 'vm'},
'Host_getConvertedVm': {'ret': 'ovf'},
'Host_getHardwareInfo': {'ret': 'info'},
'Host_getLVMVolumeGroups': {'ret': 'vglist'},
diff --git a/vdsm/rpc/bindingxmlrpc.py b/vdsm/rpc/bindingxmlrpc.py
index 05110c4..c3d4022 100644
--- a/vdsm/rpc/bindingxmlrpc.py
+++ b/vdsm/rpc/bindingxmlrpc.py
@@ -372,6 +372,10 @@
api = API.Global()
return api.getExternalVMs(uri, username, password)
+def getOvaInfo(self, ova_path):
+api = API.Global()
+return api.getOvaInfo(ova_path)
+
def convertExternalVm(self, uri, username, password, vminfo, jobid):
password = ProtectedPassword(password)
api = API.Global()
@@ -1093,6 +1097,7 @@
(self.vmSetCpuTuneQuota, 'vmSetCpuTuneQuota'),
(self.vmSetCpuTunePeriod, 'vmSetCpuTunePeriod'),
(self.getExternalVMs, 'getExternalVMs'),
+(self.getOvaInfo, 'getOvaInfo'),
(self.convertExternalVm, 'convertExternalVm'),
(self.getConvertedVm, 'getConvertedVm'),
(self.abortV2VJob, 'abortV2VJob'),
diff --git a/vdsm/rpc/vdsmapi-schema.json b/vdsm/rpc/vdsmapi-schema.json
index 7c4fe1c..08b85d7 100644
--- a/vdsm/rpc/vdsmapi-schema.json
+++ b/vdsm/rpc/vdsmapi-schema.json
@@ -3930,6 +3930,24 @@
'data': {'uri': 'str', 'username': 'str', 'password': 'str'},
'returns': ['ExternalVmInfo']}
+
+##
+# @Host.getOvaInfo:
+#
+# Get VM information (disks, interfaces, memory etc) from OVA file
+#
+# @ova_path: path to the ova file
+#
+# Returns:
+# A dictionary with information regarding the VM
+#
+# Since: 4.17.0
+##
+{'command': {'class': 'Host', 'name': 'getOvaInfo'},
+ 'data': {'ova_path': 'str'},
+ 'returns': 'ExternalVmInfo'}
+
+
##
# @Host.convertExternalVm:
#
diff --git a/vdsm/v2v.py b/vdsm/v2v.py
index 18cc9bc..e891894 100644
--- a/vdsm/v2v.py
+++ b/vdsm/v2v.py
@@ -50,6 +50,10 @@
_V2V_DIR = os.path.join(P_VDSM_RUN, 'v2v')
_VIRT_V2V = CommandPath('virt-v2v', '/usr/bin/virt-v2v')
+_OVF_NS = 'http://schemas.dmtf.org/ovf/envelope/1'
+_RASD_NS = 'http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/' \
+ 'CIM_ResourceAllocationSettingData'
+
ImportProgress = namedtuple('ImportProgress',
['current_disk', 'disk_count', 'description'])
DiskProgress = namedtuple('DiskProgress', ['progress'])
@@ -161,6 +165,22 @@
job.start()
_add_job(job_id, job)
return {'status': doneCode}
+
+
+def get_ova_info(ova_path):
+ns = {'ovf': _OVF_NS, 'rasd': _RASD_NS}
+
+try:
+root = ET.fromstring(_read_ovf_from_ova(ova_path))
+except ET.ParseError as e:
+raise V2VError('Error reading ovf from ova, position: %r' % e.position)
+
+vm = {}
+_add_general_ovf_info(vm, root, ns)
+_add_disks_ovf_info(vm, root, ns)
+_add_networks_ovf_info(vm, root, ns)
+
+return {'status': doneCode, 'vm': vm}
def get_converted_vm(job_id):
@@ -651,3 +671,49 @@
if model is not None:
i['model'] = model.get('type')
params['networks'].append(i)
+
+
+def _read_ovf_from_ova(ova_path):
+cmd = ['/usr/bin/tar', 'xf', ova_path, '*.ovf', '--to-stdout']
+rc, output, error = execCmd(cmd)
+if rc:
+raise V2VError(error)
+
+return output
+
+
+def _add_general_ovf_info(vm, node, ns
