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

[Pradipta Kumar Banerjee <bpradip@xxxxxxxxxx>]

ping.


On 12/19/2013 10:10 AM, Pradipta Kr. Banerjee wrote:
> From: "Pradipta Kr. Banerjee" <bpradip@xxxxxxxxxx>
> 
> virsh nodecpustats erroneously returns stats for offline cpus on Linux
> 
> To retrieve node cpu statistics on Linux system, the
> linuxNodeGetCPUstats function performs a minimal match of the input
> cpuid with the cpuid read from /proc/cpustat. On systems with larger
> cpus this can result in erroneous data.
> For eg if /proc/stat has similar data
>  cpu  3645648 485 361475 3691194868 176595 40645 69850 0 1526172 0
>  cpu0 494045 62 42763 138516619 16107 2339 2431 0 248345 0
>  cpu4 402646 59 34745 138617562 17738 1413 1196 0 193948 0
>  cpu8 318662 32 32119 138715131 7903 927 982 0 115429 0
>  cpu12 247004 33 30465 138791438 5251 759 913 0 51938 0
> 
> and cpu 1,2 are offline, then
> 
> 'virsh nodecpustats 1' displays data for cpu12
> 
> whereas virsh nodecpustats 2 correctly throws the following error
> 
> "error: Unable to get node cpu stats
> error: Invalid cpuNum in linuxNodeGetCPUStats"
> 
> This patch fixes the above mentioned problem
> 
> Signed-off-by: Pradipta Kr. Banerjee <bpradip@xxxxxxxxxx>
> ---
>  src/nodeinfo.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/src/nodeinfo.c b/src/nodeinfo.c
> index 1838547..c9e5ff1 100644
> --- a/src/nodeinfo.c
> +++ b/src/nodeinfo.c
> @@ -614,6 +614,8 @@ linuxNodeGetCPUStats(FILE *procstat,
>      unsigned long long usr, ni, sys, idle, iowait;
>      unsigned long long irq, softirq, steal, guest, guest_nice;
>      char cpu_header[3 + INT_BUFSIZE_BOUND(cpuNum)];
> +    char cpu_header_read[3 + INT_BUFSIZE_BOUND(cpuNum)];
> +    char cpu_header_fmt[8];
> 
>      if ((*nparams) == 0) {
>          /* Current number of cpu stats supported by linux */
> @@ -631,8 +633,11 @@ linuxNodeGetCPUStats(FILE *procstat,
> 
>      if (cpuNum == VIR_NODE_CPU_STATS_ALL_CPUS) {
>          strcpy(cpu_header, "cpu");
> +        snprintf(cpu_header_fmt, sizeof(cpu_header_fmt), "%%%ds", 3);
>      } else {
>          snprintf(cpu_header, sizeof(cpu_header), "cpu%d", cpuNum);
> +        snprintf(cpu_header_fmt, sizeof(cpu_header_fmt), "%%%ds",
> +                 (int)(3 + INT_BUFSIZE_BOUND(cpuNum)));
>      }
> 
>      while (fgets(line, sizeof(line), procstat) != NULL) {
> @@ -649,6 +654,14 @@ linuxNodeGetCPUStats(FILE *procstat,
>                  continue;
>              }
> 
> +            /*
> +             * Process with stats gathering only if the cpuid from /proc/stat
> +             * matches exactly with the input cpuid
> +            */
> +            sscanf(buf, cpu_header_fmt, cpu_header_read);
> +            if (STRNEQ(cpu_header, cpu_header_read))
> +                continue;
> +
>              for (i = 0; i < *nparams; i++) {
>                  virNodeCPUStatsPtr param = &params[i];
> 
> --
> 1.8.3.1
> 


-- 
Regards,
Pradipta

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list