On 12/19/2013 02:02 PM, Bing Bu Cao wrote: > 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 = ¶ms[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.. Thanks > >> >> -- >> 1.8.3.1 >> >> -- >> libvir-list mailing list >> libvir-list@xxxxxxxxxx >> https://www.redhat.com/mailman/listinfo/libvir-list >> >> >> > -- Regards, Pradipta -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list