[libvirt] [PATCH 7/8] Hide use of timers for domain event dispatch

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; Currently all drivers using domain events need to provide a callback for handling a timer to dispatch events in a clean stack. There is no technical reason for dispatch to go via driver specific code. It could trivially be dispatched directly from the domain event code, thus removing tedious boilerplate code from all drivers * src/conf/domain_event.c, src/conf/domain_event.h: Internalize dispatch of events from timer callback * src/libxl/libxl_driver.c, src/lxc/lxc_driver.c, src/qemu/qemu_domain.c, src/qemu/qemu_driver.c, src/remote/remote_driver.c, src/test/test_driver.c, src/uml/uml_driver.c, src/vbox/vbox_tmpl.c, src/xen/xen_driver.c: Remove all timer dispatch functions --- src/conf/domain_event.c | 86 +++++++++++++++++++++++++++++--------------- src/conf/domain_event.h | 32 +---------------- src/libxl/libxl_driver.c | 30 +--------------- src/lxc/lxc_driver.c | 33 +---------------- src/qemu/qemu_domain.c | 25 ------------- src/qemu/qemu_driver.c | 5 +-- src/remote/remote_driver.c | 37 +------------------ src/test/test_driver.c | 32 +---------------- src/uml/uml_driver.c | 33 +---------------- src/vbox/vbox_tmpl.c | 45 +---------------------- src/xen/xen_driver.c | 40 +-------------------- 11 files changed, 66 insertions(+), 332 deletions(-) diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c index bd305ec..da99d9f 100644 --- a/src/conf/domain_event.c +++ b/src/conf/domain_event.c @@ -619,21 +619,25 @@ virDomainEventStateFree(virDomainEventStatePtr state) VIR_FREE(state); } + +static void virDomainEventStateFlush(virDomainEventStatePtr state); + +static void virDomainEventTimer(int timer ATTRIBUTE_UNUSED, void *opaque) +{ + virDomainEventStatePtr state = opaque; + + virDomainEventStateFlush(state); +} + /** * 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, - bool requireTimer) +virDomainEventStateNew(bool requireTimer) { virDomainEventStatePtr state = NULL; @@ -659,9 +663,9 @@ virDomainEventStateNew(virEventTimeoutCallback timeout_cb, } if ((state->timer = virEventAddTimeout(-1, - timeout_cb, - timeout_opaque, - timeout_free)) < 0) { + virDomainEventTimer, + state, + NULL)) < 0) { if (requireTimer == false) { VIR_DEBUG("virEventAddTimeout failed: No addTimeoutImpl defined. " "continuing without events."); @@ -1086,11 +1090,19 @@ virDomainEventQueuePush(virDomainEventQueuePtr evtQueue, } -void virDomainEventDispatchDefaultFunc(virConnectPtr conn, - virDomainEventPtr event, - virConnectDomainEventGenericCallback cb, - void *cbopaque, - void *opaque ATTRIBUTE_UNUSED) +typedef void (*virDomainEventDispatchFunc)(virConnectPtr conn, + virDomainEventPtr event, + virConnectDomainEventGenericCallback cb, + void *cbopaque, + void *opaque); + + +static void +virDomainEventDispatchDefaultFunc(virConnectPtr conn, + virDomainEventPtr event, + virConnectDomainEventGenericCallback cb, + void *cbopaque, + void *opaque ATTRIBUTE_UNUSED) { virDomainPtr dom = virGetDomain(conn, event->dom.name, event->dom.uuid); if (!dom) @@ -1206,10 +1218,12 @@ static int virDomainEventDispatchMatchCallback(virDomainEventPtr event, } } -void virDomainEventDispatch(virDomainEventPtr event, - virDomainEventCallbackListPtr callbacks, - virDomainEventDispatchFunc dispatch, - void *opaque) + +static void +virDomainEventDispatch(virDomainEventPtr event, + virDomainEventCallbackListPtr callbacks, + virDomainEventDispatchFunc dispatch, + void *opaque) { int i; /* Cache this now, since we may be dropping the lock, @@ -1230,10 +1244,11 @@ void virDomainEventDispatch(virDomainEventPtr event, } -void virDomainEventQueueDispatch(virDomainEventQueuePtr queue, - virDomainEventCallbackListPtr callbacks, - virDomainEventDispatchFunc dispatch, - void *opaque) +static void +virDomainEventQueueDispatch(virDomainEventQueuePtr queue, + virDomainEventCallbackListPtr callbacks, + virDomainEventDispatchFunc dispatch, + void *opaque) { int i; @@ -1266,10 +1281,23 @@ virDomainEventStateQueue(virDomainEventStatePtr state, virDomainEventStateUnlock(state); } -void -virDomainEventStateFlush(virDomainEventStatePtr state, - virDomainEventDispatchFunc dispatchFunc, - void *opaque) + +static void virDomainEventStateDispatchFunc(virConnectPtr conn, + virDomainEventPtr event, + virConnectDomainEventGenericCallback cb, + void *cbopaque, + void *opaque) +{ + virDomainEventStatePtr state = opaque; + + /* Drop the lock whle dispatching, for sake of re-entrancy */ + virDomainEventStateUnlock(state); + virDomainEventDispatchDefaultFunc(conn, event, cb, cbopaque, NULL); + virDomainEventStateLock(state); +} + +static void +virDomainEventStateFlush(virDomainEventStatePtr state) { virDomainEventQueue tempQueue; @@ -1287,8 +1315,8 @@ virDomainEventStateFlush(virDomainEventStatePtr state, virDomainEventQueueDispatch(&tempQueue, state->callbacks, - dispatchFunc, - opaque); + virDomainEventStateDispatchFunc, + state); /* Purge any deleted callbacks */ virDomainEventStateLock(state); diff --git a/src/conf/domain_event.h b/src/conf/domain_event.h index f37309c..a6c3562 100644 --- a/src/conf/domain_event.h +++ b/src/conf/domain_event.h @@ -119,42 +119,12 @@ void virDomainEventFree(virDomainEventPtr event); void virDomainEventStateFree(virDomainEventStatePtr state); virDomainEventStatePtr -virDomainEventStateNew(virEventTimeoutCallback timeout_cb, - void *timeout_opaque, - virFreeCallback timeout_free, - bool requireTimer) - ATTRIBUTE_NONNULL(1); - -typedef void (*virDomainEventDispatchFunc)(virConnectPtr conn, - virDomainEventPtr event, - virConnectDomainEventGenericCallback cb, - void *cbopaque, - void *opaque); -void virDomainEventDispatchDefaultFunc(virConnectPtr conn, - virDomainEventPtr event, - virConnectDomainEventGenericCallback cb, - void *cbopaque, - void *opaque); - -void virDomainEventDispatch(virDomainEventPtr event, - virDomainEventCallbackListPtr cbs, - virDomainEventDispatchFunc dispatch, - void *opaque); -void virDomainEventQueueDispatch(virDomainEventQueuePtr queue, - virDomainEventCallbackListPtr cbs, - virDomainEventDispatchFunc dispatch, - void *opaque); - +virDomainEventStateNew(bool requireTimer); 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 virDomainEventStateRegister(virConnectPtr conn, virDomainEventStatePtr state, virConnectDomainEventCallback callback, diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index b9382ee..04392da 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -111,30 +111,6 @@ libxlDomainObjPrivateFree(void *data) VIR_FREE(priv); } -static void -libxlDomainEventDispatchFunc(virConnectPtr conn, virDomainEventPtr event, - virConnectDomainEventGenericCallback cb, - void *cbopaque, void *opaque) -{ - libxlDriverPrivatePtr driver = opaque; - - /* Drop the lock whle dispatching, for sake of re-entrancy */ - libxlDriverUnlock(driver); - virDomainEventDispatchDefaultFunc(conn, event, cb, cbopaque, NULL); - libxlDriverLock(driver); -} - -static void -libxlDomainEventFlush(int timer ATTRIBUTE_UNUSED, void *opaque) -{ - libxlDriverPrivatePtr driver = opaque; - - libxlDriverLock(driver); - virDomainEventStateFlush(driver->domainEventState, - libxlDomainEventDispatchFunc, - driver); - libxlDriverUnlock(driver); -} /* driver must be locked before calling */ static void @@ -952,11 +928,7 @@ libxlStartup(int privileged) { } VIR_FREE(log_file); - libxl_driver->domainEventState = virDomainEventStateNew( - libxlDomainEventFlush, - libxl_driver, - NULL, - false); + libxl_driver->domainEventState = virDomainEventStateNew(true); if (!libxl_driver->domainEventState) goto error; diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 6a9ebde..6d32ed2 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -109,7 +109,6 @@ static void lxcDomainObjPrivateFree(void *data) } -static void lxcDomainEventFlush(int timer, void *opaque); static void lxcDomainEventQueue(lxc_driver_t *driver, virDomainEventPtr event); @@ -2192,33 +2191,6 @@ lxcDomainEventDeregisterAny(virConnectPtr conn, } -static void lxcDomainEventDispatchFunc(virConnectPtr conn, - virDomainEventPtr event, - virConnectDomainEventGenericCallback cb, - void *cbopaque, - void *opaque) -{ - lxc_driver_t *driver = opaque; - - /* Drop the lock whle dispatching, for sake of re-entrancy */ - lxcDriverUnlock(driver); - virDomainEventDispatchDefaultFunc(conn, event, cb, cbopaque, NULL); - lxcDriverLock(driver); -} - - -static void lxcDomainEventFlush(int timer ATTRIBUTE_UNUSED, void *opaque) -{ - lxc_driver_t *driver = opaque; - - lxcDriverLock(driver); - virDomainEventStateFlush(driver->domainEventState, - lxcDomainEventDispatchFunc, - driver); - lxcDriverUnlock(driver); -} - - /* driver must be locked before calling */ static void lxcDomainEventQueue(lxc_driver_t *driver, virDomainEventPtr event) @@ -2446,10 +2418,7 @@ static int lxcStartup(int privileged) if (virDomainObjListInit(&lxc_driver->domains) < 0) goto cleanup; - lxc_driver->domainEventState = virDomainEventStateNew(lxcDomainEventFlush, - lxc_driver, - NULL, - true); + lxc_driver->domainEventState = virDomainEventStateNew(true); if (!lxc_driver->domainEventState) goto cleanup; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index b28c734..2d612fe 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -114,31 +114,6 @@ qemuDomainAsyncJobPhaseFromString(enum qemuDomainAsyncJob job, return -1; } -static void qemuDomainEventDispatchFunc(virConnectPtr conn, - virDomainEventPtr event, - virConnectDomainEventGenericCallback cb, - void *cbopaque, - void *opaque) -{ - struct qemud_driver *driver = opaque; - - /* Drop the lock whle dispatching, for sake of re-entrancy */ - qemuDriverUnlock(driver); - virDomainEventDispatchDefaultFunc(conn, event, cb, cbopaque, NULL); - qemuDriverLock(driver); -} - -void qemuDomainEventFlush(int timer ATTRIBUTE_UNUSED, void *opaque) -{ - struct qemud_driver *driver = opaque; - - qemuDriverLock(driver); - virDomainEventStateFlush(driver->domainEventState, - qemuDomainEventDispatchFunc, - driver); - qemuDriverUnlock(driver); -} - /* driver must be locked before calling */ void qemuDomainEventQueue(struct qemud_driver *driver, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 40bd30f..b2a8525 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -431,10 +431,7 @@ qemudStartup(int privileged) { goto out_of_memory; /* Init domain events */ - qemu_driver->domainEventState = virDomainEventStateNew(qemuDomainEventFlush, - qemu_driver, - NULL, - true); + qemu_driver->domainEventState = virDomainEventStateNew(true); if (!qemu_driver->domainEventState) goto error; diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index a10bcad..03bd424 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -145,7 +145,6 @@ static void make_nonnull_storage_vol (remote_nonnull_storage_vol *vol_dst, virSt static void make_nonnull_secret (remote_nonnull_secret *secret_dst, virSecretPtr secret_src); static void make_nonnull_nwfilter (remote_nonnull_nwfilter *nwfilter_dst, virNWFilterPtr nwfilter_src); static void make_nonnull_domain_snapshot (remote_nonnull_domain_snapshot *snapshot_dst, virDomainSnapshotPtr snapshot_src); -static void remoteDomainEventQueueFlush(int timer, void *opaque); static void remoteDomainEventQueue(struct private_data *priv, virDomainEventPtr event); /*----------------------------------------------------------------------*/ @@ -727,10 +726,7 @@ doRemoteOpen (virConnectPtr conn, } } - if (!(priv->domainEventState = virDomainEventStateNew(remoteDomainEventQueueFlush, - conn, - NULL, - false))) + if (!(priv->domainEventState = virDomainEventStateNew(false))) goto failed; /* Successful. */ @@ -4352,37 +4348,6 @@ call (virConnectPtr conn, } -static void remoteDomainEventDispatchFunc(virConnectPtr conn, - virDomainEventPtr event, - virConnectDomainEventGenericCallback cb, - void *cbopaque, - void *opaque) -{ - struct private_data *priv = opaque; - - /* Drop the lock whle dispatching, for sake of re-entrancy */ - remoteDriverUnlock(priv); - VIR_DEBUG("Dispatch event %p %p", event, conn); - virDomainEventDispatchDefaultFunc(conn, event, cb, cbopaque, NULL); - remoteDriverLock(priv); -} - -static void -remoteDomainEventQueueFlush(int timer ATTRIBUTE_UNUSED, void *opaque) -{ - virConnectPtr conn = opaque; - struct private_data *priv = conn->privateData; - - - remoteDriverLock(priv); - VIR_DEBUG("Event queue flush %p", conn); - - virDomainEventStateFlush(priv->domainEventState, - remoteDomainEventDispatchFunc, - priv); - remoteDriverUnlock(priv); -} - static void remoteDomainEventQueue(struct private_data *priv, virDomainEventPtr event) { diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 054a41a..5ec4190 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -117,7 +117,6 @@ static const virNodeInfo defaultNodeInfo = { __FUNCTION__, __LINE__, __VA_ARGS__) static int testClose(virConnectPtr conn); -static void testDomainEventFlush(int timer, void *opaque); static void testDomainEventQueue(testConnPtr driver, virDomainEventPtr event); @@ -1138,10 +1137,7 @@ static virDrvOpenStatus testOpen(virConnectPtr conn, privconn = conn->privateData; testDriverLock(privconn); - privconn->domainEventState = virDomainEventStateNew(testDomainEventFlush, - privconn, - NULL, - false); + privconn->domainEventState = virDomainEventStateNew(false); if (!privconn->domainEventState) { testDriverUnlock(privconn); testClose(conn); @@ -5496,32 +5492,6 @@ testDomainEventDeregisterAny(virConnectPtr conn, } -static void testDomainEventDispatchFunc(virConnectPtr conn, - virDomainEventPtr event, - virConnectDomainEventGenericCallback cb, - void *cbopaque, - void *opaque) -{ - testConnPtr driver = opaque; - - /* Drop the lock whle dispatching, for sake of re-entrancy */ - testDriverUnlock(driver); - virDomainEventDispatchDefaultFunc(conn, event, cb, cbopaque, NULL); - testDriverLock(driver); -} - -static void testDomainEventFlush(int timer ATTRIBUTE_UNUSED, void *opaque) -{ - testConnPtr driver = opaque; - - testDriverLock(driver); - virDomainEventStateFlush(driver->domainEventState, - testDomainEventDispatchFunc, - driver); - testDriverUnlock(driver); -} - - /* driver must be locked before calling */ static void testDomainEventQueue(testConnPtr driver, virDomainEventPtr event) diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index 4d875c8..360f0ce 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -124,7 +124,6 @@ static int umlOpenMonitor(struct uml_driver *driver, virDomainObjPtr vm); static int umlReadPidFile(struct uml_driver *driver, virDomainObjPtr vm); -static void umlDomainEventFlush(int timer, void *opaque); static void umlDomainEventQueue(struct uml_driver *driver, virDomainEventPtr event); @@ -414,10 +413,7 @@ umlStartup(int privileged) if (virDomainObjListInit(&uml_driver->domains) < 0) goto error; - uml_driver->domainEventState = virDomainEventStateNew(umlDomainEventFlush, - uml_driver, - NULL, - true); + uml_driver->domainEventState = virDomainEventStateNew(true); if (!uml_driver->domainEventState) goto error; @@ -2511,33 +2507,6 @@ umlDomainEventDeregisterAny(virConnectPtr conn, } -static void umlDomainEventDispatchFunc(virConnectPtr conn, - virDomainEventPtr event, - virConnectDomainEventGenericCallback cb, - void *cbopaque, - void *opaque) -{ - struct uml_driver *driver = opaque; - - /* Drop the lock whle dispatching, for sake of re-entrancy */ - umlDriverUnlock(driver); - virDomainEventDispatchDefaultFunc(conn, event, cb, cbopaque, NULL); - umlDriverLock(driver); -} - - -static void umlDomainEventFlush(int timer ATTRIBUTE_UNUSED, void *opaque) -{ - struct uml_driver *driver = opaque; - - umlDriverLock(driver); - virDomainEventStateFlush(driver->domainEventState, - umlDomainEventDispatchFunc, - driver); - umlDriverUnlock(driver); -} - - /* driver must be locked before calling */ static void umlDomainEventQueue(struct uml_driver *driver, virDomainEventPtr event) diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c index 1fb369b..a10f5ae 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -970,46 +970,6 @@ static void vboxUninitialize(vboxGlobalData *data) { } -#if VBOX_API_VERSION == 2002 - /* No domainEventCallbacks in 2.2.* version */ -#else /* !(VBOX_API_VERSION == 2002) */ - -static void -vboxDomainEventDispatchFunc(virConnectPtr conn, - virDomainEventPtr event, - virConnectDomainEventGenericCallback cb, - void *cbopaque, - void *opaque) -{ - vboxGlobalData *data = opaque; - - /* - * Release the lock while the callback is running so that - * we're re-entrant safe for callback work - the callback - * may want to invoke other virt functions & we have already - * protected the one piece of state we have - the callback - * list - */ - vboxDriverUnlock(data); - virDomainEventDispatchDefaultFunc(conn, event, cb, cbopaque, NULL); - vboxDriverLock(data); -} - - -static void vboxDomainEventFlush(int timer ATTRIBUTE_UNUSED, void *opaque) -{ - virConnectPtr conn = opaque; - vboxGlobalData *data = conn->privateData; - - vboxDriverLock(data); - virDomainEventStateFlush(data->domainEvents, - vboxDomainEventDispatchFunc, - data); - vboxDriverUnlock(data); -} -#endif /* !(VBOX_API_VERSION == 2002) */ - - static virDrvOpenStatus vboxOpen(virConnectPtr conn, virConnectAuthPtr auth ATTRIBUTE_UNUSED, unsigned int flags) @@ -1074,10 +1034,7 @@ static virDrvOpenStatus vboxOpen(virConnectPtr conn, #else /* !(VBOX_API_VERSION == 2002) */ - if (!(data->domainEvents = virDomainEventStateNew(vboxDomainEventFlush, - data, - NULL, - true))) { + if (!(data->domainEvents = virDomainEventStateNew(true))) { vboxUninitialize(data); return VIR_DRV_OPEN_ERROR; } diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c index a2399dd..ab49c2b 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.c @@ -63,7 +63,6 @@ static int xenUnifiedDomainGetVcpus (virDomainPtr dom, virVcpuInfoPtr info, int maxinfo, unsigned char *cpumaps, int maplen); -static void xenDomainEventFlush(int timer ATTRIBUTE_UNUSED, void *opaque); /* The five Xen drivers below us. */ @@ -326,10 +325,7 @@ xenUnifiedOpen (virConnectPtr conn, virConnectAuthPtr auth, unsigned int flags) return VIR_DRV_OPEN_ERROR; } - if (!(priv->domainEvents = virDomainEventStateNew(xenDomainEventFlush, - priv, - NULL, - NULL))) { + if (!(priv->domainEvents = virDomainEventStateNew(true))) { virMutexDestroy(&priv->lock); VIR_FREE(priv); return VIR_DRV_OPEN_ERROR; @@ -2388,40 +2384,6 @@ xenUnifiedRemoveDomainInfo(xenUnifiedDomainInfoListPtr list, } -static void -xenUnifiedDomainEventDispatchFunc(virConnectPtr conn, - virDomainEventPtr event, - virConnectDomainEventGenericCallback cb, - void *cbopaque, - void *opaque) -{ - xenUnifiedPrivatePtr priv = opaque; - - /* - * Release the lock while the callback is running so that - * we're re-entrant safe for callback work - the callback - * may want to invoke other virt functions & we have already - * protected the one piece of state we have - the callback - * list - */ - xenUnifiedUnlock(priv); - virDomainEventDispatchDefaultFunc(conn, event, cb, cbopaque, NULL); - xenUnifiedLock(priv); -} - -static void xenDomainEventFlush(int timer ATTRIBUTE_UNUSED, void *opaque) -{ - virConnectPtr conn = opaque; - xenUnifiedPrivatePtr priv = conn->privateData; - - xenUnifiedLock(priv); - virDomainEventStateFlush(priv->domainEvents, - xenUnifiedDomainEventDispatchFunc, - priv); - xenUnifiedUnlock(priv); -} - - /** * xenUnifiedDomainEventDispatch: * @priv: the connection to dispatch events on -- 1.7.7.4