On 08/28/2014 02:38 PM, Pavel Hrdina wrote:

Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- daemon/remote.c | 87 +++++++++++++++++++++++++++++++ include/libvirt/libvirt.h.in | 62 ++++++++++++++++++++++ src/conf/domain_event.c | 120 +++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_event.h | 7 +++ src/libvirt_private.syms | 2 + src/remote/remote_driver.c | 110 +++++++++++++++++++++++++++++++++++++++ src/remote/remote_protocol.x | 39 +++++++++++++- src/remote_protocol-structs | 32 ++++++++++++ tools/virsh-domain.c | 49 ++++++++++++++++++ 9 files changed, 507 insertions(+), 1 deletion(-)

Should it be noted in any documentation (docs/*.html.in) about the new event being triggered? My quick scan didn't find anything, but figured I'd ask to be sure.

diff --git a/daemon/remote.c b/daemon/remote.c index 89714ca..ae42c4d 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -969,6 +969,92 @@ remoteRelayDomainEventBlockJob2(virConnectPtr conn, }

+static int +remoteRelayDomainEventCputune(virConnectPtr conn, + virDomainPtr dom, + virDomainCputuneInfoPtr cputune, + void *opaque) +{ + daemonClientEventCallbackPtr callback = opaque; + remote_domain_event_cputune_msg data; + size_t i; + + 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)); + make_nonnull_domain(&data.dom, dom); + + data.shares = cputune->shares; + data.sharesSpecified = cputune->sharesSpecified; + data.period = cputune->period; + data.quota = cputune->quota; + data.emulatorPeriod = cputune->emulatorPeriod; + data.emulatorQuota = cputune->emulatorQuota; + data.nvcpupin = cputune->nvcpupin; + + if (cputune->emulatorpin.map) { + if (VIR_ALLOC_N(data.emulatorpin.map.map_val, + cputune->emulatorpin.mapLen) < 0) + goto error; + memcpy(data.emulatorpin.map.map_val, cputune->emulatorpin.map, + cputune->emulatorpin.mapLen); + data.emulatorpin.map.map_len = cputune->emulatorpin.mapLen; + data.emulatorpin.mapLen = cputune->emulatorpin.mapLen; + } + if (cputune->vcpupin) { + if (VIR_ALLOC_N(data.vcpupin.vcpupin_val, data.nvcpupin) < 0) + goto error; + data.vcpupin.vcpupin_len = data.nvcpupin; + + for (i = 0; i < data.nvcpupin; i++) { + data.vcpupin.vcpupin_val[i].vcpuid = cputune->vcpupin[i].vcpuid; + if (VIR_ALLOC_N(data.vcpupin.vcpupin_val[i].cpumask.map.map_val, + cputune->vcpupin[i].cpumask.mapLen) < 0) + goto error; + memcpy(data.vcpupin.vcpupin_val[i].cpumask.map.map_val, + cputune->vcpupin[i].cpumask.map, + cputune->vcpupin[i].cpumask.mapLen);

Was this more or less what the unused VIR_COPY_CPUMAP was supposed to be? Nothing you created, but something I found during my recent look into the code.

+ data.vcpupin.vcpupin_val[i].cpumask.map.map_len = + cputune->vcpupin[i].cpumask.mapLen; + data.vcpupin.vcpupin_val[i].cpumask.mapLen = + cputune->vcpupin[i].cpumask.mapLen; + } + } + + if (callback->legacy) { + remoteDispatchObjectEventSend(callback->client, remoteProgram, + REMOTE_PROC_DOMAIN_EVENT_CPUTUNE, + (xdrproc_t)xdr_remote_domain_event_cputune_msg, + &data); + } else { + remote_domain_event_callback_cputune_msg msg = { callback->callbackID, + data }; + + remoteDispatchObjectEventSend(callback->client, remoteProgram, + REMOTE_PROC_DOMAIN_EVENT_CALLBACK_CPUTUNE, + (xdrproc_t)xdr_remote_domain_event_callback_cputune_msg, + &msg); + } + + return 0; + + error: + VIR_FREE(data.emulatorpin.map.map_val); + if (data.vcpupin.vcpupin_val) { + for (i = 0; i < data.nvcpupin; i++) + VIR_FREE(data.vcpupin.vcpupin_val[i].cpumask.map.map_val); + VIR_FREE(data.vcpupin.vcpupin_val); + } + return -1; +} + + static virConnectDomainEventGenericCallback domainEventCallbacks[] = { VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventLifecycle), VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventReboot), @@ -987,6 +1073,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 9358314..636b89b 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -5127,7 +5127,68 @@ typedef void (*virConnectDomainEventDeviceRemovedCallback)(virConnectPtr conn, virDomainPtr dom, const char *devAlias, void *opaque); +/** + * _virDomainCpumaksInfo

s/maks/mask/

+ * + * This structure stores a mask with pinning information for emulator + * or vcpus.

And of course I'm muddying things up by adding a iothreadpin

+ */ +struct _virDomainCpumaskInfo { + int mapLen; + unsigned char *map; +}; +typedef struct _virDomainCpumaskInfo virDomainCpumaskInfo; +typedef virDomainCpumaskInfo *virDomainCpumaskInfoPtr; + +/** + * _virDomainVcpupinInfo + * + * This structure stores cpumask with pinning information + * for each vcpu where the pinning has been set. + */ +struct _virDomainVcpupinInfo { + int vcpuid;

Looking at this makes me think I should change the 'int vcpuid' to just 'int id' for my series (in _virDomainVcpuPinDef) - makes reusing this so much easier when it comes to the difference between vcpuid and iothreadid. Course that probably also means a structure name change from Vcpu to something more generic like Array.

+ virDomainCpumaskInfo cpumask; +}; +typedef struct _virDomainVcpupinInfo virDomainVcpupinInfo; +typedef virDomainVcpupinInfo *virDomainVcpupinInfoPtr; + +/** + * _virDomainCputuneInfo + * + * Structure containing all infromation about cputune for + * specific domain.

s/infromation/information s/for specific domain/for a specific domain/ Your call - add something about this is for events or event mgmt API's. Basically this is the user view right?

+ */ +struct _virDomainCputuneInfo { + unsigned long long shares; + int sharesSpecified; + unsigned long long period; + long long quota; + unsigned long long emulatorPeriod; + long long emulatorQuota; + size_t nvcpupin; + virDomainVcpupinInfoPtr vcpupin; + virDomainCpumaskInfo emulatorpin; +}; +typedef struct _virDomainCputuneInfo virDomainCputuneInfo; +typedef virDomainCputuneInfo *virDomainCputuneInfoPtr;

+/** + * virConnectDomainEventCputuneCallback: + * @conn: connection object + * @dom: domain on which the event occurred + * @cputune: cputune informations

s/informations/information/

+ * @opaque: application specified data + * + * This callback occurs when cpu tune is updated.

s/cpu tune/guest cputune data/ (or something similar) The remainder of this seems OK. Although I'm curious to know if there's been any thought as to whether we bump up against any limits if there are a lot of remote_domain_vcpupin's (e.g. vcpupin_val) within the remote_domain_event_cputune_msg? Perhaps 1 or 2 for testing work, but what if someone has whatever the maximum is? My other question or concern would be if/when the iothreadpin series is accepted - obviously another patch would be required to add iothreadpin data in. Would this need to be done within the same release version? And if it wasn't would there have to be a v2 type event? IOW, can event data be extended? If not, then I just have to be sure to set aside the time within the release scope. Hopefully someone with a bit more knowledge of events can double check that you've touched everything you're supposed to touch John

+ * + * 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, + virDomainCputuneInfoPtr cputune, + void *opaque);

/** * VIR_DOMAIN_EVENT_CALLBACK: @@ -5163,6 +5224,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..966562f 100644 --- a/src/conf/domain_event.c +++ b/src/conf/domain_event.c @@ -52,6 +52,7 @@ static virClassPtr virDomainEventBalloonChangeClass; static virClassPtr virDomainEventDeviceRemovedClass; static virClassPtr virDomainEventPMClass; static virClassPtr virDomainQemuMonitorEventClass; +static virClassPtr virDomainEventCputuneClass;

static void virDomainEventDispose(void *obj); @@ -67,6 +68,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 +205,14 @@ struct _virDomainQemuMonitorEvent { typedef struct _virDomainQemuMonitorEvent virDomainQemuMonitorEvent; typedef virDomainQemuMonitorEvent *virDomainQemuMonitorEventPtr;

+struct _virDomainEventCputune { + virDomainEvent parent; + + virDomainCputuneInfo cputune; +}; +typedef struct _virDomainEventCputune virDomainEventCputune; +typedef virDomainEventCputune *virDomainEventCputunePtr; +

static int virDomainEventsOnceInit(void) @@ -285,6 +295,12 @@ virDomainEventsOnceInit(void) sizeof(virDomainQemuMonitorEvent), virDomainQemuMonitorEventDispose))) return -1; + if (!(virDomainEventCputuneClass = + virClassNew(virDomainEventClass, + "virDomainEventCputune", + sizeof(virDomainEventCputune), + virDomainEventCputuneDispose))) + return -1; return 0; }

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

+static void +virDomainEventCputuneDispose(void *obj) +{ + virDomainEventCputunePtr event = obj; + VIR_DEBUG("obj=%p", event); + + VIR_FREE(event->cputune.emulatorpin.map); + if (event->cputune.vcpupin) { + size_t i; + for (i = 0; i < event->cputune.nvcpupin; i++) { + VIR_FREE(event->cputune.vcpupin[i].cpumask.map); + } + VIR_FREE(event->cputune.vcpupin); + } +} +

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

+static virObjectEventPtr +virDomainEventCputuneNew(int id, + const char *name, + unsigned char *uuid, + virDomainCputune cputune) +{ + virDomainEventCputunePtr ev; + size_t i; + + if (virDomainEventsInitialize() < 0) + return NULL; + + if (!(ev = virDomainEventNew(virDomainEventCputuneClass, + VIR_DOMAIN_EVENT_ID_CPUTUNE, + id, name, uuid))) + return NULL; + + ev->cputune.shares = cputune.shares; + ev->cputune.sharesSpecified = cputune.sharesSpecified; + ev->cputune.period = cputune.period; + ev->cputune.quota = cputune.quota; + ev->cputune.emulatorPeriod = cputune.emulator_period; + ev->cputune.emulatorQuota = cputune.emulator_quota; + ev->cputune.nvcpupin = cputune.nvcpupin; + ev->cputune.vcpupin = NULL; + + if (cputune.emulatorpin) { + if (virBitmapToData(cputune.emulatorpin->cpumask, + &ev->cputune.emulatorpin.map, + &ev->cputune.emulatorpin.mapLen) < 0) + goto error; + } + + if (cputune.vcpupin) { + if (VIR_ALLOC_N(ev->cputune.vcpupin, ev->cputune.nvcpupin) < 0) + goto error; + for (i = 0; i < ev->cputune.nvcpupin; i++) { + ev->cputune.vcpupin[i].vcpuid = cputune.vcpupin[i]->vcpuid; + if (virBitmapToData(cputune.vcpupin[i]->cpumask, + &ev->cputune.vcpupin[i].cpumask.map, + &ev->cputune.vcpupin[i].cpumask.mapLen) < 0) + goto error; + } + } + + return (virObjectEventPtr)ev; + + error: + VIR_FREE(ev->cputune.emulatorpin.map); + if (ev->cputune.vcpupin) { + for (i = 0; i < ev->cputune.nvcpupin; i++) + VIR_FREE(ev->cputune.vcpupin[i].cpumask.map); + VIR_FREE(ev->cputune.vcpupin); + } + virObjectUnref(ev); + + return NULL; +} + +virObjectEventPtr +virDomainEventCputuneNewFromObj(virDomainObjPtr obj, + virDomainCputune cputune) +{ + return virDomainEventCputuneNew(obj->def->id, + obj->def->name, + obj->def->uuid, + cputune); +} + +virObjectEventPtr +virDomainEventCputuneNewFromDom(virDomainPtr dom, + virDomainCputune cputune) +{ + return virDomainEventCputuneNew(dom->id, + dom->name, + dom->uuid, + cputune); +}

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

+ case VIR_DOMAIN_EVENT_ID_CPUTUNE: + { + virDomainEventCputunePtr cputuneEvent; + cputuneEvent = (virDomainEventCputunePtr)event; + ((virConnectDomainEventCputuneCallback)cb)(conn, dom, + &cputuneEvent->cputune, + 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..44c6a7b 100644 --- a/src/conf/domain_event.h +++ b/src/conf/domain_event.h @@ -184,6 +184,13 @@ virDomainEventDeviceRemovedNewFromObj(virDomainObjPtr obj, virObjectEventPtr virDomainEventDeviceRemovedNewFromDom(virDomainPtr dom, const char *devAlias); +virObjectEventPtr +virDomainEventCputuneNewFromObj(virDomainObjPtr obj, + virDomainCputune cputune); +virObjectEventPtr +virDomainEventCputuneNewFromDom(virDomainPtr dom, + virDomainCputune cputune); +

int virDomainEventStateRegister(virConnectPtr conn, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 71fc063..74a5487 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -439,6 +439,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 fda27f7..ba8a738 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -326,6 +326,16 @@ remoteDomainBuildEventBlockJob2(virNetClientProgramPtr prog, void *evdata, void *opaque);

static void +remoteDomainBuildEventCputune(virNetClientProgramPtr prog, + virNetClientPtr client, + 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); @@ -395,6 +405,10 @@ static virNetClientProgramEvent remoteEvents[] = { remoteDomainBuildEventDeviceRemoved, sizeof(remote_domain_event_device_removed_msg), (xdrproc_t)xdr_remote_domain_event_device_removed_msg }, + { REMOTE_PROC_DOMAIN_EVENT_CPUTUNE, + remoteDomainBuildEventCputune, + sizeof(remote_domain_event_cputune_msg), + (xdrproc_t)xdr_remote_domain_event_cputune_msg }, /* All events above here are legacy events, missing the callback * ID, which means the server has a single global registration and * we do full filtering in the client. If the server lacks @@ -476,6 +490,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 +5518,98 @@ remoteDomainBuildEventCallbackDeviceRemoved(virNetClientProgramPtr prog ATTRIBUT

static void +remoteDomainBuildEventCputuneHelper(virConnectPtr conn, + remote_domain_event_cputune_msg *msg, + int callbackID) +{ + struct private_data *priv = conn->privateData; + virDomainPtr dom; + virDomainCputune cputune; + virObjectEventPtr event = NULL; + size_t i; + + dom = get_nonnull_domain(conn, msg->dom); + if (!dom) + return; + + memset(&cputune, 0, sizeof(cputune)); + + cputune.shares = msg->shares; + cputune.sharesSpecified = msg->sharesSpecified; + cputune.period = msg->period; + cputune.quota = msg->quota; + cputune.emulator_period = msg->emulatorPeriod; + cputune.emulator_quota = msg->emulatorQuota; + cputune.nvcpupin = msg->nvcpupin; + + if (msg->nvcpupin != msg->vcpupin.vcpupin_len) + goto cleanup; + + if (msg->emulatorpin.map.map_val) { + if (VIR_ALLOC(cputune.emulatorpin) < 0) + goto cleanup; + cputune.emulatorpin->cpumask = virBitmapNewData(msg->emulatorpin.map.map_val, + msg->emulatorpin.map.map_len); + if (!cputune.emulatorpin->cpumask) + goto cleanup; + } + + if (msg->vcpupin.vcpupin_val) { + if (VIR_ALLOC_N(cputune.vcpupin, cputune.nvcpupin) < 0) + goto cleanup; + + for (i = 0; i < cputune.nvcpupin; i++) { + if (VIR_ALLOC(cputune.vcpupin[i]) < 0) + goto cleanup; + cputune.vcpupin[i]->vcpuid = msg->vcpupin.vcpupin_val[i].vcpuid; + cputune.vcpupin[i]->cpumask = virBitmapNewData(msg->vcpupin.vcpupin_val[i].cpumask.map.map_val, + msg->vcpupin.vcpupin_val[i].cpumask.map.map_len); + if (!cputune.vcpupin[i]->cpumask) + goto cleanup; + } + } + + event = virDomainEventCputuneNewFromDom(dom, cputune); + + remoteEventQueue(priv, event, callbackID); + + cleanup: + virDomainFree(dom); + if (cputune.emulatorpin) { + virBitmapFree(cputune.emulatorpin->cpumask); + VIR_FREE(cputune.emulatorpin); + } + if (cputune.vcpupin) { + for (i = 0; i < cputune.nvcpupin; i++) { + if (cputune.vcpupin[i]) { + virBitmapFree(cputune.vcpupin[i]->cpumask); + VIR_FREE(cputune.vcpupin[i]); + } + } + VIR_FREE(cputune.vcpupin); + } +} +static void +remoteDomainBuildEventCputune(virNetClientProgramPtr prog ATTRIBUTE_UNUSED, + virNetClientPtr client ATTRIBUTE_UNUSED, + void *evdata, void *opaque) +{ + virConnectPtr conn = opaque; + remote_domain_event_cputune_msg *msg = evdata; + remoteDomainBuildEventCputuneHelper(conn, msg, -1); +} +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; + remoteDomainBuildEventCputuneHelper(conn, &msg->msg, 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 8fc552f..5c2d8f5 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -2980,6 +2980,31 @@ struct remote_domain_event_block_job_2_msg { int status; };

+struct remote_domain_cpumask { + int mapLen; + unsigned char map<REMOTE_CPUMAP_MAX>; +}; +struct remote_domain_vcpupin { + int vcpuid; + remote_domain_cpumask cpumask; +}; +struct remote_domain_event_cputune_msg { + remote_nonnull_domain dom; + unsigned hyper shares; + int sharesSpecified; + unsigned hyper period; + hyper quota; + unsigned hyper emulatorPeriod; + hyper emulatorQuota; + unsigned int nvcpupin; + remote_domain_vcpupin vcpupin<REMOTE_VCPUINFO_MAX>; + remote_domain_cpumask emulatorpin; +}; +struct remote_domain_event_callback_cputune_msg { + int callbackID; + remote_domain_event_cputune_msg msg; +}; + struct remote_connect_get_cpu_model_names_args { remote_nonnull_string arch; int need_results; @@ -5456,5 +5481,17 @@ enum remote_procedure { * @acl: connect:search_domains * @aclfilter: domain:read */ - REMOTE_PROC_CONNECT_GET_ALL_DOMAIN_STATS = 344 + REMOTE_PROC_CONNECT_GET_ALL_DOMAIN_STATS = 344, + + /** + * @generate: both + * @acl: none + */ + REMOTE_PROC_DOMAIN_EVENT_CPUTUNE = 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 899f1cc..8dacdd5 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -2435,6 +2435,36 @@ struct remote_domain_event_block_job_2_msg { int type; int status; }; +struct remote_domain_cpumask { + int mapLen; + struct { + u_int map_len; + u_char * map_val; + } map; +}; +struct remote_domain_vcpupin { + int vcpuid; + remote_domain_cpumask cpumask; +}; +struct remote_domain_event_cputune_msg { + remote_nonnull_domain dom; + uint64_t shares; + int sharesSpecified; + uint64_t period; + int64_t quota; + uint64_t emulatorPeriod; + int64_t emulatorQuota; + u_int nvcpupin; + struct { + u_int vcpupin_len; + remote_domain_vcpupin * vcpupin_val; + } vcpupin; + remote_domain_cpumask emulatorpin; +}; +struct remote_domain_event_callback_cputune_msg { + int callbackID; + remote_domain_event_cputune_msg msg; +}; struct remote_connect_get_cpu_model_names_args { remote_nonnull_string arch; int need_results; @@ -2890,4 +2920,6 @@ enum remote_procedure { REMOTE_PROC_CONNECT_GET_DOMAIN_CAPABILITIES = 342, REMOTE_PROC_DOMAIN_OPEN_GRAPHICS_FD = 343, REMOTE_PROC_CONNECT_GET_ALL_DOMAIN_STATS = 344, + REMOTE_PROC_DOMAIN_EVENT_CPUTUNE = 345, + REMOTE_PROC_DOMAIN_EVENT_CALLBACK_CPUTUNE = 346, }; diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index c75cd73..d2d2d96 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -11170,6 +11170,53 @@ vshEventDeviceRemovedPrint(virConnectPtr conn ATTRIBUTE_UNUSED, vshEventDone(data->ctl); }

+static void +vshEventCputunePrint(virConnectPtr conn ATTRIBUTE_UNUSED, + virDomainPtr dom, + virDomainCputuneInfoPtr cputune, + void *opaque) +{ + vshDomEventData *data = opaque; + + if (!data->loop && *data->count) + return; + + vshPrint(data->ctl, + _("event 'cpu-tune' for domain %s:\n"), + virDomainGetName(dom)); + if (cputune->sharesSpecified) { + vshPrint(data->ctl, _("\tshares: %llu\n"), cputune->shares); + } else { + vshPrint(data->ctl, _("\tshares: not specified\n")); + } + vshPrint(data->ctl, _("\tperiod: %llu\n\tquota: %lld\n"), + cputune->period, cputune->quota); + vshPrint(data->ctl, + _("\temulator_period: %llu\n\temulator_quota: %lld\n"), + cputune->emulatorPeriod, cputune->emulatorQuota); + if (cputune->emulatorpin.map) { + char *str = virBitmapDataToString(cputune->emulatorpin.map, + cputune->emulatorpin.mapLen); + vshPrint(data->ctl, _("\temulatorpin: %s\n"), str); + VIR_FREE(str); + } + + if (cputune->vcpupin) { + size_t i; + for (i = 0; i < cputune->nvcpupin; i++) { + char *str = virBitmapDataToString(cputune->vcpupin[i].cpumask.map, + cputune->vcpupin[i].cpumask.mapLen); + vshPrint(data->ctl, _("\tvcpupin (vcpuid: %d): %s\n"), + cputune->vcpupin[i].vcpuid, str); + VIR_FREE(str); + } + } + + (*data->count)++; + if (!data->loop) + vshEventDone(data->ctl); +} + static vshEventCallback vshEventCallbacks[] = { { "lifecycle", VIR_DOMAIN_EVENT_CALLBACK(vshEventLifecyclePrint), }, @@ -11203,6 +11250,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));

On 08/28/2014 12:38 PM, Pavel Hrdina wrote:

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

@@ -395,6 +405,10 @@ static virNetClientProgramEvent remoteEvents[] = { remoteDomainBuildEventDeviceRemoved, sizeof(remote_domain_event_device_removed_msg), (xdrproc_t)xdr_remote_domain_event_device_removed_msg }, + { REMOTE_PROC_DOMAIN_EVENT_CPUTUNE, + remoteDomainBuildEventCputune, + sizeof(remote_domain_event_cputune_msg), + (xdrproc_t)xdr_remote_domain_event_cputune_msg }, /* All events above here are legacy events, missing the callback * ID, which means the server has a single global registration and * we do full filtering in the client. If the server lacks

This hunk is wrong. Per the comment, you do NOT want an old-style event.

@@ -476,6 +490,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 },

because the new-style is sufficient. [Or are you planning on backporting this to an earlier version of libvirt that didn't support new-style events?]

+ + /** + * @generate: both + * @acl: none + */ + REMOTE_PROC_DOMAIN_EVENT_CPUTUNE = 345,

Likewise, this RPC should not be needed.

+ + /** + * @generate: both + * @acl: none + */ + REMOTE_PROC_DOMAIN_EVENT_CALLBACK_CPUTUNE = 346

and this one should be 345, and is sufficient. Look at commit 1bfe73a126e; that covers the bare minimum that you need to add a new-style event without worrying about back-compat to old libvirt. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 09/04/2014 01:51 AM, John Ferlan wrote:

On 08/28/2014 02:38 PM, Pavel Hrdina wrote:

...

Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_cgroup.c | 6 ++++++ src/qemu/qemu_driver.c | 27 +++++++++++++++++++++++++++ 2 files changed, 33 insertions(+)

diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 43d14d4..95cc4d4 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -676,6 +676,7 @@ static int qemuSetupCpuCgroup(virDomainObjPtr vm) { qemuDomainObjPrivatePtr priv = vm->privateData; + virObjectEventPtr event;

if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU)) { if (vm->def->cputune.sharesSpecified) { @@ -695,6 +696,11 @@ qemuSetupCpuCgroup(virDomainObjPtr vm) if (virCgroupGetCpuShares(priv->cgroup, &val) < 0) return -1; vm->def->cputune.shares = val; + + event = virDomainEventCputuneNewFromObj(vm, vm->def->cputune); + + if (event) + qemuDomainEventQueue(vm->privateData, event);

So this is triggered during qemuProcessStart() and only when there's cputune.sharesSpecified is set and before any cgroups are set up or virProcessSetAffinity() is called. This is fairly specific, but does not include when the vcpu or emulator is initially setup. Since you're triggering events when emulatorpin/vcpupin are called - I would think similarly the qemuSetupCgroupForVcpu() and qemuSetupCgroupForEmulator() would/could be places for events since that's when they are created.

Also, should there be a similar event during qemuProcessReconnect().

IOW: When should this event be triggered and naturally - is that documented properly or enough?

John

That's a good question, probably I should have explain that in the commit message. The reason for triggering the event during qemuProcessStart() is because cgroups could change the cputune.shares and therefor we should trigger the cputune event to let the management application know that the value from domain configuration is no longer valid. Pavel [..]