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

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

 



On 12/19/2013 12:40 PM, 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];

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,




--
1.8.3.1

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




--
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]

  Powered by Linux