[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[libvirt] [PATCHv2 13/14] event: don't turn offline domain into global event



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 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  |  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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]