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
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list