qemu: enable monitor event filtering by name

Filtering monitor events by name requires tracking the name for
the duration of the filtering.  In order to free the name, I
found it easiest to just piggyback on the user's freecb function,
which gets called when the event is deregistered.

For events without a name filter, we have the design of multiple
client registrations sharing a common server registration, because
the server side uses the same callback function and we reject
duplicate use of the same function.  But with events in the mix,
we want to be able to allow the same function pointer to be used
with more than one event name.  The solution is to tweak the
duplicate detection code to only act when there is no additional
filtering; if name filtering is in use, there is exactly one
client registration per server registration.  Yes, this means
that there is no longer a bound on the number of server
registrations possible, so a malicious client could repeatedly
register for the same name event to exhaust server memory.  On
the other hand, we already restricted monitor events to require
write access (compared to normal events only needing read access),
and separated it into the intentionally unsupported
libvirt-qemu.so, with documentation that using this function is
for debug purposes only; so it is not a security risk worth
worrying about a client trying to abuse multiple registrations.

* src/conf/domain_event.c (virDomainQemuMonitorEventData): New
struct.
(virDomainQemuMonitorEventFilter)
(virDomainQemuMonitorEventCleanup): New functions.
(virDomainQemuMonitorEventDispatchFunc)
(virDomainQemuMonitorEventStateRegisterID): Use new struct.
* src/conf/object_event.c (virObjectEventCallbackListCount)
(virObjectEventCallbackListAddID)
(virObjectEventCallbackListRemoveID)
(virObjectEventCallbackListMarkDeleteID): Drop duplicate detection
when filtering is in effect.

Signed-off-by: Eric Blake <eblake@redhat.com>

diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c
index 8e1a3108ac94f99cda9b669998f57e0902b1b2c0..88d04fd82d3c440240f08a59038f88d9cc24f2ad 100644 (file)
--- a/src/conf/domain_event.c
+++ b/src/conf/domain_event.c
@@ -1384,6 +1384,18 @@ error:
 }
 
 
+/* In order to filter by event name, we need to store a copy of the
+ * name to filter on.  By wrapping the caller's freecb, we can
+ * piggyback our cleanup to happen at the same time the caller
+ * deregisters.  */
+struct virDomainQemuMonitorEventData {
+    char *event;
+    void *opaque;
+    virFreeCallback freecb;
+};
+typedef struct virDomainQemuMonitorEventData virDomainQemuMonitorEventData;
+
+
 static void
 virDomainQemuMonitorEventDispatchFunc(virConnectPtr conn,
                                       virObjectEventPtr event,
@@ -1392,6 +1404,7 @@ virDomainQemuMonitorEventDispatchFunc(virConnectPtr conn,
 {
     virDomainPtr dom = virGetDomain(conn, event->meta.name, event->meta.uuid);
     virDomainQemuMonitorEventPtr qemuMonitorEvent;
+    virDomainQemuMonitorEventData *data = cbopaque;
 
     if (!dom)
         return;
@@ -1403,7 +1416,7 @@ virDomainQemuMonitorEventDispatchFunc(virConnectPtr conn,
                                                    qemuMonitorEvent->seconds,
                                                    qemuMonitorEvent->micros,
                                                    qemuMonitorEvent->details,
-                                                   cbopaque);
+                                                   data->opaque);
     virDomainFree(dom);
 }
 
@@ -1577,6 +1590,41 @@ virDomainEventStateDeregister(virConnectPtr conn,
 }
 
 
+/**
+ * virDomainQemuMonitorEventFilter:
+ * @conn: the connection pointer
+ * @event: the event about to be dispatched
+ * @opaque: the opaque data registered with the filter
+ *
+ * Callback for filtering based on event names.  Returns true if the
+ * event should be dispatched.
+ */
+static bool
+virDomainQemuMonitorEventFilter(virConnectPtr conn ATTRIBUTE_UNUSED,
+                                virObjectEventPtr event,
+                                void *opaque)
+{
+    virDomainQemuMonitorEventData *data = opaque;
+    virDomainQemuMonitorEventPtr monitorEvent;
+
+    monitorEvent = (virDomainQemuMonitorEventPtr) event;
+
+    return STREQ(monitorEvent->event, data->event);
+}
+
+
+static void
+virDomainQemuMonitorEventCleanup(void *opaque)
+{
+    virDomainQemuMonitorEventData *data = opaque;
+
+    VIR_FREE(data->event);
+    if (data->freecb)
+        (data->freecb)(data->opaque);
+    VIR_FREE(data);
+}
+
+
 /**
  * virDomainQemuMonitorEventStateRegisterID:
  * @conn: connection to associate with callback
@@ -1605,23 +1653,30 @@ virDomainQemuMonitorEventStateRegisterID(virConnectPtr conn,
                                          unsigned int flags,
                                          int *callbackID)
 {
+    virDomainQemuMonitorEventData *data = NULL;
+    virObjectEventCallbackFilter filter = NULL;
+
     if (virDomainEventsInitialize() < 0)
         return -1;
 
-    /* FIXME support event filtering */
     if (flags != -1)
         virCheckFlags(0, -1);
-    if (event) {
-        virReportError(VIR_ERR_INTERNAL_ERROR,
-                       _("event filtering on '%s' not implemented yet"),
-                       event);
+    if (VIR_ALLOC(data) < 0)
+        return -1;
+    if (VIR_STRDUP(data->event, event) < 0) {
+        VIR_FREE(data);
         return -1;
     }
+    data->opaque = opaque;
+    data->freecb = freecb;
+    if (event)
+        filter = virDomainQemuMonitorEventFilter;
+    freecb = virDomainQemuMonitorEventCleanup;
 
     return virObjectEventStateRegisterID(conn, state, dom ? dom->uuid : NULL,
-                                         NULL, NULL,
+                                         filter, data,
                                          virDomainQemuMonitorEventClass, 0,
                                          VIR_OBJECT_EVENT_CALLBACK(cb),
-                                         opaque, freecb,
+                                         data, freecb,
                                          false, callbackID, false);
 }

diff --git a/src/conf/object_event.c b/src/conf/object_event.c
index 697c85f67b1638190514c92a915d7fd7b52b9242..4b6c7af67f9ae658281f8073e94ac7597f9c19ba 100644 (file)
--- a/src/conf/object_event.c
+++ b/src/conf/object_event.c
@@ -183,6 +183,8 @@ virObjectEventCallbackListCount(virConnectPtr conn,
     for (i = 0; i < cbList->count; i++) {
         virObjectEventCallbackPtr cb = cbList->callbacks[i];
 
+        if (cb->filter)
+            continue;
         if (cb->klass == klass &&
             cb->eventID == eventID &&
             cb->conn == conn &&
@@ -218,10 +220,11 @@ virObjectEventCallbackListRemoveID(virConnectPtr conn,
         if (cb->callbackID == callbackID && cb->conn == conn) {
             int ret;
 
-            ret = virObjectEventCallbackListCount(conn, cbList, cb->klass,
-                                                  cb->eventID,
-                                                  cb->uuid_filter ? cb->uuid : NULL,
-                                                  cb->remoteID >= 0) - 1;
+            ret = cb->filter ? 0 :
+                (virObjectEventCallbackListCount(conn, cbList, cb->klass,
+                                                 cb->eventID,
+                                                 cb->uuid_filter ? cb->uuid : NULL,
+                                                 cb->remoteID >= 0) - 1);
 
             if (cb->freecb)
                 (*cb->freecb)(cb->opaque);
@@ -251,10 +254,11 @@ virObjectEventCallbackListMarkDeleteID(virConnectPtr conn,
 
         if (cb->callbackID == callbackID && cb->conn == conn) {
             cb->deleted = true;
-            return virObjectEventCallbackListCount(conn, cbList, cb->klass,
-                                                   cb->eventID,
-                                                   cb->uuid_filter ? cb->uuid : NULL,
-                                                   cb->remoteID >= 0);
+            return cb->filter ? 0 :
+                virObjectEventCallbackListCount(conn, cbList, cb->klass,
+                                                cb->eventID,
+                                                cb->uuid_filter ? cb->uuid : NULL,
+                                                cb->remoteID >= 0);
         }
     }
 
@@ -388,8 +392,10 @@ virObjectEventCallbackListAddID(virConnectPtr conn,
         return -1;
     }
 
-    /* check if we already have this callback on our list */
-    if (virObjectEventCallbackLookup(conn, cbList, uuid,
+    /* If there is no additional filtering, then check if we already
+     * have this callback on our list.  */
+    if (!filter &&
+        virObjectEventCallbackLookup(conn, cbList, uuid,
                                      klass, eventID, callback, legacy,
                                      serverFilter ? &remoteID : NULL) != -1) {
         virReportError(VIR_ERR_INVALID_ARG, "%s",
@@ -422,10 +428,16 @@ virObjectEventCallbackListAddID(virConnectPtr conn,
     if (VIR_APPEND_ELEMENT(cbList->callbacks, cbList->count, event) < 0)
         goto cleanup;
 
-    ret = virObjectEventCallbackListCount(conn, cbList, klass, eventID,
-                                          uuid, serverFilter);
-    if (serverFilter && remoteID < 0)
-        ret++;
+    /* When additional filtering is being done, every client callback
+     * is matched to exactly one server callback.  */
+    if (filter) {
+        ret = 1;
+    } else {
+        ret = virObjectEventCallbackListCount(conn, cbList, klass, eventID,
+                                              uuid, serverFilter);
+        if (serverFilter && remoteID < 0)
+            ret++;
+    }
 
 cleanup:
     if (event)