Dan Kenigsberg has posted comments on this change.
Change subject: BZ#720385 - Report disk latency (read , write & flush ) for
each storage device.
......................................................................
Patch Set 3: I would prefer that you didn't submit this
(5 inline comments)
....................................................
File vdsm/libvirtvm.py
Line 48:
Line 49: self.highWrite = utils.AdvancedStatsFunction(self._highWrite,
Line 50: config.getint('vars',
'vm_watermark_interval'))
Line 51: self.updateVolumes =
utils.AdvancedStatsFunction(self._updateVolumes,
Line 52: config.getint('irs',
'vol_size_sample_interval'))
but I liked this whitespace!
Line 53: self.sampleCpu = utils.AdvancedStatsFunction(self._sampleCpu,
Line 54: config.getint('vars',
'vm_sample_cpu_interval'),
Line 55: config.getint('vars',
'vm_sample_cpu_window'))
Line 56: self.sampleDisk = utils.AdvancedStatsFunction(self._sampleDisk,
Line 129: return stats
Line 130:
Line 131: diskLatency = {}
Line 132: res = libvirt_qemu.qemuMonitorCommand(self._vm._dom, 'info
blockstats',
Line 133:
libvirt_qemu.VIR_DOMAIN_QEMU_MONITOR_COMMAND_HMP)
H stands for Human, which Vdsm is not... Why not use the programatical api,
which should be more robust? Instead of parsing `info blockstats` yourself,
you'd ask json to do it.
Line 134: stats = _blockstatsParses(res)
Line 135: for vmDrive in self._vm._drives:
Line 136: try:
Line 137: diskLatency[vmDrive.name] = stats[vmDrive.alias]
Line 135: for vmDrive in self._vm._drives:
Line 136: try:
Line 137: diskLatency[vmDrive.name] = stats[vmDrive.alias]
Line 138: except KeyError:
Line 139: diskLatency[vmDrive.name] = [0, 0, 0, 0, 0, 0, 0, 0]
a dictionary would make a much clearer internal interface.
Line 140: self._log.warn("Disk %s latency not available",
vmDrive.name)
Line 141:
Line 142: return diskLatency
Line 143:
Line 217: wr_total_time_ns, rd_total_time_ns, flush_total_time_ns =
rawData
Line 218: try:
Line 219: readLatency = rd_total_time_ns / rd_op
Line 220: writeLatency = wr_total_time_ns / wr_op
Line 221: flushLatency = flush_total_time_ns / flush_op
this gives us average since VM creation, which is not very interesting, as
local jumps would be hidden. We should compare start sample to end sample.
(eInfo[read_ns] - sInfo[read_ns] ) / (eInfo[rd_op] - sInfo[rd_op] )
Line 222: except ZeroDivisionError:
Line 223: self._log.debug("Disk latency not available")
Line 224: readLatency, writeLatency, flushLatency = (0, 0, 0)
Line 225:
Line 981: raise Exception('destroy() called before Vm started')
Line 982: self._initInterfaces()
Line 983: self._getUnderlyingDriveInfo()
Line 984: self._getUnderlyingDisplayPort()
Line 985: # VmStatsThread may use block devices info from libvirt.
hey, this race is old!
Line 986: # So, run it after you have this info
Line 987: self._initVmStats()
Line 988: self.guestAgent = guestIF.GuestAgent(self._guestSocektFile,
self.log,
Line 989: connect=utils.tobool(self.conf.get('vmchannel',
'true')))
--
To view, visit http://gerrit.usersys.redhat.com/935
To unsubscribe, visit http://gerrit.usersys.redhat.com/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Id0d3d4869bcf35ed4d2ff72a5e1ef7aa21fe3455
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Igor Lvovsky <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Federico Simoncelli <[email protected]>
Gerrit-Reviewer: Igor Lvovsky <[email protected]>
Gerrit-Reviewer: Rami Vaknin <[email protected]>
_______________________________________________
vdsm-patches mailing list
[email protected]
https://fedorahosted.org/mailman/listinfo/vdsm-patches
