[libvirt] [PATCH v2 7/8] virsh: implement new command to support perf

[Daniel P. Berrange]

On Wed, Dec 09, 2015 at 01:28:22PM +0000, Ren, Qiaowei wrote:
> 
> 
> > -----Original Message-----
> > From: Daniel P. Berrange [mailto:berrange at redhat.com]
> > Sent: Tuesday, December 8, 2015 7:07 PM
> > To: Ren, Qiaowei
> > Cc: libvir-list at redhat.com; Jiri Denemark
> > Subject: Re: [PATCH v2 7/8] virsh: implement new command to support perf
> > 
> > On Mon, Dec 07, 2015 at 03:53:58PM +0800, Qiaowei Ren wrote:
> > > This patch add new perf command to enable/disable perf event for a
> > > guest domain.
> > >
> > > Signed-off-by: Qiaowei Ren <qiaowei.ren at intel.com>
> > > ---
> > >  tools/virsh-domain.c | 125
> > +++++++++++++++++++++++++++++++++++++++++++++++++++
> > >  tools/virsh.pod      |  18 ++++++++
> > >  2 files changed, 143 insertions(+)
> > >
> > > diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index
> > > b7e7606..a47ef9f 100644
> > > --- a/tools/virsh-domain.c
> > > +++ b/tools/virsh-domain.c
> > > @@ -8888,6 +8888,125 @@ cmdMemtune(vshControl *ctl, const vshCmd
> > *cmd)
> > > }
> > >
> > >  /*
> > > + * "perf" command
> > > + */
> > > +static const vshCmdInfo info_perf[] = {
> > > +    {.name = "help",
> > > +        .data = N_("Get or set perf event")
> > > +    },
> > > +    {.name = "desc",
> > > +        .data = N_("Get or set the current perf events for a guest"
> > > +                   " domain.\n"
> > > +                   "    To get the perf events list use following command: \n\n"
> > > +                   "    virsh # perf <domain>")
> > > +    },
> > > +    {.name = NULL}
> > > +};
> > > +
> > > +static const vshCmdOptDef opts_perf[] = {
> > > +    {.name = "domain",
> > > +     .type = VSH_OT_DATA,
> > > +     .flags = VSH_OFLAG_REQ,
> > > +     .help = N_("domain name, id or uuid")
> > > +    },
> > > +    {.name = "cmt",
> > > +     .type = VSH_OT_INT,
> > > +     .help = N_("CMT event, as integer (default 0, disable)")
> > > +    },
> > 
> > Imagine we add many more perf events - we don't want to be adding args to
> > virsh for each one. Instead of --cmt, we should have '--event cmt'
> > 
> > 
> 
> Ok. I guess that this command will be the following style:
> 
> virsh perf <dom> --enable <event name, e.g. cmt|xxxx|xxx> --disable <....>
> 
> For instance: virsh perf guest01 --enable cmt
> 
> Daniel, do you think this is OK?

Yep, that's good

Regards,
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 :|