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@xxxxxxxxxx> --- src/conf/domain_event.c | 16 +++++----------- src/conf/network_event.c | 13 +++---------- src/conf/object_event.c | 48 +++++++++++++++++------------------------------- src/conf/object_event.h | 5 ++--- 4 files changed, 27 insertions(+), 55 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..39e92d5 100644 --- a/src/conf/object_event.h +++ b/src/conf/object_event.h @@ -71,15 +71,14 @@ int virObjectEventStateRegisterID(virConnectPtr conn, virObjectEventStatePtr state, unsigned char *uuid, - const char *name, - int id, virClassPtr klass, int eventID, virConnectObjectEventGenericCallback cb, void *opaque, virFreeCallback freecb, int *callbackID) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(8); + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(4) + ATTRIBUTE_NONNULL(6); int virObjectEventStateDeregisterID(virConnectPtr conn, virObjectEventStatePtr state, -- 1.8.4.2 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list