Nir Soffer has posted comments on this change.

Change subject: virt: faster getAllVmStats
......................................................................


Patch Set 1:

(3 comments)

Patch makes sense, need to explain why in the commit message and implement the 
logic in cif.

http://gerrit.ovirt.org/#/c/35140/1/vdsm/API.py
File vdsm/API.py:

Line 1287
Line 1288
Line 1289
Line 1290
Line 1291
> Here and in the lines below we just want to iterate in all the VMs inside v
I agree that this is horrible - API should not such code.


Line 1292
Line 1293
Line 1294
Line 1295
Line 1296
> Because I find the check bogus.
ok


Line 1287:         """
Line 1288:         Get statistics of all running VMs.
Line 1289:         """
Line 1290:         hooks.before_get_all_vm_stats()
Line 1291:         statsList = [v.getStats() for v in 
self._cif.vmContainer.values()]
Since cif is holding the vms, it should implement getting vm status - so just 
return here self._cif.get_vm_stats().

First step in removing logic from API.
Line 1292:         statsList = hooks.after_get_all_vm_stats(statsList)
Line 1293:         return {'status': doneCode, 'statsList': statsList}
Line 1294: 
Line 1295:     def hostdevListByCaps(self, caps):


-- 
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: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: 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

Reply via email to