Mark Wu has posted comments on this change.

Change subject: report cpuUser and cpuSys separately
......................................................................


Patch Set 3: I would prefer that you didn't submit this

(2 inline comments)

....................................................
File vdsm/libvirtvm.py
Line 166:     def _diff(self, prev, curr, val):
Line 167:         return prev[val] - curr[val]
Line 168: 
Line 169:     def _usagePercentage(self, val, sampleInterval):
Line 170:         return 100 * val / sampleInterval / 1000 ** 3
How about changing to use a constant?  like NANOSECONDS_PER_SECOND
Line 171: 
Line 172:     def _getCpuStats(self, stats):
Line 173:         sInfo, eInfo, sampleInterval = self.sampleCpu.getStats()
Line 174: 


Line 172:     def _getCpuStats(self, stats):
Line 173:         sInfo, eInfo, sampleInterval = self.sampleCpu.getStats()
Line 174: 
Line 175:         try:
Line 176:             stats['cpuSys'] = self._usagePercentage(
It seems that you are calculating the emulator time and guest time. So the 
names 'cpuSys' and 'cpuUser' are misleading. Why not change it?  Break API?  At 
least you need update the schema file.
Line 177:                 self._diff(eInfo, sInfo, 'user_time')
Line 178:                  + self._diff(eInfo, sInfo, 'system_time'),
Line 179:                 sampleInterval)
Line 180:             stats['cpuUser'] = self._usagePercentage(


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I663ad25ff3ff5ce426b5159b6c9a65b7f5167605
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Laszlo Hornyak <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Gal Hammer <[email protected]>
Gerrit-Reviewer: Geert Jansen <[email protected]>
Gerrit-Reviewer: Laszlo Hornyak <[email protected]>
Gerrit-Reviewer: Mark Wu <[email protected]>
Gerrit-Reviewer: Royce Lv <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to