On 01/19/2014 06:55 PM, 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};
Maybe creating an enum with VIR_ENUM_DECL/VIR_ENUM_IMPL would make the code easier to read? (You could use the FromString/ToString functions and an array of bools to tell missing/zero fields apart instead of NULL pointers)
+ virHashTablePtr cpu_stats[2];
This should be initialized to NULLs.
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)) {
Hmm, did libvirtd ever fill out the 'usage' field? I can't find it in git history.
- cpu_stats[i].util = value; + virHashAddEntry(cpu_stats[i], "usage:", value); flag_utilization = true; } } @@ -405,46 +407,68 @@ cmdNodeCpuStats(vshControl *ctl, const vshCmd *cmd)
+ virHashTablePtr diff; This should be declared at the start of the function, so you can free it in the cleanup label.
+ 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]); +
Jan