Re: [PATCH] Fix virCgroupGetPercpuStats with non-continuous present CPUs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]