2013-11-26 14:10:15 +00:00
|
|
|
/*
|
|
|
|
* object_event.c: object event queue processing helpers
|
|
|
|
*
|
2014-01-02 14:16:49 +00:00
|
|
|
* Copyright (C) 2010-2014 Red Hat, Inc.
|
2013-11-26 14:10:15 +00:00
|
|
|
* Copyright (C) 2008 VirtualIron
|
|
|
|
* Copyright (C) 2013 SUSE LINUX Products GmbH, Nuernberg, Germany.
|
|
|
|
*
|
|
|
|
* This library is free software; you can redistribute it and/or
|
|
|
|
* modify it under the terms of the GNU Lesser General Public
|
|
|
|
* License as published by the Free Software Foundation; either
|
|
|
|
* version 2.1 of the License, or (at your option) any later version.
|
|
|
|
*
|
|
|
|
* This library is distributed in the hope that it will be useful,
|
|
|
|
* but WITHOUT ANY WARRANTY; without even the implied warranty of
|
|
|
|
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
|
|
|
|
* Lesser General Public License for more details.
|
|
|
|
*
|
|
|
|
* You should have received a copy of the GNU Lesser General Public
|
|
|
|
* License along with this library. If not, see
|
|
|
|
* <http://www.gnu.org/licenses/>.
|
|
|
|
*
|
|
|
|
* Author: Ben Guthro
|
|
|
|
*/
|
|
|
|
|
|
|
|
#include <config.h>
|
|
|
|
|
|
|
|
#include "domain_event.h"
|
2013-12-11 10:37:58 +00:00
|
|
|
#include "network_event.h"
|
2013-11-26 14:10:15 +00:00
|
|
|
#include "object_event.h"
|
|
|
|
#include "object_event_private.h"
|
|
|
|
#include "virlog.h"
|
|
|
|
#include "datatypes.h"
|
|
|
|
#include "viralloc.h"
|
|
|
|
#include "virerror.h"
|
|
|
|
#include "virstring.h"
|
|
|
|
|
|
|
|
#define VIR_FROM_THIS VIR_FROM_NONE
|
|
|
|
|
2014-01-03 21:44:29 +00:00
|
|
|
struct _virObjectEventCallbackList {
|
|
|
|
unsigned int nextID;
|
|
|
|
size_t count;
|
|
|
|
virObjectEventCallbackPtr *callbacks;
|
|
|
|
};
|
|
|
|
|
2013-11-26 14:10:15 +00:00
|
|
|
struct _virObjectEventQueue {
|
2014-01-03 20:31:48 +00:00
|
|
|
size_t count;
|
2013-11-26 14:10:15 +00:00
|
|
|
virObjectEventPtr *events;
|
|
|
|
};
|
2014-01-03 21:44:29 +00:00
|
|
|
typedef struct _virObjectEventQueue virObjectEventQueue;
|
|
|
|
typedef virObjectEventQueue *virObjectEventQueuePtr;
|
|
|
|
|
|
|
|
struct _virObjectEventState {
|
|
|
|
/* The list of domain event callbacks */
|
|
|
|
virObjectEventCallbackListPtr callbacks;
|
|
|
|
/* The queue of object events */
|
|
|
|
virObjectEventQueuePtr queue;
|
|
|
|
/* Timer for flushing events queue */
|
|
|
|
int timer;
|
|
|
|
/* Flag if we're in process of dispatching */
|
|
|
|
bool isDispatching;
|
|
|
|
virMutex lock;
|
|
|
|
};
|
|
|
|
|
|
|
|
struct _virObjectEventCallback {
|
|
|
|
int callbackID;
|
|
|
|
virClassPtr klass;
|
|
|
|
int eventID;
|
|
|
|
virConnectPtr conn;
|
2014-01-06 12:32:55 +00:00
|
|
|
int remoteID;
|
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.
Note that virObjectEventPtr still needs all fields of meta,
because this is how we reconstruct a virDomainPtr inside the
dispatch handler before calling the end user's callback
pointer with the correct object, even though only the uuid
portion of meta is used in deciding whether a callback
matches the given event. So while uuid is optional for
callbacks, it is mandatory for events.
The change to testDomainCreateXMLMixed is merely on the setup
scenario (as you can't register for a domain unless it is either
running or persistent). I actually first wrote that test for
this patch, then rebased it to also cover a prior patch (commit
4221d64), but had to adjust it for that patch to use Create
instead of Define for setting up the domain long enough to
register the event in order to work around this bug. But while
the setup is changed, the main body of the test is still about
whether creation events fire as expected.
* 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_private.h (virObjectEventNew): Document
use of name and uuid in events.
* 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.
* tests/objecteventtest.c (testDomainCreateXMLMixed): Enhance test.
Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-04 00:02:17 +00:00
|
|
|
bool uuid_filter;
|
|
|
|
unsigned char uuid[VIR_UUID_BUFLEN];
|
event: filter global events by domain:getattr ACL [CVE-2014-0028]
Ever since ACL filtering was added in commit 7639736 (v1.1.1), a
user could still use event registration to obtain access to a
domain that they could not normally access via virDomainLookup*
or virConnectListAllDomains and friends. We already have the
framework in the RPC generator for creating the filter, and
previous cleanup patches got us to the point that we can now
wire the filter through the entire object event stack.
Furthermore, whether or not domain:getattr is honored, use of
global events is a form of obtaining a list of networks, which
is covered by connect:search_domains added in a93cd08 (v1.1.0).
Ideally, we'd have a way to enforce connect:search_domains when
doing global registrations while omitting that check on a
per-domain registration. But this patch just unconditionally
requires connect:search_domains, even when no list could be
obtained, based on the following observations:
1. Administrators are unlikely to grant domain:getattr for one
or all domains while still denying connect:search_domains - a
user that is able to manage domains will want to be able to
manage them efficiently, but efficient management includes being
able to list the domains they can access. The idea of denying
connect:search_domains while still granting access to individual
domains is therefore not adding any real security, but just
serves as a layer of obscurity to annoy the end user.
2. In the current implementation, domain events are filtered
on the client; the server has no idea if a domain filter was
requested, and must therefore assume that all domain event
requests are global. Even if we fix the RPC protocol to
allow for server-side filtering for newer client/server combos,
making the connect:serach_domains ACL check conditional on
whether the domain argument was NULL won't benefit older clients.
Therefore, we choose to document that connect:search_domains
is a pre-requisite to any domain event management.
Network events need the same treatment, with the obvious
change of using connect:search_networks and network:getattr.
* src/access/viraccessperm.h
(VIR_ACCESS_PERM_CONNECT_SEARCH_DOMAINS)
(VIR_ACCESS_PERM_CONNECT_SEARCH_NETWORKS): Document additional
effect of the permission.
* src/conf/domain_event.h (virDomainEventStateRegister)
(virDomainEventStateRegisterID): Add new parameter.
* src/conf/network_event.h (virNetworkEventStateRegisterID):
Likewise.
* src/conf/object_event_private.h (virObjectEventStateRegisterID):
Likewise.
* src/conf/object_event.c (_virObjectEventCallback): Track a filter.
(virObjectEventDispatchMatchCallback): Use filter.
(virObjectEventCallbackListAddID): Register filter.
* src/conf/domain_event.c (virDomainEventFilter): New function.
(virDomainEventStateRegister, virDomainEventStateRegisterID):
Adjust callers.
* src/conf/network_event.c (virNetworkEventFilter): New function.
(virNetworkEventStateRegisterID): Adjust caller.
* src/remote/remote_protocol.x
(REMOTE_PROC_CONNECT_DOMAIN_EVENT_REGISTER)
(REMOTE_PROC_CONNECT_DOMAIN_EVENT_REGISTER_ANY)
(REMOTE_PROC_CONNECT_NETWORK_EVENT_REGISTER_ANY): Generate a
filter, and require connect:search_domains instead of weaker
connect:read.
* src/test/test_driver.c (testConnectDomainEventRegister)
(testConnectDomainEventRegisterAny)
(testConnectNetworkEventRegisterAny): Update callers.
* src/remote/remote_driver.c (remoteConnectDomainEventRegister)
(remoteConnectDomainEventRegisterAny): Likewise.
* src/xen/xen_driver.c (xenUnifiedConnectDomainEventRegister)
(xenUnifiedConnectDomainEventRegisterAny): Likewise.
* src/vbox/vbox_tmpl.c (vboxDomainGetXMLDesc): Likewise.
* src/libxl/libxl_driver.c (libxlConnectDomainEventRegister)
(libxlConnectDomainEventRegisterAny): Likewise.
* src/qemu/qemu_driver.c (qemuConnectDomainEventRegister)
(qemuConnectDomainEventRegisterAny): Likewise.
* src/uml/uml_driver.c (umlConnectDomainEventRegister)
(umlConnectDomainEventRegisterAny): Likewise.
* src/network/bridge_driver.c
(networkConnectNetworkEventRegisterAny): Likewise.
* src/lxc/lxc_driver.c (lxcConnectDomainEventRegister)
(lxcConnectDomainEventRegisterAny): Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-08 20:34:48 +00:00
|
|
|
virObjectEventCallbackFilter filter;
|
|
|
|
void *filter_opaque;
|
2014-01-03 21:44:29 +00:00
|
|
|
virConnectObjectEventGenericCallback cb;
|
|
|
|
void *opaque;
|
|
|
|
virFreeCallback freecb;
|
|
|
|
bool deleted;
|
event: don't allow mix of old- and new-style registration
Consider these two calls, in either order:
id1 = virConnectDomainEventRegisterAny(conn, NULL,
VIR_DOMAIN_EVENT_ID_LIFECYCLE,
VIR_DOMAIN_EVENT_CALLBACK(callback), NULL, NULL);
virConnectDomainEventRegister(conn, callback, NULL, NULL);
Right now, the second call fails, because under the hood, the
old-style function registration is tightly coupled to the
new style lifecycle eventID, and the two calls both try
to register the same global eventID callback representation.
We've alreay documented that users should avoid old-style
registration and deregistration, so anyone heeding the advice
won't run into this situation. But it would be even nicer if
we pretend the two interfaces are completely separate, and
disallow any cross-linking. That is, a call to old-style
deregister should never remove a new-style callback even if it
is the same function pointer, and a call to new-style callback
using only callbackIDs obtained legitimately should never
remove an old-style callback (of course, since our callback
IDs are sequential, and there is still coupling under the
hood, you can easily guess the callbackID of an old style
registration and use new-style deregistration to nuke it - but
that starts to be blatantly bad coding on your part rather
than a surprising result on what looks like reasonable
stand-alone API).
With this patch, you can now register a global lifecycle event
handler twice, by using both old and new APIs; if such an event
occurs, your callback will be entered twice. But that is not a
problem in practice, since it is already possible to use the
new API to register both a global and per-domain event handler
using the same function, which will likewise fire your callback
twice for that domain. Duplicates are still prevented when
using the same API with same parameters twice (old-style twice,
new-style global twice, or new-style per-domain with same domain
twice), and things are still bounded (it is not possible to
register a single function pointer more than N+2 times per event
id, where N is the number of domains available on the connection).
Besides, it has always been possible to register as many
separate function pointers on the same event id as desired,
through either old or new style API, where the bound there is
the physical limitation of writing a program with enough
distinct function pointers.
Adding another event registration in the testsuite is sufficient
to cover this, where the test fails without the rest of the patch.
* src/conf/object_event.c (_virObjectEventCallback): Add field.
(virObjectEventCallbackLookup): Add argument.
(virObjectEventCallbackListAddID, virObjectEventStateCallbackID):
Adjust callers.
* tests/objecteventtest.c (testDomainCreateXMLMixed): Enhance test.
Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-04 12:55:55 +00:00
|
|
|
bool legacy; /* true if end user does not know callbackID */
|
2014-01-03 21:44:29 +00:00
|
|
|
};
|
2013-11-26 14:10:15 +00:00
|
|
|
|
|
|
|
static virClassPtr virObjectEventClass;
|
|
|
|
|
|
|
|
static void virObjectEventDispose(void *obj);
|
|
|
|
|
2013-12-11 10:38:03 +00:00
|
|
|
static int
|
|
|
|
virObjectEventOnceInit(void)
|
2013-11-26 14:10:15 +00:00
|
|
|
{
|
|
|
|
if (!(virObjectEventClass =
|
|
|
|
virClassNew(virClassForObject(),
|
|
|
|
"virObjectEvent",
|
|
|
|
sizeof(virObjectEvent),
|
|
|
|
virObjectEventDispose)))
|
|
|
|
return -1;
|
|
|
|
return 0;
|
|
|
|
}
|
|
|
|
|
|
|
|
VIR_ONCE_GLOBAL_INIT(virObjectEvent)
|
|
|
|
|
2014-01-02 14:16:49 +00:00
|
|
|
/**
|
|
|
|
* virClassForObjectEvent:
|
|
|
|
*
|
|
|
|
* Return the class object to be used as a parent when creating an
|
|
|
|
* event subclass.
|
|
|
|
*/
|
2013-12-11 10:38:03 +00:00
|
|
|
virClassPtr
|
|
|
|
virClassForObjectEvent(void)
|
2013-11-26 14:10:15 +00:00
|
|
|
{
|
|
|
|
if (virObjectEventInitialize() < 0)
|
|
|
|
return NULL;
|
|
|
|
return virObjectEventClass;
|
|
|
|
}
|
|
|
|
|
|
|
|
|
2013-12-11 10:38:03 +00:00
|
|
|
static void
|
|
|
|
virObjectEventDispose(void *obj)
|
2013-11-26 14:10:15 +00:00
|
|
|
{
|
|
|
|
virObjectEventPtr event = obj;
|
|
|
|
|
|
|
|
VIR_DEBUG("obj=%p", event);
|
|
|
|
|
|
|
|
VIR_FREE(event->meta.name);
|
|
|
|
}
|
|
|
|
|
|
|
|
/**
|
|
|
|
* virObjectEventCallbackListFree:
|
|
|
|
* @list: event callback list head
|
|
|
|
*
|
|
|
|
* Free the memory in the domain event callback list
|
|
|
|
*/
|
|
|
|
static void
|
|
|
|
virObjectEventCallbackListFree(virObjectEventCallbackListPtr list)
|
|
|
|
{
|
|
|
|
size_t i;
|
|
|
|
if (!list)
|
|
|
|
return;
|
|
|
|
|
2014-01-03 20:31:48 +00:00
|
|
|
for (i = 0; i < list->count; i++) {
|
2013-11-26 14:10:15 +00:00
|
|
|
virFreeCallback freecb = list->callbacks[i]->freecb;
|
|
|
|
if (freecb)
|
|
|
|
(*freecb)(list->callbacks[i]->opaque);
|
|
|
|
VIR_FREE(list->callbacks[i]);
|
|
|
|
}
|
|
|
|
VIR_FREE(list->callbacks);
|
|
|
|
VIR_FREE(list);
|
|
|
|
}
|
|
|
|
|
|
|
|
|
event: properly filter count of remaining events
On the surface, this sequence of API calls should succeed:
id1 = virConnectDomainEventRegisterAny(..., VIR_DOMAIN_EVENT_ID_LIFECYCLE,...);
id2 = virConnectDomainEventRegisterAny(..., VIR_DOMAIN_EVENT_ID_RTC_CHANGE,...);
virConnectDomainEventDeregisterAny(id1);
id1 = virConnectDomainEventRegisterAny(..., VIR_DOMAIN_EVENT_ID_LIFECYCLE,...);
And for test:///default, it does. But for qemu:///system, it fails:
libvirt: XML-RPC error : internal error: domain event 0 already registered
Looking closer, the bug is caused by miscommunication between
the object event engine and the client side of the remote driver.
In our implementation, we set up a single server-side event per
eventID, then the client side replicates that one event to all
callbacks that have been registered client side. To know when
to turn the server side eventID on or off, the client side must
track how many events for the same eventID have been registered.
But while our code was filtering by eventID on event registration,
it did not filter on event deregistration. So the above API calls
resulted in the deregister returning 1 instead of 0, so no RPC
deregister was issued, and the final register detects on the
server side that the server is already handling eventID 0.
Unfortunately, since the problem is only observable on remote
connections, it's not possible to enhance objecteventtest to
expose the semantics using only public API entry points.
* src/conf/object_event.c (virObjectEventCallbackListCount): New
function.
(virObjectEventCallbackListAddID)
(virObjectEventCallbackListRemoveID)
(virObjectEventCallbackListMarkDeleteID): Use it.
Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-03 21:50:15 +00:00
|
|
|
/**
|
|
|
|
* virObjectEventCallbackListCount:
|
|
|
|
* @conn: pointer to the connection
|
|
|
|
* @cbList: the list
|
|
|
|
* @klass: the base event class
|
|
|
|
* @eventID: the event ID
|
2014-01-06 12:32:55 +00:00
|
|
|
* @uuid: optional uuid of per-object filtering
|
|
|
|
* @serverFilter: true if server supports object filtering
|
event: properly filter count of remaining events
On the surface, this sequence of API calls should succeed:
id1 = virConnectDomainEventRegisterAny(..., VIR_DOMAIN_EVENT_ID_LIFECYCLE,...);
id2 = virConnectDomainEventRegisterAny(..., VIR_DOMAIN_EVENT_ID_RTC_CHANGE,...);
virConnectDomainEventDeregisterAny(id1);
id1 = virConnectDomainEventRegisterAny(..., VIR_DOMAIN_EVENT_ID_LIFECYCLE,...);
And for test:///default, it does. But for qemu:///system, it fails:
libvirt: XML-RPC error : internal error: domain event 0 already registered
Looking closer, the bug is caused by miscommunication between
the object event engine and the client side of the remote driver.
In our implementation, we set up a single server-side event per
eventID, then the client side replicates that one event to all
callbacks that have been registered client side. To know when
to turn the server side eventID on or off, the client side must
track how many events for the same eventID have been registered.
But while our code was filtering by eventID on event registration,
it did not filter on event deregistration. So the above API calls
resulted in the deregister returning 1 instead of 0, so no RPC
deregister was issued, and the final register detects on the
server side that the server is already handling eventID 0.
Unfortunately, since the problem is only observable on remote
connections, it's not possible to enhance objecteventtest to
expose the semantics using only public API entry points.
* src/conf/object_event.c (virObjectEventCallbackListCount): New
function.
(virObjectEventCallbackListAddID)
(virObjectEventCallbackListRemoveID)
(virObjectEventCallbackListMarkDeleteID): Use it.
Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-03 21:50:15 +00:00
|
|
|
*
|
|
|
|
* Internal function to count how many callbacks remain registered for
|
2014-01-06 12:32:55 +00:00
|
|
|
* the given @eventID and @uuid; knowing this allows the client side
|
|
|
|
* of the remote driver know when it must send an RPC to adjust the
|
|
|
|
* callbacks on the server. When @serverFilter is false, this function
|
|
|
|
* returns a count that includes both global and per-object callbacks,
|
|
|
|
* since the remote side will use a single global event to feed both.
|
|
|
|
* When true, the count is limited to the callbacks with the same
|
|
|
|
* @uuid, and where a remoteID has already been set on the callback
|
|
|
|
* with virObjectEventStateSetRemote(). Note that this function
|
|
|
|
* intentionally ignores the legacy field, since RPC calls use only a
|
|
|
|
* single callback on the server to manage both legacy and modern
|
|
|
|
* global domain lifecycle events.
|
event: properly filter count of remaining events
On the surface, this sequence of API calls should succeed:
id1 = virConnectDomainEventRegisterAny(..., VIR_DOMAIN_EVENT_ID_LIFECYCLE,...);
id2 = virConnectDomainEventRegisterAny(..., VIR_DOMAIN_EVENT_ID_RTC_CHANGE,...);
virConnectDomainEventDeregisterAny(id1);
id1 = virConnectDomainEventRegisterAny(..., VIR_DOMAIN_EVENT_ID_LIFECYCLE,...);
And for test:///default, it does. But for qemu:///system, it fails:
libvirt: XML-RPC error : internal error: domain event 0 already registered
Looking closer, the bug is caused by miscommunication between
the object event engine and the client side of the remote driver.
In our implementation, we set up a single server-side event per
eventID, then the client side replicates that one event to all
callbacks that have been registered client side. To know when
to turn the server side eventID on or off, the client side must
track how many events for the same eventID have been registered.
But while our code was filtering by eventID on event registration,
it did not filter on event deregistration. So the above API calls
resulted in the deregister returning 1 instead of 0, so no RPC
deregister was issued, and the final register detects on the
server side that the server is already handling eventID 0.
Unfortunately, since the problem is only observable on remote
connections, it's not possible to enhance objecteventtest to
expose the semantics using only public API entry points.
* src/conf/object_event.c (virObjectEventCallbackListCount): New
function.
(virObjectEventCallbackListAddID)
(virObjectEventCallbackListRemoveID)
(virObjectEventCallbackListMarkDeleteID): Use it.
Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-03 21:50:15 +00:00
|
|
|
*/
|
|
|
|
static int
|
|
|
|
virObjectEventCallbackListCount(virConnectPtr conn,
|
|
|
|
virObjectEventCallbackListPtr cbList,
|
|
|
|
virClassPtr klass,
|
2014-01-06 12:32:55 +00:00
|
|
|
int eventID,
|
|
|
|
unsigned char uuid[VIR_UUID_BUFLEN],
|
|
|
|
bool serverFilter)
|
event: properly filter count of remaining events
On the surface, this sequence of API calls should succeed:
id1 = virConnectDomainEventRegisterAny(..., VIR_DOMAIN_EVENT_ID_LIFECYCLE,...);
id2 = virConnectDomainEventRegisterAny(..., VIR_DOMAIN_EVENT_ID_RTC_CHANGE,...);
virConnectDomainEventDeregisterAny(id1);
id1 = virConnectDomainEventRegisterAny(..., VIR_DOMAIN_EVENT_ID_LIFECYCLE,...);
And for test:///default, it does. But for qemu:///system, it fails:
libvirt: XML-RPC error : internal error: domain event 0 already registered
Looking closer, the bug is caused by miscommunication between
the object event engine and the client side of the remote driver.
In our implementation, we set up a single server-side event per
eventID, then the client side replicates that one event to all
callbacks that have been registered client side. To know when
to turn the server side eventID on or off, the client side must
track how many events for the same eventID have been registered.
But while our code was filtering by eventID on event registration,
it did not filter on event deregistration. So the above API calls
resulted in the deregister returning 1 instead of 0, so no RPC
deregister was issued, and the final register detects on the
server side that the server is already handling eventID 0.
Unfortunately, since the problem is only observable on remote
connections, it's not possible to enhance objecteventtest to
expose the semantics using only public API entry points.
* src/conf/object_event.c (virObjectEventCallbackListCount): New
function.
(virObjectEventCallbackListAddID)
(virObjectEventCallbackListRemoveID)
(virObjectEventCallbackListMarkDeleteID): Use it.
Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-03 21:50:15 +00:00
|
|
|
{
|
|
|
|
size_t i;
|
|
|
|
int ret = 0;
|
|
|
|
|
|
|
|
for (i = 0; i < cbList->count; i++) {
|
|
|
|
virObjectEventCallbackPtr cb = cbList->callbacks[i];
|
|
|
|
|
|
|
|
if (cb->klass == klass &&
|
|
|
|
cb->eventID == eventID &&
|
|
|
|
cb->conn == conn &&
|
2014-01-06 12:32:55 +00:00
|
|
|
!cb->deleted &&
|
|
|
|
(!serverFilter ||
|
|
|
|
(cb->remoteID >= 0 &&
|
|
|
|
((uuid && cb->uuid_filter &&
|
|
|
|
memcmp(cb->uuid, uuid, VIR_UUID_BUFLEN) == 0) ||
|
|
|
|
(!uuid && !cb->uuid_filter)))))
|
event: properly filter count of remaining events
On the surface, this sequence of API calls should succeed:
id1 = virConnectDomainEventRegisterAny(..., VIR_DOMAIN_EVENT_ID_LIFECYCLE,...);
id2 = virConnectDomainEventRegisterAny(..., VIR_DOMAIN_EVENT_ID_RTC_CHANGE,...);
virConnectDomainEventDeregisterAny(id1);
id1 = virConnectDomainEventRegisterAny(..., VIR_DOMAIN_EVENT_ID_LIFECYCLE,...);
And for test:///default, it does. But for qemu:///system, it fails:
libvirt: XML-RPC error : internal error: domain event 0 already registered
Looking closer, the bug is caused by miscommunication between
the object event engine and the client side of the remote driver.
In our implementation, we set up a single server-side event per
eventID, then the client side replicates that one event to all
callbacks that have been registered client side. To know when
to turn the server side eventID on or off, the client side must
track how many events for the same eventID have been registered.
But while our code was filtering by eventID on event registration,
it did not filter on event deregistration. So the above API calls
resulted in the deregister returning 1 instead of 0, so no RPC
deregister was issued, and the final register detects on the
server side that the server is already handling eventID 0.
Unfortunately, since the problem is only observable on remote
connections, it's not possible to enhance objecteventtest to
expose the semantics using only public API entry points.
* src/conf/object_event.c (virObjectEventCallbackListCount): New
function.
(virObjectEventCallbackListAddID)
(virObjectEventCallbackListRemoveID)
(virObjectEventCallbackListMarkDeleteID): Use it.
Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-03 21:50:15 +00:00
|
|
|
ret++;
|
|
|
|
}
|
|
|
|
return ret;
|
|
|
|
}
|
|
|
|
|
2013-11-26 14:10:15 +00:00
|
|
|
/**
|
|
|
|
* virObjectEventCallbackListRemoveID:
|
|
|
|
* @conn: pointer to the connection
|
|
|
|
* @cbList: the list
|
|
|
|
* @callback: the callback to remove
|
|
|
|
*
|
|
|
|
* Internal function to remove a callback from a virObjectEventCallbackListPtr
|
|
|
|
*/
|
|
|
|
static int
|
|
|
|
virObjectEventCallbackListRemoveID(virConnectPtr conn,
|
|
|
|
virObjectEventCallbackListPtr cbList,
|
|
|
|
int callbackID)
|
|
|
|
{
|
|
|
|
size_t i;
|
|
|
|
|
2014-01-03 20:31:48 +00:00
|
|
|
for (i = 0; i < cbList->count; i++) {
|
|
|
|
virObjectEventCallbackPtr cb = cbList->callbacks[i];
|
|
|
|
|
|
|
|
if (cb->callbackID == callbackID && cb->conn == conn) {
|
2014-01-06 12:32:55 +00:00
|
|
|
int ret;
|
|
|
|
|
|
|
|
ret = virObjectEventCallbackListCount(conn, cbList, cb->klass,
|
|
|
|
cb->eventID,
|
|
|
|
cb->uuid_filter ? cb->uuid : NULL,
|
|
|
|
cb->remoteID >= 0) - 1;
|
event: properly filter count of remaining events
On the surface, this sequence of API calls should succeed:
id1 = virConnectDomainEventRegisterAny(..., VIR_DOMAIN_EVENT_ID_LIFECYCLE,...);
id2 = virConnectDomainEventRegisterAny(..., VIR_DOMAIN_EVENT_ID_RTC_CHANGE,...);
virConnectDomainEventDeregisterAny(id1);
id1 = virConnectDomainEventRegisterAny(..., VIR_DOMAIN_EVENT_ID_LIFECYCLE,...);
And for test:///default, it does. But for qemu:///system, it fails:
libvirt: XML-RPC error : internal error: domain event 0 already registered
Looking closer, the bug is caused by miscommunication between
the object event engine and the client side of the remote driver.
In our implementation, we set up a single server-side event per
eventID, then the client side replicates that one event to all
callbacks that have been registered client side. To know when
to turn the server side eventID on or off, the client side must
track how many events for the same eventID have been registered.
But while our code was filtering by eventID on event registration,
it did not filter on event deregistration. So the above API calls
resulted in the deregister returning 1 instead of 0, so no RPC
deregister was issued, and the final register detects on the
server side that the server is already handling eventID 0.
Unfortunately, since the problem is only observable on remote
connections, it's not possible to enhance objecteventtest to
expose the semantics using only public API entry points.
* src/conf/object_event.c (virObjectEventCallbackListCount): New
function.
(virObjectEventCallbackListAddID)
(virObjectEventCallbackListRemoveID)
(virObjectEventCallbackListMarkDeleteID): Use it.
Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-03 21:50:15 +00:00
|
|
|
|
2014-01-03 20:31:48 +00:00
|
|
|
if (cb->freecb)
|
|
|
|
(*cb->freecb)(cb->opaque);
|
|
|
|
virObjectUnref(cb->conn);
|
|
|
|
VIR_FREE(cb);
|
|
|
|
VIR_DELETE_ELEMENT(cbList->callbacks, i, cbList->count);
|
2014-01-06 12:32:55 +00:00
|
|
|
return ret;
|
2013-11-26 14:10:15 +00:00
|
|
|
}
|
|
|
|
}
|
|
|
|
|
2014-01-08 17:10:49 +00:00
|
|
|
virReportError(VIR_ERR_INVALID_ARG,
|
|
|
|
_("could not find event callback %d for deletion"),
|
|
|
|
callbackID);
|
2013-11-26 14:10:15 +00:00
|
|
|
return -1;
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
|
|
static int
|
|
|
|
virObjectEventCallbackListMarkDeleteID(virConnectPtr conn,
|
|
|
|
virObjectEventCallbackListPtr cbList,
|
|
|
|
int callbackID)
|
|
|
|
{
|
|
|
|
size_t i;
|
2014-01-03 20:31:48 +00:00
|
|
|
|
2013-11-26 14:10:15 +00:00
|
|
|
for (i = 0; i < cbList->count; i++) {
|
event: properly filter count of remaining events
On the surface, this sequence of API calls should succeed:
id1 = virConnectDomainEventRegisterAny(..., VIR_DOMAIN_EVENT_ID_LIFECYCLE,...);
id2 = virConnectDomainEventRegisterAny(..., VIR_DOMAIN_EVENT_ID_RTC_CHANGE,...);
virConnectDomainEventDeregisterAny(id1);
id1 = virConnectDomainEventRegisterAny(..., VIR_DOMAIN_EVENT_ID_LIFECYCLE,...);
And for test:///default, it does. But for qemu:///system, it fails:
libvirt: XML-RPC error : internal error: domain event 0 already registered
Looking closer, the bug is caused by miscommunication between
the object event engine and the client side of the remote driver.
In our implementation, we set up a single server-side event per
eventID, then the client side replicates that one event to all
callbacks that have been registered client side. To know when
to turn the server side eventID on or off, the client side must
track how many events for the same eventID have been registered.
But while our code was filtering by eventID on event registration,
it did not filter on event deregistration. So the above API calls
resulted in the deregister returning 1 instead of 0, so no RPC
deregister was issued, and the final register detects on the
server side that the server is already handling eventID 0.
Unfortunately, since the problem is only observable on remote
connections, it's not possible to enhance objecteventtest to
expose the semantics using only public API entry points.
* src/conf/object_event.c (virObjectEventCallbackListCount): New
function.
(virObjectEventCallbackListAddID)
(virObjectEventCallbackListRemoveID)
(virObjectEventCallbackListMarkDeleteID): Use it.
Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-03 21:50:15 +00:00
|
|
|
virObjectEventCallbackPtr cb = cbList->callbacks[i];
|
|
|
|
|
|
|
|
if (cb->callbackID == callbackID && cb->conn == conn) {
|
|
|
|
cb->deleted = true;
|
|
|
|
return virObjectEventCallbackListCount(conn, cbList, cb->klass,
|
2014-01-06 12:32:55 +00:00
|
|
|
cb->eventID,
|
|
|
|
cb->uuid_filter ? cb->uuid : NULL,
|
|
|
|
cb->remoteID >= 0);
|
2013-11-26 14:10:15 +00:00
|
|
|
}
|
|
|
|
}
|
|
|
|
|
2014-01-08 17:10:49 +00:00
|
|
|
virReportError(VIR_ERR_INVALID_ARG,
|
|
|
|
_("could not find event callback %d for deletion"),
|
|
|
|
callbackID);
|
2013-11-26 14:10:15 +00:00
|
|
|
return -1;
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
|
|
static int
|
|
|
|
virObjectEventCallbackListPurgeMarked(virObjectEventCallbackListPtr cbList)
|
|
|
|
{
|
|
|
|
int old_count = cbList->count;
|
|
|
|
int n;
|
|
|
|
for (n = 0; n < cbList->count; n++) {
|
|
|
|
if (cbList->callbacks[n]->deleted) {
|
|
|
|
virFreeCallback freecb = cbList->callbacks[n]->freecb;
|
|
|
|
if (freecb)
|
|
|
|
(*freecb)(cbList->callbacks[n]->opaque);
|
|
|
|
virObjectUnref(cbList->callbacks[n]->conn);
|
|
|
|
VIR_FREE(cbList->callbacks[n]);
|
|
|
|
|
|
|
|
if (n < (cbList->count - 1))
|
|
|
|
memmove(cbList->callbacks + n,
|
|
|
|
cbList->callbacks + n + 1,
|
|
|
|
sizeof(*(cbList->callbacks)) *
|
|
|
|
(cbList->count - (n + 1)));
|
|
|
|
cbList->count--;
|
|
|
|
n--;
|
|
|
|
}
|
|
|
|
}
|
|
|
|
if (cbList->count < old_count &&
|
|
|
|
VIR_REALLOC_N(cbList->callbacks, cbList->count) < 0) {
|
|
|
|
; /* Failure to reduce memory allocation isn't fatal */
|
|
|
|
}
|
|
|
|
return 0;
|
|
|
|
}
|
|
|
|
|
|
|
|
|
event: don't let old-style events clobber per-domain events
Right now, the older virConnectDomainEventRegister (takes a
function pointer, returns 0 on success) and the newer
virConnectDomainEventRegisterID (takes an eventID, returns a
callbackID) share the underlying implementation (the older
API ends up consuming a callbackID for eventID 0 under the
hood). We implemented that by a lot of copy and pasted
code between object_event.c and domain_event.c, according to
whether we are dealing with a function pointer or an eventID.
However, our copy and paste is not symmetric. Consider this
sequence:
id1 = virConnectDomainEventRegisterAny(conn, dom,
VIR_DOMAIN_EVENT_ID_LIFECYCLE,
VIR_DOMAIN_EVENT_CALLBACK(callback), NULL, NULL);
virConnectDomainEventRegister(conn, callback, NULL, NULL);
virConnectDomainEventDeregister(conn, callback);
virConnectDomainEventDeregsiterAny(conn, id1);
the first three calls would succeed, but the third call ended
up nuking the id1 callbackID (the per-domain new-style handler),
then the fourth call failed with an error about an unknown
callbackID, leaving us with the global handler (old-style) still
live and receiving events. It required another old-style
deregister to clean up the mess. Root cause was that
virDomainEventCallbackList{Remove,MarkDelete} were only
checking for function pointer match, rather than also checking
for whether the registration was global.
Rather than playing with the guts of object_event ourselves
in domain_event, it is nicer to add a mapping function for the
internal callback id, then share common code for event removal.
For now, the function-to-id mapping is used only internally;
I thought about whether a new public API to let a user learn
the callback would be useful, but decided exposing this to the
user is probably a disservice, since we already publicly
document that they should avoid the old style, and since this
patch already demonstrates that older libvirt versions have
weird behavior when mixing old and new styles.
And like all good bug fix patches, I enhanced the testsuite,
validating that the changes in tests/ expose the failure
without the rest of the patch.
* src/conf/object_event.c (virObjectEventCallbackLookup)
(virObjectEventStateCallbackID): New functions.
(virObjectEventCallbackLookup): Use helper function.
* src/conf/object_event_private.h (virObjectEventStateCallbackID):
Declare new function.
* src/conf/domain_event.c (virDomainEventStateRegister)
(virDomainEventStateDeregister): Let common code handle the
complexity.
(virDomainEventCallbackListRemove)
(virDomainEventCallbackListMarkDelete)
(virDomainEventCallbackListAdd): Drop unused functions.
* tests/objecteventtest.c (testDomainCreateXMLMixed): New test.
Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-03 23:50:14 +00:00
|
|
|
/**
|
|
|
|
* virObjectEventCallbackLookup:
|
|
|
|
* @conn: pointer to the connection
|
|
|
|
* @cbList: the list
|
|
|
|
* @uuid: the uuid of the object to filter on
|
|
|
|
* @klass: the base event class
|
|
|
|
* @eventID: the event ID
|
|
|
|
* @callback: the callback to locate
|
event: don't allow mix of old- and new-style registration
Consider these two calls, in either order:
id1 = virConnectDomainEventRegisterAny(conn, NULL,
VIR_DOMAIN_EVENT_ID_LIFECYCLE,
VIR_DOMAIN_EVENT_CALLBACK(callback), NULL, NULL);
virConnectDomainEventRegister(conn, callback, NULL, NULL);
Right now, the second call fails, because under the hood, the
old-style function registration is tightly coupled to the
new style lifecycle eventID, and the two calls both try
to register the same global eventID callback representation.
We've alreay documented that users should avoid old-style
registration and deregistration, so anyone heeding the advice
won't run into this situation. But it would be even nicer if
we pretend the two interfaces are completely separate, and
disallow any cross-linking. That is, a call to old-style
deregister should never remove a new-style callback even if it
is the same function pointer, and a call to new-style callback
using only callbackIDs obtained legitimately should never
remove an old-style callback (of course, since our callback
IDs are sequential, and there is still coupling under the
hood, you can easily guess the callbackID of an old style
registration and use new-style deregistration to nuke it - but
that starts to be blatantly bad coding on your part rather
than a surprising result on what looks like reasonable
stand-alone API).
With this patch, you can now register a global lifecycle event
handler twice, by using both old and new APIs; if such an event
occurs, your callback will be entered twice. But that is not a
problem in practice, since it is already possible to use the
new API to register both a global and per-domain event handler
using the same function, which will likewise fire your callback
twice for that domain. Duplicates are still prevented when
using the same API with same parameters twice (old-style twice,
new-style global twice, or new-style per-domain with same domain
twice), and things are still bounded (it is not possible to
register a single function pointer more than N+2 times per event
id, where N is the number of domains available on the connection).
Besides, it has always been possible to register as many
separate function pointers on the same event id as desired,
through either old or new style API, where the bound there is
the physical limitation of writing a program with enough
distinct function pointers.
Adding another event registration in the testsuite is sufficient
to cover this, where the test fails without the rest of the patch.
* src/conf/object_event.c (_virObjectEventCallback): Add field.
(virObjectEventCallbackLookup): Add argument.
(virObjectEventCallbackListAddID, virObjectEventStateCallbackID):
Adjust callers.
* tests/objecteventtest.c (testDomainCreateXMLMixed): Enhance test.
Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-04 12:55:55 +00:00
|
|
|
* @legacy: true if callback is tracked by function instead of callbackID
|
2014-01-06 12:32:55 +00:00
|
|
|
* @remoteID: optionally return a known remoteID
|
event: don't let old-style events clobber per-domain events
Right now, the older virConnectDomainEventRegister (takes a
function pointer, returns 0 on success) and the newer
virConnectDomainEventRegisterID (takes an eventID, returns a
callbackID) share the underlying implementation (the older
API ends up consuming a callbackID for eventID 0 under the
hood). We implemented that by a lot of copy and pasted
code between object_event.c and domain_event.c, according to
whether we are dealing with a function pointer or an eventID.
However, our copy and paste is not symmetric. Consider this
sequence:
id1 = virConnectDomainEventRegisterAny(conn, dom,
VIR_DOMAIN_EVENT_ID_LIFECYCLE,
VIR_DOMAIN_EVENT_CALLBACK(callback), NULL, NULL);
virConnectDomainEventRegister(conn, callback, NULL, NULL);
virConnectDomainEventDeregister(conn, callback);
virConnectDomainEventDeregsiterAny(conn, id1);
the first three calls would succeed, but the third call ended
up nuking the id1 callbackID (the per-domain new-style handler),
then the fourth call failed with an error about an unknown
callbackID, leaving us with the global handler (old-style) still
live and receiving events. It required another old-style
deregister to clean up the mess. Root cause was that
virDomainEventCallbackList{Remove,MarkDelete} were only
checking for function pointer match, rather than also checking
for whether the registration was global.
Rather than playing with the guts of object_event ourselves
in domain_event, it is nicer to add a mapping function for the
internal callback id, then share common code for event removal.
For now, the function-to-id mapping is used only internally;
I thought about whether a new public API to let a user learn
the callback would be useful, but decided exposing this to the
user is probably a disservice, since we already publicly
document that they should avoid the old style, and since this
patch already demonstrates that older libvirt versions have
weird behavior when mixing old and new styles.
And like all good bug fix patches, I enhanced the testsuite,
validating that the changes in tests/ expose the failure
without the rest of the patch.
* src/conf/object_event.c (virObjectEventCallbackLookup)
(virObjectEventStateCallbackID): New functions.
(virObjectEventCallbackLookup): Use helper function.
* src/conf/object_event_private.h (virObjectEventStateCallbackID):
Declare new function.
* src/conf/domain_event.c (virDomainEventStateRegister)
(virDomainEventStateDeregister): Let common code handle the
complexity.
(virDomainEventCallbackListRemove)
(virDomainEventCallbackListMarkDelete)
(virDomainEventCallbackListAdd): Drop unused functions.
* tests/objecteventtest.c (testDomainCreateXMLMixed): New test.
Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-03 23:50:14 +00:00
|
|
|
*
|
|
|
|
* Internal function to determine if @callback already has a
|
2014-01-06 12:32:55 +00:00
|
|
|
* callbackID in @cbList for the given @conn and other filters. If
|
|
|
|
* @remoteID is non-NULL, and another callback exists that can be
|
|
|
|
* serviced by the same remote event, then set it to that remote ID.
|
|
|
|
*
|
event: don't let old-style events clobber per-domain events
Right now, the older virConnectDomainEventRegister (takes a
function pointer, returns 0 on success) and the newer
virConnectDomainEventRegisterID (takes an eventID, returns a
callbackID) share the underlying implementation (the older
API ends up consuming a callbackID for eventID 0 under the
hood). We implemented that by a lot of copy and pasted
code between object_event.c and domain_event.c, according to
whether we are dealing with a function pointer or an eventID.
However, our copy and paste is not symmetric. Consider this
sequence:
id1 = virConnectDomainEventRegisterAny(conn, dom,
VIR_DOMAIN_EVENT_ID_LIFECYCLE,
VIR_DOMAIN_EVENT_CALLBACK(callback), NULL, NULL);
virConnectDomainEventRegister(conn, callback, NULL, NULL);
virConnectDomainEventDeregister(conn, callback);
virConnectDomainEventDeregsiterAny(conn, id1);
the first three calls would succeed, but the third call ended
up nuking the id1 callbackID (the per-domain new-style handler),
then the fourth call failed with an error about an unknown
callbackID, leaving us with the global handler (old-style) still
live and receiving events. It required another old-style
deregister to clean up the mess. Root cause was that
virDomainEventCallbackList{Remove,MarkDelete} were only
checking for function pointer match, rather than also checking
for whether the registration was global.
Rather than playing with the guts of object_event ourselves
in domain_event, it is nicer to add a mapping function for the
internal callback id, then share common code for event removal.
For now, the function-to-id mapping is used only internally;
I thought about whether a new public API to let a user learn
the callback would be useful, but decided exposing this to the
user is probably a disservice, since we already publicly
document that they should avoid the old style, and since this
patch already demonstrates that older libvirt versions have
weird behavior when mixing old and new styles.
And like all good bug fix patches, I enhanced the testsuite,
validating that the changes in tests/ expose the failure
without the rest of the patch.
* src/conf/object_event.c (virObjectEventCallbackLookup)
(virObjectEventStateCallbackID): New functions.
(virObjectEventCallbackLookup): Use helper function.
* src/conf/object_event_private.h (virObjectEventStateCallbackID):
Declare new function.
* src/conf/domain_event.c (virDomainEventStateRegister)
(virDomainEventStateDeregister): Let common code handle the
complexity.
(virDomainEventCallbackListRemove)
(virDomainEventCallbackListMarkDelete)
(virDomainEventCallbackListAdd): Drop unused functions.
* tests/objecteventtest.c (testDomainCreateXMLMixed): New test.
Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-03 23:50:14 +00:00
|
|
|
* Return the id if found, or -1 with no error issued if not present.
|
|
|
|
*/
|
|
|
|
static int ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2)
|
|
|
|
virObjectEventCallbackLookup(virConnectPtr conn,
|
|
|
|
virObjectEventCallbackListPtr cbList,
|
|
|
|
unsigned char uuid[VIR_UUID_BUFLEN],
|
|
|
|
virClassPtr klass,
|
|
|
|
int eventID,
|
event: don't allow mix of old- and new-style registration
Consider these two calls, in either order:
id1 = virConnectDomainEventRegisterAny(conn, NULL,
VIR_DOMAIN_EVENT_ID_LIFECYCLE,
VIR_DOMAIN_EVENT_CALLBACK(callback), NULL, NULL);
virConnectDomainEventRegister(conn, callback, NULL, NULL);
Right now, the second call fails, because under the hood, the
old-style function registration is tightly coupled to the
new style lifecycle eventID, and the two calls both try
to register the same global eventID callback representation.
We've alreay documented that users should avoid old-style
registration and deregistration, so anyone heeding the advice
won't run into this situation. But it would be even nicer if
we pretend the two interfaces are completely separate, and
disallow any cross-linking. That is, a call to old-style
deregister should never remove a new-style callback even if it
is the same function pointer, and a call to new-style callback
using only callbackIDs obtained legitimately should never
remove an old-style callback (of course, since our callback
IDs are sequential, and there is still coupling under the
hood, you can easily guess the callbackID of an old style
registration and use new-style deregistration to nuke it - but
that starts to be blatantly bad coding on your part rather
than a surprising result on what looks like reasonable
stand-alone API).
With this patch, you can now register a global lifecycle event
handler twice, by using both old and new APIs; if such an event
occurs, your callback will be entered twice. But that is not a
problem in practice, since it is already possible to use the
new API to register both a global and per-domain event handler
using the same function, which will likewise fire your callback
twice for that domain. Duplicates are still prevented when
using the same API with same parameters twice (old-style twice,
new-style global twice, or new-style per-domain with same domain
twice), and things are still bounded (it is not possible to
register a single function pointer more than N+2 times per event
id, where N is the number of domains available on the connection).
Besides, it has always been possible to register as many
separate function pointers on the same event id as desired,
through either old or new style API, where the bound there is
the physical limitation of writing a program with enough
distinct function pointers.
Adding another event registration in the testsuite is sufficient
to cover this, where the test fails without the rest of the patch.
* src/conf/object_event.c (_virObjectEventCallback): Add field.
(virObjectEventCallbackLookup): Add argument.
(virObjectEventCallbackListAddID, virObjectEventStateCallbackID):
Adjust callers.
* tests/objecteventtest.c (testDomainCreateXMLMixed): Enhance test.
Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-04 12:55:55 +00:00
|
|
|
virConnectObjectEventGenericCallback callback,
|
2014-01-06 12:32:55 +00:00
|
|
|
bool legacy,
|
|
|
|
int *remoteID)
|
event: don't let old-style events clobber per-domain events
Right now, the older virConnectDomainEventRegister (takes a
function pointer, returns 0 on success) and the newer
virConnectDomainEventRegisterID (takes an eventID, returns a
callbackID) share the underlying implementation (the older
API ends up consuming a callbackID for eventID 0 under the
hood). We implemented that by a lot of copy and pasted
code between object_event.c and domain_event.c, according to
whether we are dealing with a function pointer or an eventID.
However, our copy and paste is not symmetric. Consider this
sequence:
id1 = virConnectDomainEventRegisterAny(conn, dom,
VIR_DOMAIN_EVENT_ID_LIFECYCLE,
VIR_DOMAIN_EVENT_CALLBACK(callback), NULL, NULL);
virConnectDomainEventRegister(conn, callback, NULL, NULL);
virConnectDomainEventDeregister(conn, callback);
virConnectDomainEventDeregsiterAny(conn, id1);
the first three calls would succeed, but the third call ended
up nuking the id1 callbackID (the per-domain new-style handler),
then the fourth call failed with an error about an unknown
callbackID, leaving us with the global handler (old-style) still
live and receiving events. It required another old-style
deregister to clean up the mess. Root cause was that
virDomainEventCallbackList{Remove,MarkDelete} were only
checking for function pointer match, rather than also checking
for whether the registration was global.
Rather than playing with the guts of object_event ourselves
in domain_event, it is nicer to add a mapping function for the
internal callback id, then share common code for event removal.
For now, the function-to-id mapping is used only internally;
I thought about whether a new public API to let a user learn
the callback would be useful, but decided exposing this to the
user is probably a disservice, since we already publicly
document that they should avoid the old style, and since this
patch already demonstrates that older libvirt versions have
weird behavior when mixing old and new styles.
And like all good bug fix patches, I enhanced the testsuite,
validating that the changes in tests/ expose the failure
without the rest of the patch.
* src/conf/object_event.c (virObjectEventCallbackLookup)
(virObjectEventStateCallbackID): New functions.
(virObjectEventCallbackLookup): Use helper function.
* src/conf/object_event_private.h (virObjectEventStateCallbackID):
Declare new function.
* src/conf/domain_event.c (virDomainEventStateRegister)
(virDomainEventStateDeregister): Let common code handle the
complexity.
(virDomainEventCallbackListRemove)
(virDomainEventCallbackListMarkDelete)
(virDomainEventCallbackListAdd): Drop unused functions.
* tests/objecteventtest.c (testDomainCreateXMLMixed): New test.
Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-03 23:50:14 +00:00
|
|
|
{
|
|
|
|
size_t i;
|
|
|
|
|
2014-01-06 12:32:55 +00:00
|
|
|
if (remoteID)
|
|
|
|
*remoteID = -1;
|
|
|
|
|
event: don't let old-style events clobber per-domain events
Right now, the older virConnectDomainEventRegister (takes a
function pointer, returns 0 on success) and the newer
virConnectDomainEventRegisterID (takes an eventID, returns a
callbackID) share the underlying implementation (the older
API ends up consuming a callbackID for eventID 0 under the
hood). We implemented that by a lot of copy and pasted
code between object_event.c and domain_event.c, according to
whether we are dealing with a function pointer or an eventID.
However, our copy and paste is not symmetric. Consider this
sequence:
id1 = virConnectDomainEventRegisterAny(conn, dom,
VIR_DOMAIN_EVENT_ID_LIFECYCLE,
VIR_DOMAIN_EVENT_CALLBACK(callback), NULL, NULL);
virConnectDomainEventRegister(conn, callback, NULL, NULL);
virConnectDomainEventDeregister(conn, callback);
virConnectDomainEventDeregsiterAny(conn, id1);
the first three calls would succeed, but the third call ended
up nuking the id1 callbackID (the per-domain new-style handler),
then the fourth call failed with an error about an unknown
callbackID, leaving us with the global handler (old-style) still
live and receiving events. It required another old-style
deregister to clean up the mess. Root cause was that
virDomainEventCallbackList{Remove,MarkDelete} were only
checking for function pointer match, rather than also checking
for whether the registration was global.
Rather than playing with the guts of object_event ourselves
in domain_event, it is nicer to add a mapping function for the
internal callback id, then share common code for event removal.
For now, the function-to-id mapping is used only internally;
I thought about whether a new public API to let a user learn
the callback would be useful, but decided exposing this to the
user is probably a disservice, since we already publicly
document that they should avoid the old style, and since this
patch already demonstrates that older libvirt versions have
weird behavior when mixing old and new styles.
And like all good bug fix patches, I enhanced the testsuite,
validating that the changes in tests/ expose the failure
without the rest of the patch.
* src/conf/object_event.c (virObjectEventCallbackLookup)
(virObjectEventStateCallbackID): New functions.
(virObjectEventCallbackLookup): Use helper function.
* src/conf/object_event_private.h (virObjectEventStateCallbackID):
Declare new function.
* src/conf/domain_event.c (virDomainEventStateRegister)
(virDomainEventStateDeregister): Let common code handle the
complexity.
(virDomainEventCallbackListRemove)
(virDomainEventCallbackListMarkDelete)
(virDomainEventCallbackListAdd): Drop unused functions.
* tests/objecteventtest.c (testDomainCreateXMLMixed): New test.
Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-03 23:50:14 +00:00
|
|
|
for (i = 0; i < cbList->count; i++) {
|
|
|
|
virObjectEventCallbackPtr cb = cbList->callbacks[i];
|
|
|
|
|
|
|
|
if (cb->deleted)
|
|
|
|
continue;
|
2014-01-06 12:32:55 +00:00
|
|
|
if (cb->klass == klass &&
|
event: don't let old-style events clobber per-domain events
Right now, the older virConnectDomainEventRegister (takes a
function pointer, returns 0 on success) and the newer
virConnectDomainEventRegisterID (takes an eventID, returns a
callbackID) share the underlying implementation (the older
API ends up consuming a callbackID for eventID 0 under the
hood). We implemented that by a lot of copy and pasted
code between object_event.c and domain_event.c, according to
whether we are dealing with a function pointer or an eventID.
However, our copy and paste is not symmetric. Consider this
sequence:
id1 = virConnectDomainEventRegisterAny(conn, dom,
VIR_DOMAIN_EVENT_ID_LIFECYCLE,
VIR_DOMAIN_EVENT_CALLBACK(callback), NULL, NULL);
virConnectDomainEventRegister(conn, callback, NULL, NULL);
virConnectDomainEventDeregister(conn, callback);
virConnectDomainEventDeregsiterAny(conn, id1);
the first three calls would succeed, but the third call ended
up nuking the id1 callbackID (the per-domain new-style handler),
then the fourth call failed with an error about an unknown
callbackID, leaving us with the global handler (old-style) still
live and receiving events. It required another old-style
deregister to clean up the mess. Root cause was that
virDomainEventCallbackList{Remove,MarkDelete} were only
checking for function pointer match, rather than also checking
for whether the registration was global.
Rather than playing with the guts of object_event ourselves
in domain_event, it is nicer to add a mapping function for the
internal callback id, then share common code for event removal.
For now, the function-to-id mapping is used only internally;
I thought about whether a new public API to let a user learn
the callback would be useful, but decided exposing this to the
user is probably a disservice, since we already publicly
document that they should avoid the old style, and since this
patch already demonstrates that older libvirt versions have
weird behavior when mixing old and new styles.
And like all good bug fix patches, I enhanced the testsuite,
validating that the changes in tests/ expose the failure
without the rest of the patch.
* src/conf/object_event.c (virObjectEventCallbackLookup)
(virObjectEventStateCallbackID): New functions.
(virObjectEventCallbackLookup): Use helper function.
* src/conf/object_event_private.h (virObjectEventStateCallbackID):
Declare new function.
* src/conf/domain_event.c (virDomainEventStateRegister)
(virDomainEventStateDeregister): Let common code handle the
complexity.
(virDomainEventCallbackListRemove)
(virDomainEventCallbackListMarkDelete)
(virDomainEventCallbackListAdd): Drop unused functions.
* tests/objecteventtest.c (testDomainCreateXMLMixed): New test.
Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-03 23:50:14 +00:00
|
|
|
cb->eventID == eventID &&
|
|
|
|
cb->conn == conn &&
|
event: don't allow mix of old- and new-style registration
Consider these two calls, in either order:
id1 = virConnectDomainEventRegisterAny(conn, NULL,
VIR_DOMAIN_EVENT_ID_LIFECYCLE,
VIR_DOMAIN_EVENT_CALLBACK(callback), NULL, NULL);
virConnectDomainEventRegister(conn, callback, NULL, NULL);
Right now, the second call fails, because under the hood, the
old-style function registration is tightly coupled to the
new style lifecycle eventID, and the two calls both try
to register the same global eventID callback representation.
We've alreay documented that users should avoid old-style
registration and deregistration, so anyone heeding the advice
won't run into this situation. But it would be even nicer if
we pretend the two interfaces are completely separate, and
disallow any cross-linking. That is, a call to old-style
deregister should never remove a new-style callback even if it
is the same function pointer, and a call to new-style callback
using only callbackIDs obtained legitimately should never
remove an old-style callback (of course, since our callback
IDs are sequential, and there is still coupling under the
hood, you can easily guess the callbackID of an old style
registration and use new-style deregistration to nuke it - but
that starts to be blatantly bad coding on your part rather
than a surprising result on what looks like reasonable
stand-alone API).
With this patch, you can now register a global lifecycle event
handler twice, by using both old and new APIs; if such an event
occurs, your callback will be entered twice. But that is not a
problem in practice, since it is already possible to use the
new API to register both a global and per-domain event handler
using the same function, which will likewise fire your callback
twice for that domain. Duplicates are still prevented when
using the same API with same parameters twice (old-style twice,
new-style global twice, or new-style per-domain with same domain
twice), and things are still bounded (it is not possible to
register a single function pointer more than N+2 times per event
id, where N is the number of domains available on the connection).
Besides, it has always been possible to register as many
separate function pointers on the same event id as desired,
through either old or new style API, where the bound there is
the physical limitation of writing a program with enough
distinct function pointers.
Adding another event registration in the testsuite is sufficient
to cover this, where the test fails without the rest of the patch.
* src/conf/object_event.c (_virObjectEventCallback): Add field.
(virObjectEventCallbackLookup): Add argument.
(virObjectEventCallbackListAddID, virObjectEventStateCallbackID):
Adjust callers.
* tests/objecteventtest.c (testDomainCreateXMLMixed): Enhance test.
Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-04 12:55:55 +00:00
|
|
|
cb->legacy == legacy &&
|
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.
Note that virObjectEventPtr still needs all fields of meta,
because this is how we reconstruct a virDomainPtr inside the
dispatch handler before calling the end user's callback
pointer with the correct object, even though only the uuid
portion of meta is used in deciding whether a callback
matches the given event. So while uuid is optional for
callbacks, it is mandatory for events.
The change to testDomainCreateXMLMixed is merely on the setup
scenario (as you can't register for a domain unless it is either
running or persistent). I actually first wrote that test for
this patch, then rebased it to also cover a prior patch (commit
4221d64), but had to adjust it for that patch to use Create
instead of Define for setting up the domain long enough to
register the event in order to work around this bug. But while
the setup is changed, the main body of the test is still about
whether creation events fire as expected.
* 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_private.h (virObjectEventNew): Document
use of name and uuid in events.
* 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.
* tests/objecteventtest.c (testDomainCreateXMLMixed): Enhance test.
Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-04 00:02:17 +00:00
|
|
|
((uuid && cb->uuid_filter &&
|
|
|
|
memcmp(cb->uuid, uuid, VIR_UUID_BUFLEN) == 0) ||
|
|
|
|
(!uuid && !cb->uuid_filter))) {
|
2014-01-06 12:32:55 +00:00
|
|
|
if (remoteID)
|
|
|
|
*remoteID = cb->remoteID;
|
|
|
|
if (cb->cb == callback)
|
|
|
|
return cb->callbackID;
|
event: don't let old-style events clobber per-domain events
Right now, the older virConnectDomainEventRegister (takes a
function pointer, returns 0 on success) and the newer
virConnectDomainEventRegisterID (takes an eventID, returns a
callbackID) share the underlying implementation (the older
API ends up consuming a callbackID for eventID 0 under the
hood). We implemented that by a lot of copy and pasted
code between object_event.c and domain_event.c, according to
whether we are dealing with a function pointer or an eventID.
However, our copy and paste is not symmetric. Consider this
sequence:
id1 = virConnectDomainEventRegisterAny(conn, dom,
VIR_DOMAIN_EVENT_ID_LIFECYCLE,
VIR_DOMAIN_EVENT_CALLBACK(callback), NULL, NULL);
virConnectDomainEventRegister(conn, callback, NULL, NULL);
virConnectDomainEventDeregister(conn, callback);
virConnectDomainEventDeregsiterAny(conn, id1);
the first three calls would succeed, but the third call ended
up nuking the id1 callbackID (the per-domain new-style handler),
then the fourth call failed with an error about an unknown
callbackID, leaving us with the global handler (old-style) still
live and receiving events. It required another old-style
deregister to clean up the mess. Root cause was that
virDomainEventCallbackList{Remove,MarkDelete} were only
checking for function pointer match, rather than also checking
for whether the registration was global.
Rather than playing with the guts of object_event ourselves
in domain_event, it is nicer to add a mapping function for the
internal callback id, then share common code for event removal.
For now, the function-to-id mapping is used only internally;
I thought about whether a new public API to let a user learn
the callback would be useful, but decided exposing this to the
user is probably a disservice, since we already publicly
document that they should avoid the old style, and since this
patch already demonstrates that older libvirt versions have
weird behavior when mixing old and new styles.
And like all good bug fix patches, I enhanced the testsuite,
validating that the changes in tests/ expose the failure
without the rest of the patch.
* src/conf/object_event.c (virObjectEventCallbackLookup)
(virObjectEventStateCallbackID): New functions.
(virObjectEventCallbackLookup): Use helper function.
* src/conf/object_event_private.h (virObjectEventStateCallbackID):
Declare new function.
* src/conf/domain_event.c (virDomainEventStateRegister)
(virDomainEventStateDeregister): Let common code handle the
complexity.
(virDomainEventCallbackListRemove)
(virDomainEventCallbackListMarkDelete)
(virDomainEventCallbackListAdd): Drop unused functions.
* tests/objecteventtest.c (testDomainCreateXMLMixed): New test.
Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-03 23:50:14 +00:00
|
|
|
}
|
|
|
|
}
|
2014-01-06 12:32:55 +00:00
|
|
|
return -1;
|
event: don't let old-style events clobber per-domain events
Right now, the older virConnectDomainEventRegister (takes a
function pointer, returns 0 on success) and the newer
virConnectDomainEventRegisterID (takes an eventID, returns a
callbackID) share the underlying implementation (the older
API ends up consuming a callbackID for eventID 0 under the
hood). We implemented that by a lot of copy and pasted
code between object_event.c and domain_event.c, according to
whether we are dealing with a function pointer or an eventID.
However, our copy and paste is not symmetric. Consider this
sequence:
id1 = virConnectDomainEventRegisterAny(conn, dom,
VIR_DOMAIN_EVENT_ID_LIFECYCLE,
VIR_DOMAIN_EVENT_CALLBACK(callback), NULL, NULL);
virConnectDomainEventRegister(conn, callback, NULL, NULL);
virConnectDomainEventDeregister(conn, callback);
virConnectDomainEventDeregsiterAny(conn, id1);
the first three calls would succeed, but the third call ended
up nuking the id1 callbackID (the per-domain new-style handler),
then the fourth call failed with an error about an unknown
callbackID, leaving us with the global handler (old-style) still
live and receiving events. It required another old-style
deregister to clean up the mess. Root cause was that
virDomainEventCallbackList{Remove,MarkDelete} were only
checking for function pointer match, rather than also checking
for whether the registration was global.
Rather than playing with the guts of object_event ourselves
in domain_event, it is nicer to add a mapping function for the
internal callback id, then share common code for event removal.
For now, the function-to-id mapping is used only internally;
I thought about whether a new public API to let a user learn
the callback would be useful, but decided exposing this to the
user is probably a disservice, since we already publicly
document that they should avoid the old style, and since this
patch already demonstrates that older libvirt versions have
weird behavior when mixing old and new styles.
And like all good bug fix patches, I enhanced the testsuite,
validating that the changes in tests/ expose the failure
without the rest of the patch.
* src/conf/object_event.c (virObjectEventCallbackLookup)
(virObjectEventStateCallbackID): New functions.
(virObjectEventCallbackLookup): Use helper function.
* src/conf/object_event_private.h (virObjectEventStateCallbackID):
Declare new function.
* src/conf/domain_event.c (virDomainEventStateRegister)
(virDomainEventStateDeregister): Let common code handle the
complexity.
(virDomainEventCallbackListRemove)
(virDomainEventCallbackListMarkDelete)
(virDomainEventCallbackListAdd): Drop unused functions.
* tests/objecteventtest.c (testDomainCreateXMLMixed): New test.
Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-03 23:50:14 +00:00
|
|
|
}
|
|
|
|
|
|
|
|
|
2013-11-26 14:10:15 +00:00
|
|
|
/**
|
|
|
|
* virObjectEventCallbackListAddID:
|
|
|
|
* @conn: pointer to the connection
|
|
|
|
* @cbList: the list
|
event: filter global events by domain:getattr ACL [CVE-2014-0028]
Ever since ACL filtering was added in commit 7639736 (v1.1.1), a
user could still use event registration to obtain access to a
domain that they could not normally access via virDomainLookup*
or virConnectListAllDomains and friends. We already have the
framework in the RPC generator for creating the filter, and
previous cleanup patches got us to the point that we can now
wire the filter through the entire object event stack.
Furthermore, whether or not domain:getattr is honored, use of
global events is a form of obtaining a list of networks, which
is covered by connect:search_domains added in a93cd08 (v1.1.0).
Ideally, we'd have a way to enforce connect:search_domains when
doing global registrations while omitting that check on a
per-domain registration. But this patch just unconditionally
requires connect:search_domains, even when no list could be
obtained, based on the following observations:
1. Administrators are unlikely to grant domain:getattr for one
or all domains while still denying connect:search_domains - a
user that is able to manage domains will want to be able to
manage them efficiently, but efficient management includes being
able to list the domains they can access. The idea of denying
connect:search_domains while still granting access to individual
domains is therefore not adding any real security, but just
serves as a layer of obscurity to annoy the end user.
2. In the current implementation, domain events are filtered
on the client; the server has no idea if a domain filter was
requested, and must therefore assume that all domain event
requests are global. Even if we fix the RPC protocol to
allow for server-side filtering for newer client/server combos,
making the connect:serach_domains ACL check conditional on
whether the domain argument was NULL won't benefit older clients.
Therefore, we choose to document that connect:search_domains
is a pre-requisite to any domain event management.
Network events need the same treatment, with the obvious
change of using connect:search_networks and network:getattr.
* src/access/viraccessperm.h
(VIR_ACCESS_PERM_CONNECT_SEARCH_DOMAINS)
(VIR_ACCESS_PERM_CONNECT_SEARCH_NETWORKS): Document additional
effect of the permission.
* src/conf/domain_event.h (virDomainEventStateRegister)
(virDomainEventStateRegisterID): Add new parameter.
* src/conf/network_event.h (virNetworkEventStateRegisterID):
Likewise.
* src/conf/object_event_private.h (virObjectEventStateRegisterID):
Likewise.
* src/conf/object_event.c (_virObjectEventCallback): Track a filter.
(virObjectEventDispatchMatchCallback): Use filter.
(virObjectEventCallbackListAddID): Register filter.
* src/conf/domain_event.c (virDomainEventFilter): New function.
(virDomainEventStateRegister, virDomainEventStateRegisterID):
Adjust callers.
* src/conf/network_event.c (virNetworkEventFilter): New function.
(virNetworkEventStateRegisterID): Adjust caller.
* src/remote/remote_protocol.x
(REMOTE_PROC_CONNECT_DOMAIN_EVENT_REGISTER)
(REMOTE_PROC_CONNECT_DOMAIN_EVENT_REGISTER_ANY)
(REMOTE_PROC_CONNECT_NETWORK_EVENT_REGISTER_ANY): Generate a
filter, and require connect:search_domains instead of weaker
connect:read.
* src/test/test_driver.c (testConnectDomainEventRegister)
(testConnectDomainEventRegisterAny)
(testConnectNetworkEventRegisterAny): Update callers.
* src/remote/remote_driver.c (remoteConnectDomainEventRegister)
(remoteConnectDomainEventRegisterAny): Likewise.
* src/xen/xen_driver.c (xenUnifiedConnectDomainEventRegister)
(xenUnifiedConnectDomainEventRegisterAny): Likewise.
* src/vbox/vbox_tmpl.c (vboxDomainGetXMLDesc): Likewise.
* src/libxl/libxl_driver.c (libxlConnectDomainEventRegister)
(libxlConnectDomainEventRegisterAny): Likewise.
* src/qemu/qemu_driver.c (qemuConnectDomainEventRegister)
(qemuConnectDomainEventRegisterAny): Likewise.
* src/uml/uml_driver.c (umlConnectDomainEventRegister)
(umlConnectDomainEventRegisterAny): Likewise.
* src/network/bridge_driver.c
(networkConnectNetworkEventRegisterAny): Likewise.
* src/lxc/lxc_driver.c (lxcConnectDomainEventRegister)
(lxcConnectDomainEventRegisterAny): Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-08 20:34:48 +00:00
|
|
|
* @uuid: the optional uuid of the object to filter on
|
|
|
|
* @filter: optional last-ditch filter callback
|
|
|
|
* @filter_opaque: opaque data to pass to @filter
|
2013-12-11 14:10:34 +00:00
|
|
|
* @klass: the base event class
|
2013-11-26 14:10:15 +00:00
|
|
|
* @eventID: the event ID
|
|
|
|
* @callback: the callback to add
|
2014-01-02 14:16:49 +00:00
|
|
|
* @opaque: opaque data to pass to @callback
|
|
|
|
* @freecb: callback to free @opaque
|
2013-11-26 14:10:15 +00:00
|
|
|
* @callbackID: filled with callback ID
|
2014-01-06 12:32:55 +00:00
|
|
|
* @serverFilter: true if server supports object filtering
|
2013-11-26 14:10:15 +00:00
|
|
|
*
|
|
|
|
* Internal function to add a callback from a virObjectEventCallbackListPtr
|
|
|
|
*/
|
2014-01-03 21:44:29 +00:00
|
|
|
static int
|
2013-11-26 14:10:15 +00:00
|
|
|
virObjectEventCallbackListAddID(virConnectPtr conn,
|
|
|
|
virObjectEventCallbackListPtr cbList,
|
|
|
|
unsigned char uuid[VIR_UUID_BUFLEN],
|
event: filter global events by domain:getattr ACL [CVE-2014-0028]
Ever since ACL filtering was added in commit 7639736 (v1.1.1), a
user could still use event registration to obtain access to a
domain that they could not normally access via virDomainLookup*
or virConnectListAllDomains and friends. We already have the
framework in the RPC generator for creating the filter, and
previous cleanup patches got us to the point that we can now
wire the filter through the entire object event stack.
Furthermore, whether or not domain:getattr is honored, use of
global events is a form of obtaining a list of networks, which
is covered by connect:search_domains added in a93cd08 (v1.1.0).
Ideally, we'd have a way to enforce connect:search_domains when
doing global registrations while omitting that check on a
per-domain registration. But this patch just unconditionally
requires connect:search_domains, even when no list could be
obtained, based on the following observations:
1. Administrators are unlikely to grant domain:getattr for one
or all domains while still denying connect:search_domains - a
user that is able to manage domains will want to be able to
manage them efficiently, but efficient management includes being
able to list the domains they can access. The idea of denying
connect:search_domains while still granting access to individual
domains is therefore not adding any real security, but just
serves as a layer of obscurity to annoy the end user.
2. In the current implementation, domain events are filtered
on the client; the server has no idea if a domain filter was
requested, and must therefore assume that all domain event
requests are global. Even if we fix the RPC protocol to
allow for server-side filtering for newer client/server combos,
making the connect:serach_domains ACL check conditional on
whether the domain argument was NULL won't benefit older clients.
Therefore, we choose to document that connect:search_domains
is a pre-requisite to any domain event management.
Network events need the same treatment, with the obvious
change of using connect:search_networks and network:getattr.
* src/access/viraccessperm.h
(VIR_ACCESS_PERM_CONNECT_SEARCH_DOMAINS)
(VIR_ACCESS_PERM_CONNECT_SEARCH_NETWORKS): Document additional
effect of the permission.
* src/conf/domain_event.h (virDomainEventStateRegister)
(virDomainEventStateRegisterID): Add new parameter.
* src/conf/network_event.h (virNetworkEventStateRegisterID):
Likewise.
* src/conf/object_event_private.h (virObjectEventStateRegisterID):
Likewise.
* src/conf/object_event.c (_virObjectEventCallback): Track a filter.
(virObjectEventDispatchMatchCallback): Use filter.
(virObjectEventCallbackListAddID): Register filter.
* src/conf/domain_event.c (virDomainEventFilter): New function.
(virDomainEventStateRegister, virDomainEventStateRegisterID):
Adjust callers.
* src/conf/network_event.c (virNetworkEventFilter): New function.
(virNetworkEventStateRegisterID): Adjust caller.
* src/remote/remote_protocol.x
(REMOTE_PROC_CONNECT_DOMAIN_EVENT_REGISTER)
(REMOTE_PROC_CONNECT_DOMAIN_EVENT_REGISTER_ANY)
(REMOTE_PROC_CONNECT_NETWORK_EVENT_REGISTER_ANY): Generate a
filter, and require connect:search_domains instead of weaker
connect:read.
* src/test/test_driver.c (testConnectDomainEventRegister)
(testConnectDomainEventRegisterAny)
(testConnectNetworkEventRegisterAny): Update callers.
* src/remote/remote_driver.c (remoteConnectDomainEventRegister)
(remoteConnectDomainEventRegisterAny): Likewise.
* src/xen/xen_driver.c (xenUnifiedConnectDomainEventRegister)
(xenUnifiedConnectDomainEventRegisterAny): Likewise.
* src/vbox/vbox_tmpl.c (vboxDomainGetXMLDesc): Likewise.
* src/libxl/libxl_driver.c (libxlConnectDomainEventRegister)
(libxlConnectDomainEventRegisterAny): Likewise.
* src/qemu/qemu_driver.c (qemuConnectDomainEventRegister)
(qemuConnectDomainEventRegisterAny): Likewise.
* src/uml/uml_driver.c (umlConnectDomainEventRegister)
(umlConnectDomainEventRegisterAny): Likewise.
* src/network/bridge_driver.c
(networkConnectNetworkEventRegisterAny): Likewise.
* src/lxc/lxc_driver.c (lxcConnectDomainEventRegister)
(lxcConnectDomainEventRegisterAny): Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-08 20:34:48 +00:00
|
|
|
virObjectEventCallbackFilter filter,
|
|
|
|
void *filter_opaque,
|
2013-12-11 14:10:34 +00:00
|
|
|
virClassPtr klass,
|
2013-11-26 14:10:15 +00:00
|
|
|
int eventID,
|
|
|
|
virConnectObjectEventGenericCallback callback,
|
|
|
|
void *opaque,
|
|
|
|
virFreeCallback freecb,
|
2014-01-06 12:32:55 +00:00
|
|
|
int *callbackID,
|
|
|
|
bool serverFilter)
|
2013-11-26 14:10:15 +00:00
|
|
|
{
|
|
|
|
virObjectEventCallbackPtr event;
|
2014-01-03 20:31:48 +00:00
|
|
|
int ret = -1;
|
2014-01-06 12:32:55 +00:00
|
|
|
int remoteID = -1;
|
2013-11-26 14:10:15 +00:00
|
|
|
|
event: filter global events by domain:getattr ACL [CVE-2014-0028]
Ever since ACL filtering was added in commit 7639736 (v1.1.1), a
user could still use event registration to obtain access to a
domain that they could not normally access via virDomainLookup*
or virConnectListAllDomains and friends. We already have the
framework in the RPC generator for creating the filter, and
previous cleanup patches got us to the point that we can now
wire the filter through the entire object event stack.
Furthermore, whether or not domain:getattr is honored, use of
global events is a form of obtaining a list of networks, which
is covered by connect:search_domains added in a93cd08 (v1.1.0).
Ideally, we'd have a way to enforce connect:search_domains when
doing global registrations while omitting that check on a
per-domain registration. But this patch just unconditionally
requires connect:search_domains, even when no list could be
obtained, based on the following observations:
1. Administrators are unlikely to grant domain:getattr for one
or all domains while still denying connect:search_domains - a
user that is able to manage domains will want to be able to
manage them efficiently, but efficient management includes being
able to list the domains they can access. The idea of denying
connect:search_domains while still granting access to individual
domains is therefore not adding any real security, but just
serves as a layer of obscurity to annoy the end user.
2. In the current implementation, domain events are filtered
on the client; the server has no idea if a domain filter was
requested, and must therefore assume that all domain event
requests are global. Even if we fix the RPC protocol to
allow for server-side filtering for newer client/server combos,
making the connect:serach_domains ACL check conditional on
whether the domain argument was NULL won't benefit older clients.
Therefore, we choose to document that connect:search_domains
is a pre-requisite to any domain event management.
Network events need the same treatment, with the obvious
change of using connect:search_networks and network:getattr.
* src/access/viraccessperm.h
(VIR_ACCESS_PERM_CONNECT_SEARCH_DOMAINS)
(VIR_ACCESS_PERM_CONNECT_SEARCH_NETWORKS): Document additional
effect of the permission.
* src/conf/domain_event.h (virDomainEventStateRegister)
(virDomainEventStateRegisterID): Add new parameter.
* src/conf/network_event.h (virNetworkEventStateRegisterID):
Likewise.
* src/conf/object_event_private.h (virObjectEventStateRegisterID):
Likewise.
* src/conf/object_event.c (_virObjectEventCallback): Track a filter.
(virObjectEventDispatchMatchCallback): Use filter.
(virObjectEventCallbackListAddID): Register filter.
* src/conf/domain_event.c (virDomainEventFilter): New function.
(virDomainEventStateRegister, virDomainEventStateRegisterID):
Adjust callers.
* src/conf/network_event.c (virNetworkEventFilter): New function.
(virNetworkEventStateRegisterID): Adjust caller.
* src/remote/remote_protocol.x
(REMOTE_PROC_CONNECT_DOMAIN_EVENT_REGISTER)
(REMOTE_PROC_CONNECT_DOMAIN_EVENT_REGISTER_ANY)
(REMOTE_PROC_CONNECT_NETWORK_EVENT_REGISTER_ANY): Generate a
filter, and require connect:search_domains instead of weaker
connect:read.
* src/test/test_driver.c (testConnectDomainEventRegister)
(testConnectDomainEventRegisterAny)
(testConnectNetworkEventRegisterAny): Update callers.
* src/remote/remote_driver.c (remoteConnectDomainEventRegister)
(remoteConnectDomainEventRegisterAny): Likewise.
* src/xen/xen_driver.c (xenUnifiedConnectDomainEventRegister)
(xenUnifiedConnectDomainEventRegisterAny): Likewise.
* src/vbox/vbox_tmpl.c (vboxDomainGetXMLDesc): Likewise.
* src/libxl/libxl_driver.c (libxlConnectDomainEventRegister)
(libxlConnectDomainEventRegisterAny): Likewise.
* src/qemu/qemu_driver.c (qemuConnectDomainEventRegister)
(qemuConnectDomainEventRegisterAny): Likewise.
* src/uml/uml_driver.c (umlConnectDomainEventRegister)
(umlConnectDomainEventRegisterAny): Likewise.
* src/network/bridge_driver.c
(networkConnectNetworkEventRegisterAny): Likewise.
* src/lxc/lxc_driver.c (lxcConnectDomainEventRegister)
(lxcConnectDomainEventRegisterAny): Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-08 20:34:48 +00:00
|
|
|
VIR_DEBUG("conn=%p cblist=%p uuid=%p filter=%p filter_opaque=%p "
|
2013-12-11 15:27:13 +00:00
|
|
|
"klass=%p eventID=%d callback=%p opaque=%p",
|
event: filter global events by domain:getattr ACL [CVE-2014-0028]
Ever since ACL filtering was added in commit 7639736 (v1.1.1), a
user could still use event registration to obtain access to a
domain that they could not normally access via virDomainLookup*
or virConnectListAllDomains and friends. We already have the
framework in the RPC generator for creating the filter, and
previous cleanup patches got us to the point that we can now
wire the filter through the entire object event stack.
Furthermore, whether or not domain:getattr is honored, use of
global events is a form of obtaining a list of networks, which
is covered by connect:search_domains added in a93cd08 (v1.1.0).
Ideally, we'd have a way to enforce connect:search_domains when
doing global registrations while omitting that check on a
per-domain registration. But this patch just unconditionally
requires connect:search_domains, even when no list could be
obtained, based on the following observations:
1. Administrators are unlikely to grant domain:getattr for one
or all domains while still denying connect:search_domains - a
user that is able to manage domains will want to be able to
manage them efficiently, but efficient management includes being
able to list the domains they can access. The idea of denying
connect:search_domains while still granting access to individual
domains is therefore not adding any real security, but just
serves as a layer of obscurity to annoy the end user.
2. In the current implementation, domain events are filtered
on the client; the server has no idea if a domain filter was
requested, and must therefore assume that all domain event
requests are global. Even if we fix the RPC protocol to
allow for server-side filtering for newer client/server combos,
making the connect:serach_domains ACL check conditional on
whether the domain argument was NULL won't benefit older clients.
Therefore, we choose to document that connect:search_domains
is a pre-requisite to any domain event management.
Network events need the same treatment, with the obvious
change of using connect:search_networks and network:getattr.
* src/access/viraccessperm.h
(VIR_ACCESS_PERM_CONNECT_SEARCH_DOMAINS)
(VIR_ACCESS_PERM_CONNECT_SEARCH_NETWORKS): Document additional
effect of the permission.
* src/conf/domain_event.h (virDomainEventStateRegister)
(virDomainEventStateRegisterID): Add new parameter.
* src/conf/network_event.h (virNetworkEventStateRegisterID):
Likewise.
* src/conf/object_event_private.h (virObjectEventStateRegisterID):
Likewise.
* src/conf/object_event.c (_virObjectEventCallback): Track a filter.
(virObjectEventDispatchMatchCallback): Use filter.
(virObjectEventCallbackListAddID): Register filter.
* src/conf/domain_event.c (virDomainEventFilter): New function.
(virDomainEventStateRegister, virDomainEventStateRegisterID):
Adjust callers.
* src/conf/network_event.c (virNetworkEventFilter): New function.
(virNetworkEventStateRegisterID): Adjust caller.
* src/remote/remote_protocol.x
(REMOTE_PROC_CONNECT_DOMAIN_EVENT_REGISTER)
(REMOTE_PROC_CONNECT_DOMAIN_EVENT_REGISTER_ANY)
(REMOTE_PROC_CONNECT_NETWORK_EVENT_REGISTER_ANY): Generate a
filter, and require connect:search_domains instead of weaker
connect:read.
* src/test/test_driver.c (testConnectDomainEventRegister)
(testConnectDomainEventRegisterAny)
(testConnectNetworkEventRegisterAny): Update callers.
* src/remote/remote_driver.c (remoteConnectDomainEventRegister)
(remoteConnectDomainEventRegisterAny): Likewise.
* src/xen/xen_driver.c (xenUnifiedConnectDomainEventRegister)
(xenUnifiedConnectDomainEventRegisterAny): Likewise.
* src/vbox/vbox_tmpl.c (vboxDomainGetXMLDesc): Likewise.
* src/libxl/libxl_driver.c (libxlConnectDomainEventRegister)
(libxlConnectDomainEventRegisterAny): Likewise.
* src/qemu/qemu_driver.c (qemuConnectDomainEventRegister)
(qemuConnectDomainEventRegisterAny): Likewise.
* src/uml/uml_driver.c (umlConnectDomainEventRegister)
(umlConnectDomainEventRegisterAny): Likewise.
* src/network/bridge_driver.c
(networkConnectNetworkEventRegisterAny): Likewise.
* src/lxc/lxc_driver.c (lxcConnectDomainEventRegister)
(lxcConnectDomainEventRegisterAny): Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-08 20:34:48 +00:00
|
|
|
conn, cbList, uuid, filter, filter_opaque,
|
|
|
|
klass, eventID, callback, opaque);
|
2013-12-11 15:27:13 +00:00
|
|
|
|
2013-11-26 14:10:15 +00:00
|
|
|
/* Check incoming */
|
|
|
|
if (!cbList) {
|
|
|
|
return -1;
|
|
|
|
}
|
|
|
|
|
|
|
|
/* check if we already have this callback on our list */
|
event: don't let old-style events clobber per-domain events
Right now, the older virConnectDomainEventRegister (takes a
function pointer, returns 0 on success) and the newer
virConnectDomainEventRegisterID (takes an eventID, returns a
callbackID) share the underlying implementation (the older
API ends up consuming a callbackID for eventID 0 under the
hood). We implemented that by a lot of copy and pasted
code between object_event.c and domain_event.c, according to
whether we are dealing with a function pointer or an eventID.
However, our copy and paste is not symmetric. Consider this
sequence:
id1 = virConnectDomainEventRegisterAny(conn, dom,
VIR_DOMAIN_EVENT_ID_LIFECYCLE,
VIR_DOMAIN_EVENT_CALLBACK(callback), NULL, NULL);
virConnectDomainEventRegister(conn, callback, NULL, NULL);
virConnectDomainEventDeregister(conn, callback);
virConnectDomainEventDeregsiterAny(conn, id1);
the first three calls would succeed, but the third call ended
up nuking the id1 callbackID (the per-domain new-style handler),
then the fourth call failed with an error about an unknown
callbackID, leaving us with the global handler (old-style) still
live and receiving events. It required another old-style
deregister to clean up the mess. Root cause was that
virDomainEventCallbackList{Remove,MarkDelete} were only
checking for function pointer match, rather than also checking
for whether the registration was global.
Rather than playing with the guts of object_event ourselves
in domain_event, it is nicer to add a mapping function for the
internal callback id, then share common code for event removal.
For now, the function-to-id mapping is used only internally;
I thought about whether a new public API to let a user learn
the callback would be useful, but decided exposing this to the
user is probably a disservice, since we already publicly
document that they should avoid the old style, and since this
patch already demonstrates that older libvirt versions have
weird behavior when mixing old and new styles.
And like all good bug fix patches, I enhanced the testsuite,
validating that the changes in tests/ expose the failure
without the rest of the patch.
* src/conf/object_event.c (virObjectEventCallbackLookup)
(virObjectEventStateCallbackID): New functions.
(virObjectEventCallbackLookup): Use helper function.
* src/conf/object_event_private.h (virObjectEventStateCallbackID):
Declare new function.
* src/conf/domain_event.c (virDomainEventStateRegister)
(virDomainEventStateDeregister): Let common code handle the
complexity.
(virDomainEventCallbackListRemove)
(virDomainEventCallbackListMarkDelete)
(virDomainEventCallbackListAdd): Drop unused functions.
* tests/objecteventtest.c (testDomainCreateXMLMixed): New test.
Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-03 23:50:14 +00:00
|
|
|
if (virObjectEventCallbackLookup(conn, cbList, uuid,
|
event: don't allow mix of old- and new-style registration
Consider these two calls, in either order:
id1 = virConnectDomainEventRegisterAny(conn, NULL,
VIR_DOMAIN_EVENT_ID_LIFECYCLE,
VIR_DOMAIN_EVENT_CALLBACK(callback), NULL, NULL);
virConnectDomainEventRegister(conn, callback, NULL, NULL);
Right now, the second call fails, because under the hood, the
old-style function registration is tightly coupled to the
new style lifecycle eventID, and the two calls both try
to register the same global eventID callback representation.
We've alreay documented that users should avoid old-style
registration and deregistration, so anyone heeding the advice
won't run into this situation. But it would be even nicer if
we pretend the two interfaces are completely separate, and
disallow any cross-linking. That is, a call to old-style
deregister should never remove a new-style callback even if it
is the same function pointer, and a call to new-style callback
using only callbackIDs obtained legitimately should never
remove an old-style callback (of course, since our callback
IDs are sequential, and there is still coupling under the
hood, you can easily guess the callbackID of an old style
registration and use new-style deregistration to nuke it - but
that starts to be blatantly bad coding on your part rather
than a surprising result on what looks like reasonable
stand-alone API).
With this patch, you can now register a global lifecycle event
handler twice, by using both old and new APIs; if such an event
occurs, your callback will be entered twice. But that is not a
problem in practice, since it is already possible to use the
new API to register both a global and per-domain event handler
using the same function, which will likewise fire your callback
twice for that domain. Duplicates are still prevented when
using the same API with same parameters twice (old-style twice,
new-style global twice, or new-style per-domain with same domain
twice), and things are still bounded (it is not possible to
register a single function pointer more than N+2 times per event
id, where N is the number of domains available on the connection).
Besides, it has always been possible to register as many
separate function pointers on the same event id as desired,
through either old or new style API, where the bound there is
the physical limitation of writing a program with enough
distinct function pointers.
Adding another event registration in the testsuite is sufficient
to cover this, where the test fails without the rest of the patch.
* src/conf/object_event.c (_virObjectEventCallback): Add field.
(virObjectEventCallbackLookup): Add argument.
(virObjectEventCallbackListAddID, virObjectEventStateCallbackID):
Adjust callers.
* tests/objecteventtest.c (testDomainCreateXMLMixed): Enhance test.
Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-04 12:55:55 +00:00
|
|
|
klass, eventID, callback,
|
2014-01-06 12:32:55 +00:00
|
|
|
!callbackID,
|
|
|
|
serverFilter ? &remoteID : NULL) != -1) {
|
2014-01-08 17:10:49 +00:00
|
|
|
virReportError(VIR_ERR_INVALID_ARG, "%s",
|
event: don't let old-style events clobber per-domain events
Right now, the older virConnectDomainEventRegister (takes a
function pointer, returns 0 on success) and the newer
virConnectDomainEventRegisterID (takes an eventID, returns a
callbackID) share the underlying implementation (the older
API ends up consuming a callbackID for eventID 0 under the
hood). We implemented that by a lot of copy and pasted
code between object_event.c and domain_event.c, according to
whether we are dealing with a function pointer or an eventID.
However, our copy and paste is not symmetric. Consider this
sequence:
id1 = virConnectDomainEventRegisterAny(conn, dom,
VIR_DOMAIN_EVENT_ID_LIFECYCLE,
VIR_DOMAIN_EVENT_CALLBACK(callback), NULL, NULL);
virConnectDomainEventRegister(conn, callback, NULL, NULL);
virConnectDomainEventDeregister(conn, callback);
virConnectDomainEventDeregsiterAny(conn, id1);
the first three calls would succeed, but the third call ended
up nuking the id1 callbackID (the per-domain new-style handler),
then the fourth call failed with an error about an unknown
callbackID, leaving us with the global handler (old-style) still
live and receiving events. It required another old-style
deregister to clean up the mess. Root cause was that
virDomainEventCallbackList{Remove,MarkDelete} were only
checking for function pointer match, rather than also checking
for whether the registration was global.
Rather than playing with the guts of object_event ourselves
in domain_event, it is nicer to add a mapping function for the
internal callback id, then share common code for event removal.
For now, the function-to-id mapping is used only internally;
I thought about whether a new public API to let a user learn
the callback would be useful, but decided exposing this to the
user is probably a disservice, since we already publicly
document that they should avoid the old style, and since this
patch already demonstrates that older libvirt versions have
weird behavior when mixing old and new styles.
And like all good bug fix patches, I enhanced the testsuite,
validating that the changes in tests/ expose the failure
without the rest of the patch.
* src/conf/object_event.c (virObjectEventCallbackLookup)
(virObjectEventStateCallbackID): New functions.
(virObjectEventCallbackLookup): Use helper function.
* src/conf/object_event_private.h (virObjectEventStateCallbackID):
Declare new function.
* src/conf/domain_event.c (virDomainEventStateRegister)
(virDomainEventStateDeregister): Let common code handle the
complexity.
(virDomainEventCallbackListRemove)
(virDomainEventCallbackListMarkDelete)
(virDomainEventCallbackListAdd): Drop unused functions.
* tests/objecteventtest.c (testDomainCreateXMLMixed): New test.
Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-03 23:50:14 +00:00
|
|
|
_("event callback already tracked"));
|
|
|
|
return -1;
|
2013-11-26 14:10:15 +00:00
|
|
|
}
|
|
|
|
/* Allocate new event */
|
|
|
|
if (VIR_ALLOC(event) < 0)
|
2014-01-03 20:31:48 +00:00
|
|
|
goto cleanup;
|
|
|
|
event->conn = virObjectRef(conn);
|
|
|
|
event->callbackID = cbList->nextID++;
|
2013-11-26 14:10:15 +00:00
|
|
|
event->cb = callback;
|
2013-12-11 14:10:34 +00:00
|
|
|
event->klass = klass;
|
2013-11-26 14:10:15 +00:00
|
|
|
event->eventID = eventID;
|
|
|
|
event->opaque = opaque;
|
|
|
|
event->freecb = freecb;
|
2014-01-06 12:32:55 +00:00
|
|
|
event->remoteID = remoteID;
|
2013-11-26 14:10:15 +00:00
|
|
|
|
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.
Note that virObjectEventPtr still needs all fields of meta,
because this is how we reconstruct a virDomainPtr inside the
dispatch handler before calling the end user's callback
pointer with the correct object, even though only the uuid
portion of meta is used in deciding whether a callback
matches the given event. So while uuid is optional for
callbacks, it is mandatory for events.
The change to testDomainCreateXMLMixed is merely on the setup
scenario (as you can't register for a domain unless it is either
running or persistent). I actually first wrote that test for
this patch, then rebased it to also cover a prior patch (commit
4221d64), but had to adjust it for that patch to use Create
instead of Define for setting up the domain long enough to
register the event in order to work around this bug. But while
the setup is changed, the main body of the test is still about
whether creation events fire as expected.
* 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_private.h (virObjectEventNew): Document
use of name and uuid in events.
* 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.
* tests/objecteventtest.c (testDomainCreateXMLMixed): Enhance test.
Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-04 00:02:17 +00:00
|
|
|
/* 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);
|
2013-11-26 14:10:15 +00:00
|
|
|
}
|
event: filter global events by domain:getattr ACL [CVE-2014-0028]
Ever since ACL filtering was added in commit 7639736 (v1.1.1), a
user could still use event registration to obtain access to a
domain that they could not normally access via virDomainLookup*
or virConnectListAllDomains and friends. We already have the
framework in the RPC generator for creating the filter, and
previous cleanup patches got us to the point that we can now
wire the filter through the entire object event stack.
Furthermore, whether or not domain:getattr is honored, use of
global events is a form of obtaining a list of networks, which
is covered by connect:search_domains added in a93cd08 (v1.1.0).
Ideally, we'd have a way to enforce connect:search_domains when
doing global registrations while omitting that check on a
per-domain registration. But this patch just unconditionally
requires connect:search_domains, even when no list could be
obtained, based on the following observations:
1. Administrators are unlikely to grant domain:getattr for one
or all domains while still denying connect:search_domains - a
user that is able to manage domains will want to be able to
manage them efficiently, but efficient management includes being
able to list the domains they can access. The idea of denying
connect:search_domains while still granting access to individual
domains is therefore not adding any real security, but just
serves as a layer of obscurity to annoy the end user.
2. In the current implementation, domain events are filtered
on the client; the server has no idea if a domain filter was
requested, and must therefore assume that all domain event
requests are global. Even if we fix the RPC protocol to
allow for server-side filtering for newer client/server combos,
making the connect:serach_domains ACL check conditional on
whether the domain argument was NULL won't benefit older clients.
Therefore, we choose to document that connect:search_domains
is a pre-requisite to any domain event management.
Network events need the same treatment, with the obvious
change of using connect:search_networks and network:getattr.
* src/access/viraccessperm.h
(VIR_ACCESS_PERM_CONNECT_SEARCH_DOMAINS)
(VIR_ACCESS_PERM_CONNECT_SEARCH_NETWORKS): Document additional
effect of the permission.
* src/conf/domain_event.h (virDomainEventStateRegister)
(virDomainEventStateRegisterID): Add new parameter.
* src/conf/network_event.h (virNetworkEventStateRegisterID):
Likewise.
* src/conf/object_event_private.h (virObjectEventStateRegisterID):
Likewise.
* src/conf/object_event.c (_virObjectEventCallback): Track a filter.
(virObjectEventDispatchMatchCallback): Use filter.
(virObjectEventCallbackListAddID): Register filter.
* src/conf/domain_event.c (virDomainEventFilter): New function.
(virDomainEventStateRegister, virDomainEventStateRegisterID):
Adjust callers.
* src/conf/network_event.c (virNetworkEventFilter): New function.
(virNetworkEventStateRegisterID): Adjust caller.
* src/remote/remote_protocol.x
(REMOTE_PROC_CONNECT_DOMAIN_EVENT_REGISTER)
(REMOTE_PROC_CONNECT_DOMAIN_EVENT_REGISTER_ANY)
(REMOTE_PROC_CONNECT_NETWORK_EVENT_REGISTER_ANY): Generate a
filter, and require connect:search_domains instead of weaker
connect:read.
* src/test/test_driver.c (testConnectDomainEventRegister)
(testConnectDomainEventRegisterAny)
(testConnectNetworkEventRegisterAny): Update callers.
* src/remote/remote_driver.c (remoteConnectDomainEventRegister)
(remoteConnectDomainEventRegisterAny): Likewise.
* src/xen/xen_driver.c (xenUnifiedConnectDomainEventRegister)
(xenUnifiedConnectDomainEventRegisterAny): Likewise.
* src/vbox/vbox_tmpl.c (vboxDomainGetXMLDesc): Likewise.
* src/libxl/libxl_driver.c (libxlConnectDomainEventRegister)
(libxlConnectDomainEventRegisterAny): Likewise.
* src/qemu/qemu_driver.c (qemuConnectDomainEventRegister)
(qemuConnectDomainEventRegisterAny): Likewise.
* src/uml/uml_driver.c (umlConnectDomainEventRegister)
(umlConnectDomainEventRegisterAny): Likewise.
* src/network/bridge_driver.c
(networkConnectNetworkEventRegisterAny): Likewise.
* src/lxc/lxc_driver.c (lxcConnectDomainEventRegister)
(lxcConnectDomainEventRegisterAny): Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-08 20:34:48 +00:00
|
|
|
event->filter = filter;
|
|
|
|
event->filter_opaque = filter_opaque;
|
2013-11-26 14:10:15 +00:00
|
|
|
|
2014-01-03 20:31:48 +00:00
|
|
|
if (callbackID)
|
|
|
|
*callbackID = event->callbackID;
|
event: don't allow mix of old- and new-style registration
Consider these two calls, in either order:
id1 = virConnectDomainEventRegisterAny(conn, NULL,
VIR_DOMAIN_EVENT_ID_LIFECYCLE,
VIR_DOMAIN_EVENT_CALLBACK(callback), NULL, NULL);
virConnectDomainEventRegister(conn, callback, NULL, NULL);
Right now, the second call fails, because under the hood, the
old-style function registration is tightly coupled to the
new style lifecycle eventID, and the two calls both try
to register the same global eventID callback representation.
We've alreay documented that users should avoid old-style
registration and deregistration, so anyone heeding the advice
won't run into this situation. But it would be even nicer if
we pretend the two interfaces are completely separate, and
disallow any cross-linking. That is, a call to old-style
deregister should never remove a new-style callback even if it
is the same function pointer, and a call to new-style callback
using only callbackIDs obtained legitimately should never
remove an old-style callback (of course, since our callback
IDs are sequential, and there is still coupling under the
hood, you can easily guess the callbackID of an old style
registration and use new-style deregistration to nuke it - but
that starts to be blatantly bad coding on your part rather
than a surprising result on what looks like reasonable
stand-alone API).
With this patch, you can now register a global lifecycle event
handler twice, by using both old and new APIs; if such an event
occurs, your callback will be entered twice. But that is not a
problem in practice, since it is already possible to use the
new API to register both a global and per-domain event handler
using the same function, which will likewise fire your callback
twice for that domain. Duplicates are still prevented when
using the same API with same parameters twice (old-style twice,
new-style global twice, or new-style per-domain with same domain
twice), and things are still bounded (it is not possible to
register a single function pointer more than N+2 times per event
id, where N is the number of domains available on the connection).
Besides, it has always been possible to register as many
separate function pointers on the same event id as desired,
through either old or new style API, where the bound there is
the physical limitation of writing a program with enough
distinct function pointers.
Adding another event registration in the testsuite is sufficient
to cover this, where the test fails without the rest of the patch.
* src/conf/object_event.c (_virObjectEventCallback): Add field.
(virObjectEventCallbackLookup): Add argument.
(virObjectEventCallbackListAddID, virObjectEventStateCallbackID):
Adjust callers.
* tests/objecteventtest.c (testDomainCreateXMLMixed): Enhance test.
Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-04 12:55:55 +00:00
|
|
|
else
|
|
|
|
event->legacy = true;
|
2013-11-26 14:10:15 +00:00
|
|
|
|
2014-01-03 20:31:48 +00:00
|
|
|
if (VIR_APPEND_ELEMENT(cbList->callbacks, cbList->count, event) < 0)
|
|
|
|
goto cleanup;
|
2013-11-26 14:10:15 +00:00
|
|
|
|
2014-01-06 12:32:55 +00:00
|
|
|
ret = virObjectEventCallbackListCount(conn, cbList, klass, eventID,
|
|
|
|
uuid, serverFilter);
|
|
|
|
if (serverFilter && remoteID < 0)
|
|
|
|
ret++;
|
2013-11-26 14:10:15 +00:00
|
|
|
|
2014-01-03 20:31:48 +00:00
|
|
|
cleanup:
|
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.
Note that virObjectEventPtr still needs all fields of meta,
because this is how we reconstruct a virDomainPtr inside the
dispatch handler before calling the end user's callback
pointer with the correct object, even though only the uuid
portion of meta is used in deciding whether a callback
matches the given event. So while uuid is optional for
callbacks, it is mandatory for events.
The change to testDomainCreateXMLMixed is merely on the setup
scenario (as you can't register for a domain unless it is either
running or persistent). I actually first wrote that test for
this patch, then rebased it to also cover a prior patch (commit
4221d64), but had to adjust it for that patch to use Create
instead of Define for setting up the domain long enough to
register the event in order to work around this bug. But while
the setup is changed, the main body of the test is still about
whether creation events fire as expected.
* 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_private.h (virObjectEventNew): Document
use of name and uuid in events.
* 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.
* tests/objecteventtest.c (testDomainCreateXMLMixed): Enhance test.
Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-04 00:02:17 +00:00
|
|
|
if (event)
|
2014-01-03 20:31:48 +00:00
|
|
|
virObjectUnref(event->conn);
|
2013-11-26 14:10:15 +00:00
|
|
|
VIR_FREE(event);
|
2014-01-03 20:31:48 +00:00
|
|
|
return ret;
|
2013-11-26 14:10:15 +00:00
|
|
|
}
|
|
|
|
|
|
|
|
|
|
|
|
/**
|
|
|
|
* virObjectEventQueueClear:
|
|
|
|
* @queue: pointer to the queue
|
|
|
|
*
|
|
|
|
* Removes all elements from the queue
|
|
|
|
*/
|
2014-01-03 21:44:29 +00:00
|
|
|
static void
|
2013-11-26 14:10:15 +00:00
|
|
|
virObjectEventQueueClear(virObjectEventQueuePtr queue)
|
|
|
|
{
|
|
|
|
size_t i;
|
|
|
|
if (!queue)
|
|
|
|
return;
|
|
|
|
|
|
|
|
for (i = 0; i < queue->count; i++) {
|
|
|
|
virObjectUnref(queue->events[i]);
|
|
|
|
}
|
|
|
|
VIR_FREE(queue->events);
|
|
|
|
queue->count = 0;
|
|
|
|
}
|
|
|
|
|
|
|
|
/**
|
|
|
|
* virObjectEventQueueFree:
|
|
|
|
* @queue: pointer to the queue
|
|
|
|
*
|
|
|
|
* Free the memory in the queue. We process this like a list here
|
|
|
|
*/
|
|
|
|
static void
|
|
|
|
virObjectEventQueueFree(virObjectEventQueuePtr queue)
|
|
|
|
{
|
|
|
|
if (!queue)
|
|
|
|
return;
|
|
|
|
|
|
|
|
virObjectEventQueueClear(queue);
|
|
|
|
VIR_FREE(queue);
|
|
|
|
}
|
|
|
|
|
|
|
|
static virObjectEventQueuePtr
|
|
|
|
virObjectEventQueueNew(void)
|
|
|
|
{
|
|
|
|
virObjectEventQueuePtr ret;
|
|
|
|
|
|
|
|
ignore_value(VIR_ALLOC(ret));
|
|
|
|
return ret;
|
|
|
|
}
|
|
|
|
|
2014-01-02 14:16:49 +00:00
|
|
|
|
|
|
|
/**
|
|
|
|
* virObjectEventStateLock:
|
|
|
|
* @state: the event state object
|
|
|
|
*
|
|
|
|
* Lock event state before calling functions from object_event_private.h.
|
|
|
|
*/
|
2014-01-03 21:44:29 +00:00
|
|
|
static void
|
2013-11-26 14:10:15 +00:00
|
|
|
virObjectEventStateLock(virObjectEventStatePtr state)
|
|
|
|
{
|
|
|
|
virMutexLock(&state->lock);
|
|
|
|
}
|
|
|
|
|
2014-01-02 14:16:49 +00:00
|
|
|
|
|
|
|
/**
|
|
|
|
* virObjectEventStateUnlock:
|
|
|
|
* @state: the event state object
|
|
|
|
*
|
|
|
|
* Unlock event state after calling functions from object_event_private.h.
|
|
|
|
*/
|
2014-01-03 21:44:29 +00:00
|
|
|
static void
|
2013-11-26 14:10:15 +00:00
|
|
|
virObjectEventStateUnlock(virObjectEventStatePtr state)
|
|
|
|
{
|
|
|
|
virMutexUnlock(&state->lock);
|
|
|
|
}
|
|
|
|
|
2014-01-02 14:16:49 +00:00
|
|
|
|
2013-11-26 14:10:15 +00:00
|
|
|
/**
|
|
|
|
* virObjectEventStateFree:
|
|
|
|
* @list: virObjectEventStatePtr to free
|
|
|
|
*
|
|
|
|
* Free a virObjectEventStatePtr and its members, and unregister the timer.
|
|
|
|
*/
|
|
|
|
void
|
|
|
|
virObjectEventStateFree(virObjectEventStatePtr state)
|
|
|
|
{
|
|
|
|
if (!state)
|
|
|
|
return;
|
|
|
|
|
|
|
|
virObjectEventCallbackListFree(state->callbacks);
|
|
|
|
virObjectEventQueueFree(state->queue);
|
|
|
|
|
|
|
|
if (state->timer != -1)
|
|
|
|
virEventRemoveTimeout(state->timer);
|
|
|
|
|
|
|
|
virMutexDestroy(&state->lock);
|
|
|
|
VIR_FREE(state);
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
|
|
static void virObjectEventStateFlush(virObjectEventStatePtr state);
|
|
|
|
|
2014-01-02 14:16:49 +00:00
|
|
|
|
|
|
|
/**
|
|
|
|
* virObjectEventTimer:
|
|
|
|
* @timer: id of the event loop timer
|
|
|
|
* @opaque: the event state object
|
|
|
|
*
|
|
|
|
* Register this function with the event state as its opaque data as
|
|
|
|
* the callback of a periodic timer on the event loop, in order to
|
|
|
|
* flush the callback queue.
|
|
|
|
*/
|
2014-01-03 21:44:29 +00:00
|
|
|
static void
|
2013-11-26 14:10:15 +00:00
|
|
|
virObjectEventTimer(int timer ATTRIBUTE_UNUSED, void *opaque)
|
|
|
|
{
|
|
|
|
virObjectEventStatePtr state = opaque;
|
|
|
|
|
|
|
|
virObjectEventStateFlush(state);
|
|
|
|
}
|
|
|
|
|
2014-01-02 14:16:49 +00:00
|
|
|
|
2013-11-26 14:10:15 +00:00
|
|
|
/**
|
|
|
|
* virObjectEventStateNew:
|
2014-01-02 14:16:49 +00:00
|
|
|
*
|
|
|
|
* Allocate a new event state object.
|
2013-11-26 14:10:15 +00:00
|
|
|
*/
|
|
|
|
virObjectEventStatePtr
|
|
|
|
virObjectEventStateNew(void)
|
|
|
|
{
|
|
|
|
virObjectEventStatePtr state = NULL;
|
|
|
|
|
|
|
|
if (VIR_ALLOC(state) < 0)
|
|
|
|
goto error;
|
|
|
|
|
|
|
|
if (virMutexInit(&state->lock) < 0) {
|
|
|
|
virReportSystemError(errno, "%s",
|
|
|
|
_("unable to initialize state mutex"));
|
|
|
|
VIR_FREE(state);
|
|
|
|
goto error;
|
|
|
|
}
|
|
|
|
|
|
|
|
if (VIR_ALLOC(state->callbacks) < 0)
|
|
|
|
goto error;
|
|
|
|
|
|
|
|
if (!(state->queue = virObjectEventQueueNew()))
|
|
|
|
goto error;
|
|
|
|
|
|
|
|
state->timer = -1;
|
|
|
|
|
|
|
|
return state;
|
|
|
|
|
|
|
|
error:
|
|
|
|
virObjectEventStateFree(state);
|
|
|
|
return NULL;
|
|
|
|
}
|
|
|
|
|
2014-01-02 14:16:49 +00:00
|
|
|
|
|
|
|
/**
|
|
|
|
* virObjectEventNew:
|
|
|
|
* @klass: subclass of event to be created
|
|
|
|
* @dispatcher: callback for dispatching the particular subclass of event
|
|
|
|
* @eventID: id of the event
|
|
|
|
* @id: id of the object the event describes, or 0
|
|
|
|
* @name: name of the object the event describes
|
|
|
|
* @uuid: uuid of the object the event describes
|
|
|
|
*
|
|
|
|
* Create a new event, with the information common to all events.
|
|
|
|
*/
|
2013-12-11 10:38:03 +00:00
|
|
|
void *
|
|
|
|
virObjectEventNew(virClassPtr klass,
|
2013-12-12 17:47:42 +00:00
|
|
|
virObjectEventDispatchFunc dispatcher,
|
2013-12-11 10:38:03 +00:00
|
|
|
int eventID,
|
|
|
|
int id,
|
|
|
|
const char *name,
|
|
|
|
const unsigned char *uuid)
|
2013-11-26 14:10:15 +00:00
|
|
|
{
|
|
|
|
virObjectEventPtr event;
|
|
|
|
|
|
|
|
if (virObjectEventInitialize() < 0)
|
|
|
|
return NULL;
|
|
|
|
|
|
|
|
if (!virClassIsDerivedFrom(klass, virObjectEventClass)) {
|
|
|
|
virReportInvalidArg(klass,
|
|
|
|
_("Class %s must derive from virObjectEvent"),
|
|
|
|
virClassName(klass));
|
|
|
|
return NULL;
|
|
|
|
}
|
|
|
|
|
|
|
|
if (!(event = virObjectNew(klass)))
|
|
|
|
return NULL;
|
|
|
|
|
2013-12-12 17:47:42 +00:00
|
|
|
event->dispatch = dispatcher;
|
2013-11-26 14:10:15 +00:00
|
|
|
event->eventID = eventID;
|
2014-01-06 12:32:55 +00:00
|
|
|
event->remoteID = -1;
|
2013-11-26 14:10:15 +00:00
|
|
|
|
|
|
|
if (VIR_STRDUP(event->meta.name, name) < 0) {
|
|
|
|
VIR_FREE(event);
|
|
|
|
return NULL;
|
|
|
|
}
|
|
|
|
event->meta.id = id;
|
|
|
|
memcpy(event->meta.uuid, uuid, VIR_UUID_BUFLEN);
|
|
|
|
|
|
|
|
VIR_DEBUG("obj=%p", event);
|
|
|
|
return event;
|
|
|
|
}
|
|
|
|
|
2014-01-02 14:16:49 +00:00
|
|
|
|
2013-11-26 14:10:15 +00:00
|
|
|
/**
|
|
|
|
* virObjectEventQueuePush:
|
|
|
|
* @evtQueue: the object event queue
|
|
|
|
* @event: the event to add
|
|
|
|
*
|
|
|
|
* Internal function to push to the back of a virObjectEventQueue
|
|
|
|
*
|
|
|
|
* Returns: 0 on success, -1 on failure
|
|
|
|
*/
|
|
|
|
static int
|
|
|
|
virObjectEventQueuePush(virObjectEventQueuePtr evtQueue,
|
|
|
|
virObjectEventPtr event)
|
|
|
|
{
|
2014-01-03 20:31:48 +00:00
|
|
|
if (!evtQueue)
|
2013-11-26 14:10:15 +00:00
|
|
|
return -1;
|
|
|
|
|
2014-01-03 20:31:48 +00:00
|
|
|
if (VIR_APPEND_ELEMENT(evtQueue->events, evtQueue->count, event) < 0)
|
2013-11-26 14:10:15 +00:00
|
|
|
return -1;
|
|
|
|
return 0;
|
|
|
|
}
|
|
|
|
|
|
|
|
|
2014-01-03 17:34:39 +00:00
|
|
|
static bool
|
2013-12-11 10:38:03 +00:00
|
|
|
virObjectEventDispatchMatchCallback(virObjectEventPtr event,
|
|
|
|
virObjectEventCallbackPtr cb)
|
2013-11-26 14:10:15 +00:00
|
|
|
{
|
|
|
|
if (!cb)
|
2014-01-03 17:34:39 +00:00
|
|
|
return false;
|
2013-11-26 14:10:15 +00:00
|
|
|
if (cb->deleted)
|
2014-01-03 17:34:39 +00:00
|
|
|
return false;
|
2013-12-11 14:10:34 +00:00
|
|
|
if (!virObjectIsClass(event, cb->klass))
|
2014-01-03 17:34:39 +00:00
|
|
|
return false;
|
2014-01-02 14:40:50 +00:00
|
|
|
if (cb->eventID != event->eventID)
|
2014-01-03 17:34:39 +00:00
|
|
|
return false;
|
2014-01-06 12:32:55 +00:00
|
|
|
if (cb->remoteID != event->remoteID)
|
|
|
|
return false;
|
2013-11-26 14:10:15 +00:00
|
|
|
|
event: filter global events by domain:getattr ACL [CVE-2014-0028]
Ever since ACL filtering was added in commit 7639736 (v1.1.1), a
user could still use event registration to obtain access to a
domain that they could not normally access via virDomainLookup*
or virConnectListAllDomains and friends. We already have the
framework in the RPC generator for creating the filter, and
previous cleanup patches got us to the point that we can now
wire the filter through the entire object event stack.
Furthermore, whether or not domain:getattr is honored, use of
global events is a form of obtaining a list of networks, which
is covered by connect:search_domains added in a93cd08 (v1.1.0).
Ideally, we'd have a way to enforce connect:search_domains when
doing global registrations while omitting that check on a
per-domain registration. But this patch just unconditionally
requires connect:search_domains, even when no list could be
obtained, based on the following observations:
1. Administrators are unlikely to grant domain:getattr for one
or all domains while still denying connect:search_domains - a
user that is able to manage domains will want to be able to
manage them efficiently, but efficient management includes being
able to list the domains they can access. The idea of denying
connect:search_domains while still granting access to individual
domains is therefore not adding any real security, but just
serves as a layer of obscurity to annoy the end user.
2. In the current implementation, domain events are filtered
on the client; the server has no idea if a domain filter was
requested, and must therefore assume that all domain event
requests are global. Even if we fix the RPC protocol to
allow for server-side filtering for newer client/server combos,
making the connect:serach_domains ACL check conditional on
whether the domain argument was NULL won't benefit older clients.
Therefore, we choose to document that connect:search_domains
is a pre-requisite to any domain event management.
Network events need the same treatment, with the obvious
change of using connect:search_networks and network:getattr.
* src/access/viraccessperm.h
(VIR_ACCESS_PERM_CONNECT_SEARCH_DOMAINS)
(VIR_ACCESS_PERM_CONNECT_SEARCH_NETWORKS): Document additional
effect of the permission.
* src/conf/domain_event.h (virDomainEventStateRegister)
(virDomainEventStateRegisterID): Add new parameter.
* src/conf/network_event.h (virNetworkEventStateRegisterID):
Likewise.
* src/conf/object_event_private.h (virObjectEventStateRegisterID):
Likewise.
* src/conf/object_event.c (_virObjectEventCallback): Track a filter.
(virObjectEventDispatchMatchCallback): Use filter.
(virObjectEventCallbackListAddID): Register filter.
* src/conf/domain_event.c (virDomainEventFilter): New function.
(virDomainEventStateRegister, virDomainEventStateRegisterID):
Adjust callers.
* src/conf/network_event.c (virNetworkEventFilter): New function.
(virNetworkEventStateRegisterID): Adjust caller.
* src/remote/remote_protocol.x
(REMOTE_PROC_CONNECT_DOMAIN_EVENT_REGISTER)
(REMOTE_PROC_CONNECT_DOMAIN_EVENT_REGISTER_ANY)
(REMOTE_PROC_CONNECT_NETWORK_EVENT_REGISTER_ANY): Generate a
filter, and require connect:search_domains instead of weaker
connect:read.
* src/test/test_driver.c (testConnectDomainEventRegister)
(testConnectDomainEventRegisterAny)
(testConnectNetworkEventRegisterAny): Update callers.
* src/remote/remote_driver.c (remoteConnectDomainEventRegister)
(remoteConnectDomainEventRegisterAny): Likewise.
* src/xen/xen_driver.c (xenUnifiedConnectDomainEventRegister)
(xenUnifiedConnectDomainEventRegisterAny): Likewise.
* src/vbox/vbox_tmpl.c (vboxDomainGetXMLDesc): Likewise.
* src/libxl/libxl_driver.c (libxlConnectDomainEventRegister)
(libxlConnectDomainEventRegisterAny): Likewise.
* src/qemu/qemu_driver.c (qemuConnectDomainEventRegister)
(qemuConnectDomainEventRegisterAny): Likewise.
* src/uml/uml_driver.c (umlConnectDomainEventRegister)
(umlConnectDomainEventRegisterAny): Likewise.
* src/network/bridge_driver.c
(networkConnectNetworkEventRegisterAny): Likewise.
* src/lxc/lxc_driver.c (lxcConnectDomainEventRegister)
(lxcConnectDomainEventRegisterAny): Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-08 20:34:48 +00:00
|
|
|
if (cb->filter && !(cb->filter)(cb->conn, event, cb->filter_opaque))
|
|
|
|
return false;
|
|
|
|
|
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.
Note that virObjectEventPtr still needs all fields of meta,
because this is how we reconstruct a virDomainPtr inside the
dispatch handler before calling the end user's callback
pointer with the correct object, even though only the uuid
portion of meta is used in deciding whether a callback
matches the given event. So while uuid is optional for
callbacks, it is mandatory for events.
The change to testDomainCreateXMLMixed is merely on the setup
scenario (as you can't register for a domain unless it is either
running or persistent). I actually first wrote that test for
this patch, then rebased it to also cover a prior patch (commit
4221d64), but had to adjust it for that patch to use Create
instead of Define for setting up the domain long enough to
register the event in order to work around this bug. But while
the setup is changed, the main body of the test is still about
whether creation events fire as expected.
* 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_private.h (virObjectEventNew): Document
use of name and uuid in events.
* 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.
* tests/objecteventtest.c (testDomainCreateXMLMixed): Enhance test.
Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-04 00:02:17 +00:00
|
|
|
if (cb->uuid_filter) {
|
2013-11-26 14:10:15 +00:00
|
|
|
/* 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
|
2014-01-02 14:16:49 +00:00
|
|
|
* leaving 'uuid' as the only truly reliable ID we can use. */
|
2013-11-26 14:10:15 +00:00
|
|
|
|
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.
Note that virObjectEventPtr still needs all fields of meta,
because this is how we reconstruct a virDomainPtr inside the
dispatch handler before calling the end user's callback
pointer with the correct object, even though only the uuid
portion of meta is used in deciding whether a callback
matches the given event. So while uuid is optional for
callbacks, it is mandatory for events.
The change to testDomainCreateXMLMixed is merely on the setup
scenario (as you can't register for a domain unless it is either
running or persistent). I actually first wrote that test for
this patch, then rebased it to also cover a prior patch (commit
4221d64), but had to adjust it for that patch to use Create
instead of Define for setting up the domain long enough to
register the event in order to work around this bug. But while
the setup is changed, the main body of the test is still about
whether creation events fire as expected.
* 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_private.h (virObjectEventNew): Document
use of name and uuid in events.
* 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.
* tests/objecteventtest.c (testDomainCreateXMLMixed): Enhance test.
Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-04 00:02:17 +00:00
|
|
|
return memcmp(event->meta.uuid, cb->uuid, VIR_UUID_BUFLEN) == 0;
|
2013-11-26 14:10:15 +00:00
|
|
|
}
|
2014-01-03 17:34:39 +00:00
|
|
|
return true;
|
2013-11-26 14:10:15 +00:00
|
|
|
}
|
|
|
|
|
|
|
|
|
|
|
|
static void
|
2013-12-12 17:47:42 +00:00
|
|
|
virObjectEventStateDispatchCallbacks(virObjectEventStatePtr state,
|
|
|
|
virObjectEventPtr event,
|
|
|
|
virObjectEventCallbackListPtr callbacks)
|
2013-11-26 14:10:15 +00:00
|
|
|
{
|
|
|
|
size_t i;
|
|
|
|
/* Cache this now, since we may be dropping the lock,
|
|
|
|
and have more callbacks added. We're guaranteed not
|
|
|
|
to have any removed */
|
2014-01-03 20:31:48 +00:00
|
|
|
size_t cbCount = callbacks->count;
|
2013-11-26 14:10:15 +00:00
|
|
|
|
|
|
|
for (i = 0; i < cbCount; i++) {
|
2014-01-03 20:31:48 +00:00
|
|
|
virObjectEventCallbackPtr cb = callbacks->callbacks[i];
|
|
|
|
|
|
|
|
if (!virObjectEventDispatchMatchCallback(event, cb))
|
2013-11-26 14:10:15 +00:00
|
|
|
continue;
|
|
|
|
|
2013-12-12 17:47:42 +00:00
|
|
|
/* Drop the lock whle dispatching, for sake of re-entrancy */
|
|
|
|
virObjectEventStateUnlock(state);
|
2014-01-03 20:31:48 +00:00
|
|
|
event->dispatch(cb->conn, event, cb->cb, cb->opaque);
|
2013-12-12 17:47:42 +00:00
|
|
|
virObjectEventStateLock(state);
|
2013-11-26 14:10:15 +00:00
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
|
|
static void
|
2013-12-12 17:47:42 +00:00
|
|
|
virObjectEventStateQueueDispatch(virObjectEventStatePtr state,
|
|
|
|
virObjectEventQueuePtr queue,
|
|
|
|
virObjectEventCallbackListPtr callbacks)
|
2013-11-26 14:10:15 +00:00
|
|
|
{
|
|
|
|
size_t i;
|
|
|
|
|
|
|
|
for (i = 0; i < queue->count; i++) {
|
2014-01-03 20:31:48 +00:00
|
|
|
virObjectEventStateDispatchCallbacks(state, queue->events[i],
|
|
|
|
callbacks);
|
2013-11-26 14:10:15 +00:00
|
|
|
virObjectUnref(queue->events[i]);
|
|
|
|
}
|
|
|
|
VIR_FREE(queue->events);
|
|
|
|
queue->count = 0;
|
|
|
|
}
|
|
|
|
|
2014-01-02 14:16:49 +00:00
|
|
|
|
|
|
|
/**
|
2014-01-06 12:32:55 +00:00
|
|
|
* virObjectEventStateQueueRemote:
|
2014-01-02 14:16:49 +00:00
|
|
|
* @state: the event state object
|
|
|
|
* @event: event to add to the queue
|
2014-01-06 12:32:55 +00:00
|
|
|
* @remoteID: limit dispatch to callbacks with the same remote id
|
2014-01-02 14:16:49 +00:00
|
|
|
*
|
|
|
|
* Adds @event to the queue of events to be dispatched at the next
|
|
|
|
* safe moment. The caller should no longer use @event after this
|
2014-01-06 12:32:55 +00:00
|
|
|
* call. If @remoteID is non-negative, the event will only be sent to
|
|
|
|
* callbacks where virObjectEventStateSetRemote() registered a remote
|
|
|
|
* id.
|
2014-01-02 14:16:49 +00:00
|
|
|
*/
|
2013-11-26 14:10:15 +00:00
|
|
|
void
|
2014-01-06 12:32:55 +00:00
|
|
|
virObjectEventStateQueueRemote(virObjectEventStatePtr state,
|
|
|
|
virObjectEventPtr event,
|
|
|
|
int remoteID)
|
2013-11-26 14:10:15 +00:00
|
|
|
{
|
|
|
|
if (state->timer < 0) {
|
|
|
|
virObjectUnref(event);
|
|
|
|
return;
|
|
|
|
}
|
|
|
|
|
|
|
|
virObjectEventStateLock(state);
|
|
|
|
|
2014-01-06 12:32:55 +00:00
|
|
|
event->remoteID = remoteID;
|
2013-11-26 14:10:15 +00:00
|
|
|
if (virObjectEventQueuePush(state->queue, event) < 0) {
|
|
|
|
VIR_DEBUG("Error adding event to queue");
|
|
|
|
virObjectUnref(event);
|
|
|
|
}
|
|
|
|
|
|
|
|
if (state->queue->count == 1)
|
|
|
|
virEventUpdateTimeout(state->timer, 0);
|
|
|
|
virObjectEventStateUnlock(state);
|
|
|
|
}
|
|
|
|
|
|
|
|
|
2014-01-06 12:32:55 +00:00
|
|
|
/**
|
|
|
|
* virObjectEventStateQueue:
|
|
|
|
* @state: the event state object
|
|
|
|
* @event: event to add to the queue
|
|
|
|
*
|
|
|
|
* Adds @event to the queue of events to be dispatched at the next
|
|
|
|
* safe moment. The caller should no longer use @event after this
|
|
|
|
* call.
|
|
|
|
*/
|
|
|
|
void
|
|
|
|
virObjectEventStateQueue(virObjectEventStatePtr state,
|
|
|
|
virObjectEventPtr event)
|
|
|
|
{
|
|
|
|
virObjectEventStateQueueRemote(state, event, -1);
|
|
|
|
}
|
|
|
|
|
|
|
|
|
2013-11-26 14:10:15 +00:00
|
|
|
static void
|
|
|
|
virObjectEventStateFlush(virObjectEventStatePtr state)
|
|
|
|
{
|
|
|
|
virObjectEventQueue tempQueue;
|
|
|
|
|
|
|
|
virObjectEventStateLock(state);
|
|
|
|
state->isDispatching = true;
|
|
|
|
|
|
|
|
/* Copy the queue, so we're reentrant safe when dispatchFunc drops the
|
|
|
|
* driver lock */
|
|
|
|
tempQueue.count = state->queue->count;
|
|
|
|
tempQueue.events = state->queue->events;
|
|
|
|
state->queue->count = 0;
|
|
|
|
state->queue->events = NULL;
|
|
|
|
virEventUpdateTimeout(state->timer, -1);
|
|
|
|
|
2013-12-12 17:47:42 +00:00
|
|
|
virObjectEventStateQueueDispatch(state,
|
|
|
|
&tempQueue,
|
|
|
|
state->callbacks);
|
2013-11-26 14:10:15 +00:00
|
|
|
|
|
|
|
/* Purge any deleted callbacks */
|
|
|
|
virObjectEventCallbackListPurgeMarked(state->callbacks);
|
|
|
|
|
|
|
|
state->isDispatching = false;
|
|
|
|
virObjectEventStateUnlock(state);
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
|
|
/**
|
|
|
|
* virObjectEventStateRegisterID:
|
|
|
|
* @conn: connection to associate with callback
|
|
|
|
* @state: domain event state
|
2014-01-02 14:16:49 +00:00
|
|
|
* @uuid: uuid of the object for event filtering
|
2013-12-11 14:10:34 +00:00
|
|
|
* @klass: the base event class
|
2013-11-26 14:10:15 +00:00
|
|
|
* @eventID: ID of the event type to register for
|
2014-01-02 14:16:49 +00:00
|
|
|
* @cb: function to invoke when event occurs
|
|
|
|
* @opaque: data blob to pass to @callback
|
2013-11-26 14:10:15 +00:00
|
|
|
* @freecb: callback to free @opaque
|
|
|
|
* @callbackID: filled with callback ID
|
2014-01-06 12:32:55 +00:00
|
|
|
* @serverFilter: true if server supports object filtering
|
2013-11-26 14:10:15 +00:00
|
|
|
*
|
2014-01-02 14:16:49 +00:00
|
|
|
* Register the function @cb with connection @conn, from @state, for
|
|
|
|
* events of type @eventID, and return the registration handle in
|
|
|
|
* @callbackID.
|
2013-11-26 14:10:15 +00:00
|
|
|
*
|
2014-01-06 12:32:55 +00:00
|
|
|
* The return value is only important when registering client-side
|
|
|
|
* mirroring of remote events (since the public API is documented to
|
|
|
|
* return the callbackID rather than a count). A return of 1 means
|
|
|
|
* that this is the first use of this type of event, so a remote event
|
|
|
|
* must be enabled; a return larger than 1 means that an existing
|
|
|
|
* remote event can already feed this callback. If @serverFilter is
|
|
|
|
* false, the return count assumes that a single global remote feeds
|
|
|
|
* both generic and per-object callbacks, and that the event queue
|
|
|
|
* will be fed with virObjectEventStateQueue(). If it is true, then
|
|
|
|
* the return count assumes that the remote side is capable of per-
|
|
|
|
* object filtering; the user must call virObjectEventStateSetRemote()
|
|
|
|
* to record the remote id, and queue events with
|
|
|
|
* virObjectEventStateQueueRemote().
|
|
|
|
*
|
|
|
|
* Returns: the number of callbacks now registered, or -1 on error.
|
2013-11-26 14:10:15 +00:00
|
|
|
*/
|
|
|
|
int
|
|
|
|
virObjectEventStateRegisterID(virConnectPtr conn,
|
|
|
|
virObjectEventStatePtr state,
|
|
|
|
unsigned char *uuid,
|
event: filter global events by domain:getattr ACL [CVE-2014-0028]
Ever since ACL filtering was added in commit 7639736 (v1.1.1), a
user could still use event registration to obtain access to a
domain that they could not normally access via virDomainLookup*
or virConnectListAllDomains and friends. We already have the
framework in the RPC generator for creating the filter, and
previous cleanup patches got us to the point that we can now
wire the filter through the entire object event stack.
Furthermore, whether or not domain:getattr is honored, use of
global events is a form of obtaining a list of networks, which
is covered by connect:search_domains added in a93cd08 (v1.1.0).
Ideally, we'd have a way to enforce connect:search_domains when
doing global registrations while omitting that check on a
per-domain registration. But this patch just unconditionally
requires connect:search_domains, even when no list could be
obtained, based on the following observations:
1. Administrators are unlikely to grant domain:getattr for one
or all domains while still denying connect:search_domains - a
user that is able to manage domains will want to be able to
manage them efficiently, but efficient management includes being
able to list the domains they can access. The idea of denying
connect:search_domains while still granting access to individual
domains is therefore not adding any real security, but just
serves as a layer of obscurity to annoy the end user.
2. In the current implementation, domain events are filtered
on the client; the server has no idea if a domain filter was
requested, and must therefore assume that all domain event
requests are global. Even if we fix the RPC protocol to
allow for server-side filtering for newer client/server combos,
making the connect:serach_domains ACL check conditional on
whether the domain argument was NULL won't benefit older clients.
Therefore, we choose to document that connect:search_domains
is a pre-requisite to any domain event management.
Network events need the same treatment, with the obvious
change of using connect:search_networks and network:getattr.
* src/access/viraccessperm.h
(VIR_ACCESS_PERM_CONNECT_SEARCH_DOMAINS)
(VIR_ACCESS_PERM_CONNECT_SEARCH_NETWORKS): Document additional
effect of the permission.
* src/conf/domain_event.h (virDomainEventStateRegister)
(virDomainEventStateRegisterID): Add new parameter.
* src/conf/network_event.h (virNetworkEventStateRegisterID):
Likewise.
* src/conf/object_event_private.h (virObjectEventStateRegisterID):
Likewise.
* src/conf/object_event.c (_virObjectEventCallback): Track a filter.
(virObjectEventDispatchMatchCallback): Use filter.
(virObjectEventCallbackListAddID): Register filter.
* src/conf/domain_event.c (virDomainEventFilter): New function.
(virDomainEventStateRegister, virDomainEventStateRegisterID):
Adjust callers.
* src/conf/network_event.c (virNetworkEventFilter): New function.
(virNetworkEventStateRegisterID): Adjust caller.
* src/remote/remote_protocol.x
(REMOTE_PROC_CONNECT_DOMAIN_EVENT_REGISTER)
(REMOTE_PROC_CONNECT_DOMAIN_EVENT_REGISTER_ANY)
(REMOTE_PROC_CONNECT_NETWORK_EVENT_REGISTER_ANY): Generate a
filter, and require connect:search_domains instead of weaker
connect:read.
* src/test/test_driver.c (testConnectDomainEventRegister)
(testConnectDomainEventRegisterAny)
(testConnectNetworkEventRegisterAny): Update callers.
* src/remote/remote_driver.c (remoteConnectDomainEventRegister)
(remoteConnectDomainEventRegisterAny): Likewise.
* src/xen/xen_driver.c (xenUnifiedConnectDomainEventRegister)
(xenUnifiedConnectDomainEventRegisterAny): Likewise.
* src/vbox/vbox_tmpl.c (vboxDomainGetXMLDesc): Likewise.
* src/libxl/libxl_driver.c (libxlConnectDomainEventRegister)
(libxlConnectDomainEventRegisterAny): Likewise.
* src/qemu/qemu_driver.c (qemuConnectDomainEventRegister)
(qemuConnectDomainEventRegisterAny): Likewise.
* src/uml/uml_driver.c (umlConnectDomainEventRegister)
(umlConnectDomainEventRegisterAny): Likewise.
* src/network/bridge_driver.c
(networkConnectNetworkEventRegisterAny): Likewise.
* src/lxc/lxc_driver.c (lxcConnectDomainEventRegister)
(lxcConnectDomainEventRegisterAny): Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-08 20:34:48 +00:00
|
|
|
virObjectEventCallbackFilter filter,
|
|
|
|
void *filter_opaque,
|
2013-12-11 14:10:34 +00:00
|
|
|
virClassPtr klass,
|
2013-11-26 14:10:15 +00:00
|
|
|
int eventID,
|
|
|
|
virConnectObjectEventGenericCallback cb,
|
|
|
|
void *opaque,
|
|
|
|
virFreeCallback freecb,
|
2014-01-06 12:32:55 +00:00
|
|
|
int *callbackID,
|
|
|
|
bool serverFilter)
|
2013-11-26 14:10:15 +00:00
|
|
|
{
|
|
|
|
int ret = -1;
|
|
|
|
|
|
|
|
virObjectEventStateLock(state);
|
|
|
|
|
|
|
|
if ((state->callbacks->count == 0) &&
|
|
|
|
(state->timer == -1) &&
|
|
|
|
(state->timer = virEventAddTimeout(-1,
|
|
|
|
virObjectEventTimer,
|
|
|
|
state,
|
|
|
|
NULL)) < 0) {
|
|
|
|
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
|
|
|
|
_("could not initialize domain event timer"));
|
|
|
|
goto cleanup;
|
|
|
|
}
|
|
|
|
|
|
|
|
ret = virObjectEventCallbackListAddID(conn, state->callbacks,
|
event: filter global events by domain:getattr ACL [CVE-2014-0028]
Ever since ACL filtering was added in commit 7639736 (v1.1.1), a
user could still use event registration to obtain access to a
domain that they could not normally access via virDomainLookup*
or virConnectListAllDomains and friends. We already have the
framework in the RPC generator for creating the filter, and
previous cleanup patches got us to the point that we can now
wire the filter through the entire object event stack.
Furthermore, whether or not domain:getattr is honored, use of
global events is a form of obtaining a list of networks, which
is covered by connect:search_domains added in a93cd08 (v1.1.0).
Ideally, we'd have a way to enforce connect:search_domains when
doing global registrations while omitting that check on a
per-domain registration. But this patch just unconditionally
requires connect:search_domains, even when no list could be
obtained, based on the following observations:
1. Administrators are unlikely to grant domain:getattr for one
or all domains while still denying connect:search_domains - a
user that is able to manage domains will want to be able to
manage them efficiently, but efficient management includes being
able to list the domains they can access. The idea of denying
connect:search_domains while still granting access to individual
domains is therefore not adding any real security, but just
serves as a layer of obscurity to annoy the end user.
2. In the current implementation, domain events are filtered
on the client; the server has no idea if a domain filter was
requested, and must therefore assume that all domain event
requests are global. Even if we fix the RPC protocol to
allow for server-side filtering for newer client/server combos,
making the connect:serach_domains ACL check conditional on
whether the domain argument was NULL won't benefit older clients.
Therefore, we choose to document that connect:search_domains
is a pre-requisite to any domain event management.
Network events need the same treatment, with the obvious
change of using connect:search_networks and network:getattr.
* src/access/viraccessperm.h
(VIR_ACCESS_PERM_CONNECT_SEARCH_DOMAINS)
(VIR_ACCESS_PERM_CONNECT_SEARCH_NETWORKS): Document additional
effect of the permission.
* src/conf/domain_event.h (virDomainEventStateRegister)
(virDomainEventStateRegisterID): Add new parameter.
* src/conf/network_event.h (virNetworkEventStateRegisterID):
Likewise.
* src/conf/object_event_private.h (virObjectEventStateRegisterID):
Likewise.
* src/conf/object_event.c (_virObjectEventCallback): Track a filter.
(virObjectEventDispatchMatchCallback): Use filter.
(virObjectEventCallbackListAddID): Register filter.
* src/conf/domain_event.c (virDomainEventFilter): New function.
(virDomainEventStateRegister, virDomainEventStateRegisterID):
Adjust callers.
* src/conf/network_event.c (virNetworkEventFilter): New function.
(virNetworkEventStateRegisterID): Adjust caller.
* src/remote/remote_protocol.x
(REMOTE_PROC_CONNECT_DOMAIN_EVENT_REGISTER)
(REMOTE_PROC_CONNECT_DOMAIN_EVENT_REGISTER_ANY)
(REMOTE_PROC_CONNECT_NETWORK_EVENT_REGISTER_ANY): Generate a
filter, and require connect:search_domains instead of weaker
connect:read.
* src/test/test_driver.c (testConnectDomainEventRegister)
(testConnectDomainEventRegisterAny)
(testConnectNetworkEventRegisterAny): Update callers.
* src/remote/remote_driver.c (remoteConnectDomainEventRegister)
(remoteConnectDomainEventRegisterAny): Likewise.
* src/xen/xen_driver.c (xenUnifiedConnectDomainEventRegister)
(xenUnifiedConnectDomainEventRegisterAny): Likewise.
* src/vbox/vbox_tmpl.c (vboxDomainGetXMLDesc): Likewise.
* src/libxl/libxl_driver.c (libxlConnectDomainEventRegister)
(libxlConnectDomainEventRegisterAny): Likewise.
* src/qemu/qemu_driver.c (qemuConnectDomainEventRegister)
(qemuConnectDomainEventRegisterAny): Likewise.
* src/uml/uml_driver.c (umlConnectDomainEventRegister)
(umlConnectDomainEventRegisterAny): Likewise.
* src/network/bridge_driver.c
(networkConnectNetworkEventRegisterAny): Likewise.
* src/lxc/lxc_driver.c (lxcConnectDomainEventRegister)
(lxcConnectDomainEventRegisterAny): Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-08 20:34:48 +00:00
|
|
|
uuid, filter, filter_opaque,
|
|
|
|
klass, eventID,
|
2013-12-11 14:10:34 +00:00
|
|
|
cb, opaque, freecb,
|
2014-01-06 12:32:55 +00:00
|
|
|
callbackID, serverFilter);
|
2013-11-26 14:10:15 +00:00
|
|
|
|
|
|
|
if (ret == -1 &&
|
|
|
|
state->callbacks->count == 0 &&
|
|
|
|
state->timer != -1) {
|
|
|
|
virEventRemoveTimeout(state->timer);
|
|
|
|
state->timer = -1;
|
|
|
|
}
|
|
|
|
|
|
|
|
cleanup:
|
|
|
|
virObjectEventStateUnlock(state);
|
|
|
|
return ret;
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
|
|
/**
|
|
|
|
* virObjectEventStateDeregisterID:
|
|
|
|
* @conn: connection to associate with callback
|
|
|
|
* @state: object event state
|
|
|
|
* @callbackID: ID of the function to remove from event
|
|
|
|
*
|
|
|
|
* Unregister the function @callbackID with connection @conn,
|
|
|
|
* from @state, for events.
|
|
|
|
*
|
|
|
|
* Returns: the number of callbacks still registered, or -1 on error
|
|
|
|
*/
|
|
|
|
int
|
|
|
|
virObjectEventStateDeregisterID(virConnectPtr conn,
|
|
|
|
virObjectEventStatePtr state,
|
|
|
|
int callbackID)
|
|
|
|
{
|
|
|
|
int ret;
|
|
|
|
|
|
|
|
virObjectEventStateLock(state);
|
|
|
|
if (state->isDispatching)
|
|
|
|
ret = virObjectEventCallbackListMarkDeleteID(conn,
|
2014-01-02 14:16:49 +00:00
|
|
|
state->callbacks,
|
|
|
|
callbackID);
|
2013-11-26 14:10:15 +00:00
|
|
|
else
|
|
|
|
ret = virObjectEventCallbackListRemoveID(conn,
|
|
|
|
state->callbacks, callbackID);
|
|
|
|
|
|
|
|
if (state->callbacks->count == 0 &&
|
|
|
|
state->timer != -1) {
|
|
|
|
virEventRemoveTimeout(state->timer);
|
|
|
|
state->timer = -1;
|
|
|
|
virObjectEventQueueClear(state->queue);
|
|
|
|
}
|
|
|
|
|
|
|
|
virObjectEventStateUnlock(state);
|
|
|
|
return ret;
|
|
|
|
}
|
|
|
|
|
event: don't let old-style events clobber per-domain events
Right now, the older virConnectDomainEventRegister (takes a
function pointer, returns 0 on success) and the newer
virConnectDomainEventRegisterID (takes an eventID, returns a
callbackID) share the underlying implementation (the older
API ends up consuming a callbackID for eventID 0 under the
hood). We implemented that by a lot of copy and pasted
code between object_event.c and domain_event.c, according to
whether we are dealing with a function pointer or an eventID.
However, our copy and paste is not symmetric. Consider this
sequence:
id1 = virConnectDomainEventRegisterAny(conn, dom,
VIR_DOMAIN_EVENT_ID_LIFECYCLE,
VIR_DOMAIN_EVENT_CALLBACK(callback), NULL, NULL);
virConnectDomainEventRegister(conn, callback, NULL, NULL);
virConnectDomainEventDeregister(conn, callback);
virConnectDomainEventDeregsiterAny(conn, id1);
the first three calls would succeed, but the third call ended
up nuking the id1 callbackID (the per-domain new-style handler),
then the fourth call failed with an error about an unknown
callbackID, leaving us with the global handler (old-style) still
live and receiving events. It required another old-style
deregister to clean up the mess. Root cause was that
virDomainEventCallbackList{Remove,MarkDelete} were only
checking for function pointer match, rather than also checking
for whether the registration was global.
Rather than playing with the guts of object_event ourselves
in domain_event, it is nicer to add a mapping function for the
internal callback id, then share common code for event removal.
For now, the function-to-id mapping is used only internally;
I thought about whether a new public API to let a user learn
the callback would be useful, but decided exposing this to the
user is probably a disservice, since we already publicly
document that they should avoid the old style, and since this
patch already demonstrates that older libvirt versions have
weird behavior when mixing old and new styles.
And like all good bug fix patches, I enhanced the testsuite,
validating that the changes in tests/ expose the failure
without the rest of the patch.
* src/conf/object_event.c (virObjectEventCallbackLookup)
(virObjectEventStateCallbackID): New functions.
(virObjectEventCallbackLookup): Use helper function.
* src/conf/object_event_private.h (virObjectEventStateCallbackID):
Declare new function.
* src/conf/domain_event.c (virDomainEventStateRegister)
(virDomainEventStateDeregister): Let common code handle the
complexity.
(virDomainEventCallbackListRemove)
(virDomainEventCallbackListMarkDelete)
(virDomainEventCallbackListAdd): Drop unused functions.
* tests/objecteventtest.c (testDomainCreateXMLMixed): New test.
Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-03 23:50:14 +00:00
|
|
|
/**
|
|
|
|
* virObjectEventStateCallbackID:
|
|
|
|
* @conn: connection associated with callback
|
|
|
|
* @state: object event state
|
|
|
|
* @klass: the base event class
|
|
|
|
* @eventID: the event ID
|
|
|
|
* @callback: function registered as a callback
|
|
|
|
*
|
|
|
|
* Returns the callbackID of @callback, or -1 with an error issued if the
|
event: don't allow mix of old- and new-style registration
Consider these two calls, in either order:
id1 = virConnectDomainEventRegisterAny(conn, NULL,
VIR_DOMAIN_EVENT_ID_LIFECYCLE,
VIR_DOMAIN_EVENT_CALLBACK(callback), NULL, NULL);
virConnectDomainEventRegister(conn, callback, NULL, NULL);
Right now, the second call fails, because under the hood, the
old-style function registration is tightly coupled to the
new style lifecycle eventID, and the two calls both try
to register the same global eventID callback representation.
We've alreay documented that users should avoid old-style
registration and deregistration, so anyone heeding the advice
won't run into this situation. But it would be even nicer if
we pretend the two interfaces are completely separate, and
disallow any cross-linking. That is, a call to old-style
deregister should never remove a new-style callback even if it
is the same function pointer, and a call to new-style callback
using only callbackIDs obtained legitimately should never
remove an old-style callback (of course, since our callback
IDs are sequential, and there is still coupling under the
hood, you can easily guess the callbackID of an old style
registration and use new-style deregistration to nuke it - but
that starts to be blatantly bad coding on your part rather
than a surprising result on what looks like reasonable
stand-alone API).
With this patch, you can now register a global lifecycle event
handler twice, by using both old and new APIs; if such an event
occurs, your callback will be entered twice. But that is not a
problem in practice, since it is already possible to use the
new API to register both a global and per-domain event handler
using the same function, which will likewise fire your callback
twice for that domain. Duplicates are still prevented when
using the same API with same parameters twice (old-style twice,
new-style global twice, or new-style per-domain with same domain
twice), and things are still bounded (it is not possible to
register a single function pointer more than N+2 times per event
id, where N is the number of domains available on the connection).
Besides, it has always been possible to register as many
separate function pointers on the same event id as desired,
through either old or new style API, where the bound there is
the physical limitation of writing a program with enough
distinct function pointers.
Adding another event registration in the testsuite is sufficient
to cover this, where the test fails without the rest of the patch.
* src/conf/object_event.c (_virObjectEventCallback): Add field.
(virObjectEventCallbackLookup): Add argument.
(virObjectEventCallbackListAddID, virObjectEventStateCallbackID):
Adjust callers.
* tests/objecteventtest.c (testDomainCreateXMLMixed): Enhance test.
Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-04 12:55:55 +00:00
|
|
|
* function is not currently registered. This only finds functions
|
|
|
|
* registered via virConnectDomainEventRegister, even if modern style
|
|
|
|
* virConnectDomainEventRegisterAny also registers the same callback.
|
event: don't let old-style events clobber per-domain events
Right now, the older virConnectDomainEventRegister (takes a
function pointer, returns 0 on success) and the newer
virConnectDomainEventRegisterID (takes an eventID, returns a
callbackID) share the underlying implementation (the older
API ends up consuming a callbackID for eventID 0 under the
hood). We implemented that by a lot of copy and pasted
code between object_event.c and domain_event.c, according to
whether we are dealing with a function pointer or an eventID.
However, our copy and paste is not symmetric. Consider this
sequence:
id1 = virConnectDomainEventRegisterAny(conn, dom,
VIR_DOMAIN_EVENT_ID_LIFECYCLE,
VIR_DOMAIN_EVENT_CALLBACK(callback), NULL, NULL);
virConnectDomainEventRegister(conn, callback, NULL, NULL);
virConnectDomainEventDeregister(conn, callback);
virConnectDomainEventDeregsiterAny(conn, id1);
the first three calls would succeed, but the third call ended
up nuking the id1 callbackID (the per-domain new-style handler),
then the fourth call failed with an error about an unknown
callbackID, leaving us with the global handler (old-style) still
live and receiving events. It required another old-style
deregister to clean up the mess. Root cause was that
virDomainEventCallbackList{Remove,MarkDelete} were only
checking for function pointer match, rather than also checking
for whether the registration was global.
Rather than playing with the guts of object_event ourselves
in domain_event, it is nicer to add a mapping function for the
internal callback id, then share common code for event removal.
For now, the function-to-id mapping is used only internally;
I thought about whether a new public API to let a user learn
the callback would be useful, but decided exposing this to the
user is probably a disservice, since we already publicly
document that they should avoid the old style, and since this
patch already demonstrates that older libvirt versions have
weird behavior when mixing old and new styles.
And like all good bug fix patches, I enhanced the testsuite,
validating that the changes in tests/ expose the failure
without the rest of the patch.
* src/conf/object_event.c (virObjectEventCallbackLookup)
(virObjectEventStateCallbackID): New functions.
(virObjectEventCallbackLookup): Use helper function.
* src/conf/object_event_private.h (virObjectEventStateCallbackID):
Declare new function.
* src/conf/domain_event.c (virDomainEventStateRegister)
(virDomainEventStateDeregister): Let common code handle the
complexity.
(virDomainEventCallbackListRemove)
(virDomainEventCallbackListMarkDelete)
(virDomainEventCallbackListAdd): Drop unused functions.
* tests/objecteventtest.c (testDomainCreateXMLMixed): New test.
Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-03 23:50:14 +00:00
|
|
|
*/
|
|
|
|
int
|
|
|
|
virObjectEventStateCallbackID(virConnectPtr conn,
|
|
|
|
virObjectEventStatePtr state,
|
|
|
|
virClassPtr klass,
|
|
|
|
int eventID,
|
|
|
|
virConnectObjectEventGenericCallback callback)
|
|
|
|
{
|
|
|
|
int ret = -1;
|
|
|
|
|
|
|
|
virObjectEventStateLock(state);
|
|
|
|
ret = virObjectEventCallbackLookup(conn, state->callbacks, NULL,
|
2014-01-06 12:32:55 +00:00
|
|
|
klass, eventID, callback, true, NULL);
|
event: don't let old-style events clobber per-domain events
Right now, the older virConnectDomainEventRegister (takes a
function pointer, returns 0 on success) and the newer
virConnectDomainEventRegisterID (takes an eventID, returns a
callbackID) share the underlying implementation (the older
API ends up consuming a callbackID for eventID 0 under the
hood). We implemented that by a lot of copy and pasted
code between object_event.c and domain_event.c, according to
whether we are dealing with a function pointer or an eventID.
However, our copy and paste is not symmetric. Consider this
sequence:
id1 = virConnectDomainEventRegisterAny(conn, dom,
VIR_DOMAIN_EVENT_ID_LIFECYCLE,
VIR_DOMAIN_EVENT_CALLBACK(callback), NULL, NULL);
virConnectDomainEventRegister(conn, callback, NULL, NULL);
virConnectDomainEventDeregister(conn, callback);
virConnectDomainEventDeregsiterAny(conn, id1);
the first three calls would succeed, but the third call ended
up nuking the id1 callbackID (the per-domain new-style handler),
then the fourth call failed with an error about an unknown
callbackID, leaving us with the global handler (old-style) still
live and receiving events. It required another old-style
deregister to clean up the mess. Root cause was that
virDomainEventCallbackList{Remove,MarkDelete} were only
checking for function pointer match, rather than also checking
for whether the registration was global.
Rather than playing with the guts of object_event ourselves
in domain_event, it is nicer to add a mapping function for the
internal callback id, then share common code for event removal.
For now, the function-to-id mapping is used only internally;
I thought about whether a new public API to let a user learn
the callback would be useful, but decided exposing this to the
user is probably a disservice, since we already publicly
document that they should avoid the old style, and since this
patch already demonstrates that older libvirt versions have
weird behavior when mixing old and new styles.
And like all good bug fix patches, I enhanced the testsuite,
validating that the changes in tests/ expose the failure
without the rest of the patch.
* src/conf/object_event.c (virObjectEventCallbackLookup)
(virObjectEventStateCallbackID): New functions.
(virObjectEventCallbackLookup): Use helper function.
* src/conf/object_event_private.h (virObjectEventStateCallbackID):
Declare new function.
* src/conf/domain_event.c (virDomainEventStateRegister)
(virDomainEventStateDeregister): Let common code handle the
complexity.
(virDomainEventCallbackListRemove)
(virDomainEventCallbackListMarkDelete)
(virDomainEventCallbackListAdd): Drop unused functions.
* tests/objecteventtest.c (testDomainCreateXMLMixed): New test.
Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-03 23:50:14 +00:00
|
|
|
virObjectEventStateUnlock(state);
|
|
|
|
|
|
|
|
if (ret < 0)
|
2014-01-08 17:10:49 +00:00
|
|
|
virReportError(VIR_ERR_INVALID_ARG,
|
event: don't let old-style events clobber per-domain events
Right now, the older virConnectDomainEventRegister (takes a
function pointer, returns 0 on success) and the newer
virConnectDomainEventRegisterID (takes an eventID, returns a
callbackID) share the underlying implementation (the older
API ends up consuming a callbackID for eventID 0 under the
hood). We implemented that by a lot of copy and pasted
code between object_event.c and domain_event.c, according to
whether we are dealing with a function pointer or an eventID.
However, our copy and paste is not symmetric. Consider this
sequence:
id1 = virConnectDomainEventRegisterAny(conn, dom,
VIR_DOMAIN_EVENT_ID_LIFECYCLE,
VIR_DOMAIN_EVENT_CALLBACK(callback), NULL, NULL);
virConnectDomainEventRegister(conn, callback, NULL, NULL);
virConnectDomainEventDeregister(conn, callback);
virConnectDomainEventDeregsiterAny(conn, id1);
the first three calls would succeed, but the third call ended
up nuking the id1 callbackID (the per-domain new-style handler),
then the fourth call failed with an error about an unknown
callbackID, leaving us with the global handler (old-style) still
live and receiving events. It required another old-style
deregister to clean up the mess. Root cause was that
virDomainEventCallbackList{Remove,MarkDelete} were only
checking for function pointer match, rather than also checking
for whether the registration was global.
Rather than playing with the guts of object_event ourselves
in domain_event, it is nicer to add a mapping function for the
internal callback id, then share common code for event removal.
For now, the function-to-id mapping is used only internally;
I thought about whether a new public API to let a user learn
the callback would be useful, but decided exposing this to the
user is probably a disservice, since we already publicly
document that they should avoid the old style, and since this
patch already demonstrates that older libvirt versions have
weird behavior when mixing old and new styles.
And like all good bug fix patches, I enhanced the testsuite,
validating that the changes in tests/ expose the failure
without the rest of the patch.
* src/conf/object_event.c (virObjectEventCallbackLookup)
(virObjectEventStateCallbackID): New functions.
(virObjectEventCallbackLookup): Use helper function.
* src/conf/object_event_private.h (virObjectEventStateCallbackID):
Declare new function.
* src/conf/domain_event.c (virDomainEventStateRegister)
(virDomainEventStateDeregister): Let common code handle the
complexity.
(virDomainEventCallbackListRemove)
(virDomainEventCallbackListMarkDelete)
(virDomainEventCallbackListAdd): Drop unused functions.
* tests/objecteventtest.c (testDomainCreateXMLMixed): New test.
Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-03 23:50:14 +00:00
|
|
|
_("event callback function %p not registered"),
|
|
|
|
callback);
|
|
|
|
return ret;
|
|
|
|
}
|
|
|
|
|
2013-11-26 14:10:15 +00:00
|
|
|
|
|
|
|
/**
|
|
|
|
* virObjectEventStateEventID:
|
|
|
|
* @conn: connection associated with the callback
|
|
|
|
* @state: object event state
|
|
|
|
* @callbackID: the callback to query
|
2014-01-06 12:32:55 +00:00
|
|
|
* @remoteID: optionally output remote ID of the callback
|
2013-11-26 14:10:15 +00:00
|
|
|
*
|
2014-01-06 12:32:55 +00:00
|
|
|
* Query what event ID type is associated with the callback
|
|
|
|
* @callbackID for connection @conn. If @remoteID is non-null, it
|
|
|
|
* will be set to the remote id previously registered with
|
|
|
|
* virObjectEventStateSetRemote().
|
2013-11-26 14:10:15 +00:00
|
|
|
*
|
|
|
|
* Returns 0 on success, -1 on error
|
|
|
|
*/
|
|
|
|
int
|
|
|
|
virObjectEventStateEventID(virConnectPtr conn,
|
|
|
|
virObjectEventStatePtr state,
|
2014-01-06 12:32:55 +00:00
|
|
|
int callbackID,
|
|
|
|
int *remoteID)
|
2013-11-26 14:10:15 +00:00
|
|
|
{
|
2014-01-08 17:10:49 +00:00
|
|
|
int ret = -1;
|
|
|
|
size_t i;
|
|
|
|
virObjectEventCallbackListPtr cbList = state->callbacks;
|
2013-11-26 14:10:15 +00:00
|
|
|
|
|
|
|
virObjectEventStateLock(state);
|
2014-01-08 17:10:49 +00:00
|
|
|
for (i = 0; i < cbList->count; i++) {
|
|
|
|
virObjectEventCallbackPtr cb = cbList->callbacks[i];
|
|
|
|
|
|
|
|
if (cb->deleted)
|
|
|
|
continue;
|
|
|
|
|
|
|
|
if (cb->callbackID == callbackID && cb->conn == conn) {
|
2014-01-06 12:32:55 +00:00
|
|
|
if (remoteID)
|
|
|
|
*remoteID = cb->remoteID;
|
2014-01-08 17:10:49 +00:00
|
|
|
ret = cb->eventID;
|
|
|
|
break;
|
|
|
|
}
|
|
|
|
}
|
2013-11-26 14:10:15 +00:00
|
|
|
virObjectEventStateUnlock(state);
|
2014-01-08 17:10:49 +00:00
|
|
|
|
|
|
|
if (ret < 0)
|
|
|
|
virReportError(VIR_ERR_INVALID_ARG,
|
|
|
|
_("event callback id %d not registered"),
|
|
|
|
callbackID);
|
2013-11-26 14:10:15 +00:00
|
|
|
return ret;
|
|
|
|
}
|
2014-01-06 12:32:55 +00:00
|
|
|
|
|
|
|
|
|
|
|
/**
|
|
|
|
* virObjectEventStateSetRemote:
|
|
|
|
* @conn: connection associated with the callback
|
|
|
|
* @state: object event state
|
|
|
|
* @callbackID: the callback to adjust
|
|
|
|
* @remoteID: the remote ID to associate with the callback
|
|
|
|
*
|
|
|
|
* Update @callbackID for connection @conn to record that it is now
|
|
|
|
* tied to @remoteID, and will therefore only match events that are
|
|
|
|
* sent with virObjectEventStateQueueRemote() with the same remote ID.
|
|
|
|
* Silently does nothing if @callbackID is invalid.
|
|
|
|
*/
|
|
|
|
void
|
|
|
|
virObjectEventStateSetRemote(virConnectPtr conn,
|
|
|
|
virObjectEventStatePtr state,
|
|
|
|
int callbackID,
|
|
|
|
int remoteID)
|
|
|
|
{
|
|
|
|
size_t i;
|
|
|
|
|
|
|
|
virObjectEventStateLock(state);
|
|
|
|
for (i = 0; i < state->callbacks->count; i++) {
|
|
|
|
virObjectEventCallbackPtr cb = state->callbacks->callbacks[i];
|
|
|
|
|
|
|
|
if (cb->deleted)
|
|
|
|
continue;
|
|
|
|
|
|
|
|
if (cb->callbackID == callbackID && cb->conn == conn) {
|
|
|
|
cb->remoteID = remoteID;
|
|
|
|
break;
|
|
|
|
}
|
|
|
|
}
|
|
|
|
virObjectEventStateUnlock(state);
|
|
|
|
}
|