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

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

> +
> +    while (fgets(line, sizeof(line), procstat) != NULL) {
> +        char *buf = line;
> +
> +        if (STRPREFIX(buf, cpu_header)) { /* aka logical CPU time */
> +            int i;
> +
> +            if (sscanf(buf,
> +                       "%*s %llu %llu %llu %llu %llu" // user ~ iowait
> +                       "%llu %llu %llu %llu %llu",    // irq  ~ guest_nice
> +                       &usr, &ni, &sys, &idle, &iowait,
> +                       &irq, &softirq, &steal, &guest, &guest_nice) < 4) {
> +                continue;
> +            }
> +
> +            for (i = 0; i < *nparams; i++) {
> +                virCPUStatsPtr param = &params[i];
> +
> +                switch (i) {
> +                case 0: /* fill kernel cpu time here */
> +                    if (virStrcpyStatic(param->field, VIR_CPU_STATS_KERNEL)== NULL) {
> +                        nodeReportError(VIR_ERR_INTERNAL_ERROR,
> +                                        "%s", _("Field kernel cpu time too long for destination"));
> +                        goto cleanup;
> +                    }
> +                    param->value = (sys + irq + softirq) * TICK_TO_NSEC;
> +                    break;
> +
> +                case 1: /* fill user cpu time here */
> +                    if (virStrcpyStatic(param->field, VIR_CPU_STATS_USER) == NULL) {
> +                        nodeReportError(VIR_ERR_INTERNAL_ERROR,
> +                                        "%s", _("Field kernel cpu time too long for destination"));
> +                        goto cleanup;
> +                    }
> +                    param->value = (usr + ni) * TICK_TO_NSEC;
> +                    break;
> +
> +                case 2: /* fill idle cpu time here */
> +                    if (virStrcpyStatic(param->field, VIR_CPU_STATS_IDLE) == NULL) {
> +                        nodeReportError(VIR_ERR_INTERNAL_ERROR,
> +                                        "%s", _("Field kernel cpu time too long for destination"));
> +                        goto cleanup;
> +                    }
> +                    param->value = idle * TICK_TO_NSEC;
> +                    break;
> +
> +                case 3: /* fill iowait cpu time here */
> +                    if (virStrcpyStatic(param->field, VIR_CPU_STATS_IOWAIT) == NULL) {
> +                        nodeReportError(VIR_ERR_INTERNAL_ERROR,
> +                                        "%s", _("Field kernel cpu time too long for destination"));
> +                        goto cleanup;
> +                    }
> +                    param->value = iowait * TICK_TO_NSEC;
> +                    break;
> +
> +                default:
> +                    break;
> +                    /* should not hit here */
> +                }
> +            }
> +            ret = 0;
> +            goto cleanup;
> +        }
> +    }
> +
> +    nodeReportError(VIR_ERR_INVALID_ARG, "%s", _("Invalid cpu number"));
> +
> +cleanup:
> +    return ret;
> +}
>  #endif
>  
>  int nodeGetInfo(virConnectPtr conn ATTRIBUTE_UNUSED, virNodeInfoPtr nodeinfo) {
> @@ -416,6 +526,34 @@ int nodeGetInfo(virConnectPtr conn ATTRIBUTE_UNUSED, virNodeInfoPtr nodeinfo) {
>  #endif
>  }
>  
> +int nodeGetCPUStats(virConnectPtr conn ATTRIBUTE_UNUSED,
> +                    int cpuNum,
> +                    virCPUStatsPtr params,
> +                    int *nparams,
> +                    unsigned int flags ATTRIBUTE_UNUSED)
> +{
> +
> +#ifdef __linux__
> +    {
> +    int ret;
> +    FILE *procstat = fopen(PROCSTAT_PATH, "r");
> +    if (!procstat) {
> +        virReportSystemError(errno,
> +                             _("cannot open %s"), PROCSTAT_PATH);
> +        return -1;
> +    }
> +    ret = linuxNodeGetCPUStats(procstat, cpuNum, params, nparams);
> +    VIR_FORCE_FCLOSE(procstat);
> +
> +    return ret;
> +    }
> +#else
> +    nodeReportError(VIR_ERR_NO_SUPPORT, "%s",
> +                    _("node CPU stats not implemented on this platform"));
> +    return -1;
> +#endif
> +}
> +
>  #if HAVE_NUMACTL
>  # if LIBNUMA_API_VERSION <= 1
>  #  define NUMA_MAX_N_CPUS 4096
> diff --git a/src/nodeinfo.h b/src/nodeinfo.h
> index 88bac6c..361e3e5 100644
> --- a/src/nodeinfo.h
> +++ b/src/nodeinfo.h
> @@ -30,7 +30,11 @@
>  int nodeGetInfo(virConnectPtr conn, virNodeInfoPtr nodeinfo);
>  int nodeCapsInitNUMA(virCapsPtr caps);
>  
> -
> +int nodeGetCPUStats(virConnectPtr conn ATTRIBUTE_UNUSED,
> +                    int cpuNum,
> +                    virCPUStatsPtr params,
> +                    int *nparams,
> +                    unsigned int flags ATTRIBUTE_UNUSED);
>  int nodeGetCellsFreeMemory(virConnectPtr conn,
>                             unsigned long long *freeMems,
>                             int startCell,
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 5632d62..108a37f 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -8044,6 +8044,7 @@ static virDriver qemuDriver = {
>      .domainBlockPeek = qemudDomainBlockPeek, /* 0.4.4 */
>      .domainMemoryPeek = qemudDomainMemoryPeek, /* 0.4.4 */
>      .domainGetBlockInfo = qemuDomainGetBlockInfo, /* 0.8.1 */
> +    .nodeGetCPUStats = nodeGetCPUStats, /* 0.9.3 */
>      .nodeGetCellsFreeMemory = nodeGetCellsFreeMemory, /* 0.4.4 */
>      .nodeGetFreeMemory = nodeGetFreeMemory, /* 0.4.4 */
>      .domainEventRegister = qemuDomainEventRegister, /* 0.5.0 */
> diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c
> index 536cd8c..471277d 100644
> --- a/src/uml/uml_driver.c
> +++ b/src/uml/uml_driver.c
> @@ -2218,6 +2218,7 @@ static virDriver umlDriver = {
>      .domainGetAutostart = umlDomainGetAutostart, /* 0.5.0 */
>      .domainSetAutostart = umlDomainSetAutostart, /* 0.5.0 */
>      .domainBlockPeek = umlDomainBlockPeek, /* 0.5.0 */
> +    .nodeGetCPUStats = nodeGetCPUStats, /* 0.9.3 */
>      .nodeGetCellsFreeMemory = nodeGetCellsFreeMemory, /* 0.5.0 */
>      .nodeGetFreeMemory = nodeGetFreeMemory, /* 0.5.0 */
>      .isEncrypted = umlIsEncrypted, /* 0.7.3 */

ACK if the buffer overflow problem is solved

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


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