[libvirt] [PATCH 1/8] Convert Xen & VBox drivers to use virDomainEventState

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; The Xen & VBox drivers deal with callbacks & dispatching of events directly. All the other drivers use a timer to dispatch events from a clean stack state, rather than deep inside the drivers. Convert Xen & VBox over to virDomainEventState so that they match behaviour of other drivers * src/conf/domain_event.c: Return count of remaining callbacks when unregistering event callback * src/vbox/vbox_tmpl.c, src/xen/xen_driver.c, src/xen/xen_driver.h: Convert to virDomainEventState --- src/conf/domain_event.c | 58 ++++++++++++++++-- src/conf/domain_event.h | 6 +- src/vbox/vbox_tmpl.c | 146 +++++++++++++++++++++++++---------------------- src/xen/xen_driver.c | 68 ++++++++++------------ src/xen/xen_driver.h | 4 +- 5 files changed, 164 insertions(+), 118 deletions(-) diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c index 614ab97..0e481cf 100644 --- a/src/conf/domain_event.c +++ b/src/conf/domain_event.c @@ -133,6 +133,7 @@ virDomainEventCallbackListRemove(virConnectPtr conn, virDomainEventCallbackListPtr cbList, virConnectDomainEventCallback callback) { + int ret = 0; int i; for (i = 0 ; i < cbList->count ; i++) { if (cbList->callbacks[i]->cb == VIR_DOMAIN_EVENT_CALLBACK(callback) && @@ -156,7 +157,11 @@ virDomainEventCallbackListRemove(virConnectPtr conn, } cbList->count--; - return 0; + for (i = 0 ; i < cbList->count ; i++) { + if (!cbList->callbacks[i]->deleted) + ret++; + } + return ret; } } @@ -179,6 +184,7 @@ virDomainEventCallbackListRemoveID(virConnectPtr conn, virDomainEventCallbackListPtr cbList, int callbackID) { + int ret = 0; int i; for (i = 0 ; i < cbList->count ; i++) { if (cbList->callbacks[i]->callbackID == callbackID && @@ -201,7 +207,11 @@ virDomainEventCallbackListRemoveID(virConnectPtr conn, } cbList->count--; - return 0; + for (i = 0 ; i < cbList->count ; i++) { + if (!cbList->callbacks[i]->deleted) + ret++; + } + return ret; } } @@ -254,13 +264,18 @@ int virDomainEventCallbackListMarkDelete(virConnectPtr conn, virDomainEventCallbackListPtr cbList, virConnectDomainEventCallback callback) { + int ret = 0; int i; for (i = 0 ; i < cbList->count ; i++) { if (cbList->callbacks[i]->cb == VIR_DOMAIN_EVENT_CALLBACK(callback) && cbList->callbacks[i]->eventID == VIR_DOMAIN_EVENT_ID_LIFECYCLE && cbList->callbacks[i]->conn == conn) { cbList->callbacks[i]->deleted = 1; - return 0; + for (i = 0 ; i < cbList->count ; i++) { + if (!cbList->callbacks[i]->deleted) + ret++; + } + return ret; } } @@ -274,12 +289,17 @@ int virDomainEventCallbackListMarkDeleteID(virConnectPtr conn, virDomainEventCallbackListPtr cbList, int callbackID) { + int ret = 0; int i; for (i = 0 ; i < cbList->count ; i++) { if (cbList->callbacks[i]->callbackID == callbackID && cbList->callbacks[i]->conn == conn) { cbList->callbacks[i]->deleted = 1; - return 0; + for (i = 0 ; i < cbList->count ; i++) { + if (!cbList->callbacks[i]->deleted) + ret++; + } + return ret; } } @@ -1307,6 +1327,18 @@ virDomainEventStateFlush(virDomainEventStatePtr state, virDomainEventStateUnlock(state); } + +/** + * virDomainEventStateDeregister: + * @state: domain event state + * @conn: connection to associate with callback + * @callback: function to remove from event + * + * Unregister the function @callback with connection @conn, + * from @state, for lifecycle events. + * + * Returns: the number of lifecycle callbacks still registered, or -1 on error + */ int virDomainEventStateDeregister(virConnectPtr conn, virDomainEventStatePtr state, @@ -1324,10 +1356,22 @@ virDomainEventStateDeregister(virConnectPtr conn, return ret; } + +/** + * virDomainEventStateDeregisterID: + * @state: domain event state + * @conn: connection to associate with callback + * @callbackID: ID of the function to remove from event + * + * Unregister the function @callbackID with connection @conn, + * from @state, for lifecycle events. + * + * Returns: the number of lifecycle callbacks still registered, or -1 on error + */ int -virDomainEventStateDeregisterAny(virConnectPtr conn, - virDomainEventStatePtr state, - int callbackID) +virDomainEventStateDeregisterID(virConnectPtr conn, + virDomainEventStatePtr state, + int callbackID) { int ret; diff --git a/src/conf/domain_event.h b/src/conf/domain_event.h index 3ba418e..10a55b0 100644 --- a/src/conf/domain_event.h +++ b/src/conf/domain_event.h @@ -242,9 +242,9 @@ virDomainEventStateDeregister(virConnectPtr conn, virConnectDomainEventCallback callback) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); int -virDomainEventStateDeregisterAny(virConnectPtr conn, - virDomainEventStatePtr state, - int callbackID) +virDomainEventStateDeregisterID(virConnectPtr conn, + virDomainEventStatePtr state, + int callbackID) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); #endif diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c index 9b74a7b..7f13f90 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -188,11 +188,9 @@ typedef struct { #else /* !(VBOX_API_VERSION == 2002) */ - /* An array of callbacks */ - virDomainEventCallbackListPtr domainEventCallbacks; - + /* Async event handling */ + virDomainEventStatePtr domainEvents; int fdWatch; - int domainEventDispatching; # if VBOX_API_VERSION <= 3002 /* IVirtualBoxCallback is used in VirtualBox 3.x only */ @@ -966,11 +964,52 @@ static void vboxUninitialize(vboxGlobalData *data) { #if VBOX_API_VERSION == 2002 /* No domainEventCallbacks in 2.2.* version */ #else /* !(VBOX_API_VERSION == 2002) */ - VIR_FREE(data->domainEventCallbacks); + virDomainEventStateFree(data->domainEvents); #endif /* !(VBOX_API_VERSION == 2002) */ VIR_FREE(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) @@ -1035,8 +1074,11 @@ static virDrvOpenStatus vboxOpen(virConnectPtr conn, #else /* !(VBOX_API_VERSION == 2002) */ - if (VIR_ALLOC(data->domainEventCallbacks) < 0) { - virReportOOMError(); + if (!(data->domainEvents = virDomainEventStateNew(vboxDomainEventFlush, + data, + NULL, + true))) { + vboxUninitialize(data); return VIR_DRV_OPEN_ERROR; } @@ -6770,7 +6812,6 @@ vboxCallbackOnMachineStateChange(IVirtualBoxCallback *pThis ATTRIBUTE_UNUSED, int event = 0; int detail = 0; - g_pVBoxGlobalData->domainEventDispatching = 1; vboxDriverLock(g_pVBoxGlobalData); VIR_DEBUG("IVirtualBoxCallback: %p, State: %d", pThis, state); @@ -6818,20 +6859,12 @@ vboxCallbackOnMachineStateChange(IVirtualBoxCallback *pThis ATTRIBUTE_UNUSED, ev = virDomainEventNewFromDom(dom, event, detail); - if (ev) { - virDomainEventDispatch(ev, - g_pVBoxGlobalData->domainEventCallbacks, - virDomainEventDispatchDefaultFunc, - NULL); - virDomainEventFree(ev); - } + if (ev) + virDomainEventStateQueue(g_pVBoxGlobalData->domainEvents, ev); } } - virDomainEventCallbackListPurgeMarked(g_pVBoxGlobalData->domainEventCallbacks); - vboxDriverUnlock(g_pVBoxGlobalData); - g_pVBoxGlobalData->domainEventDispatching = 0; return NS_OK; } @@ -6898,7 +6931,6 @@ vboxCallbackOnMachineRegistered(IVirtualBoxCallback *pThis ATTRIBUTE_UNUSED, int event = 0; int detail = 0; - g_pVBoxGlobalData->domainEventDispatching = 1; vboxDriverLock(g_pVBoxGlobalData); VIR_DEBUG("IVirtualBoxCallback: %p, registered: %s", pThis, registered ? "true" : "false"); @@ -6931,20 +6963,12 @@ vboxCallbackOnMachineRegistered(IVirtualBoxCallback *pThis ATTRIBUTE_UNUSED, ev = virDomainEventNewFromDom(dom, event, detail); - if (ev) { - virDomainEventDispatch(ev, - g_pVBoxGlobalData->domainEventCallbacks, - virDomainEventDispatchDefaultFunc, - NULL); - virDomainEventFree(ev); - } + if (ev) + virDomainEventStateQueue(g_pVBoxGlobalData->domainEvents, ev); } } - virDomainEventCallbackListPurgeMarked(g_pVBoxGlobalData->domainEventCallbacks); - vboxDriverUnlock(g_pVBoxGlobalData); - g_pVBoxGlobalData->domainEventDispatching = 0; return NS_OK; } @@ -7164,11 +7188,11 @@ static int vboxDomainEventRegister (virConnectPtr conn, * later you can iterate over them */ - ret = virDomainEventCallbackListAdd(conn, data->domainEventCallbacks, + ret = virDomainEventCallbackListAdd(conn, data->domainEvents->callbacks, callback, opaque, freecb); VIR_DEBUG("virDomainEventCallbackListAdd (ret = %d) ( conn: %p, " - "data->domainEventCallbacks: %p, callback: %p, opaque: %p, " - "freecb: %p )", ret, conn, data->domainEventCallbacks, callback, + "data->domainEvents->callbacks: %p, callback: %p, opaque: %p, " + "freecb: %p )", ret, conn, data->domainEvents->callbacks, callback, opaque, freecb); } } @@ -7188,31 +7212,23 @@ static int vboxDomainEventRegister (virConnectPtr conn, static int vboxDomainEventDeregister (virConnectPtr conn, virConnectDomainEventCallback callback) { VBOX_OBJECT_CHECK(conn, int, -1); + int cnt; /* Locking has to be there as callbacks are not * really fully thread safe */ vboxDriverLock(data); - if (data->domainEventDispatching) - ret = virDomainEventCallbackListMarkDelete(conn, data->domainEventCallbacks, - callback); - else - ret = virDomainEventCallbackListRemove(conn, data->domainEventCallbacks, - callback); + cnt = virDomainEventStateDeregister(conn, data->domainEvents, + callback); - if (data->vboxCallback) { - /* check count here of how many times register was called - * and only on the last de-register do the un-register call - */ - if (data->domainEventCallbacks && virDomainEventCallbackListCount(data->domainEventCallbacks) == 0) { - data->vboxObj->vtbl->UnregisterCallback(data->vboxObj, data->vboxCallback); - VBOX_RELEASE(data->vboxCallback); + if (data->vboxCallback && cnt == 0) { + data->vboxObj->vtbl->UnregisterCallback(data->vboxObj, data->vboxCallback); + VBOX_RELEASE(data->vboxCallback); - /* Remove the Event file handle on which we are listening as well */ - virEventRemoveHandle(data->fdWatch); - data->fdWatch = -1; - } + /* Remove the Event file handle on which we are listening as well */ + virEventRemoveHandle(data->fdWatch); + data->fdWatch = -1; } vboxDriverUnlock(data); @@ -7264,12 +7280,12 @@ static int vboxDomainEventRegisterAny(virConnectPtr conn, * later you can iterate over them */ - ret = virDomainEventCallbackListAddID(conn, data->domainEventCallbacks, + ret = virDomainEventCallbackListAddID(conn, data->domainEvents->callbacks, dom, eventID, callback, opaque, freecb); VIR_DEBUG("virDomainEventCallbackListAddID (ret = %d) ( conn: %p, " - "data->domainEventCallbacks: %p, callback: %p, opaque: %p, " - "freecb: %p )", ret, conn, data->domainEventCallbacks, callback, + "data->domainEvents->callbacks: %p, callback: %p, opaque: %p, " + "freecb: %p )", ret, conn, data->domainEvents->callbacks, callback, opaque, freecb); } } @@ -7289,31 +7305,23 @@ static int vboxDomainEventRegisterAny(virConnectPtr conn, static int vboxDomainEventDeregisterAny(virConnectPtr conn, int callbackID) { VBOX_OBJECT_CHECK(conn, int, -1); + int cnt; /* Locking has to be there as callbacks are not * really fully thread safe */ vboxDriverLock(data); - if (data->domainEventDispatching) - ret = virDomainEventCallbackListMarkDeleteID(conn, data->domainEventCallbacks, - callbackID); - else - ret = virDomainEventCallbackListRemoveID(conn, data->domainEventCallbacks, - callbackID); + cnt = virDomainEventStateDeregisterAny(conn, data->domainEvents, + callbackID); - if (data->vboxCallback) { - /* check count here of how many times register was called - * and only on the last de-register do the un-register call - */ - if (data->domainEventCallbacks && virDomainEventCallbackListCount(data->domainEventCallbacks) == 0) { - data->vboxObj->vtbl->UnregisterCallback(data->vboxObj, data->vboxCallback); - VBOX_RELEASE(data->vboxCallback); + if (data->vboxCallback && cnt == 0) { + data->vboxObj->vtbl->UnregisterCallback(data->vboxObj, data->vboxCallback); + VBOX_RELEASE(data->vboxCallback); - /* Remove the Event file handle on which we are listening as well */ - virEventRemoveHandle(data->fdWatch); - data->fdWatch = -1; - } + /* Remove the Event file handle on which we are listening as well */ + virEventRemoveHandle(data->fdWatch); + data->fdWatch = -1; } vboxDriverUnlock(data); diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c index d394ff7..8c47ad5 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.c @@ -63,6 +63,8 @@ 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. */ static struct xenUnifiedDriver const * const drivers[XEN_UNIFIED_NR_DRIVERS] = { @@ -248,12 +250,13 @@ xenUnifiedXendProbe (void) } #endif + + static virDrvOpenStatus xenUnifiedOpen (virConnectPtr conn, virConnectAuthPtr auth, unsigned int flags) { int i, ret = VIR_DRV_OPEN_DECLINED; xenUnifiedPrivatePtr priv; - virDomainEventCallbackListPtr cbList; #ifdef __sun /* @@ -323,17 +326,16 @@ xenUnifiedOpen (virConnectPtr conn, virConnectAuthPtr auth, unsigned int flags) return VIR_DRV_OPEN_ERROR; } - /* Allocate callback list */ - if (VIR_ALLOC(cbList) < 0) { - virReportOOMError(); + if (!(priv->domainEvents = virDomainEventStateNew(xenDomainEventFlush, + priv, + NULL, + NULL))) { virMutexDestroy(&priv->lock); VIR_FREE(priv); return VIR_DRV_OPEN_ERROR; } conn->privateData = priv; - priv->domainEventCallbacks = cbList; - priv->handle = -1; priv->xendConfigVersion = -1; priv->xshandle = NULL; @@ -423,7 +425,7 @@ xenUnifiedClose (virConnectPtr conn) int i; virCapabilitiesFree(priv->caps); - virDomainEventCallbackListFree(priv->domainEventCallbacks); + virDomainEventStateFree(priv->domainEvents); for (i = 0; i < XEN_UNIFIED_NR_DRIVERS; ++i) if (priv->opened[i]) @@ -1845,7 +1847,7 @@ xenUnifiedDomainEventRegister(virConnectPtr conn, return -1; } - ret = virDomainEventCallbackListAdd(conn, priv->domainEventCallbacks, + ret = virDomainEventCallbackListAdd(conn, priv->domainEvents->callbacks, callback, opaque, freefunc); xenUnifiedUnlock(priv); @@ -1867,12 +1869,9 @@ xenUnifiedDomainEventDeregister(virConnectPtr conn, return -1; } - if (priv->domainEventDispatching) - ret = virDomainEventCallbackListMarkDelete(conn, priv->domainEventCallbacks, - callback); - else - ret = virDomainEventCallbackListRemove(conn, priv->domainEventCallbacks, - callback); + ret = virDomainEventStateDeregister(conn, + priv->domainEvents, + callback); xenUnifiedUnlock(priv); return ret; @@ -1898,7 +1897,7 @@ xenUnifiedDomainEventRegisterAny(virConnectPtr conn, return -1; } - ret = virDomainEventCallbackListAddID(conn, priv->domainEventCallbacks, + ret = virDomainEventCallbackListAddID(conn, priv->domainEvents->callbacks, dom, eventID, callback, opaque, freefunc); @@ -1920,12 +1919,9 @@ xenUnifiedDomainEventDeregisterAny(virConnectPtr conn, return -1; } - if (priv->domainEventDispatching) - ret = virDomainEventCallbackListMarkDeleteID(conn, priv->domainEventCallbacks, - callbackID); - else - ret = virDomainEventCallbackListRemoveID(conn, priv->domainEventCallbacks, - callbackID); + ret = virDomainEventStateDeregisterAny(conn, + priv->domainEvents, + callbackID); xenUnifiedUnlock(priv); return ret; @@ -2390,6 +2386,7 @@ xenUnifiedRemoveDomainInfo(xenUnifiedDomainInfoListPtr list, return -1; } + static void xenUnifiedDomainEventDispatchFunc(virConnectPtr conn, virDomainEventPtr event, @@ -2411,6 +2408,19 @@ xenUnifiedDomainEventDispatchFunc(virConnectPtr conn, 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 @@ -2427,21 +2437,7 @@ void xenUnifiedDomainEventDispatch (xenUnifiedPrivatePtr priv, if (!priv) return; - priv->domainEventDispatching = 1; - - if (priv->domainEventCallbacks) { - virDomainEventDispatch(event, - priv->domainEventCallbacks, - xenUnifiedDomainEventDispatchFunc, - priv); - - /* Purge any deleted callbacks */ - virDomainEventCallbackListPurgeMarked(priv->domainEventCallbacks); - } - - virDomainEventFree(event); - - priv->domainEventDispatching = 0; + virDomainEventStateQueue(priv->domainEvents, event); } void xenUnifiedLock(xenUnifiedPrivatePtr priv) diff --git a/src/xen/xen_driver.h b/src/xen/xen_driver.h index ca711d4..4122378 100644 --- a/src/xen/xen_driver.h +++ b/src/xen/xen_driver.h @@ -183,9 +183,7 @@ struct _xenUnifiedPrivate { int nbNodeCells; int nbNodeCpus; - /* An list of callbacks */ - virDomainEventCallbackListPtr domainEventCallbacks; - int domainEventDispatching; + virDomainEventStatePtr domainEvents; /* Location of config files, either /etc * or /var/lib/xen */ -- 1.7.7.4