Francesco Romani has posted comments on this change. Change subject: virt: sampling: consolidate disk statistics ......................................................................
Patch Set 5: (3 comments) http://gerrit.ovirt.org/#/c/29953/5/vdsm/virt/vm.py File vdsm/virt/vm.py: Line 219: (config.getint('vars', 'vm_sample_disk_interval') != Line 220: config.getint('vars', 'vm_sample_disk_latency_interval')) Line 221: or Line 222: (config.getint('vars', 'vm_sample_disk_window') != Line 223: config.getint('vars', 'vm_sample_disk_latency_window'))) > Lets remove this unneeded complexity. OK, will reintroduce, if needed, in a later patch. Line 224: Line 225: self.highWrite = ( Line 226: AdvancedStatsFunction( Line 227: self._highWrite, Line 306: Line 307: diskSamples = {} Line 308: for vmDrive in self._vm.getDiskDevices(): Line 309: diskSamples[vmDrive.name] = self._vm._dom.blockStatsFlags( Line 310: vmDrive.name, flags=libvirt.VIR_TYPED_PARAM_STRING_OKAY) > How is this related? This is the call getDiskLatency uses until this change, and gives us a superset of the informations of blockStats(). In the new optimized code paths, we calc both disk stats with the result of one blockStatsFlags call here, then this hunk is needed. Line 311: Line 312: return diskSamples Line 313: Line 314: def _sampleDiskLatency(self): Line 633: def _calcDiskRate(vmDrive, sInfo, eInfo, sampleInterval): Line 634: return { Line 635: 'readRate': ( Line 636: (eInfo[vmDrive.name]['rd_bytes'] - Line 637: sInfo[vmDrive.name]['rd_bytes']) > How is this related? It is because blockStats and blockStatsFlags returns different objects. blockStats gives back a tuple of longs: /* convert to a Python tuple of long objects */ if ((info = PyTuple_New(5)) == NULL) return VIR_PY_NONE; PyTuple_SetItem(info, 0, libvirt_longlongWrap(stats.rd_req)); PyTuple_SetItem(info, 1, libvirt_longlongWrap(stats.rd_bytes)); PyTuple_SetItem(info, 2, libvirt_longlongWrap(stats.wr_req)); PyTuple_SetItem(info, 3, libvirt_longlongWrap(stats.wr_bytes)); PyTuple_SetItem(info, 4, libvirt_longlongWrap(stats.errs)); return info; blockStatsFlags returns a dict wrapping libvirt's so-called 'typed parameters'. Then we must change how we access the fields in the returned object. Maybe I'm biased but I find the code a little less obscure with this change applied. Line 638: / sampleInterval), Line 639: 'writeRate': ( Line 640: (eInfo[vmDrive.name]['wr_bytes'] - Line 641: sInfo[vmDrive.name]['wr_bytes']) -- To view, visit http://gerrit.ovirt.org/29953 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0dabd079f81270c7099c74469a18f8b23c97cc8c Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani <from...@redhat.com> Gerrit-Reviewer: Adam Litke <ali...@redhat.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: Francesco Romani <from...@redhat.com> Gerrit-Reviewer: Martin Polednik <mpoled...@redhat.com> Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com> Gerrit-Reviewer: Vinzenz Feenstra <vfeen...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
