[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCHv5 5/6] virNodeGetCPUTimeParameters: Implement virsh support



On Fri, 20 May 2011 11:42:26 +0100
"Daniel P. Berrange" <berrange redhat com> wrote:

> On Tue, May 17, 2011 at 04:00:07PM +0900, Minoru Usui wrote:
> > virNodeGetCPUTimeParameters: Implement virsh support
> > 
> > Signed-off-by: Minoru Usui <usui mxm nes nec co jp>
> > ---
> >  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 redhat com/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 mxm nes nec co jp>


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]