2:57 a.m.
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
2:57 a.m.
New subject: [libvirt] [PATCH 1/3] qemuDomainAttachDeviceLive: Rework event emitting
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(a)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
2:57 a.m.
New subject: [libvirt] [PATCH 2/3] Introduce VIR_DOMAIN_EVENT_ID_LEASE_CHANGE
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(a)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
2:57 a.m.
New subject: [libvirt] [PATCH 3/3] qemu: Emit event on lease attach/detach
https://bugzilla.redhat.com/show_bug.cgi?id=1639228
Signed-off-by: Michal Privoznik <mprivozn(a)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
6:39 p.m.
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
8:06 a.m.
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
8:32 a.m.
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 :|
9:20 a.m.
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
2040
days inactive
2058
days old
7 comments
3 participants
participants (3)
-
Cole Robinson -
Daniel P. Berrangé -
Michal Privoznik