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

[Daniel P. Berrange]

On Tue, Jun 14, 2011 at 10:15:36AM +0100, Daniel P. Berrange wrote:
> On Tue, Jun 07, 2011 at 10:02:55AM +0900, Minoru Usui wrote:
> > virNodeGetCPUStats: Implement linux support
> > 
> > Signed-off-by: Minoru Usui <usui at mxm.nes.nec.co.jp>
> > ---
> >  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....

Jim Meyering reminded me that it is INT_BUFSIZE_BOUND. So eg you
want todo

   char cpu_header[3 + INT_BUFSIZE_BOUND(cpuNum) + 1];

to allow enough space for 'cpu' + any cpuNum value formatted as
a string + the trailing NULL.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|