Change in vdsm[master]: vm: avoid to reply with half-baked statistics

danken at redhat.com danken at redhat.com
Sun Apr 6 21:17:40 UTC 2014


Dan Kenigsberg has posted comments on this change.

Change subject: vm: avoid to reply with half-baked statistics
......................................................................


Patch Set 5: Code-Review-1

(4 comments)

Please add a unit test, documenting the expected result for getAllVmStats if one of two VMs has partial statistics?

http://gerrit.ovirt.org/#/c/25803/5//COMMIT_MSG
Commit Message:

Line 9: commit 8fedf8bde3c28edb07add23c3e9b72681cb48e49 introduced a tiny
Line 10: window for races between libvirt notifications (vm._onQemuDeath)
Line 11: and polling for stats from engine.
Line 12: 
Line 13: This patch address tackles this problem by moving toward a more
s/address//
Line 14: 'transactional' behaviour for statistics gathering.
Line 15: Either we reply to the engine's enquiry with a well formed,
Line 16: complete statistic sample, or we don't reply at all, and instead
Line 17: we raise an error code.


Line 16: complete statistic sample, or we don't reply at all, and instead
Line 17: we raise an error code.
Line 18: 
Line 19: Change-Id: I65197459cd183af5e7a1634a5ffb01719364a070
Line 20: Bug-Url: https://bugzilla.redhat.com/1073478
Please explain how this patch resolves this bug - I do not understand it at all.


http://gerrit.ovirt.org/#/c/25803/5/vdsm/API.py
File vdsm/API.py:

Line 361:         """
Line 362:         v = self._cif.vmContainer.get(self._UUID)
Line 363:         if not v:
Line 364:             return errCode['noVM']
Line 365:         stats = v.getStats().copy()
why do we need the intermediate conversion of Exception to an empty dict? it would be a little less ugly to catch Exception up here instead of in vm.py.
Line 366:         if not stats:
Line 367:             return errCode['statsErr']
Line 368:         stats['vmId'] = self._UUID
Line 369:         return {'status': doneCode, 'statsList': [stats]}


http://gerrit.ovirt.org/#/c/25803/5/vdsm/virt/vm.py
File vdsm/virt/vm.py:

Line 2901:         stats['timeOffset'] = self.conf.get('timeOffset', '0')
Line 2902:         stats['clientIp'] = self.conf.get('clientIp', '')
Line 2903:         if 'pauseCode' in self.conf:
Line 2904:             stats['pauseCode'] = self.conf['pauseCode']
Line 2905:         stats.update(self.guestAgent.getGuestInfo())
much better, but please pre-verify that guestAgent is not None.
Line 2906:         memUsage = 0
Line 2907:         realMemUsage = int(stats['memUsage'])
Line 2908:         if realMemUsage != 0:
Line 2909:             memUsage = (100 - float(realMemUsage) /


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I65197459cd183af5e7a1634a5ffb01719364a070
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Jarod.w <work.iec23801 at gmail.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek at redhat.com>
Gerrit-Reviewer: Omer Frenkel <ofrenkel at redhat.com>
Gerrit-Reviewer: Roy Golan <rgolan at redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr 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