[libvirt] [PATCH v2 0/8] Add perf and Intel CMT feature support

The series mainly adds Intel CMT feature support into libvirt. CMT is new introduced PQos (Platform Qos) feature to monitor the usage of cache by applications running on the platform. Currently CMT patches has been merged into Linux kernel mainline. The CMT implementation in Linux kernel is based on perf mechanism and there is no support for perf in libvirt, and so this series firstly add perf support into libvirt, including two public API and a set of util interfaces. And based on these APIs and interfaces, thie series implements CMT perf event support. TODO: 1. add support for new APIs into libvirt-python library. Changes since v1: * change perf APIs implementation to match new style of the API. * add new xml element * reenable all perf events previously enabled when libvirtd daemon restart. * redesign perf util functions. Qiaowei Ren (8): perf: add new public APIs for perf event perf: implement the remote protocol for perf event perf: implement a set of util functions for perf event qemu_driver: add support to perf event perf: add new xml element perf: reenable perf events when libvirtd restart virsh: implement new command to support perf virsh: extend domstats command daemon/remote.c | 47 ++++++ docs/schemas/domaincommon.rng | 27 ++++ include/libvirt/libvirt-domain.h | 19 +++ include/libvirt/virterror.h | 1 + src/Makefile.am | 1 + src/conf/domain_conf.c | 37 +++++ src/conf/domain_conf.h | 10 ++ src/driver-hypervisor.h | 12 ++ src/libvirt-domain.c | 93 ++++++++++++ src/libvirt_private.syms | 12 ++ src/libvirt_public.syms | 6 + src/qemu/qemu_domain.h | 3 + src/qemu/qemu_driver.c | 206 +++++++++++++++++++++++++++ src/qemu/qemu_process.c | 9 ++ src/remote/remote_driver.c | 39 +++++ src/remote/remote_protocol.x | 30 +++- src/remote_protocol-structs | 18 +++ src/util/virerror.c | 1 + src/util/virperf.c | 298 +++++++++++++++++++++++++++++++++++++++ src/util/virperf.h | 61 ++++++++ tools/virsh-domain-monitor.c | 7 + tools/virsh-domain.c | 125 ++++++++++++++++ tools/virsh.pod | 25 +++- 23 files changed, 1084 insertions(+), 3 deletions(-) create mode 100644 src/util/virperf.c create mode 100644 src/util/virperf.h -- 1.9.1

API agreed on in https://www.redhat.com/archives/libvir-list/2015-October/msg00872.html * include/libvirt/libvirt-domain.h (virDomainGetPerfEvents, virDomainSetPerfEvents): New declarations. * src/libvirt_public.syms: Export new symbols. * src/driver-hypervisor.h (virDrvDomainGetPerfEvents, virDrvDomainSetPerfEvents): New typedefs. * src/libvirt-domain.c: Implement virDomainGetPerfEvents and virDomainSetPerfEvents. Signed-off-by: Qiaowei Ren <qiaowei.ren@intel.com> --- include/libvirt/libvirt-domain.h | 18 ++++++++ src/driver-hypervisor.h | 12 ++++++ src/libvirt-domain.c | 93 ++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 6 +++ 4 files changed, 129 insertions(+) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index a1ea6a5..cc1b29b 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -1802,6 +1802,24 @@ int virDomainListGetStats(virDomainPtr *doms, void virDomainStatsRecordListFree(virDomainStatsRecordPtr *stats); /* + * Perf Event API + */ + +/** + * VIR_PERF_PARAM_CMT: + * + * Macro for typed parameter name that represents CMT perf event. + */ +# define VIR_PERF_PARAM_CMT "cmt" + +int virDomainGetPerfEvents(virDomainPtr dom, + virTypedParameterPtr *params, + int *nparams); +int virDomainSetPerfEvents(virDomainPtr dom, + virTypedParameterPtr params, + int nparams); + +/* * BlockJob API */ diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h index ae2ec4d..aedbc83 100644 --- a/src/driver-hypervisor.h +++ b/src/driver-hypervisor.h @@ -958,6 +958,16 @@ typedef int unsigned int flags); typedef int +(*virDrvDomainGetPerfEvents)(virDomainPtr dom, + virTypedParameterPtr *params, + int *nparams); + +typedef int +(*virDrvDomainSetPerfEvents)(virDomainPtr dom, + virTypedParameterPtr params, + int nparams); + +typedef int (*virDrvDomainBlockJobAbort)(virDomainPtr dom, const char *path, unsigned int flags); @@ -1413,6 +1423,8 @@ struct _virHypervisorDriver { virDrvConnectSetKeepAlive connectSetKeepAlive; virDrvConnectIsAlive connectIsAlive; virDrvNodeSuspendForDuration nodeSuspendForDuration; + virDrvDomainGetPerfEvents domainGetPerfEvents; + virDrvDomainSetPerfEvents domainSetPerfEvents; virDrvDomainSetBlockIoTune domainSetBlockIoTune; virDrvDomainGetBlockIoTune domainGetBlockIoTune; virDrvDomainGetCPUStats domainGetCPUStats; diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index b91388e..f105803 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -9564,6 +9564,99 @@ virDomainOpenChannel(virDomainPtr dom, /** + * virDomainGetPerfEvents: + * @domain: a domain object + * @params: where to store perf events setting + * @nparams: number of items in @params + * + * Get all perf events setting. Possible fields returned in @params are + * defined by VIR_DOMAIN_PERF_* macros and new fields will likely be + * introduced in the future. + * + * Returns -1 in case of failure, 0 in case of success. + */ +int virDomainGetPerfEvents(virDomainPtr domain, + virTypedParameterPtr *params, + int *nparams) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(domain, "params=%p, nparams=%p", + params, nparams); + + virResetLastError(); + + virCheckDomainReturn(domain, -1); + virCheckNonNullArgGoto(params, error); + virCheckNonNullArgGoto(nparams, error); + + conn = domain->conn; + + if (conn->driver->domainGetPerfEvents) { + int ret; + ret = conn->driver->domainGetPerfEvents(domain, params, nparams); + if (ret < 0) + goto error; + return ret; + } + virReportUnsupportedError(); + + error: + virDispatchError(domain->conn); + return -1; +} + + +/** + * virDomainSetPerfEvents: + * @domain: a domain object + * @params: pointer to perf events parameter object + * @nparams: number of perf event parameters (this value can be the same + * less than the number of parameters supported) + * + * Enable or disable the particular list of perf events you care about. + * + * Returns -1 in case of error, 0 in case of success. + */ +int virDomainSetPerfEvents(virDomainPtr domain, + virTypedParameterPtr params, + int nparams) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(domain, "params=%p, nparams=%d", + params, nparams); + VIR_TYPED_PARAMS_DEBUG(params, nparams); + + virResetLastError(); + + virCheckDomainReturn(domain, -1); + conn = domain->conn; + + virCheckReadOnlyGoto(conn->flags, error); + virCheckNonNullArgGoto(params, error); + virCheckPositiveArgGoto(nparams, error); + + if (virTypedParameterValidateSet(conn, params, nparams) < 0) + goto error; + + if (conn->driver->domainSetPerfEvents) { + int ret; + ret = conn->driver->domainSetPerfEvents(domain, params, nparams); + if (ret < 0) + goto error; + return ret; + } + + virReportUnsupportedError(); + + error: + virDispatchError(domain->conn); + return -1; +} + + +/** * virDomainBlockJobAbort: * @dom: pointer to domain object * @disk: path to the block device, or device shorthand diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index dd94191..03206e7 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -725,4 +725,10 @@ LIBVIRT_1.2.19 { virDomainRename; } LIBVIRT_1.2.17; +LIBVIRT_1.3.0 { + global: + virDomainGetPerfEvents; + virDomainSetPerfEvents; +} LIBVIRT_1.2.19; + # .... define new API here using predicted next version number .... -- 1.9.1

On Mon, Dec 07, 2015 at 03:53:52PM +0800, Qiaowei Ren wrote:
API agreed on in https://www.redhat.com/archives/libvir-list/2015-October/msg00872.html
* include/libvirt/libvirt-domain.h (virDomainGetPerfEvents, virDomainSetPerfEvents): New declarations. * src/libvirt_public.syms: Export new symbols. * src/driver-hypervisor.h (virDrvDomainGetPerfEvents, virDrvDomainSetPerfEvents): New typedefs. * src/libvirt-domain.c: Implement virDomainGetPerfEvents and virDomainSetPerfEvents.
Signed-off-by: Qiaowei Ren <qiaowei.ren@intel.com> --- include/libvirt/libvirt-domain.h | 18 ++++++++ src/driver-hypervisor.h | 12 ++++++ src/libvirt-domain.c | 93 ++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 6 +++ 4 files changed, 129 insertions(+)
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index a1ea6a5..cc1b29b 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -1802,6 +1802,24 @@ int virDomainListGetStats(virDomainPtr *doms, void virDomainStatsRecordListFree(virDomainStatsRecordPtr *stats);
/* + * Perf Event API + */ + +/** + * VIR_PERF_PARAM_CMT: + * + * Macro for typed parameter name that represents CMT perf event. + */ +# define VIR_PERF_PARAM_CMT "cmt" + +int virDomainGetPerfEvents(virDomainPtr dom, + virTypedParameterPtr *params, + int *nparams); +int virDomainSetPerfEvents(virDomainPtr dom, + virTypedParameterPtr params, + int nparams); + +/* * BlockJob API */
diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h index ae2ec4d..aedbc83 100644 --- a/src/driver-hypervisor.h +++ b/src/driver-hypervisor.h @@ -958,6 +958,16 @@ typedef int unsigned int flags);
typedef int +(*virDrvDomainGetPerfEvents)(virDomainPtr dom, + virTypedParameterPtr *params, + int *nparams); + +typedef int +(*virDrvDomainSetPerfEvents)(virDomainPtr dom, + virTypedParameterPtr params, + int nparams); + +typedef int (*virDrvDomainBlockJobAbort)(virDomainPtr dom, const char *path, unsigned int flags); @@ -1413,6 +1423,8 @@ struct _virHypervisorDriver { virDrvConnectSetKeepAlive connectSetKeepAlive; virDrvConnectIsAlive connectIsAlive; virDrvNodeSuspendForDuration nodeSuspendForDuration; + virDrvDomainGetPerfEvents domainGetPerfEvents; + virDrvDomainSetPerfEvents domainSetPerfEvents; virDrvDomainSetBlockIoTune domainSetBlockIoTune; virDrvDomainGetBlockIoTune domainGetBlockIoTune; virDrvDomainGetCPUStats domainGetCPUStats; diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index b91388e..f105803 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -9564,6 +9564,99 @@ virDomainOpenChannel(virDomainPtr dom,
/** + * virDomainGetPerfEvents: + * @domain: a domain object + * @params: where to store perf events setting + * @nparams: number of items in @params + * + * Get all perf events setting. Possible fields returned in @params are + * defined by VIR_DOMAIN_PERF_* macros and new fields will likely be + * introduced in the future.
You called the constants VIR_PERF in the header - either this doc needs changing or the constants need renaming to VIR_DOMAIN_PERF
+ * + * Returns -1 in case of failure, 0 in case of success. + */ +int virDomainGetPerfEvents(virDomainPtr domain, + virTypedParameterPtr *params, + int *nparams) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(domain, "params=%p, nparams=%p", + params, nparams); + + virResetLastError(); + + virCheckDomainReturn(domain, -1); + virCheckNonNullArgGoto(params, error); + virCheckNonNullArgGoto(nparams, error); + + conn = domain->conn; + + if (conn->driver->domainGetPerfEvents) { + int ret; + ret = conn->driver->domainGetPerfEvents(domain, params, nparams); + if (ret < 0) + goto error; + return ret; + } + virReportUnsupportedError(); + + error: + virDispatchError(domain->conn); + return -1; +} + + +/** + * virDomainSetPerfEvents: + * @domain: a domain object + * @params: pointer to perf events parameter object + * @nparams: number of perf event parameters (this value can be the same + * less than the number of parameters supported) + * + * Enable or disable the particular list of perf events you care about. + * + * Returns -1 in case of error, 0 in case of success. + */ +int virDomainSetPerfEvents(virDomainPtr domain, + virTypedParameterPtr params, + int nparams) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(domain, "params=%p, nparams=%d", + params, nparams); + VIR_TYPED_PARAMS_DEBUG(params, nparams); + + virResetLastError(); + + virCheckDomainReturn(domain, -1); + conn = domain->conn; + + virCheckReadOnlyGoto(conn->flags, error); + virCheckNonNullArgGoto(params, error); + virCheckPositiveArgGoto(nparams, error); + + if (virTypedParameterValidateSet(conn, params, nparams) < 0) + goto error; + + if (conn->driver->domainSetPerfEvents) { + int ret; + ret = conn->driver->domainSetPerfEvents(domain, params, nparams); + if (ret < 0) + goto error; + return ret; + } + + virReportUnsupportedError(); + + error: + virDispatchError(domain->conn); + return -1; +} + + +/** * virDomainBlockJobAbort: * @dom: pointer to domain object * @disk: path to the block device, or device shorthand diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index dd94191..03206e7 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -725,4 +725,10 @@ LIBVIRT_1.2.19 { virDomainRename; } LIBVIRT_1.2.17;
+LIBVIRT_1.3.0 { + global: + virDomainGetPerfEvents; + virDomainSetPerfEvents; +} LIBVIRT_1.2.19;
This will need to be 1.3.1 next time you post this ACK with the minor fixes 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 :|

Add remote support for perf event. Signed-off-by: Qiaowei Ren <qiaowei.ren@intel.com> --- daemon/remote.c | 47 ++++++++++++++++++++++++++++++++++++++++++++ src/remote/remote_driver.c | 39 ++++++++++++++++++++++++++++++++++++ src/remote/remote_protocol.x | 30 +++++++++++++++++++++++++++- src/remote_protocol-structs | 18 +++++++++++++++++ 4 files changed, 133 insertions(+), 1 deletion(-) diff --git a/daemon/remote.c b/daemon/remote.c index 3a3eb09..9df29ef 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -2810,6 +2810,53 @@ remoteDispatchNodeGetMemoryStats(virNetServerPtr server ATTRIBUTE_UNUSED, } static int +remoteDispatchDomainGetPerfEvents(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client ATTRIBUTE_UNUSED, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr, + remote_domain_get_perf_events_args *args, + remote_domain_get_perf_events_ret *ret) +{ + virDomainPtr dom = NULL; + virTypedParameterPtr params = NULL; + int nparams = 0; + int rv = -1; + struct daemonClientPrivate *priv = + virNetServerClientGetPrivateData(client); + + if (!priv->conn) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); + goto cleanup; + } + + if (!(dom = get_nonnull_domain(priv->conn, args->dom))) + goto cleanup; + + if (virDomainGetPerfEvents(dom, ¶ms, &nparams) < 0) + goto cleanup; + + if (nparams > REMOTE_DOMAIN_MEMORY_PARAMETERS_MAX) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams too large")); + goto cleanup; + } + + if (remoteSerializeTypedParameters(params, nparams, + &ret->params.params_val, + &ret->params.params_len, + 0) < 0) + goto cleanup; + + rv = 0; + + cleanup: + if (rv < 0) + virNetMessageSaveError(rerr); + virTypedParamsFree(params, nparams); + virObjectUnref(dom); + return rv; +} + +static int remoteDispatchDomainGetBlockJobInfo(virNetServerPtr server ATTRIBUTE_UNUSED, virNetServerClientPtr client ATTRIBUTE_UNUSED, virNetMessagePtr msg ATTRIBUTE_UNUSED, diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index a1dd640..59caa62 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -2028,6 +2028,43 @@ remoteDomainGetNumaParameters(virDomainPtr domain, } static int +remoteDomainGetPerfEvents(virDomainPtr domain, + virTypedParameterPtr *params, + int *nparams) +{ + int rv = -1; + remote_domain_get_perf_events_args args; + remote_domain_get_perf_events_ret ret; + struct private_data *priv = domain->conn->privateData; + + remoteDriverLock(priv); + + make_nonnull_domain(&args.dom, domain); + + memset(&ret, 0, sizeof(ret)); + if (call(domain->conn, priv, 0, REMOTE_PROC_DOMAIN_GET_PERF_EVENTS, + (xdrproc_t) xdr_remote_domain_get_perf_events_args, (char *) &args, + (xdrproc_t) xdr_remote_domain_get_perf_events_ret, (char *) &ret) == -1) + goto done; + + if (remoteDeserializeTypedParameters(ret.params.params_val, + ret.params.params_len, + REMOTE_DOMAIN_PERF_EVENTS_MAX, + params, + nparams) < 0) + goto cleanup; + + rv = 0; + + cleanup: + xdr_free((xdrproc_t) xdr_remote_domain_get_perf_events_ret, + (char *) &ret); + done: + remoteDriverUnlock(priv); + return rv; +} + +static int remoteDomainGetBlkioParameters(virDomainPtr domain, virTypedParameterPtr params, int *nparams, unsigned int flags) @@ -8253,6 +8290,8 @@ static virHypervisorDriver hypervisor_driver = { .domainGetMemoryParameters = remoteDomainGetMemoryParameters, /* 0.8.5 */ .domainSetBlkioParameters = remoteDomainSetBlkioParameters, /* 0.9.0 */ .domainGetBlkioParameters = remoteDomainGetBlkioParameters, /* 0.9.0 */ + .domainGetPerfEvents = remoteDomainGetPerfEvents, /* 1.3.0 */ + .domainSetPerfEvents = remoteDomainSetPerfEvents, /* 1.3.0 */ .domainGetInfo = remoteDomainGetInfo, /* 0.3.0 */ .domainGetState = remoteDomainGetState, /* 0.9.2 */ .domainGetControlInfo = remoteDomainGetControlInfo, /* 0.9.3 */ diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 80f4a8b..3039958 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -127,6 +127,9 @@ const REMOTE_DOMAIN_BLOCK_IO_TUNE_PARAMETERS_MAX = 16; /* Upper limit on list of numa parameters. */ const REMOTE_DOMAIN_NUMA_PARAMETERS_MAX = 16; +/* Upper limit on list of perf events. */ +const REMOTE_DOMAIN_PERF_EVENTS_MAX = 64; + /* Upper limit on block copy tunable parameters. */ const REMOTE_DOMAIN_BLOCK_COPY_PARAMETERS_MAX = 16; @@ -646,6 +649,19 @@ struct remote_domain_get_numa_parameters_ret { int nparams; }; +struct remote_domain_set_perf_events_args { + remote_nonnull_domain dom; + remote_typed_param params<REMOTE_DOMAIN_PERF_EVENTS_MAX>; +}; + +struct remote_domain_get_perf_events_args { + remote_nonnull_domain dom; +}; + +struct remote_domain_get_perf_events_ret { + remote_typed_param params<REMOTE_DOMAIN_PERF_EVENTS_MAX>; +}; + struct remote_domain_block_stats_args { remote_nonnull_domain dom; remote_nonnull_string path; @@ -5712,5 +5728,17 @@ enum remote_procedure { * @acl: domain:write * @acl: domain:save */ - REMOTE_PROC_DOMAIN_RENAME = 358 + REMOTE_PROC_DOMAIN_RENAME = 358, + + /** + * @generate: none + * @acl: domain:read + */ + REMOTE_PROC_DOMAIN_GET_PERF_EVENTS = 359, + + /** + * @generate: both + * @acl: domain:write + */ + REMOTE_PROC_DOMAIN_SET_PERF_EVENTS = 360 }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index ff99c00..e93e7e8 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -304,6 +304,22 @@ struct remote_domain_get_numa_parameters_ret { } params; int nparams; }; +struct remote_domain_set_perf_events_args { + remote_nonnull_domain dom; + struct { + u_int params_len; + remote_typed_param * params_val; + } params; +}; +struct remote_domain_get_perf_events_args { + remote_nonnull_domain dom; +}; +struct remote_domain_get_perf_events_ret { + struct { + u_int params_len; + remote_typed_param * params_val; + } params; +}; struct remote_domain_block_stats_args { remote_nonnull_domain dom; remote_nonnull_string path; @@ -3051,4 +3067,6 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_DEL_IOTHREAD = 356, REMOTE_PROC_DOMAIN_SET_USER_PASSWORD = 357, REMOTE_PROC_DOMAIN_RENAME = 358, + REMOTE_PROC_DOMAIN_GET_PERF_EVENTS = 359, + REMOTE_PROC_DOMAIN_SET_PERF_EVENTS = 360, }; -- 1.9.1

On Mon, Dec 07, 2015 at 03:53:53PM +0800, Qiaowei Ren wrote:
Add remote support for perf event.
Signed-off-by: Qiaowei Ren <qiaowei.ren@intel.com> --- daemon/remote.c | 47 ++++++++++++++++++++++++++++++++++++++++++++ src/remote/remote_driver.c | 39 ++++++++++++++++++++++++++++++++++++ src/remote/remote_protocol.x | 30 +++++++++++++++++++++++++++- src/remote_protocol-structs | 18 +++++++++++++++++ 4 files changed, 133 insertions(+), 1 deletion(-)
remoteDomainGetBlkioParameters(virDomainPtr domain, virTypedParameterPtr params, int *nparams, unsigned int flags) @@ -8253,6 +8290,8 @@ static virHypervisorDriver hypervisor_driver = { .domainGetMemoryParameters = remoteDomainGetMemoryParameters, /* 0.8.5 */ .domainSetBlkioParameters = remoteDomainSetBlkioParameters, /* 0.9.0 */ .domainGetBlkioParameters = remoteDomainGetBlkioParameters, /* 0.9.0 */ + .domainGetPerfEvents = remoteDomainGetPerfEvents, /* 1.3.0 */ + .domainSetPerfEvents = remoteDomainSetPerfEvents, /* 1.3.0 */
Change to 1.3.1 next time you post ACK 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 :|

This patch implement a set of interfaces for perf event. Based on these interfaces, we can implement internal driver API for perf, and get the results of perf conuter you care about. Signed-off-by: Qiaowei Ren <qiaowei.ren@intel.com> --- include/libvirt/virterror.h | 1 + src/Makefile.am | 1 + src/libvirt_private.syms | 12 ++ src/util/virerror.c | 1 + src/util/virperf.c | 298 ++++++++++++++++++++++++++++++++++++++++++++ src/util/virperf.h | 61 +++++++++ 6 files changed, 374 insertions(+) create mode 100644 src/util/virperf.c create mode 100644 src/util/virperf.h diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h index 0539e48..7e87162 100644 --- a/include/libvirt/virterror.h +++ b/include/libvirt/virterror.h @@ -128,6 +128,7 @@ typedef enum { VIR_FROM_THREAD = 61, /* Error from thread utils */ VIR_FROM_ADMIN = 62, /* Error from admin backend */ VIR_FROM_LOGGING = 63, /* Error from log manager */ + VIR_FROM_PERF = 64, /* Error from perf */ # ifdef VIR_ENUM_SENTINELS VIR_ERR_DOMAIN_LAST diff --git a/src/Makefile.am b/src/Makefile.am index 7219f7c..26ac935 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -98,6 +98,7 @@ UTIL_SOURCES = \ util/virauthconfig.c util/virauthconfig.h \ util/virbitmap.c util/virbitmap.h \ util/virbuffer.c util/virbuffer.h \ + util/virperf.c util/virperf.h \ util/vircgroup.c util/vircgroup.h util/vircgrouppriv.h \ util/virclosecallbacks.c util/virclosecallbacks.h \ util/vircommand.c util/vircommand.h util/vircommandpriv.h \ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index dd085c3..5e64fa6 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2001,6 +2001,18 @@ virPCIGetVirtualFunctions; virPCIIsVirtualFunction; +# util/virperf.h +virPerfNew; +virPerfFree; +virPerfEventTypeFromString; +virPerfEventTypeToString; +virPerfEventEnable; +virPerfEventDisable; +virPerfEventIsEnabled; +virPerfGetEventFd; +virPerfGetCmtScale; + + # util/virpidfile.h virPidFileAcquire; virPidFileAcquirePath; diff --git a/src/util/virerror.c b/src/util/virerror.c index 098211a..7e6918d 100644 --- a/src/util/virerror.c +++ b/src/util/virerror.c @@ -135,6 +135,7 @@ VIR_ENUM_IMPL(virErrorDomain, VIR_ERR_DOMAIN_LAST, "Thread jobs", "Admin Interface", "Log Manager", + "Perf", ) diff --git a/src/util/virperf.c b/src/util/virperf.c new file mode 100644 index 0000000..9c40307 --- /dev/null +++ b/src/util/virperf.c @@ -0,0 +1,298 @@ +/* + * virperf.c: methods for managing perf events + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + * Authors: + * Ren Qiaowei <qiaowei.ren@intel.com> + */ +#include <config.h> + +#include <sys/ioctl.h> +#if defined HAVE_SYS_SYSCALL_H +# include <sys/syscall.h> +#endif + +#include "virperf.h" +#include "viralloc.h" +#include "virerror.h" +#include "virlog.h" +#include "virfile.h" +#include "virstring.h" +#include "virtypedparam.h" + +VIR_LOG_INIT("util.perf"); + +#define VIR_FROM_THIS VIR_FROM_PERF + +VIR_ENUM_IMPL(virPerfEvent, VIR_PERF_EVENT_LAST, + "cmt"); + +struct virPerfEvent { + int type; + int fd; + bool enabled; + union { + /* cmt */ + struct { + int scale; + } cmt; + } efields; +}; +typedef struct virPerfEvent *virPerfEventPtr; + +struct virPerf { + struct virPerfEvent events[VIR_PERF_EVENT_LAST]; +}; + +virPerfPtr +virPerfNew(void) +{ + size_t i; + virPerfPtr perf; + + if (VIR_ALLOC(perf) < 0) + return NULL; + + for (i = 0; i < VIR_PERF_EVENT_LAST; i++) { + perf->events[i].type = i; + perf->events[i].fd = -1; + perf->events[i].enabled = false; + } + + return perf; +} + +void +virPerfFree(virPerfPtr perf) +{ + size_t i; + + if (perf == NULL) + return; + + for (i = 0; i < VIR_PERF_EVENT_LAST; i++) { + if (perf->events[i].enabled) + virPerfEventDisable(perf, i); + } + + VIR_FREE(perf); +} + +#if defined(__linux__) && defined(HAVE_SYS_SYSCALL_H) + +# include <linux/perf_event.h> + +static virPerfEventPtr +virPerfGetEvent(virPerfPtr perf, + virPerfEventType type) +{ + if (type >= VIR_PERF_EVENT_LAST) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Event '%d' is not supported"), + type); + return NULL; + } + + return perf->events + type; +} + +static int +virPerfCmtEnable(virPerfEventPtr event, + pid_t pid) +{ + struct perf_event_attr cmt_attr; + int event_type; + FILE *fp = NULL; + FILE *fp_scale = NULL; + int scale = 0; + + fp = fopen("/sys/devices/intel_cqm/type", "r"); + if (!fp) { + virReportSystemError(errno, "%s", + _("CMT is not available on this host")); + return -1; + } + if (fscanf(fp, "%d", &event_type) != 1) { + virReportSystemError(errno, "%s", + _("Unable to read event type file.")); + goto cleanup; + } + + fp_scale = fopen("/sys/devices/intel_cqm/events/llc_occupancy.scale", "r"); + if (!fp_scale) { + virReportSystemError(errno, "%s", + _("Unable to open CMT scale file")); + goto cleanup; + } + if (fscanf(fp_scale, "%d", &scale) != 1) { + virReportSystemError(errno, "%s", + _("Unable to read CMT scale file")); + goto cleanup; + } + event->efields.cmt.scale = scale; + + memset(&cmt_attr, 0, sizeof(cmt_attr)); + cmt_attr.size = sizeof(cmt_attr); + cmt_attr.type = event_type; + cmt_attr.config = 1; + cmt_attr.inherit = 1; + cmt_attr.disabled = 1; + cmt_attr.enable_on_exec = 0; + + event->fd = syscall(__NR_perf_event_open, &cmt_attr, pid, -1, -1, 0); + if (event->fd < 0) { + virReportSystemError(errno, + _("Unable to open perf type=%d for pid=%d"), + event_type, pid); + goto cleanup; + } + + if (ioctl(event->fd, PERF_EVENT_IOC_ENABLE) < 0) { + virReportSystemError(errno, "%s", + _("Unable to enable perf event for CMT")); + goto cleanup; + } + + event->enabled = true; + return 0; + + cleanup: + if (fp) + VIR_FORCE_FCLOSE(fp); + if (fp_scale) + VIR_FORCE_FCLOSE(fp_scale); + if (event->fd >= 0) + VIR_FORCE_CLOSE(event->fd); + return -1; +} + +int +virPerfEventEnable(virPerfPtr perf, + virPerfEventType type, + pid_t pid) +{ + virPerfEventPtr event = virPerfGetEvent(perf, type); + if (event == NULL) + return -1; + + switch (type) { + case VIR_PERF_EVENT_CMT: + if (virPerfCmtEnable(event, pid)) + return -1; + break; + case VIR_PERF_EVENT_LAST: + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unexpected perf event type=%d"), type); + return -1; + } + + return 0; +} + +int +virPerfEventDisable(virPerfPtr perf, + virPerfEventType type) +{ + virPerfEventPtr event = virPerfGetEvent(perf, type); + if (event == NULL) + return -1; + + if (ioctl(event->fd, PERF_EVENT_IOC_DISABLE) < 0) { + virReportSystemError(errno, + _("Unable to disable perf event type=%d"), + event->type); + return -1; + } + + event->enabled = false; + VIR_FORCE_CLOSE(event->fd); + return 0; +} + +bool virPerfEventIsEnabled(virPerfPtr perf, + virPerfEventType type) +{ + virPerfEventPtr event = virPerfGetEvent(perf, type); + if (event == NULL) + return false; + + return event->enabled; +} + +int virPerfGetEventFd(virPerfPtr perf, + virPerfEventType type) +{ + virPerfEventPtr event = virPerfGetEvent(perf, type); + if (event == NULL) + return false; + + return event->fd; +} + +int +virPerfGetCmtScale(virPerfPtr perf) +{ + virPerfEventPtr event = virPerfGetEvent(perf, VIR_PERF_EVENT_CMT); + if (event == NULL || !event->enabled) + return -1; + + return event->efields.cmt.scale; +} + +#else +int +virPerfEventEnable(virPerfPtr perf ATTRIBUTE_UNUSED, + pid_t pid ATTRIBUTE_UNUSED) +{ + virReportSystemError(ENXIO, "%s", + _("Perf not supported on this platform")); + return -1; +} + +int +virPerfEventDisable(virPerfPtr perf ATTRIBUTE_UNUSED, + int event ATTRIBUTE_UNUSED) +{ + virReportSystemError(ENXIO, "%s", + _("Perf not supported on this platform")); + return -1; +} + +bool +virPerfEventIsEnabled(virPerfPtr perf, + virPerfEventType type) +{ + return false; +} + +int +virPerfGetEventFd(virPerfPtr perf, + virPerfEventType type) +{ + virReportSystemError(ENXIO, "%s", + _("Perf not supported on this platform")); + return -1; +} + +int +virPerfGetCmtScale(virPerfPtr perf) +{ + virReportSystemError(ENXIO, "%s", + _("Perf not supported on this platform")); + return -1; +} + +#endif diff --git a/src/util/virperf.h b/src/util/virperf.h new file mode 100644 index 0000000..ad60464 --- /dev/null +++ b/src/util/virperf.h @@ -0,0 +1,61 @@ +/* + * virperf.h: methods for managing perf events + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + * Authors: + * Ren Qiaowei <qiaowei.ren@intel.com> + */ + +#ifndef __VIR_PERF_H__ +# define __VIR_PERF_H__ + +# include "virutil.h" + +typedef enum { + VIR_PERF_EVENT_CMT, + + VIR_PERF_EVENT_LAST +} virPerfEventType; + +VIR_ENUM_DECL(virPerfEvent); + +# define VIR_PERF_PARAMETERS \ + VIR_PERF_PARAM_CMT, VIR_TYPED_PARAM_BOOLEAN, \ + NULL + +struct virPerf; +typedef struct virPerf *virPerfPtr; + +virPerfPtr virPerfNew(void); + +void virPerfFree(virPerfPtr perf); + +int virPerfEventEnable(virPerfPtr perf, + virPerfEventType type, + pid_t pid); + +int virPerfEventDisable(virPerfPtr perf, + virPerfEventType type); + +bool virPerfEventIsEnabled(virPerfPtr perf, + virPerfEventType type); + +int virPerfGetEventFd(virPerfPtr perf, + virPerfEventType type); + +int virPerfGetCmtScale(virPerfPtr perf); + +#endif /* __VIR_PERF_H__ */ -- 1.9.1

On Mon, Dec 07, 2015 at 03:53:54PM +0800, Qiaowei Ren wrote:
This patch implement a set of interfaces for perf event. Based on these interfaces, we can implement internal driver API for perf, and get the results of perf conuter you care about.
Signed-off-by: Qiaowei Ren <qiaowei.ren@intel.com> --- include/libvirt/virterror.h | 1 + src/Makefile.am | 1 + src/libvirt_private.syms | 12 ++ src/util/virerror.c | 1 + src/util/virperf.c | 298 ++++++++++++++++++++++++++++++++++++++++++++ src/util/virperf.h | 61 +++++++++ 6 files changed, 374 insertions(+) create mode 100644 src/util/virperf.c create mode 100644 src/util/virperf.h
+VIR_ENUM_IMPL(virPerfEvent, VIR_PERF_EVENT_LAST, + "cmt"); + +struct virPerfEvent { + int type; + int fd; + bool enabled; + union { + /* cmt */ + struct { + int scale; + } cmt; + } efields; +}; +typedef struct virPerfEvent *virPerfEventPtr; + +struct virPerf { + struct virPerfEvent events[VIR_PERF_EVENT_LAST]; +}; + +virPerfPtr +virPerfNew(void) +{ + size_t i; + virPerfPtr perf; + + if (VIR_ALLOC(perf) < 0) + return NULL; + + for (i = 0; i < VIR_PERF_EVENT_LAST; i++) { + perf->events[i].type = i; + perf->events[i].fd = -1; + perf->events[i].enabled = false; + } + + return perf; +} + +void +virPerfFree(virPerfPtr perf) +{ + size_t i; + + if (perf == NULL) + return; + + for (i = 0; i < VIR_PERF_EVENT_LAST; i++) { + if (perf->events[i].enabled) + virPerfEventDisable(perf, i); + } + + VIR_FREE(perf); +} + +#if defined(__linux__) && defined(HAVE_SYS_SYSCALL_H) + +# include <linux/perf_event.h> + +static virPerfEventPtr +virPerfGetEvent(virPerfPtr perf, + virPerfEventType type) +{ + if (type >= VIR_PERF_EVENT_LAST) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Event '%d' is not supported"), + type); + return NULL; + } + + return perf->events + type; +} + +static int +virPerfCmtEnable(virPerfEventPtr event, + pid_t pid) +{ + struct perf_event_attr cmt_attr; + int event_type; + FILE *fp = NULL; + FILE *fp_scale = NULL; + int scale = 0; + + fp = fopen("/sys/devices/intel_cqm/type", "r"); + if (!fp) { + virReportSystemError(errno, "%s", + _("CMT is not available on this host")); + return -1; + } + if (fscanf(fp, "%d", &event_type) != 1) { + virReportSystemError(errno, "%s", + _("Unable to read event type file.")); + goto cleanup; + }
Scanf has pretty poor error reporting, so we'd usually do char *buf; if (virFileReadAll("/path", 100, &buf) < 0) return -1; if (virStrToLong_ui(buf, NULL, 10, &scale) < 0) { VIR_FREE(buf); ....report error... return -1; } VIR_FREE(buf);
+ + fp_scale = fopen("/sys/devices/intel_cqm/events/llc_occupancy.scale", "r"); + if (!fp_scale) { + virReportSystemError(errno, "%s", + _("Unable to open CMT scale file")); + goto cleanup; + } + if (fscanf(fp_scale, "%d", &scale) != 1) { + virReportSystemError(errno, "%s", + _("Unable to read CMT scale file")); + goto cleanup; + }
Same comment here.
+ event->efields.cmt.scale = scale; + + memset(&cmt_attr, 0, sizeof(cmt_attr)); + cmt_attr.size = sizeof(cmt_attr); + cmt_attr.type = event_type; + cmt_attr.config = 1; + cmt_attr.inherit = 1; + cmt_attr.disabled = 1; + cmt_attr.enable_on_exec = 0; + + event->fd = syscall(__NR_perf_event_open, &cmt_attr, pid, -1, -1, 0); + if (event->fd < 0) { + virReportSystemError(errno, + _("Unable to open perf type=%d for pid=%d"), + event_type, pid); + goto cleanup; + } + + if (ioctl(event->fd, PERF_EVENT_IOC_ENABLE) < 0) { + virReportSystemError(errno, "%s", + _("Unable to enable perf event for CMT")); + goto cleanup; + } + + event->enabled = true; + return 0; + + cleanup: + if (fp) + VIR_FORCE_FCLOSE(fp); + if (fp_scale) + VIR_FORCE_FCLOSE(fp_scale); + if (event->fd >= 0) + VIR_FORCE_CLOSE(event->fd); + return -1;
+int +virPerfEventDisable(virPerfPtr perf, + virPerfEventType type) +{ + virPerfEventPtr event = virPerfGetEvent(perf, type); + if (event == NULL) + return -1; + + if (ioctl(event->fd, PERF_EVENT_IOC_DISABLE) < 0) { + virReportSystemError(errno, + _("Unable to disable perf event type=%d"), + event->type); + return -1; + }
Since we're closing the file handle next, is there any benefit in doing this ioctl() ?
+ + event->enabled = false; + VIR_FORCE_CLOSE(event->fd); + return 0; +}
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 :|

-----Original Message----- From: Daniel P. Berrange [mailto:berrange@redhat.com] Sent: Tuesday, December 8, 2015 6:54 PM To: Ren, Qiaowei Cc: libvir-list@redhat.com; Jiri Denemark Subject: Re: [PATCH v2 3/8] perf: implement a set of util functions for perf event
On Mon, Dec 07, 2015 at 03:53:54PM +0800, Qiaowei Ren wrote:
This patch implement a set of interfaces for perf event. Based on these interfaces, we can implement internal driver API for perf, and get the results of perf conuter you care about.
Signed-off-by: Qiaowei Ren <qiaowei.ren@intel.com> --- include/libvirt/virterror.h | 1 + src/Makefile.am | 1 + src/libvirt_private.syms | 12 ++ src/util/virerror.c | 1 + src/util/virperf.c | 298 ++++++++++++++++++++++++++++++++++++++++++++ src/util/virperf.h | 61 +++++++++ 6 files changed, 374 insertions(+) create mode 100644 src/util/virperf.c create mode 100644 src/util/virperf.h
+int +virPerfEventDisable(virPerfPtr perf, + virPerfEventType type) { + virPerfEventPtr event = virPerfGetEvent(perf, type); + if (event == NULL) + return -1; + + if (ioctl(event->fd, PERF_EVENT_IOC_DISABLE) < 0) { + virReportSystemError(errno, + _("Unable to disable perf event type=%d"), + event->type); + return -1; + }
Since we're closing the file handle next, is there any benefit in doing this ioctl() ?
I guess that VIR_FORCE_CLOSE() will just close the file handle, but the perf event have to be disabled before this. In fact it is from the example in man page of perf_event_open(). Thanks, Qiaowei

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.ren@intel.com> --- include/libvirt/libvirt-domain.h | 1 + src/qemu/qemu_domain.h | 3 + src/qemu/qemu_driver.c | 147 +++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_process.c | 7 ++ 4 files changed, 158 insertions(+) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index cc1b29b..4f29f54 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 14892fd..080498e 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" @@ -191,6 +192,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 ae1d8e7..53f5089 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" @@ -10249,6 +10250,86 @@ qemuDomainGetNumaParameters(virDomainPtr dom, } static int +qemuDomainSetPerfEvents(virDomainPtr dom, + virTypedParameterPtr params, + int nparams) +{ + size_t i; + virDomainObjPtr vm = NULL; + qemuDomainObjPrivatePtr priv; + int ret = -1; + virPerfEventType type; + bool enabled; + + if (virTypedParamsValidate(params, nparams, VIR_PERF_PARAMETERS) < 0) + return -1; + + if (!(vm = qemuDomObjFromDomain(dom))) + return -1; + + priv = vm->privateData; + + if (virDomainSetPerfEventsEnsureACL(dom->conn, vm->def) < 0) + goto cleanup; + + for (i = 0; i < nparams; i++) { + virTypedParameterPtr param = ¶ms[i]; + enabled = params->value.b; + type = virPerfEventTypeFromString(param->field); + + if (!enabled && virPerfEventDisable(priv->perf, type)) + goto cleanup; + if (enabled && virPerfEventEnable(priv->perf, type, vm->pid)) + goto cleanup; + } + + ret = 0; + + cleanup: + virDomainObjEndAPI(&vm); + return ret; +} + +static int +qemuDomainGetPerfEvents(virDomainPtr dom, + virTypedParameterPtr *params, + int *nparams) +{ + size_t i; + virDomainObjPtr vm = NULL; + qemuDomainObjPrivatePtr priv; + int ret = -1; + virTypedParameterPtr par = NULL; + int maxpar = 0; + int npar = 0; + + if (!(vm = qemuDomObjFromDomain(dom))) + goto cleanup; + + priv = vm->privateData; + + if (virDomainGetPerfEventsEnsureACL(dom->conn, vm->def) < 0) + goto cleanup; + + for (i = 0; i < VIR_PERF_EVENT_LAST; i++) { + if (virTypedParamsAddBoolean(&par, &npar, &maxpar, + virPerfEventTypeToString(i), + virPerfEventIsEnabled(priv->perf, i)) < 0) { + virTypedParamsFree(par, npar); + goto cleanup; + } + } + + *params = par; + *nparams = npar; + ret = 0; + + cleanup: + virDomainObjEndAPI(&vm); + return ret; +} + +static int qemuSetVcpusBWLive(virDomainObjPtr vm, virCgroupPtr cgroup, unsigned long long period, long long quota) { @@ -19427,6 +19508,69 @@ qemuDomainGetStatsBlock(virQEMUDriverPtr driver, #undef QEMU_ADD_COUNT_PARAM +static int +qemuDomainGetStatsPerfCmt(virPerfPtr perf, + virDomainStatsRecordPtr record, + int *maxparams) +{ + unsigned long long cache = 0; + int scaling_factor = 0; + int event_fd = virPerfGetEventFd(perf, VIR_PERF_EVENT_CMT); + + if (event_fd <= 0) + return -1; + + if (read(event_fd, &cache, sizeof(uint64_t)) < 0) { + virReportSystemError(errno, "%s", + _("Unable to read cache data")); + return -1; + } + + scaling_factor = virPerfGetCmtScale(perf); + if (scaling_factor <= 0) + return -1; + + cache *= scaling_factor; + + if (virTypedParamsAddULLong(&record->params, + &record->nparams, + maxparams, + "perf.cache", + cache) < 0) + return -1; + + return 0; +} + +static int +qemuDomainGetStatsPerf(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, + virDomainObjPtr dom, + virDomainStatsRecordPtr record, + int *maxparams, + unsigned int privflags ATTRIBUTE_UNUSED) +{ + size_t i; + qemuDomainObjPrivatePtr priv = dom->privateData; + int ret = -1; + + for (i = 0; i < VIR_PERF_EVENT_LAST; i++) { + if (!virPerfEventIsEnabled(priv->perf, i)) + continue; + + switch (i) { + case VIR_PERF_EVENT_CMT: + if (qemuDomainGetStatsPerfCmt(priv->perf, record, maxparams)) + goto cleanup; + break; + } + } + + ret = 0; + + cleanup: + return ret; +} + typedef int (*qemuDomainGetStatsFunc)(virQEMUDriverPtr driver, virDomainObjPtr dom, @@ -19447,6 +19591,7 @@ static struct qemuDomainGetStatsWorker qemuDomainGetStatsWorkers[] = { { qemuDomainGetStatsVcpu, VIR_DOMAIN_STATS_VCPU, false }, { qemuDomainGetStatsInterface, VIR_DOMAIN_STATS_INTERFACE, false }, { qemuDomainGetStatsBlock, VIR_DOMAIN_STATS_BLOCK, true }, + { qemuDomainGetStatsPerf, VIR_DOMAIN_STATS_PERF, false }, { NULL, 0, false } }; @@ -20207,6 +20352,8 @@ static virHypervisorDriver qemuHypervisorDriver = { .domainMigrateFinish3 = qemuDomainMigrateFinish3, /* 0.9.2 */ .domainMigrateConfirm3 = qemuDomainMigrateConfirm3, /* 0.9.2 */ .domainSendKey = qemuDomainSendKey, /* 0.9.4 */ + .domainGetPerfEvents = qemuDomainGetPerfEvents, /* 1.3.0 */ + .domainSetPerfEvents = qemuDomainSetPerfEvents, /* 1.3.0 */ .domainBlockJobAbort = qemuDomainBlockJobAbort, /* 0.9.4 */ .domainGetBlockJobInfo = qemuDomainGetBlockJobInfo, /* 0.9.4 */ .domainBlockJobSetSpeed = qemuDomainBlockJobSetSpeed, /* 0.9.4 */ diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 4201962..0ee8655 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4844,6 +4844,11 @@ qemuProcessLaunch(virConnectPtr conn, if (qemuSetupCgroup(driver, vm, nnicindexes, nicindexes) < 0) goto cleanup; + VIR_DEBUG("Initializing perf event"); + priv->perf = virPerfNew(); + if (!priv->perf) + goto cleanup; + /* This must be done after cgroup placement to avoid resetting CPU * affinity */ if (!vm->def->cputune.emulatorpin && @@ -5391,6 +5396,8 @@ void qemuProcessStop(virQEMUDriverPtr driver, } virCgroupFree(&priv->cgroup); + virPerfFree(priv->perf); + qemuProcessRemoveDomainStatus(driver, vm); /* Remove VNC and Spice ports from port reservation bitmap, but only if -- 1.9.1

On Mon, Dec 07, 2015 at 03:53:55PM +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.ren@intel.com> --- include/libvirt/libvirt-domain.h | 1 + src/qemu/qemu_domain.h | 3 + src/qemu/qemu_driver.c | 147 +++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_process.c | 7 ++ 4 files changed, 158 insertions(+)
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index cc1b29b..4f29f54 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 14892fd..080498e 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" @@ -191,6 +192,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 ae1d8e7..53f5089 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" @@ -10249,6 +10250,86 @@ qemuDomainGetNumaParameters(virDomainPtr dom, }
static int +qemuDomainSetPerfEvents(virDomainPtr dom, + virTypedParameterPtr params, + int nparams) +{ + size_t i; + virDomainObjPtr vm = NULL; + qemuDomainObjPrivatePtr priv; + int ret = -1; + virPerfEventType type; + bool enabled; + + if (virTypedParamsValidate(params, nparams, VIR_PERF_PARAMETERS) < 0) + return -1;
Nit-pick - indented one spec too far
+ + if (!(vm = qemuDomObjFromDomain(dom))) + return -1; + + priv = vm->privateData; + + if (virDomainSetPerfEventsEnsureACL(dom->conn, vm->def) < 0) + goto cleanup; + + for (i = 0; i < nparams; i++) { + virTypedParameterPtr param = ¶ms[i]; + enabled = params->value.b; + type = virPerfEventTypeFromString(param->field); + + if (!enabled && virPerfEventDisable(priv->perf, type)) + goto cleanup; + if (enabled && virPerfEventEnable(priv->perf, type, vm->pid)) + goto cleanup; + } + + ret = 0; + + cleanup: + virDomainObjEndAPI(&vm); + return ret; +} + +static int +qemuDomainGetPerfEvents(virDomainPtr dom, + virTypedParameterPtr *params, + int *nparams) +{ + size_t i; + virDomainObjPtr vm = NULL; + qemuDomainObjPrivatePtr priv; + int ret = -1; + virTypedParameterPtr par = NULL; + int maxpar = 0; + int npar = 0; + + if (!(vm = qemuDomObjFromDomain(dom))) + goto cleanup; + + priv = vm->privateData; + + if (virDomainGetPerfEventsEnsureACL(dom->conn, vm->def) < 0) + goto cleanup; + + for (i = 0; i < VIR_PERF_EVENT_LAST; i++) { + if (virTypedParamsAddBoolean(&par, &npar, &maxpar, + virPerfEventTypeToString(i), + virPerfEventIsEnabled(priv->perf, i)) < 0) { + virTypedParamsFree(par, npar); + goto cleanup; + } + } + + *params = par; + *nparams = npar; + ret = 0; + + cleanup: + virDomainObjEndAPI(&vm); + return ret; +} + +static int qemuSetVcpusBWLive(virDomainObjPtr vm, virCgroupPtr cgroup, unsigned long long period, long long quota) { @@ -19427,6 +19508,69 @@ qemuDomainGetStatsBlock(virQEMUDriverPtr driver,
#undef QEMU_ADD_COUNT_PARAM
+static int +qemuDomainGetStatsPerfCmt(virPerfPtr perf, + virDomainStatsRecordPtr record, + int *maxparams) +{ + unsigned long long cache = 0; + int scaling_factor = 0; + int event_fd = virPerfGetEventFd(perf, VIR_PERF_EVENT_CMT); + + if (event_fd <= 0) + return -1; + + if (read(event_fd, &cache, sizeof(uint64_t)) < 0) { + virReportSystemError(errno, "%s", + _("Unable to read cache data")); + return -1; + } + + scaling_factor = virPerfGetCmtScale(perf); + if (scaling_factor <= 0) + return -1; + + cache *= scaling_factor;
This feels like it is exposing impl detail from virPerf. I'd expect the virPerf API to support us doing virPerfReadEvent(perf, VIR_PERF_EVENT_CMT, &cache); and hide the scaling_factor & read(event_fd) code in virPerfReadEvent
+ + if (virTypedParamsAddULLong(&record->params, + &record->nparams, + maxparams, + "perf.cache", + cache) < 0) + return -1; + + return 0; +}
@@ -20207,6 +20352,8 @@ static virHypervisorDriver qemuHypervisorDriver = { .domainMigrateFinish3 = qemuDomainMigrateFinish3, /* 0.9.2 */ .domainMigrateConfirm3 = qemuDomainMigrateConfirm3, /* 0.9.2 */ .domainSendKey = qemuDomainSendKey, /* 0.9.4 */ + .domainGetPerfEvents = qemuDomainGetPerfEvents, /* 1.3.0 */ + .domainSetPerfEvents = qemuDomainSetPerfEvents, /* 1.3.0 */
1.3.1
+ VIR_DEBUG("Initializing perf event");
Best to put that debug line inside virPerfNew in fact
+ priv->perf = virPerfNew(); + if (!priv->perf) + goto cleanup; + /* This must be done after cgroup placement to avoid resetting CPU * affinity */ if (!vm->def->cputune.emulatorpin && @@ -5391,6 +5396,8 @@ void qemuProcessStop(virQEMUDriverPtr driver, } virCgroupFree(&priv->cgroup);
+ virPerfFree(priv->perf); + qemuProcessRemoveDomainStatus(driver, vm);
/* Remove VNC and Spice ports from port reservation bitmap, but only if
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 :|

This patch adds new xml element, and so we can have the option of also having perf events enabled immediately at startup. Signed-off-by: Qiaowei Ren <qiaowei.ren@intel.com> --- docs/schemas/domaincommon.rng | 27 +++++++++++++++++++++++++++ src/conf/domain_conf.c | 37 +++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 10 ++++++++++ src/qemu/qemu_driver.c | 26 ++++++++++++++++++++++++++ src/qemu/qemu_process.c | 6 ++++-- 5 files changed, 104 insertions(+), 2 deletions(-) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 4804c69..fb4bf2b 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -393,6 +393,33 @@ </define> <!-- + Enable or disable perf events for the domain. For each + of the events the following rules apply: + on: the event will be forcefully enabled + off: the event will be forcefully disabled + not specified: the event will be disabled by default + --> + <define name="perf"> + <element name="perf"> + <interleave> + <optional> + <element name="cmt"> + <ref name="enableChoices"/> + </element> + </optional> + </interleave> + <empty/> + </element> + </define> + <define name="enableChoices"> + <optional> + <attribute name="enabled"> + <ref name="virYesNo"/> + </attribute> + </optional> + </define> + + <!-- The Identifiers can be: - an optional id attribute with a number on the domain element - a mandatory name diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 2f5c0ed..833e69f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -12231,6 +12231,29 @@ virDomainPMStateParseXML(xmlXPathContextPtr ctxt, static int +virDomainPerfParseXML(xmlXPathContextPtr ctxt, + const char *xpath, + int *val) +{ + int ret = -1; + char *tmp = virXPathString(xpath, ctxt); + if (tmp) { + *val = virTristateBoolTypeFromString(tmp); + if (*val < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown perf event state %s"), tmp); + goto cleanup; + } + } + + ret = 0; + cleanup: + VIR_FREE(tmp); + return ret; +} + + +static int virDomainMemorySourceDefParseXML(xmlNodePtr node, xmlXPathContextPtr ctxt, virDomainMemoryDefPtr def) @@ -15388,6 +15411,11 @@ virDomainDefParseXML(xmlDocPtr xml, &def->pm.s4) < 0) goto error; + if (virDomainPerfParseXML(ctxt, + "string(./perf/cmt/@enabled)", + &def->perf.cmt) < 0) + goto error; + if ((tmp = virXPathString("string(./clock/@offset)", ctxt)) && (def->clock.offset = virDomainClockOffsetTypeFromString(tmp)) < 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, @@ -22011,6 +22039,15 @@ virDomainDefFormatInternal(virDomainDefPtr def, virBufferAddLit(buf, "</pm>\n"); } + if (def->perf.cmt) { + virBufferAddLit(buf, "<perf>\n"); + virBufferAdjustIndent(buf, 2); + virBufferAsprintf(buf, "<cmt enabled='%s'/>\n", + virTristateBoolTypeToString(def->perf.cmt)); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</perf>\n"); + } + virBufferAddLit(buf, "<devices>\n"); virBufferAdjustIndent(buf, 2); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 90d8e13..7383faf 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2192,6 +2192,14 @@ struct _virDomainPowerManagement { int s4; }; +typedef struct _virDomainPerf virDomainPerf; +typedef virDomainPerf *virDomainPerfPtr; + +struct _virDomainPerf { + /* These options are of type enum virTristateBool */ + int cmt; +}; + typedef struct _virDomainKeyWrapDef virDomainKeyWrapDef; typedef virDomainKeyWrapDef *virDomainKeyWrapDefPtr; struct _virDomainKeyWrapDef { @@ -2242,6 +2250,8 @@ struct _virDomainDef { virDomainPowerManagement pm; + virDomainPerf perf; + virDomainOSDef os; char *emulator; /* These three options are of type virTristateSwitch, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 53f5089..7f3c2a5 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -10254,9 +10254,14 @@ qemuDomainSetPerfEvents(virDomainPtr dom, virTypedParameterPtr params, int nparams) { + virQEMUDriverPtr driver = dom->conn->privateData; size_t i; virDomainObjPtr vm = NULL; + virQEMUDriverConfigPtr cfg = NULL; qemuDomainObjPrivatePtr priv; + virDomainDefPtr def; + virDomainDefPtr persistentDef; + unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT; int ret = -1; virPerfEventType type; bool enabled; @@ -10267,11 +10272,15 @@ qemuDomainSetPerfEvents(virDomainPtr dom, if (!(vm = qemuDomObjFromDomain(dom))) return -1; + cfg = virQEMUDriverGetConfig(driver); priv = vm->privateData; if (virDomainSetPerfEventsEnsureACL(dom->conn, vm->def) < 0) goto cleanup; + if (virDomainObjGetDefs(vm, flags, &def, &persistentDef) < 0) + goto cleanup; + for (i = 0; i < nparams; i++) { virTypedParameterPtr param = ¶ms[i]; enabled = params->value.b; @@ -10281,12 +10290,29 @@ qemuDomainSetPerfEvents(virDomainPtr dom, goto cleanup; if (enabled && virPerfEventEnable(priv->perf, type, vm->pid)) goto cleanup; + + if (def) { + def->perf.cmt = enabled ? + VIR_TRISTATE_BOOL_YES : VIR_TRISTATE_BOOL_NO; + + if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0) + goto cleanup; + } + + if (persistentDef) { + persistentDef->perf.cmt = enabled ? + VIR_TRISTATE_BOOL_YES : VIR_TRISTATE_BOOL_NO; + + if (virDomainSaveConfig(cfg->configDir, persistentDef) < 0) + goto cleanup; + } } ret = 0; cleanup: virDomainObjEndAPI(&vm); + virObjectUnref(cfg); return ret; } diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 0ee8655..45c16ac 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4846,8 +4846,10 @@ qemuProcessLaunch(virConnectPtr conn, VIR_DEBUG("Initializing perf event"); priv->perf = virPerfNew(); - if (!priv->perf) - goto cleanup; + if (priv->perf) { + if (vm->def->perf.cmt == VIR_TRISTATE_BOOL_YES) + virPerfEventEnable(priv->perf, VIR_PERF_EVENT_CMT, vm->pid); + } /* This must be done after cgroup placement to avoid resetting CPU * affinity */ -- 1.9.1

On Mon, Dec 07, 2015 at 03:53:56PM +0800, Qiaowei Ren wrote:
This patch adds new xml element, and so we can have the option of also having perf events enabled immediately at startup.
Signed-off-by: Qiaowei Ren <qiaowei.ren@intel.com> --- docs/schemas/domaincommon.rng | 27 +++++++++++++++++++++++++++ src/conf/domain_conf.c | 37 +++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 10 ++++++++++ src/qemu/qemu_driver.c | 26 ++++++++++++++++++++++++++ src/qemu/qemu_process.c | 6 ++++-- 5 files changed, 104 insertions(+), 2 deletions(-)
Needs an addition to the tests to exercise the XML round-trip parse+format
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 4804c69..fb4bf2b 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -393,6 +393,33 @@ </define>
<!-- + Enable or disable perf events for the domain. For each + of the events the following rules apply: + on: the event will be forcefully enabled + off: the event will be forcefully disabled + not specified: the event will be disabled by default + --> + <define name="perf"> + <element name="perf"> + <interleave> + <optional> + <element name="cmt"> + <ref name="enableChoices"/> + </element> + </optional> + </interleave> + <empty/> + </element> + </define> + <define name="enableChoices"> + <optional> + <attribute name="enabled"> + <ref name="virYesNo"/> + </attribute> + </optional> + </define> +
You have not used the 'perf' define anywhere, so this is currently orphaned. Presumably you're adding this at the top level of the <domain> XML. If you add an example XML file to the test suite then you'd see a test error about this.
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 2f5c0ed..833e69f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -12231,6 +12231,29 @@ virDomainPMStateParseXML(xmlXPathContextPtr ctxt,
static int +virDomainPerfParseXML(xmlXPathContextPtr ctxt, + const char *xpath, + int *val) +{ + int ret = -1; + char *tmp = virXPathString(xpath, ctxt); + if (tmp) { + *val = virTristateBoolTypeFromString(tmp); + if (*val < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown perf event state %s"), tmp); + goto cleanup; + } + } + + ret = 0; + cleanup: + VIR_FREE(tmp); + return ret; +} + + +static int virDomainMemorySourceDefParseXML(xmlNodePtr node, xmlXPathContextPtr ctxt, virDomainMemoryDefPtr def) @@ -15388,6 +15411,11 @@ virDomainDefParseXML(xmlDocPtr xml, &def->pm.s4) < 0) goto error;
+ if (virDomainPerfParseXML(ctxt, + "string(./perf/cmt/@enabled)", + &def->perf.cmt) < 0) + goto error; + if ((tmp = virXPathString("string(./clock/@offset)", ctxt)) && (def->clock.offset = virDomainClockOffsetTypeFromString(tmp)) < 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, @@ -22011,6 +22039,15 @@ virDomainDefFormatInternal(virDomainDefPtr def, virBufferAddLit(buf, "</pm>\n"); }
+ if (def->perf.cmt) { + virBufferAddLit(buf, "<perf>\n"); + virBufferAdjustIndent(buf, 2); + virBufferAsprintf(buf, "<cmt enabled='%s'/>\n", + virTristateBoolTypeToString(def->perf.cmt));
It might be slightly nicer to have <event name="cmt" enabled="%s"/> Also, we should probably have a VIR_ENUM defined for VIR_PERF_EVENT* types, so you can just do virPerfEventTypeToString() to get the value for the 'name' attribute. That would avoid us needing to extend the parsing code every time we add a new event
+ virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</perf>\n"); + } + virBufferAddLit(buf, "<devices>\n"); virBufferAdjustIndent(buf, 2);
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 90d8e13..7383faf 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2192,6 +2192,14 @@ struct _virDomainPowerManagement { int s4; };
+typedef struct _virDomainPerf virDomainPerf; +typedef virDomainPerf *virDomainPerfPtr; + +struct _virDomainPerf { + /* These options are of type enum virTristateBool */ + int cmt; +};
This should be an array with size VIR_PERF_EVENT_LAST really, so we don't hardcode 'cmt' in the parser. 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 :|

When libvirtd daemon restart, this patch will reenable those perf events previously enabled. Signed-off-by: Qiaowei Ren <qiaowei.ren@intel.com> --- src/qemu/qemu_driver.c | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 7f3c2a5..91aa79f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -164,6 +164,9 @@ static int qemuDomainGetMaxVcpus(virDomainPtr dom); static int qemuDomainManagedSaveLoad(virDomainObjPtr vm, void *opaque); +static int qemuDomainPerfRestart(virDomainObjPtr vm, + void *data); + static int qemuOpenFile(virQEMUDriverPtr driver, virDomainObjPtr vm, const char *path, int oflags, @@ -939,6 +942,10 @@ qemuStateInitialize(bool privileged, qemuDomainManagedSaveLoad, qemu_driver); + virDomainObjListForEach(qemu_driver->domains, + qemuDomainPerfRestart, + NULL); + qemuProcessReconnectAll(conn, qemu_driver); qemu_driver->workerPool = virThreadPoolNew(0, 1, 0, qemuProcessEventHandler, qemu_driver); @@ -3460,6 +3467,32 @@ qemuDomainManagedSaveLoad(virDomainObjPtr vm, return ret; } +static int +qemuDomainPerfRestart(virDomainObjPtr vm, + void *data ATTRIBUTE_UNUSED) +{ + virDomainDefPtr def = vm->def; + qemuDomainObjPrivatePtr priv = vm->privateData; + + if (priv->perf) + virPerfFree(priv->perf); + + priv->perf = virPerfNew(); + if (!priv->perf) + return -1; + + if (def->perf.cmt && def->perf.cmt == VIR_TRISTATE_BOOL_YES) { + if (virPerfEventEnable(priv->perf, VIR_PERF_EVENT_CMT, vm->pid)) + goto cleanup; + } + + return 0; + + cleanup: + virPerfFree(priv->perf); + return -1; +} + static int qemuDomainHasManagedSaveImage(virDomainPtr dom, unsigned int flags) -- 1.9.1

On Mon, Dec 07, 2015 at 03:53:57PM +0800, Qiaowei Ren wrote:
When libvirtd daemon restart, this patch will reenable those perf events previously enabled.
Signed-off-by: Qiaowei Ren <qiaowei.ren@intel.com> --- src/qemu/qemu_driver.c | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 7f3c2a5..91aa79f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -164,6 +164,9 @@ static int qemuDomainGetMaxVcpus(virDomainPtr dom); static int qemuDomainManagedSaveLoad(virDomainObjPtr vm, void *opaque);
+static int qemuDomainPerfRestart(virDomainObjPtr vm, + void *data); + static int qemuOpenFile(virQEMUDriverPtr driver, virDomainObjPtr vm, const char *path, int oflags, @@ -939,6 +942,10 @@ qemuStateInitialize(bool privileged, qemuDomainManagedSaveLoad, qemu_driver);
+ virDomainObjListForEach(qemu_driver->domains, + qemuDomainPerfRestart, + NULL); + qemuProcessReconnectAll(conn, qemu_driver);
qemu_driver->workerPool = virThreadPoolNew(0, 1, 0, qemuProcessEventHandler, qemu_driver); @@ -3460,6 +3467,32 @@ qemuDomainManagedSaveLoad(virDomainObjPtr vm, return ret; }
+static int +qemuDomainPerfRestart(virDomainObjPtr vm, + void *data ATTRIBUTE_UNUSED) +{ + virDomainDefPtr def = vm->def; + qemuDomainObjPrivatePtr priv = vm->privateData; + + if (priv->perf) + virPerfFree(priv->perf);
No need for the if() as virPerfFree accepts NULL
+ priv->perf = virPerfNew(); + if (!priv->perf) + return -1; + + if (def->perf.cmt && def->perf.cmt == VIR_TRISTATE_BOOL_YES) { + if (virPerfEventEnable(priv->perf, VIR_PERF_EVENT_CMT, vm->pid)) + goto cleanup; + }
If you change the XML as I suggest, you can do for (i = 0; i < VIR_PERF_EVENT_LAST; i++) { if (def->perf.events[i] && virPerfEventEnable(priv->perf, i, vm->pid)) goto cleanup; } Which again avoids hardcoding 'CMT' anywhere 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 :|

This patch add new perf command to enable/disable perf event for a guest domain. Signed-off-by: Qiaowei Ren <qiaowei.ren@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)") + }, + {.name = NULL} +}; + +static int +virshPerfGetEventEnabled(vshControl *ctl, const vshCmd *cmd, + const char *name, bool *value) +{ + int ret, tmp; + const char *str; + char *end; + + ret = vshCommandOptString(ctl, cmd, name, &str); + if (ret <= 0) + return ret; + if (virStrToLong_i(str, &end, 0, &tmp) < 0) + return -1; + if (tmp == 1) { + *value = true; + ret = 1; + } else if (tmp == 0) { + *value = false; + ret = 1; + } else { + ret = -1; + } + + return ret; +} + +static bool +cmdPerf(vshControl *ctl, const vshCmd *cmd) +{ + virDomainPtr dom; + bool tmpVal; + int nparams = 0; + int maxparams = 0; + int rc; + size_t i; + virTypedParameterPtr params = NULL; + bool ret = false; + + if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) + return false; + +#define PARSE_PERF_PARAM(NAME, FIELD) \ + if ((rc = virshPerfGetEventEnabled(ctl, cmd, NAME, &tmpVal)) < 0) { \ + vshError(ctl, _("Unable to parse boolean parameter %s"), NAME); \ + goto cleanup; \ + } \ + if (rc == 1) { \ + if (virTypedParamsAddBoolean(¶ms, &nparams, &maxparams, \ + FIELD, tmpVal) < 0) \ + goto save_error; \ + } \ + + + PARSE_PERF_PARAM("cmt", VIR_PERF_PARAM_CMT); + +#undef PARSE_PERF_PARAM + + if (nparams == 0) { + if (virDomainGetPerfEvents(dom, ¶ms, &nparams) != 0) { + vshError(ctl, "%s", _("Unable to get perf events")); + goto cleanup; + } + for (i = 0; i < nparams; i++) { + if (params[i].type == VIR_TYPED_PARAM_BOOLEAN && + params[i].value.b) { + vshPrint(ctl, "%-15s: %s\n", params[i].field, _("enabled")); + } else { + vshPrint(ctl, "%-15s: %s\n", params[i].field, _("disabled")); + } + } + } else { + if (virDomainSetPerfEvents(dom, params, nparams) != 0) + goto error; + } + + ret = true; + + cleanup: + virTypedParamsFree(params, nparams); + virDomainFree(dom); + return ret; + + save_error: + vshSaveLibvirtError(); + error: + vshError(ctl, "%s", _("Unable to enable/disable perf events")); + goto cleanup; +} + + +/* * "numatune" command */ static const vshCmdInfo info_numatune[] = { @@ -13406,6 +13525,12 @@ const vshCmdDef domManagementCmds[] = { .info = info_memtune, .flags = 0 }, + {.name = "perf", + .handler = cmdPerf, + .opts = opts_perf, + .info = info_perf, + .flags = 0 + }, {.name = "metadata", .handler = cmdMetadata, .opts = opts_metadata, diff --git a/tools/virsh.pod b/tools/virsh.pod index 21ae4a3..6ddf360 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -2113,6 +2113,24 @@ The guaranteed minimum memory allocation for the guest. Specifying -1 as a value for these limits is interpreted as unlimited. +=item B<perf> I<domain> [I<--cmt> B<enable-disable>] + +Get the current perf events setting or enable/disable specific perf +event for a guest domain. + +Perf is a performance analyzing tool in Linux, and it can instrument +CPU performance counters, tracepoints, kprobes, and uprobes (dynamic +tracing). Perf supports a list of measurable events, and can measure +events coming from different sources. For instance, some event are +pure kernel counters, in this case they are called software events, +including context-switches, minor-faults, etc.. Now dozens of events +from different sources can be supported by perf. + +Currently only QEMU/KVM supports I<--cmt>. CMT is a PQos (Platform Qos) +feature to monitor the usage of cache by applications running on the +platform. B<enable-disable> may be 0 or 1. 0 means cmt perf event will +be disabled, and 1 means cmt perf event will be enabled. + =item B<blkiotune> I<domain> [I<--weight> B<weight>] [I<--device-weights> B<device-weights>] [I<--device-read-iops-sec> B<device-read-iops-sec>] -- 1.9.1

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@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' 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 :|

-----Original Message----- From: Daniel P. Berrange [mailto:berrange@redhat.com] Sent: Tuesday, December 8, 2015 7:07 PM To: Ren, Qiaowei Cc: libvir-list@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@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? Thanks, Qiaowei

On Wed, Dec 09, 2015 at 01:28:22PM +0000, Ren, Qiaowei wrote:
-----Original Message----- From: Daniel P. Berrange [mailto:berrange@redhat.com] Sent: Tuesday, December 8, 2015 7:07 PM To: Ren, Qiaowei Cc: libvir-list@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@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 :|

This patch extend domstats command to match extended virDomainListGetStats API in previous patch. Signed-off-by: Qiaowei Ren <qiaowei.ren@intel.com> --- tools/virsh-domain-monitor.c | 7 +++++++ tools/virsh.pod | 7 +++++-- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c index 7dcf833..645936c 100644 --- a/tools/virsh-domain-monitor.c +++ b/tools/virsh-domain-monitor.c @@ -2017,6 +2017,10 @@ static const vshCmdOptDef opts_domstats[] = { .type = VSH_OT_BOOL, .help = N_("report domain block device statistics"), }, + {.name = "perf", + .type = VSH_OT_BOOL, + .help = N_("report domain perf event statistics"), + }, {.name = "list-active", .type = VSH_OT_BOOL, .help = N_("list only active domains"), @@ -2128,6 +2132,9 @@ cmdDomstats(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptBool(cmd, "block")) stats |= VIR_DOMAIN_STATS_BLOCK; + if (vshCommandOptBool(cmd, "perf")) + stats |= VIR_DOMAIN_STATS_PERF; + if (vshCommandOptBool(cmd, "list-active")) flags |= VIR_CONNECT_GET_ALL_DOMAINS_STATS_ACTIVE; diff --git a/tools/virsh.pod b/tools/virsh.pod index 6ddf360..2337a0d 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -846,7 +846,7 @@ or unique source names printed by this command. =item B<domstats> [I<--raw>] [I<--enforce>] [I<--backing>] [I<--state>] [I<--cpu-total>] [I<--balloon>] [I<--vcpu>] [I<--interface>] [I<--block>] -[[I<--list-active>] [I<--list-inactive>] [I<--list-persistent>] +[I<--perf>] [[I<--list-active>] [I<--list-inactive>] [I<--list-persistent>] [I<--list-transient>] [I<--list-running>] [I<--list-paused>] [I<--list-shutoff>] [I<--list-other>]] | [I<domain> ...] @@ -864,7 +864,7 @@ behavior use the I<--raw> flag. The individual statistics groups are selectable via specific flags. By default all supported statistics groups are returned. Supported statistics groups flags are: I<--state>, I<--cpu-total>, I<--balloon>, -I<--vcpu>, I<--interface>, I<--block>. +I<--vcpu>, I<--interface>, I<--block>, I<--perf>. When selecting the I<--state> group the following fields are returned: "state.state" - state of the VM, returned as number from virDomainState enum, @@ -899,6 +899,9 @@ I<--interface> returns: "net.<num>.tx.errs" - number of transmission errors, "net.<num>.tx.drop" - number of transmit packets dropped +I<--perf> returns the statistics of all enabled perf events: +"perf.cache" - the cache usage in Byte currently used + I<--block> returns information about disks associated with each domain. Using the I<--backing> flag extends this information to cover all resources in the backing chain, rather than the default -- 1.9.1
participants (3)
-
Daniel P. Berrange
-
Qiaowei Ren
-
Ren, Qiaowei