Nir Soffer has posted comments on this change. Change subject: virt: faster getAllVmStats ......................................................................
Patch Set 1: (5 comments) r http://gerrit.ovirt.org/#/c/35140/1//COMMIT_MSG Commit Message: Line 7: virt: faster getAllVmStats Line 8: Line 9: avoid (apparently?) wasteful work and make Line 10: the verb implementation faster with no Line 11: changes in behaviour. It is not clear from the patch was is the wasteful work, and what is the effect of this change. Did you profile this or run some benchmarks? Do we have a test for this code? Line 12: Line 13: Change-Id: If1ed3f49c236086c6184973762d9909d7f79bf7e http://gerrit.ovirt.org/#/c/35140/1/vdsm/API.py File vdsm/API.py: Line 1287 Line 1288 Line 1289 Line 1290 Line 1291 Why did you replace this with self._cif.vmContainer.values()? Line 1290 Line 1291 Line 1292 Line 1293 Line 1294 Why did you remove the runHooks=False argument to getStats()? Line 1292 Line 1293 Line 1294 Line 1295 Line 1296 Why did you remove the check fro falsy response? And why did you return now the response, instead of response['statsList'][0]? http://gerrit.ovirt.org/#/c/35140/1/vdsm/virt/vm.py File vdsm/virt/vm.py: Line 2259: Please note that some values are provided by client (engine) Line 2260: but can change as result as interaction with libvirt (display*) Line 2261: """ Line 2262: stats = { Line 2263: 'vmId': self.conf['vmId'], Is this related? Line 2264: 'pid': self.conf['pid'], Line 2265: 'vmType': self.conf['vmType'], Line 2266: 'kvmEnable': self._kvmEnable, Line 2267: 'acpiEnable': self.conf.get('acpiEnable', 'true')} -- To view, visit http://gerrit.ovirt.org/35140 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: If1ed3f49c236086c6184973762d9909d7f79bf7e Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani <[email protected]> Gerrit-Reviewer: Nir Soffer <[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
