Re: [PATCH v2 2/2] virsh: report only filled values in 'nodecpustats'

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

 



  Roman Bogorodskiy wrote:

> A set of fields for CPU stats could vary on different platforms,
> for example, FreeBSD doesn't report 'iowait'.
> 
> Make virsh print out only the fields that were actually filled.
> ---
>  tools/virsh-host.c | 118 ++++++++++++++++++++++++++++++++---------------------
>  1 file changed, 71 insertions(+), 47 deletions(-)
> 
> diff --git a/tools/virsh-host.c b/tools/virsh-host.c
> index ac41177..8c1218e 100644
> --- a/tools/virsh-host.c
> +++ b/tools/virsh-host.c
> @@ -34,6 +34,7 @@
>  #include "internal.h"
>  #include "virbuffer.h"
>  #include "viralloc.h"
> +#include "virhash.h"
>  #include "virsh-domain.h"
>  #include "virxml.h"
>  #include "virtypedparam.h"
> @@ -335,23 +336,15 @@ static const vshCmdOptDef opts_node_cpustats[] = {
>  static bool
>  cmdNodeCpuStats(vshControl *ctl, const vshCmd *cmd)
>  {
> -    size_t i, j;
> +    size_t i, j, k;
>      bool flag_utilization = false;
>      bool flag_percent = vshCommandOptBool(cmd, "percent");
>      int cpuNum = VIR_NODE_CPU_STATS_ALL_CPUS;
>      virNodeCPUStatsPtr params;
>      int nparams = 0;
>      bool ret = false;
> -    struct cpu_stats {
> -        unsigned long long user;
> -        unsigned long long sys;
> -        unsigned long long idle;
> -        unsigned long long iowait;
> -        unsigned long long intr;
> -        unsigned long long util;
> -    } cpu_stats[2];
> -    double user_time, sys_time, idle_time, iowait_time, intr_time, total_time;
> -    double usage;
> +    const char *fields[] = {"user:", "system:", "idle:", "iowait:", "intr:", NULL};
> +    virHashTablePtr cpu_stats[2];
>  
>      if (vshCommandOptInt(cmd, "cpu", &cpuNum) < 0) {
>          vshError(ctl, "%s", _("Invalid value of cpuNum"));
> @@ -372,6 +365,10 @@ cmdNodeCpuStats(vshControl *ctl, const vshCmd *cmd)
>      params = vshCalloc(ctl, nparams, sizeof(*params));
>  
>      for (i = 0; i < 2; i++) {
> +        cpu_stats[i] = virHashCreate(0, (virHashDataFree) free);
> +        if (cpu_stats[i] == NULL)
> +            goto cleanup;
> +
>          if (i > 0)
>              sleep(1);
>  
> @@ -381,20 +378,25 @@ cmdNodeCpuStats(vshControl *ctl, const vshCmd *cmd)
>          }
>  
>          for (j = 0; j < nparams; j++) {
> -            unsigned long long value = params[j].value;
> +            unsigned long long *value;
> +
> +            if (VIR_ALLOC(value) < 0)
> +                goto cleanup;
> +
> +            *value = params[j].value;
>  
>              if (STREQ(params[j].field, VIR_NODE_CPU_STATS_KERNEL)) {
> -                cpu_stats[i].sys = value;
> +                virHashAddEntry(cpu_stats[i], "system:", value);
>              } else if (STREQ(params[j].field, VIR_NODE_CPU_STATS_USER)) {
> -                cpu_stats[i].user = value;
> +                virHashAddEntry(cpu_stats[i], "user:", value);
>              } else if (STREQ(params[j].field, VIR_NODE_CPU_STATS_IDLE)) {
> -                cpu_stats[i].idle = value;
> +                virHashAddEntry(cpu_stats[i], "idle:", value);
>              } else if (STREQ(params[j].field, VIR_NODE_CPU_STATS_IOWAIT)) {
> -                cpu_stats[i].iowait = value;
> +                virHashAddEntry(cpu_stats[i], "iowait:", value);
>              } else if (STREQ(params[j].field, VIR_NODE_CPU_STATS_INTR)) {
> -                cpu_stats[i].intr = value;
> +                virHashAddEntry(cpu_stats[i], "intr:", value);
>              } else if (STREQ(params[j].field, VIR_NODE_CPU_STATS_UTILIZATION)) {
> -                cpu_stats[i].util = value;
> +                virHashAddEntry(cpu_stats[i], "usage:", value);
>                  flag_utilization = true;
>              }
>          }
> @@ -405,46 +407,68 @@ cmdNodeCpuStats(vshControl *ctl, const vshCmd *cmd)
>  
>      if (!flag_percent) {
>          if (!flag_utilization) {
> -            vshPrint(ctl, "%-15s %20llu\n", _("user:"), cpu_stats[0].user);
> -            vshPrint(ctl, "%-15s %20llu\n", _("system:"), cpu_stats[0].sys);
> -            vshPrint(ctl, "%-15s %20llu\n", _("idle:"), cpu_stats[0].idle);
> -            vshPrint(ctl, "%-15s %20llu\n", _("iowait:"), cpu_stats[0].iowait);
> -            vshPrint(ctl, "%-15s %20llu\n", _("intr:"), cpu_stats[0].intr);
> +            for (k = 0; fields[k] != NULL; k++) {
> +                unsigned long long *value = virHashLookup(cpu_stats[0], fields[k]);
> +
> +                if (value)
> +                    vshPrint(ctl, "%-15s %20llu\n", _(fields[k]), *value);
> +            }
>          }
>      } else {
>          if (flag_utilization) {
> -            usage = cpu_stats[0].util;
> +            double *usage = virHashLookup(cpu_stats[0], "usage:");
>  
> -            vshPrint(ctl, "%-15s %5.1lf%%\n", _("usage:"), usage);
> -            vshPrint(ctl, "%-15s %5.1lf%%\n", _("idle:"), 100 - usage);
> +            vshPrint(ctl, "%-15s %5.1lf%%\n", _("usage:"), *usage);
> +            vshPrint(ctl, "%-15s %5.1lf%%\n", _("idle:"), 100 - *usage);
>          } else {
> -            user_time   = cpu_stats[1].user   - cpu_stats[0].user;
> -            sys_time    = cpu_stats[1].sys    - cpu_stats[0].sys;
> -            idle_time   = cpu_stats[1].idle   - cpu_stats[0].idle;
> -            iowait_time = cpu_stats[1].iowait - cpu_stats[0].iowait;
> -            intr_time   = cpu_stats[1].intr   - cpu_stats[0].intr;
> -            total_time  = user_time + sys_time + idle_time + iowait_time + intr_time;
> -
> -            usage = (user_time + sys_time) / total_time * 100;
> -
> -            vshPrint(ctl, "%-15s %5.1lf%%\n",
> -                     _("usage:"), usage);
> -            vshPrint(ctl, "%-15s %5.1lf%%\n",
> -                     _("user:"), user_time / total_time * 100);
> -            vshPrint(ctl, "%-15s %5.1lf%%\n",
> -                     _("system:"), sys_time  / total_time * 100);
> -            vshPrint(ctl, "%-15s %5.1lf%%\n",
> -                     _("idle:"), idle_time     / total_time * 100);
> -            vshPrint(ctl, "%-15s %5.1lf%%\n",
> -                     _("iowait:"), iowait_time   / total_time * 100);
> -            vshPrint(ctl, "%-15s %5.1lf%%\n",
> -                     _("intr:"), intr_time         / total_time * 100);
> +            virHashTablePtr diff;
> +            double total_time = 0;
> +            double usage = -1;
> +
> +            diff = virHashCreate(0, (virHashDataFree) free);
> +            if (diff == NULL)
> +                goto cleanup;
> +
> +            for (k = 0; fields[k] != NULL; k++) {
> +                double *value_diff;
> +                unsigned long long *oldval = virHashLookup(cpu_stats[0], fields[k]);
> +                unsigned long long *newval = virHashLookup(cpu_stats[1], fields[k]);
> +
> +                if (!oldval || !newval)
> +                    continue;
> +
> +                if (VIR_ALLOC(value_diff) < 0)
> +                    goto cleanup;
> +
> +                *value_diff = *newval - *oldval;
> +                virHashAddEntry(diff, fields[k], value_diff);
> +                total_time += *value_diff;
> +            }
> +
> +            for (k = 0; fields[k] != NULL; k++) {
> +                double *value = virHashLookup(diff, fields[k]);
> +
> +                if (!value)
> +                    continue;
> +
> +                vshPrint(ctl, "%-15s %5.1lf%%\n",
> +                         _(fields[k]), *value / total_time * 100);
> +
> +                if (STREQ("idle:", fields[k]))
> +                    usage = (total_time - *value) / total_time * 100;
> +            }
> +
> +            if (usage != -1)
> +                vshPrint(ctl, "%-15s %5.1lf%%\n",
> +                         _("usage:"), usage);
This needs 
           virHashFree(diff);

I'll wait for other comments before updating the patch.
>          }
>      }
>  
>      ret = true;
>  
>    cleanup:
> +    for (i = 0; i < 2; i++)
> +        virHashFree(cpu_stats[i]);
>      VIR_FREE(params);
>      return ret;
>  }
> -- 
> 1.8.4.3
> 

Roman Bogorodskiy

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