On Fri, 20 May 2011 11:42:26 +0100
"Daniel P. Berrange" <berrange@xxxxxxxxxx> wrote:
> On Tue, May 17, 2011 at 04:00:07PM +0900, Minoru Usui wrote:
> > virNodeGetCPUTimeParameters: Implement virsh support
> >
> > Signed-off-by: Minoru Usui <usui@xxxxxxxxxxxxxxxxx>
> > ---
> > tools/virsh.c | 107 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > tools/virsh.pod | 4 ++
> > 2 files changed, 111 insertions(+), 0 deletions(-)
> >
> > diff --git a/tools/virsh.c b/tools/virsh.c
> > index a38750f..7c9fa29 100644
> > --- a/tools/virsh.c
> > +++ b/tools/virsh.c
> > @@ -3453,6 +3453,112 @@ cmdNodeinfo(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
> > }
> >
> > /*
> > + * "nodecputime" command
> > + */
> > +static const vshCmdInfo info_nodecputime[] = {
> > + {"help", N_("Prints cpu utilization of the node.")},
> > + {"desc", N_("Returns cpu utilization of the node.(%)")},
> > + {NULL, NULL}
> > +};
> > +
> > +static bool
> > +cmdNodeCPUTimeParameters(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
> > +{
> > + int i, j;
> > + bool flag_utilization = false;
> > + virCPUTimeParameterPtr params;
> > + int nparams = 0;
> > + bool ret = false;
> > + struct cpu_time {
> > + unsigned long long user;
> > + unsigned long long sys;
> > + unsigned long long idle;
> > + unsigned long long iowait;
> > + unsigned long long util;
> > + } cpu_time[2];
> > + double user_time, sys_time, idle_time, iowait_time, total_time;
> > + double usage;
> > +
> > + if (!vshConnectionUsability(ctl, ctl->conn))
> > + return false;
> > +
> > + if (virNodeGetCPUTimeParameters(ctl->conn, NULL, &nparams, 0) != 0) {
> > + vshError(ctl, "%s",
> > + _("Unable to get number of cpu time parameters"));
> > + return false;
> > + }
> > + if (nparams == 0) {
> > + /* nothing to output */
> > + return true;
> > + }
> > +
> > + memset(cpu_time, 0, sizeof(struct cpu_time) * 2);
> > + params = vshCalloc(ctl, nparams, sizeof(*params));
> > + for (i = 0; i < 2; i++) {
> > + if (virNodeGetCPUTimeParameters(ctl->conn, params, &nparams, 0) != 0) {
> > + vshError(ctl, "%s", _("Unable to get node cpu time parameters"));
> > + goto cleanup;
> > + }
> > +
> > + for (j = 0; j < nparams; j++) {
> > + unsigned long long value = params[j].value;
> > +
> > + if (strcmp(params[j].field, VIR_CPU_TIME_KERNEL) == 0)
> > + cpu_time[i].sys = value;
> > +
> > + if (strcmp(params[j].field, VIR_CPU_TIME_USER) == 0)
> > + cpu_time[i].user = value;
> > +
> > + if (strcmp(params[j].field, VIR_CPU_TIME_IDLE) == 0)
> > + cpu_time[i].idle = value;
> > +
> > + if (strcmp(params[j].field, VIR_CPU_TIME_IOWAIT) == 0)
> > + cpu_time[i].iowait = value;
> > +
> > + if (strcmp(params[j].field, VIR_CPU_TIME_UTILIZATION) == 0) {
> > + cpu_time[i].util = value;
> > + flag_utilization = true;
> > + break;
> > + }
> > + }
> > +
> > + sleep(1);
> > + }
> > +
> > + if (flag_utilization == true) {
> > + usage = cpu_time[0].util;
> > +
> > + vshPrint(ctl, "%-15s %5.1lf%%\n", _("usage:"), usage);
> > + vshPrint(ctl, "%-15s %5.1lf%%\n", _("idle :"), 100 - usage);
> > + } else {
> > + user_time = cpu_time[1].user - cpu_time[0].user;
> > + sys_time = cpu_time[1].sys - cpu_time[0].sys;
> > + idle_time = cpu_time[1].idle - cpu_time[0].idle;
> > + iowait_time = cpu_time[1].iowait - cpu_time[0].iowait;
> > + total_time = user_time + sys_time + idle_time + iowait_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);
> > + }
> > +
> > + ret = true;
> > +
> > + cleanup:
> > + VIR_FREE(params);
> > + return ret;
> > +}
>
> I'm not sure this is the best way todo this command. I think we want
> a way to see the raw/absolute values without a sleep() in there by
> default. Only do the repeated calls+sleep+delta calculation if asked
> by some flag
Please see the following thread.
http://www.mail-archive.com/libvir-list@xxxxxxxxxx/msg36821.html
I received same comment for v2 patch from you,
and I asked you the question but you didn't answer.
Could I ask you a same question?
If we don't care the ESX hypervisor,
I'll change this virsh subcommand prints out raw/absolute values.
But I think it is not legal for libvirt.
>
> > +
> > +/*
> > * "capabilities" command
> > */
> > static const vshCmdInfo info_capabilities[] = {
> > @@ -10908,6 +11014,7 @@ static const vshCmdDef hostAndHypervisorCmds[] = {
> > {"freecell", cmdFreecell, opts_freecell, info_freecell},
> > {"hostname", cmdHostname, NULL, info_hostname},
> > {"nodeinfo", cmdNodeinfo, NULL, info_nodeinfo},
> > + {"nodecputime", cmdNodeCPUTimeParameters, NULL, info_nodecputime},
> > {"qemu-monitor-command", cmdQemuMonitorCommand, opts_qemu_monitor_command, info_qemu_monitor_command},
> > {"sysinfo", cmdSysinfo, NULL, info_sysinfo},
> > {"uri", cmdURI, NULL, info_uri},
> > diff --git a/tools/virsh.pod b/tools/virsh.pod
> > index d11a0e3..fe0755f 100644
> > --- a/tools/virsh.pod
> > +++ b/tools/virsh.pod
> > @@ -237,6 +237,10 @@ 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<nodecputime>
> > +
> > +Returns cpu utilization of the node.
> > +
> > =item B<capabilities>
> >
> > Print an XML document describing the capabilities of the hypervisor
>
>
> Daniel
> --
> |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
> |: http://libvirt.org -o- http://virt-manager.org :|
> |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
> |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
Minoru Usui <usui@xxxxxxxxxxxxxxxxx>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list