On 22.01.2015 15:52, Ján Tomko wrote:
> 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.
Okay, ACK then.
Michal
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list