On 01/22/2015 03:21 PM, Michal Privoznik wrote:
> On 22.01.2015 12:26, Ján Tomko wrote:
>> Per-cpu stats are only shown for present CPUs in the cgroups,
>> but we were only parsing the largest CPU number from
>> /sys/devices/system/cpu/present and looking for stats even for
>> non-present CPUs.
>> This resulted in:
>> internal error: cpuacct parse error
>> ---
>> cfg.mk | 2 +-
>> src/nodeinfo.c | 17 +++++++++++++++++
>> src/nodeinfo.h | 1 +
>> src/util/vircgroup.c | 24 ++++++++++++++++++------
>> tests/vircgroupmock.c | 44 ++++++++++++++++++++++++++++++++++++++------
>> tests/vircgrouptest.c | 47 +++++++++++++++++++++++++++++++++++------------
>> 6 files changed, 110 insertions(+), 25 deletions(-)
>>
>> diff --git a/cfg.mk b/cfg.mk
>> index 21f83c3..70612f8 100644
>> --- a/cfg.mk
>> +++ b/cfg.mk
>> @@ -1095,7 +1095,7 @@ exclude_file_name_regexp--sc_prohibit_asprintf = \
>> ^(bootstrap.conf$$|src/util/virstring\.[ch]$$|tests/vircgroupmock\.c$$)
>>
>> exclude_file_name_regexp--sc_prohibit_strdup = \
>> - ^(docs/|examples/|src/util/virstring\.c|tests/virnetserverclientmock.c$$)
>> + ^(docs/|examples/|src/util/virstring\.c|tests/vir(netserverclient|cgroup)mock.c$$)
>>
>> exclude_file_name_regexp--sc_prohibit_close = \
>> (\.p[yl]$$|\.spec\.in$$|^docs/|^(src/util/virfile\.c|src/libvirt-stream\.c|tests/vir(cgroup|pci)mock\.c)$$)
>> diff --git a/src/nodeinfo.c b/src/nodeinfo.c
>> index 3c22ebc..3a27c22 100644
>> --- a/src/nodeinfo.c
>> +++ b/src/nodeinfo.c
>> @@ -1242,6 +1242,23 @@ nodeGetCPUCount(void)
>> }
>>
>> virBitmapPtr
>> +nodeGetPresentCPUBitmap(void)
>> +{
>> + int max_present;
>> +
>> + if ((max_present = nodeGetCPUCount()) < 0)
>> + return NULL;
>> +
>> +#ifdef __linux__
>> + if (virFileExists(SYSFS_SYSTEM_PATH "/cpu/present"))
>> + return linuxParseCPUmap(max_present, SYSFS_SYSTEM_PATH "/cpu/present");
>> +#endif
>> + virReportError(VIR_ERR_NO_SUPPORT, "%s",
>> + _("non-continuous host cpu numbers not implemented on this platform"));
>> + return NULL;
>> +}
>> +
>> +virBitmapPtr
>> nodeGetCPUBitmap(int *max_id ATTRIBUTE_UNUSED)
>> {
>
>>From my understanding the nodeGetPresentCPUBitmap() is no different than
> nodeGetCPUBitmap(). The latter can do everything that the former and
> more. Can we just use already existing function then?
>
This functions checks the present CPUs, nodeGetCPUBitmap checks the online CPUs.
I don't think splitting their common part into another function would be more
readable - IMO the fact that both use linuxParseCPUmap should be enough.
Jan
> Otherwise looking good.
>
> Michal
>
> --
> libvir-list mailing list
> libvir-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list
>
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list