[libvirt] [PATCH 1/1] virsh nodecpustats erroneously returns stats for offline cpus on Linux

[Bing Bu Cao]

On 12/20/2013 05:47 PM, Pradipta Kumar Banerjee wrote:
> [snip]
>>>>                for (i = 0; i < *nparams; i++) {
>>>>                    virNodeCPUStatsPtr param = &params[i];
>>>
>>> What about this?
>>>
>>> diff --git a/src/nodeinfo.c b/src/nodeinfo.c
>>> index 1838547..aa1ad81 100644
>>> --- a/src/nodeinfo.c
>>> +++ b/src/nodeinfo.c
>>> @@ -637,8 +637,9 @@ linuxNodeGetCPUStats(FILE *procstat,
>>>
>>>        while (fgets(line, sizeof(line), procstat) != NULL) {
>>>            char *buf = line;
>>> +        char **buf_header = virStringSplit(buf, " ", 2);
>>>
>>> -        if (STRPREFIX(buf, cpu_header)) { /* aka logical CPU time */
>>> +        if (STREQ(buf_header[0], cpu_header)) { /* aka logical CPU time */
>>>                size_t i;
>>>
>>>                if (sscanf(buf,
>>> @@ -697,6 +698,7 @@ linuxNodeGetCPUStats(FILE *procstat,
>>>                ret = 0;
>>>                goto cleanup;
>>>            }
>>> +        virStringFreeList(buf_header);
>>>        }
>>>
>>>        virReportInvalidArg(cpuNum,
>>>
>>>
>> This is definitely better and lesser amount of code..
>
> I think the version with virStringSplit would need some fine tuning since in its
> current form it will not free the memory for the failure cases..Comments ?
Good point.
We should add virStringFreeList(buf_header) to cleanup.
>
> Also can some expert here provide some tips on whether in this particular
> circumstance it should be fine to allocate/realloc/free memory inside the while
> loop.  Would be very helpful..
I think putting allocate/free memory inside the while is OK for me.
Because we must loop for searching the expected line,the times of loop 
depends on the content of /proc/stat.
However, as you said, we better get some advise from the experts here.
>
>
> Thanks,
> Pradipta
>
>>
>> Thanks
>>>
>>>>
>>>> --
>>>> 1.8.3.1
>>>>
>>>> --
>>>> libvir-list mailing list
>>>> libvir-list at redhat.com
>>>> https://www.redhat.com/mailman/listinfo/libvir-list
>>>>
>>>>
>>>>
>>>
>>
>>
>
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>


-- 
Best Regards,
Bing Bu Cao