Change in vdsm[master]: virt: sampling: consolidate disk statistics
Francesco Romani has posted comments on this change. Change subject: virt: sampling: consolidate disk statistics .. Patch Set 9: updated commit message with explanation about the involved libvirt calls. -- 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: 9 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: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: sampling: consolidate disk statistics
Francesco Romani has posted comments on this change. Change subject: virt: sampling: consolidate disk statistics .. Patch Set 9: Verified+1 verified using vdsClient, inspecting output of getAllVmStats. - booted a VM on a patched VDSM and let it run - in a shell, did watch vdsClient -s 0 getAllVmStats - verified that the *rate and *latency values are still present. -- 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: 9 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: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: sampling: consolidate disk statistics
oVirt Jenkins CI Server has posted comments on this change. Change subject: virt: sampling: consolidate disk statistics .. Patch Set 9: Build Failed http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/12873/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_virt_functional_tests_gerrit/1764/ : There was an infra issue, please contact in...@ovirt.org http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/12715/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/11924/ : FAILURE -- 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: 9 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: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: sampling: consolidate disk statistics
Dan Kenigsberg has posted comments on this change. Change subject: virt: sampling: consolidate disk statistics .. Patch Set 9: Code-Review+2 (1 comment) http://gerrit.ovirt.org/#/c/29953/9/vdsm/virt/vm.py File vdsm/virt/vm.py: Line 559 Line 560 Line 561 Line 562 Line 563 oh, you are dropping the code that offended me. cool. -- 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: 9 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
Change in vdsm[master]: virt: sampling: consolidate disk statistics
Dan Kenigsberg has submitted this change and it was merged. Change subject: virt: sampling: consolidate disk statistics .. virt: sampling: consolidate disk statistics We used to have two different sampling, and thus two libvirt calls per sampling, per disk. One sampling gathers basic disk data and R/W rate; the other one gathers the disk latency. Most often, as the default values are unchanged, the configuration for these sampling is identical, so it is safe and cheaper to read all the needed information in one go, saving one libvirt call without losing any information. These sampling uses two different libvirt calls. sampleDisk, which gathers the disk I/O rate, uses the older virDomainGetBlockStats call; sampleDiskLatency, which gathers disk latency, uses the newer and more flexible virDomainGetBlockStatsFlags; Inside libvirt, these two calls are implemented on top of the same QEMU monitor function (qemuMonitorGetBlockStatsInfo). The newer call returns all the data gathered by the QEMU monitor, while the older call returns just a subset of it. Other differences in the API exists, like the return type and the presence of the flags, but those can safely be considered of no interest for VDSM. Change-Id: I0dabd079f81270c7099c74469a18f8b23c97cc8c Signed-off-by: Francesco Romani from...@redhat.com Reviewed-on: http://gerrit.ovirt.org/29953 Reviewed-by: Nir Soffer nsof...@redhat.com Reviewed-by: Dan Kenigsberg dan...@redhat.com --- M vdsm/virt/vm.py 1 file changed, 14 insertions(+), 45 deletions(-) Approvals: Nir Soffer: Looks good to me, but someone else must approve Dan Kenigsberg: Looks good to me, approved Francesco Romani: Verified -- To view, visit http://gerrit.ovirt.org/29953 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: I0dabd079f81270c7099c74469a18f8b23c97cc8c Gerrit-PatchSet: 10 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 ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: sampling: consolidate disk statistics
oVirt Jenkins CI Server has posted comments on this change. Change subject: virt: sampling: consolidate disk statistics .. Patch Set 10: Build Successful http://jenkins.ovirt.org/job/vdsm_master_unit-tests_merged/4066/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_create-rpms-fc21-x86_64_merged/52/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_create-rpms-el6-x86_64_merged/79/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_create-rpms_merged_test_debug/274/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_verify-error-codes_merged/5906/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_create-rpms-el7-x86_64_merged/76/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_create-rpms-fc20-x86_64_merged/72/ : SUCCESS -- 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: 10 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: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: sampling: consolidate disk statistics
Francesco Romani has posted comments on this change. Change subject: virt: sampling: consolidate disk statistics .. Patch Set 7: rebased. Shed more no longer needed code. documented VIR_TYPED_PARAM_STRING_OKAY as requested. -- 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: 7 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: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: sampling: consolidate disk statistics
oVirt Jenkins CI Server has posted comments on this change. Change subject: virt: sampling: consolidate disk statistics .. Patch Set 7: Build Failed http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/12845/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/11896/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_virt_functional_tests_gerrit/1753/ : There was an infra issue, please contact in...@ovirt.org http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/12687/ : SUCCESS -- 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: 7 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: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: sampling: consolidate disk statistics
Nir Soffer has posted comments on this change. Change subject: virt: sampling: consolidate disk statistics .. Patch Set 7: (1 comment) http://gerrit.ovirt.org/#/c/29953/7/vdsm/virt/vm.py File vdsm/virt/vm.py: Line 299: # depending on is ok with that. Quoting libvirt header: Line 300: # Older servers lacked the ability to handle string typed Line 301: # parameters.[...] This flag is automatically set when needed, Line 302: # [...] however, manually setting the flag can be used to Line 303: # reject servers that cannot return typed strings [...] Thanks for the verbose explanation!, Two issues: - It should come before the line using the cryptic parameter, either the blocksStatsFlags() line or the entire loop so the loop is more readable. - Capitialization: the - The Line 304: return diskSamples Line 305: Line 306: def _sampleNet(self): Line 307: netSamples = {} -- 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: 7 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
Change in vdsm[master]: virt: sampling: consolidate disk statistics
Nir Soffer has posted comments on this change. Change subject: virt: sampling: consolidate disk statistics .. Patch Set 7: (1 comment) http://gerrit.ovirt.org/#/c/29953/7/vdsm/virt/vm.py File vdsm/virt/vm.py: Line 299: # depending on is ok with that. Quoting libvirt header: Line 300: # Older servers lacked the ability to handle string typed Line 301: # parameters.[...] This flag is automatically set when needed, Line 302: # [...] however, manually setting the flag can be used to Line 303: # reject servers that cannot return typed strings [...] Thanks for the verbose explanation!, Hmm, according to the current patch, the line with this cryptic flag already exists in the older code, but in version 5 (http://gerrit.ovirt.org/#/c/29953/5/vdsm/virt/vm.py), it was new. Is this a result of a rebase? If this patch does not add this flag, there is no point to add the documentation here, and it is best done in a separate documentation patch (which does not need verification). Line 304: return diskSamples Line 305: Line 306: def _sampleNet(self): Line 307: netSamples = {} -- 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: 7 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
Change in vdsm[master]: virt: sampling: consolidate disk statistics
Francesco Romani has posted comments on this change. Change subject: virt: sampling: consolidate disk statistics .. Patch Set 7: (1 comment) http://gerrit.ovirt.org/#/c/29953/7/vdsm/virt/vm.py File vdsm/virt/vm.py: Line 299: # depending on is ok with that. Quoting libvirt header: Line 300: # Older servers lacked the ability to handle string typed Line 301: # parameters.[...] This flag is automatically set when needed, Line 302: # [...] however, manually setting the flag can be used to Line 303: # reject servers that cannot return typed strings [...] Hmm, according to the current patch, the line with this cryptic flag alread Sorry for the rebase noise, I'll rebase less often (here and of course in any other future patch). The cryptic flag was always used since sampleDiskLatency existed; in this patch I'm just moving from blockStats to blockStatsFlags in sampleDisk, hence the reason why it popped out here. Will move the explanation in a new separate doc-only patch. Line 304: return diskSamples Line 305: Line 306: def _sampleNet(self): Line 307: netSamples = {} -- 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: 7 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
Change in vdsm[master]: virt: sampling: consolidate disk statistics
oVirt Jenkins CI Server has posted comments on this change. Change subject: virt: sampling: consolidate disk statistics .. Patch Set 8: Build Failed http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/12852/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_virt_functional_tests_gerrit/1760/ : There was an infra issue, please contact in...@ovirt.org http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/12694/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/11903/ : FAILURE -- 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: 8 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: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: sampling: consolidate disk statistics
Nir Soffer has posted comments on this change. Change subject: virt: sampling: consolidate disk statistics .. Patch Set 8: Code-Review+1 Can be nice if you explain the difference between blockStats() and blockStatsFlags() in the commit message. -- 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: 8 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: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: sampling: consolidate disk statistics
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
Change in vdsm[master]: virt: sampling: consolidate disk statistics
oVirt Jenkins CI Server has posted comments on this change. Change subject: virt: sampling: consolidate disk statistics .. Patch Set 6: Build Failed http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/12779/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/11833/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_virt_functional_tests_gerrit/1736/ : There was an infra issue, please contact in...@ovirt.org http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/12622/ : SUCCESS -- 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: 6 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: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: sampling: consolidate disk statistics
Nir Soffer has posted comments on this change. Change subject: virt: sampling: consolidate disk statistics .. Patch Set 6: Code-Review+1 -- 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: 6 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: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: sampling: consolidate disk statistics
Nir Soffer has posted comments on this change. Change subject: virt: sampling: consolidate disk statistics .. Patch Set 5: (1 comment) http://gerrit.ovirt.org/#/c/29953/5/vdsm/virt/vm.py File vdsm/virt/vm.py: 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) This is the call getDiskLatency uses until this change, and gives us a supe Ok - it would be nice to document this cryptic flag (in another patch if you like) Line 311: Line 312: return diskSamples Line 313: Line 314: def _sampleDiskLatency(self): -- 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
Change in vdsm[master]: virt: sampling: consolidate disk statistics
Martin Polednik has posted comments on this change. Change subject: virt: sampling: consolidate disk statistics .. Patch Set 6: Code-Review+1 -- 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: 6 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: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: sampling: consolidate disk statistics
Nir Soffer has posted comments on this change. Change subject: virt: sampling: consolidate disk statistics .. Patch Set 5: Code-Review-1 (3 comments) Please remove unrelated changes. 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. 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? 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? 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
Change in vdsm[master]: virt: sampling: consolidate disk statistics
Nir Soffer has posted comments on this change. Change subject: virt: sampling: consolidate disk statistics .. Patch Set 4: (1 comment) Partial review http://gerrit.ovirt.org/#/c/29953/4//COMMIT_MSG Commit Message: Line 16: and cheaper to read all the needed information in one go, Line 17: saving one libvirt call without losing any information. Line 18: Line 19: If the configuration is different, the old behaviour is Line 20: preserved for the sake of backward compatibility. Do we need this backward compatibility? I think this configuration is for us, so we can modify the behavior easily, not for users. This configuration is not documented right? Lets drop the false backward compatibility and simplify the code. Line 21: Line 22: Change-Id: I0dabd079f81270c7099c74469a18f8b23c97cc8c -- 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: 4 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
Change in vdsm[master]: virt: sampling: consolidate disk statistics
Francesco Romani has posted comments on this change. Change subject: virt: sampling: consolidate disk statistics .. Patch Set 4: (1 comment) http://gerrit.ovirt.org/#/c/29953/4//COMMIT_MSG Commit Message: Line 16: and cheaper to read all the needed information in one go, Line 17: saving one libvirt call without losing any information. Line 18: Line 19: If the configuration is different, the old behaviour is Line 20: preserved for the sake of backward compatibility. Do we need this backward compatibility? I think this configuration is for u I agree with you, because the backward compatibility makes the code uglier in every way I can conceive. But we exposed these settings to the user (and long ago), so we are bound to keep them for all the 3.x.y cycle, I believe. Dan? Line 21: Line 22: Change-Id: I0dabd079f81270c7099c74469a18f8b23c97cc8c -- 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: 4 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
Change in vdsm[master]: virt: sampling: consolidate disk statistics
Martin Polednik has posted comments on this change. Change subject: virt: sampling: consolidate disk statistics .. Patch Set 4: Code-Review+1 -- 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: 4 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: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: sampling: consolidate disk statistics
oVirt Jenkins CI Server has posted comments on this change. Change subject: virt: sampling: consolidate disk statistics .. Patch Set 5: Build Failed http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/11791/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/12737/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_virt_functional_tests_gerrit/1712/ : There was an infra issue, please contact in...@ovirt.org http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/12580/ : SUCCESS -- 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: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: sampling: consolidate disk statistics
oVirt Jenkins CI Server has posted comments on this change. Change subject: virt: sampling: consolidate disk statistics .. Patch Set 4: Build Failed http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/11755/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/12699/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_virt_functional_tests_gerrit/1704/ : There was an infra issue, please contact in...@ovirt.org http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/12544/ : SUCCESS -- 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: 4 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: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: sampling: consolidate disk statistics
Francesco Romani has posted comments on this change. Change subject: virt: sampling: consolidate disk statistics .. Patch Set 3: rebased. -- 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: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: sampling: consolidate disk statistics
oVirt Jenkins CI Server has posted comments on this change. Change subject: virt: sampling: consolidate disk statistics .. Patch Set 3: Build Successful http://jenkins.ovirt.org/job/vdsm_master_virt_functional_tests_gerrit/1424/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/10757/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/11699/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/11542/ : SUCCESS -- 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: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: sampling: consolidate disk statistics
Dan Kenigsberg has posted comments on this change. Change subject: virt: sampling: consolidate disk statistics .. Patch Set 2: Code-Review+1 (1 comment) http://gerrit.ovirt.org/#/c/29953/2/vdsm/virt/vm.py File vdsm/virt/vm.py: Line 185: Line 186: def __init__(self, vm): Line 187: AdvancedStatsThread.__init__(self, log=vm.log, daemon=True) Line 188: self._vm = vm Line 189: # ugly hack to preserve backward compatibility I see no reason to keep a separate vm_sample_disk_latency_interval or ANY window != 2. Please merge vm_sample_disk_latency_interval into vm_sample_disk_interval in a separate patch. Line 190: self._splitDiskStats = ( Line 191: (config.getint('vars', 'vm_sample_disk_interval') != Line 192: config.getint('vars', 'vm_sample_disk_latency_interval')) Line 193: or -- 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: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Francesco Romani from...@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
Change in vdsm[master]: virt: sampling: consolidate disk statistics
Francesco Romani has posted comments on this change. Change subject: virt: sampling: consolidate disk statistics .. Patch Set 2: Verified+1 verified with 29951 and 29952 using both the new and old flow. Stats shows correctly on vdsClient. -- 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: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: sampling: consolidate disk statistics
oVirt Jenkins CI Server has posted comments on this change. Change subject: virt: sampling: consolidate disk statistics .. Patch Set 2: Build Successful http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/10223/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/11008/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_virt_functional_tests_gerrit/1265/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/11165/ : SUCCESS -- 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: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: sampling: consolidate disk statistics
Francesco Romani has uploaded a new change for review. Change subject: virt: sampling: consolidate disk statistics .. virt: sampling: consolidate disk statistics We used to have two different sampling, and thus two libvirt calls for sampling, for disk. One sampling gathers basic disk data and R/W rate; the other one gathers the disk latency. We can read all the needed information in one go, thus saving one not cheap libvirt call without losing any information. This patch does that. Change-Id: I0dabd079f81270c7099c74469a18f8b23c97cc8c Signed-off-by: Francesco Romani from...@redhat.com --- M lib/vdsm/config.py.in M vdsm/virt/vm.py 2 files changed, 17 insertions(+), 42 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/53/29953/1 diff --git a/lib/vdsm/config.py.in b/lib/vdsm/config.py.in index 52e0cd9..9f67c25 100644 --- a/lib/vdsm/config.py.in +++ b/lib/vdsm/config.py.in @@ -165,10 +165,6 @@ ('vm_sample_disk_window', '2', None), -('vm_sample_disk_latency_interval', '60', None), - -('vm_sample_disk_latency_window', '2', None), - ('vm_sample_net_interval', '15', None), ('vm_sample_net_window', '2', None), diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py index 1082d1c..667b9ca 100644 --- a/vdsm/virt/vm.py +++ b/vdsm/virt/vm.py @@ -205,11 +205,6 @@ self._sampleDisk, config.getint('vars', 'vm_sample_disk_interval'), config.getint('vars', 'vm_sample_disk_window'))) -self.sampleDiskLatency = ( -AdvancedStatsFunction( -self._sampleDiskLatency, -config.getint('vars', 'vm_sample_disk_latency_interval'), -config.getint('vars', 'vm_sample_disk_latency_window'))) self.sampleNet = ( AdvancedStatsFunction( self._sampleNet, @@ -238,8 +233,8 @@ self.addStatsFunction( self.highWrite, self.updateVolumes, self.sampleCpu, -self.sampleDisk, self.sampleDiskLatency, self.sampleNet, -self.sampleBalloon, self.sampleVmJobs, self.sampleVcpuPinning, +self.sampleDisk, self.sampleNet, self.sampleBalloon, +self.sampleVmJobs, self.sampleVcpuPinning, self.sampleCpuTune) def _highWrite(self): @@ -265,25 +260,16 @@ # Avoid queries from storage during recovery process return -diskSamples = {} -for vmDrive in self._vm.getDiskDevices(): -diskSamples[vmDrive.name] = self._vm._dom.blockStats(vmDrive.name) - -return diskSamples - -def _sampleDiskLatency(self): -if not self._vm.isDisksStatsCollectionEnabled(): -# Avoid queries from storage during recovery process -return # {'wr_total_times': 0L, 'rd_operations': 9638L, # 'flush_total_times': 0L,'rd_total_times': 7622718001L, # 'rd_bytes': 85172430L, 'flush_operations': 0L, # 'wr_operations': 0L, 'wr_bytes': 0L} -diskLatency = {} +diskSamples = {} for vmDrive in self._vm.getDiskDevices(): -diskLatency[vmDrive.name] = self._vm._dom.blockStatsFlags( +diskSamples[vmDrive.name] = self._vm._dom.blockStatsFlags( vmDrive.name, flags=libvirt.VIR_TYPED_PARAM_STRING_OKAY) -return diskLatency + +return diskSamples def _sampleNet(self): netSamples = {} @@ -517,6 +503,13 @@ # will be None if sampled during recovery dStats.update(self._calcDiskRate(vmDrive, sInfo, eInfo, sampleInterval)) +dLatency = self._calcDiskLatency(vmDrive, sInfo, eInfo, + sampleInterval) +else: +dLatency = {'readLatency': '0', +'writeLatency': '0', +'flushLatency': '0'} +dStats.update(dLantency) except (AttributeError, KeyError, TypeError, ZeroDivisionError): self._log.exception(Disk %s stats not available, dName) @@ -526,29 +519,16 @@ try: return { 'readRate': ( -(eInfo[vmDrive.name][1] - sInfo[vmDrive.name][1]) +(eInfo[vmDrive.name]['rd_bytes'] - + sInfo[vmDrive.name]['rd_bytes']) / sampleInterval), 'writeRate': ( -(eInfo[vmDrive.name][3] - sInfo[vmDrive.name][3]) +(eInfo[vmDrive.name]['wr_bytes'] - + sInfo[vmDrive.name]['wr_bytes']) / sampleInterval) } except (AttributeError, KeyError, TypeError, ZeroDivisionError): self._log.exception(Disk %s stats not available,
Change in vdsm[master]: virt: sampling: consolidate disk statistics
oVirt Jenkins CI Server has posted comments on this change. Change subject: virt: sampling: consolidate disk statistics .. Patch Set 1: Code-Review-1 Verified-1 Build Failed http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/10167/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/10952/ : UNSTABLE http://jenkins.ovirt.org/job/vdsm_master_virt_functional_tests_gerrit/1240/ : There was an infra issue, please contact in...@ovirt.org http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/11108/ : SUCCESS -- 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: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches