[libvirt] [PATCH v2 0/2] BSD: implement nodeGetCPUStats
Updated patch which includes virsh update to not output unsupported CPU stat fields. For example, Linux output: user: 1.0% system: 1.0% idle: 98.0% iowait: 0.0% usage: 2.0% Includes iowait, doesn't have intr. FreeBSD output: user: 0,8% system: 0,9% idle: 97,5% intr: 0,8% usage: 2,5% Includes intr, doesn't have iowait. Roman Bogorodskiy (2): BSD: implement nodeGetCPUStats virsh: report only filled values in 'nodecpustats' include/libvirt/libvirt.h.in | 8 +++ src/nodeinfo.c | 104 +++++++++++++++++++++++++++++++++++++++ tools/virsh-host.c | 113 +++++++++++++++++++++++++++---------------- 3 files changed, 184 insertions(+), 41 deletions(-) -- 1.8.4.3
Implementation obtains CPU usage information using kern.cp_time and kern.cp_times sysctl(8)s and reports CPU utilization. --- include/libvirt/libvirt.h.in | 8 ++++ src/nodeinfo.c | 104 +++++++++++++++++++++++++++++++++++++++++++ tools/virsh-host.c | 11 ++++- 3 files changed, 121 insertions(+), 2 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 5bdb2bc..e8cdbbf 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -692,6 +692,14 @@ typedef enum { #define VIR_NODE_CPU_STATS_IOWAIT "iowait" /** + * VIR_NODE_CPU_STATS_INTR: + * + * The cumulative interrupt CPU time, + * since the node booting up (in nanoseconds). + */ +#define VIR_NODE_CPU_STATS_INTR "intr" + +/** * VIR_NODE_CPU_STATS_UTILIZATION: * * The CPU utilization of a node. diff --git a/src/nodeinfo.c b/src/nodeinfo.c index 05bc038..fd2f8c8 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -34,8 +34,10 @@ #include "conf/domain_conf.h" #if defined(__FreeBSD__) || defined(__APPLE__) +# include <sys/time.h> # include <sys/types.h> # include <sys/sysctl.h> +# include <sys/resource.h> #endif #include "c-ctype.h" @@ -99,8 +101,108 @@ appleFreebsdNodeGetMemorySize(unsigned long *memory) #endif /* defined(__FreeBSD__) || defined(__APPLE__) */ #ifdef __FreeBSD__ +# define BSD_CPU_STATS_ALL 4 # define BSD_MEMORY_STATS_ALL 4 +# define TICK_TO_NSEC (1000ull * 1000ull * 1000ull / (stathz ? stathz : hz)) + +static int +freebsdNodeGetCPUStats(int cpuNum, + virNodeCPUStatsPtr params, + int *nparams) +{ + const char *sysctl_name; + long *cpu_times; + struct clockinfo clkinfo; + size_t i, j, cpu_times_size, clkinfo_size; + int cpu_times_num, offset, hz, stathz, ret = -1; + struct field_cpu_map { + const char *field; + int idx[CPUSTATES]; + } cpu_map[] = { + {VIR_NODE_CPU_STATS_KERNEL, {CP_SYS}}, + {VIR_NODE_CPU_STATS_USER, {CP_USER, CP_NICE}}, + {VIR_NODE_CPU_STATS_IDLE, {CP_IDLE}}, + {VIR_NODE_CPU_STATS_INTR, {CP_INTR}}, + {NULL, {0}} + }; + + if ((*nparams) == 0) { + *nparams = BSD_CPU_STATS_ALL; + return 0; + } + + if ((*nparams) != BSD_CPU_STATS_ALL) { + virReportInvalidArg(*nparams, + _("nparams in %s must be equal to %d"), + __FUNCTION__, BSD_CPU_STATS_ALL); + return -1; + } + + clkinfo_size = sizeof(clkinfo); + if (sysctlbyname("kern.clockrate", &clkinfo, &clkinfo_size, NULL, 0) < 0) { + virReportSystemError(errno, + _("sysctl failed for '%s'"), + "kern.clockrate"); + return -1; + } + + stathz = clkinfo.stathz; + hz = clkinfo.hz; + + if (cpuNum == VIR_NODE_CPU_STATS_ALL_CPUS) { + sysctl_name = "kern.cp_time"; + cpu_times_num = 1; + offset = 0; + } else { + sysctl_name = "kern.cp_times"; + cpu_times_num = appleFreebsdNodeGetCPUCount(); + + if (cpuNum >= cpu_times_num) { + virReportInvalidArg(cpuNum, + _("Invalid cpuNum in %s"), + __FUNCTION__); + return -1; + } + + offset = cpu_times_num * CPUSTATES; + } + + cpu_times_size = sizeof(long) * cpu_times_num * CPUSTATES; + + if (VIR_ALLOC_N(cpu_times, cpu_times_num * CPUSTATES) < 0) + goto cleanup; + + if (sysctlbyname(sysctl_name, cpu_times, &cpu_times_size, NULL, 0) < 0) { + virReportSystemError(errno, + _("sysctl failed for '%s'"), + sysctl_name); + goto cleanup; + } + + for (i = 0; cpu_map[i].field != NULL; i++) { + virNodeCPUStatsPtr param = ¶ms[i]; + + if (virStrcpyStatic(param->field, cpu_map[i].field) == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Field '%s' too long for destination"), + cpu_map[i].field); + goto cleanup; + } + + param->value = 0; + for (j = 0; j < ARRAY_CARDINALITY(cpu_map[i].idx); j++) + param->value += cpu_times[offset + cpu_map[i].idx[j]] * TICK_TO_NSEC; + } + + ret = 0; + +cleanup: + VIR_FREE(cpu_times); + + return ret; +} + static int freebsdNodeGetMemoryStats(virNodeMemoryStatsPtr params, int *nparams) @@ -1066,6 +1168,8 @@ int nodeGetCPUStats(int cpuNum ATTRIBUTE_UNUSED, return ret; } +#elif defined(__FreeBSD__) + return freebsdNodeGetCPUStats(cpuNum, params, nparams); #else virReportError(VIR_ERR_NO_SUPPORT, "%s", _("node CPU stats not implemented on this platform")); diff --git a/tools/virsh-host.c b/tools/virsh-host.c index 1d1bb97..ac41177 100644 --- a/tools/virsh-host.c +++ b/tools/virsh-host.c @@ -347,9 +347,10 @@ cmdNodeCpuStats(vshControl *ctl, const vshCmd *cmd) 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, total_time; + double user_time, sys_time, idle_time, iowait_time, intr_time, total_time; double usage; if (vshCommandOptInt(cmd, "cpu", &cpuNum) < 0) { @@ -390,6 +391,8 @@ cmdNodeCpuStats(vshControl *ctl, const vshCmd *cmd) cpu_stats[i].idle = value; } else if (STREQ(params[j].field, VIR_NODE_CPU_STATS_IOWAIT)) { cpu_stats[i].iowait = value; + } else if (STREQ(params[j].field, VIR_NODE_CPU_STATS_INTR)) { + cpu_stats[i].intr = value; } else if (STREQ(params[j].field, VIR_NODE_CPU_STATS_UTILIZATION)) { cpu_stats[i].util = value; flag_utilization = true; @@ -406,6 +409,7 @@ cmdNodeCpuStats(vshControl *ctl, const vshCmd *cmd) 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); } } else { if (flag_utilization) { @@ -418,7 +422,8 @@ cmdNodeCpuStats(vshControl *ctl, const vshCmd *cmd) 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; - total_time = user_time + sys_time + idle_time + iowait_time; + 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; @@ -432,6 +437,8 @@ cmdNodeCpuStats(vshControl *ctl, const vshCmd *cmd) _("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); } } -- 1.8.4.3
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); } } ret = true; cleanup: + for (i = 0; i < 2; i++) + virHashFree(cpu_stats[i]); VIR_FREE(params); return ret; } -- 1.8.4.3
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
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
Ján Tomko wrote:
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)
I'll try to play around with that approach. Although, it doesn't seem that it would be _much_ more readable, but maybe I'm wrong.
Hmm, did libvirtd ever fill out the 'usage' field? I can't find it in git history.
IIRC, Daniel said it's provided for the systems that could not report a detailed CPU info when I was asking what would be a proper way to implement CPU stats on FreeBSD. Meanwhile, I've uploaded an updated patch which revolves your comments. Thanks, Roman Bogorodskiy
participants (2)
-
Ján Tomko -
Roman Bogorodskiy