8 a.m.
If a user registers for a domain event filtered to a particular
domain, but the persistent domain is offline at the time, then
the code silently failed to set up the filter. As a result,
the event fires for all domains, rather than being filtered.
Network events were immune, since they always passed an id
0 argument.
The key to this patch is realizing that
virObjectEventDispatchMatchCallback() only cared about uuid;
so refusing to create a meta for a negative id is pointless,
and in fact, malloc'ing meta at all was overkill; instead,
just directly store a uuid and a flag of whether to filter.
* src/conf/object_event_private.h (_virObjectEventCallback):
Replace meta with uuid and flag.
(virObjectEventCallbackListAddID): Update signature.
* src/conf/object_event.h (virObjectEventStateRegisterID):
Likewise.
* src/conf/object_event.c (virObjectEventCallbackListAddID): Drop
arguments that don't affect filtering.
(virObjectEventCallbackListRemoveID)
(virObjectEventDispatchMatchCallback)
(virObjectEventStateRegisterID): Update clients.
* src/conf/domain_event.c (virDomainEventCallbackListAdd)
(virDomainEventStateRegisterID): Likewise.
* src/conf/network_event.c (virNetworkEventStateRegisterID):
Likewise.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
src/conf/domain_event.c | 16 +++++-----------
src/conf/network_event.c | 13 +++----------
src/conf/object_event.c | 48 +++++++++++++++++-------------------------------
src/conf/object_event.h | 2 --
4 files changed, 25 insertions(+), 54 deletions(-)
diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c
index 4f8ede5..35212ef 100644
--- a/src/conf/domain_event.c
+++ b/src/conf/domain_event.c
@@ -1284,7 +1284,7 @@ virDomainEventStateRegister(virConnectPtr conn,
if (virDomainEventsInitialize() < 0)
return -1;
- return virObjectEventStateRegisterID(conn, state, NULL, NULL, 0,
+ return virObjectEventStateRegisterID(conn, state, NULL,
virDomainEventClass,
VIR_DOMAIN_EVENT_ID_LIFECYCLE,
VIR_OBJECT_EVENT_CALLBACK(callback),
@@ -1322,16 +1322,10 @@ virDomainEventStateRegisterID(virConnectPtr conn,
if (virDomainEventsInitialize() < 0)
return -1;
- if (dom)
- return virObjectEventStateRegisterID(conn, state, dom->uuid, dom->name,
- dom->id, virDomainEventClass, eventID,
- VIR_OBJECT_EVENT_CALLBACK(cb),
- opaque, freecb, callbackID);
- else
- return virObjectEventStateRegisterID(conn, state, NULL, NULL, 0,
- virDomainEventClass, eventID,
- VIR_OBJECT_EVENT_CALLBACK(cb),
- opaque, freecb, callbackID);
+ return virObjectEventStateRegisterID(conn, state, dom ? dom->uuid : NULL,
+ virDomainEventClass, eventID,
+ VIR_OBJECT_EVENT_CALLBACK(cb),
+ opaque, freecb, callbackID);
}
diff --git a/src/conf/network_event.c b/src/conf/network_event.c
index a192ffe..38c74d1 100644
--- a/src/conf/network_event.c
+++ b/src/conf/network_event.c
@@ -151,16 +151,9 @@ virNetworkEventStateRegisterID(virConnectPtr conn,
if (virNetworkEventsInitialize() < 0)
return -1;
- if (net)
- return virObjectEventStateRegisterID(conn, state,
- net->uuid, net->name, 0,
- virNetworkEventClass, eventID,
- cb, opaque, freecb, callbackID);
- else
- return virObjectEventStateRegisterID(conn, state,
- NULL, NULL, 0,
- virNetworkEventClass, eventID,
- cb, opaque, freecb, callbackID);
+ return virObjectEventStateRegisterID(conn, state, net ? net->uuid : NULL,
+ virNetworkEventClass, eventID,
+ cb, opaque, freecb, callbackID);
}
diff --git a/src/conf/object_event.c b/src/conf/object_event.c
index 87e10e0..320a422 100644
--- a/src/conf/object_event.c
+++ b/src/conf/object_event.c
@@ -66,7 +66,8 @@ struct _virObjectEventCallback {
virClassPtr klass;
int eventID;
virConnectPtr conn;
- virObjectMetaPtr meta;
+ bool uuid_filter;
+ unsigned char uuid[VIR_UUID_BUFLEN];
virConnectObjectEventGenericCallback cb;
void *opaque;
virFreeCallback freecb;
@@ -201,9 +202,6 @@ virObjectEventCallbackListRemoveID(virConnectPtr conn,
if (cb->freecb)
(*cb->freecb)(cb->opaque);
virObjectUnref(cb->conn);
- if (cb->meta)
- VIR_FREE(cb->meta->name);
- VIR_FREE(cb->meta);
VIR_FREE(cb);
VIR_DELETE_ELEMENT(cbList->callbacks, i, cbList->count);
@@ -309,9 +307,9 @@ virObjectEventCallbackLookup(virConnectPtr conn,
cb->eventID == eventID &&
cb->conn == conn &&
cb->legacy == legacy &&
- ((uuid && cb->meta &&
- memcmp(cb->meta->uuid, uuid, VIR_UUID_BUFLEN) == 0) ||
- (!uuid && !cb->meta))) {
+ ((uuid && cb->uuid_filter &&
+ memcmp(cb->uuid, uuid, VIR_UUID_BUFLEN) == 0) ||
+ (!uuid && !cb->uuid_filter))) {
ret = cb->callbackID;
break;
}
@@ -325,8 +323,6 @@ virObjectEventCallbackLookup(virConnectPtr conn,
* @conn: pointer to the connection
* @cbList: the list
* @uuid: the uuid of the object to filter on
- * @name: the name of the object to filter on
- * @id: the ID of the object to filter on
* @klass: the base event class
* @eventID: the event ID
* @callback: the callback to add
@@ -340,8 +336,6 @@ static int
virObjectEventCallbackListAddID(virConnectPtr conn,
virObjectEventCallbackListPtr cbList,
unsigned char uuid[VIR_UUID_BUFLEN],
- const char *name,
- int id,
virClassPtr klass,
int eventID,
virConnectObjectEventGenericCallback callback,
@@ -352,10 +346,9 @@ virObjectEventCallbackListAddID(virConnectPtr conn,
virObjectEventCallbackPtr event;
int ret = -1;
- VIR_DEBUG("conn=%p cblist=%p uuid=%p name=%s id=%d "
+ VIR_DEBUG("conn=%p cblist=%p uuid=%p "
"klass=%p eventID=%d callback=%p opaque=%p",
- conn, cbList, uuid, name, id, klass, eventID,
- callback, opaque);
+ conn, cbList, uuid, klass, eventID, callback, opaque);
/* Check incoming */
if (!cbList) {
@@ -381,13 +374,12 @@ virObjectEventCallbackListAddID(virConnectPtr conn,
event->opaque = opaque;
event->freecb = freecb;
- if (name && uuid && id > 0) {
- if (VIR_ALLOC(event->meta) < 0)
- goto cleanup;
- if (VIR_STRDUP(event->meta->name, name) < 0)
- goto cleanup;
- memcpy(event->meta->uuid, uuid, VIR_UUID_BUFLEN);
- event->meta->id = id;
+ /* Only need 'uuid' for matching; 'id' can change as domain
+ * switches between running and shutoff, and 'name' can change in
+ * Xen migration. */
+ if (uuid) {
+ event->uuid_filter = true;
+ memcpy(event->uuid, uuid, VIR_UUID_BUFLEN);
}
if (callbackID)
@@ -401,12 +393,8 @@ virObjectEventCallbackListAddID(virConnectPtr conn,
ret = virObjectEventCallbackListCount(conn, cbList, klass, eventID);
cleanup:
- if (event) {
+ if (event)
virObjectUnref(event->conn);
- if (event->meta)
- VIR_FREE(event->meta->name);
- VIR_FREE(event->meta);
- }
VIR_FREE(event);
return ret;
}
@@ -669,14 +657,14 @@ virObjectEventDispatchMatchCallback(virObjectEventPtr event,
if (cb->eventID != event->eventID)
return false;
- if (cb->meta) {
+ if (cb->uuid_filter) {
/* Deliberately ignoring 'id' for matching, since that
* will cause problems when a domain switches between
* running & shutoff states & ignoring 'name' since
* Xen sometimes renames guests during migration, thus
* leaving 'uuid' as the only truly reliable ID we can use. */
- return memcmp(event->meta.uuid, cb->meta->uuid, VIR_UUID_BUFLEN) == 0;
+ return memcmp(event->meta.uuid, cb->uuid, VIR_UUID_BUFLEN) == 0;
}
return true;
}
@@ -807,8 +795,6 @@ int
virObjectEventStateRegisterID(virConnectPtr conn,
virObjectEventStatePtr state,
unsigned char *uuid,
- const char *name,
- int id,
virClassPtr klass,
int eventID,
virConnectObjectEventGenericCallback cb,
@@ -832,7 +818,7 @@ virObjectEventStateRegisterID(virConnectPtr conn,
}
ret = virObjectEventCallbackListAddID(conn, state->callbacks,
- uuid, name, id, klass, eventID,
+ uuid, klass, eventID,
cb, opaque, freecb,
callbackID);
diff --git a/src/conf/object_event.h b/src/conf/object_event.h
index fa1281c..e44d4f2 100644
--- a/src/conf/object_event.h
+++ b/src/conf/object_event.h
@@ -71,8 +71,6 @@ int
virObjectEventStateRegisterID(virConnectPtr conn,
virObjectEventStatePtr state,
unsigned char *uuid,
- const char *name,
- int id,
virClassPtr klass,
int eventID,
virConnectObjectEventGenericCallback cb,
--
1.8.4.2