Change in vdsm[master]: External hypervisor VMs integration

danken at redhat.com danken at redhat.com
Tue Dec 9 16:29:02 UTC 2014


Dan Kenigsberg has posted comments on this change.

Change subject: External hypervisor VMs integration
......................................................................


Patch Set 33:

(2 comments)

http://gerrit.ovirt.org/#/c/33309/33/vdsm/v2v.py
File vdsm/v2v.py:

Line 38:         for vm in conn.listAllDomains():
Line 39:             root = ET.fromstring(vm.XMLDesc(0))
Line 40:             params = {}
Line 41:             params['vmName'] = vm.name()
Line 42:             if vm.state()[0] == libvirt.VIR_DOMAIN_SHUTOFF:
> Dan,
I'm not sure I understand the question: Are you worried about the O(n) complexity of this function? It requires 2*n+1 libvirt calls, which seems reasonable. 

The only problem is that a single blocking qemu process can block the whole verb. This is why in vdsm proper we collect the stats in one thread, cache it, and report the cached value on another thread.
Line 43:                 params['status'] = "Down"
Line 44:             else:
Line 45:                 params['status'] = "Up"
Line 46:             try:


Line 41:             params['vmName'] = vm.name()
Line 42:             if vm.state()[0] == libvirt.VIR_DOMAIN_SHUTOFF:
Line 43:                 params['status'] = "Down"
Line 44:             else:
Line 45:                 params['status'] = "Up"
All other states, such as VIR_DOMAIN_SHUTDOWN, VIR_DOMAIN_SHUTOFF_CRASHED etc?

Should they really be translated to "up"?

Why do you need the state at all? Is there any logic on Engine regarding it?
Line 46:             try:
Line 47:                 _addGeneralInfo(root, params)
Line 48:             except InvalidVMConfiguration as e:
Line 49:                 logging.error('error parsing domain xml, msg: %s  xml: %s',


-- 
To view, visit http://gerrit.ovirt.org/33309
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7dcfb860626a844d1d08590274b508519a33f4a3
Gerrit-PatchSet: 33
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shahar Havivi <shavivi at redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski at gmail.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi at redhat.com>
Gerrit-Reviewer: Shahar Havivi <shavivi at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list