On 09/18/2014 04:39 AM, Pavel Hrdina wrote:

This new event will use typedParameters to expose what has been actually updated and the reason is that we can in the future extend any tunable values or add new tunable values. With typedParameters we don't have to worry about creating some other events, we will just use this universal event to inform user about updates.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- daemon/remote.c | 45 +++++++++++++++++++++ include/libvirt/libvirt.h.in | 22 +++++++++++ src/conf/domain_event.c | 93 ++++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_event.h | 9 +++++ src/libvirt_private.syms | 2 + src/remote/remote_driver.c | 42 ++++++++++++++++++++ src/remote/remote_protocol.x | 14 ++++++- src/remote_protocol-structs | 9 +++++ tools/virsh-domain.c | 33 ++++++++++++++++ 9 files changed, 268 insertions(+), 1 deletion(-)

Seems you covered previous code review comments just fine, although I have a couple of questions #1. Is virsh-domain.c relative here or perhaps later? Or should the verb/action be changed from "cpu-tune" to something more generic? "tunable"? #2. Should the REMOTE_CPUMAPS_MAX be replaced by something more name (and size) relative to these events? And secondarily, since event max params is relevant to the size of the params buffer allocated for each event series shouldn't that be passed along and eventually used as the limit for the remoteDeserializeTypedParameters? Although I'm not quite clear/sure how that works with the protocol definition file. I think this is ACK-able - just want to make sure of the above - #1 should be 'simple' right? #2 is just one of those size things - the current usage goes on a byte max, but is that relative to how this has changed/evolved. John

diff --git a/daemon/remote.c b/daemon/remote.c index daa4b60..ddd510c 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -111,6 +111,13 @@ remoteDeserializeTypedParameters(remote_typed_param *args_params_val, int *nparams);

static int +remoteSerializeTypedParameters(virTypedParameterPtr params, + int nparams, + remote_typed_param **ret_params_val, + u_int *ret_params_len, + unsigned int flags); + +static int remoteSerializeDomainDiskErrors(virDomainDiskErrorPtr errors, int nerrors, remote_domain_disk_error **ret_errors_val, @@ -969,6 +976,43 @@ remoteRelayDomainEventBlockJob2(virConnectPtr conn, }

+static int +remoteRelayDomainEventTunable(virConnectPtr conn, + virDomainPtr dom, + virTypedParameterPtr params, + int nparams, + void *opaque) +{ + daemonClientEventCallbackPtr callback = opaque; + remote_domain_event_callback_tunable_msg data; + + if (callback->callbackID < 0 || + !remoteRelayDomainEventCheckACL(callback->client, conn, dom)) + return -1; + + VIR_DEBUG("Relaying domain tunable event %s %d, callback %d", + dom->name, dom->id, callback->callbackID); + + /* build return data */ + memset(&data, 0, sizeof(data)); + data.callbackID = callback->callbackID; + make_nonnull_domain(&data.dom, dom); + + if (remoteSerializeTypedParameters(params, nparams, + &data.params.params_val, + &data.params.params_len, + VIR_TYPED_PARAM_STRING_OKAY) < 0) + return -1; + + remoteDispatchObjectEventSend(callback->client, remoteProgram, + REMOTE_PROC_DOMAIN_EVENT_CALLBACK_TUNABLE, + (xdrproc_t)xdr_remote_domain_event_callback_tunable_msg, + &data); + + return 0; +} + + static virConnectDomainEventGenericCallback domainEventCallbacks[] = { VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventLifecycle), VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventReboot), @@ -987,6 +1031,7 @@ static virConnectDomainEventGenericCallback domainEventCallbacks[] = { VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventPMSuspendDisk), VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventDeviceRemoved), VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventBlockJob2), + VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventTunable), };

verify(ARRAY_CARDINALITY(domainEventCallbacks) == VIR_DOMAIN_EVENT_ID_LAST); diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 724314e..4149596 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -5177,6 +5177,27 @@ typedef void (*virConnectDomainEventDeviceRemovedCallback)(virConnectPtr conn, const char *devAlias, void *opaque);

+/** + * virConnectDomainEventTunableCallback: + * @conn: connection object + * @dom: domain on which the event occurred + * @params: changed tunable values stored as array of virTypedParameter + * @nparams: size of the array + * @opaque: application specified data + * + * This callback occurs when tunable values are updated. The params must not + * be freed in the callback handler as it's done internally after the callback + * handler is executed. + * + * The callback signature to use when registering for an event of type + * VIR_DOMAIN_EVENT_ID_TUNABLE with virConnectDomainEventRegisterAny() + */ +typedef void (*virConnectDomainEventTunableCallback)(virConnectPtr conn, + virDomainPtr dom, + virTypedParameterPtr params, + int nparams, + void *opaque); +

/** * VIR_DOMAIN_EVENT_CALLBACK: @@ -5212,6 +5233,7 @@ typedef enum { VIR_DOMAIN_EVENT_ID_PMSUSPEND_DISK = 14, /* virConnectDomainEventPMSuspendDiskCallback */ VIR_DOMAIN_EVENT_ID_DEVICE_REMOVED = 15, /* virConnectDomainEventDeviceRemovedCallback */ VIR_DOMAIN_EVENT_ID_BLOCK_JOB_2 = 16, /* virConnectDomainEventBlockJobCallback */ + VIR_DOMAIN_EVENT_ID_TUNABLE = 17, /* virConnectDomainEventTunableCallback */

#ifdef VIR_ENUM_SENTINELS VIR_DOMAIN_EVENT_ID_LAST diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c index 73ae289..bf187cd 100644 --- a/src/conf/domain_event.c +++ b/src/conf/domain_event.c @@ -34,6 +34,7 @@ #include "viralloc.h" #include "virerror.h" #include "virstring.h" +#include "virtypedparam.h"

#define VIR_FROM_THIS VIR_FROM_NONE

@@ -52,6 +53,7 @@ static virClassPtr virDomainEventBalloonChangeClass; static virClassPtr virDomainEventDeviceRemovedClass; static virClassPtr virDomainEventPMClass; static virClassPtr virDomainQemuMonitorEventClass; +static virClassPtr virDomainEventTunableClass;

static void virDomainEventDispose(void *obj); @@ -67,6 +69,7 @@ static void virDomainEventBalloonChangeDispose(void *obj); static void virDomainEventDeviceRemovedDispose(void *obj); static void virDomainEventPMDispose(void *obj); static void virDomainQemuMonitorEventDispose(void *obj); +static void virDomainEventTunableDispose(void *obj);

static void virDomainEventDispatchDefaultFunc(virConnectPtr conn, @@ -203,6 +206,15 @@ struct _virDomainQemuMonitorEvent { typedef struct _virDomainQemuMonitorEvent virDomainQemuMonitorEvent; typedef virDomainQemuMonitorEvent *virDomainQemuMonitorEventPtr;

+struct _virDomainEventTunable { + virDomainEvent parent; + + virTypedParameterPtr params; + int nparams; +}; +typedef struct _virDomainEventTunable virDomainEventTunable; +typedef virDomainEventTunable *virDomainEventTunablePtr; +

static int virDomainEventsOnceInit(void) @@ -285,6 +297,12 @@ virDomainEventsOnceInit(void) sizeof(virDomainQemuMonitorEvent), virDomainQemuMonitorEventDispose))) return -1; + if (!(virDomainEventTunableClass = + virClassNew(virDomainEventClass, + "virDomainEventTunable", + sizeof(virDomainEventTunable), + virDomainEventTunableDispose))) + return -1; return 0; }

@@ -420,6 +438,15 @@ virDomainQemuMonitorEventDispose(void *obj) VIR_FREE(event->details); }

+static void +virDomainEventTunableDispose(void *obj) +{ + virDomainEventTunablePtr event = obj; + VIR_DEBUG("obj=%p", event); + + virTypedParamsFree(event->params, event->nparams); +} +

static void * virDomainEventNew(virClassPtr klass, @@ -1175,6 +1202,61 @@ virDomainEventDeviceRemovedNewFromDom(virDomainPtr dom, devAlias); }

+/* This function consumes the params so caller don't have to care about + * freeing it even if error occurs. The reason is to not have to do deep + * copy of params. + */ +static virObjectEventPtr +virDomainEventTunableNew(int id, + const char *name, + unsigned char *uuid, + virTypedParameterPtr params, + int nparams) +{ + virDomainEventTunablePtr ev; + + if (virDomainEventsInitialize() < 0) + goto error; + + if (!(ev = virDomainEventNew(virDomainEventTunableClass, + VIR_DOMAIN_EVENT_ID_TUNABLE, + id, name, uuid))) + goto error; + + ev->params = params; + ev->nparams = nparams; + + return (virObjectEventPtr)ev; + + error: + virTypedParamsFree(params, nparams); + return NULL; +} + +virObjectEventPtr +virDomainEventTunableNewFromObj(virDomainObjPtr obj, + virTypedParameterPtr params, + int nparams) +{ + return virDomainEventTunableNew(obj->def->id, + obj->def->name, + obj->def->uuid, + params, + nparams); +} + +virObjectEventPtr +virDomainEventTunableNewFromDom(virDomainPtr dom, + virTypedParameterPtr params, + int nparams) +{ + return virDomainEventTunableNew(dom->id, + dom->name, + dom->uuid, + params, + nparams); +} +

static void virDomainEventDispatchDefaultFunc(virConnectPtr conn, @@ -1366,6 +1448,17 @@ virDomainEventDispatchDefaultFunc(virConnectPtr conn, goto cleanup; }

+ case VIR_DOMAIN_EVENT_ID_TUNABLE: + { + virDomainEventTunablePtr tunableEvent; + tunableEvent = (virDomainEventTunablePtr)event; + ((virConnectDomainEventTunableCallback)cb)(conn, dom, + tunableEvent->params, + tunableEvent->nparams, + cbopaque); + goto cleanup; + } + case VIR_DOMAIN_EVENT_ID_LAST: break; } diff --git a/src/conf/domain_event.h b/src/conf/domain_event.h index a3330ca..dc0109c 100644 --- a/src/conf/domain_event.h +++ b/src/conf/domain_event.h @@ -184,6 +184,15 @@ virDomainEventDeviceRemovedNewFromObj(virDomainObjPtr obj, virObjectEventPtr virDomainEventDeviceRemovedNewFromDom(virDomainPtr dom, const char *devAlias); +virObjectEventPtr +virDomainEventTunableNewFromObj(virDomainObjPtr obj, + virTypedParameterPtr params, + int nparams); +virObjectEventPtr +virDomainEventTunableNewFromDom(virDomainPtr dom, + virTypedParameterPtr params, + int nparams); +

int virDomainEventStateRegister(virConnectPtr conn, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 51a692b..a339ced 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -473,6 +473,8 @@ virDomainEventStateRegister; virDomainEventStateRegisterID; virDomainEventTrayChangeNewFromDom; virDomainEventTrayChangeNewFromObj; +virDomainEventTunableNewFromDom; +virDomainEventTunableNewFromObj; virDomainEventWatchdogNewFromDom; virDomainEventWatchdogNewFromObj; virDomainQemuMonitorEventNew; diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 75a3a7b..79e3fd4 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -331,6 +331,11 @@ remoteDomainBuildEventBlockJob2(virNetClientProgramPtr prog, void *evdata, void *opaque);

static void +remoteDomainBuildEventCallbackTunable(virNetClientProgramPtr prog, + virNetClientPtr client, + void *evdata, void *opaque); + +static void remoteNetworkBuildEventLifecycle(virNetClientProgramPtr prog ATTRIBUTE_UNUSED, virNetClientPtr client ATTRIBUTE_UNUSED, void *evdata, void *opaque); @@ -481,6 +486,10 @@ static virNetClientProgramEvent remoteEvents[] = { remoteDomainBuildEventBlockJob2, sizeof(remote_domain_event_block_job_2_msg), (xdrproc_t)xdr_remote_domain_event_block_job_2_msg }, + { REMOTE_PROC_DOMAIN_EVENT_CALLBACK_TUNABLE, + remoteDomainBuildEventCallbackTunable, + sizeof(remote_domain_event_callback_tunable_msg), + (xdrproc_t)xdr_remote_domain_event_callback_tunable_msg }, };

@@ -5514,6 +5523,39 @@ remoteDomainBuildEventCallbackDeviceRemoved(virNetClientProgramPtr prog ATTRIBUT

static void +remoteDomainBuildEventCallbackTunable(virNetClientProgramPtr prog ATTRIBUTE_UNUSED, + virNetClientPtr client ATTRIBUTE_UNUSED, + void *evdata, void *opaque) +{ + virConnectPtr conn = opaque; + remote_domain_event_callback_tunable_msg *msg = evdata; + struct private_data *priv = conn->privateData; + virDomainPtr dom; + virTypedParameterPtr params = NULL; + int nparams = 0; + virObjectEventPtr event = NULL; + + if (remoteDeserializeTypedParameters(msg->params.params_val, + msg->params.params_len, + REMOTE_CPUMAPS_MAX, + ¶ms, &nparams) < 0) + return; + + dom = get_nonnull_domain(conn, msg->dom); + if (!dom) { + virTypedParamsFree(params, nparams); + return; + } + + event = virDomainEventTunableNewFromDom(dom, params, nparams); + + virDomainFree(dom); + + remoteEventQueue(priv, event, msg->callbackID); +} + + +static void remoteNetworkBuildEventLifecycle(virNetClientProgramPtr prog ATTRIBUTE_UNUSED, virNetClientPtr client ATTRIBUTE_UNUSED, void *evdata, void *opaque) diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index a4ca0c3..324d9e4 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -2990,6 +2990,12 @@ struct remote_domain_event_block_job_2_msg { int status; };

+struct remote_domain_event_callback_tunable_msg { + int callbackID; + remote_nonnull_domain dom; + remote_typed_param params<REMOTE_CPUMAPS_MAX>; +}; + struct remote_connect_get_cpu_model_names_args { remote_nonnull_string arch; int need_results; @@ -5472,5 +5478,11 @@ enum remote_procedure { * @generate: both * @acl: domain:block_write */ - REMOTE_PROC_DOMAIN_BLOCK_COPY = 345 + REMOTE_PROC_DOMAIN_BLOCK_COPY = 345, + + /** + * @generate: both + * @acl: none + */ + REMOTE_PROC_DOMAIN_EVENT_CALLBACK_TUNABLE = 346 }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index e960415..6128a85 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -2445,6 +2445,14 @@ struct remote_domain_event_block_job_2_msg { int type; int status; }; +struct remote_domain_event_callback_tunable_msg { + int callbackID; + remote_nonnull_domain dom; + struct { + u_int params_len; + remote_typed_param * params_val; + } params; +}; struct remote_connect_get_cpu_model_names_args { remote_nonnull_string arch; int need_results; @@ -2901,4 +2909,5 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_OPEN_GRAPHICS_FD = 343, REMOTE_PROC_CONNECT_GET_ALL_DOMAIN_STATS = 344, REMOTE_PROC_DOMAIN_BLOCK_COPY = 345, + REMOTE_PROC_DOMAIN_EVENT_CALLBACK_TUNABLE = 346, }; diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 435d045..cbf323a 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -11420,6 +11420,37 @@ vshEventDeviceRemovedPrint(virConnectPtr conn ATTRIBUTE_UNUSED, vshEventDone(data->ctl); }

+static void +vshEventTunablePrint(virConnectPtr conn ATTRIBUTE_UNUSED, + virDomainPtr dom, + virTypedParameterPtr params, + int nparams, + void *opaque) +{ + vshDomEventData *data = opaque; + size_t i; + char *value = NULL; + + if (!data->loop && *data->count) + return; + + vshPrint(data->ctl, + _("event 'tunable' for domain %s:\n"), + virDomainGetName(dom)); + + for (i = 0; i < nparams; i++) { + value = virTypedParameterToString(¶ms[i]); + if (value) { + vshPrint(data->ctl, _("\t%s: %s\n"), params[i].field, value); + VIR_FREE(value); + } + } + + (*data->count)++; + if (!data->loop) + vshEventDone(data->ctl); +} + static vshEventCallback vshEventCallbacks[] = { { "lifecycle", VIR_DOMAIN_EVENT_CALLBACK(vshEventLifecyclePrint), }, @@ -11453,6 +11484,8 @@ static vshEventCallback vshEventCallbacks[] = { VIR_DOMAIN_EVENT_CALLBACK(vshEventDeviceRemovedPrint), }, { "block-job-2", VIR_DOMAIN_EVENT_CALLBACK(vshEventBlockJobPrint), }, + { "cpu-tune", + VIR_DOMAIN_EVENT_CALLBACK(vshEventTunablePrint), }, }; verify(VIR_DOMAIN_EVENT_ID_LAST == ARRAY_CARDINALITY(vshEventCallbacks));

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> --- since v4: - added macro definitions for cputune typedParameters fileds include/libvirt/libvirt.h.in | 63 ++++++++++++++++++++++++++++++++++ src/qemu/qemu_cgroup.c | 19 ++++++++++- src/qemu/qemu_driver.c | 81 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 162 insertions(+), 1 deletion(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 86be86f..898f8b5 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -5204,6 +5204,66 @@ typedef void (*virConnectDomainEventDeviceRemovedCallback)(virConnectPtr conn, void *opaque); /** + * VIR_DOMAIN_EVENT_CPUTUNE_VCPUPIN: + * + * Macro represents formatted pinning for one vcpu specified by id which is + * appended to the parameter name, for example "cputune.vcpupin1", + * as VIR_TYPED_PARAM_STRING. + */ +#define VIR_DOMAIN_EVENT_CPUTUNE_VCPUPIN "cputune.vcpupin%u" + +/** + * VIR_DOMAIN_EVENT_CPUTUNE_EMULATORIN: + * + * Macro represents formatted pinning for emulator process, + * as VIR_TYPED_PARAM_STRING. + */ +#define VIR_DOMAIN_EVENT_CPUTUNE_EMULATORIN "cputune.emulatorpin" + +/** + * VIR_DOMAIN_EVENT_CPUTUNE_CPU_SHARES: + * + * Macro represents proportional weight of the scheduler used on the + * host cpu, when using the posix scheduler, as VIR_TYPED_PARAM_ULLONG. + */ +#define VIR_DOMAIN_EVENT_CPUTUNE_CPU_SHARES "cputune.cpu_shares" + +/** + * VIR_DOMAIN_EVENT_CPUTUNE_VCPU_PERIOD: + * + * Macro represents the enforcement period for a quota, in microseconds, + * for vcpus only, when using the posix scheduler, as VIR_TYPED_PARAM_ULLONG. + */ +#define VIR_DOMAIN_EVENT_CPUTUNE_VCPU_PERIOD "cputune.vcpu_period" + +/** + * VIR_DOMAIN_EVENT_CPUTUNE_VCPU_QUOTA: + * + * Macro represents the maximum bandwidth to be used within a period for + * vcpus only, when using the posix scheduler, as VIR_TYPED_PARAM_LLONG. + */ +#define VIR_DOMAIN_EVENT_CPUTUNE_VCPU_QUOTA "cputune.vcpu_quota" + +/** + * VIR_DOMAIN_EVENT_CPUTUNE_EMULATOR_PERIOD: + * + * Macro represents the enforcement period for a quota in microseconds, + * when using the posix scheduler, for all emulator activity not tied to + * vcpus, as VIR_TYPED_PARAM_ULLONG. + */ +#define VIR_DOMAIN_EVENT_CPUTUNE_EMULATOR_PERIOD "cputune.emulator_period" + +/** + * VIR_DOMAIN_EVENT_CPUTUNE_EMULATOR_QUOTA: + * + * Macro represents the maximum bandwidth to be used within a period for + * all emulator activity not tied to vcpus, when using the posix scheduler, + * as an VIR_TYPED_PARAM_LLONG. + */ +#define VIR_DOMAIN_EVENT_CPUTUNE_EMULATOR_QUOTA "cputune.emulator_quota" + + +/** * virConnectDomainEventTunableCallback: * @conn: connection object * @dom: domain on which the event occurred @@ -5215,6 +5275,9 @@ typedef void (*virConnectDomainEventDeviceRemovedCallback)(virConnectPtr conn, * be freed in the callback handler as it's done internally after the callback * handler is executed. * + * Currently supported name spaces: + * "cputune.*" + * * The callback signature to use when registering for an event of type * VIR_DOMAIN_EVENT_ID_TUNABLE with virConnectDomainEventRegisterAny() */ diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 7c6b2c1..41d7057 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 = 0; + int eventNparams = 0; + int eventMaxparams = 0; if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU)) { if (vm->def->cputune.sharesSpecified) { @@ -694,7 +699,19 @@ 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, + VIR_DOMAIN_EVENT_CPUTUNE_CPU_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 e73d4f9..d1a0657 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, + VIR_DOMAIN_EVENT_CPUTUNE_VCPUPIN, vcpu) < 0) { + 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,15 @@ qemuDomainPinEmulator(virDomainPtr dom, if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0) goto cleanup; + + str = virBitmapFormat(pcpumap); + if (virTypedParamsAddString(&eventParams, &eventNparams, + &eventMaxparams, + VIR_DOMAIN_EVENT_CPUTUNE_EMULATORIN, + str) < 0) + goto cleanup; + + event = virDomainEventTunableNewFromDom(dom, eventParams, eventNparams); } if (flags & VIR_DOMAIN_AFFECT_CONFIG) { @@ -4938,6 +4974,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) @@ -9202,6 +9241,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); @@ -9272,6 +9315,12 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom, vm->def->cputune.shares = val; vm->def->cputune.sharesSpecified = true; + + if (virTypedParamsAddULLong(&eventParams, &eventNparams, + &eventMaxNparams, + VIR_DOMAIN_EVENT_CPUTUNE_CPU_SHARES, + val) < 0) + goto cleanup; } if (flags & VIR_DOMAIN_AFFECT_CONFIG) { @@ -9289,6 +9338,12 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom, goto cleanup; vm->def->cputune.period = value_ul; + + if (virTypedParamsAddULLong(&eventParams, &eventNparams, + &eventMaxNparams, + VIR_DOMAIN_EVENT_CPUTUNE_VCPU_PERIOD, + value_ul) < 0) + goto cleanup; } if (flags & VIR_DOMAIN_AFFECT_CONFIG) @@ -9303,6 +9358,12 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom, goto cleanup; vm->def->cputune.quota = value_l; + + if (virTypedParamsAddLLong(&eventParams, &eventNparams, + &eventMaxNparams, + VIR_DOMAIN_EVENT_CPUTUNE_VCPU_QUOTA, + value_l) < 0) + goto cleanup; } if (flags & VIR_DOMAIN_AFFECT_CONFIG) @@ -9318,6 +9379,12 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom, goto cleanup; vm->def->cputune.emulator_period = value_ul; + + if (virTypedParamsAddULLong(&eventParams, &eventNparams, + &eventMaxNparams, + VIR_DOMAIN_EVENT_CPUTUNE_EMULATOR_PERIOD, + value_ul) < 0) + goto cleanup; } if (flags & VIR_DOMAIN_AFFECT_CONFIG) @@ -9333,6 +9400,12 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom, goto cleanup; vm->def->cputune.emulator_quota = value_l; + + if (virTypedParamsAddLLong(&eventParams, &eventNparams, + &eventMaxNparams, + VIR_DOMAIN_EVENT_CPUTUNE_EMULATOR_QUOTA, + value_l) < 0) + goto cleanup; } if (flags & VIR_DOMAIN_AFFECT_CONFIG) @@ -9343,6 +9416,12 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom, if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0) goto cleanup; + if (eventNparams) { + event = virDomainEventTunableNewFromDom(dom, eventParams, eventNparams); + eventNparams = 0; + if (event) + qemuDomainEventQueue(driver, event); + } if (flags & VIR_DOMAIN_AFFECT_CONFIG) { rc = virDomainSaveConfig(cfg->configDir, vmdef); @@ -9359,6 +9438,8 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom, virDomainDefFree(vmdef); if (vm) virObjectUnlock(vm); + if (eventNparams) + virTypedParamsFree(eventParams, eventNparams); virObjectUnref(caps); virObjectUnref(cfg); return ret; -- 1.8.5.5

On 09/23/2014 09:37 PM, John Ferlan wrote:

On 09/23/2014 02:47 PM, 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> ---

since v4: - added macro definitions for cputune typedParameters fileds

include/libvirt/libvirt.h.in | 63 ++++++++++++++++++++++++++++++++++ src/qemu/qemu_cgroup.c | 19 ++++++++++- src/qemu/qemu_driver.c | 81 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 162 insertions(+), 1 deletion(-)

diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 86be86f..898f8b5 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -5204,6 +5204,66 @@ typedef void (*virConnectDomainEventDeviceRemovedCallback)(virConnectPtr conn, void *opaque);

/** + * VIR_DOMAIN_EVENT_CPUTUNE_VCPUPIN: + * + * Macro represents formatted pinning for one vcpu specified by id which is + * appended to the parameter name, for example "cputune.vcpupin1", + * as VIR_TYPED_PARAM_STRING. + */ +#define VIR_DOMAIN_EVENT_CPUTUNE_VCPUPIN "cputune.vcpupin%u" + +/** + * VIR_DOMAIN_EVENT_CPUTUNE_EMULATORIN: + * + * Macro represents formatted pinning for emulator process, + * as VIR_TYPED_PARAM_STRING. + */ +#define VIR_DOMAIN_EVENT_CPUTUNE_EMULATORIN "cputune.emulatorpin" + +/** + * VIR_DOMAIN_EVENT_CPUTUNE_CPU_SHARES: + * + * Macro represents proportional weight of the scheduler used on the + * host cpu, when using the posix scheduler, as VIR_TYPED_PARAM_ULLONG. + */ +#define VIR_DOMAIN_EVENT_CPUTUNE_CPU_SHARES "cputune.cpu_shares" + +/** + * VIR_DOMAIN_EVENT_CPUTUNE_VCPU_PERIOD: + * + * Macro represents the enforcement period for a quota, in microseconds, + * for vcpus only, when using the posix scheduler, as VIR_TYPED_PARAM_ULLONG. + */ +#define VIR_DOMAIN_EVENT_CPUTUNE_VCPU_PERIOD "cputune.vcpu_period" + +/** + * VIR_DOMAIN_EVENT_CPUTUNE_VCPU_QUOTA: + * + * Macro represents the maximum bandwidth to be used within a period for + * vcpus only, when using the posix scheduler, as VIR_TYPED_PARAM_LLONG. + */ +#define VIR_DOMAIN_EVENT_CPUTUNE_VCPU_QUOTA "cputune.vcpu_quota" + +/** + * VIR_DOMAIN_EVENT_CPUTUNE_EMULATOR_PERIOD: + * + * Macro represents the enforcement period for a quota in microseconds, + * when using the posix scheduler, for all emulator activity not tied to + * vcpus, as VIR_TYPED_PARAM_ULLONG. + */ +#define VIR_DOMAIN_EVENT_CPUTUNE_EMULATOR_PERIOD "cputune.emulator_period" + +/** + * VIR_DOMAIN_EVENT_CPUTUNE_EMULATOR_QUOTA: + * + * Macro represents the maximum bandwidth to be used within a period for + * all emulator activity not tied to vcpus, when using the posix scheduler, + * as an VIR_TYPED_PARAM_LLONG. + */ +#define VIR_DOMAIN_EVENT_CPUTUNE_EMULATOR_QUOTA "cputune.emulator_quota" + + +/** * virConnectDomainEventTunableCallback: * @conn: connection object * @dom: domain on which the event occurred @@ -5215,6 +5275,9 @@ typedef void (*virConnectDomainEventDeviceRemovedCallback)(virConnectPtr conn, * be freed in the callback handler as it's done internally after the callback * handler is executed. * + * Currently supported name spaces: + * "cputune.*" + * * The callback signature to use when registering for an event of type * VIR_DOMAIN_EVENT_ID_TUNABLE with virConnectDomainEventRegisterAny() */ diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 7c6b2c1..41d7057 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 = 0;

s/0/NULL/

ACK w/ that.

And again thanks, pushed. Pavel

John

...

+ int eventNparams = 0; + int eventMaxparams = 0;

if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU)) { if (vm->def->cputune.sharesSpecified) { @@ -694,7 +699,19 @@ 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, + VIR_DOMAIN_EVENT_CPUTUNE_CPU_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 e73d4f9..d1a0657 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, + VIR_DOMAIN_EVENT_CPUTUNE_VCPUPIN, vcpu) < 0) { + 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,15 @@ qemuDomainPinEmulator(virDomainPtr dom,

if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0) goto cleanup; + + str = virBitmapFormat(pcpumap); + if (virTypedParamsAddString(&eventParams, &eventNparams, + &eventMaxparams, + VIR_DOMAIN_EVENT_CPUTUNE_EMULATORIN, + str) < 0) + goto cleanup; + + event = virDomainEventTunableNewFromDom(dom, eventParams, eventNparams); }

if (flags & VIR_DOMAIN_AFFECT_CONFIG) { @@ -4938,6 +4974,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) @@ -9202,6 +9241,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); @@ -9272,6 +9315,12 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom,

vm->def->cputune.shares = val; vm->def->cputune.sharesSpecified = true; + + if (virTypedParamsAddULLong(&eventParams, &eventNparams, + &eventMaxNparams, + VIR_DOMAIN_EVENT_CPUTUNE_CPU_SHARES, + val) < 0) + goto cleanup; }

if (flags & VIR_DOMAIN_AFFECT_CONFIG) { @@ -9289,6 +9338,12 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom, goto cleanup;

vm->def->cputune.period = value_ul; + + if (virTypedParamsAddULLong(&eventParams, &eventNparams, + &eventMaxNparams, + VIR_DOMAIN_EVENT_CPUTUNE_VCPU_PERIOD, + value_ul) < 0) + goto cleanup; }

if (flags & VIR_DOMAIN_AFFECT_CONFIG) @@ -9303,6 +9358,12 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom, goto cleanup;

vm->def->cputune.quota = value_l; + + if (virTypedParamsAddLLong(&eventParams, &eventNparams, + &eventMaxNparams, + VIR_DOMAIN_EVENT_CPUTUNE_VCPU_QUOTA, + value_l) < 0) + goto cleanup; }

if (flags & VIR_DOMAIN_AFFECT_CONFIG) @@ -9318,6 +9379,12 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom, goto cleanup;

vm->def->cputune.emulator_period = value_ul; + + if (virTypedParamsAddULLong(&eventParams, &eventNparams, + &eventMaxNparams, + VIR_DOMAIN_EVENT_CPUTUNE_EMULATOR_PERIOD, + value_ul) < 0) + goto cleanup; }

if (flags & VIR_DOMAIN_AFFECT_CONFIG) @@ -9333,6 +9400,12 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom, goto cleanup;

vm->def->cputune.emulator_quota = value_l; + + if (virTypedParamsAddLLong(&eventParams, &eventNparams, + &eventMaxNparams, + VIR_DOMAIN_EVENT_CPUTUNE_EMULATOR_QUOTA, + value_l) < 0) + goto cleanup; }

if (flags & VIR_DOMAIN_AFFECT_CONFIG) @@ -9343,6 +9416,12 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom, if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0) goto cleanup;

+ if (eventNparams) { + event = virDomainEventTunableNewFromDom(dom, eventParams, eventNparams); + eventNparams = 0; + if (event) + qemuDomainEventQueue(driver, event); + }

if (flags & VIR_DOMAIN_AFFECT_CONFIG) { rc = virDomainSaveConfig(cfg->configDir, vmdef); @@ -9359,6 +9438,8 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom, virDomainDefFree(vmdef); if (vm) virObjectUnlock(vm); + if (eventNparams) + virTypedParamsFree(eventParams, eventNparams); virObjectUnref(caps); virObjectUnref(cfg); return ret;

On 09/23/2014 09:36 PM, John Ferlan wrote:

On 09/23/2014 02:46 PM, Pavel Hrdina wrote:

...

This new event will use typedParameters to expose what has been actually updated and the reason is that we can in the future extend any tunable values or add new tunable values. With typedParameters we don't have to worry about creating some other events, we will just use this universal event to inform user about updates.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com> ---

since v4: - changed cpu-tune to tunable in virsh-domain.c - added REMOTE_DOMAIN_EVENT_TUNABLE_MAX to limit maximal length of tunable event msg

daemon/remote.c | 45 +++++++++++++++++++++ include/libvirt/libvirt.h.in | 22 +++++++++++ src/conf/domain_event.c | 93 ++++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_event.h | 9 +++++ src/libvirt_private.syms | 2 + src/remote/remote_driver.c | 42 ++++++++++++++++++++ src/remote/remote_protocol.x | 17 +++++++- src/remote_protocol-structs | 9 +++++ tools/virsh-domain.c | 33 ++++++++++++++++ 9 files changed, 271 insertions(+), 1 deletion(-)

diff --git a/daemon/remote.c b/daemon/remote.c index daa4b60..ddd510c 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -111,6 +111,13 @@ remoteDeserializeTypedParameters(remote_typed_param *args_params_val, int *nparams);

static int +remoteSerializeTypedParameters(virTypedParameterPtr params, + int nparams, + remote_typed_param **ret_params_val, + u_int *ret_params_len, + unsigned int flags); + +static int remoteSerializeDomainDiskErrors(virDomainDiskErrorPtr errors, int nerrors, remote_domain_disk_error **ret_errors_val, @@ -969,6 +976,43 @@ remoteRelayDomainEventBlockJob2(virConnectPtr conn, }

+static int +remoteRelayDomainEventTunable(virConnectPtr conn, + virDomainPtr dom, + virTypedParameterPtr params, + int nparams, + void *opaque) +{ + daemonClientEventCallbackPtr callback = opaque; + remote_domain_event_callback_tunable_msg data; + + if (callback->callbackID < 0 || + !remoteRelayDomainEventCheckACL(callback->client, conn, dom)) + return -1; + + VIR_DEBUG("Relaying domain tunable event %s %d, callback %d", + dom->name, dom->id, callback->callbackID); + + /* build return data */ + memset(&data, 0, sizeof(data)); + data.callbackID = callback->callbackID; + make_nonnull_domain(&data.dom, dom); + + if (remoteSerializeTypedParameters(params, nparams, + &data.params.params_val, + &data.params.params_len, + VIR_TYPED_PARAM_STRING_OKAY) < 0) + return -1; + + remoteDispatchObjectEventSend(callback->client, remoteProgram, + REMOTE_PROC_DOMAIN_EVENT_CALLBACK_TUNABLE, + (xdrproc_t)xdr_remote_domain_event_callback_tunable_msg, + &data); + + return 0; +} + + static virConnectDomainEventGenericCallback domainEventCallbacks[] = { VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventLifecycle), VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventReboot), @@ -987,6 +1031,7 @@ static virConnectDomainEventGenericCallback domainEventCallbacks[] = { VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventPMSuspendDisk), VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventDeviceRemoved), VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventBlockJob2), + VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventTunable), };

verify(ARRAY_CARDINALITY(domainEventCallbacks) == VIR_DOMAIN_EVENT_ID_LAST); diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 6371b7b..86be86f 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -5203,6 +5203,27 @@ typedef void (*virConnectDomainEventDeviceRemovedCallback)(virConnectPtr conn, const char *devAlias, void *opaque);

+/** + * virConnectDomainEventTunableCallback: + * @conn: connection object + * @dom: domain on which the event occurred + * @params: changed tunable values stored as array of virTypedParameter + * @nparams: size of the array + * @opaque: application specified data + * + * This callback occurs when tunable values are updated. The params must not + * be freed in the callback handler as it's done internally after the callback + * handler is executed. + * + * The callback signature to use when registering for an event of type + * VIR_DOMAIN_EVENT_ID_TUNABLE with virConnectDomainEventRegisterAny() + */ +typedef void (*virConnectDomainEventTunableCallback)(virConnectPtr conn, + virDomainPtr dom, + virTypedParameterPtr params, + int nparams, + void *opaque); +

/** * VIR_DOMAIN_EVENT_CALLBACK: @@ -5238,6 +5259,7 @@ typedef enum { VIR_DOMAIN_EVENT_ID_PMSUSPEND_DISK = 14, /* virConnectDomainEventPMSuspendDiskCallback */ VIR_DOMAIN_EVENT_ID_DEVICE_REMOVED = 15, /* virConnectDomainEventDeviceRemovedCallback */ VIR_DOMAIN_EVENT_ID_BLOCK_JOB_2 = 16, /* virConnectDomainEventBlockJobCallback */ + VIR_DOMAIN_EVENT_ID_TUNABLE = 17, /* virConnectDomainEventTunableCallback */

#ifdef VIR_ENUM_SENTINELS VIR_DOMAIN_EVENT_ID_LAST diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c index 73ae289..bf187cd 100644 --- a/src/conf/domain_event.c +++ b/src/conf/domain_event.c @@ -34,6 +34,7 @@ #include "viralloc.h" #include "virerror.h" #include "virstring.h" +#include "virtypedparam.h"

#define VIR_FROM_THIS VIR_FROM_NONE

@@ -52,6 +53,7 @@ static virClassPtr virDomainEventBalloonChangeClass; static virClassPtr virDomainEventDeviceRemovedClass; static virClassPtr virDomainEventPMClass; static virClassPtr virDomainQemuMonitorEventClass; +static virClassPtr virDomainEventTunableClass;

static void virDomainEventDispose(void *obj); @@ -67,6 +69,7 @@ static void virDomainEventBalloonChangeDispose(void *obj); static void virDomainEventDeviceRemovedDispose(void *obj); static void virDomainEventPMDispose(void *obj); static void virDomainQemuMonitorEventDispose(void *obj); +static void virDomainEventTunableDispose(void *obj);

static void virDomainEventDispatchDefaultFunc(virConnectPtr conn, @@ -203,6 +206,15 @@ struct _virDomainQemuMonitorEvent { typedef struct _virDomainQemuMonitorEvent virDomainQemuMonitorEvent; typedef virDomainQemuMonitorEvent *virDomainQemuMonitorEventPtr;

+struct _virDomainEventTunable { + virDomainEvent parent; + + virTypedParameterPtr params; + int nparams; +}; +typedef struct _virDomainEventTunable virDomainEventTunable; +typedef virDomainEventTunable *virDomainEventTunablePtr; +

static int virDomainEventsOnceInit(void) @@ -285,6 +297,12 @@ virDomainEventsOnceInit(void) sizeof(virDomainQemuMonitorEvent), virDomainQemuMonitorEventDispose))) return -1; + if (!(virDomainEventTunableClass = + virClassNew(virDomainEventClass, + "virDomainEventTunable", + sizeof(virDomainEventTunable), + virDomainEventTunableDispose))) + return -1; return 0; }

@@ -420,6 +438,15 @@ virDomainQemuMonitorEventDispose(void *obj) VIR_FREE(event->details); }

+static void +virDomainEventTunableDispose(void *obj) +{ + virDomainEventTunablePtr event = obj; + VIR_DEBUG("obj=%p", event); + + virTypedParamsFree(event->params, event->nparams); +} +

static void * virDomainEventNew(virClassPtr klass, @@ -1175,6 +1202,61 @@ virDomainEventDeviceRemovedNewFromDom(virDomainPtr dom, devAlias); }

+/* This function consumes the params so caller don't have to care about + * freeing it even if error occurs. The reason is to not have to do deep + * copy of params. + */ +static virObjectEventPtr +virDomainEventTunableNew(int id, + const char *name, + unsigned char *uuid, + virTypedParameterPtr params, + int nparams) +{ + virDomainEventTunablePtr ev; + + if (virDomainEventsInitialize() < 0) + goto error; + + if (!(ev = virDomainEventNew(virDomainEventTunableClass, + VIR_DOMAIN_EVENT_ID_TUNABLE, + id, name, uuid))) + goto error; + + ev->params = params; + ev->nparams = nparams; + + return (virObjectEventPtr)ev; + + error: + virTypedParamsFree(params, nparams); + return NULL; +} + +virObjectEventPtr +virDomainEventTunableNewFromObj(virDomainObjPtr obj, + virTypedParameterPtr params, + int nparams) +{ + return virDomainEventTunableNew(obj->def->id, + obj->def->name, + obj->def->uuid, + params, + nparams); +} + +virObjectEventPtr +virDomainEventTunableNewFromDom(virDomainPtr dom, + virTypedParameterPtr params, + int nparams) +{ + return virDomainEventTunableNew(dom->id, + dom->name, + dom->uuid, + params, + nparams); +} +

static void virDomainEventDispatchDefaultFunc(virConnectPtr conn, @@ -1366,6 +1448,17 @@ virDomainEventDispatchDefaultFunc(virConnectPtr conn, goto cleanup; }

+ case VIR_DOMAIN_EVENT_ID_TUNABLE: + { + virDomainEventTunablePtr tunableEvent; + tunableEvent = (virDomainEventTunablePtr)event; + ((virConnectDomainEventTunableCallback)cb)(conn, dom, + tunableEvent->params, + tunableEvent->nparams, + cbopaque); + goto cleanup; + } + case VIR_DOMAIN_EVENT_ID_LAST: break; } diff --git a/src/conf/domain_event.h b/src/conf/domain_event.h index a3330ca..dc0109c 100644 --- a/src/conf/domain_event.h +++ b/src/conf/domain_event.h @@ -184,6 +184,15 @@ virDomainEventDeviceRemovedNewFromObj(virDomainObjPtr obj, virObjectEventPtr virDomainEventDeviceRemovedNewFromDom(virDomainPtr dom, const char *devAlias); +virObjectEventPtr +virDomainEventTunableNewFromObj(virDomainObjPtr obj, + virTypedParameterPtr params, + int nparams); +virObjectEventPtr +virDomainEventTunableNewFromDom(virDomainPtr dom, + virTypedParameterPtr params, + int nparams); +

int virDomainEventStateRegister(virConnectPtr conn, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 51a692b..a339ced 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -473,6 +473,8 @@ virDomainEventStateRegister; virDomainEventStateRegisterID; virDomainEventTrayChangeNewFromDom; virDomainEventTrayChangeNewFromObj; +virDomainEventTunableNewFromDom; +virDomainEventTunableNewFromObj; virDomainEventWatchdogNewFromDom; virDomainEventWatchdogNewFromObj; virDomainQemuMonitorEventNew; diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 75a3a7b..79e3fd4 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -331,6 +331,11 @@ remoteDomainBuildEventBlockJob2(virNetClientProgramPtr prog, void *evdata, void *opaque);

static void +remoteDomainBuildEventCallbackTunable(virNetClientProgramPtr prog, + virNetClientPtr client, + void *evdata, void *opaque); + +static void remoteNetworkBuildEventLifecycle(virNetClientProgramPtr prog ATTRIBUTE_UNUSED, virNetClientPtr client ATTRIBUTE_UNUSED, void *evdata, void *opaque); @@ -481,6 +486,10 @@ static virNetClientProgramEvent remoteEvents[] = { remoteDomainBuildEventBlockJob2, sizeof(remote_domain_event_block_job_2_msg), (xdrproc_t)xdr_remote_domain_event_block_job_2_msg }, + { REMOTE_PROC_DOMAIN_EVENT_CALLBACK_TUNABLE, + remoteDomainBuildEventCallbackTunable, + sizeof(remote_domain_event_callback_tunable_msg), + (xdrproc_t)xdr_remote_domain_event_callback_tunable_msg }, };

@@ -5514,6 +5523,39 @@ remoteDomainBuildEventCallbackDeviceRemoved(virNetClientProgramPtr prog ATTRIBUT

static void +remoteDomainBuildEventCallbackTunable(virNetClientProgramPtr prog ATTRIBUTE_UNUSED, + virNetClientPtr client ATTRIBUTE_UNUSED, + void *evdata, void *opaque) +{ + virConnectPtr conn = opaque; + remote_domain_event_callback_tunable_msg *msg = evdata; + struct private_data *priv = conn->privateData; + virDomainPtr dom; + virTypedParameterPtr params = NULL; + int nparams = 0; + virObjectEventPtr event = NULL; + + if (remoteDeserializeTypedParameters(msg->params.params_val, + msg->params.params_len, + REMOTE_CPUMAPS_MAX,

s/REMOTE_CPUMAPS_MAX/REMOTE_DOMAIN_EVENT_TUNABLE_MAX

ACK w/ that.

Ouch, thanks for caching it. Pushed, Pavel

John

...

+ ¶ms, &nparams) < 0) + return; + + dom = get_nonnull_domain(conn, msg->dom); + if (!dom) { + virTypedParamsFree(params, nparams); + return; + } + + event = virDomainEventTunableNewFromDom(dom, params, nparams); + + virDomainFree(dom); + + remoteEventQueue(priv, event, msg->callbackID); +} + + +static void remoteNetworkBuildEventLifecycle(virNetClientProgramPtr prog ATTRIBUTE_UNUSED, virNetClientPtr client ATTRIBUTE_UNUSED, void *evdata, void *opaque) diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index a4ca0c3..0c6a91e 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -247,6 +247,9 @@ const REMOTE_NETWORK_DHCP_LEASES_MAX = 65536; /* Upper limit on count of parameters returned via bulk stats API */ const REMOTE_CONNECT_GET_ALL_DOMAIN_STATS_MAX = 4096;

+/* Upper limit of message size for tunable event. */ +const REMOTE_DOMAIN_EVENT_TUNABLE_MAX = 8388608; + /* UUID. VIR_UUID_BUFLEN definition comes from libvirt.h */ typedef opaque remote_uuid[VIR_UUID_BUFLEN];

@@ -2990,6 +2993,12 @@ struct remote_domain_event_block_job_2_msg { int status; };

+struct remote_domain_event_callback_tunable_msg { + int callbackID; + remote_nonnull_domain dom; + remote_typed_param params<REMOTE_DOMAIN_EVENT_TUNABLE_MAX>; +}; + struct remote_connect_get_cpu_model_names_args { remote_nonnull_string arch; int need_results; @@ -5472,5 +5481,11 @@ enum remote_procedure { * @generate: both * @acl: domain:block_write */ - REMOTE_PROC_DOMAIN_BLOCK_COPY = 345 + REMOTE_PROC_DOMAIN_BLOCK_COPY = 345, + + /** + * @generate: both + * @acl: none + */ + REMOTE_PROC_DOMAIN_EVENT_CALLBACK_TUNABLE = 346 }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index e960415..6128a85 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -2445,6 +2445,14 @@ struct remote_domain_event_block_job_2_msg { int type; int status; }; +struct remote_domain_event_callback_tunable_msg { + int callbackID; + remote_nonnull_domain dom; + struct { + u_int params_len; + remote_typed_param * params_val; + } params; +}; struct remote_connect_get_cpu_model_names_args { remote_nonnull_string arch; int need_results; @@ -2901,4 +2909,5 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_OPEN_GRAPHICS_FD = 343, REMOTE_PROC_CONNECT_GET_ALL_DOMAIN_STATS = 344, REMOTE_PROC_DOMAIN_BLOCK_COPY = 345, + REMOTE_PROC_DOMAIN_EVENT_CALLBACK_TUNABLE = 346, }; diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index a6ced5f..ce59406 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -11454,6 +11454,37 @@ vshEventDeviceRemovedPrint(virConnectPtr conn ATTRIBUTE_UNUSED, vshEventDone(data->ctl); }

+static void +vshEventTunablePrint(virConnectPtr conn ATTRIBUTE_UNUSED, + virDomainPtr dom, + virTypedParameterPtr params, + int nparams, + void *opaque) +{ + vshDomEventData *data = opaque; + size_t i; + char *value = NULL; + + if (!data->loop && *data->count) + return; + + vshPrint(data->ctl, + _("event 'tunable' for domain %s:\n"), + virDomainGetName(dom)); + + for (i = 0; i < nparams; i++) { + value = virTypedParameterToString(¶ms[i]); + if (value) { + vshPrint(data->ctl, _("\t%s: %s\n"), params[i].field, value); + VIR_FREE(value); + } + } + + (*data->count)++; + if (!data->loop) + vshEventDone(data->ctl); +} + static vshEventCallback vshEventCallbacks[] = { { "lifecycle", VIR_DOMAIN_EVENT_CALLBACK(vshEventLifecyclePrint), }, @@ -11487,6 +11518,8 @@ static vshEventCallback vshEventCallbacks[] = { VIR_DOMAIN_EVENT_CALLBACK(vshEventDeviceRemovedPrint), }, { "block-job-2", VIR_DOMAIN_EVENT_CALLBACK(vshEventBlockJobPrint), }, + { "tunable", + VIR_DOMAIN_EVENT_CALLBACK(vshEventTunablePrint), }, }; verify(VIR_DOMAIN_EVENT_ID_LAST == ARRAY_CARDINALITY(vshEventCallbacks));

On 09/23/2014 10:08 PM, Eric Blake wrote:

On 09/23/2014 12:46 PM, Pavel Hrdina wrote:

...

This new event will use typedParameters to expose what has been actually updated and the reason is that we can in the future extend any tunable values or add new tunable values. With typedParameters we don't have to worry about creating some other events, we will just use this universal event to inform user about updates.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com> ---

When the event is issued, does it contain ONLY parameters for what just changed, or does it list ALL tunables including the unchanged ones?

It feels like your API is only capable of listing the new tunable value. Is anyone using this event going to want to know both former and new value at the same time?

The API returns only the new tunable values. I've inspired myself with other APIs that they also returns only the updated values without the old ones. The only user that I know right now will be oVirt and they seems to be care only about new values.

...

+/** + * virConnectDomainEventTunableCallback: + * @conn: connection object + * @dom: domain on which the event occurred + * @params: changed tunable values stored as array of virTypedParameter + * @nparams: size of the array + * @opaque: application specified data + * + * This callback occurs when tunable values are updated. The params must not + * be freed in the callback handler as it's done internally after the callback + * handler is executed. + * + * The callback signature to use when registering for an event of type + * VIR_DOMAIN_EVENT_ID_TUNABLE with virConnectDomainEventRegisterAny() + */ +typedef void (*virConnectDomainEventTunableCallback)(virConnectPtr conn, + virDomainPtr dom, + virTypedParameterPtr params, + int nparams, + void *opaque);

Where do we document what names of tunables to expect?

It's documented in the 4/4 patch as I'm adding for now only cputune and it will be documented in the description of this callback as existing namespace and also there will be definitions for each tunable value that we will return.

...

+++ b/daemon/remote.c

...

+static int +remoteRelayDomainEventTunable(virConnectPtr conn, + virDomainPtr dom, + virTypedParameterPtr params, + int nparams, + void *opaque) +{ + daemonClientEventCallbackPtr callback = opaque; + remote_domain_event_callback_tunable_msg data; + + if (callback->callbackID < 0 || + !remoteRelayDomainEventCheckACL(callback->client, conn, dom)) + return -1; + + VIR_DEBUG("Relaying domain tunable event %s %d, callback %d", + dom->name, dom->id, callback->callbackID); +

Might also be nice to log "%p %n", params, nparams

Yes, that would be probably nice, but since I've pushed this patch already I can create a following patch with this small update?

...

+++ b/src/remote/remote_protocol.x @@ -247,6 +247,9 @@ const REMOTE_NETWORK_DHCP_LEASES_MAX = 65536; /* Upper limit on count of parameters returned via bulk stats API */ const REMOTE_CONNECT_GET_ALL_DOMAIN_STATS_MAX = 4096;

+/* Upper limit of message size for tunable event. */ +const REMOTE_DOMAIN_EVENT_TUNABLE_MAX = 8388608;

That feels excessive...

...

+ /* UUID. VIR_UUID_BUFLEN definition comes from libvirt.h */ typedef opaque remote_uuid[VIR_UUID_BUFLEN];

@@ -2990,6 +2993,12 @@ struct remote_domain_event_block_job_2_msg { int status; };

+struct remote_domain_event_callback_tunable_msg { + int callbackID; + remote_nonnull_domain dom; + remote_typed_param params<REMOTE_DOMAIN_EVENT_TUNABLE_MAX>;

...each param in the array will occupy multiple bytes. I think that something as low as 2048 for REMOTE_DOMAIN_EVENT_TUNABLE_MAX is still plenty (we don't have that many tunables yet); even if each tunable requires 64 bytes to transmit (mostly in the name of the parameter, but also in the type and value), that's still well under a megabyte limit of information passed on an instance of the event.

Well, yes and no :). Let's say, that we will add in the future (and I'm planning to do it) blkiotune where you can update at the same time all of the tunables for all host's disks where all params for now will be only VIR_TYPED_PARAM_STRING and that could consume a lot of memory. I know that it will probably never be that much, but I wanted to be sure that we will have enough space for all possible tunable events. Pavel

The code looks okay, but I'm still a bit worried about the design.

On 09/23/2014 11:50 PM, Eric Blake wrote:

On 09/23/2014 03:26 PM, Pavel Hrdina wrote:

...

...

...

+ + VIR_DEBUG("Relaying domain tunable event %s %d, callback %d", + dom->name, dom->id, callback->callbackID); +

Might also be nice to log "%p %n", params, nparams

Yes, that would be probably nice, but since I've pushed this patch already I can create a following patch with this small update?

Yes, a followup is fine.

...

...

...

+++ b/src/remote/remote_protocol.x @@ -247,6 +247,9 @@ const REMOTE_NETWORK_DHCP_LEASES_MAX = 65536; /* Upper limit on count of parameters returned via bulk stats API */ const REMOTE_CONNECT_GET_ALL_DOMAIN_STATS_MAX = 4096;

+/* Upper limit of message size for tunable event. */ +const REMOTE_DOMAIN_EVENT_TUNABLE_MAX = 8388608;

That feels excessive...

...

+ /* UUID. VIR_UUID_BUFLEN definition comes from libvirt.h */ typedef opaque remote_uuid[VIR_UUID_BUFLEN];

@@ -2990,6 +2993,12 @@ struct remote_domain_event_block_job_2_msg { int status; };

+struct remote_domain_event_callback_tunable_msg { + int callbackID; + remote_nonnull_domain dom; + remote_typed_param params<REMOTE_DOMAIN_EVENT_TUNABLE_MAX>;

...each param in the array will occupy multiple bytes. I think that something as low as 2048 for REMOTE_DOMAIN_EVENT_TUNABLE_MAX is still plenty (we don't have that many tunables yet); even if each tunable requires 64 bytes to transmit (mostly in the name of the parameter, but also in the type and value), that's still well under a megabyte limit of information passed on an instance of the event.

Well, yes and no :). Let's say, that we will add in the future (and I'm planning to do it) blkiotune where you can update at the same time all of the tunables for all host's disks where all params for now will be only VIR_TYPED_PARAM_STRING and that could consume a lot of memory. I know that it will probably never be that much, but I wanted to be sure that we will have enough space for all possible tunable events.

Still, are you going to return 8 million separate strings? Or just 8 million bytes but still contained within 2000 strings? Seriously, I think 2048 is a perfectly LARGE limit - there are not THAT many tunables per domain. The params<LIMIT> is not the overall size of the command, but the number of parameters (each of which can be quite large if they are type string)

Sigh, I should not work that late, because I've misunderstood the meaning of the "LIMIT". I'll post a new value with patch for the debug message. Thanks, Pavel

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;