[libvirt] [PATCH 0/3] Emit event on lease attach/detach
Unfortunately, we can't emit VIR_DOMAIN_EVENT_ID_DEVICE_ADDED or VIR_DOMAIN_EVENT_ID_DEVICE_REMOVED event because that carries device alias within itself and leases don't have one. Michal Prívozník (3): qemuDomainAttachDeviceLive: Rework event emitting Introduce VIR_DOMAIN_EVENT_ID_LEASE_CHANGE qemu: Emit event on lease attach/detach examples/object-events/event-test.c | 34 +++++++++++ include/libvirt/libvirt-domain.h | 33 +++++++++++ src/conf/domain_event.c | 89 +++++++++++++++++++++++++++++ src/conf/domain_event.h | 12 ++++ src/libvirt_private.syms | 2 + src/qemu/qemu_driver.c | 43 +++++++------- src/qemu/qemu_hotplug.c | 7 +++ src/remote/remote_daemon_dispatch.c | 40 +++++++++++++ src/remote/remote_driver.c | 32 +++++++++++ src/remote/remote_protocol.x | 16 +++++- src/remote_protocol-structs | 8 +++ tools/virsh-domain.c | 31 ++++++++++ 12 files changed, 325 insertions(+), 22 deletions(-) -- 2.21.0
Some devices that we want to emit event for do not have an alias. Rework event emitting to make code more generic. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_driver.c | 36 ++++++++++++++++-------------------- 1 file changed, 16 insertions(+), 20 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 7e5bbc3cc9..6eabcfce18 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7838,15 +7838,15 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm, virDomainDeviceDefPtr dev, virQEMUDriverPtr driver) { + virObjectEventPtr event = NULL; int ret = -1; - const char *alias = NULL; switch ((virDomainDeviceType)dev->type) { case VIR_DOMAIN_DEVICE_DISK: qemuDomainObjCheckDiskTaint(driver, vm, dev->data.disk, NULL); ret = qemuDomainAttachDeviceDiskLive(driver, vm, dev); if (!ret) { - alias = dev->data.disk->info.alias; + event = virDomainEventDeviceAddedNewFromObj(vm, dev->data.disk->info.alias); dev->data.disk = NULL; } break; @@ -7854,7 +7854,7 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm, case VIR_DOMAIN_DEVICE_CONTROLLER: ret = qemuDomainAttachControllerDevice(driver, vm, dev->data.controller); if (!ret) { - alias = dev->data.controller->info.alias; + event = virDomainEventDeviceAddedNewFromObj(vm, dev->data.controller->info.alias); dev->data.controller = NULL; } break; @@ -7870,7 +7870,7 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm, qemuDomainObjCheckNetTaint(driver, vm, dev->data.net, NULL); ret = qemuDomainAttachNetDevice(driver, vm, dev->data.net); if (!ret) { - alias = dev->data.net->info.alias; + event = virDomainEventDeviceAddedNewFromObj(vm, dev->data.net->info.alias); dev->data.net = NULL; } break; @@ -7880,7 +7880,7 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm, ret = qemuDomainAttachHostDevice(driver, vm, dev->data.hostdev); if (!ret) { - alias = dev->data.hostdev->info->alias; + event = virDomainEventDeviceAddedNewFromObj(vm, dev->data.hostdev->info->alias); dev->data.hostdev = NULL; } break; @@ -7889,7 +7889,7 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm, ret = qemuDomainAttachRedirdevDevice(driver, vm, dev->data.redirdev); if (!ret) { - alias = dev->data.redirdev->info.alias; + event = virDomainEventDeviceAddedNewFromObj(vm, dev->data.redirdev->info.alias); dev->data.redirdev = NULL; } break; @@ -7898,7 +7898,7 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm, ret = qemuDomainAttachChrDevice(driver, vm, dev->data.chr); if (!ret) { - alias = dev->data.chr->info.alias; + event = virDomainEventDeviceAddedNewFromObj(vm, dev->data.chr->info.alias); dev->data.chr = NULL; } break; @@ -7907,7 +7907,7 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm, ret = qemuDomainAttachRNGDevice(driver, vm, dev->data.rng); if (!ret) { - alias = dev->data.rng->info.alias; + event = virDomainEventDeviceAddedNewFromObj(vm, dev->data.rng->info.alias); dev->data.rng = NULL; } break; @@ -7924,7 +7924,7 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm, ret = qemuDomainAttachShmemDevice(driver, vm, dev->data.shmem); if (!ret) { - alias = dev->data.shmem->info.alias; + event = virDomainEventDeviceAddedNewFromObj(vm, dev->data.shmem->info.alias); dev->data.shmem = NULL; } break; @@ -7933,7 +7933,7 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm, ret = qemuDomainAttachWatchdog(driver, vm, dev->data.watchdog); if (!ret) { - alias = dev->data.watchdog->info.alias; + event = virDomainEventDeviceAddedNewFromObj(vm, dev->data.watchdog->info.alias); dev->data.watchdog = NULL; } break; @@ -7941,7 +7941,7 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm, case VIR_DOMAIN_DEVICE_INPUT: ret = qemuDomainAttachInputDevice(driver, vm, dev->data.input); if (ret == 0) { - alias = dev->data.input->info.alias; + event = virDomainEventDeviceAddedNewFromObj(vm, dev->data.input->info.alias); dev->data.input = NULL; } break; @@ -7949,7 +7949,7 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm, case VIR_DOMAIN_DEVICE_VSOCK: ret = qemuDomainAttachVsockDevice(driver, vm, dev->data.vsock); if (ret == 0) { - alias = dev->data.vsock->info.alias; + event = virDomainEventDeviceAddedNewFromObj(vm, dev->data.vsock->info.alias); dev->data.vsock = NULL; } break; @@ -7973,14 +7973,10 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm, break; } - if (alias) { - /* queue the event before the alias has a chance to get freed - * if the domain disappears while qemuDomainUpdateDeviceList - * is in monitor */ - virObjectEventPtr event; - event = virDomainEventDeviceAddedNewFromObj(vm, alias); - virObjectEventStateQueue(driver->domainEventState, event); - } + /* queue the event before the alias has a chance to get freed + * if the domain disappears while qemuDomainUpdateDeviceList + * is in monitor */ + virObjectEventStateQueue(driver->domainEventState, event); if (ret == 0) ret = qemuDomainUpdateDeviceList(driver, vm, QEMU_ASYNC_JOB_NONE); -- 2.21.0
https://bugzilla.redhat.com/show_bug.cgi?id=1639228 This event will be emitted whenever a lease is attached or detached. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- examples/object-events/event-test.c | 34 +++++++++++ include/libvirt/libvirt-domain.h | 33 +++++++++++ src/conf/domain_event.c | 89 +++++++++++++++++++++++++++++ src/conf/domain_event.h | 12 ++++ src/libvirt_private.syms | 2 + src/remote/remote_daemon_dispatch.c | 40 +++++++++++++ src/remote/remote_driver.c | 32 +++++++++++ src/remote/remote_protocol.x | 16 +++++- src/remote_protocol-structs | 8 +++ tools/virsh-domain.c | 31 ++++++++++ 10 files changed, 296 insertions(+), 1 deletion(-) diff --git a/examples/object-events/event-test.c b/examples/object-events/event-test.c index fcf4492470..e9775d7450 100644 --- a/examples/object-events/event-test.c +++ b/examples/object-events/event-test.c @@ -957,6 +957,39 @@ myDomainEventBlockThresholdCallback(virConnectPtr conn ATTRIBUTE_UNUSED, } +static const char * +leaseActionTypeToStr(int action) +{ + switch ((virConnectDomainEventLeaseAction) action) { + case VIR_CONNECT_DOMAIN_EVENT_LEASE_ACTION_ATTACH: + return "attach"; + + case VIR_CONNECT_DOMAIN_EVENT_LEASE_ACTION_DETACH: + return "detach"; + + case VIR_CONNECT_DOMAIN_EVENT_LEASE_ACTION_LAST: + break; + } + + return "unknown"; +} + + +static int +myDomainEventLeaseChangeCallback(virConnectPtr conn ATTRIBUTE_UNUSED, + virDomainPtr dom, + int action, + const char *lockspace, + const char *key, + void *opaque ATTRIBUTE_UNUSED) +{ + printf("%s EVENT domain %s(%d) lease change: action %s lockspace %s key %s", + __func__, virDomainGetName(dom), virDomainGetID(dom), + leaseActionTypeToStr(action), lockspace, key); + return 0; +} + + static int myDomainEventMigrationIterationCallback(virConnectPtr conn ATTRIBUTE_UNUSED, virDomainPtr dom, @@ -1087,6 +1120,7 @@ struct domainEventData domainEvents[] = { DOMAIN_EVENT(VIR_DOMAIN_EVENT_ID_DEVICE_REMOVAL_FAILED, myDomainEventDeviceRemovalFailedCallback), DOMAIN_EVENT(VIR_DOMAIN_EVENT_ID_METADATA_CHANGE, myDomainEventMetadataChangeCallback), DOMAIN_EVENT(VIR_DOMAIN_EVENT_ID_BLOCK_THRESHOLD, myDomainEventBlockThresholdCallback), + DOMAIN_EVENT(VIR_DOMAIN_EVENT_ID_LEASE_CHANGE, myDomainEventLeaseChangeCallback), }; struct storagePoolEventData { diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 7d36820b5a..8a68b84135 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -4473,6 +4473,38 @@ typedef void (*virConnectDomainEventBlockThresholdCallback)(virConnectPtr conn, unsigned long long excess, void *opaque); + +typedef enum { + VIR_CONNECT_DOMAIN_EVENT_LEASE_ACTION_ATTACH = 1, /* lease attached */ + VIR_CONNECT_DOMAIN_EVENT_LEASE_ACTION_DETACH = 2, /* lease detached */ + +# ifdef VIR_ENUM_SENTINELS + VIR_CONNECT_DOMAIN_EVENT_LEASE_ACTION_LAST +# endif +} virConnectDomainEventLeaseAction; + +/** + * virConnectDomainEventLeaseChangeCallback: + * @conn: connection object + * @dom: domain on which the event occurred + * @action: action which occurred, one of virConnectDomainEventLeaseAction + * @lockspace: string identifying within which lockspace @key is held + * @key: unique key + * @opaque: application specified data + * + * The callback occurs on lease attach or detach. + * + * The callback signature to use when registering for an event of type + * VIR_DOMAIN_EVENT_ID_LEASE_CHANGE with virConnectDomainEventRegisterAny() + */ +typedef void (*virConnectDomainEventLeaseChangeCallback)(virConnectPtr conn, + virDomainPtr dom, + int action, + const char *lockspace, + const char *key, + void *opaque); + + /** * VIR_DOMAIN_EVENT_CALLBACK: * @@ -4515,6 +4547,7 @@ typedef enum { VIR_DOMAIN_EVENT_ID_DEVICE_REMOVAL_FAILED = 22, /* virConnectDomainEventDeviceRemovalFailedCallback */ VIR_DOMAIN_EVENT_ID_METADATA_CHANGE = 23, /* virConnectDomainEventMetadataChangeCallback */ VIR_DOMAIN_EVENT_ID_BLOCK_THRESHOLD = 24, /* virConnectDomainEventBlockThresholdCallback */ + VIR_DOMAIN_EVENT_ID_LEASE_CHANGE = 25, /* virConnectDomainEventLeaseChangeCallback */ # ifdef VIR_ENUM_SENTINELS VIR_DOMAIN_EVENT_ID_LAST diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c index b33589f472..436a0c9eb1 100644 --- a/src/conf/domain_event.c +++ b/src/conf/domain_event.c @@ -59,6 +59,7 @@ static virClassPtr virDomainEventJobCompletedClass; static virClassPtr virDomainEventDeviceRemovalFailedClass; static virClassPtr virDomainEventMetadataChangeClass; static virClassPtr virDomainEventBlockThresholdClass; +static virClassPtr virDomainEventLeaseChangeClass; static void virDomainEventDispose(void *obj); static void virDomainEventLifecycleDispose(void *obj); @@ -81,6 +82,7 @@ static void virDomainEventJobCompletedDispose(void *obj); static void virDomainEventDeviceRemovalFailedDispose(void *obj); static void virDomainEventMetadataChangeDispose(void *obj); static void virDomainEventBlockThresholdDispose(void *obj); +static void virDomainEventLeaseChangeDispose(void *obj); static void virDomainEventDispatchDefaultFunc(virConnectPtr conn, @@ -289,6 +291,17 @@ struct _virDomainEventBlockThreshold { typedef struct _virDomainEventBlockThreshold virDomainEventBlockThreshold; typedef virDomainEventBlockThreshold *virDomainEventBlockThresholdPtr; +struct _virDomainEventLeaseChange { + virDomainEvent parent; + + int action; + + char *lockspace; + char *key; +}; +typedef struct _virDomainEventLeaseChange virDomainEventLeaseChange; +typedef virDomainEventLeaseChange *virDomainEventLeaseChangePtr; + static int virDomainEventsOnceInit(void) @@ -335,6 +348,8 @@ virDomainEventsOnceInit(void) return -1; if (!VIR_CLASS_NEW(virDomainEventBlockThreshold, virDomainEventClass)) return -1; + if (!VIR_CLASS_NEW(virDomainEventLeaseChange, virDomainEventClass)) + return -1; return 0; } @@ -544,6 +559,17 @@ virDomainEventBlockThresholdDispose(void *obj) } +static void +virDomainEventLeaseChangeDispose(void *obj) +{ + virDomainEventLeaseChangePtr event = obj; + VIR_DEBUG("obj=%p", event); + + VIR_FREE(event->lockspace); + VIR_FREE(event->key); +} + + static void * virDomainEventNew(virClassPtr klass, int eventID, @@ -1672,6 +1698,55 @@ virDomainEventBlockThresholdNewFromDom(virDomainPtr dom, } +static virObjectEventPtr +virDomainEventLeaseChangeNew(int id, + const char *name, + unsigned char *uuid, + int action, + const char *lockspace, + const char *key) +{ + virDomainEventLeaseChangePtr ev; + + if (virDomainEventsInitialize() < 0) + return NULL; + + if (!(ev = virDomainEventNew(virDomainEventLeaseChangeClass, + VIR_DOMAIN_EVENT_ID_LEASE_CHANGE, + id, name, uuid))) + return NULL; + + if (VIR_STRDUP(ev->lockspace, lockspace) < 0 || + VIR_STRDUP(ev->key, key) < 0) { + virObjectUnref(ev); + return NULL; + } + ev->action = action; + + return (virObjectEventPtr)ev; +} + +virObjectEventPtr +virDomainEventLeaseChangeNewFromObj(virDomainObjPtr obj, + int action, + const char *lockspace, + const char *key) +{ + return virDomainEventLeaseChangeNew(obj->def->id, obj->def->name, + obj->def->uuid, action, lockspace, key); +} + +virObjectEventPtr +virDomainEventLeaseChangeNewFromDom(virDomainPtr dom, + int action, + const char *lockspace, + const char *key) +{ + return virDomainEventLeaseChangeNew(dom->id, dom->name, dom->uuid, + action, lockspace, key); +} + + static void virDomainEventDispatchDefaultFunc(virConnectPtr conn, virObjectEventPtr event, @@ -1955,6 +2030,20 @@ virDomainEventDispatchDefaultFunc(virConnectPtr conn, cbopaque); goto cleanup; } + + case VIR_DOMAIN_EVENT_ID_LEASE_CHANGE: + { + virDomainEventLeaseChangePtr leaseChangeEvent; + + leaseChangeEvent = (virDomainEventLeaseChangePtr)event; + ((virConnectDomainEventLeaseChangeCallback)cb)(conn, dom, + leaseChangeEvent->action, + leaseChangeEvent->lockspace, + leaseChangeEvent->key, + 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 637f1daf68..bcd9c139a2 100644 --- a/src/conf/domain_event.h +++ b/src/conf/domain_event.h @@ -256,6 +256,18 @@ virDomainEventBlockThresholdNewFromDom(virDomainPtr dom, unsigned long long threshold, unsigned long long excess); +virObjectEventPtr +virDomainEventLeaseChangeNewFromObj(virDomainObjPtr obj, + int action, + const char *lockspace, + const char *key); + +virObjectEventPtr +virDomainEventLeaseChangeNewFromDom(virDomainPtr dom, + int action, + const char *lockspace, + const char *key); + int virDomainEventStateRegister(virConnectPtr conn, virObjectEventStatePtr state, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 212adf53c1..06295b031d 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -635,6 +635,8 @@ virDomainEventIOErrorReasonNewFromDom; virDomainEventIOErrorReasonNewFromObj; virDomainEventJobCompletedNewFromDom; virDomainEventJobCompletedNewFromObj; +virDomainEventLeaseChangeNewFromDom; +virDomainEventLeaseChangeNewFromObj; virDomainEventLifecycleNew; virDomainEventLifecycleNewFromDef; virDomainEventLifecycleNewFromDom; diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c index df28259042..82a75c73bd 100644 --- a/src/remote/remote_daemon_dispatch.c +++ b/src/remote/remote_daemon_dispatch.c @@ -1449,6 +1449,45 @@ remoteRelayDomainEventBlockThreshold(virConnectPtr conn, } +static int +remoteRelayDomainEventLeaseChange(virConnectPtr conn, + virDomainPtr dom, + int action, + const char *lockspace, + const char *key, + void *opaque) +{ + daemonClientEventCallbackPtr callback = opaque; + remote_domain_event_lease_change_msg data; + + if (callback->callbackID < 0 || + !remoteRelayDomainEventCheckACL(callback->client, conn, dom)) + return -1; + + VIR_DEBUG("Relaying domain lease change event %s %d %d %s %s, callback %d", + dom->name, dom->id, action, lockspace, key, callback->callbackID); + + memset(&data, 0, sizeof(data)); + data.callbackID = callback->callbackID; + if (VIR_STRDUP(data.locspace, lockspace) < 0 || + VIR_STRDUP(data.key, key) < 0) + goto error; + data.action = action; + if (make_nonnull_domain(&data.dom, dom) < 0) + goto error; + + remoteDispatchObjectEventSend(callback->client, remoteProgram, + REMOTE_PROC_DOMAIN_EVENT_LEASE_CHANGE, + (xdrproc_t)xdr_remote_domain_event_lease_change_msg, &data); + return 0; + + error: + xdr_free((xdrproc_t)xdr_remote_domain_event_lease_change_msg, + (char *) &data); + return -1; +} + + static virConnectDomainEventGenericCallback domainEventCallbacks[] = { VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventLifecycle), VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventReboot), @@ -1475,6 +1514,7 @@ static virConnectDomainEventGenericCallback domainEventCallbacks[] = { VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventDeviceRemovalFailed), VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventMetadataChange), VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventBlockThreshold), + VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventLeaseChange), }; verify(ARRAY_CARDINALITY(domainEventCallbacks) == VIR_DOMAIN_EVENT_ID_LAST); diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 5c4dd41227..7f6a463f2b 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -418,6 +418,11 @@ remoteDomainBuildEventBlockThreshold(virNetClientProgramPtr prog, virNetClientPtr client, void *evdata, void *opaque); +static void +remoteDomainBuildEventLeaseChange(virNetClientProgramPtr prog, + virNetClientPtr client, + void *evdata, void *opaque); + static void remoteConnectNotifyEventConnectionClosed(virNetClientProgramPtr prog ATTRIBUTE_UNUSED, virNetClientPtr client ATTRIBUTE_UNUSED, @@ -629,6 +634,10 @@ static virNetClientProgramEvent remoteEvents[] = { remoteDomainBuildEventBlockThreshold, sizeof(remote_domain_event_block_threshold_msg), (xdrproc_t)xdr_remote_domain_event_block_threshold_msg }, + { REMOTE_PROC_DOMAIN_EVENT_LEASE_CHANGE, + remoteDomainBuildEventLeaseChange, + sizeof(remote_domain_event_lease_change_msg), + (xdrproc_t)xdr_remote_domain_event_lease_change_msg }, }; static void @@ -5590,6 +5599,29 @@ remoteDomainBuildEventBlockThreshold(virNetClientProgramPtr prog ATTRIBUTE_UNUSE } +static void +remoteDomainBuildEventLeaseChange(virNetClientProgramPtr prog ATTRIBUTE_UNUSED, + virNetClientPtr client ATTRIBUTE_UNUSED, + void *evdata, void *opaque) +{ + virConnectPtr conn = opaque; + remote_domain_event_lease_change_msg *msg = evdata; + struct private_data *priv = conn->privateData; + virDomainPtr dom; + virObjectEventPtr event = NULL; + + if (!(dom = get_nonnull_domain(conn, msg->dom))) + return; + + event = virDomainEventLeaseChangeNewFromDom(dom, + msg->action, + msg->locspace, + msg->key); + virObjectUnref(dom); + virObjectEventStateQueueRemote(priv->eventState, event, msg->callbackID); +} + + static int remoteStreamSend(virStreamPtr st, const char *data, diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 74be4b37d0..780b2ca9cb 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -3573,6 +3573,14 @@ struct remote_connect_get_storage_pool_capabilities_ret { remote_nonnull_string capabilities; }; +struct remote_domain_event_lease_change_msg { + int callbackID; + remote_nonnull_domain dom; + int action; + remote_nonnull_string locspace; + remote_nonnull_string key; +}; + /*----- Protocol. -----*/ /* Define the program number, protocol version and procedure numbers here. */ @@ -6342,5 +6350,11 @@ enum remote_procedure { * @generate: both * @acl: connect:read */ - REMOTE_PROC_CONNECT_GET_STORAGE_POOL_CAPABILITIES = 403 + REMOTE_PROC_CONNECT_GET_STORAGE_POOL_CAPABILITIES = 403, + + /** + * @generate: both + * @acl: none + */ + REMOTE_PROC_DOMAIN_EVENT_LEASE_CHANGE = 404 }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 768189c573..5aae22bf78 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -2981,6 +2981,13 @@ struct remote_connect_get_storage_pool_capabilities_args { struct remote_connect_get_storage_pool_capabilities_ret { remote_nonnull_string capabilities; }; +struct remote_domain_event_lease_change_msg { + int callbackID; + remote_nonnull_domain dom; + int action; + remote_nonnull_string locspace; + remote_nonnull_string key; +}; enum remote_procedure { REMOTE_PROC_CONNECT_OPEN = 1, REMOTE_PROC_CONNECT_CLOSE = 2, @@ -3385,4 +3392,5 @@ enum remote_procedure { REMOTE_PROC_CONNECT_LIST_ALL_NWFILTER_BINDINGS = 401, REMOTE_PROC_DOMAIN_SET_IOTHREAD_PARAMS = 402, REMOTE_PROC_CONNECT_GET_STORAGE_POOL_CAPABILITIES = 403, + REMOTE_PROC_DOMAIN_EVENT_LEASE_CHANGE = 404, }; diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index e8d5404acf..ced5e62484 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -13405,6 +13405,35 @@ virshEventBlockThresholdPrint(virConnectPtr conn ATTRIBUTE_UNUSED, } +VIR_ENUM_DECL(virshEventLeaseChangeAction); +VIR_ENUM_IMPL(virshEventLeaseChangeAction, + VIR_CONNECT_DOMAIN_EVENT_LEASE_ACTION_LAST, + N_("unknown"), + N_("attach"), + N_("detach")); + + +static void +virshEventLeaseChangePrint(virConnectPtr conn ATTRIBUTE_UNUSED, + virDomainPtr dom, + int action, + const char *lockspace, + const char *key, + void *opaque) +{ + virBuffer buf = VIR_BUFFER_INITIALIZER; + + virBufferAsprintf(&buf, + _("event 'lease-change' for domain %s: " + "action: %s lockspace: %s key: %s\n"), + virDomainGetName(dom), + UNKNOWNSTR(virshEventLeaseChangeActionTypeToString(action)), + lockspace, key); + + virshEventPrint(opaque, &buf); +} + + virshDomainEventCallback virshDomainEventCallbacks[] = { { "lifecycle", VIR_DOMAIN_EVENT_CALLBACK(virshEventLifecyclePrint), }, @@ -13454,6 +13483,8 @@ virshDomainEventCallback virshDomainEventCallbacks[] = { VIR_DOMAIN_EVENT_CALLBACK(virshEventMetadataChangePrint), }, { "block-threshold", VIR_DOMAIN_EVENT_CALLBACK(virshEventBlockThresholdPrint), }, + { "lease-change", + VIR_DOMAIN_EVENT_CALLBACK(virshEventLeaseChangePrint), }, }; verify(VIR_DOMAIN_EVENT_ID_LAST == ARRAY_CARDINALITY(virshDomainEventCallbacks)); -- 2.21.0
https://bugzilla.redhat.com/show_bug.cgi?id=1639228 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_driver.c | 7 ++++++- src/qemu/qemu_hotplug.c | 7 +++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 6eabcfce18..dad5fff6a8 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7862,8 +7862,13 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm, case VIR_DOMAIN_DEVICE_LEASE: ret = qemuDomainAttachLease(driver, vm, dev->data.lease); - if (ret == 0) + if (ret == 0) { + event = virDomainEventLeaseChangeNewFromObj(vm, + VIR_CONNECT_DOMAIN_EVENT_LEASE_ACTION_ATTACH, + dev->data.lease->lockspace, + dev->data.lease->key); dev->data.lease = NULL; + } break; case VIR_DOMAIN_DEVICE_NET: diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 34249bd030..10dee24724 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -5934,6 +5934,7 @@ qemuDomainDetachDeviceLease(virQEMUDriverPtr driver, virDomainLeaseDefPtr lease) { virDomainLeaseDefPtr det_lease; + virObjectEventPtr event = NULL; int idx; if ((idx = virDomainLeaseIndex(vm->def, lease)) < 0) { @@ -5946,6 +5947,12 @@ qemuDomainDetachDeviceLease(virQEMUDriverPtr driver, if (virDomainLockLeaseDetach(driver->lockManager, vm, lease) < 0) return -1; + event = virDomainEventLeaseChangeNewFromObj(vm, + VIR_CONNECT_DOMAIN_EVENT_LEASE_ACTION_DETACH, + lease->lockspace, + lease->key); + virObjectEventStateQueue(driver->domainEventState, event); + det_lease = virDomainLeaseRemoveAt(vm->def, idx); virDomainLeaseDefFree(det_lease); return 0; -- 2.21.0
On 4/5/19 3:57 AM, Michal Privoznik wrote:
Unfortunately, we can't emit VIR_DOMAIN_EVENT_ID_DEVICE_ADDED or VIR_DOMAIN_EVENT_ID_DEVICE_REMOVED event because that carries device alias within itself and leases don't have one.
Hmm. I understand that <leases> aren't really devices so it doesn't have any direct reason to give it an info structure and assign it an alias. But that's really an argument for why they shouldn't have been a device to begin with. I presume we did it that way to take advantage of the existing hotplug APIs. But then adding a special purpose event seems like going in the opposite direction. How invasive is it to add an 'info' bit to lease devices, allocate aliases with the rest of qemu devices, maybe add an ADDRESS_TYPE_LEASE or something that is just an explicit no op for the ADDRESS consumers. My guess is it's not too bad but there could be dragons lurking There's already places internally where this would simplify things: it's annoying that device iterator code already needs to play games like 'check the address of all devices, oh except leases and graphics which don't have info, oh and hostdev info is a pointer for some reason' - Cole
On 4/16/19 1:39 AM, Cole Robinson wrote:
On 4/5/19 3:57 AM, Michal Privoznik wrote:
Unfortunately, we can't emit VIR_DOMAIN_EVENT_ID_DEVICE_ADDED or VIR_DOMAIN_EVENT_ID_DEVICE_REMOVED event because that carries device alias within itself and leases don't have one.
Hmm. I understand that <leases> aren't really devices so it doesn't have any direct reason to give it an info structure and assign it an alias. But that's really an argument for why they shouldn't have been a device to begin with. I presume we did it that way to take advantage of the existing hotplug APIs. But then adding a special purpose event seems like going in the opposite direction.
How invasive is it to add an 'info' bit to lease devices, allocate aliases with the rest of qemu devices, maybe add an ADDRESS_TYPE_LEASE or something that is just an explicit no op for the ADDRESS consumers. My guess is it's not too bad but there could be dragons lurking
There's already places internally where this would simplify things: it's annoying that device iterator code already needs to play games like 'check the address of all devices, oh except leases and graphics which don't have info, oh and hostdev info is a pointer for some reason'
Yeah, it's annoying, but I remember discussion from when I was introducing user aliases. I wanted to just have some dummy element that would not mean anything to libvirt but users could set it and then match devices using it as a mark. It was rejected because it doesn't map onto anything in qemu. Well, nor do leases, nor would their address. I don't have a preference here really. But if we wanted to invent addresees for leases then I guess we would have to do it for already running domains too (when daemon restarts). I guess we need somebody else's input here. Dan/Peter? Michal
On Tue, Apr 23, 2019 at 03:06:22PM +0200, Michal Privoznik wrote:
On 4/16/19 1:39 AM, Cole Robinson wrote:
On 4/5/19 3:57 AM, Michal Privoznik wrote:
Unfortunately, we can't emit VIR_DOMAIN_EVENT_ID_DEVICE_ADDED or VIR_DOMAIN_EVENT_ID_DEVICE_REMOVED event because that carries device alias within itself and leases don't have one.
Hmm. I understand that <leases> aren't really devices so it doesn't have any direct reason to give it an info structure and assign it an alias. But that's really an argument for why they shouldn't have been a device to begin with. I presume we did it that way to take advantage of the existing hotplug APIs. But then adding a special purpose event seems like going in the opposite direction.
How invasive is it to add an 'info' bit to lease devices, allocate aliases with the rest of qemu devices, maybe add an ADDRESS_TYPE_LEASE or something that is just an explicit no op for the ADDRESS consumers. My guess is it's not too bad but there could be dragons lurking
There's already places internally where this would simplify things: it's annoying that device iterator code already needs to play games like 'check the address of all devices, oh except leases and graphics which don't have info, oh and hostdev info is a pointer for some reason'
Yeah, it's annoying, but I remember discussion from when I was introducing user aliases. I wanted to just have some dummy element that would not mean anything to libvirt but users could set it and then match devices using it as a mark. It was rejected because it doesn't map onto anything in qemu. Well, nor do leases, nor would their address.
I don't have a preference here really. But if we wanted to invent addresees for leases then I guess we would have to do it for already running domains too (when daemon restarts).
My preference is for the new event as this series does. We shouldn't try to force something to be more like a device when it clearly isn't. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On 4/23/19 9:32 AM, Daniel P. Berrangé wrote:
On Tue, Apr 23, 2019 at 03:06:22PM +0200, Michal Privoznik wrote:
On 4/16/19 1:39 AM, Cole Robinson wrote:
On 4/5/19 3:57 AM, Michal Privoznik wrote:
Unfortunately, we can't emit VIR_DOMAIN_EVENT_ID_DEVICE_ADDED or VIR_DOMAIN_EVENT_ID_DEVICE_REMOVED event because that carries device alias within itself and leases don't have one.
Hmm. I understand that <leases> aren't really devices so it doesn't have any direct reason to give it an info structure and assign it an alias. But that's really an argument for why they shouldn't have been a device to begin with. I presume we did it that way to take advantage of the existing hotplug APIs. But then adding a special purpose event seems like going in the opposite direction.
How invasive is it to add an 'info' bit to lease devices, allocate aliases with the rest of qemu devices, maybe add an ADDRESS_TYPE_LEASE or something that is just an explicit no op for the ADDRESS consumers. My guess is it's not too bad but there could be dragons lurking
There's already places internally where this would simplify things: it's annoying that device iterator code already needs to play games like 'check the address of all devices, oh except leases and graphics which don't have info, oh and hostdev info is a pointer for some reason'
Yeah, it's annoying, but I remember discussion from when I was introducing user aliases. I wanted to just have some dummy element that would not mean anything to libvirt but users could set it and then match devices using it as a mark. It was rejected because it doesn't map onto anything in qemu. Well, nor do leases, nor would their address.
I don't have a preference here really. But if we wanted to invent addresees for leases then I guess we would have to do it for already running domains too (when daemon restarts).
My preference is for the new event as this series does. We shouldn't try to force something to be more like a device when it clearly isn't.
IMO that ship has sailed. From the API perspective it's largely already a device: it appears in the <devices> block of the domain, it's added/removed/updated with the *Device* APIs. Yes it shouldn't output or accept an <address>, but that's basically its current state. If we add an .info struct internally I suspect it won't take much extra work to preserve that behavior. For info alias I think we should be adding one anyways. From my reading the primary motivation of the user aliases support is that it gives API users a unique way to identify the device, which seems just as valid for leases as anything else listed in <devices>. BTW I think all that logic applies to <graphics> devices as well. They don't have .info internally, so don't get aliases. If there's a future when <graphics> is hotpluggable we are going to have to add a custom event for that too. The alias uniqueness bit applies as well, though not too much an issue in practice for qemu because we enforce that only a single <graphics> for each @type is allowed (but that's an artificial distinction AFAICT, qemu looks like it can support multiple vnc server instances for example) Thanks, Cole
participants (3)
-
Cole Robinson -
Daniel P. Berrangé -
Michal Privoznik