[libvirt] [PATCH v2 5/5] cputune_event: queue the event for cputune updates
John Ferlan
jferlan at redhat.com
Fri Sep 12 20:31:17 UTC 2014
s//"something to describe what's being done!"
Might be nice to see what the event would look like when it's triggered.
May help in that documentation effort in the future.
On 09/10/2014 08:08 AM, Pavel Hrdina wrote:
> Signed-off-by: Pavel Hrdina <phrdina at redhat.com>
> ---
> src/qemu/qemu_cgroup.c | 18 ++++++++++++-
> src/qemu/qemu_driver.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 90 insertions(+), 1 deletion(-)
>
So this is only going to occur as asked in 4/4 in v1 when/if the
cputune.shares value is adjusted during qemuProcessStart.
Also events are only triggered when someone uses the api that virsh
vcpupin, emulatorpin, and schedinfo(set) call. Which is "ok' by me for
the onlist iothreadpin changes which don't yet have a similar
iothreadpin api. Meaning my other series shouldn't be affected
> diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
> index 43d14d4..43b5340 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;
> + 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, "shares",
> + val) < 0)
> + return -1;
> +
> + event = virDomainEventCputuneNewFromObj(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 a8cda43..2389e32 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -4462,6 +4462,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);
> @@ -4569,6 +4575,18 @@ qemuDomainPinVcpuFlags(virDomainPtr dom,
>
> if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0)
> goto cleanup;
> +
> + if (virSnprintf(paramField, "vcpu%d", vcpu) < 0) {
As pointed out for 1/5 - this really could just be an snprintf()
> + goto cleanup;
> + }
The extra {} ^^^ and vvv are probably not necessary
> +
> + str = virBitmapFormat(pcpumap);
> + if (virTypedParamsAddString(&eventParams, &eventNparams,
> + &eventMaxparams, paramField, str) < 0) {
> + goto cleanup;
> + }
> +
> + event = virDomainEventCputuneNewFromDom(dom, eventParams, eventNparams);
> }
>
> if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
> @@ -4604,6 +4622,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);
> @@ -4728,6 +4749,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);
> @@ -4833,6 +4860,14 @@ qemuDomainPinEmulator(virDomainPtr dom,
>
> if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0)
> goto cleanup;
> +
> + str = virBitmapFormat(pcpumap);
> + if (virTypedParamsAddString(&eventParams, &eventNparams,
> + &eventMaxparams, "emulatorpin", str) < 0) {
> + goto cleanup;
> + }
ditto on the {}
> +
> + event = virDomainEventCputuneNewFromDom(dom, eventParams, eventNparams);
> }
>
> if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
> @@ -4862,6 +4897,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)
> @@ -9054,6 +9092,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);
> @@ -9124,6 +9166,11 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom,
>
> vm->def->cputune.shares = val;
> vm->def->cputune.sharesSpecified = true;
> +
> + if (virTypedParamsAddULLong(&eventParams, &eventNparams,
> + &eventMaxNparams, "shares",
> + val) < 0)
> + goto cleanup;
> }
>
> if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
> @@ -9141,6 +9188,11 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom,
> goto cleanup;
>
> vm->def->cputune.period = value_ul;
> +
> + if (virTypedParamsAddULLong(&eventParams, &eventNparams,
> + &eventMaxNparams, "period",
> + value_ul) < 0)
> + goto cleanup;
> }
>
> if (flags & VIR_DOMAIN_AFFECT_CONFIG)
> @@ -9155,6 +9207,11 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom,
> goto cleanup;
>
> vm->def->cputune.quota = value_l;
> +
> + if (virTypedParamsAddLLong(&eventParams, &eventNparams,
> + &eventMaxNparams, "quota",
> + value_l) < 0)
> + goto cleanup;
> }
>
> if (flags & VIR_DOMAIN_AFFECT_CONFIG)
> @@ -9170,6 +9227,11 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom,
> goto cleanup;
>
> vm->def->cputune.emulator_period = value_ul;
> +
> + if (virTypedParamsAddULLong(&eventParams, &eventNparams,
> + &eventMaxNparams, "emulator_period",
> + value_ul) < 0)
> + goto cleanup;
> }
>
> if (flags & VIR_DOMAIN_AFFECT_CONFIG)
> @@ -9185,6 +9247,11 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom,
> goto cleanup;
>
> vm->def->cputune.emulator_quota = value_l;
> +
> + if (virTypedParamsAddLLong(&eventParams, &eventNparams,
> + &eventMaxNparams, "emulator_quota",
> + value_l) < 0)
> + goto cleanup;
> }
>
> if (flags & VIR_DOMAIN_AFFECT_CONFIG)
> @@ -9205,12 +9272,18 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom,
> vmdef = NULL;
> }
>
> +
Extraneous line
ACK with the adjustments mentioned. I think you should just skip the
virSnprintf for this series and add one in another series to encompass
all callers (and of course add the 'rule' like Eric suggests in 1/5
John
> ret = 0;
>
> cleanup:
> virDomainDefFree(vmdef);
> if (vm)
> virObjectUnlock(vm);
> + if (eventNparams) {
> + event = virDomainEventCputuneNewFromDom(dom, eventParams, eventNparams);
> + if (event)
> + qemuDomainEventQueue(driver, event);
> + }
> virObjectUnref(caps);
> virObjectUnref(cfg);
> return ret;
>
More information about the libvir-list
mailing list