Re: [libvirt] [PATCH 6/8] qemu_driver: add support to perf event

2015-11-30 Thread Ren, Qiaowei

> -Original Message-
> From: Jiri Denemark [mailto:jdene...@redhat.com]
> Sent: Monday, November 30, 2015 7:09 PM
> To: Ren, Qiaowei
> Cc: libvir-list@redhat.com
> Subject: Re: [libvirt] [PATCH 6/8] qemu_driver: add support to perf event
> 
> On Mon, Nov 30, 2015 at 08:17:05 +, Ren, Qiaowei wrote:
> >
> > > -Original Message-
> > > From: Jiri Denemark [mailto:jdene...@redhat.com]
> > > Sent: Tuesday, November 24, 2015 9:01 PM
> > > To: Ren, Qiaowei
> > > Cc: libvir-list@redhat.com
> > > Subject: Re: [libvirt] [PATCH 6/8] qemu_driver: add support to perf
> > > event
> > >
> > > On Tue, Nov 17, 2015 at 16:00:46 +0800, Qiaowei Ren wrote:
> > > > This patch implement the internal driver API for perf event into
> > > > qemu driver.
> > > >
> > > > In addition, this patch extend virDomainListGetStats API to get
> > > > the statistics for perf event. To do so, we add a
> 'VIR_DOMAIN_STATS_PERF'
> > > > enum to causes reporting of all previously enabled perf events.
> > > >
> > > > Signed-off-by: Qiaowei Ren <qiaowei@intel.com>
> > > > ---
> > > >  include/libvirt/libvirt-domain.h |   1 +
> > > >  src/qemu/qemu_domain.h   |   3 +
> > > >  src/qemu/qemu_driver.c   | 181
> > > +++
> > > >  #define QEMU_SCHED_MIN_PERIOD  1000LL
> > > >  #define QEMU_SCHED_MAX_PERIOD   100LL
> > > >  #define QEMU_SCHED_MIN_QUOTA   1000LL
> > > > @@ -10244,6 +10247,106 @@
> > > qemuDomainGetNumaParameters(virDomainPtr
> > > > dom,  }
> > > >
> > > >  static int
> > > > +qemuDomainSetPerfEvents(virDomainPtr dom,
> > > > +virTypedParameterPtr params,
> > > > +int nparams) {
> > > > +size_t i;
> > > > +virDomainObjPtr vm = NULL;
> > > > +qemuDomainObjPrivatePtr priv;
> > > > +int ret = -1;
> > > > +bool enabled;
> > > > +
> > > > +if (virTypedParamsValidate(params, nparams,
> > > > +   VIR_DOMAIN_PERF_CMT,
> > > > +   VIR_TYPED_PARAM_BOOLEAN,
> > > > +   NULL) < 0)
> > > > +return -1;
> > >
> > > Use virTypedParamsCheck and define the data for it in virperf.h so
> > > that you don't need to change the code here if new event type is added.
> > >
> >
> > Jirka, I checked the difference between virTypedParamsValidate and
> > virTypedParamsCheck, and found that virTypedParamsCheck just check
> > whether the param is valid through param name, but
> > virTypedParamsValidate will check whether the param type is right.
> 
> Oh, sorry for the confusion then. I actually wanted the code to be something 
> like
> 
> if (virTypedParamsValidate(params, nparams, VIR_PERF_PARAMS) < 0)
> return -1;
> 
> and VIR_PERF_PARAMS would be a macro defined in virparams.h:
> 
> # define VIR_PERF_PARAMS \
> VIR_DOMAIN_PERF_CMT, VIR_TYPED_PARAM_BOOLEAN, \
> NULL
> 
> Similar to QEMU_MIGRATION_PARAMETERS and few others.
> 

Got it! Thanks, Jirka.

Qiaowei


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH 6/8] qemu_driver: add support to perf event

2015-11-30 Thread Ren, Qiaowei

> -Original Message-
> From: Jiri Denemark [mailto:jdene...@redhat.com]
> Sent: Tuesday, November 24, 2015 9:01 PM
> To: Ren, Qiaowei
> Cc: libvir-list@redhat.com
> Subject: Re: [libvirt] [PATCH 6/8] qemu_driver: add support to perf event
> 
> On Tue, Nov 17, 2015 at 16:00:46 +0800, Qiaowei Ren wrote:
> > This patch implement the internal driver API for perf event into qemu
> > driver.
> >
> > In addition, this patch extend virDomainListGetStats API to get the
> > statistics for perf event. To do so, we add a 'VIR_DOMAIN_STATS_PERF'
> > enum to causes reporting of all previously enabled perf events.
> >
> > Signed-off-by: Qiaowei Ren <qiaowei@intel.com>
> > ---
> >  include/libvirt/libvirt-domain.h |   1 +
> >  src/qemu/qemu_domain.h   |   3 +
> >  src/qemu/qemu_driver.c   | 181
> +++
> >  #define QEMU_SCHED_MIN_PERIOD  1000LL
> >  #define QEMU_SCHED_MAX_PERIOD   100LL
> >  #define QEMU_SCHED_MIN_QUOTA   1000LL
> > @@ -10244,6 +10247,106 @@
> qemuDomainGetNumaParameters(virDomainPtr
> > dom,  }
> >
> >  static int
> > +qemuDomainSetPerfEvents(virDomainPtr dom,
> > +virTypedParameterPtr params,
> > +int nparams)
> > +{
> > +size_t i;
> > +virDomainObjPtr vm = NULL;
> > +qemuDomainObjPrivatePtr priv;
> > +int ret = -1;
> > +bool enabled;
> > +
> > +if (virTypedParamsValidate(params, nparams,
> > +   VIR_DOMAIN_PERF_CMT,
> > +   VIR_TYPED_PARAM_BOOLEAN,
> > +   NULL) < 0)
> > +return -1;
> 
> Use virTypedParamsCheck and define the data for it in virperf.h so that you
> don't need to change the code here if new event type is added.
> 

Jirka, I checked the difference between virTypedParamsValidate and 
virTypedParamsCheck, and found that virTypedParamsCheck just check whether the 
param is valid through param name, but virTypedParamsValidate will check 
whether the param type is right.

In addition, virTypedParamsCheck is just used in three functions in 
libvirt-domain.c, and all driver APIs use virTypedParamsValidate to validate 
the parms like this patch. What do you think about it? 

Thanks,
Qiaowei

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH 6/8] qemu_driver: add support to perf event

2015-11-30 Thread Jiri Denemark
On Mon, Nov 30, 2015 at 08:17:05 +, Ren, Qiaowei wrote:
> 
> > -Original Message-
> > From: Jiri Denemark [mailto:jdene...@redhat.com]
> > Sent: Tuesday, November 24, 2015 9:01 PM
> > To: Ren, Qiaowei
> > Cc: libvir-list@redhat.com
> > Subject: Re: [libvirt] [PATCH 6/8] qemu_driver: add support to perf event
> > 
> > On Tue, Nov 17, 2015 at 16:00:46 +0800, Qiaowei Ren wrote:
> > > This patch implement the internal driver API for perf event into qemu
> > > driver.
> > >
> > > In addition, this patch extend virDomainListGetStats API to get the
> > > statistics for perf event. To do so, we add a 'VIR_DOMAIN_STATS_PERF'
> > > enum to causes reporting of all previously enabled perf events.
> > >
> > > Signed-off-by: Qiaowei Ren <qiaowei@intel.com>
> > > ---
> > >  include/libvirt/libvirt-domain.h |   1 +
> > >  src/qemu/qemu_domain.h   |   3 +
> > >  src/qemu/qemu_driver.c   | 181
> > +++
> > >  #define QEMU_SCHED_MIN_PERIOD  1000LL
> > >  #define QEMU_SCHED_MAX_PERIOD   100LL
> > >  #define QEMU_SCHED_MIN_QUOTA   1000LL
> > > @@ -10244,6 +10247,106 @@
> > qemuDomainGetNumaParameters(virDomainPtr
> > > dom,  }
> > >
> > >  static int
> > > +qemuDomainSetPerfEvents(virDomainPtr dom,
> > > +virTypedParameterPtr params,
> > > +int nparams)
> > > +{
> > > +size_t i;
> > > +virDomainObjPtr vm = NULL;
> > > +qemuDomainObjPrivatePtr priv;
> > > +int ret = -1;
> > > +bool enabled;
> > > +
> > > +if (virTypedParamsValidate(params, nparams,
> > > +   VIR_DOMAIN_PERF_CMT,
> > > +   VIR_TYPED_PARAM_BOOLEAN,
> > > +   NULL) < 0)
> > > +return -1;
> > 
> > Use virTypedParamsCheck and define the data for it in virperf.h so that you
> > don't need to change the code here if new event type is added.
> > 
> 
> Jirka, I checked the difference between virTypedParamsValidate and
> virTypedParamsCheck, and found that virTypedParamsCheck just check
> whether the param is valid through param name, but
> virTypedParamsValidate will check whether the param type is right.

Oh, sorry for the confusion then. I actually wanted the code to be
something like

if (virTypedParamsValidate(params, nparams, VIR_PERF_PARAMS) < 0)
return -1;

and VIR_PERF_PARAMS would be a macro defined in virparams.h:

# define VIR_PERF_PARAMS \
VIR_DOMAIN_PERF_CMT, VIR_TYPED_PARAM_BOOLEAN, \
NULL

Similar to QEMU_MIGRATION_PARAMETERS and few others.

Jirka

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH 6/8] qemu_driver: add support to perf event

2015-11-24 Thread Jiri Denemark
On Tue, Nov 17, 2015 at 16:00:46 +0800, Qiaowei Ren wrote:
> This patch implement the internal driver API for perf event into
> qemu driver.
> 
> In addition, this patch extend virDomainListGetStats API to get
> the statistics for perf event. To do so, we add a
> 'VIR_DOMAIN_STATS_PERF' enum to causes reporting of all previously
> enabled perf events.
> 
> Signed-off-by: Qiaowei Ren 
> ---
>  include/libvirt/libvirt-domain.h |   1 +
>  src/qemu/qemu_domain.h   |   3 +
>  src/qemu/qemu_driver.c   | 181 
> +++
>  src/qemu/qemu_process.c  |   6 ++
>  4 files changed, 191 insertions(+)
> 
> diff --git a/include/libvirt/libvirt-domain.h 
> b/include/libvirt/libvirt-domain.h
> index 69965e6..5e74b29 100644
> --- a/include/libvirt/libvirt-domain.h
> +++ b/include/libvirt/libvirt-domain.h
> @@ -1771,6 +1771,7 @@ typedef enum {
>  VIR_DOMAIN_STATS_VCPU = (1 << 3), /* return domain virtual CPU info */
>  VIR_DOMAIN_STATS_INTERFACE = (1 << 4), /* return domain interfaces info 
> */
>  VIR_DOMAIN_STATS_BLOCK = (1 << 5), /* return domain block info */
> +VIR_DOMAIN_STATS_PERF = (1 << 6), /* return domain perf event info */
>  } virDomainStatsTypes;
>  
>  typedef enum {
> diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
> index 4be998c..0348d4a 100644
> --- a/src/qemu/qemu_domain.h
> +++ b/src/qemu/qemu_domain.h
> @@ -26,6 +26,7 @@
>  
>  # include "virthread.h"
>  # include "vircgroup.h"
> +# include "virperf.h"
>  # include "domain_addr.h"
>  # include "domain_conf.h"
>  # include "snapshot_conf.h"
> @@ -190,6 +191,8 @@ struct _qemuDomainObjPrivate {
>  
>  virCgroupPtr cgroup;
>  
> +virPerfPtr perf;
> +
>  virCond unplugFinished; /* signals that unpluggingDevice was unplugged */
>  const char *unpluggingDevice; /* alias of the device that is being 
> unplugged */
>  char **qemuDevices; /* NULL-terminated list of devices aliases known to 
> QEMU */
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 92a9961..5ad4d79 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -98,6 +98,7 @@
>  #include "virhostdev.h"
>  #include "domain_capabilities.h"
>  #include "vircgroup.h"
> +#include "virperf.h"
>  #include "virnuma.h"
>  #include "dirname.h"
>  #include "network/bridge_driver.h"
> @@ -113,6 +114,8 @@ VIR_LOG_INIT("qemu.qemu_driver");
>  
>  #define QEMU_NB_NUMA_PARAM 2
>  
> +#define QEMU_NB_PERF_PARAM VIR_PERF_EVENT_LAST
> +

VIR_PERF_EVENT_LAST is good enough, no need to define another symbol for
it.

>  #define QEMU_SCHED_MIN_PERIOD  1000LL
>  #define QEMU_SCHED_MAX_PERIOD   100LL
>  #define QEMU_SCHED_MIN_QUOTA   1000LL
> @@ -10244,6 +10247,106 @@ qemuDomainGetNumaParameters(virDomainPtr dom,
>  }
>  
>  static int
> +qemuDomainSetPerfEvents(virDomainPtr dom,
> +virTypedParameterPtr params,
> +int nparams)
> +{
> +size_t i;
> +virDomainObjPtr vm = NULL;
> +qemuDomainObjPrivatePtr priv;
> +int ret = -1;
> +bool enabled;
> +
> +if (virTypedParamsValidate(params, nparams,
> +   VIR_DOMAIN_PERF_CMT,
> +   VIR_TYPED_PARAM_BOOLEAN,
> +   NULL) < 0)
> +return -1;

Use virTypedParamsCheck and define the data for it in virperf.h so that
you don't need to change the code here if new event type is added.

> +
> +if (!(vm = qemuDomObjFromDomain(dom)))
> +return -1;
> +
> +priv = vm->privateData;
> +
> +if (virDomainGetPerfEventsEnsureACL(dom->conn, vm->def) < 0)
> +goto cleanup;
> +
> +for (i = 0; i < nparams; i++) {
> +virTypedParameterPtr param = [i];
> +enabled = params[i].value.b;
> +
> +if (STREQ(param->field, VIR_DOMAIN_PERF_CMT)) {

Just use the appropriate FromString to convert the param field into the
perf type enum and call virPerfEvent{Disable,Enable} depending on the
value. Thus no change will be needed here if you add more perf events.

> +if (!enabled && virPerfEventDisable(VIR_PERF_EVENT_CMT, 
> priv->perf)) {
> +virReportError(VIR_ERR_NO_SUPPORT,
> +   _("can't disable perf event: %s"),
> +   VIR_DOMAIN_PERF_CMT);
> +goto cleanup;
> +}
> +if (enabled && virPerfCmtEnable(vm->pid, priv->perf)) {
> +virReportError(VIR_ERR_NO_SUPPORT,
> +   _("can't enable perf event: %s"),
> +   VIR_DOMAIN_PERF_CMT);
> +goto cleanup;
> +}

Both virPerfEventDisable and virPerfEventEnable should have reported the
error, no need to mask it with another one.

> +}
> +}
> +
> +ret = 0;
> +
> + cleanup:
> +virDomainObjEndAPI();
> +return ret;
> +}
> +
> +static int
> 

[libvirt] [PATCH 6/8] qemu_driver: add support to perf event

2015-11-17 Thread Qiaowei Ren
This patch implement the internal driver API for perf event into
qemu driver.

In addition, this patch extend virDomainListGetStats API to get
the statistics for perf event. To do so, we add a
'VIR_DOMAIN_STATS_PERF' enum to causes reporting of all previously
enabled perf events.

Signed-off-by: Qiaowei Ren 
---
 include/libvirt/libvirt-domain.h |   1 +
 src/qemu/qemu_domain.h   |   3 +
 src/qemu/qemu_driver.c   | 181 +++
 src/qemu/qemu_process.c  |   6 ++
 4 files changed, 191 insertions(+)

diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index 69965e6..5e74b29 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -1771,6 +1771,7 @@ typedef enum {
 VIR_DOMAIN_STATS_VCPU = (1 << 3), /* return domain virtual CPU info */
 VIR_DOMAIN_STATS_INTERFACE = (1 << 4), /* return domain interfaces info */
 VIR_DOMAIN_STATS_BLOCK = (1 << 5), /* return domain block info */
+VIR_DOMAIN_STATS_PERF = (1 << 6), /* return domain perf event info */
 } virDomainStatsTypes;
 
 typedef enum {
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 4be998c..0348d4a 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -26,6 +26,7 @@
 
 # include "virthread.h"
 # include "vircgroup.h"
+# include "virperf.h"
 # include "domain_addr.h"
 # include "domain_conf.h"
 # include "snapshot_conf.h"
@@ -190,6 +191,8 @@ struct _qemuDomainObjPrivate {
 
 virCgroupPtr cgroup;
 
+virPerfPtr perf;
+
 virCond unplugFinished; /* signals that unpluggingDevice was unplugged */
 const char *unpluggingDevice; /* alias of the device that is being 
unplugged */
 char **qemuDevices; /* NULL-terminated list of devices aliases known to 
QEMU */
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 92a9961..5ad4d79 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -98,6 +98,7 @@
 #include "virhostdev.h"
 #include "domain_capabilities.h"
 #include "vircgroup.h"
+#include "virperf.h"
 #include "virnuma.h"
 #include "dirname.h"
 #include "network/bridge_driver.h"
@@ -113,6 +114,8 @@ VIR_LOG_INIT("qemu.qemu_driver");
 
 #define QEMU_NB_NUMA_PARAM 2
 
+#define QEMU_NB_PERF_PARAM VIR_PERF_EVENT_LAST
+
 #define QEMU_SCHED_MIN_PERIOD  1000LL
 #define QEMU_SCHED_MAX_PERIOD   100LL
 #define QEMU_SCHED_MIN_QUOTA   1000LL
@@ -10244,6 +10247,106 @@ qemuDomainGetNumaParameters(virDomainPtr dom,
 }
 
 static int
+qemuDomainSetPerfEvents(virDomainPtr dom,
+virTypedParameterPtr params,
+int nparams)
+{
+size_t i;
+virDomainObjPtr vm = NULL;
+qemuDomainObjPrivatePtr priv;
+int ret = -1;
+bool enabled;
+
+if (virTypedParamsValidate(params, nparams,
+   VIR_DOMAIN_PERF_CMT,
+   VIR_TYPED_PARAM_BOOLEAN,
+   NULL) < 0)
+return -1;
+
+if (!(vm = qemuDomObjFromDomain(dom)))
+return -1;
+
+priv = vm->privateData;
+
+if (virDomainGetPerfEventsEnsureACL(dom->conn, vm->def) < 0)
+goto cleanup;
+
+for (i = 0; i < nparams; i++) {
+virTypedParameterPtr param = [i];
+enabled = params[i].value.b;
+
+if (STREQ(param->field, VIR_DOMAIN_PERF_CMT)) {
+if (!enabled && virPerfEventDisable(VIR_PERF_EVENT_CMT, 
priv->perf)) {
+virReportError(VIR_ERR_NO_SUPPORT,
+   _("can't disable perf event: %s"),
+   VIR_DOMAIN_PERF_CMT);
+goto cleanup;
+}
+if (enabled && virPerfCmtEnable(vm->pid, priv->perf)) {
+virReportError(VIR_ERR_NO_SUPPORT,
+   _("can't enable perf event: %s"),
+   VIR_DOMAIN_PERF_CMT);
+goto cleanup;
+}
+}
+}
+
+ret = 0;
+
+ cleanup:
+virDomainObjEndAPI();
+return ret;
+}
+
+static int
+qemuDomainGetPerfEvents(virDomainPtr dom,
+virTypedParameterPtr params,
+int * nparams)
+{
+size_t i;
+virDomainObjPtr vm = NULL;
+qemuDomainObjPrivatePtr priv;
+int ret = -1;
+
+if (!(vm = qemuDomObjFromDomain(dom)))
+goto cleanup;
+
+priv = vm->privateData;
+
+if (virDomainGetPerfEventsEnsureACL(dom->conn, vm->def) < 0)
+goto cleanup;
+
+if ((*nparams) == 0) {
+*nparams = QEMU_NB_PERF_PARAM;
+ret = 0;
+goto cleanup;
+}
+
+for (i = 0; i < QEMU_NB_PERF_PARAM && i < *nparams; i++) {
+virMemoryParameterPtr param = [i];
+
+switch (i) {
+case VIR_PERF_EVENT_CMT:
+if (virTypedParameterAssign(param, VIR_DOMAIN_PERF_CMT,
+VIR_TYPED_PARAM_BOOLEAN,
+