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

[Daniel P. Berrange]

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 at 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 = &params[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 :|