[libvirt] [PATCH 0/7 v3] events: Add helpers for driver dispatching
I noticed that there is quite a bit of code duplication among the drivers that support domain events. This patch series is an attempt to consolidate the shared logic. The new virDomainEventState structure isn't opaque to the clients, though it could be. It would just require adding wrappers for a few more event functions, and possibly finding a way to integrate this cleanup with the xen and vbox event impls, which use less infrastructure than the converted drivers. v2: 2 patches were applied Addressed Eric's comments: NONNULL tagging Use bool for isDispatching Move libvirt_private.syms earlier Add NULL check in StateFree v3: Rebased to latest Convert libxl driver Addressed danpb's comment: virDomainEventStateNew now takes a requireTimer parameter Cole Robinson (7): domain_event: Add virDomainEventState structure domain_event: Add common domain event queue/flush helpers qemu: Use virDomainEventState helpers lxc: Use virDomainEventState helpers test: Use virDomainEventState helpers libxl: Convert to virDomainEventState remote: Use virDomainEventState helpers .gnulib | 2 +- cfg.mk | 1 + src/conf/domain_event.c | 183 +++++++++++++++++++++++++++++++++++++++----- src/conf/domain_event.h | 70 +++++++++++++---- src/libvirt_private.syms | 6 ++ src/libxl/libxl_conf.h | 6 +- src/libxl/libxl_driver.c | 80 ++++++-------------- src/lxc/lxc_conf.h | 6 +- src/lxc/lxc_driver.c | 74 +++++------------- src/qemu/qemu_conf.h | 6 +- src/qemu/qemu_domain.c | 29 +------ src/qemu/qemu_driver.c | 46 +++++------- src/remote/remote_driver.c | 163 +++++++++++++++------------------------ src/test/test_driver.c | 105 ++++++++------------------ 14 files changed, 391 insertions(+), 386 deletions(-) -- 1.7.4.4
This structure will be used to unify lots of duplicated event handling code across the state drivers. v2: Check for state == NULL in StateFree Add NONNULL tagging Use bool for isDispatching Signed-off-by: Cole Robinson <crobinso@redhat.com> --- cfg.mk | 1 + src/conf/domain_event.c | 94 ++++++++++++++++++++++++++++++++++++---------- src/conf/domain_event.h | 48 ++++++++++++++++------- src/libvirt_private.syms | 2 + 4 files changed, 111 insertions(+), 34 deletions(-) diff --git a/cfg.mk b/cfg.mk index cb059f5..06b638b 100644 --- a/cfg.mk +++ b/cfg.mk @@ -103,6 +103,7 @@ useless_free_options = \ --name=virDomainEventCallbackListFree \ --name=virDomainEventFree \ --name=virDomainEventQueueFree \ + --name=virDomainEventStateFree \ --name=virDomainFSDefFree \ --name=virDomainGraphicsDefFree \ --name=virDomainHostdevDefFree \ diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c index 688bf6c..2771887 100644 --- a/src/conf/domain_event.c +++ b/src/conf/domain_event.c @@ -24,6 +24,7 @@ #include <config.h> #include "domain_event.h" +#include "event.h" #include "logging.h" #include "datatypes.h" #include "memory.h" @@ -505,6 +506,25 @@ void virDomainEventFree(virDomainEventPtr event) VIR_FREE(event); } +/** + * virDomainEventQueueFree: + * @queue: pointer to the queue + * + * Free the memory in the queue. We process this like a list here + */ +void +virDomainEventQueueFree(virDomainEventQueuePtr queue) +{ + int i; + if (!queue) + return; + + for (i = 0; i < queue->count ; i++) { + virDomainEventFree(queue->events[i]); + } + VIR_FREE(queue->events); + VIR_FREE(queue); +} virDomainEventQueuePtr virDomainEventQueueNew(void) { @@ -518,6 +538,60 @@ virDomainEventQueuePtr virDomainEventQueueNew(void) return ret; } +/** + * virDomainEventStateFree: + * @list: virDomainEventStatePtr to free + * + * Free a virDomainEventStatePtr and its members, and unregister the timer. + */ +void +virDomainEventStateFree(virDomainEventStatePtr state) +{ + if (!state) + return; + + virDomainEventCallbackListFree(state->callbacks); + virDomainEventQueueFree(state->queue); + + if (state->timer != -1) + virEventRemoveTimeout(state->timer); +} + +virDomainEventStatePtr +virDomainEventStateNew(virEventTimeoutCallback timeout_cb, + void *timeout_opaque, + virFreeCallback timeout_free) +{ + virDomainEventStatePtr state = NULL; + + if (VIR_ALLOC(state) < 0) { + virReportOOMError(); + goto error; + } + + if (VIR_ALLOC(state->callbacks) < 0) { + virReportOOMError(); + goto error; + } + + if (!(state->queue = virDomainEventQueueNew())) { + goto error; + } + + if ((state->timer = virEventAddTimeout(-1, + timeout_cb, + timeout_opaque, + timeout_free)) < 0) { + goto error; + } + + return state; + +error: + virDomainEventStateFree(state); + return NULL; +} + static virDomainEventPtr virDomainEventNewInternal(int eventID, int id, const char *name, @@ -784,26 +858,6 @@ virDomainEventPtr virDomainEventGraphicsNewFromObj(virDomainObjPtr obj, /** - * virDomainEventQueueFree: - * @queue: pointer to the queue - * - * Free the memory in the queue. We process this like a list here - */ -void -virDomainEventQueueFree(virDomainEventQueuePtr queue) -{ - int i; - if (!queue) - return; - - for (i = 0; i < queue->count ; i++) { - virDomainEventFree(queue->events[i]); - } - VIR_FREE(queue->events); - VIR_FREE(queue); -} - -/** * virDomainEventQueuePop: * @evtQueue: the queue of events * diff --git a/src/conf/domain_event.h b/src/conf/domain_event.h index c03a159..2ac3ecc 100644 --- a/src/conf/domain_event.h +++ b/src/conf/domain_event.h @@ -25,6 +25,7 @@ #ifndef __DOMAIN_EVENT_H__ # define __DOMAIN_EVENT_H__ +# include "event.h" # include "domain_conf.h" typedef struct _virDomainEventCallback virDomainEventCallback; @@ -38,6 +39,33 @@ struct _virDomainEventCallbackList { typedef struct _virDomainEventCallbackList virDomainEventCallbackList; typedef virDomainEventCallbackList *virDomainEventCallbackListPtr; +/** + * Dispatching domain events that come in while + * in a call / response rpc + */ +typedef struct _virDomainEvent virDomainEvent; +typedef virDomainEvent *virDomainEventPtr; + +struct _virDomainEventQueue { + unsigned int count; + virDomainEventPtr *events; +}; +typedef struct _virDomainEventQueue virDomainEventQueue; +typedef virDomainEventQueue *virDomainEventQueuePtr; + +struct _virDomainEventState { + /* The list of domain event callbacks */ + virDomainEventCallbackListPtr callbacks; + /* The queue of domain events */ + virDomainEventQueuePtr queue; + /* Timer for flushing events queue */ + int timer; + /* Flag if we're in process of dispatching */ + bool isDispatching; +}; +typedef struct _virDomainEventState virDomainEventState; +typedef virDomainEventState *virDomainEventStatePtr; + void virDomainEventCallbackListFree(virDomainEventCallbackListPtr list); int virDomainEventCallbackListAdd(virConnectPtr conn, @@ -91,20 +119,6 @@ int virDomainEventCallbackListEventID(virConnectPtr conn, int callbackID) ATTRIBUTE_NONNULL(1); -/** - * Dispatching domain events that come in while - * in a call / response rpc - */ -typedef struct _virDomainEvent virDomainEvent; -typedef virDomainEvent *virDomainEventPtr; - -struct _virDomainEventQueue { - unsigned int count; - virDomainEventPtr *events; -}; -typedef struct _virDomainEventQueue virDomainEventQueue; -typedef virDomainEventQueue *virDomainEventQueuePtr; - virDomainEventQueuePtr virDomainEventQueueNew(void); virDomainEventPtr virDomainEventNew(int id, const char *name, const unsigned char *uuid, int type, int detail); @@ -164,6 +178,12 @@ virDomainEventQueuePop(virDomainEventQueuePtr evtQueue); void virDomainEventFree(virDomainEventPtr event); void virDomainEventQueueFree(virDomainEventQueuePtr queue); +void virDomainEventStateFree(virDomainEventStatePtr state); +virDomainEventStatePtr +virDomainEventStateNew(virEventTimeoutCallback timeout_cb, + void *timeout_opaque, + virFreeCallback timeout_free) + ATTRIBUTE_NONNULL(1); typedef void (*virDomainEventDispatchFunc)(virConnectPtr conn, virDomainEventPtr event, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 7e5b1d7..d4ad0c8 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -389,6 +389,8 @@ virDomainEventRTCChangeNewFromObj; virDomainEventRebootNew; virDomainEventRebootNewFromDom; virDomainEventRebootNewFromObj; +virDomainEventStateFree; +virDomainEventStateNew; virDomainEventWatchdogNewFromDom; virDomainEventWatchdogNewFromObj; -- 1.7.4.4
On Thu, May 12, 2011 at 01:14:25PM -0400, Cole Robinson wrote:
This structure will be used to unify lots of duplicated event handling code across the state drivers.
v2: Check for state == NULL in StateFree Add NONNULL tagging Use bool for isDispatching
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- cfg.mk | 1 + src/conf/domain_event.c | 94 ++++++++++++++++++++++++++++++++++++---------- src/conf/domain_event.h | 48 ++++++++++++++++------- src/libvirt_private.syms | 2 + 4 files changed, 111 insertions(+), 34 deletions(-)
diff --git a/cfg.mk b/cfg.mk index cb059f5..06b638b 100644 --- a/cfg.mk +++ b/cfg.mk @@ -103,6 +103,7 @@ useless_free_options = \ --name=virDomainEventCallbackListFree \ --name=virDomainEventFree \ --name=virDomainEventQueueFree \ + --name=virDomainEventStateFree \ --name=virDomainFSDefFree \ --name=virDomainGraphicsDefFree \ --name=virDomainHostdevDefFree \ diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c index 688bf6c..2771887 100644 --- a/src/conf/domain_event.c +++ b/src/conf/domain_event.c @@ -24,6 +24,7 @@ #include <config.h>
#include "domain_event.h" +#include "event.h" #include "logging.h" #include "datatypes.h" #include "memory.h" @@ -505,6 +506,25 @@ void virDomainEventFree(virDomainEventPtr event) VIR_FREE(event); }
+/** + * virDomainEventQueueFree: + * @queue: pointer to the queue + * + * Free the memory in the queue. We process this like a list here + */ +void +virDomainEventQueueFree(virDomainEventQueuePtr queue) +{ + int i; + if (!queue) + return; + + for (i = 0; i < queue->count ; i++) { + virDomainEventFree(queue->events[i]); + } + VIR_FREE(queue->events); + VIR_FREE(queue); +}
virDomainEventQueuePtr virDomainEventQueueNew(void) { @@ -518,6 +538,60 @@ virDomainEventQueuePtr virDomainEventQueueNew(void) return ret; }
+/** + * virDomainEventStateFree: + * @list: virDomainEventStatePtr to free + * + * Free a virDomainEventStatePtr and its members, and unregister the timer. + */ +void +virDomainEventStateFree(virDomainEventStatePtr state) +{ + if (!state) + return; + + virDomainEventCallbackListFree(state->callbacks); + virDomainEventQueueFree(state->queue); + + if (state->timer != -1) + virEventRemoveTimeout(state->timer); +} + +virDomainEventStatePtr +virDomainEventStateNew(virEventTimeoutCallback timeout_cb, + void *timeout_opaque, + virFreeCallback timeout_free) +{ + virDomainEventStatePtr state = NULL; + + if (VIR_ALLOC(state) < 0) { + virReportOOMError(); + goto error; + } + + if (VIR_ALLOC(state->callbacks) < 0) { + virReportOOMError(); + goto error; + } + + if (!(state->queue = virDomainEventQueueNew())) { + goto error; + } + + if ((state->timer = virEventAddTimeout(-1, + timeout_cb, + timeout_opaque, + timeout_free)) < 0) { + goto error; + } + + return state; + +error: + virDomainEventStateFree(state); + return NULL; +} + static virDomainEventPtr virDomainEventNewInternal(int eventID, int id, const char *name, @@ -784,26 +858,6 @@ virDomainEventPtr virDomainEventGraphicsNewFromObj(virDomainObjPtr obj,
/** - * virDomainEventQueueFree: - * @queue: pointer to the queue - * - * Free the memory in the queue. We process this like a list here - */ -void -virDomainEventQueueFree(virDomainEventQueuePtr queue) -{ - int i; - if (!queue) - return; - - for (i = 0; i < queue->count ; i++) { - virDomainEventFree(queue->events[i]); - } - VIR_FREE(queue->events); - VIR_FREE(queue); -} - -/** * virDomainEventQueuePop: * @evtQueue: the queue of events * diff --git a/src/conf/domain_event.h b/src/conf/domain_event.h index c03a159..2ac3ecc 100644 --- a/src/conf/domain_event.h +++ b/src/conf/domain_event.h @@ -25,6 +25,7 @@ #ifndef __DOMAIN_EVENT_H__ # define __DOMAIN_EVENT_H__
+# include "event.h" # include "domain_conf.h"
typedef struct _virDomainEventCallback virDomainEventCallback; @@ -38,6 +39,33 @@ struct _virDomainEventCallbackList { typedef struct _virDomainEventCallbackList virDomainEventCallbackList; typedef virDomainEventCallbackList *virDomainEventCallbackListPtr;
+/** + * Dispatching domain events that come in while + * in a call / response rpc + */ +typedef struct _virDomainEvent virDomainEvent; +typedef virDomainEvent *virDomainEventPtr; + +struct _virDomainEventQueue { + unsigned int count; + virDomainEventPtr *events; +}; +typedef struct _virDomainEventQueue virDomainEventQueue; +typedef virDomainEventQueue *virDomainEventQueuePtr; + +struct _virDomainEventState { + /* The list of domain event callbacks */ + virDomainEventCallbackListPtr callbacks; + /* The queue of domain events */ + virDomainEventQueuePtr queue; + /* Timer for flushing events queue */ + int timer; + /* Flag if we're in process of dispatching */ + bool isDispatching; +};
If possible, it would be desirable to make these two struct impls private in domain_event.c
+typedef struct _virDomainEventState virDomainEventState; +typedef virDomainEventState *virDomainEventStatePtr; + void virDomainEventCallbackListFree(virDomainEventCallbackListPtr list);
int virDomainEventCallbackListAdd(virConnectPtr conn, @@ -91,20 +119,6 @@ int virDomainEventCallbackListEventID(virConnectPtr conn, int callbackID) ATTRIBUTE_NONNULL(1);
-/** - * Dispatching domain events that come in while - * in a call / response rpc - */ -typedef struct _virDomainEvent virDomainEvent; -typedef virDomainEvent *virDomainEventPtr; - -struct _virDomainEventQueue { - unsigned int count; - virDomainEventPtr *events; -}; -typedef struct _virDomainEventQueue virDomainEventQueue; -typedef virDomainEventQueue *virDomainEventQueuePtr; - virDomainEventQueuePtr virDomainEventQueueNew(void);
virDomainEventPtr virDomainEventNew(int id, const char *name, const unsigned char *uuid, int type, int detail); @@ -164,6 +178,12 @@ virDomainEventQueuePop(virDomainEventQueuePtr evtQueue);
void virDomainEventFree(virDomainEventPtr event); void virDomainEventQueueFree(virDomainEventQueuePtr queue); +void virDomainEventStateFree(virDomainEventStatePtr state); +virDomainEventStatePtr +virDomainEventStateNew(virEventTimeoutCallback timeout_cb, + void *timeout_opaque, + virFreeCallback timeout_free) + ATTRIBUTE_NONNULL(1);
typedef void (*virDomainEventDispatchFunc)(virConnectPtr conn, virDomainEventPtr event, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 7e5b1d7..d4ad0c8 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -389,6 +389,8 @@ virDomainEventRTCChangeNewFromObj; virDomainEventRebootNew; virDomainEventRebootNewFromDom; virDomainEventRebootNewFromObj; +virDomainEventStateFree; +virDomainEventStateNew; virDomainEventWatchdogNewFromDom; virDomainEventWatchdogNewFromObj;
ACK Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
The same code for queueing, flushing, and deregistering events exists in multiple drivers, which will soon use these common functions. v2: Adjust libvirt_private.syms isDispatching bool fixes NONNULL tagging v3: Add requireTimer parameter to virDomainEventStateNew Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/conf/domain_event.c | 93 +++++++++++++++++++++++++++++++++++++++++++++- src/conf/domain_event.h | 24 +++++++++++- src/libvirt_private.syms | 4 ++ 3 files changed, 118 insertions(+), 3 deletions(-) diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c index 2771887..b85765e 100644 --- a/src/conf/domain_event.c +++ b/src/conf/domain_event.c @@ -557,10 +557,21 @@ virDomainEventStateFree(virDomainEventStatePtr state) virEventRemoveTimeout(state->timer); } +/** + * virDomainEventStateNew: + * @timeout_cb: virEventTimeoutCallback to call when timer expires + * @timeout_opaque: Data for timeout_cb + * @timeout_free: Optional virFreeCallback for freeing timeout_opaque + * @requireTimer: If true, return an error if registering the timer fails. + * This is fatal for drivers that sit behind the daemon + * (qemu, lxc), since there should always be a event impl + * registered. + */ virDomainEventStatePtr virDomainEventStateNew(virEventTimeoutCallback timeout_cb, void *timeout_opaque, - virFreeCallback timeout_free) + virFreeCallback timeout_free, + bool requireTimer) { virDomainEventStatePtr state = NULL; @@ -582,7 +593,14 @@ virDomainEventStateNew(virEventTimeoutCallback timeout_cb, timeout_cb, timeout_opaque, timeout_free)) < 0) { - goto error; + if (requireTimer == false) { + VIR_DEBUG("virEventAddTimeout failed: No addTimeoutImpl defined. " + "continuing without events."); + } else { + eventReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("could not initialize domain event timer")); + goto error; + } } return state; @@ -1059,3 +1077,74 @@ void virDomainEventQueueDispatch(virDomainEventQueuePtr queue, VIR_FREE(queue->events); queue->count = 0; } + +void +virDomainEventStateQueue(virDomainEventStatePtr state, + virDomainEventPtr event) +{ + if (state->timer < 0) { + virDomainEventFree(event); + return; + } + + if (virDomainEventQueuePush(state->queue, event) < 0) { + VIR_DEBUG("Error adding event to queue"); + virDomainEventFree(event); + } + + if (state->queue->count == 1) + virEventUpdateTimeout(state->timer, 0); +} + +void +virDomainEventStateFlush(virDomainEventStatePtr state, + virDomainEventDispatchFunc dispatchFunc, + void *opaque) +{ + virDomainEventQueue tempQueue; + + 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); + virDomainEventQueueDispatch(&tempQueue, + state->callbacks, + dispatchFunc, + opaque); + + /* Purge any deleted callbacks */ + virDomainEventCallbackListPurgeMarked(state->callbacks); + + state->isDispatching = false; +} + +int +virDomainEventStateDeregister(virConnectPtr conn, + virDomainEventStatePtr state, + virConnectDomainEventCallback callback) +{ + if (state->isDispatching) + return virDomainEventCallbackListMarkDelete(conn, + state->callbacks, callback); + else + return virDomainEventCallbackListRemove(conn, state->callbacks, callback); +} + +int +virDomainEventStateDeregisterAny(virConnectPtr conn, + virDomainEventStatePtr state, + int callbackID) +{ + if (state->isDispatching) + return virDomainEventCallbackListMarkDeleteID(conn, + state->callbacks, callbackID); + else + return virDomainEventCallbackListRemoveID(conn, + state->callbacks, callbackID); +} diff --git a/src/conf/domain_event.h b/src/conf/domain_event.h index 2ac3ecc..efc05f9 100644 --- a/src/conf/domain_event.h +++ b/src/conf/domain_event.h @@ -182,7 +182,8 @@ void virDomainEventStateFree(virDomainEventStatePtr state); virDomainEventStatePtr virDomainEventStateNew(virEventTimeoutCallback timeout_cb, void *timeout_opaque, - virFreeCallback timeout_free) + virFreeCallback timeout_free, + bool requireTimer) ATTRIBUTE_NONNULL(1); typedef void (*virDomainEventDispatchFunc)(virConnectPtr conn, @@ -205,4 +206,25 @@ void virDomainEventQueueDispatch(virDomainEventQueuePtr queue, virDomainEventDispatchFunc dispatch, void *opaque); + +void +virDomainEventStateQueue(virDomainEventStatePtr state, + virDomainEventPtr event) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); +void +virDomainEventStateFlush(virDomainEventStatePtr state, + virDomainEventDispatchFunc dispatchFunc, + void *opaque) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); +int +virDomainEventStateDeregister(virConnectPtr conn, + virDomainEventStatePtr state, + virConnectDomainEventCallback callback) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); +int +virDomainEventStateDeregisterAny(virConnectPtr conn, + virDomainEventStatePtr state, + int callbackID) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); + #endif diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index d4ad0c8..c98efdc 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -389,8 +389,12 @@ virDomainEventRTCChangeNewFromObj; virDomainEventRebootNew; virDomainEventRebootNewFromDom; virDomainEventRebootNewFromObj; +virDomainEventStateDeregister; +virDomainEventStateDeregisterAny; +virDomainEventStateFlush; virDomainEventStateFree; virDomainEventStateNew; +virDomainEventStateQueue; virDomainEventWatchdogNewFromDom; virDomainEventWatchdogNewFromObj; -- 1.7.4.4
On Thu, May 12, 2011 at 01:14:26PM -0400, Cole Robinson wrote:
The same code for queueing, flushing, and deregistering events exists in multiple drivers, which will soon use these common functions.
v2: Adjust libvirt_private.syms isDispatching bool fixes NONNULL tagging
v3: Add requireTimer parameter to virDomainEventStateNew
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/conf/domain_event.c | 93 +++++++++++++++++++++++++++++++++++++++++++++- src/conf/domain_event.h | 24 +++++++++++- src/libvirt_private.syms | 4 ++ 3 files changed, 118 insertions(+), 3 deletions(-)
diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c index 2771887..b85765e 100644 --- a/src/conf/domain_event.c +++ b/src/conf/domain_event.c @@ -557,10 +557,21 @@ virDomainEventStateFree(virDomainEventStatePtr state) virEventRemoveTimeout(state->timer); }
+/** + * virDomainEventStateNew: + * @timeout_cb: virEventTimeoutCallback to call when timer expires + * @timeout_opaque: Data for timeout_cb + * @timeout_free: Optional virFreeCallback for freeing timeout_opaque + * @requireTimer: If true, return an error if registering the timer fails. + * This is fatal for drivers that sit behind the daemon + * (qemu, lxc), since there should always be a event impl + * registered. + */ virDomainEventStatePtr virDomainEventStateNew(virEventTimeoutCallback timeout_cb, void *timeout_opaque, - virFreeCallback timeout_free) + virFreeCallback timeout_free, + bool requireTimer) { virDomainEventStatePtr state = NULL;
@@ -582,7 +593,14 @@ virDomainEventStateNew(virEventTimeoutCallback timeout_cb, timeout_cb, timeout_opaque, timeout_free)) < 0) { - goto error; + if (requireTimer == false) { + VIR_DEBUG("virEventAddTimeout failed: No addTimeoutImpl defined. " + "continuing without events."); + } else { + eventReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("could not initialize domain event timer")); + goto error; + } }
return state; @@ -1059,3 +1077,74 @@ void virDomainEventQueueDispatch(virDomainEventQueuePtr queue, VIR_FREE(queue->events); queue->count = 0; } + +void +virDomainEventStateQueue(virDomainEventStatePtr state, + virDomainEventPtr event) +{ + if (state->timer < 0) { + virDomainEventFree(event); + return; + } + + if (virDomainEventQueuePush(state->queue, event) < 0) { + VIR_DEBUG("Error adding event to queue"); + virDomainEventFree(event); + } + + if (state->queue->count == 1) + virEventUpdateTimeout(state->timer, 0); +} + +void +virDomainEventStateFlush(virDomainEventStatePtr state, + virDomainEventDispatchFunc dispatchFunc, + void *opaque) +{ + virDomainEventQueue tempQueue; + + 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); + virDomainEventQueueDispatch(&tempQueue, + state->callbacks, + dispatchFunc, + opaque); + + /* Purge any deleted callbacks */ + virDomainEventCallbackListPurgeMarked(state->callbacks); + + state->isDispatching = false; +} + +int +virDomainEventStateDeregister(virConnectPtr conn, + virDomainEventStatePtr state, + virConnectDomainEventCallback callback) +{ + if (state->isDispatching) + return virDomainEventCallbackListMarkDelete(conn, + state->callbacks, callback); + else + return virDomainEventCallbackListRemove(conn, state->callbacks, callback); +} + +int +virDomainEventStateDeregisterAny(virConnectPtr conn, + virDomainEventStatePtr state, + int callbackID) +{ + if (state->isDispatching) + return virDomainEventCallbackListMarkDeleteID(conn, + state->callbacks, callbackID); + else + return virDomainEventCallbackListRemoveID(conn, + state->callbacks, callbackID); +} diff --git a/src/conf/domain_event.h b/src/conf/domain_event.h index 2ac3ecc..efc05f9 100644 --- a/src/conf/domain_event.h +++ b/src/conf/domain_event.h @@ -182,7 +182,8 @@ void virDomainEventStateFree(virDomainEventStatePtr state); virDomainEventStatePtr virDomainEventStateNew(virEventTimeoutCallback timeout_cb, void *timeout_opaque, - virFreeCallback timeout_free) + virFreeCallback timeout_free, + bool requireTimer) ATTRIBUTE_NONNULL(1);
typedef void (*virDomainEventDispatchFunc)(virConnectPtr conn, @@ -205,4 +206,25 @@ void virDomainEventQueueDispatch(virDomainEventQueuePtr queue, virDomainEventDispatchFunc dispatch, void *opaque);
+ +void +virDomainEventStateQueue(virDomainEventStatePtr state, + virDomainEventPtr event) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); +void +virDomainEventStateFlush(virDomainEventStatePtr state, + virDomainEventDispatchFunc dispatchFunc, + void *opaque) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); +int +virDomainEventStateDeregister(virConnectPtr conn, + virDomainEventStatePtr state, + virConnectDomainEventCallback callback) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); +int +virDomainEventStateDeregisterAny(virConnectPtr conn, + virDomainEventStatePtr state, + int callbackID) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); + #endif diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index d4ad0c8..c98efdc 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -389,8 +389,12 @@ virDomainEventRTCChangeNewFromObj; virDomainEventRebootNew; virDomainEventRebootNewFromDom; virDomainEventRebootNewFromObj; +virDomainEventStateDeregister; +virDomainEventStateDeregisterAny; +virDomainEventStateFlush; virDomainEventStateFree; virDomainEventStateNew; +virDomainEventStateQueue; virDomainEventWatchdogNewFromDom; virDomainEventWatchdogNewFromObj;
ACK Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
v2: Drop libvirt_private.syms changes v3: Adjust for new virDomainEventStateNew argument Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/qemu/qemu_conf.h | 6 +----- src/qemu/qemu_domain.c | 29 ++++------------------------- src/qemu/qemu_driver.c | 46 ++++++++++++++++++---------------------------- 3 files changed, 23 insertions(+), 58 deletions(-) diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index f2bfa1e..ceec16d 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -109,11 +109,7 @@ struct qemud_driver { virCapsPtr caps; - /* An array of callbacks */ - virDomainEventCallbackListPtr domainEventCallbacks; - virDomainEventQueuePtr domainEventQueue; - int domainEventTimer; - int domainEventDispatching; + virDomainEventStatePtr domainEventState; char *securityDriverName; virSecurityManagerPtr securityManager; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 19e1938..fd465e5 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -65,28 +65,11 @@ static void qemuDomainEventDispatchFunc(virConnectPtr conn, void qemuDomainEventFlush(int timer ATTRIBUTE_UNUSED, void *opaque) { struct qemud_driver *driver = opaque; - virDomainEventQueue tempQueue; qemuDriverLock(driver); - - driver->domainEventDispatching = 1; - - /* Copy the queue, so we're reentrant safe */ - tempQueue.count = driver->domainEventQueue->count; - tempQueue.events = driver->domainEventQueue->events; - driver->domainEventQueue->count = 0; - driver->domainEventQueue->events = NULL; - - virEventUpdateTimeout(driver->domainEventTimer, -1); - virDomainEventQueueDispatch(&tempQueue, - driver->domainEventCallbacks, - qemuDomainEventDispatchFunc, - driver); - - /* Purge any deleted callbacks */ - virDomainEventCallbackListPurgeMarked(driver->domainEventCallbacks); - - driver->domainEventDispatching = 0; + virDomainEventStateFlush(driver->domainEventState, + qemuDomainEventDispatchFunc, + driver); qemuDriverUnlock(driver); } @@ -95,11 +78,7 @@ void qemuDomainEventFlush(int timer ATTRIBUTE_UNUSED, void *opaque) void qemuDomainEventQueue(struct qemud_driver *driver, virDomainEventPtr event) { - if (virDomainEventQueuePush(driver->domainEventQueue, - event) < 0) - virDomainEventFree(event); - if (driver->domainEventQueue->count == 1) - virEventUpdateTimeout(driver->domainEventTimer, 0); + virDomainEventStateQueue(driver->domainEventState, event); } diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 732c187..f98c163 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -393,14 +393,12 @@ qemudStartup(int privileged) { if (virDomainObjListInit(&qemu_driver->domains) < 0) goto out_of_memory; - /* Init callback list */ - if (VIR_ALLOC(qemu_driver->domainEventCallbacks) < 0) - goto out_of_memory; - if (!(qemu_driver->domainEventQueue = virDomainEventQueueNew())) - goto out_of_memory; - - if ((qemu_driver->domainEventTimer = - virEventAddTimeout(-1, qemuDomainEventFlush, qemu_driver, NULL)) < 0) + /* Init domain events */ + qemu_driver->domainEventState = virDomainEventStateNew(qemuDomainEventFlush, + qemu_driver, + NULL, + true); + if (!qemu_driver->domainEventState) goto error; /* Allocate bitmap for vnc port reservation */ @@ -764,11 +762,7 @@ qemudShutdown(void) { } /* Free domain callback list */ - virDomainEventCallbackListFree(qemu_driver->domainEventCallbacks); - virDomainEventQueueFree(qemu_driver->domainEventQueue); - - if (qemu_driver->domainEventTimer != -1) - virEventRemoveTimeout(qemu_driver->domainEventTimer); + virDomainEventStateFree(qemu_driver->domainEventState); if (qemu_driver->brctl) brShutdown(qemu_driver->brctl); @@ -856,7 +850,8 @@ static int qemudClose(virConnectPtr conn) { /* Get rid of callbacks registered for this conn */ qemuDriverLock(driver); - virDomainEventCallbackListRemoveConn(conn, driver->domainEventCallbacks); + virDomainEventCallbackListRemoveConn(conn, + driver->domainEventState->callbacks); qemuDriverUnlock(driver); conn->privateData = NULL; @@ -5563,7 +5558,8 @@ qemuDomainEventRegister(virConnectPtr conn, int ret; qemuDriverLock(driver); - ret = virDomainEventCallbackListAdd(conn, driver->domainEventCallbacks, + ret = virDomainEventCallbackListAdd(conn, + driver->domainEventState->callbacks, callback, opaque, freecb); qemuDriverUnlock(driver); @@ -5579,12 +5575,9 @@ qemuDomainEventDeregister(virConnectPtr conn, int ret; qemuDriverLock(driver); - if (driver->domainEventDispatching) - ret = virDomainEventCallbackListMarkDelete(conn, driver->domainEventCallbacks, - callback); - else - ret = virDomainEventCallbackListRemove(conn, driver->domainEventCallbacks, - callback); + ret = virDomainEventStateDeregister(conn, + driver->domainEventState, + callback); qemuDriverUnlock(driver); return ret; @@ -5604,7 +5597,7 @@ qemuDomainEventRegisterAny(virConnectPtr conn, qemuDriverLock(driver); ret = virDomainEventCallbackListAddID(conn, - driver->domainEventCallbacks, + driver->domainEventState->callbacks, dom, eventID, callback, opaque, freecb); qemuDriverUnlock(driver); @@ -5621,12 +5614,9 @@ qemuDomainEventDeregisterAny(virConnectPtr conn, int ret; qemuDriverLock(driver); - if (driver->domainEventDispatching) - ret = virDomainEventCallbackListMarkDeleteID(conn, driver->domainEventCallbacks, - callbackID); - else - ret = virDomainEventCallbackListRemoveID(conn, driver->domainEventCallbacks, - callbackID); + ret = virDomainEventStateDeregisterAny(conn, + driver->domainEventState, + callbackID); qemuDriverUnlock(driver); return ret; -- 1.7.4.4
On Thu, May 12, 2011 at 01:14:27PM -0400, Cole Robinson wrote:
v2: Drop libvirt_private.syms changes
v3: Adjust for new virDomainEventStateNew argument
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/qemu/qemu_conf.h | 6 +----- src/qemu/qemu_domain.c | 29 ++++------------------------- src/qemu/qemu_driver.c | 46 ++++++++++++++++++---------------------------- 3 files changed, 23 insertions(+), 58 deletions(-)
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index f2bfa1e..ceec16d 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -109,11 +109,7 @@ struct qemud_driver {
virCapsPtr caps;
- /* An array of callbacks */ - virDomainEventCallbackListPtr domainEventCallbacks; - virDomainEventQueuePtr domainEventQueue; - int domainEventTimer; - int domainEventDispatching; + virDomainEventStatePtr domainEventState;
char *securityDriverName; virSecurityManagerPtr securityManager; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 19e1938..fd465e5 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -65,28 +65,11 @@ static void qemuDomainEventDispatchFunc(virConnectPtr conn, void qemuDomainEventFlush(int timer ATTRIBUTE_UNUSED, void *opaque) { struct qemud_driver *driver = opaque; - virDomainEventQueue tempQueue;
qemuDriverLock(driver); - - driver->domainEventDispatching = 1; - - /* Copy the queue, so we're reentrant safe */ - tempQueue.count = driver->domainEventQueue->count; - tempQueue.events = driver->domainEventQueue->events; - driver->domainEventQueue->count = 0; - driver->domainEventQueue->events = NULL; - - virEventUpdateTimeout(driver->domainEventTimer, -1); - virDomainEventQueueDispatch(&tempQueue, - driver->domainEventCallbacks, - qemuDomainEventDispatchFunc, - driver); - - /* Purge any deleted callbacks */ - virDomainEventCallbackListPurgeMarked(driver->domainEventCallbacks); - - driver->domainEventDispatching = 0; + virDomainEventStateFlush(driver->domainEventState, + qemuDomainEventDispatchFunc, + driver); qemuDriverUnlock(driver); }
@@ -95,11 +78,7 @@ void qemuDomainEventFlush(int timer ATTRIBUTE_UNUSED, void *opaque) void qemuDomainEventQueue(struct qemud_driver *driver, virDomainEventPtr event) { - if (virDomainEventQueuePush(driver->domainEventQueue, - event) < 0) - virDomainEventFree(event); - if (driver->domainEventQueue->count == 1) - virEventUpdateTimeout(driver->domainEventTimer, 0); + virDomainEventStateQueue(driver->domainEventState, event); }
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 732c187..f98c163 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -393,14 +393,12 @@ qemudStartup(int privileged) { if (virDomainObjListInit(&qemu_driver->domains) < 0) goto out_of_memory;
- /* Init callback list */ - if (VIR_ALLOC(qemu_driver->domainEventCallbacks) < 0) - goto out_of_memory; - if (!(qemu_driver->domainEventQueue = virDomainEventQueueNew())) - goto out_of_memory; - - if ((qemu_driver->domainEventTimer = - virEventAddTimeout(-1, qemuDomainEventFlush, qemu_driver, NULL)) < 0) + /* Init domain events */ + qemu_driver->domainEventState = virDomainEventStateNew(qemuDomainEventFlush, + qemu_driver, + NULL, + true); + if (!qemu_driver->domainEventState) goto error;
/* Allocate bitmap for vnc port reservation */ @@ -764,11 +762,7 @@ qemudShutdown(void) { }
/* Free domain callback list */ - virDomainEventCallbackListFree(qemu_driver->domainEventCallbacks); - virDomainEventQueueFree(qemu_driver->domainEventQueue); - - if (qemu_driver->domainEventTimer != -1) - virEventRemoveTimeout(qemu_driver->domainEventTimer); + virDomainEventStateFree(qemu_driver->domainEventState);
if (qemu_driver->brctl) brShutdown(qemu_driver->brctl); @@ -856,7 +850,8 @@ static int qemudClose(virConnectPtr conn) {
/* Get rid of callbacks registered for this conn */ qemuDriverLock(driver); - virDomainEventCallbackListRemoveConn(conn, driver->domainEventCallbacks); + virDomainEventCallbackListRemoveConn(conn, + driver->domainEventState->callbacks); qemuDriverUnlock(driver);
conn->privateData = NULL; @@ -5563,7 +5558,8 @@ qemuDomainEventRegister(virConnectPtr conn, int ret;
qemuDriverLock(driver); - ret = virDomainEventCallbackListAdd(conn, driver->domainEventCallbacks, + ret = virDomainEventCallbackListAdd(conn, + driver->domainEventState->callbacks, callback, opaque, freecb); qemuDriverUnlock(driver);
@@ -5579,12 +5575,9 @@ qemuDomainEventDeregister(virConnectPtr conn, int ret;
qemuDriverLock(driver); - if (driver->domainEventDispatching) - ret = virDomainEventCallbackListMarkDelete(conn, driver->domainEventCallbacks, - callback); - else - ret = virDomainEventCallbackListRemove(conn, driver->domainEventCallbacks, - callback); + ret = virDomainEventStateDeregister(conn, + driver->domainEventState, + callback); qemuDriverUnlock(driver);
return ret; @@ -5604,7 +5597,7 @@ qemuDomainEventRegisterAny(virConnectPtr conn,
qemuDriverLock(driver); ret = virDomainEventCallbackListAddID(conn, - driver->domainEventCallbacks, + driver->domainEventState->callbacks, dom, eventID, callback, opaque, freecb); qemuDriverUnlock(driver); @@ -5621,12 +5614,9 @@ qemuDomainEventDeregisterAny(virConnectPtr conn, int ret;
qemuDriverLock(driver); - if (driver->domainEventDispatching) - ret = virDomainEventCallbackListMarkDeleteID(conn, driver->domainEventCallbacks, - callbackID); - else - ret = virDomainEventCallbackListRemoveID(conn, driver->domainEventCallbacks, - callbackID); + ret = virDomainEventStateDeregisterAny(conn, + driver->domainEventState, + callbackID); qemuDriverUnlock(driver);
return ret;
ACK Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
v3: Adjust for new virDomainEventStateNew argument Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/lxc/lxc_conf.h | 6 +--- src/lxc/lxc_driver.c | 74 ++++++++++++++----------------------------------- 2 files changed, 22 insertions(+), 58 deletions(-) diff --git a/src/lxc/lxc_conf.h b/src/lxc/lxc_conf.h index 4f1ead3..66aa469 100644 --- a/src/lxc/lxc_conf.h +++ b/src/lxc/lxc_conf.h @@ -55,11 +55,7 @@ struct __lxc_driver { int log_libvirtd; int have_netns; - /* An array of callbacks */ - virDomainEventCallbackListPtr domainEventCallbacks; - virDomainEventQueuePtr domainEventQueue; - int domainEventTimer; - int domainEventDispatching; + virDomainEventStatePtr domainEventState; }; int lxcLoadDriverConfig(lxc_driver_t *driver); diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 7a3e33d..31adcb7 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -159,7 +159,8 @@ static int lxcClose(virConnectPtr conn) lxc_driver_t *driver = conn->privateData; lxcDriverLock(driver); - virDomainEventCallbackListRemoveConn(conn, driver->domainEventCallbacks); + virDomainEventCallbackListRemoveConn(conn, + driver->domainEventState->callbacks); lxcDriverUnlock(driver); conn->privateData = NULL; @@ -1648,7 +1649,8 @@ lxcDomainEventRegister(virConnectPtr conn, int ret; lxcDriverLock(driver); - ret = virDomainEventCallbackListAdd(conn, driver->domainEventCallbacks, + ret = virDomainEventCallbackListAdd(conn, + driver->domainEventState->callbacks, callback, opaque, freecb); lxcDriverUnlock(driver); @@ -1664,12 +1666,9 @@ lxcDomainEventDeregister(virConnectPtr conn, int ret; lxcDriverLock(driver); - if (driver->domainEventDispatching) - ret = virDomainEventCallbackListMarkDelete(conn, driver->domainEventCallbacks, - callback); - else - ret = virDomainEventCallbackListRemove(conn, driver->domainEventCallbacks, - callback); + ret = virDomainEventStateDeregister(conn, + driver->domainEventState, + callback); lxcDriverUnlock(driver); return ret; @@ -1689,7 +1688,7 @@ lxcDomainEventRegisterAny(virConnectPtr conn, lxcDriverLock(driver); ret = virDomainEventCallbackListAddID(conn, - driver->domainEventCallbacks, + driver->domainEventState->callbacks, dom, eventID, callback, opaque, freecb); lxcDriverUnlock(driver); @@ -1706,12 +1705,9 @@ lxcDomainEventDeregisterAny(virConnectPtr conn, int ret; lxcDriverLock(driver); - if (driver->domainEventDispatching) - ret = virDomainEventCallbackListMarkDeleteID(conn, driver->domainEventCallbacks, - callbackID); - else - ret = virDomainEventCallbackListRemoveID(conn, driver->domainEventCallbacks, - callbackID); + ret = virDomainEventStateDeregisterAny(conn, + driver->domainEventState, + callbackID); lxcDriverUnlock(driver); return ret; @@ -1736,28 +1732,11 @@ static void lxcDomainEventDispatchFunc(virConnectPtr conn, static void lxcDomainEventFlush(int timer ATTRIBUTE_UNUSED, void *opaque) { lxc_driver_t *driver = opaque; - virDomainEventQueue tempQueue; lxcDriverLock(driver); - - driver->domainEventDispatching = 1; - - /* Copy the queue, so we're reentrant safe */ - tempQueue.count = driver->domainEventQueue->count; - tempQueue.events = driver->domainEventQueue->events; - driver->domainEventQueue->count = 0; - driver->domainEventQueue->events = NULL; - - virEventUpdateTimeout(driver->domainEventTimer, -1); - virDomainEventQueueDispatch(&tempQueue, - driver->domainEventCallbacks, - lxcDomainEventDispatchFunc, - driver); - - /* Purge any deleted callbacks */ - virDomainEventCallbackListPurgeMarked(driver->domainEventCallbacks); - - driver->domainEventDispatching = 0; + virDomainEventStateFlush(driver->domainEventState, + lxcDomainEventDispatchFunc, + driver); lxcDriverUnlock(driver); } @@ -1766,11 +1745,7 @@ static void lxcDomainEventFlush(int timer ATTRIBUTE_UNUSED, void *opaque) static void lxcDomainEventQueue(lxc_driver_t *driver, virDomainEventPtr event) { - if (virDomainEventQueuePush(driver->domainEventQueue, - event) < 0) - virDomainEventFree(event); - if (lxc_driver->domainEventQueue->count == 1) - virEventUpdateTimeout(driver->domainEventTimer, 0); + virDomainEventStateQueue(driver->domainEventState, event); } /** @@ -1985,13 +1960,11 @@ static int lxcStartup(int privileged) if (virDomainObjListInit(&lxc_driver->domains) < 0) goto cleanup; - if (VIR_ALLOC(lxc_driver->domainEventCallbacks) < 0) - goto cleanup; - if (!(lxc_driver->domainEventQueue = virDomainEventQueueNew())) - goto cleanup; - - if ((lxc_driver->domainEventTimer = - virEventAddTimeout(-1, lxcDomainEventFlush, lxc_driver, NULL)) < 0) + lxc_driver->domainEventState = virDomainEventStateNew(lxcDomainEventFlush, + lxc_driver, + NULL, + true); + if (!lxc_driver->domainEventState) goto cleanup; lxc_driver->log_libvirtd = 0; /* by default log to container logfile */ @@ -2083,12 +2056,7 @@ static int lxcShutdown(void) lxcDriverLock(lxc_driver); virDomainObjListDeinit(&lxc_driver->domains); - - virDomainEventCallbackListFree(lxc_driver->domainEventCallbacks); - virDomainEventQueueFree(lxc_driver->domainEventQueue); - - if (lxc_driver->domainEventTimer != -1) - virEventRemoveTimeout(lxc_driver->domainEventTimer); + virDomainEventStateFree(lxc_driver->domainEventState); virCapabilitiesFree(lxc_driver->caps); VIR_FREE(lxc_driver->configDir); -- 1.7.4.4
On Thu, May 12, 2011 at 01:14:28PM -0400, Cole Robinson wrote:
v3: Adjust for new virDomainEventStateNew argument
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/lxc/lxc_conf.h | 6 +--- src/lxc/lxc_driver.c | 74 ++++++++++++++----------------------------------- 2 files changed, 22 insertions(+), 58 deletions(-)
diff --git a/src/lxc/lxc_conf.h b/src/lxc/lxc_conf.h index 4f1ead3..66aa469 100644 --- a/src/lxc/lxc_conf.h +++ b/src/lxc/lxc_conf.h @@ -55,11 +55,7 @@ struct __lxc_driver { int log_libvirtd; int have_netns;
- /* An array of callbacks */ - virDomainEventCallbackListPtr domainEventCallbacks; - virDomainEventQueuePtr domainEventQueue; - int domainEventTimer; - int domainEventDispatching; + virDomainEventStatePtr domainEventState; };
int lxcLoadDriverConfig(lxc_driver_t *driver); diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 7a3e33d..31adcb7 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -159,7 +159,8 @@ static int lxcClose(virConnectPtr conn) lxc_driver_t *driver = conn->privateData;
lxcDriverLock(driver); - virDomainEventCallbackListRemoveConn(conn, driver->domainEventCallbacks); + virDomainEventCallbackListRemoveConn(conn, + driver->domainEventState->callbacks); lxcDriverUnlock(driver);
conn->privateData = NULL; @@ -1648,7 +1649,8 @@ lxcDomainEventRegister(virConnectPtr conn, int ret;
lxcDriverLock(driver); - ret = virDomainEventCallbackListAdd(conn, driver->domainEventCallbacks, + ret = virDomainEventCallbackListAdd(conn, + driver->domainEventState->callbacks, callback, opaque, freecb); lxcDriverUnlock(driver);
@@ -1664,12 +1666,9 @@ lxcDomainEventDeregister(virConnectPtr conn, int ret;
lxcDriverLock(driver); - if (driver->domainEventDispatching) - ret = virDomainEventCallbackListMarkDelete(conn, driver->domainEventCallbacks, - callback); - else - ret = virDomainEventCallbackListRemove(conn, driver->domainEventCallbacks, - callback); + ret = virDomainEventStateDeregister(conn, + driver->domainEventState, + callback); lxcDriverUnlock(driver);
return ret; @@ -1689,7 +1688,7 @@ lxcDomainEventRegisterAny(virConnectPtr conn,
lxcDriverLock(driver); ret = virDomainEventCallbackListAddID(conn, - driver->domainEventCallbacks, + driver->domainEventState->callbacks, dom, eventID, callback, opaque, freecb); lxcDriverUnlock(driver); @@ -1706,12 +1705,9 @@ lxcDomainEventDeregisterAny(virConnectPtr conn, int ret;
lxcDriverLock(driver); - if (driver->domainEventDispatching) - ret = virDomainEventCallbackListMarkDeleteID(conn, driver->domainEventCallbacks, - callbackID); - else - ret = virDomainEventCallbackListRemoveID(conn, driver->domainEventCallbacks, - callbackID); + ret = virDomainEventStateDeregisterAny(conn, + driver->domainEventState, + callbackID); lxcDriverUnlock(driver);
return ret; @@ -1736,28 +1732,11 @@ static void lxcDomainEventDispatchFunc(virConnectPtr conn, static void lxcDomainEventFlush(int timer ATTRIBUTE_UNUSED, void *opaque) { lxc_driver_t *driver = opaque; - virDomainEventQueue tempQueue;
lxcDriverLock(driver); - - driver->domainEventDispatching = 1; - - /* Copy the queue, so we're reentrant safe */ - tempQueue.count = driver->domainEventQueue->count; - tempQueue.events = driver->domainEventQueue->events; - driver->domainEventQueue->count = 0; - driver->domainEventQueue->events = NULL; - - virEventUpdateTimeout(driver->domainEventTimer, -1); - virDomainEventQueueDispatch(&tempQueue, - driver->domainEventCallbacks, - lxcDomainEventDispatchFunc, - driver); - - /* Purge any deleted callbacks */ - virDomainEventCallbackListPurgeMarked(driver->domainEventCallbacks); - - driver->domainEventDispatching = 0; + virDomainEventStateFlush(driver->domainEventState, + lxcDomainEventDispatchFunc, + driver); lxcDriverUnlock(driver); }
@@ -1766,11 +1745,7 @@ static void lxcDomainEventFlush(int timer ATTRIBUTE_UNUSED, void *opaque) static void lxcDomainEventQueue(lxc_driver_t *driver, virDomainEventPtr event) { - if (virDomainEventQueuePush(driver->domainEventQueue, - event) < 0) - virDomainEventFree(event); - if (lxc_driver->domainEventQueue->count == 1) - virEventUpdateTimeout(driver->domainEventTimer, 0); + virDomainEventStateQueue(driver->domainEventState, event); }
/** @@ -1985,13 +1960,11 @@ static int lxcStartup(int privileged) if (virDomainObjListInit(&lxc_driver->domains) < 0) goto cleanup;
- if (VIR_ALLOC(lxc_driver->domainEventCallbacks) < 0) - goto cleanup; - if (!(lxc_driver->domainEventQueue = virDomainEventQueueNew())) - goto cleanup; - - if ((lxc_driver->domainEventTimer = - virEventAddTimeout(-1, lxcDomainEventFlush, lxc_driver, NULL)) < 0) + lxc_driver->domainEventState = virDomainEventStateNew(lxcDomainEventFlush, + lxc_driver, + NULL, + true); + if (!lxc_driver->domainEventState) goto cleanup;
lxc_driver->log_libvirtd = 0; /* by default log to container logfile */ @@ -2083,12 +2056,7 @@ static int lxcShutdown(void)
lxcDriverLock(lxc_driver); virDomainObjListDeinit(&lxc_driver->domains); - - virDomainEventCallbackListFree(lxc_driver->domainEventCallbacks); - virDomainEventQueueFree(lxc_driver->domainEventQueue); - - if (lxc_driver->domainEventTimer != -1) - virEventRemoveTimeout(lxc_driver->domainEventTimer); + virDomainEventStateFree(lxc_driver->domainEventState);
virCapabilitiesFree(lxc_driver->caps); VIR_FREE(lxc_driver->configDir);
ACK Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
v3: Adjust for new virDomainEventStateNew argument Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/test/test_driver.c | 105 +++++++++++++++--------------------------------- 1 files changed, 32 insertions(+), 73 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index bf33d13..f244a0f 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -90,12 +90,7 @@ struct _testConn { int numCells; testCell cells[MAX_CELLS]; - - /* An array of callbacks */ - virDomainEventCallbackListPtr domainEventCallbacks; - virDomainEventQueuePtr domainEventQueue; - int domainEventTimer; - int domainEventDispatching; + virDomainEventStatePtr domainEventState; }; typedef struct _testConn testConn; typedef struct _testConn *testConnPtr; @@ -1124,6 +1119,7 @@ static virDrvOpenStatus testOpen(virConnectPtr conn, int flags ATTRIBUTE_UNUSED) { int ret; + testConnPtr privconn; if (!conn->uri) return VIR_DRV_OPEN_DECLINED; @@ -1150,26 +1146,25 @@ static virDrvOpenStatus testOpen(virConnectPtr conn, ret = testOpenFromFile(conn, conn->uri->path); - if (ret == VIR_DRV_OPEN_SUCCESS) { - testConnPtr privconn = conn->privateData; - testDriverLock(privconn); - /* Init callback list */ - if (VIR_ALLOC(privconn->domainEventCallbacks) < 0 || - !(privconn->domainEventQueue = virDomainEventQueueNew())) { - virReportOOMError(); - testDriverUnlock(privconn); - testClose(conn); - return VIR_DRV_OPEN_ERROR; - } + if (ret != VIR_DRV_OPEN_SUCCESS) + return ret; + + privconn = conn->privateData; + testDriverLock(privconn); - if ((privconn->domainEventTimer = - virEventAddTimeout(-1, testDomainEventFlush, privconn, NULL)) < 0) - VIR_DEBUG("virEventAddTimeout failed: No addTimeoutImpl defined. " - "continuing without events."); + privconn->domainEventState = virDomainEventStateNew(testDomainEventFlush, + privconn, + NULL, + false); + if (!privconn->domainEventState) { testDriverUnlock(privconn); + testClose(conn); + return VIR_DRV_OPEN_ERROR; } - return (ret); + testDriverUnlock(privconn); + + return VIR_DRV_OPEN_SUCCESS; } static int testClose(virConnectPtr conn) @@ -1182,12 +1177,7 @@ static int testClose(virConnectPtr conn) virNetworkObjListFree(&privconn->networks); virInterfaceObjListFree(&privconn->ifaces); virStoragePoolObjListFree(&privconn->pools); - - virDomainEventCallbackListFree(privconn->domainEventCallbacks); - virDomainEventQueueFree(privconn->domainEventQueue); - - if (privconn->domainEventTimer != -1) - virEventRemoveTimeout(privconn->domainEventTimer); + virDomainEventStateFree(privconn->domainEventState); testDriverUnlock(privconn); virMutexDestroy(&privconn->lock); @@ -5177,7 +5167,8 @@ testDomainEventRegister(virConnectPtr conn, int ret; testDriverLock(driver); - ret = virDomainEventCallbackListAdd(conn, driver->domainEventCallbacks, + ret = virDomainEventCallbackListAdd(conn, + driver->domainEventState->callbacks, callback, opaque, freecb); testDriverUnlock(driver); @@ -5193,12 +5184,9 @@ testDomainEventDeregister(virConnectPtr conn, int ret; testDriverLock(driver); - if (driver->domainEventDispatching) - ret = virDomainEventCallbackListMarkDelete(conn, driver->domainEventCallbacks, - callback); - else - ret = virDomainEventCallbackListRemove(conn, driver->domainEventCallbacks, - callback); + ret = virDomainEventStateDeregister(conn, + driver->domainEventState, + callback); testDriverUnlock(driver); return ret; @@ -5217,7 +5205,8 @@ testDomainEventRegisterAny(virConnectPtr conn, int ret; testDriverLock(driver); - ret = virDomainEventCallbackListAddID(conn, driver->domainEventCallbacks, + ret = virDomainEventCallbackListAddID(conn, + driver->domainEventState->callbacks, dom, eventID, callback, opaque, freecb); testDriverUnlock(driver); @@ -5233,12 +5222,9 @@ testDomainEventDeregisterAny(virConnectPtr conn, int ret; testDriverLock(driver); - if (driver->domainEventDispatching) - ret = virDomainEventCallbackListMarkDeleteID(conn, driver->domainEventCallbacks, - callbackID); - else - ret = virDomainEventCallbackListRemoveID(conn, driver->domainEventCallbacks, - callbackID); + ret = virDomainEventStateDeregisterAny(conn, + driver->domainEventState, + callbackID); testDriverUnlock(driver); return ret; @@ -5262,28 +5248,11 @@ static void testDomainEventDispatchFunc(virConnectPtr conn, static void testDomainEventFlush(int timer ATTRIBUTE_UNUSED, void *opaque) { testConnPtr driver = opaque; - virDomainEventQueue tempQueue; testDriverLock(driver); - - driver->domainEventDispatching = 1; - - /* Copy the queue, so we're reentrant safe */ - tempQueue.count = driver->domainEventQueue->count; - tempQueue.events = driver->domainEventQueue->events; - driver->domainEventQueue->count = 0; - driver->domainEventQueue->events = NULL; - - virEventUpdateTimeout(driver->domainEventTimer, -1); - virDomainEventQueueDispatch(&tempQueue, - driver->domainEventCallbacks, - testDomainEventDispatchFunc, - driver); - - /* Purge any deleted callbacks */ - virDomainEventCallbackListPurgeMarked(driver->domainEventCallbacks); - - driver->domainEventDispatching = 0; + virDomainEventStateFlush(driver->domainEventState, + testDomainEventDispatchFunc, + driver); testDriverUnlock(driver); } @@ -5292,17 +5261,7 @@ static void testDomainEventFlush(int timer ATTRIBUTE_UNUSED, void *opaque) static void testDomainEventQueue(testConnPtr driver, virDomainEventPtr event) { - if (driver->domainEventTimer < 0) { - virDomainEventFree(event); - return; - } - - if (virDomainEventQueuePush(driver->domainEventQueue, - event) < 0) - virDomainEventFree(event); - - if (driver->domainEventQueue->count == 1) - virEventUpdateTimeout(driver->domainEventTimer, 0); + virDomainEventStateQueue(driver->domainEventState, event); } static virDrvOpenStatus testSecretOpen(virConnectPtr conn, -- 1.7.4.4
On Thu, May 12, 2011 at 01:14:29PM -0400, Cole Robinson wrote:
v3: Adjust for new virDomainEventStateNew argument
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/test/test_driver.c | 105 +++++++++++++++--------------------------------- 1 files changed, 32 insertions(+), 73 deletions(-)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c index bf33d13..f244a0f 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -90,12 +90,7 @@ struct _testConn { int numCells; testCell cells[MAX_CELLS];
- - /* An array of callbacks */ - virDomainEventCallbackListPtr domainEventCallbacks; - virDomainEventQueuePtr domainEventQueue; - int domainEventTimer; - int domainEventDispatching; + virDomainEventStatePtr domainEventState; }; typedef struct _testConn testConn; typedef struct _testConn *testConnPtr; @@ -1124,6 +1119,7 @@ static virDrvOpenStatus testOpen(virConnectPtr conn, int flags ATTRIBUTE_UNUSED) { int ret; + testConnPtr privconn;
if (!conn->uri) return VIR_DRV_OPEN_DECLINED; @@ -1150,26 +1146,25 @@ static virDrvOpenStatus testOpen(virConnectPtr conn, ret = testOpenFromFile(conn, conn->uri->path);
- if (ret == VIR_DRV_OPEN_SUCCESS) { - testConnPtr privconn = conn->privateData; - testDriverLock(privconn); - /* Init callback list */ - if (VIR_ALLOC(privconn->domainEventCallbacks) < 0 || - !(privconn->domainEventQueue = virDomainEventQueueNew())) { - virReportOOMError(); - testDriverUnlock(privconn); - testClose(conn); - return VIR_DRV_OPEN_ERROR; - } + if (ret != VIR_DRV_OPEN_SUCCESS) + return ret; + + privconn = conn->privateData; + testDriverLock(privconn);
- if ((privconn->domainEventTimer = - virEventAddTimeout(-1, testDomainEventFlush, privconn, NULL)) < 0) - VIR_DEBUG("virEventAddTimeout failed: No addTimeoutImpl defined. " - "continuing without events."); + privconn->domainEventState = virDomainEventStateNew(testDomainEventFlush, + privconn, + NULL, + false); + if (!privconn->domainEventState) { testDriverUnlock(privconn); + testClose(conn); + return VIR_DRV_OPEN_ERROR; }
- return (ret); + testDriverUnlock(privconn); + + return VIR_DRV_OPEN_SUCCESS; }
static int testClose(virConnectPtr conn) @@ -1182,12 +1177,7 @@ static int testClose(virConnectPtr conn) virNetworkObjListFree(&privconn->networks); virInterfaceObjListFree(&privconn->ifaces); virStoragePoolObjListFree(&privconn->pools); - - virDomainEventCallbackListFree(privconn->domainEventCallbacks); - virDomainEventQueueFree(privconn->domainEventQueue); - - if (privconn->domainEventTimer != -1) - virEventRemoveTimeout(privconn->domainEventTimer); + virDomainEventStateFree(privconn->domainEventState);
testDriverUnlock(privconn); virMutexDestroy(&privconn->lock); @@ -5177,7 +5167,8 @@ testDomainEventRegister(virConnectPtr conn, int ret;
testDriverLock(driver); - ret = virDomainEventCallbackListAdd(conn, driver->domainEventCallbacks, + ret = virDomainEventCallbackListAdd(conn, + driver->domainEventState->callbacks, callback, opaque, freecb); testDriverUnlock(driver);
@@ -5193,12 +5184,9 @@ testDomainEventDeregister(virConnectPtr conn, int ret;
testDriverLock(driver); - if (driver->domainEventDispatching) - ret = virDomainEventCallbackListMarkDelete(conn, driver->domainEventCallbacks, - callback); - else - ret = virDomainEventCallbackListRemove(conn, driver->domainEventCallbacks, - callback); + ret = virDomainEventStateDeregister(conn, + driver->domainEventState, + callback); testDriverUnlock(driver);
return ret; @@ -5217,7 +5205,8 @@ testDomainEventRegisterAny(virConnectPtr conn, int ret;
testDriverLock(driver); - ret = virDomainEventCallbackListAddID(conn, driver->domainEventCallbacks, + ret = virDomainEventCallbackListAddID(conn, + driver->domainEventState->callbacks, dom, eventID, callback, opaque, freecb); testDriverUnlock(driver); @@ -5233,12 +5222,9 @@ testDomainEventDeregisterAny(virConnectPtr conn, int ret;
testDriverLock(driver); - if (driver->domainEventDispatching) - ret = virDomainEventCallbackListMarkDeleteID(conn, driver->domainEventCallbacks, - callbackID); - else - ret = virDomainEventCallbackListRemoveID(conn, driver->domainEventCallbacks, - callbackID); + ret = virDomainEventStateDeregisterAny(conn, + driver->domainEventState, + callbackID); testDriverUnlock(driver);
return ret; @@ -5262,28 +5248,11 @@ static void testDomainEventDispatchFunc(virConnectPtr conn, static void testDomainEventFlush(int timer ATTRIBUTE_UNUSED, void *opaque) { testConnPtr driver = opaque; - virDomainEventQueue tempQueue;
testDriverLock(driver); - - driver->domainEventDispatching = 1; - - /* Copy the queue, so we're reentrant safe */ - tempQueue.count = driver->domainEventQueue->count; - tempQueue.events = driver->domainEventQueue->events; - driver->domainEventQueue->count = 0; - driver->domainEventQueue->events = NULL; - - virEventUpdateTimeout(driver->domainEventTimer, -1); - virDomainEventQueueDispatch(&tempQueue, - driver->domainEventCallbacks, - testDomainEventDispatchFunc, - driver); - - /* Purge any deleted callbacks */ - virDomainEventCallbackListPurgeMarked(driver->domainEventCallbacks); - - driver->domainEventDispatching = 0; + virDomainEventStateFlush(driver->domainEventState, + testDomainEventDispatchFunc, + driver); testDriverUnlock(driver); }
@@ -5292,17 +5261,7 @@ static void testDomainEventFlush(int timer ATTRIBUTE_UNUSED, void *opaque) static void testDomainEventQueue(testConnPtr driver, virDomainEventPtr event) { - if (driver->domainEventTimer < 0) { - virDomainEventFree(event); - return; - } - - if (virDomainEventQueuePush(driver->domainEventQueue, - event) < 0) - virDomainEventFree(event); - - if (driver->domainEventQueue->count == 1) - virEventUpdateTimeout(driver->domainEventTimer, 0); + virDomainEventStateQueue(driver->domainEventState, event); }
static virDrvOpenStatus testSecretOpen(virConnectPtr conn,
ACK Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/libxl/libxl_conf.h | 6 +--- src/libxl/libxl_driver.c | 80 +++++++++++++-------------------------------- 2 files changed, 24 insertions(+), 62 deletions(-) diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h index 8c87786..65110cf 100644 --- a/src/libxl/libxl_conf.h +++ b/src/libxl/libxl_conf.h @@ -61,11 +61,7 @@ struct _libxlDriverPrivate { virBitmapPtr reservedVNCPorts; virDomainObjList domains; - /* A list of callbacks */ - virDomainEventCallbackListPtr domainEventCallbacks; - virDomainEventQueuePtr domainEventQueue; - int domainEventTimer; - int domainEventDispatching; + virDomainEventStatePtr domainEventState; char *configDir; char *autostartDir; diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index d7ff0c6..a1c4509 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -130,28 +130,11 @@ static void libxlDomainEventFlush(int timer ATTRIBUTE_UNUSED, void *opaque) { libxlDriverPrivatePtr driver = opaque; - virDomainEventQueue tempQueue; libxlDriverLock(driver); - - driver->domainEventDispatching = 1; - - /* Copy the queue, so we're reentrant safe */ - tempQueue.count = driver->domainEventQueue->count; - tempQueue.events = driver->domainEventQueue->events; - driver->domainEventQueue->count = 0; - driver->domainEventQueue->events = NULL; - - virEventUpdateTimeout(driver->domainEventTimer, -1); - virDomainEventQueueDispatch(&tempQueue, - driver->domainEventCallbacks, - libxlDomainEventDispatchFunc, - driver); - - /* Purge any deleted callbacks */ - virDomainEventCallbackListPurgeMarked(driver->domainEventCallbacks); - - driver->domainEventDispatching = 0; + virDomainEventStateFlush(driver->domainEventState, + libxmlDomainEventDispatchFunc, + driver); libxlDriverUnlock(driver); } @@ -159,10 +142,7 @@ libxlDomainEventFlush(int timer ATTRIBUTE_UNUSED, void *opaque) static void libxlDomainEventQueue(libxlDriverPrivatePtr driver, virDomainEventPtr event) { - if (virDomainEventQueuePush(driver->domainEventQueue, event) < 0) - virDomainEventFree(event); - if (driver->domainEventQueue->count == 1) - virEventUpdateTimeout(driver->domainEventTimer, 0); + virDomainEventStateQueue(driver->domainEventState, event); } /* @@ -706,12 +686,7 @@ libxlShutdown(void) VIR_FREE(libxl_driver->libDir); VIR_FREE(libxl_driver->saveDir); - /* Free domain callback list */ - virDomainEventCallbackListFree(libxl_driver->domainEventCallbacks); - virDomainEventQueueFree(libxl_driver->domainEventQueue); - - if (libxl_driver->domainEventTimer != -1) - virEventRemoveTimeout(libxl_driver->domainEventTimer); + virDomainEventStateFree(privconn->domainEventState); libxlDriverUnlock(libxl_driver); virMutexDestroy(&libxl_driver->lock); @@ -822,16 +797,14 @@ libxlStartup(int privileged) { } VIR_FREE(log_file); - /* Init callback list */ - if (VIR_ALLOC(libxl_driver->domainEventCallbacks) < 0) - goto out_of_memory; - if (!(libxl_driver->domainEventQueue = virDomainEventQueueNew())) - goto out_of_memory; - if ((libxl_driver->domainEventTimer = - virEventAddTimeout(-1, libxlDomainEventFlush, libxl_driver, NULL)) < 0) + libxl_driver->domainEventState = virDomainEventStateNew( + libxlDomainEventFlush, + libxml_driver, + NULL, + false); + if (!libxml_driver->domainEventState) goto error; - libxl_driver->logger = (xentoollog_logger *)xtl_createlogger_stdiostream(libxl_driver->logger_file, XTL_DEBUG, 0); if (!libxl_driver->logger) { @@ -982,7 +955,8 @@ libxlClose(virConnectPtr conn ATTRIBUTE_UNUSED) libxlDriverPrivatePtr driver = conn->privateData; libxlDriverLock(driver); - virDomainEventCallbackListRemoveConn(conn, driver->domainEventCallbacks); + virDomainEventCallbackListRemoveConn(conn, + driver->domainEventState->callbacks); libxlDriverUnlock(driver); conn->privateData = NULL; return 0; @@ -2224,7 +2198,8 @@ libxlDomainEventRegister(virConnectPtr conn, int ret; libxlDriverLock(driver); - ret = virDomainEventCallbackListAdd(conn, driver->domainEventCallbacks, + ret = virDomainEventCallbackListAdd(conn, + driver->domainEventState->callbacks, callback, opaque, freecb); libxlDriverUnlock(driver); @@ -2240,14 +2215,9 @@ libxlDomainEventDeregister(virConnectPtr conn, int ret; libxlDriverLock(driver); - if (driver->domainEventDispatching) - ret = virDomainEventCallbackListMarkDelete(conn, - driver->domainEventCallbacks, - callback); - else - ret = virDomainEventCallbackListRemove(conn, - driver->domainEventCallbacks, - callback); + ret = virDomainEventStateDeregister(conn, + driver->domainEventState, + callback); libxlDriverUnlock(driver); return ret; @@ -2648,7 +2618,8 @@ libxlDomainEventRegisterAny(virConnectPtr conn, virDomainPtr dom, int eventID, int ret; libxlDriverLock(driver); - ret = virDomainEventCallbackListAddID(conn, driver->domainEventCallbacks, + ret = virDomainEventCallbackListAddID(conn, + driver->domainEventState->callbacks, dom, eventID, callback, opaque, freecb); libxlDriverUnlock(driver); @@ -2664,14 +2635,9 @@ libxlDomainEventDeregisterAny(virConnectPtr conn, int callbackID) int ret; libxlDriverLock(driver); - if (driver->domainEventDispatching) - ret = virDomainEventCallbackListMarkDeleteID(conn, - driver->domainEventCallbacks, - callbackID); - else - ret = virDomainEventCallbackListRemoveID(conn, - driver->domainEventCallbacks, - callbackID); + ret = virDomainEventStateDeregisterAny(conn, + driver->domainEventState, + callbackID); libxlDriverUnlock(driver); return ret; -- 1.7.4.4
On Thu, May 12, 2011 at 01:14:30PM -0400, Cole Robinson wrote:
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/libxl/libxl_conf.h | 6 +--- src/libxl/libxl_driver.c | 80 +++++++++++++-------------------------------- 2 files changed, 24 insertions(+), 62 deletions(-)
diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h index 8c87786..65110cf 100644 --- a/src/libxl/libxl_conf.h +++ b/src/libxl/libxl_conf.h @@ -61,11 +61,7 @@ struct _libxlDriverPrivate { virBitmapPtr reservedVNCPorts; virDomainObjList domains;
- /* A list of callbacks */ - virDomainEventCallbackListPtr domainEventCallbacks; - virDomainEventQueuePtr domainEventQueue; - int domainEventTimer; - int domainEventDispatching; + virDomainEventStatePtr domainEventState;
char *configDir; char *autostartDir; diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index d7ff0c6..a1c4509 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -130,28 +130,11 @@ static void libxlDomainEventFlush(int timer ATTRIBUTE_UNUSED, void *opaque) { libxlDriverPrivatePtr driver = opaque; - virDomainEventQueue tempQueue;
libxlDriverLock(driver); - - driver->domainEventDispatching = 1; - - /* Copy the queue, so we're reentrant safe */ - tempQueue.count = driver->domainEventQueue->count; - tempQueue.events = driver->domainEventQueue->events; - driver->domainEventQueue->count = 0; - driver->domainEventQueue->events = NULL; - - virEventUpdateTimeout(driver->domainEventTimer, -1); - virDomainEventQueueDispatch(&tempQueue, - driver->domainEventCallbacks, - libxlDomainEventDispatchFunc, - driver); - - /* Purge any deleted callbacks */ - virDomainEventCallbackListPurgeMarked(driver->domainEventCallbacks); - - driver->domainEventDispatching = 0; + virDomainEventStateFlush(driver->domainEventState, + libxmlDomainEventDispatchFunc, + driver); libxlDriverUnlock(driver); }
@@ -159,10 +142,7 @@ libxlDomainEventFlush(int timer ATTRIBUTE_UNUSED, void *opaque) static void libxlDomainEventQueue(libxlDriverPrivatePtr driver, virDomainEventPtr event) { - if (virDomainEventQueuePush(driver->domainEventQueue, event) < 0) - virDomainEventFree(event); - if (driver->domainEventQueue->count == 1) - virEventUpdateTimeout(driver->domainEventTimer, 0); + virDomainEventStateQueue(driver->domainEventState, event); }
/* @@ -706,12 +686,7 @@ libxlShutdown(void) VIR_FREE(libxl_driver->libDir); VIR_FREE(libxl_driver->saveDir);
- /* Free domain callback list */ - virDomainEventCallbackListFree(libxl_driver->domainEventCallbacks); - virDomainEventQueueFree(libxl_driver->domainEventQueue); - - if (libxl_driver->domainEventTimer != -1) - virEventRemoveTimeout(libxl_driver->domainEventTimer); + virDomainEventStateFree(privconn->domainEventState);
libxlDriverUnlock(libxl_driver); virMutexDestroy(&libxl_driver->lock); @@ -822,16 +797,14 @@ libxlStartup(int privileged) { } VIR_FREE(log_file);
- /* Init callback list */ - if (VIR_ALLOC(libxl_driver->domainEventCallbacks) < 0) - goto out_of_memory; - if (!(libxl_driver->domainEventQueue = virDomainEventQueueNew())) - goto out_of_memory; - if ((libxl_driver->domainEventTimer = - virEventAddTimeout(-1, libxlDomainEventFlush, libxl_driver, NULL)) < 0) + libxl_driver->domainEventState = virDomainEventStateNew( + libxlDomainEventFlush, + libxml_driver, + NULL, + false); + if (!libxml_driver->domainEventState) goto error;
- libxl_driver->logger = (xentoollog_logger *)xtl_createlogger_stdiostream(libxl_driver->logger_file, XTL_DEBUG, 0); if (!libxl_driver->logger) { @@ -982,7 +955,8 @@ libxlClose(virConnectPtr conn ATTRIBUTE_UNUSED) libxlDriverPrivatePtr driver = conn->privateData;
libxlDriverLock(driver); - virDomainEventCallbackListRemoveConn(conn, driver->domainEventCallbacks); + virDomainEventCallbackListRemoveConn(conn, + driver->domainEventState->callbacks); libxlDriverUnlock(driver); conn->privateData = NULL; return 0; @@ -2224,7 +2198,8 @@ libxlDomainEventRegister(virConnectPtr conn, int ret;
libxlDriverLock(driver); - ret = virDomainEventCallbackListAdd(conn, driver->domainEventCallbacks, + ret = virDomainEventCallbackListAdd(conn, + driver->domainEventState->callbacks, callback, opaque, freecb); libxlDriverUnlock(driver);
@@ -2240,14 +2215,9 @@ libxlDomainEventDeregister(virConnectPtr conn, int ret;
libxlDriverLock(driver); - if (driver->domainEventDispatching) - ret = virDomainEventCallbackListMarkDelete(conn, - driver->domainEventCallbacks, - callback); - else - ret = virDomainEventCallbackListRemove(conn, - driver->domainEventCallbacks, - callback); + ret = virDomainEventStateDeregister(conn, + driver->domainEventState, + callback); libxlDriverUnlock(driver);
return ret; @@ -2648,7 +2618,8 @@ libxlDomainEventRegisterAny(virConnectPtr conn, virDomainPtr dom, int eventID, int ret;
libxlDriverLock(driver); - ret = virDomainEventCallbackListAddID(conn, driver->domainEventCallbacks, + ret = virDomainEventCallbackListAddID(conn, + driver->domainEventState->callbacks, dom, eventID, callback, opaque, freecb); libxlDriverUnlock(driver); @@ -2664,14 +2635,9 @@ libxlDomainEventDeregisterAny(virConnectPtr conn, int callbackID) int ret;
libxlDriverLock(driver); - if (driver->domainEventDispatching) - ret = virDomainEventCallbackListMarkDeleteID(conn, - driver->domainEventCallbacks, - callbackID); - else - ret = virDomainEventCallbackListRemoveID(conn, - driver->domainEventCallbacks, - callbackID); + ret = virDomainEventStateDeregisterAny(conn, + driver->domainEventState, + callbackID); libxlDriverUnlock(driver);
return ret;
ACK Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 05/12/2011 11:14 AM, Cole Robinson wrote:
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/libxl/libxl_conf.h | 6 +--- src/libxl/libxl_driver.c | 80 +++++++++++++-------------------------------- 2 files changed, 24 insertions(+), 62 deletions(-)
The conversion wasn't complete: CC libvirt_driver_libxl_la-libxl_driver.lo libxl/libxl_driver.c: In function 'libxlDomainEventFlush': libxl/libxl_driver.c:136:30: error: 'libxmlDomainEventDispatchFunc' undeclared (first use in this function) libxl/libxl_driver.c:136:30: note: each undeclared identifier is reported only once for each function it appears in libxl/libxl_driver.c: In function 'libxlShutdown': libxl/libxl_driver.c:689:29: error: 'privconn' undeclared (first use in this function) libxl/libxl_driver.c: In function 'libxlStartup': libxl/libxl_driver.c:802:57: error: 'libxml_driver' undeclared (first use in this function) cc1: warnings being treated as errors libxl/libxl_driver.c: At top level: libxl/libxl_driver.c:117:1: error: 'libxlDomainEventDispatchFunc' defined but not used [-Wunused-function] -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On 05/13/2011 01:18 PM, Eric Blake wrote:
On 05/12/2011 11:14 AM, Cole Robinson wrote:
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/libxl/libxl_conf.h | 6 +--- src/libxl/libxl_driver.c | 80 +++++++++++++-------------------------------- 2 files changed, 24 insertions(+), 62 deletions(-)
The conversion wasn't complete:
CC libvirt_driver_libxl_la-libxl_driver.lo libxl/libxl_driver.c: In function 'libxlDomainEventFlush': libxl/libxl_driver.c:136:30: error: 'libxmlDomainEventDispatchFunc' undeclared (first use in this function) libxl/libxl_driver.c:136:30: note: each undeclared identifier is reported only once for each function it appears in libxl/libxl_driver.c: In function 'libxlShutdown': libxl/libxl_driver.c:689:29: error: 'privconn' undeclared (first use in this function) libxl/libxl_driver.c: In function 'libxlStartup': libxl/libxl_driver.c:802:57: error: 'libxml_driver' undeclared (first use in this function) cc1: warnings being treated as errors libxl/libxl_driver.c: At top level: libxl/libxl_driver.c:117:1: error: 'libxlDomainEventDispatchFunc' defined but not used [-Wunused-function]
Urgh, yeah, thought I was actually building the driver but clearly was not. Attached patch should fix the breakage, can you test if it works? Thanks, Cole
One functionality change here is that we no longer force enable the event timeout for every queued event, only enable it for the first event after the queue has been flushed. This is how other drivers have already done it, and I haven't encountered problems in practice. v3: Adjust for new virDomainEventStateNew argument Signed-off-by: Cole Robinson <crobinso@redhat.com> --- .gnulib | 2 +- src/remote/remote_driver.c | 163 +++++++++++++++++--------------------------- 2 files changed, 64 insertions(+), 101 deletions(-) diff --git a/.gnulib b/.gnulib index 64a5e38..a6676cc 160000 --- a/.gnulib +++ b/.gnulib @@ -1 +1 @@ -Subproject commit 64a5e38bced6c8f5117efbed95cdfd8ca133ed54 +Subproject commit a6676cca6498ce67c5a3c8d7221b8d6c30b61dc4 diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index c50ff25..1d64a63 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -184,15 +184,7 @@ struct private_data { unsigned int bufferLength; unsigned int bufferOffset; - /* The list of domain event callbacks */ - virDomainEventCallbackListPtr callbackList; - /* The queue of domain events generated - during a call / response rpc */ - virDomainEventQueuePtr domainEvents; - /* Timer for flushing domainEvents queue */ - int eventFlushTimer; - /* Flag if we're in process of dispatching */ - int domainEventDispatching; + virDomainEventStatePtr domainEventState; /* Self-pipe to wakeup threads waiting in poll() */ int wakeupSendFD; @@ -264,6 +256,7 @@ static void make_nonnull_nwfilter (remote_nonnull_nwfilter *nwfilter_dst, virNWF static void make_nonnull_domain_snapshot (remote_nonnull_domain_snapshot *snapshot_dst, virDomainSnapshotPtr snapshot_src); void remoteDomainEventFired(int watch, int fd, int event, void *data); void remoteDomainEventQueueFlush(int timer, void *opaque); +void remoteDomainEventQueue(struct private_data *priv, virDomainEventPtr event); /*----------------------------------------------------------------------*/ /* Helper functions for remoteOpen. */ @@ -913,17 +906,6 @@ doRemoteOpen (virConnectPtr conn, } } - if(VIR_ALLOC(priv->callbackList)<0) { - virReportOOMError(); - goto failed; - } - - if(VIR_ALLOC(priv->domainEvents)<0) { - virReportOOMError(); - goto failed; - } - - VIR_DEBUG("Adding Handler for remote events"); /* Set up a callback to listen on the socket data */ if ((priv->watch = virEventAddHandle(priv->sock, VIR_EVENT_HANDLE_READABLE, @@ -931,18 +913,21 @@ doRemoteOpen (virConnectPtr conn, conn, NULL)) < 0) { VIR_DEBUG("virEventAddHandle failed: No addHandleImpl defined." " continuing without events."); - } else { + priv->watch = -1; + } - VIR_DEBUG("Adding Timeout for remote event queue flushing"); - if ( (priv->eventFlushTimer = virEventAddTimeout(-1, - remoteDomainEventQueueFlush, - conn, NULL)) < 0) { - VIR_DEBUG("virEventAddTimeout failed: No addTimeoutImpl defined. " - "continuing without events."); - virEventRemoveHandle(priv->watch); - priv->watch = -1; - } + priv->domainEventState = virDomainEventStateNew(remoteDomainEventQueueFlush, + conn, + NULL, + false); + if (!priv->domainEventState) { + goto failed; + } + if (priv->domainEventState->timer < 0 && priv->watch != -1) { + virEventRemoveHandle(priv->watch); + priv->watch = -1; } + /* Successful. */ retcode = VIR_DRV_OPEN_SUCCESS; @@ -1559,12 +1544,13 @@ verify_certificate (virConnectPtr conn ATTRIBUTE_UNUSED, static int doRemoteClose (virConnectPtr conn, struct private_data *priv) { - if (priv->eventFlushTimer >= 0) { - /* Remove timeout */ - virEventRemoveTimeout(priv->eventFlushTimer); - /* Remove handle for remote events */ + /* Remove timer before closing the connection, to avoid possible + * remoteDomainEventFired with a free'd connection */ + if (priv->domainEventState->timer >= 0) { + virEventRemoveTimeout(priv->domainEventState->timer); virEventRemoveHandle(priv->watch); priv->watch = -1; + priv->domainEventState->timer = -1; } if (call (conn, priv, 0, REMOTE_PROC_CLOSE, @@ -1605,11 +1591,7 @@ retry: /* See comment for remoteType. */ VIR_FREE(priv->type); - /* Free callback list */ - virDomainEventCallbackListFree(priv->callbackList); - - /* Free queued events */ - virDomainEventQueueFree(priv->domainEvents); + virDomainEventStateFree(priv->domainEventState); return 0; } @@ -3800,17 +3782,20 @@ static int remoteDomainEventRegister(virConnectPtr conn, remoteDriverLock(priv); - if (priv->eventFlushTimer < 0) { + if (priv->domainEventState->timer < 0) { remoteError(VIR_ERR_NO_SUPPORT, "%s", _("no event support")); goto done; } - if (virDomainEventCallbackListAdd(conn, priv->callbackList, + + if (virDomainEventCallbackListAdd(conn, priv->domainEventState->callbacks, callback, opaque, freecb) < 0) { remoteError(VIR_ERR_RPC, "%s", _("adding cb to list")); goto done; } - if (virDomainEventCallbackListCountID(conn, priv->callbackList, VIR_DOMAIN_EVENT_ID_LIFECYCLE) == 1) { + if (virDomainEventCallbackListCountID(conn, + priv->domainEventState->callbacks, + VIR_DOMAIN_EVENT_ID_LIFECYCLE) == 1) { /* Tell the server when we are the first callback deregistering */ if (call (conn, priv, 0, REMOTE_PROC_DOMAIN_EVENTS_REGISTER, (xdrproc_t) xdr_void, (char *) NULL, @@ -3833,21 +3818,14 @@ static int remoteDomainEventDeregister(virConnectPtr conn, remoteDriverLock(priv); - if (priv->domainEventDispatching) { - if (virDomainEventCallbackListMarkDelete(conn, priv->callbackList, - callback) < 0) { - remoteError(VIR_ERR_RPC, "%s", _("marking cb for deletion")); - goto done; - } - } else { - if (virDomainEventCallbackListRemove(conn, priv->callbackList, - callback) < 0) { - remoteError(VIR_ERR_RPC, "%s", _("removing cb from list")); - goto done; - } - } + if (virDomainEventStateDeregister(conn, + priv->domainEventState, + callback) < 0) + goto done; - if (virDomainEventCallbackListCountID(conn, priv->callbackList, VIR_DOMAIN_EVENT_ID_LIFECYCLE) == 0) { + if (virDomainEventCallbackListCountID(conn, + priv->domainEventState->callbacks, + VIR_DOMAIN_EVENT_ID_LIFECYCLE) == 0) { /* Tell the server when we are the last callback deregistering */ if (call (conn, priv, 0, REMOTE_PROC_DOMAIN_EVENTS_DEREGISTER, (xdrproc_t) xdr_void, (char *) NULL, @@ -4723,12 +4701,13 @@ static int remoteDomainEventRegisterAny(virConnectPtr conn, remoteDriverLock(priv); - if (priv->eventFlushTimer < 0) { + if (priv->domainEventState->timer < 0) { remoteError(VIR_ERR_NO_SUPPORT, "%s", _("no event support")); goto done; } - if ((callbackID = virDomainEventCallbackListAddID(conn, priv->callbackList, + if ((callbackID = virDomainEventCallbackListAddID(conn, + priv->domainEventState->callbacks, dom, eventID, callback, opaque, freecb)) < 0) { remoteError(VIR_ERR_RPC, "%s", _("adding cb to list")); @@ -4737,13 +4716,17 @@ static int remoteDomainEventRegisterAny(virConnectPtr conn, /* If this is the first callback for this eventID, we need to enable * events on the server */ - if (virDomainEventCallbackListCountID(conn, priv->callbackList, eventID) == 1) { + if (virDomainEventCallbackListCountID(conn, + priv->domainEventState->callbacks, + eventID) == 1) { args.eventID = eventID; if (call (conn, priv, 0, REMOTE_PROC_DOMAIN_EVENTS_REGISTER_ANY, (xdrproc_t) xdr_remote_domain_events_register_any_args, (char *) &args, (xdrproc_t) xdr_void, (char *)NULL) == -1) { - virDomainEventCallbackListRemoveID(conn, priv->callbackList, callbackID); + virDomainEventCallbackListRemoveID(conn, + priv->domainEventState->callbacks, + callbackID); goto done; } } @@ -4766,28 +4749,23 @@ static int remoteDomainEventDeregisterAny(virConnectPtr conn, remoteDriverLock(priv); - if ((eventID = virDomainEventCallbackListEventID(conn, priv->callbackList, callbackID)) < 0) { + if ((eventID = virDomainEventCallbackListEventID(conn, + priv->domainEventState->callbacks, + callbackID)) < 0) { remoteError(VIR_ERR_RPC, _("unable to find callback ID %d"), callbackID); goto done; } - if (priv->domainEventDispatching) { - if (virDomainEventCallbackListMarkDeleteID(conn, priv->callbackList, - callbackID) < 0) { - remoteError(VIR_ERR_RPC, "%s", _("marking cb for deletion")); - goto done; - } - } else { - if (virDomainEventCallbackListRemoveID(conn, priv->callbackList, - callbackID) < 0) { - remoteError(VIR_ERR_RPC, "%s", _("removing cb from list")); - goto done; - } - } + if (virDomainEventStateDeregisterAny(conn, + priv->domainEventState, + callbackID) < 0) + goto done; /* If that was the last callback for this eventID, we need to disable * events on the server */ - if (virDomainEventCallbackListCountID(conn, priv->callbackList, eventID) == 0) { + if (virDomainEventCallbackListCountID(conn, + priv->domainEventState->callbacks, + eventID) == 0) { args.eventID = eventID; if (call (conn, priv, 0, REMOTE_PROC_DOMAIN_EVENTS_DEREGISTER_ANY, @@ -5553,13 +5531,7 @@ processCallDispatchMessage(virConnectPtr conn, struct private_data *priv, if (!event) return -1; - if (virDomainEventQueuePush(priv->domainEvents, - event) < 0) { - VIR_DEBUG("Error adding event to queue"); - virDomainEventFree(event); - } - virEventUpdateTimeout(priv->eventFlushTimer, 0); - + remoteDomainEventQueue(priv, event); return 0; } @@ -6230,31 +6202,22 @@ remoteDomainEventQueueFlush(int timer ATTRIBUTE_UNUSED, void *opaque) { virConnectPtr conn = opaque; struct private_data *priv = conn->privateData; - virDomainEventQueue tempQueue; - remoteDriverLock(priv); + remoteDriverLock(priv); VIR_DEBUG("Event queue flush %p", conn); - priv->domainEventDispatching = 1; - - /* Copy the queue, so we're reentrant safe */ - tempQueue.count = priv->domainEvents->count; - tempQueue.events = priv->domainEvents->events; - priv->domainEvents->count = 0; - priv->domainEvents->events = NULL; - - virEventUpdateTimeout(priv->eventFlushTimer, -1); - virDomainEventQueueDispatch(&tempQueue, priv->callbackList, - remoteDomainEventDispatchFunc, priv); - - /* Purge any deleted callbacks */ - virDomainEventCallbackListPurgeMarked(priv->callbackList); - - priv->domainEventDispatching = 0; + virDomainEventStateFlush(priv->domainEventState, + remoteDomainEventDispatchFunc, + priv); remoteDriverUnlock(priv); } +void +remoteDomainEventQueue(struct private_data *priv, virDomainEventPtr event) +{ + virDomainEventStateQueue(priv->domainEventState, event); +} /* get_nonnull_domain and get_nonnull_network turn an on-wire * (name, uuid) pair into virDomainPtr or virNetworkPtr object. -- 1.7.4.4
On Thu, May 12, 2011 at 01:14:31PM -0400, Cole Robinson wrote:
One functionality change here is that we no longer force enable the event timeout for every queued event, only enable it for the first event after the queue has been flushed. This is how other drivers have already done it, and I haven't encountered problems in practice.
v3: Adjust for new virDomainEventStateNew argument
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- .gnulib | 2 +- src/remote/remote_driver.c | 163 +++++++++++++++++--------------------------- 2 files changed, 64 insertions(+), 101 deletions(-)
diff --git a/.gnulib b/.gnulib index 64a5e38..a6676cc 160000 --- a/.gnulib +++ b/.gnulib @@ -1 +1 @@ -Subproject commit 64a5e38bced6c8f5117efbed95cdfd8ca133ed54 +Subproject commit a6676cca6498ce67c5a3c8d7221b8d6c30b61dc4 diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index c50ff25..1d64a63 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -184,15 +184,7 @@ struct private_data { unsigned int bufferLength; unsigned int bufferOffset;
- /* The list of domain event callbacks */ - virDomainEventCallbackListPtr callbackList; - /* The queue of domain events generated - during a call / response rpc */ - virDomainEventQueuePtr domainEvents; - /* Timer for flushing domainEvents queue */ - int eventFlushTimer; - /* Flag if we're in process of dispatching */ - int domainEventDispatching; + virDomainEventStatePtr domainEventState;
/* Self-pipe to wakeup threads waiting in poll() */ int wakeupSendFD; @@ -264,6 +256,7 @@ static void make_nonnull_nwfilter (remote_nonnull_nwfilter *nwfilter_dst, virNWF static void make_nonnull_domain_snapshot (remote_nonnull_domain_snapshot *snapshot_dst, virDomainSnapshotPtr snapshot_src); void remoteDomainEventFired(int watch, int fd, int event, void *data); void remoteDomainEventQueueFlush(int timer, void *opaque); +void remoteDomainEventQueue(struct private_data *priv, virDomainEventPtr event); /*----------------------------------------------------------------------*/
/* Helper functions for remoteOpen. */ @@ -913,17 +906,6 @@ doRemoteOpen (virConnectPtr conn, } }
- if(VIR_ALLOC(priv->callbackList)<0) { - virReportOOMError(); - goto failed; - } - - if(VIR_ALLOC(priv->domainEvents)<0) { - virReportOOMError(); - goto failed; - } - - VIR_DEBUG("Adding Handler for remote events"); /* Set up a callback to listen on the socket data */ if ((priv->watch = virEventAddHandle(priv->sock, VIR_EVENT_HANDLE_READABLE, @@ -931,18 +913,21 @@ doRemoteOpen (virConnectPtr conn, conn, NULL)) < 0) { VIR_DEBUG("virEventAddHandle failed: No addHandleImpl defined." " continuing without events."); - } else { + priv->watch = -1; + }
- VIR_DEBUG("Adding Timeout for remote event queue flushing"); - if ( (priv->eventFlushTimer = virEventAddTimeout(-1, - remoteDomainEventQueueFlush, - conn, NULL)) < 0) { - VIR_DEBUG("virEventAddTimeout failed: No addTimeoutImpl defined. " - "continuing without events."); - virEventRemoveHandle(priv->watch); - priv->watch = -1; - } + priv->domainEventState = virDomainEventStateNew(remoteDomainEventQueueFlush, + conn, + NULL, + false); + if (!priv->domainEventState) { + goto failed; + } + if (priv->domainEventState->timer < 0 && priv->watch != -1) { + virEventRemoveHandle(priv->watch); + priv->watch = -1; } + /* Successful. */ retcode = VIR_DRV_OPEN_SUCCESS;
@@ -1559,12 +1544,13 @@ verify_certificate (virConnectPtr conn ATTRIBUTE_UNUSED, static int doRemoteClose (virConnectPtr conn, struct private_data *priv) { - if (priv->eventFlushTimer >= 0) { - /* Remove timeout */ - virEventRemoveTimeout(priv->eventFlushTimer); - /* Remove handle for remote events */ + /* Remove timer before closing the connection, to avoid possible + * remoteDomainEventFired with a free'd connection */ + if (priv->domainEventState->timer >= 0) { + virEventRemoveTimeout(priv->domainEventState->timer); virEventRemoveHandle(priv->watch); priv->watch = -1; + priv->domainEventState->timer = -1; }
if (call (conn, priv, 0, REMOTE_PROC_CLOSE, @@ -1605,11 +1591,7 @@ retry: /* See comment for remoteType. */ VIR_FREE(priv->type);
- /* Free callback list */ - virDomainEventCallbackListFree(priv->callbackList); - - /* Free queued events */ - virDomainEventQueueFree(priv->domainEvents); + virDomainEventStateFree(priv->domainEventState);
return 0; } @@ -3800,17 +3782,20 @@ static int remoteDomainEventRegister(virConnectPtr conn,
remoteDriverLock(priv);
- if (priv->eventFlushTimer < 0) { + if (priv->domainEventState->timer < 0) { remoteError(VIR_ERR_NO_SUPPORT, "%s", _("no event support")); goto done; } - if (virDomainEventCallbackListAdd(conn, priv->callbackList, + + if (virDomainEventCallbackListAdd(conn, priv->domainEventState->callbacks, callback, opaque, freecb) < 0) { remoteError(VIR_ERR_RPC, "%s", _("adding cb to list")); goto done; }
- if (virDomainEventCallbackListCountID(conn, priv->callbackList, VIR_DOMAIN_EVENT_ID_LIFECYCLE) == 1) { + if (virDomainEventCallbackListCountID(conn, + priv->domainEventState->callbacks, + VIR_DOMAIN_EVENT_ID_LIFECYCLE) == 1) { /* Tell the server when we are the first callback deregistering */ if (call (conn, priv, 0, REMOTE_PROC_DOMAIN_EVENTS_REGISTER, (xdrproc_t) xdr_void, (char *) NULL, @@ -3833,21 +3818,14 @@ static int remoteDomainEventDeregister(virConnectPtr conn,
remoteDriverLock(priv);
- if (priv->domainEventDispatching) { - if (virDomainEventCallbackListMarkDelete(conn, priv->callbackList, - callback) < 0) { - remoteError(VIR_ERR_RPC, "%s", _("marking cb for deletion")); - goto done; - } - } else { - if (virDomainEventCallbackListRemove(conn, priv->callbackList, - callback) < 0) { - remoteError(VIR_ERR_RPC, "%s", _("removing cb from list")); - goto done; - } - } + if (virDomainEventStateDeregister(conn, + priv->domainEventState, + callback) < 0) + goto done;
- if (virDomainEventCallbackListCountID(conn, priv->callbackList, VIR_DOMAIN_EVENT_ID_LIFECYCLE) == 0) { + if (virDomainEventCallbackListCountID(conn, + priv->domainEventState->callbacks, + VIR_DOMAIN_EVENT_ID_LIFECYCLE) == 0) { /* Tell the server when we are the last callback deregistering */ if (call (conn, priv, 0, REMOTE_PROC_DOMAIN_EVENTS_DEREGISTER, (xdrproc_t) xdr_void, (char *) NULL, @@ -4723,12 +4701,13 @@ static int remoteDomainEventRegisterAny(virConnectPtr conn,
remoteDriverLock(priv);
- if (priv->eventFlushTimer < 0) { + if (priv->domainEventState->timer < 0) { remoteError(VIR_ERR_NO_SUPPORT, "%s", _("no event support")); goto done; }
- if ((callbackID = virDomainEventCallbackListAddID(conn, priv->callbackList, + if ((callbackID = virDomainEventCallbackListAddID(conn, + priv->domainEventState->callbacks, dom, eventID, callback, opaque, freecb)) < 0) { remoteError(VIR_ERR_RPC, "%s", _("adding cb to list")); @@ -4737,13 +4716,17 @@ static int remoteDomainEventRegisterAny(virConnectPtr conn,
/* If this is the first callback for this eventID, we need to enable * events on the server */ - if (virDomainEventCallbackListCountID(conn, priv->callbackList, eventID) == 1) { + if (virDomainEventCallbackListCountID(conn, + priv->domainEventState->callbacks, + eventID) == 1) { args.eventID = eventID;
if (call (conn, priv, 0, REMOTE_PROC_DOMAIN_EVENTS_REGISTER_ANY, (xdrproc_t) xdr_remote_domain_events_register_any_args, (char *) &args, (xdrproc_t) xdr_void, (char *)NULL) == -1) { - virDomainEventCallbackListRemoveID(conn, priv->callbackList, callbackID); + virDomainEventCallbackListRemoveID(conn, + priv->domainEventState->callbacks, + callbackID); goto done; } } @@ -4766,28 +4749,23 @@ static int remoteDomainEventDeregisterAny(virConnectPtr conn,
remoteDriverLock(priv);
- if ((eventID = virDomainEventCallbackListEventID(conn, priv->callbackList, callbackID)) < 0) { + if ((eventID = virDomainEventCallbackListEventID(conn, + priv->domainEventState->callbacks, + callbackID)) < 0) { remoteError(VIR_ERR_RPC, _("unable to find callback ID %d"), callbackID); goto done; }
- if (priv->domainEventDispatching) { - if (virDomainEventCallbackListMarkDeleteID(conn, priv->callbackList, - callbackID) < 0) { - remoteError(VIR_ERR_RPC, "%s", _("marking cb for deletion")); - goto done; - } - } else { - if (virDomainEventCallbackListRemoveID(conn, priv->callbackList, - callbackID) < 0) { - remoteError(VIR_ERR_RPC, "%s", _("removing cb from list")); - goto done; - } - } + if (virDomainEventStateDeregisterAny(conn, + priv->domainEventState, + callbackID) < 0) + goto done;
/* If that was the last callback for this eventID, we need to disable * events on the server */ - if (virDomainEventCallbackListCountID(conn, priv->callbackList, eventID) == 0) { + if (virDomainEventCallbackListCountID(conn, + priv->domainEventState->callbacks, + eventID) == 0) { args.eventID = eventID;
if (call (conn, priv, 0, REMOTE_PROC_DOMAIN_EVENTS_DEREGISTER_ANY, @@ -5553,13 +5531,7 @@ processCallDispatchMessage(virConnectPtr conn, struct private_data *priv, if (!event) return -1;
- if (virDomainEventQueuePush(priv->domainEvents, - event) < 0) { - VIR_DEBUG("Error adding event to queue"); - virDomainEventFree(event); - } - virEventUpdateTimeout(priv->eventFlushTimer, 0); - + remoteDomainEventQueue(priv, event); return 0; }
@@ -6230,31 +6202,22 @@ remoteDomainEventQueueFlush(int timer ATTRIBUTE_UNUSED, void *opaque) { virConnectPtr conn = opaque; struct private_data *priv = conn->privateData; - virDomainEventQueue tempQueue;
- remoteDriverLock(priv);
+ remoteDriverLock(priv); VIR_DEBUG("Event queue flush %p", conn); - priv->domainEventDispatching = 1; - - /* Copy the queue, so we're reentrant safe */ - tempQueue.count = priv->domainEvents->count; - tempQueue.events = priv->domainEvents->events; - priv->domainEvents->count = 0; - priv->domainEvents->events = NULL; - - virEventUpdateTimeout(priv->eventFlushTimer, -1); - virDomainEventQueueDispatch(&tempQueue, priv->callbackList, - remoteDomainEventDispatchFunc, priv); - - /* Purge any deleted callbacks */ - virDomainEventCallbackListPurgeMarked(priv->callbackList); - - priv->domainEventDispatching = 0;
+ virDomainEventStateFlush(priv->domainEventState, + remoteDomainEventDispatchFunc, + priv); remoteDriverUnlock(priv); }
+void +remoteDomainEventQueue(struct private_data *priv, virDomainEventPtr event) +{ + virDomainEventStateQueue(priv->domainEventState, event); +}
/* get_nonnull_domain and get_nonnull_network turn an on-wire * (name, uuid) pair into virDomainPtr or virNetworkPtr object.
ACK Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 05/13/2011 05:26 AM, Daniel P. Berrange wrote:
On Thu, May 12, 2011 at 01:14:31PM -0400, Cole Robinson wrote:
One functionality change here is that we no longer force enable the event timeout for every queued event, only enable it for the first event after the queue has been flushed. This is how other drivers have already done it, and I haven't encountered problems in practice.
v3: Adjust for new virDomainEventStateNew argument
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- .gnulib | 2 +-
Whoops, this was unintentional, dropped before pushing.
ACK
Thanks, pushed the series. - Cole
participants (3)
-
Cole Robinson -
Daniel P. Berrange -
Eric Blake