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

Re: [libvirt] [PATCH v4 4/4] cputune_event: queue the event for cputune updates




On 09/18/2014 04:39 AM, Pavel Hrdina wrote:
> Now we have universal tunable event so we can use it for reporting
> changes to user. The cputune values will be prefixed with "cputune" to
> distinguish it from other tunable events.
> 
> Signed-off-by: Pavel Hrdina <phrdina redhat com>
> ---
>  src/qemu/qemu_cgroup.c | 18 +++++++++++-
>  src/qemu/qemu_driver.c | 76 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 93 insertions(+), 1 deletion(-)
> 
> diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
> index 9d39370..27dcba3 100644
> --- a/src/qemu/qemu_cgroup.c
> +++ b/src/qemu/qemu_cgroup.c
> @@ -34,6 +34,7 @@
>  #include "virscsi.h"
>  #include "virstring.h"
>  #include "virfile.h"
> +#include "virtypedparam.h"
>  
>  #define VIR_FROM_THIS VIR_FROM_QEMU
>  
> @@ -676,6 +677,10 @@ static int
>  qemuSetupCpuCgroup(virDomainObjPtr vm)
>  {
>      qemuDomainObjPrivatePtr priv = vm->privateData;
> +    virObjectEventPtr event = NULL;
> +    virTypedParameterPtr eventParams;

Consistency with other defs "eventParams = NULL".

> +    int eventNparams = 0;
> +    int eventMaxparams = 0;
>  
>      if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU)) {
>         if (vm->def->cputune.sharesSpecified) {
> @@ -694,7 +699,18 @@ qemuSetupCpuCgroup(virDomainObjPtr vm)
>  
>          if (virCgroupGetCpuShares(priv->cgroup, &val) < 0)
>              return -1;
> -        vm->def->cputune.shares = val;
> +        if (vm->def->cputune.shares != val) {
> +            vm->def->cputune.shares = val;
> +            if (virTypedParamsAddULLong(&eventParams, &eventNparams,
> +                                        &eventMaxparams, "cputune.shares",

Perhaps this name space ('cputune.*') should be what goes into libvirt.h
(mentioned in the v2 3/5 review).

That is define and use

#define VIR_DOMAIN_EVENT_CPUTUNE_SHARES "cputune.shares"

> +                                        val) < 0)
> +                return -1;
> +
> +            event = virDomainEventTunableNewFromObj(vm, eventParams, eventNparams);
> +        }
> +
> +        if (event)
> +            qemuDomainEventQueue(vm->privateData, event);
>      }
>  
>      return 0;
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 5fd89a3..22c6e55 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -4538,6 +4538,12 @@ qemuDomainPinVcpuFlags(virDomainPtr dom,
>      virBitmapPtr pcpumap = NULL;
>      virQEMUDriverConfigPtr cfg = NULL;
>      virCapsPtr caps = NULL;
> +    virObjectEventPtr event = NULL;
> +    char paramField[VIR_TYPED_PARAM_FIELD_LENGTH] = "";
> +    char *str = NULL;
> +    virTypedParameterPtr eventParams = NULL;
> +    int eventNparams = 0;
> +    int eventMaxparams = 0;
>  
>      virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
>                    VIR_DOMAIN_AFFECT_CONFIG, -1);
> @@ -4645,6 +4651,18 @@ qemuDomainPinVcpuFlags(virDomainPtr dom,
>  
>          if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0)
>              goto cleanup;
> +
> +        if (snprintf(paramField, VIR_TYPED_PARAM_FIELD_LENGTH,
> +                     "cputune.vcpupin%d", vcpu) < 0) {

#define VIR_DOMAIN_EVENT_CPUTUNE_VCPUPIN "cputune.vcpupin%u"

BTW: vcpu is an unsigned...  Somehow have to comment that the event will
*start with* this string with the postfix being the vCPU number as
defined by the guest.

> +            goto cleanup;
> +        }
> +
> +        str = virBitmapFormat(pcpumap);
> +        if (virTypedParamsAddString(&eventParams, &eventNparams,
> +                                    &eventMaxparams, paramField, str) < 0)
> +            goto cleanup;
> +
> +        event = virDomainEventTunableNewFromDom(dom, eventParams, eventNparams);
>      }
>  
>      if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
> @@ -4680,6 +4698,9 @@ qemuDomainPinVcpuFlags(virDomainPtr dom,
>          virCgroupFree(&cgroup_vcpu);
>      if (vm)
>          virObjectUnlock(vm);
> +    if (event)
> +        qemuDomainEventQueue(driver, event);
> +    VIR_FREE(str);
>      virBitmapFree(pcpumap);
>      virObjectUnref(caps);
>      virObjectUnref(cfg);
> @@ -4804,6 +4825,12 @@ qemuDomainPinEmulator(virDomainPtr dom,
>      virBitmapPtr pcpumap = NULL;
>      virQEMUDriverConfigPtr cfg = NULL;
>      virCapsPtr caps = NULL;
> +    virObjectEventPtr event = NULL;
> +    char * str = NULL;
> +    virTypedParameterPtr eventParams = NULL;
> +    int eventNparams = 0;
> +    int eventMaxparams = 0;
> +
>  
>      virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
>                    VIR_DOMAIN_AFFECT_CONFIG, -1);
> @@ -4909,6 +4936,13 @@ qemuDomainPinEmulator(virDomainPtr dom,
>  
>          if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0)
>              goto cleanup;
> +
> +        str = virBitmapFormat(pcpumap);
> +        if (virTypedParamsAddString(&eventParams, &eventNparams,
> +                                    &eventMaxparams, "cputune.emulatorpin", str) < 0)

#define VIR_DOMAIN_EVENT_CPUTUNE_EMULATORPIN "cputune.emulatorpin"


> +            goto cleanup;
> +
> +        event = virDomainEventTunableNewFromDom(dom, eventParams, eventNparams);
>      }
>  
>      if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
> @@ -4938,6 +4972,9 @@ qemuDomainPinEmulator(virDomainPtr dom,
>   cleanup:
>      if (cgroup_emulator)
>          virCgroupFree(&cgroup_emulator);
> +    if (event)
> +        qemuDomainEventQueue(driver, event);
> +    VIR_FREE(str);
>      virBitmapFree(pcpumap);
>      virObjectUnref(caps);
>      if (vm)
> @@ -9096,6 +9133,10 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom,
>      virQEMUDriverConfigPtr cfg = NULL;
>      virCapsPtr caps = NULL;
>      qemuDomainObjPrivatePtr priv;
> +    virObjectEventPtr event = NULL;
> +    virTypedParameterPtr eventParams = NULL;
> +    int eventNparams = 0;
> +    int eventMaxNparams = 0;
>  
>      virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
>                    VIR_DOMAIN_AFFECT_CONFIG, -1);
> @@ -9166,6 +9207,11 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom,
>  
>                  vm->def->cputune.shares = val;
>                  vm->def->cputune.sharesSpecified = true;
> +
> +                if (virTypedParamsAddULLong(&eventParams, &eventNparams,
> +                                            &eventMaxNparams, "cputune.shares",

VIR_DOMAIN_EVENT_CPUTUNE_SHARES

> +                                            val) < 0)
> +                    goto cleanup;
>              }
>  
>              if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
> @@ -9183,6 +9229,11 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom,
>                      goto cleanup;
>  
>                  vm->def->cputune.period = value_ul;
> +
> +                if (virTypedParamsAddULLong(&eventParams, &eventNparams,
> +                                            &eventMaxNparams, "cputune.period",

#define VIR_DOMAIN_EVENT_CPUTUNE_VCPU_PERIOD "cputune.vcpu_period"

> +                                            value_ul) < 0)
> +                    goto cleanup;
>              }
>  
>              if (flags & VIR_DOMAIN_AFFECT_CONFIG)
> @@ -9197,6 +9248,11 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom,
>                      goto cleanup;
>  
>                  vm->def->cputune.quota = value_l;
> +
> +                if (virTypedParamsAddLLong(&eventParams, &eventNparams,
> +                                           &eventMaxNparams, "cputune.quota",

#define VIR_DOMAIN_EVENT_CPUTUNE_VCPU_QUOTA "cputune.vcpu_quota"

> +                                           value_l) < 0)
> +                    goto cleanup;
>              }
>  
>              if (flags & VIR_DOMAIN_AFFECT_CONFIG)
> @@ -9212,6 +9268,12 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom,
>                      goto cleanup;
>  
>                  vm->def->cputune.emulator_period = value_ul;
> +
> +                if (virTypedParamsAddULLong(&eventParams, &eventNparams,
> +                                            &eventMaxNparams,
> +                                            "cputune.emulator_period",

#define VIR_DOMAIN_EVENT_CPUTUNE_EMULATOR_PERIOD "cputune.emulator_period"

> +                                            value_ul) < 0)
> +                    goto cleanup;
>              }
>  
>              if (flags & VIR_DOMAIN_AFFECT_CONFIG)
> @@ -9227,6 +9289,12 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom,
>                      goto cleanup;
>  
>                  vm->def->cputune.emulator_quota = value_l;
> +
> +                if (virTypedParamsAddLLong(&eventParams, &eventNparams,
> +                                           &eventMaxNparams,
> +                                           "cputune.emulator_quota",

#define VIR_DOMAIN_EVENT_CPUTUNE_EMULATOR_QUOTA "cputune.emulator_quota"

> +                                           value_l) < 0)
> +                    goto cleanup;
>              }
>  
>              if (flags & VIR_DOMAIN_AFFECT_CONFIG)
> @@ -9237,6 +9305,12 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom,
>      if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0)
>          goto cleanup;
>  
> +    if (eventNparams) {
> +        event = virDomainEventTunableNewFromDom(dom, eventParams, eventNparams);

FWIW: This is the code that got me to thinking in 2/4 about the usage of
eventMaxNparams vs. just eventNparams.  They I believe are the same, but
can we "foresee" any reason they could/would differ.


In any case, I think with the usage of libvirt.h #define's this is ACK-able.

John

> +        eventNparams = 0;
> +        if (event)
> +            qemuDomainEventQueue(driver, event);
> +    }
>  
>      if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
>          rc = virDomainSaveConfig(cfg->configDir, vmdef);
> @@ -9253,6 +9327,8 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom,
>      virDomainDefFree(vmdef);
>      if (vm)
>          virObjectUnlock(vm);
> +    if (eventNparams)
> +        virTypedParamsFree(eventParams, eventNparams);
>      virObjectUnref(caps);
>      virObjectUnref(cfg);
>      return ret;
> 


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