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