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