10:20 a.m.
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(a)redhat.com>
---
src/conf/domain_event.c | 71 +++++++++++++++++++++++++++++++++++++++++++------
src/conf/object_event.c | 40 ++++++++++++++++++----------
2 files changed, 89 insertions(+), 22 deletions(-)
diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c
index 01c0ae6..a2880fd 100644
--- a/src/conf/domain_event.c
+++ b/src/conf/domain_event.c
@@ -1383,6 +1383,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,
@@ -1391,6 +1403,7 @@ virDomainQemuMonitorEventDispatchFunc(virConnectPtr conn,
{
virDomainPtr dom = virGetDomain(conn, event->meta.name, event->meta.uuid);
virDomainQemuMonitorEventPtr qemuMonitorEvent;
+ virDomainQemuMonitorEventData *data = cbopaque;
if (!dom)
return;
@@ -1402,7 +1415,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
* @state: object event state
@@ -1604,23 +1652,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 5ceca8a..1a78fb8 100644
--- a/src/conf/object_event.c
+++ b/src/conf/object_event.c
@@ -181,6 +181,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 &&
@@ -216,10 +218,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);
@@ -249,10 +252,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);
}
}
@@ -386,8 +390,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",
@@ -420,10 +426,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)
--
1.8.5.3