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 <[email protected]> Gerrit-Reviewer: Antoni Segura Puimedon <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: Piotr Kliczewski <[email protected]> Gerrit-Reviewer: Saggi Mizrahi <[email protected]> Gerrit-Reviewer: Shahar Havivi <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
