11:55 a.m.
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
11:55 a.m.
New subject: [libvirt] [PATCH v2 1/2] BSD: implement nodeGetCPUStats
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
11:55 a.m.
New subject: [libvirt] [PATCH v2 2/2] virsh: report only filled values in 'nodecpustats'
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
12:24 p.m.
New subject: [libvirt] [PATCH v2 2/2] virsh: report only filled values in 'nodecpustats'
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
8:15 a.m.
New subject: [libvirt] [PATCH v2 2/2] virsh: report only filled values in 'nodecpustats'
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
11:58 a.m.
New subject: [libvirt] [PATCH v2 2/2] virsh: report only filled values in 'nodecpustats'
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
3923
days inactive
3932
days old
5 comments
2 participants
participants (2)
-
Ján Tomko -
Roman Bogorodskiy