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@xxxxxxxxxxxxxxxxx>
>> ---
>> 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@xxxxxxxxxx +1-801-349-2682
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list