[libvirt] [PATCH v3 0/4] Introduce new cputune event

This patch series introduce new cputune event to inform management applications about every change of cputune values for running domains. There is missing documentation for all events so the documentation for this event will be part of the patches to document all events. Changes from v2: - removed virSnprintf helper - fix typo and PM -> PowerManagement in the first patch - fixed memory leak - updated virConnectDomainEventCputuneCallback docs - virConnectDomainEventCputuneCallback now consumes params - updated call sequence in virConnectDomainEventCputuneCallback Pavel Hrdina (4): domain_conf: separate structures from virDomainDef event: introduce new event for cputune add an example how to use cputune event cputune_event: queue the event for cputune updates daemon/remote.c | 45 +++++++++++++++++ examples/object-events/event-test.c | 52 +++++++++++++++++++- include/libvirt/libvirt.h.in | 22 +++++++++ src/conf/domain_conf.h | 98 ++++++++++++++++++++++--------------- src/conf/domain_event.c | 93 +++++++++++++++++++++++++++++++++++ src/conf/domain_event.h | 9 ++++ src/libvirt_private.syms | 2 + src/qemu/qemu_cgroup.c | 18 ++++++- src/qemu/qemu_driver.c | 74 ++++++++++++++++++++++++++++ src/remote/remote_driver.c | 42 ++++++++++++++++ src/remote/remote_protocol.x | 14 +++++- src/remote_protocol-structs | 9 ++++ tools/virsh-domain.c | 33 +++++++++++++ 13 files changed, 469 insertions(+), 42 deletions(-) -- 1.8.5.5

Cleanup virDomanDef structure from other nested structure and create separate type definition for them. Fix a typo in virDomainHugePage. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.h | 98 ++++++++++++++++++++++++++++++-------------------- 1 file changed, 59 insertions(+), 39 deletions(-) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index efae2f5..548c568 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1886,14 +1886,67 @@ struct _virDomainResourceDef { char *partition; }; -typedef struct _virDomaiHugePage virDomainHugePage; +typedef struct _virDomainHugePage virDomainHugePage; typedef virDomainHugePage *virDomainHugePagePtr; -struct _virDomaiHugePage { +struct _virDomainHugePage { virBitmapPtr nodemask; /* guest's NUMA node mask */ unsigned long long size; /* hugepage size in KiB */ }; +typedef struct _virDomainCputune virDomainCputune; +typedef virDomainCputune *virDomainCputunePtr; + +struct _virDomainCputune { + unsigned long shares; + bool sharesSpecified; + unsigned long long period; + long long quota; + unsigned long long emulator_period; + long long emulator_quota; + size_t nvcpupin; + virDomainVcpuPinDefPtr *vcpupin; + virDomainVcpuPinDefPtr emulatorpin; +}; + +typedef struct _virDomainBlkiotune virDomainBlkiotune; +typedef virDomainBlkiotune *virDomainBlkiotunePtr; + +struct _virDomainBlkiotune { + unsigned int weight; + + size_t ndevices; + virBlkioDevicePtr devices; +}; + +typedef struct _virDomainMemtune virDomainMemtune; +typedef virDomainMemtune *virDomainMemtunePtr; + +struct _virDomainMemtune { + unsigned long long max_balloon; /* in kibibytes */ + unsigned long long cur_balloon; /* in kibibytes */ + + virDomainHugePagePtr hugepages; + size_t nhugepages; + + bool nosharepages; + bool locked; + int dump_core; /* enum virTristateSwitch */ + unsigned long long hard_limit; /* in kibibytes */ + unsigned long long soft_limit; /* in kibibytes */ + unsigned long long min_guarantee; /* in kibibytes */ + unsigned long long swap_hard_limit; /* in kibibytes */ +}; + +typedef struct _virDomainPowerManagement virDomainPowerManagement; +typedef virDomainPowerManagement *virDomainPowerManagementPtr; + +struct _virDomainPowerManagement { + /* These options are of type enum virTristateBool */ + int s3; + int s4; +}; + /* * Guest VM main configuration * @@ -1910,28 +1963,9 @@ struct _virDomainDef { char *title; char *description; - struct { - unsigned int weight; - - size_t ndevices; - virBlkioDevicePtr devices; - } blkio; + virDomainBlkiotune blkio; + virDomainMemtune mem; - struct { - unsigned long long max_balloon; /* in kibibytes */ - unsigned long long cur_balloon; /* in kibibytes */ - - virDomainHugePagePtr hugepages; - size_t nhugepages; - - bool nosharepages; - bool locked; - int dump_core; /* enum virTristateSwitch */ - unsigned long long hard_limit; /* in kibibytes */ - unsigned long long soft_limit; /* in kibibytes */ - unsigned long long min_guarantee; /* in kibibytes */ - unsigned long long swap_hard_limit; /* in kibibytes */ - } mem; unsigned short vcpus; unsigned short maxvcpus; int placement_mode; @@ -1939,17 +1973,7 @@ struct _virDomainDef { unsigned int iothreads; - struct { - unsigned long shares; - bool sharesSpecified; - unsigned long long period; - long long quota; - unsigned long long emulator_period; - long long emulator_quota; - size_t nvcpupin; - virDomainVcpuPinDefPtr *vcpupin; - virDomainVcpuPinDefPtr emulatorpin; - } cputune; + virDomainCputune cputune; virDomainNumatunePtr numatune; virDomainResourceDefPtr resource; @@ -1962,11 +1986,7 @@ struct _virDomainDef { int onLockFailure; /* enum virDomainLockFailureAction */ - struct { - /* These options are of type enum virTristateBool */ - int s3; - int s4; - } pm; + virDomainPowerManagement pm; virDomainOSDef os; char *emulator; -- 1.8.5.5

This new event will use typedParameters to expose what has been actually updated and the reason is that we can in the future extend the cputune. With typedParameters we don't have to worry about creating some sort of v2 of cputune event if there will be new features implemented for cputune. 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(-) diff --git a/daemon/remote.c b/daemon/remote.c index 0ea2815..798738e 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 +remoteRelayDomainEventCputune(virConnectPtr conn, + virDomainPtr dom, + virTypedParameterPtr params, + int nparams, + void *opaque) +{ + daemonClientEventCallbackPtr callback = opaque; + remote_domain_event_callback_cputune_msg data; + + if (callback->callbackID < 0 || + !remoteRelayDomainEventCheckACL(callback->client, conn, dom)) + return -1; + + VIR_DEBUG("Relaying domain cputune 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_CPUTUNE, + (xdrproc_t)xdr_remote_domain_event_callback_cputune_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(remoteRelayDomainEventCputune), }; verify(ARRAY_CARDINALITY(domainEventCallbacks) == VIR_DOMAIN_EVENT_ID_LAST); diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index c2f9d26..454acc8 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -5172,6 +5172,27 @@ typedef void (*virConnectDomainEventDeviceRemovedCallback)(virConnectPtr conn, const char *devAlias, void *opaque); +/** + * virConnectDomainEventCputuneCallback: + * @conn: connection object + * @dom: domain on which the event occurred + * @params: changed cpupin values stored as array of virTypedParameter + * @nparams: size of the array + * @opaque: application specified data + * + * This callback occurs when cpu tune is 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_CPUTUNE with virConnectDomainEventRegisterAny() + */ +typedef void (*virConnectDomainEventCputuneCallback)(virConnectPtr conn, + virDomainPtr dom, + virTypedParameterPtr params, + int nparams, + void *opaque); + /** * VIR_DOMAIN_EVENT_CALLBACK: @@ -5207,6 +5228,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_CPUTUNE = 17, /* virConnectDomainEventCputuneCallback */ #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..a84d791 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 virDomainEventCputuneClass; 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 virDomainEventCputuneDispose(void *obj); static void virDomainEventDispatchDefaultFunc(virConnectPtr conn, @@ -203,6 +206,15 @@ struct _virDomainQemuMonitorEvent { typedef struct _virDomainQemuMonitorEvent virDomainQemuMonitorEvent; typedef virDomainQemuMonitorEvent *virDomainQemuMonitorEventPtr; +struct _virDomainEventCputune { + virDomainEvent parent; + + virTypedParameterPtr params; + int nparams; +}; +typedef struct _virDomainEventCputune virDomainEventCputune; +typedef virDomainEventCputune *virDomainEventCputunePtr; + static int virDomainEventsOnceInit(void) @@ -285,6 +297,12 @@ virDomainEventsOnceInit(void) sizeof(virDomainQemuMonitorEvent), virDomainQemuMonitorEventDispose))) return -1; + if (!(virDomainEventCputuneClass = + virClassNew(virDomainEventClass, + "virDomainEventCputune", + sizeof(virDomainEventCputune), + virDomainEventCputuneDispose))) + return -1; return 0; } @@ -420,6 +438,15 @@ virDomainQemuMonitorEventDispose(void *obj) VIR_FREE(event->details); } +static void +virDomainEventCputuneDispose(void *obj) +{ + virDomainEventCputunePtr 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 +virDomainEventCputuneNew(int id, + const char *name, + unsigned char *uuid, + virTypedParameterPtr params, + int nparams) +{ + virDomainEventCputunePtr ev; + + if (virDomainEventsInitialize() < 0) + goto error; + + if (!(ev = virDomainEventNew(virDomainEventCputuneClass, + VIR_DOMAIN_EVENT_ID_CPUTUNE, + id, name, uuid))) + goto error; + + ev->params = params; + ev->nparams = nparams; + + return (virObjectEventPtr)ev; + + error: + virTypedParamsFree(params, nparams); + return NULL; +} + +virObjectEventPtr +virDomainEventCputuneNewFromObj(virDomainObjPtr obj, + virTypedParameterPtr params, + int nparams) +{ + return virDomainEventCputuneNew(obj->def->id, + obj->def->name, + obj->def->uuid, + params, + nparams); +} + +virObjectEventPtr +virDomainEventCputuneNewFromDom(virDomainPtr dom, + virTypedParameterPtr params, + int nparams) +{ + return virDomainEventCputuneNew(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_CPUTUNE: + { + virDomainEventCputunePtr cputuneEvent; + cputuneEvent = (virDomainEventCputunePtr)event; + ((virConnectDomainEventCputuneCallback)cb)(conn, dom, + cputuneEvent->params, + cputuneEvent->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..bf8c30e 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 +virDomainEventCputuneNewFromObj(virDomainObjPtr obj, + virTypedParameterPtr params, + int nparams); +virObjectEventPtr +virDomainEventCputuneNewFromDom(virDomainPtr dom, + virTypedParameterPtr params, + int nparams); + int virDomainEventStateRegister(virConnectPtr conn, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index f128b0c..e854234 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -443,6 +443,8 @@ virDomainEventBlockJobNewFromDom; virDomainEventBlockJobNewFromObj; virDomainEventControlErrorNewFromDom; virDomainEventControlErrorNewFromObj; +virDomainEventCputuneNewFromDom; +virDomainEventCputuneNewFromObj; virDomainEventDeviceRemovedNewFromDom; virDomainEventDeviceRemovedNewFromObj; virDomainEventDiskChangeNewFromDom; diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 8221683..efce6ad 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -326,6 +326,11 @@ remoteDomainBuildEventBlockJob2(virNetClientProgramPtr prog, void *evdata, void *opaque); static void +remoteDomainBuildEventCallbackCputune(virNetClientProgramPtr prog, + virNetClientPtr client, + void *evdata, void *opaque); + +static void remoteNetworkBuildEventLifecycle(virNetClientProgramPtr prog ATTRIBUTE_UNUSED, virNetClientPtr client ATTRIBUTE_UNUSED, void *evdata, void *opaque); @@ -476,6 +481,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_CPUTUNE, + remoteDomainBuildEventCallbackCputune, + sizeof(remote_domain_event_callback_cputune_msg), + (xdrproc_t)xdr_remote_domain_event_callback_cputune_msg }, }; @@ -5500,6 +5509,39 @@ remoteDomainBuildEventCallbackDeviceRemoved(virNetClientProgramPtr prog ATTRIBUT static void +remoteDomainBuildEventCallbackCputune(virNetClientProgramPtr prog ATTRIBUTE_UNUSED, + virNetClientPtr client ATTRIBUTE_UNUSED, + void *evdata, void *opaque) +{ + virConnectPtr conn = opaque; + remote_domain_event_callback_cputune_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 = virDomainEventCputuneNewFromDom(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..7e13822 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_cputune_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_CPUTUNE = 346 }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index e960415..d878874 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_cputune_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_CPUTUNE = 346, }; diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index f80741c..bd8981b 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -11414,6 +11414,37 @@ vshEventDeviceRemovedPrint(virConnectPtr conn ATTRIBUTE_UNUSED, vshEventDone(data->ctl); } +static void +vshEventCputunePrint(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 'cpu-tune' 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), }, @@ -11447,6 +11478,8 @@ static vshEventCallback vshEventCallbacks[] = { VIR_DOMAIN_EVENT_CALLBACK(vshEventDeviceRemovedPrint), }, { "block-job-2", VIR_DOMAIN_EVENT_CALLBACK(vshEventBlockJobPrint), }, + { "cpu-tune", + VIR_DOMAIN_EVENT_CALLBACK(vshEventCputunePrint), }, }; verify(VIR_DOMAIN_EVENT_ID_LAST == ARRAY_CARDINALITY(vshEventCallbacks)); -- 1.8.5.5

Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- examples/object-events/event-test.c | 52 ++++++++++++++++++++++++++++++++++++- 1 file changed, 51 insertions(+), 1 deletion(-) diff --git a/examples/object-events/event-test.c b/examples/object-events/event-test.c index d6cfe46..f1b9cda 100644 --- a/examples/object-events/event-test.c +++ b/examples/object-events/event-test.c @@ -464,6 +464,48 @@ static int myNetworkEventCallback(virConnectPtr conn ATTRIBUTE_UNUSED, return 0; } +static int +myDomainEventCputuneCallback(virConnectPtr conn ATTRIBUTE_UNUSED, + virDomainPtr dom, + virTypedParameterPtr params, + int nparams, + void *opaque ATTRIBUTE_UNUSED) +{ + size_t i; + + printf("%s EVENT: Domain %s(%d) cputune updated:\n", + __func__, virDomainGetName(dom), virDomainGetID(dom)); + + for (i = 0; i < nparams; i++) { + switch (params[i].type) { + case VIR_TYPED_PARAM_INT: + printf("\t%s: %d\n", params[i].field, params[i].value.i); + break; + case VIR_TYPED_PARAM_UINT: + printf("\t%s: %u\n", params[i].field, params[i].value.ui); + break; + case VIR_TYPED_PARAM_LLONG: + printf("\t%s: %lld\n", params[i].field, params[i].value.l); + break; + case VIR_TYPED_PARAM_ULLONG: + printf("\t%s: %llu\n", params[i].field, params[i].value.ul); + break; + case VIR_TYPED_PARAM_DOUBLE: + printf("\t%s: %g\n", params[i].field, params[i].value.d); + break; + case VIR_TYPED_PARAM_BOOLEAN: + printf("\t%s: %d\n", params[i].field, params[i].value.b); + break; + case VIR_TYPED_PARAM_STRING: + printf("\t%s: %s\n", params[i].field, params[i].value.s); + break; + default: + printf("\t%s: unknown type\n", params[i].field); + } + } + + return 0; +} static void myFreeFunc(void *opaque) { @@ -506,6 +548,7 @@ int main(int argc, char **argv) int callback14ret = -1; int callback15ret = -1; int callback16ret = -1; + int callback17ret = -1; struct sigaction action_stop; memset(&action_stop, 0, sizeof(action_stop)); @@ -624,6 +667,11 @@ int main(int argc, char **argv) VIR_NETWORK_EVENT_ID_LIFECYCLE, VIR_NETWORK_EVENT_CALLBACK(myNetworkEventCallback), strdup("net callback"), myFreeFunc); + callback17ret = virConnectDomainEventRegisterAny(dconn, + NULL, + VIR_DOMAIN_EVENT_ID_CPUTUNE, + VIR_DOMAIN_EVENT_CALLBACK(myDomainEventCputuneCallback), + strdup("cputune"), myFreeFunc); if ((callback1ret != -1) && (callback2ret != -1) && @@ -639,7 +687,8 @@ int main(int argc, char **argv) (callback13ret != -1) && (callback14ret != -1) && (callback15ret != -1) && - (callback16ret != -1)) { + (callback16ret != -1) && + (callback17ret != -1)) { if (virConnectSetKeepAlive(dconn, 5, 3) < 0) { virErrorPtr err = virGetLastError(); fprintf(stderr, "Failed to start keepalive protocol: %s\n", @@ -671,6 +720,7 @@ int main(int argc, char **argv) virConnectDomainEventDeregisterAny(dconn, callback14ret); virConnectDomainEventDeregisterAny(dconn, callback15ret); virConnectNetworkEventDeregisterAny(dconn, callback16ret); + virConnectDomainEventDeregisterAny(dconn, callback17ret); if (callback8ret != -1) virConnectDomainEventDeregisterAny(dconn, callback8ret); } -- 1.8.5.5

Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_cgroup.c | 18 +++++++++++- src/qemu/qemu_driver.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 91 insertions(+), 1 deletion(-) 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 73edda3..615ccf9 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4467,6 +4467,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); @@ -4574,6 +4580,18 @@ qemuDomainPinVcpuFlags(virDomainPtr dom, if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0) goto cleanup; + + if (snprintf(paramField, VIR_TYPED_PARAM_FIELD_LENGTH, + "vcpu%d", vcpu) < 0) { + goto cleanup; + } + + str = virBitmapFormat(pcpumap); + if (virTypedParamsAddString(&eventParams, &eventNparams, + &eventMaxparams, paramField, str) < 0) + goto cleanup; + + event = virDomainEventCputuneNewFromDom(dom, eventParams, eventNparams); } if (flags & VIR_DOMAIN_AFFECT_CONFIG) { @@ -4609,6 +4627,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); @@ -4733,6 +4754,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); @@ -4838,6 +4865,13 @@ 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; + + event = virDomainEventCputuneNewFromDom(dom, eventParams, eventNparams); } if (flags & VIR_DOMAIN_AFFECT_CONFIG) { @@ -4867,6 +4901,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) @@ -9079,6 +9116,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); @@ -9149,6 +9190,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) { @@ -9166,6 +9212,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) @@ -9180,6 +9231,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) @@ -9195,6 +9251,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) @@ -9210,6 +9271,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) @@ -9220,6 +9286,12 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom, if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0) goto cleanup; + if (eventNparams) { + event = virDomainEventCputuneNewFromDom(dom, eventParams, eventNparams); + eventNparams = 0; + if (event) + qemuDomainEventQueue(driver, event); + } if (flags & VIR_DOMAIN_AFFECT_CONFIG) { rc = virDomainSaveConfig(cfg->configDir, vmdef); @@ -9236,6 +9308,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 Mon, Sep 15, 2014 at 05:12:12PM +0200, Pavel Hrdina wrote:
This patch series introduce new cputune event to inform management applications about every change of cputune values for running domains.
There is missing documentation for all events so the documentation for this event will be part of the patches to document all events.
Do you have any background on the motivation for this feature ? It would help to understand the use case better in order to decide whether this is the right approach for the events. Specifically I am wondering whether returning all the values in the event is the best way. The alternative would be to have a generic "resource tunable changed" event where we just specify the type of data that changed, and allow the app to then fetch the new values if they actually want them. This would let us deal with all the resource tunables in a single event, isntead of having to add more events for memory tunables, numa tunables, disk I/O, net I/O etc. 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 :|

On 09/15/2014 05:16 PM, Daniel P. Berrange wrote:
On Mon, Sep 15, 2014 at 05:12:12PM +0200, Pavel Hrdina wrote:
This patch series introduce new cputune event to inform management applications about every change of cputune values for running domains.
There is missing documentation for all events so the documentation for this event will be part of the patches to document all events.
Do you have any background on the motivation for this feature ?
This feature is request from oVirt and they would also like to have event for blkdeviotune.
It would help to understand the use case better in order to decide whether this is the right approach for the events. Specifically I am wondering whether returning all the values in the event is the best way. The alternative would be to have a generic "resource tunable changed" event where we just specify the type of data that changed, and allow the app to then fetch the new values if they actually want them. This would let us deal with all the resource tunables in a single event, isntead of having to add more events for memory tunables, numa tunables, disk I/O, net I/O etc.
This event will return only the values that has been changed, not all values that we have for cputune. Having one "big" event for all tunables is a good idea and with the typedParameters it should be easy. Let's say that the event would be generic, then the typedParameter's field could be for cputune evetns: "cpu.shares" "cpu.emulatorpin" "cpu.vcpu0" or for example the blkiodevtune: "blkdevio.total_bytes_sec" etc... Pavel
Regards, Daniel

On Mon, Sep 15, 2014 at 05:43:30PM +0200, Pavel Hrdina wrote:
On 09/15/2014 05:16 PM, Daniel P. Berrange wrote:
On Mon, Sep 15, 2014 at 05:12:12PM +0200, Pavel Hrdina wrote:
This patch series introduce new cputune event to inform management applications about every change of cputune values for running domains.
There is missing documentation for all events so the documentation for this event will be part of the patches to document all events.
Do you have any background on the motivation for this feature ?
This feature is request from oVirt and they would also like to have event for blkdeviotune.
It would help to understand the use case better in order to decide whether this is the right approach for the events. Specifically I am wondering whether returning all the values in the event is the best way. The alternative would be to have a generic "resource tunable changed" event where we just specify the type of data that changed, and allow the app to then fetch the new values if they actually want them. This would let us deal with all the resource tunables in a single event, isntead of having to add more events for memory tunables, numa tunables, disk I/O, net I/O etc.
This event will return only the values that has been changed, not all values that we have for cputune.
Having one "big" event for all tunables is a good idea and with the typedParameters it should be easy. Let's say that the event would be generic, then the typedParameter's field could be for cputune evetns:
"cpu.shares" "cpu.emulatorpin" "cpu.vcpu0"
or for example the blkiodevtune:
"blkdevio.total_bytes_sec"
Yes, it seems like we ought to have an event with keys that are 100% identical to keys used for virConnectGetAllDomainStats. That said, we need to be careful what we guarantee here for events. For example, if someone uses systemd to change the cgroup settings behind libvirt's back we don't currently see that change in libvirt, so we won't emit any event in that scenario. So we should document that we only emit events for changes made via libvirt and will not issue events if apps go directly to cgroups or systemd 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 :|
participants (2)
-
Daniel P. Berrange
-
Pavel Hrdina