Francesco Romani has posted comments on this change. Change subject: host stats: Add onlineCpus to vdsStats ......................................................................
Patch Set 4: Code-Review-1 (5 comments) mostly OK, a few changes needed. Still not sure about the best Engine API. https://gerrit.ovirt.org/#/c/46270/4/tests/samplingTests.py File tests/samplingTests.py: Line 265: 'onlineCpus': '0,2' Line 266: } Line 267: Line 268: def get(): Line 269: class LibvirtConnection: Please move into a separate test, I don't want to overload this until we move to py3, for consistency: class LibvirtConnection(object): you can also use a free function (a bit hackish, but works) Line 270: def getCPUMap(self): Line 271: return (4, [True, False, True, False], 2) Line 272: return LibvirtConnection() Line 273: https://gerrit.ovirt.org/#/c/46270/4/vdsm/rpc/vdsmapi-schema.json File vdsm/rpc/vdsmapi-schema.json: Line 2072: # the KSM feature (in bytes) Line 2073: # Line 2074: # @memCommitted: Amount of memory committed to running VMs (in MB) Line 2075: # Line 2076: # @onlineCpus: The list of online logical CPUs I was wondering if it is better to report the list of online or offline cpu. Maybe it's just me, but I'd expect that most of time len(online) >= len(offline). Line 2077: # (new in version 4.17.2) Line 2078: # Line 2079: # @swapTotal: The total amount of swap space (in MB) Line 2080: # Line 2073: # Line 2074: # @memCommitted: Amount of memory committed to running VMs (in MB) Line 2075: # Line 2076: # @onlineCpus: The list of online logical CPUs Line 2077: # (new in version 4.17.2) Do you need to have this in 3.6.x? last released version is 4.17.7 Line 2078: # Line 2079: # @swapTotal: The total amount of swap space (in MB) Line 2080: # Line 2081: # @swapFree: The amount of free swap space remaining (in MB) https://gerrit.ovirt.org/#/c/46270/4/vdsm/virt/sampling.py File vdsm/virt/sampling.py: Line 685: The result is a list containing the indices of all online cpu cores. Line 686: Line 687: :return list like ['1', '2', '5', '6', '7', '8'] Line 688: """ Line 689: (_, cpu_register, _) = libvirtconnection.get().getCPUMap() are the parenthesis needed? why not just _, cpu_register, _ = ... Line 690: cpu_map = [] Line 691: for index, online in enumerate(cpu_register): Line 692: if online: Line 693: cpu_map.append(str(index)) Line 690: cpu_map = [] Line 691: for index, online in enumerate(cpu_register): Line 692: if online: Line 693: cpu_map.append(str(index)) Line 694: return cpu_map what about return [str(index) for index, online in enumerate(cpu_register) if online] ? Line 695: Line 696: Line 697: def _get_cpu_core_stats(first_sample, last_sample): Line 698: interval = last_sample.timestamp - first_sample.timestamp -- To view, visit https://gerrit.ovirt.org/46270 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I73c92f563c7c3e8ffbbcf639eba27fbe55389994 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Roman Mohr <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Jenkins CI 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
