[libvirt] [PATCH 0/6 v2] 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. 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 Cole Robinson (6): 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 remote: Use virDomainEventState helpers cfg.mk | 1 + src/conf/domain_event.c | 166 ++++++++++++++++++++++++++++++++++++++----- src/conf/domain_event.h | 69 +++++++++++++++---- src/libvirt_private.syms | 6 ++ src/lxc/lxc_conf.h | 6 +-- src/lxc/lxc_driver.c | 76 ++++++-------------- src/qemu/qemu_conf.h | 6 +-- src/qemu/qemu_driver.c | 80 +++++++-------------- src/remote/remote_driver.c | 162 ++++++++++++++++-------------------------- src/test/test_driver.c | 104 ++++++++------------------- 10 files changed, 353 insertions(+), 323 deletions(-) -- 1.7.3.3
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 03186b3..3cea22b 100644 --- a/cfg.mk +++ b/cfg.mk @@ -92,6 +92,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 5f086bd..c80b891 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, @@ -776,26 +850,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 e28293d..a465536 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); @@ -163,6 +177,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 e9b8cb7..5bfbb92 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -349,6 +349,8 @@ virDomainEventRTCChangeNewFromDom; virDomainEventRTCChangeNewFromObj; virDomainEventRebootNewFromDom; virDomainEventRebootNewFromObj; +virDomainEventStateFree; +virDomainEventStateNew; virDomainEventWatchdogNewFromDom; virDomainEventWatchdogNewFromObj; -- 1.7.3.3
On Mon, Jan 10, 2011 at 03:28:55PM -0500, 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(-)
ACK Daniel
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 Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/conf/domain_event.c | 74 +++++++++++++++++++++++++++++++++++++++++++++- src/conf/domain_event.h | 21 +++++++++++++ src/libvirt_private.syms | 4 ++ 3 files changed, 98 insertions(+), 1 deletions(-) diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c index c80b891..90153df 100644 --- a/src/conf/domain_event.c +++ b/src/conf/domain_event.c @@ -582,7 +582,8 @@ virDomainEventStateNew(virEventTimeoutCallback timeout_cb, timeout_cb, timeout_opaque, timeout_free)) < 0) { - goto error; + DEBUG0("virEventAddTimeout failed: No addTimeoutImpl defined. " + "continuing without events."); } return state; @@ -1051,3 +1052,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) { + DEBUG0("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 a465536..ad7e825 100644 --- a/src/conf/domain_event.h +++ b/src/conf/domain_event.h @@ -204,4 +204,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 5bfbb92..699d602 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -349,8 +349,12 @@ virDomainEventRTCChangeNewFromDom; virDomainEventRTCChangeNewFromObj; virDomainEventRebootNewFromDom; virDomainEventRebootNewFromObj; +virDomainEventStateDeregister; +virDomainEventStateDeregisterAny; +virDomainEventStateFlush; virDomainEventStateFree; virDomainEventStateNew; +virDomainEventStateQueue; virDomainEventWatchdogNewFromDom; virDomainEventWatchdogNewFromObj; -- 1.7.3.3
On Mon, Jan 10, 2011 at 03:28:56PM -0500, 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
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/conf/domain_event.c | 74 +++++++++++++++++++++++++++++++++++++++++++++- src/conf/domain_event.h | 21 +++++++++++++ src/libvirt_private.syms | 4 ++ 3 files changed, 98 insertions(+), 1 deletions(-)
ACK Daniel
v2: Drop libvirt_private.syms changes Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/qemu/qemu_conf.h | 6 +--- src/qemu/qemu_driver.c | 80 +++++++++++++++-------------------------------- 2 files changed, 27 insertions(+), 59 deletions(-) diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 5a5748b..2d878b5 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -107,11 +107,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_driver.c b/src/qemu/qemu_driver.c index 9eb9cd5..5acf1d9 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1185,15 +1185,17 @@ 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); + if (!qemu_driver->domainEventState) goto error; + if (!qemu_driver->domainEventState->timer < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("could not initialize domain event timer")); + goto error; + } /* Allocate bitmap for vnc port reservation */ if ((qemu_driver->reservedVNCPorts = @@ -1555,12 +1557,7 @@ qemudShutdown(void) { VIR_FREE(qemu_driver->cgroupDeviceACL); } - /* 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); @@ -3181,7 +3178,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; @@ -7778,7 +7776,8 @@ qemuDomainEventRegister(virConnectPtr conn, int ret; qemuDriverLock(driver); - ret = virDomainEventCallbackListAdd(conn, driver->domainEventCallbacks, + ret = virDomainEventCallbackListAdd(conn, + driver->domainEventState->callbacks, callback, opaque, freecb); qemuDriverUnlock(driver); @@ -7794,12 +7793,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; @@ -7819,7 +7815,7 @@ qemuDomainEventRegisterAny(virConnectPtr conn, qemuDriverLock(driver); ret = virDomainEventCallbackListAddID(conn, - driver->domainEventCallbacks, + driver->domainEventState->callbacks, dom, eventID, callback, opaque, freecb); qemuDriverUnlock(driver); @@ -7836,12 +7832,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; @@ -7865,28 +7858,11 @@ static void qemuDomainEventDispatchFunc(virConnectPtr conn, static 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); } @@ -7895,11 +7871,7 @@ static void qemuDomainEventFlush(int timer ATTRIBUTE_UNUSED, void *opaque) static void qemuDomainEventQueue(struct qemud_driver *driver, virDomainEventPtr event) { - if (virDomainEventQueuePush(driver->domainEventQueue, - event) < 0) - virDomainEventFree(event); - if (qemu_driver->domainEventQueue->count == 1) - virEventUpdateTimeout(driver->domainEventTimer, 0); + virDomainEventStateQueue(driver->domainEventState, event); } /* Migration support. */ -- 1.7.3.3
On Mon, Jan 10, 2011 at 03:28:57PM -0500, Cole Robinson wrote:
v2: Drop libvirt_private.syms changes
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/qemu/qemu_conf.h | 6 +--- src/qemu/qemu_driver.c | 80 +++++++++++++++-------------------------------- 2 files changed, 27 insertions(+), 59 deletions(-)
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 5a5748b..2d878b5 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -107,11 +107,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_driver.c b/src/qemu/qemu_driver.c index 9eb9cd5..5acf1d9 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1185,15 +1185,17 @@ 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); + if (!qemu_driver->domainEventState) goto error; + if (!qemu_driver->domainEventState->timer < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("could not initialize domain event timer")); + goto error; + }
I'm curious about this ->timer < 0 check here. I'd kind of expect the 'domainEventState' struct to be opaque, and only accessible to the helper APIs you added as virDomainEventStateXXX. Could the virDomainEventStateNew() function simply return NULL if it was unable to initilize the timer, or is there a need for this to be treated as a non-fatal scenario ? If it needs to be treated as non-fatal, could we pass in a bool flag to virDomainEventStateNew() 'bool requireTimer' so that it can enforce this itself rather than making the callers check Daniel
On 01/13/2011 08:16 AM, Daniel P. Berrange wrote:
On Mon, Jan 10, 2011 at 03:28:57PM -0500, Cole Robinson wrote:
v2: Drop libvirt_private.syms changes
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/qemu/qemu_conf.h | 6 +--- src/qemu/qemu_driver.c | 80 +++++++++++++++-------------------------------- 2 files changed, 27 insertions(+), 59 deletions(-)
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 5a5748b..2d878b5 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -107,11 +107,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_driver.c b/src/qemu/qemu_driver.c index 9eb9cd5..5acf1d9 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1185,15 +1185,17 @@ 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); + if (!qemu_driver->domainEventState) goto error; + if (!qemu_driver->domainEventState->timer < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("could not initialize domain event timer")); + goto error; + }
I'm curious about this ->timer < 0 check here. I'd kind of expect the 'domainEventState' struct to be opaque, and only accessible to the helper APIs you added as virDomainEventStateXXX. Could the virDomainEventStateNew() function simply return NULL if it was unable to initilize the timer, or is there a need for this to be treated as a non-fatal scenario ?
For QEMU it is fatal, since the remote driver should always be registering event handlers. But for the 'test' driver that isn't true.
If it needs to be treated as non-fatal, could we pass in a bool flag to virDomainEventStateNew() 'bool requireTimer' so that it can enforce this itself rather than making the callers check
Sounds good. Thanks, Cole
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/lxc/lxc_conf.h | 6 +--- src/lxc/lxc_driver.c | 76 ++++++++++++++++---------------------------------- 2 files changed, 25 insertions(+), 57 deletions(-) diff --git a/src/lxc/lxc_conf.h b/src/lxc/lxc_conf.h index f820d6d..308a3dd 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 eb58086..8717d08 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -158,7 +158,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; @@ -1772,7 +1773,8 @@ lxcDomainEventRegister(virConnectPtr conn, int ret; lxcDriverLock(driver); - ret = virDomainEventCallbackListAdd(conn, driver->domainEventCallbacks, + ret = virDomainEventCallbackListAdd(conn, + driver->domainEventState->callbacks, callback, opaque, freecb); lxcDriverUnlock(driver); @@ -1788,12 +1790,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; @@ -1813,7 +1812,7 @@ lxcDomainEventRegisterAny(virConnectPtr conn, lxcDriverLock(driver); ret = virDomainEventCallbackListAddID(conn, - driver->domainEventCallbacks, + driver->domainEventState->callbacks, dom, eventID, callback, opaque, freecb); lxcDriverUnlock(driver); @@ -1830,12 +1829,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; @@ -1860,28 +1856,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); } @@ -1890,11 +1869,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); } /** @@ -2109,14 +2084,16 @@ static int lxcStartup(int privileged) if (virDomainObjListInit(&lxc_driver->domains) < 0) goto cleanup; - if (VIR_ALLOC(lxc_driver->domainEventCallbacks) < 0) + lxc_driver->domainEventState = virDomainEventStateNew(lxcDomainEventFlush, + lxc_driver, + NULL); + if (!lxc_driver->domainEventState) goto cleanup; - if (!(lxc_driver->domainEventQueue = virDomainEventQueueNew())) - goto cleanup; - - if ((lxc_driver->domainEventTimer = - virEventAddTimeout(-1, lxcDomainEventFlush, lxc_driver, NULL)) < 0) + if (!lxc_driver->domainEventState->timer < 0) { + lxcError(VIR_ERR_INTERNAL_ERROR, "%s", + _("could not initialize domain event timer")); goto cleanup; + } lxc_driver->log_libvirtd = 0; /* by default log to container logfile */ lxc_driver->have_netns = lxcCheckNetNsSupport(); @@ -2204,12 +2181,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.3.3
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/test/test_driver.c | 104 ++++++++++++++--------------------------------- 1 files changed, 31 insertions(+), 73 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index ddff160..87350e8 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,24 @@ 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) - DEBUG0("virEventAddTimeout failed: No addTimeoutImpl defined. " - "continuing without events."); + privconn->domainEventState = virDomainEventStateNew(testDomainEventFlush, + privconn, + NULL); + 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 +1176,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 +5166,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 +5183,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 +5204,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 +5221,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 +5247,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 +5260,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.3.3
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 it haven't encountered problems in practice. Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/remote/remote_driver.c | 162 +++++++++++++++++--------------------------- 1 files changed, 62 insertions(+), 100 deletions(-) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index ea119c6..56f99ff 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -182,15 +182,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; @@ -262,6 +254,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. */ @@ -897,17 +890,8 @@ doRemoteOpen (virConnectPtr conn, } } - if(VIR_ALLOC(priv->callbackList)<0) { - virReportOOMError(); - goto failed; - } - - if(VIR_ALLOC(priv->domainEvents)<0) { - virReportOOMError(); - goto failed; - } - DEBUG0("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, @@ -915,18 +899,20 @@ doRemoteOpen (virConnectPtr conn, conn, NULL)) < 0) { DEBUG0("virEventAddHandle failed: No addHandleImpl defined." " continuing without events."); - } else { + priv->watch = -1; + } - DEBUG0("Adding Timeout for remote event queue flushing"); - if ( (priv->eventFlushTimer = virEventAddTimeout(-1, - remoteDomainEventQueueFlush, - conn, NULL)) < 0) { - DEBUG0("virEventAddTimeout failed: No addTimeoutImpl defined. " - "continuing without events."); - virEventRemoveHandle(priv->watch); - priv->watch = -1; - } + priv->domainEventState = virDomainEventStateNew(remoteDomainEventQueueFlush, + conn, + NULL); + 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; @@ -1440,12 +1426,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, @@ -1486,11 +1473,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; } @@ -7506,17 +7489,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, @@ -7539,21 +7525,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, @@ -9159,12 +9138,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")); @@ -9173,13 +9153,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; } } @@ -9202,28 +9186,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, @@ -9901,13 +9880,7 @@ processCallDispatchMessage(virConnectPtr conn, struct private_data *priv, if (!event) return -1; - if (virDomainEventQueuePush(priv->domainEvents, - event) < 0) { - DEBUG0("Error adding event to queue"); - virDomainEventFree(event); - } - virEventUpdateTimeout(priv->eventFlushTimer, 0); - + remoteDomainEventQueue(priv, event); return 0; } @@ -10545,30 +10518,19 @@ remoteDomainEventQueueFlush(int timer ATTRIBUTE_UNUSED, void *opaque) { virConnectPtr conn = opaque; struct private_data *priv = conn->privateData; - virDomainEventQueue tempQueue; remoteDriverLock(priv); - - 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.3.3
On 01/10/2011 01:28 PM, Cole Robinson wrote:
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.
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
ACK series; you covered all my findings from v1. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
participants (3)
-
Cole Robinson -
Daniel P. Berrange -
Eric Blake