Re: [libvirt] [PATCH 06/12] [v7] virNodeGetCPUStats: Implement linux support

On Tue, Jun 07, 2011 at 10:02:55AM +0900, Minoru Usui wrote: > virNodeGetCPUStats: Implement linux support > > Signed-off-by: Minoru Usui <usui(a)mxm.nes.nec.co.jp&gt; > --- > src/libvirt_private.syms | 1 + > src/lxc/lxc_driver.c | 1 + > src/nodeinfo.c | 138 ++++++++++++++++++++++++++++++++++++++++++++++ > src/nodeinfo.h | 6 ++- > src/qemu/qemu_driver.c | 1 + > src/uml/uml_driver.c | 1 + > 6 files changed, 147 insertions(+), 1 deletions(-) > > diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms > index e6ab870..a8c77f2 100644 > --- a/src/libvirt_private.syms > +++ b/src/libvirt_private.syms > @@ -731,6 +731,7 @@ virNodeDeviceObjUnlock; > > # nodeinfo.h > nodeCapsInitNUMA; > +nodeGetCPUStats; > nodeGetCellsFreeMemory; > nodeGetFreeMemory; > nodeGetInfo; > diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c > index 8eb87a2..3286154 100644 > --- a/src/lxc/lxc_driver.c > +++ b/src/lxc/lxc_driver.c > @@ -2779,6 +2779,7 @@ static virDriver lxcDriver = { > .domainSetSchedulerParameters = lxcSetSchedulerParameters, /* 0.5.0 */ > .domainSetSchedulerParametersFlags = lxcSetSchedulerParametersFlags, /* 0.9.2 */ > .domainInterfaceStats = lxcDomainInterfaceStats, /* 0.7.3 */ > + .nodeGetCPUStats = nodeGetCPUStats, /* 0.9.3 */ > .nodeGetCellsFreeMemory = nodeGetCellsFreeMemory, /* 0.6.5 */ > .nodeGetFreeMemory = nodeGetFreeMemory, /* 0.6.5 */ > .domainEventRegister = lxcDomainEventRegister, /* 0.7.0 */ > diff --git a/src/nodeinfo.c b/src/nodeinfo.c > index f55c83e..39afd0e 100644 > --- a/src/nodeinfo.c > +++ b/src/nodeinfo.c > @@ -57,12 +57,20 @@ > #ifdef __linux__ > # define CPUINFO_PATH "/proc/cpuinfo" > # define CPU_SYS_PATH "/sys/devices/system/cpu" > +# define PROCSTAT_PATH "/proc/stat" > + > +# define LINUX_NB_CPU_STATS 4 > > /* NB, this is not static as we need to call it from the testsuite */ > int linuxNodeInfoCPUPopulate(FILE *cpuinfo, > virNodeInfoPtr nodeinfo, > bool need_hyperthreads); > > +int linuxNodeGetCPUStats(FILE *procstat, > + int cpuNum, > + virCPUStatsPtr params, > + int *nparams); > + > /* Return the positive decimal contents of the given > * CPU_SYS_PATH/cpu%u/FILE, or -1 on error. If MISSING_OK and the > * file could not be found, return 1 instead of an error; this is > @@ -378,6 +386,108 @@ int linuxNodeInfoCPUPopulate(FILE *cpuinfo, > return 0; > } > > +#define TICK_TO_NSEC (1000ull * 1000ull * 1000ull / sysconf(_SC_CLK_TCK)) > +#define CPU_HEADER_LEN 8 > + > +int linuxNodeGetCPUStats(FILE *procstat, > + int cpuNum, > + virCPUStatsPtr params, > + int *nparams) > +{ > + int ret = -1; > + char line[1024]; > + unsigned long long usr, ni, sys, idle, iowait; > + unsigned long long irq, softirq, steal, guest, guest_nice; > + char cpu_header[CPU_HEADER_LEN]; > + > + if ((*nparams) == 0) { > + /* Current number of cpu stats supported by linux */ > + *nparams = LINUX_NB_CPU_STATS; > + ret = 0; > + goto cleanup; > + } > + > + if ((*nparams) != LINUX_NB_CPU_STATS) { > + nodeReportError(VIR_ERR_INVALID_ARG, > + "%s", _("Invalid parameter count")); > + goto cleanup; > + } > + > + if (cpuNum == VIR_CPU_STATS_ALL_CPUS) { > + strcpy(cpu_header, "cpu"); > + } else { > + sprintf(cpu_header, "cpu%d", cpuNum); > + } cpu_header is declared to be 8 bytes in size, which only allows for integers upto 4 digits long, before we get a buffer overflow here. gnulib has some macro which can be used to declare a string buffer large enough to hold an integer, but I can't remember what the macro is called right now. Hopefully Eric will remind us shortly....