Francesco Romani has posted comments on this change. Change subject: virt: stats: make disk_rate more robust ......................................................................
Patch Set 12: Code-Review-1 (2 comments) https://gerrit.ovirt.org/#/c/48493/12/vdsm/virt/vmstats.py File vdsm/virt/vmstats.py: Line 308: stats = {} Line 309: Line 310: for name, mode in (("readRate", "rd"), ("writeRate", "wr")): Line 311: first_key = 'block.%d.%s.bytes' % (first_index, mode) Line 312: last_key = 'block.%d.%s.bytes' % (last_index, mode) > Are you sure first_index and last_index are not always the same? We check s Unfortunately we cannot be sure. libvirt does not guarantee the order of devices is the same across calls *for this API*. It *usually* is (except for hotplug/hotunplug), but we must reconstruct the map after each call to be 100% safe. Then we need different indexes. Line 313: try: Line 314: stats[name] = str( Line 315: (last_sample[last_key] - first_sample[first_key]) Line 316: / interval Line 315: (last_sample[last_key] - first_sample[first_key]) Line 316: / interval Line 317: ) Line 318: except KeyError: Line 319: pass > Please separate lookup from formatting. We don't gain anything by having st sure, will fix. Line 320: Line 321: return stats Line 322: Line 323: -- To view, visit https://gerrit.ovirt.org/48493 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib2900e73161932f7fbecd9d7bfaba36dc1e84e19 Gerrit-PatchSet: 12 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: Jenkins CI Gerrit-Reviewer: Martin Polednik <mpoled...@redhat.com> Gerrit-Reviewer: Milan Zamazal <mzama...@redhat.com> Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com> Gerrit-Reviewer: Vinzenz Feenstra <vfeen...@redhat.com> Gerrit-Reviewer: gerrit-hooks <automat...@ovirt.org> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
