Nir Soffer has posted comments on this change. Change subject: host stats: Collect stats from online cpu cores only ......................................................................
Patch Set 6: (2 comments) https://gerrit.ovirt.org/#/c/46269/6//COMMIT_MSG Commit Message: Line 26: 'nodeIndex': 0}, Line 27: '1': {'cpuIdle': '96.07', Line 28: 'cpuSys': '0.47', Line 29: 'cpuUser': '3.46', Line 30: 'nodeIndex': 0}} The indentation make it hard to understand this format - it is { '0': {...}, '1': {...}, ... } Right? Then, regarding https://gerrit.ovirt.org/46270, why not add the status of the cpu in the same dict? For example - online cpu: { 'online': True, cpuIdle: 90.74, 'cpuSys': '1.40', 'cpuUser': '7.86', 'nodeIndex': 0 } And offline: { 'online': False, 'nodeIndex': 0 } Having all the information in one data structure is nicer to the consumer of this api. Line 31: [...] Line 32: Line 33: When only one CPU out of two has been online before libvird was Line 34: started, vdsm reports: Line 50: cpuStatistics = {'0': {'cpuIdle': '90.74', Line 51: 'cpuSys': '1.40', Line 52: 'cpuUser': '7.86', Line 53: 'nodeIndex': 0}} Line 54: [...] We should report all cpus, with the relevant info for each. We should have the same format regardless cpus taken offline or online while libvirtd/vdsm are running. Line 55: Line 56: Change-Id: Ia9c247f9138e02a9230a0849a04cb2e1705e7fac -- To view, visit https://gerrit.ovirt.org/46269 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia9c247f9138e02a9230a0849a04cb2e1705e7fac Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Roman Mohr <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: Omer Frenkel <[email protected]> Gerrit-Reviewer: Roman Mohr <[email protected]> Gerrit-Reviewer: Roy Golan <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
