4:32 p.m.
On 06/14/2011 03:06 AM, Daniel P. Berrange wrote:
On Tue, Jun 07, 2011 at 10:02:13AM +0900, Minoru Usui wrote:
> virNodeGetCPUStats: Implement virsh support
>
> Signed-off-by: Minoru Usui <usui(a)mxm.nes.nec.co.jp>
> ---
> tools/virsh.c | 134 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> tools/virsh.pod | 7 +++
> 2 files changed, 141 insertions(+), 0 deletions(-)
>
> diff --git a/tools/virsh.c b/tools/virsh.c
> index 5679a2d..2fbc91a 100644
> --- a/tools/virsh.c
> +++ b/tools/virsh.c
> @@ -3682,6 +3682,139 @@ cmdNodeinfo(vshControl *ctl, const vshCmd *cmd
ATTRIBUTE_UNUSED)
> }
>
> /*
> + * "nodecpustats" command
> + */
> +static const vshCmdInfo info_nodecpustats[] = {
> + {"help", N_("Prints cpu stats of the node.")},
> + {"desc", N_("Returns cpu stats of the node.(nsec)")},
Not sure that the (nsec) string aids understanding here, so I nuked it.
> + struct cpu_stats {
> + unsigned long long user;
> + unsigned long long sys;
> + unsigned long long idle;
> + unsigned long long iowait;
> + unsigned long long util;
> + } cpu_stats[2];
> +
> + memset(cpu_stats, 0, sizeof(struct cpu_stats) * 2);
2 is a magic number here, easily avoided.
> +
> + if (strcmp(params[j].field, VIR_CPU_STATS_KERNEL) == 0)
> + cpu_stats[i].sys = value;
'make syntax-check' called you on this; use STREQ instead.
> +
> + if (strcmp(params[j].field, VIR_CPU_STATS_USER) == 0)
> + cpu_stats[i].user = value;
For a (minor) efficiency boost, we can use 'else if' instead of 'if'.
> +
> + if (flag_utilization || (flag_percent == false))
> + break;
> +
> + i++;
> + sleep(1);
> + } while(i < 2);
> +
> + if (flag_percent == false) {
Comparison against bool is discouraged in HACKING.
> @@ -11293,6 +11426,7 @@ static const vshCmdDef
hostAndHypervisorCmds[] = {
> {"freecell", cmdFreecell, opts_freecell, info_freecell, 0},
> {"hostname", cmdHostname, NULL, info_hostname, 0},
> {"nodeinfo", cmdNodeinfo, NULL, info_nodeinfo, 0},
> + {"nodecpustats", cmdNodeCPUStats, opts_node_cpustats,
info_nodecpustats},
Sort these lines, and for consistency, keep the trailing 0.
> {"qemu-monitor-command", cmdQemuMonitorCommand,
opts_qemu_monitor_command,
> info_qemu_monitor_command, 0},
> {"sysinfo", cmdSysinfo, NULL, info_sysinfo, 0},
> diff --git a/tools/virsh.pod b/tools/virsh.pod
> index 6ca3002..ade4eab 100644
> --- a/tools/virsh.pod
> +++ b/tools/virsh.pod
> @@ -237,6 +237,13 @@ Print the XML representation of the hypervisor sysinfo, if
available.
> Returns basic information about the node, like number and type of CPU,
> and size of the physical memory.
>
> +=item B<nodecpustats> optional I<--cpu> I<--percent>
The usage is [--cpu] <cpu>; I've tweaked this slightly.
> +
> +Returns cpu stats of the node.
> +If I<--cpu> is specified, this will prints specified cpu statistics only.
> +If I<--percent> is specified, this will prints percentage of each kind of cpu
> +statistics during 1 second.
> +
> =item B<capabilities>
>
> Print an XML document describing the capabilities of the hypervisor
ACK
Here's what I squashed in before pushing:
diff --git i/tools/virsh.c w/tools/virsh.c
index 6071aaa..1e2385c 100644
--- i/tools/virsh.c
+++ w/tools/virsh.c
@@ -3718,7 +3718,7 @@ cmdNodeinfo(vshControl *ctl, const vshCmd *cmd
ATTRIBUTE_UNUSED)
*/
static const vshCmdInfo info_nodecpustats[] = {
{"help", N_("Prints cpu stats of the node.")},
- {"desc", N_("Returns cpu stats of the node.(nsec)")},
+ {"desc", N_("Returns cpu stats of the node.")},
{NULL, NULL}
};
@@ -3766,7 +3766,7 @@ cmdNodeCPUStats(vshControl *ctl, const vshCmd *cmd
ATTRIBUTE_UNUSED)
return true;
}
- memset(cpu_stats, 0, sizeof(struct cpu_stats) * 2);
+ memset(cpu_stats, 0, sizeof(cpu_stats));
params = vshCalloc(ctl, nparams, sizeof(*params));
i = 0;
@@ -3779,33 +3779,29 @@ cmdNodeCPUStats(vshControl *ctl, const vshCmd
*cmd ATTRIBUTE_UNUSED)
for (j = 0; j < nparams; j++) {
unsigned long long value = params[j].value;
- if (strcmp(params[j].field, VIR_CPU_STATS_KERNEL) == 0)
+ if (STREQ(params[j].field, VIR_CPU_STATS_KERNEL)) {
cpu_stats[i].sys = value;
-
- if (strcmp(params[j].field, VIR_CPU_STATS_USER) == 0)
+ } else if (STREQ(params[j].field, VIR_CPU_STATS_USER)) {
cpu_stats[i].user = value;
-
- if (strcmp(params[j].field, VIR_CPU_STATS_IDLE) == 0)
+ } else if (STREQ(params[j].field, VIR_CPU_STATS_IDLE)) {
cpu_stats[i].idle = value;
-
- if (strcmp(params[j].field, VIR_CPU_STATS_IOWAIT) == 0)
+ } else if (STREQ(params[j].field, VIR_CPU_STATS_IOWAIT)) {
cpu_stats[i].iowait = value;
-
- if (strcmp(params[j].field, VIR_CPU_STATS_UTILIZATION) == 0) {
+ } else if (STREQ(params[j].field, VIR_CPU_STATS_UTILIZATION)) {
cpu_stats[i].util = value;
flag_utilization = true;
}
}
- if (flag_utilization || (flag_percent == false))
+ if (flag_utilization || !flag_percent)
break;
i++;
sleep(1);
} while(i < 2);
- if (flag_percent == false) {
- if (flag_utilization == false) {
+ 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);
@@ -11526,8 +11522,8 @@ static const vshCmdDef hostAndHypervisorCmds[] = {
VSH_CMD_FLAG_NOCONNECT},
{"freecell", cmdFreecell, opts_freecell, info_freecell, 0},
{"hostname", cmdHostname, NULL, info_hostname, 0},
+ {"nodecpustats", cmdNodeCPUStats, opts_node_cpustats,
info_nodecpustats, 0},
{"nodeinfo", cmdNodeinfo, NULL, info_nodeinfo, 0},
- {"nodecpustats", cmdNodeCPUStats, opts_node_cpustats,
info_nodecpustats},
{"qemu-monitor-command", cmdQemuMonitorCommand,
opts_qemu_monitor_command,
info_qemu_monitor_command, 0},
{"sysinfo", cmdSysinfo, NULL, info_sysinfo, 0},
diff --git i/tools/virsh.pod w/tools/virsh.pod
index b2dbbb9..b4c580a 100644
--- i/tools/virsh.pod
+++ w/tools/virsh.pod
@@ -239,10 +239,10 @@ and size of the physical memory. The output
corresponds to virNodeInfo
structure. Specifically, the "CPU socket(s)" field means number of CPU
sockets per NUMA cell.
-=item B<nodecpustats> optional I<--cpu> I<--percent>
+=item B<nodecpustats> optional I<cpu> I<--percent>
Returns cpu stats of the node.
-If I<--cpu> is specified, this will prints specified cpu statistics only.
+If I<cpu> is specified, this will prints specified cpu statistics only.
If I<--percent> is specified, this will prints percentage of each kind
of cpu
statistics during 1 second.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org