6:38 p.m.
When running
while true ; do date ; ../tools/virsh -q -c test:///default 'shutdown test;
undefine test; dominfo test' ; done
we often experiance hangs or crashes or memory corruption in
glibc. The reason is that when the virConnectPtr is free'd this
in turn frees the event state, which in turn unregisters the
event timer. The timer may, however, still fire one last time
and it is possible this occurs after the event state has been
freed.
The solution in this series is to only register the timer when
the first callback is added and unregister the timer, when the
last callback is removed.
In doing this patch, I decide to unify more of the event handling
across drivers, and simplify the API usage by drivers, and hide
all the impl details.
6:38 p.m.
New subject: [libvirt] [PATCH 1/8] Convert Xen & VBox drivers to use virDomainEventState
From: "Daniel P. Berrange" <berrange(a)redhat.com>
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
12:17 p.m.
New subject: [libvirt] [PATCH 1/8] Convert Xen & VBox drivers to use virDomainEventState
On 12/13/2011 05:38 PM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange(a)redhat.com>
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
Does this part of the commit message belong with 2/8?
* 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 +-
The rename of virDomainEventStateDeregisterAny to
virDomainEventStateDeregisterID wasn't mentioned in the commit message.
Did you rebase things incorrectly? That said, these changes look
useful, even if they were meant for a separate commit.
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(-)
ACK.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
12:26 p.m.
New subject: [libvirt] [PATCH 1/8] Convert Xen & VBox drivers to use virDomainEventState
On 12/14/2011 11:17 AM, Eric Blake wrote:
On 12/13/2011 05:38 PM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange(a)redhat.com>
>
> 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
Does this part of the commit message belong with 2/8?
> * 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 +-
The rename of virDomainEventStateDeregisterAny to
virDomainEventStateDeregisterID wasn't mentioned in the commit message.
Did you rebase things incorrectly? That said, these changes look
useful, even if they were meant for a separate commit.
Actually, compiling this patch fails:
test/test_driver.c: In function 'testDomainEventDeregisterAny':
test/test_driver.c:5462:5: error: implicit declaration of function
'virDomainEventStateDeregisterAny' [-Werror=implicit-function-declaration]
> 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(-)
ACK.
Can you resubmit with that fixed?
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
4:46 a.m.
New subject: [libvirt] [PATCH 1/8] Convert Xen & VBox drivers to use virDomainEventState
On Wed, Dec 14, 2011 at 11:26:57AM -0700, Eric Blake wrote:
On 12/14/2011 11:17 AM, Eric Blake wrote:
> On 12/13/2011 05:38 PM, Daniel P. Berrange wrote:
>> From: "Daniel P. Berrange" <berrange(a)redhat.com>
>>
>> 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
>
> Does this part of the commit message belong with 2/8?
>
>> * 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 +-
>
> The rename of virDomainEventStateDeregisterAny to
> virDomainEventStateDeregisterID wasn't mentioned in the commit message.
> Did you rebase things incorrectly? That said, these changes look
> useful, even if they were meant for a separate commit.
Actually, compiling this patch fails:
test/test_driver.c: In function 'testDomainEventDeregisterAny':
test/test_driver.c:5462:5: error: implicit declaration of function
'virDomainEventStateDeregisterAny' [-Werror=implicit-function-declaration]
>
>> 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(-)
>
> ACK.
Can you resubmit with that fixed?
This isn't as bad as it seems. The rename of 's/Any/ID/' in
domain_event.[ch] simply needed to move into patch 2 and all
the problems go away.
Regards,
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 :|
3:46 a.m.
New subject: [libvirt] [PATCH 1/8] Convert Xen & VBox drivers to use virDomainEventState
On Wed, Dec 14, 2011 at 11:17:47AM -0700, Eric Blake wrote:
On 12/13/2011 05:38 PM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange(a)redhat.com>
>
> 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
Does this part of the commit message belong with 2/8?
No, this is done in this patch. The next patch only deals with
returning counts when registering callbacks.
> * 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 +-
The rename of virDomainEventStateDeregisterAny to
virDomainEventStateDeregisterID wasn't mentioned in the commit message.
Did you rebase things incorrectly? That said, these changes look
useful, even if they were meant for a separate commit.
I've updated the commit message.
> 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(-)
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 :|
2:50 p.m.
New subject: [libvirt] [PATCH 1/8] Convert Xen & VBox drivers to use virDomainEventState
On 12/13/2011 05:38 PM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange(a)redhat.com>
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
@@ -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))) {
This last parameter should be false, not NULL (wonder why the compiler
didn't complain). It all goes away in later patches, but it might as
well be type-correct while it exists.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
6:38 p.m.
New subject: [libvirt] [PATCH 2/8] Return count of callbacks when registering callbacks
From: "Daniel P. Berrange" <berrange(a)redhat.com>
When registering a callback for a particular event some callers
need to know how many callbacks already exist for that event.
While it is possible to ask for a count, this is not free from
race conditions when threaded. Thus the API for registering
callbacks should return the count of callbacks
* src/conf/domain_event.c, src/conf/domain_event.h,
src/libvirt_private.syms: Return count of callbacks when
registering callbacks
* src/libxl/libxl_driver.c, src/libxl/libxl_driver.c,
src/qemu/qemu_driver.c, src/remote/remote_driver.c,
src/remote/remote_driver.c, src/uml/uml_driver.c,
src/vbox/vbox_tmpl.c, src/xen/xen_driver.c: Update
for change in APIs
---
src/conf/domain_event.c | 23 ++++++++++++++++++-----
src/conf/domain_event.h | 3 ++-
src/libvirt_private.syms | 2 +-
src/libxl/libxl_driver.c | 15 ++++++++-------
src/lxc/lxc_driver.c | 15 ++++++++-------
src/qemu/qemu_driver.c | 15 ++++++++-------
src/remote/remote_driver.c | 33 ++++++++++++++++-----------------
src/test/test_driver.c | 15 ++++++++-------
src/uml/uml_driver.c | 15 ++++++++-------
src/vbox/vbox_tmpl.c | 15 ++++++++-------
src/xen/xen_driver.c | 13 +++++++------
11 files changed, 92 insertions(+), 72 deletions(-)
diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c
index 0e481cf..00c5dbf 100644
--- a/src/conf/domain_event.c
+++ b/src/conf/domain_event.c
@@ -357,7 +357,7 @@ virDomainEventCallbackListAdd(virConnectPtr conn,
return virDomainEventCallbackListAddID(conn, cbList, NULL,
VIR_DOMAIN_EVENT_ID_LIFECYCLE,
VIR_DOMAIN_EVENT_CALLBACK(callback),
- opaque, freecb);
+ opaque, freecb, NULL);
}
@@ -368,6 +368,7 @@ virDomainEventCallbackListAdd(virConnectPtr conn,
* @eventID: the event ID
* @callback: the callback to add
* @opaque: opaque data tio pass to callback
+ * @callbackID: filled with callback ID
*
* Internal function to add a callback from a virDomainEventCallbackListPtr
*/
@@ -378,10 +379,12 @@ virDomainEventCallbackListAddID(virConnectPtr conn,
int eventID,
virConnectDomainEventGenericCallback callback,
void *opaque,
- virFreeCallback freecb)
+ virFreeCallback freecb,
+ int *callbackID)
{
virDomainEventCallbackPtr event;
int i;
+ int ret = 0;
/* Check incoming */
if ( !cbList ) {
@@ -427,7 +430,17 @@ virDomainEventCallbackListAddID(virConnectPtr conn,
event->callbackID = cbList->nextID++;
- return event->callbackID;
+ for (i = 0 ; i < cbList->count ; i++) {
+ if (cbList->callbacks[i]->eventID == VIR_DOMAIN_EVENT_ID_LIFECYCLE
&&
+ cbList->callbacks[i]->conn == conn &&
+ !cbList->callbacks[i]->deleted)
+ ret++;
+ }
+
+ if (callbackID)
+ *callbackID = event->callbackID;
+
+ return ret;
no_memory:
virReportOOMError();
@@ -1364,9 +1377,9 @@ virDomainEventStateDeregister(virConnectPtr conn,
* @callbackID: ID of the function to remove from event
*
* Unregister the function @callbackID with connection @conn,
- * from @state, for lifecycle events.
+ * from @state, for events.
*
- * Returns: the number of lifecycle callbacks still registered, or -1 on error
+ * Returns: the number of callbacks still registered, or -1 on error
*/
int
virDomainEventStateDeregisterID(virConnectPtr conn,
diff --git a/src/conf/domain_event.h b/src/conf/domain_event.h
index 10a55b0..83656e6 100644
--- a/src/conf/domain_event.h
+++ b/src/conf/domain_event.h
@@ -80,7 +80,8 @@ int virDomainEventCallbackListAddID(virConnectPtr conn,
int eventID,
virConnectDomainEventGenericCallback cb,
void *opaque,
- virFreeCallback freecb)
+ virFreeCallback freecb,
+ int *callbackID)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(5);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index a81c230..3d1d7d2 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -510,7 +510,7 @@ virDomainEventRebootNew;
virDomainEventRebootNewFromDom;
virDomainEventRebootNewFromObj;
virDomainEventStateDeregister;
-virDomainEventStateDeregisterAny;
+virDomainEventStateDeregisterID;
virDomainEventStateFlush;
virDomainEventStateFree;
virDomainEventStateNew;
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 7cc32ad..46504dc 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -3851,10 +3851,11 @@ libxlDomainEventRegisterAny(virConnectPtr conn, virDomainPtr dom,
int eventID,
int ret;
libxlDriverLock(driver);
- ret = virDomainEventCallbackListAddID(conn,
- driver->domainEventState->callbacks,
- dom, eventID, callback, opaque,
- freecb);
+ if (virDomainEventCallbackListAddID(conn,
+ driver->domainEventState->callbacks,
+ dom, eventID, callback, opaque,
+ freecb, &ret) < 0)
+ ret = -1;
libxlDriverUnlock(driver);
return ret;
@@ -3868,9 +3869,9 @@ libxlDomainEventDeregisterAny(virConnectPtr conn, int callbackID)
int ret;
libxlDriverLock(driver);
- ret = virDomainEventStateDeregisterAny(conn,
- driver->domainEventState,
- callbackID);
+ ret = virDomainEventStateDeregisterID(conn,
+ driver->domainEventState,
+ callbackID);
libxlDriverUnlock(driver);
return ret;
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index b16cfd8..6b17c75 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -2164,10 +2164,11 @@ lxcDomainEventRegisterAny(virConnectPtr conn,
int ret;
lxcDriverLock(driver);
- ret = virDomainEventCallbackListAddID(conn,
- driver->domainEventState->callbacks,
- dom, eventID,
- callback, opaque, freecb);
+ if (virDomainEventCallbackListAddID(conn,
+ driver->domainEventState->callbacks,
+ dom, eventID,
+ callback, opaque, freecb, &ret) < 0)
+ ret = -1;
lxcDriverUnlock(driver);
return ret;
@@ -2182,9 +2183,9 @@ lxcDomainEventDeregisterAny(virConnectPtr conn,
int ret;
lxcDriverLock(driver);
- ret = virDomainEventStateDeregisterAny(conn,
- driver->domainEventState,
- callbackID);
+ ret = virDomainEventStateDeregisterID(conn,
+ driver->domainEventState,
+ callbackID);
lxcDriverUnlock(driver);
return ret;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 10a289e..7423340 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -8180,10 +8180,11 @@ qemuDomainEventRegisterAny(virConnectPtr conn,
int ret;
qemuDriverLock(driver);
- ret = virDomainEventCallbackListAddID(conn,
- driver->domainEventState->callbacks,
- dom, eventID,
- callback, opaque, freecb);
+ if (virDomainEventCallbackListAddID(conn,
+ driver->domainEventState->callbacks,
+ dom, eventID,
+ callback, opaque, freecb, &ret) < 0)
+ ret = -1;
qemuDriverUnlock(driver);
return ret;
@@ -8198,9 +8199,9 @@ qemuDomainEventDeregisterAny(virConnectPtr conn,
int ret;
qemuDriverLock(driver);
- ret = virDomainEventStateDeregisterAny(conn,
- driver->domainEventState,
- callbackID);
+ ret = virDomainEventStateDeregisterID(conn,
+ driver->domainEventState,
+ callbackID);
qemuDriverUnlock(driver);
return ret;
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index ff2d4b4..a0cf7d3 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -3120,6 +3120,7 @@ static int remoteDomainEventRegister(virConnectPtr conn,
{
int rv = -1;
struct private_data *priv = conn->privateData;
+ int count;
remoteDriverLock(priv);
@@ -3128,15 +3129,13 @@ static int remoteDomainEventRegister(virConnectPtr conn,
goto done;
}
- if (virDomainEventCallbackListAdd(conn, priv->domainEventState->callbacks,
- callback, opaque, freecb) < 0) {
+ if ((count = virDomainEventCallbackListAdd(conn,
priv->domainEventState->callbacks,
+ callback, opaque, freecb)) < 0) {
remoteError(VIR_ERR_RPC, "%s", _("adding cb to list"));
goto done;
}
- if (virDomainEventCallbackListCountID(conn,
- priv->domainEventState->callbacks,
- VIR_DOMAIN_EVENT_ID_LIFECYCLE) == 1) {
+ if (count == 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,
@@ -3760,6 +3759,7 @@ static int remoteDomainEventRegisterAny(virConnectPtr conn,
struct private_data *priv = conn->privateData;
remote_domain_events_register_any_args args;
int callbackID;
+ int count;
remoteDriverLock(priv);
@@ -3768,19 +3768,18 @@ static int remoteDomainEventRegisterAny(virConnectPtr conn,
goto done;
}
- if ((callbackID = virDomainEventCallbackListAddID(conn,
-
priv->domainEventState->callbacks,
- dom, eventID,
- callback, opaque, freecb)) < 0)
{
- remoteError(VIR_ERR_RPC, "%s", _("adding cb to list"));
- goto done;
+ if ((count = virDomainEventCallbackListAddID(conn,
+
priv->domainEventState->callbacks,
+ dom, eventID,
+ callback, opaque, freecb,
+ &callbackID)) < 0) {
+ remoteError(VIR_ERR_RPC, "%s", _("adding cb to list"));
+ goto done;
}
/* If this is the first callback for this eventID, we need to enable
* events on the server */
- if (virDomainEventCallbackListCountID(conn,
- priv->domainEventState->callbacks,
- eventID) == 1) {
+ if (count == 1) {
args.eventID = eventID;
if (call (conn, priv, 0, REMOTE_PROC_DOMAIN_EVENTS_REGISTER_ANY,
@@ -3818,9 +3817,9 @@ static int remoteDomainEventDeregisterAny(virConnectPtr conn,
goto done;
}
- if (virDomainEventStateDeregisterAny(conn,
- priv->domainEventState,
- callbackID) < 0)
+ if (virDomainEventStateDeregisterID(conn,
+ priv->domainEventState,
+ callbackID) < 0)
goto done;
/* If that was the last callback for this eventID, we need to disable
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 89f7df1..f32990b 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -5469,10 +5469,11 @@ testDomainEventRegisterAny(virConnectPtr conn,
int ret;
testDriverLock(driver);
- ret = virDomainEventCallbackListAddID(conn,
- driver->domainEventState->callbacks,
- dom, eventID,
- callback, opaque, freecb);
+ if (virDomainEventCallbackListAddID(conn,
+ driver->domainEventState->callbacks,
+ dom, eventID,
+ callback, opaque, freecb, &ret) < 0)
+ ret = -1;
testDriverUnlock(driver);
return ret;
@@ -5486,9 +5487,9 @@ testDomainEventDeregisterAny(virConnectPtr conn,
int ret;
testDriverLock(driver);
- ret = virDomainEventStateDeregisterAny(conn,
- driver->domainEventState,
- callbackID);
+ ret = virDomainEventStateDeregisterID(conn,
+ driver->domainEventState,
+ callbackID);
testDriverUnlock(driver);
return ret;
diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c
index faea313..420488d 100644
--- a/src/uml/uml_driver.c
+++ b/src/uml/uml_driver.c
@@ -2483,10 +2483,11 @@ umlDomainEventRegisterAny(virConnectPtr conn,
int ret;
umlDriverLock(driver);
- ret = virDomainEventCallbackListAddID(conn,
- driver->domainEventState->callbacks,
- dom, eventID,
- callback, opaque, freecb);
+ if (virDomainEventCallbackListAddID(conn,
+ driver->domainEventState->callbacks,
+ dom, eventID,
+ callback, opaque, freecb, &ret) < 0)
+ ret = -1;
umlDriverUnlock(driver);
return ret;
@@ -2501,9 +2502,9 @@ umlDomainEventDeregisterAny(virConnectPtr conn,
int ret;
umlDriverLock(driver);
- ret = virDomainEventStateDeregisterAny(conn,
- driver->domainEventState,
- callbackID);
+ ret = virDomainEventStateDeregisterID(conn,
+ driver->domainEventState,
+ callbackID);
umlDriverUnlock(driver);
return ret;
diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index 7f13f90..670de9c 100644
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -7280,13 +7280,14 @@ static int vboxDomainEventRegisterAny(virConnectPtr conn,
* later you can iterate over them
*/
- ret = virDomainEventCallbackListAddID(conn,
data->domainEvents->callbacks,
- dom, eventID,
- callback, opaque, freecb);
+ if (virDomainEventCallbackListAddID(conn,
data->domainEvents->callbacks,
+ dom, eventID,
+ callback, opaque, freecb, &ret) <
0)
+ ret = -1;
VIR_DEBUG("virDomainEventCallbackListAddID (ret = %d) ( conn: %p,
"
- "data->domainEvents->callbacks: %p, callback: %p, opaque:
%p, "
- "freecb: %p )", ret, conn,
data->domainEvents->callbacks, callback,
- opaque, freecb);
+ "data->domainEvents->callbacks: %p, callback: %p,
opaque: %p, "
+ "freecb: %p )", ret, conn,
data->domainEvents->callbacks, callback,
+ opaque, freecb);
}
}
@@ -7312,7 +7313,7 @@ static int vboxDomainEventDeregisterAny(virConnectPtr conn,
*/
vboxDriverLock(data);
- cnt = virDomainEventStateDeregisterAny(conn, data->domainEvents,
+ cnt = virDomainEventStateDeregisterID(conn, data->domainEvents,
callbackID);
if (data->vboxCallback && cnt == 0) {
diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
index 8c47ad5..b11dd9e 100644
--- a/src/xen/xen_driver.c
+++ b/src/xen/xen_driver.c
@@ -1897,9 +1897,10 @@ xenUnifiedDomainEventRegisterAny(virConnectPtr conn,
return -1;
}
- ret = virDomainEventCallbackListAddID(conn, priv->domainEvents->callbacks,
- dom, eventID,
- callback, opaque, freefunc);
+ if (virDomainEventCallbackListAddID(conn, priv->domainEvents->callbacks,
+ dom, eventID,
+ callback, opaque, freefunc, &ret) < 0)
+ ret = -1;
xenUnifiedUnlock(priv);
return (ret);
@@ -1919,9 +1920,9 @@ xenUnifiedDomainEventDeregisterAny(virConnectPtr conn,
return -1;
}
- ret = virDomainEventStateDeregisterAny(conn,
- priv->domainEvents,
- callbackID);
+ ret = virDomainEventStateDeregisterID(conn,
+ priv->domainEvents,
+ callbackID);
xenUnifiedUnlock(priv);
return ret;
--
1.7.7.4
12:58 p.m.
New subject: [libvirt] [PATCH 2/8] Return count of callbacks when registering callbacks
On 12/13/2011 05:38 PM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange(a)redhat.com>
When registering a callback for a particular event some callers
need to know how many callbacks already exist for that event.
While it is possible to ask for a count, this is not free from
race conditions when threaded. Thus the API for registering
callbacks should return the count of callbacks
I think this needs to swap places with the previous patch, and pick up
the hunks that I questioned there.
* src/conf/domain_event.c, src/conf/domain_event.h,
src/libvirt_private.syms: Return count of callbacks when
registering callbacks
* src/libxl/libxl_driver.c, src/libxl/libxl_driver.c,
src/qemu/qemu_driver.c, src/remote/remote_driver.c,
src/remote/remote_driver.c, src/uml/uml_driver.c,
src/vbox/vbox_tmpl.c, src/xen/xen_driver.c: Update
for change in APIs
The changes themselves make sense. I had a minor merge conflict with my
own resolution to the compile errors left over in 1/8 and this patch,
due to an indentation problem.
---
src/conf/domain_event.c | 23 ++++++++++++++++++-----
src/conf/domain_event.h | 3 ++-
src/libvirt_private.syms | 2 +-
src/libxl/libxl_driver.c | 15 ++++++++-------
src/lxc/lxc_driver.c | 15 ++++++++-------
src/qemu/qemu_driver.c | 15 ++++++++-------
src/remote/remote_driver.c | 33 ++++++++++++++++-----------------
src/test/test_driver.c | 15 ++++++++-------
src/uml/uml_driver.c | 15 ++++++++-------
src/vbox/vbox_tmpl.c | 15 ++++++++-------
src/xen/xen_driver.c | 13 +++++++------
11 files changed, 92 insertions(+), 72 deletions(-)
+++ b/src/libxl/libxl_driver.c
@@ -3851,10 +3851,11 @@ libxlDomainEventRegisterAny(virConnectPtr conn, virDomainPtr dom,
int eventID,
int ret;
libxlDriverLock(driver);
- ret = virDomainEventCallbackListAddID(conn,
- driver->domainEventState->callbacks,
- dom, eventID, callback, opaque,
- freecb);
+ if (virDomainEventCallbackListAddID(conn,
+ driver->domainEventState->callbacks,
+ dom, eventID, callback, opaque,
+ freecb, &ret) < 0)
+ ret = -1;
Should we make virDomainEventCallbackListAddID guarantee that the
callbackID argument, if non-NULL, is set to -1 on failure?
@@ -7312,7 +7313,7 @@ static int
vboxDomainEventDeregisterAny(virConnectPtr conn,
*/
vboxDriverLock(data);
- cnt = virDomainEventStateDeregisterAny(conn, data->domainEvents,
+ cnt = virDomainEventStateDeregisterID(conn, data->domainEvents,
callbackID);
Indentation.
ACK.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
6:38 p.m.
New subject: [libvirt] [PATCH 3/8] Add APIs to allow management of callbacks purely with virDomainEventState
From: "Daniel P. Berrange" <berrange(a)redhat.com>
While virDomainEventState has APIs for managing removal of callbacks,
while locked, adding callbacks in the first place requires direct
access to the virDomainEventCallbackList structure. This is not
threadsafe since it is bypassing the virDomainEventState locks
* src/conf/domain_event.c, src/conf/domain_event.h,
src/libvirt_private.syms: Add APIs for managing callbacks
via virDomainEventState.
---
src/conf/domain_event.c | 99 ++++++++++++++++++++++++++++++++++++++++++++++
src/conf/domain_event.h | 24 +++++++++++
src/libvirt_private.syms | 4 ++
3 files changed, 127 insertions(+), 0 deletions(-)
diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c
index 00c5dbf..c6adae4 100644
--- a/src/conf/domain_event.c
+++ b/src/conf/domain_event.c
@@ -1342,6 +1342,68 @@ virDomainEventStateFlush(virDomainEventStatePtr state,
/**
+ * virDomainEventStateRegister:
+ * @state: domain event state
+ * @conn: connection to associate with callback
+ * @callback: function to remove from event
+ * @opaque: data blob to pass to callback
+ * @freecb: callback to free @opaque
+ *
+ * Register the function @callback with connection @conn,
+ * from @state, for lifecycle events.
+ *
+ * Returns: the number of lifecycle callbacks now registered, or -1 on error
+ */
+int virDomainEventStateRegister(virConnectPtr conn,
+ virDomainEventStatePtr state,
+ virConnectDomainEventCallback callback,
+ void *opaque,
+ virFreeCallback freecb)
+{
+ int ret;
+ virDomainEventStateLock(state);
+ ret = virDomainEventCallbackListAdd(conn, state->callbacks,
+ callback, opaque, freecb);
+ virDomainEventStateUnlock(state);
+ return ret;
+}
+
+
+/**
+ * virDomainEventStateRegisterID:
+ * @state: domain event state
+ * @conn: connection to associate with callback
+ * @eventID: ID of the event type to register for
+ * @cb: function to remove from event
+ * @opaque: data blob to pass to callback
+ * @freecb: callback to free @opaque
+ * @callbackID: filled with callback ID
+ *
+ * Register the function @callbackID with connection @conn,
+ * from @state, for events of type @eventID.
+ *
+ * Returns: the number of callbacks now registered, or -1 on error
+ */
+int virDomainEventStateRegisterID(virConnectPtr conn,
+ virDomainEventStatePtr state,
+ virDomainPtr dom,
+ int eventID,
+ virConnectDomainEventGenericCallback cb,
+ void *opaque,
+ virFreeCallback freecb,
+ int *callbackID)
+{
+ int ret;
+ virDomainEventStateLock(state);
+ ret = virDomainEventCallbackListAddID(conn, state->callbacks,
+ dom, eventID, cb, opaque, freecb,
+ callbackID);
+ virDomainEventStateUnlock(state);
+ return ret;
+}
+
+
+/**
* virDomainEventStateDeregister:
* @state: domain event state
* @conn: connection to associate with callback
@@ -1398,3 +1460,40 @@ virDomainEventStateDeregisterID(virConnectPtr conn,
virDomainEventStateUnlock(state);
return ret;
}
+
+
+/**
+ * virDomainEventStateDeregisterConn:
+ * @state: domain event state
+ * @conn: connection to associate with callbacks
+ *
+ * Remove all callbacks from @state associated with the
+ * connection @conn
+ *
+ * Returns 0 on success, -1 on error
+ */
+int
+virDomainEventStateDeregisterConn(virConnectPtr conn,
+ virDomainEventStatePtr state)
+{
+ int ret;
+ virDomainEventStateLock(state);
+ ret = virDomainEventCallbackListRemoveConn(conn, state->callbacks);
+ virDomainEventStateUnlock(state);
+ return ret;
+}
+
+
+int
+virDomainEventStateEventID(virConnectPtr conn,
+ virDomainEventStatePtr state,
+ int callbackID)
+{
+ int ret;
+
+ virDomainEventStateLock(state);
+ ret = virDomainEventCallbackListEventID(conn,
+ state->callbacks, callbackID);
+ virDomainEventStateUnlock(state);
+ return ret;
+}
diff --git a/src/conf/domain_event.h b/src/conf/domain_event.h
index 83656e6..7eefadb 100644
--- a/src/conf/domain_event.h
+++ b/src/conf/domain_event.h
@@ -237,6 +237,21 @@ virDomainEventStateFlush(virDomainEventStatePtr state,
virDomainEventDispatchFunc dispatchFunc,
void *opaque)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+int virDomainEventStateRegister(virConnectPtr conn,
+ virDomainEventStatePtr state,
+ virConnectDomainEventCallback callback,
+ void *opaque,
+ virFreeCallback freecb)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
+int virDomainEventStateRegisterID(virConnectPtr conn,
+ virDomainEventStatePtr state,
+ virDomainPtr dom,
+ int eventID,
+ virConnectDomainEventGenericCallback cb,
+ void *opaque,
+ virFreeCallback freecb,
+ int *callbackID)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(5);
int
virDomainEventStateDeregister(virConnectPtr conn,
virDomainEventStatePtr state,
@@ -247,5 +262,14 @@ virDomainEventStateDeregisterID(virConnectPtr conn,
virDomainEventStatePtr state,
int callbackID)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+int
+virDomainEventStateDeregisterConn(virConnectPtr conn,
+ virDomainEventStatePtr state)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+int
+virDomainEventStateEventID(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 3d1d7d2..0c664ed 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -511,6 +511,10 @@ virDomainEventRebootNewFromDom;
virDomainEventRebootNewFromObj;
virDomainEventStateDeregister;
virDomainEventStateDeregisterID;
+virDomainEventStateDeregisterConn;
+virDomainEventStateEventID;
+virDomainEventStateRegister;
+virDomainEventStateRegisterID;
virDomainEventStateFlush;
virDomainEventStateFree;
virDomainEventStateNew;
--
1.7.7.4
1:56 p.m.
New subject: [libvirt] [PATCH 3/8] Add APIs to allow management of callbacks purely with virDomainEventState
On 12/13/2011 05:38 PM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange(a)redhat.com>
While virDomainEventState has APIs for managing removal of callbacks,
while locked, adding callbacks in the first place requires direct
access to the virDomainEventCallbackList structure. This is not
threadsafe since it is bypassing the virDomainEventState locks
* src/conf/domain_event.c, src/conf/domain_event.h,
src/libvirt_private.syms: Add APIs for managing callbacks
via virDomainEventState.
---
src/conf/domain_event.c | 99 ++++++++++++++++++++++++++++++++++++++++++++++
src/conf/domain_event.h | 24 +++++++++++
src/libvirt_private.syms | 4 ++
3 files changed, 127 insertions(+), 0 deletions(-)
/**
+ * virDomainEventStateRegister:
+ * @state: domain event state
+ * @conn: connection to associate with callback
This order...
+ * @callback: function to remove from event
+ * @opaque: data blob to pass to callback
+ * @freecb: callback to free @opaque
+ *
+ * Register the function @callback with connection @conn,
+ * from @state, for lifecycle events.
+ *
+ * Returns: the number of lifecycle callbacks now registered, or -1 on error
+ */
+int virDomainEventStateRegister(virConnectPtr conn,
+ virDomainEventStatePtr state,
...doesn't match this.
+/**
+ * virDomainEventStateRegisterID:
+ * @state: domain event state
+ * @conn: connection to associate with callback
Again ordering doesn't match. And you missed @dom.
+/**
+ * virDomainEventStateDeregisterConn:
+ * @state: domain event state
+ * @conn: connection to associate with callbacks
and again.
+
+
+int
+virDomainEventStateEventID(virConnectPtr conn,
+ virDomainEventStatePtr state,
+ int callbackID)
No doc comments?
+++ b/src/libvirt_private.syms
@@ -511,6 +511,10 @@ virDomainEventRebootNewFromDom;
virDomainEventRebootNewFromObj;
virDomainEventStateDeregister;
virDomainEventStateDeregisterID;
+virDomainEventStateDeregisterConn;
Not quite sorted :)
But all my findings are trivial, so ACK with nits fixed.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
6:38 p.m.
New subject: [libvirt] [PATCH 4/8] Convert drivers to thread safe APIs for adding callbacks
From: "Daniel P. Berrange" <berrange(a)redhat.com>
* src/libxl/libxl_driver.c, src/lxc/lxc_driver.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: Convert
to threadsafe APIs
---
src/libxl/libxl_driver.c | 18 ++++++------
src/lxc/lxc_driver.c | 18 ++++++------
src/qemu/qemu_driver.c | 18 ++++++------
src/remote/remote_driver.c | 64 ++++++++++++++++++-------------------------
src/test/test_driver.c | 14 +++++-----
src/uml/uml_driver.c | 18 ++++++------
src/vbox/vbox_tmpl.c | 24 ++++++++--------
src/xen/xen_driver.c | 10 +++---
8 files changed, 87 insertions(+), 97 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 46504dc..b9382ee 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -1115,8 +1115,8 @@ libxlClose(virConnectPtr conn ATTRIBUTE_UNUSED)
libxlDriverPrivatePtr driver = conn->privateData;
libxlDriverLock(driver);
- virDomainEventCallbackListRemoveConn(conn,
- driver->domainEventState->callbacks);
+ virDomainEventStateDeregisterConn(conn,
+ driver->domainEventState);
libxlDriverUnlock(driver);
conn->privateData = NULL;
return 0;
@@ -3404,9 +3404,9 @@ libxlDomainEventRegister(virConnectPtr conn,
int ret;
libxlDriverLock(driver);
- ret = virDomainEventCallbackListAdd(conn,
- driver->domainEventState->callbacks,
- callback, opaque, freecb);
+ ret = virDomainEventStateRegister(conn,
+ driver->domainEventState,
+ callback, opaque, freecb);
libxlDriverUnlock(driver);
return ret;
@@ -3851,10 +3851,10 @@ libxlDomainEventRegisterAny(virConnectPtr conn, virDomainPtr dom,
int eventID,
int ret;
libxlDriverLock(driver);
- if (virDomainEventCallbackListAddID(conn,
- driver->domainEventState->callbacks,
- dom, eventID, callback, opaque,
- freecb, &ret) < 0)
+ if (virDomainEventStateRegisterID(conn,
+ driver->domainEventState,
+ dom, eventID, callback, opaque,
+ freecb, &ret) < 0)
ret = -1;
libxlDriverUnlock(driver);
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 6b17c75..6a9ebde 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -179,8 +179,8 @@ static int lxcClose(virConnectPtr conn)
lxc_driver_t *driver = conn->privateData;
lxcDriverLock(driver);
- virDomainEventCallbackListRemoveConn(conn,
- driver->domainEventState->callbacks);
+ virDomainEventStateDeregisterConn(conn,
+ driver->domainEventState);
lxcProcessAutoDestroyRun(driver, conn);
lxcDriverUnlock(driver);
@@ -2126,9 +2126,9 @@ lxcDomainEventRegister(virConnectPtr conn,
int ret;
lxcDriverLock(driver);
- ret = virDomainEventCallbackListAdd(conn,
- driver->domainEventState->callbacks,
- callback, opaque, freecb);
+ ret = virDomainEventStateRegister(conn,
+ driver->domainEventState,
+ callback, opaque, freecb);
lxcDriverUnlock(driver);
return ret;
@@ -2164,10 +2164,10 @@ lxcDomainEventRegisterAny(virConnectPtr conn,
int ret;
lxcDriverLock(driver);
- if (virDomainEventCallbackListAddID(conn,
- driver->domainEventState->callbacks,
- dom, eventID,
- callback, opaque, freecb, &ret) < 0)
+ if (virDomainEventStateRegisterID(conn,
+ driver->domainEventState,
+ dom, eventID,
+ callback, opaque, freecb, &ret) < 0)
ret = -1;
lxcDriverUnlock(driver);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 7423340..40bd30f 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -900,8 +900,8 @@ static int qemudClose(virConnectPtr conn) {
/* Get rid of callbacks registered for this conn */
qemuDriverLock(driver);
- virDomainEventCallbackListRemoveConn(conn,
- driver->domainEventState->callbacks);
+ virDomainEventStateDeregisterConn(conn,
+ driver->domainEventState);
qemuProcessAutoDestroyRun(driver, conn);
qemuDriverUnlock(driver);
@@ -8142,9 +8142,9 @@ qemuDomainEventRegister(virConnectPtr conn,
int ret;
qemuDriverLock(driver);
- ret = virDomainEventCallbackListAdd(conn,
- driver->domainEventState->callbacks,
- callback, opaque, freecb);
+ ret = virDomainEventStateRegister(conn,
+ driver->domainEventState,
+ callback, opaque, freecb);
qemuDriverUnlock(driver);
return ret;
@@ -8180,10 +8180,10 @@ qemuDomainEventRegisterAny(virConnectPtr conn,
int ret;
qemuDriverLock(driver);
- if (virDomainEventCallbackListAddID(conn,
- driver->domainEventState->callbacks,
- dom, eventID,
- callback, opaque, freecb, &ret) < 0)
+ if (virDomainEventStateRegisterID(conn,
+ driver->domainEventState,
+ dom, eventID,
+ callback, opaque, freecb, &ret) < 0)
ret = -1;
qemuDriverUnlock(driver);
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index a0cf7d3..a10bcad 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -3124,13 +3124,8 @@ static int remoteDomainEventRegister(virConnectPtr conn,
remoteDriverLock(priv);
- if (priv->domainEventState->timer < 0) {
- remoteError(VIR_ERR_NO_SUPPORT, "%s", _("no event
support"));
- goto done;
- }
-
- if ((count = virDomainEventCallbackListAdd(conn,
priv->domainEventState->callbacks,
- callback, opaque, freecb)) < 0) {
+ if ((count = virDomainEventStateRegister(conn, priv->domainEventState,
+ callback, opaque, freecb)) < 0) {
remoteError(VIR_ERR_RPC, "%s", _("adding cb to list"));
goto done;
}
@@ -3155,17 +3150,16 @@ static int remoteDomainEventDeregister(virConnectPtr conn,
{
struct private_data *priv = conn->privateData;
int rv = -1;
+ int count;
remoteDriverLock(priv);
- if (virDomainEventStateDeregister(conn,
- priv->domainEventState,
- callback) < 0)
+ if ((count = virDomainEventStateDeregister(conn,
+ priv->domainEventState,
+ callback)) < 0)
goto done;
- if (virDomainEventCallbackListCountID(conn,
- priv->domainEventState->callbacks,
- VIR_DOMAIN_EVENT_ID_LIFECYCLE) == 0) {
+ if (count == 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,
@@ -3763,20 +3757,15 @@ static int remoteDomainEventRegisterAny(virConnectPtr conn,
remoteDriverLock(priv);
- if (priv->domainEventState->timer < 0) {
- remoteError(VIR_ERR_NO_SUPPORT, "%s", _("no event
support"));
+ if ((count = virDomainEventStateRegisterID(conn,
+ priv->domainEventState,
+ dom, eventID,
+ callback, opaque, freecb,
+ &callbackID)) < 0) {
+ remoteError(VIR_ERR_RPC, "%s", _("adding cb to list"));
goto done;
}
- if ((count = virDomainEventCallbackListAddID(conn,
-
priv->domainEventState->callbacks,
- dom, eventID,
- callback, opaque, freecb,
- &callbackID)) < 0) {
- remoteError(VIR_ERR_RPC, "%s", _("adding cb to list"));
- goto done;
- }
-
/* If this is the first callback for this eventID, we need to enable
* events on the server */
if (count == 1) {
@@ -3785,9 +3774,9 @@ static int remoteDomainEventRegisterAny(virConnectPtr conn,
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->domainEventState->callbacks,
- callbackID);
+ virDomainEventStateDeregisterID(conn,
+ priv->domainEventState,
+ callbackID);
goto done;
}
}
@@ -3807,27 +3796,28 @@ static int remoteDomainEventDeregisterAny(virConnectPtr conn,
int rv = -1;
remote_domain_events_deregister_any_args args;
int eventID;
+ int count;
remoteDriverLock(priv);
- if ((eventID = virDomainEventCallbackListEventID(conn,
-
priv->domainEventState->callbacks,
- callbackID)) < 0) {
+ if ((eventID = virDomainEventStateEventID(conn,
+ priv->domainEventState,
+ callbackID)) < 0) {
remoteError(VIR_ERR_RPC, _("unable to find callback ID %d"),
callbackID);
goto done;
}
- if (virDomainEventStateDeregisterID(conn,
- priv->domainEventState,
- callbackID) < 0)
+ if ((count = virDomainEventStateDeregisterID(conn,
+ priv->domainEventState,
+ callbackID)) < 0) {
+ remoteError(VIR_ERR_RPC, _("unable to find callback ID %d"),
callbackID);
goto done;
+ }
/* If that was the last callback for this eventID, we need to disable
* events on the server */
- if (virDomainEventCallbackListCountID(conn,
- priv->domainEventState->callbacks,
- eventID) == 0) {
- args.eventID = eventID;
+ if (count == 0) {
+ args.eventID = callbackID;
if (call (conn, priv, 0, REMOTE_PROC_DOMAIN_EVENTS_DEREGISTER_ANY,
(xdrproc_t) xdr_remote_domain_events_deregister_any_args, (char *)
&args,
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index f32990b..054a41a 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -5431,9 +5431,9 @@ testDomainEventRegister(virConnectPtr conn,
int ret;
testDriverLock(driver);
- ret = virDomainEventCallbackListAdd(conn,
- driver->domainEventState->callbacks,
- callback, opaque, freecb);
+ ret = virDomainEventStateRegister(conn,
+ driver->domainEventState,
+ callback, opaque, freecb);
testDriverUnlock(driver);
return ret;
@@ -5469,10 +5469,10 @@ testDomainEventRegisterAny(virConnectPtr conn,
int ret;
testDriverLock(driver);
- if (virDomainEventCallbackListAddID(conn,
- driver->domainEventState->callbacks,
- dom, eventID,
- callback, opaque, freecb, &ret) < 0)
+ if (virDomainEventStateRegisterID(conn,
+ driver->domainEventState,
+ dom, eventID,
+ callback, opaque, freecb, &ret) < 0)
ret = -1;
testDriverUnlock(driver);
diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c
index 420488d..4d875c8 100644
--- a/src/uml/uml_driver.c
+++ b/src/uml/uml_driver.c
@@ -1194,8 +1194,8 @@ static int umlClose(virConnectPtr conn) {
struct uml_driver *driver = conn->privateData;
umlDriverLock(driver);
- virDomainEventCallbackListRemoveConn(conn,
- driver->domainEventState->callbacks);
+ virDomainEventStateDeregisterConn(conn,
+ driver->domainEventState);
umlProcessAutoDestroyRun(driver, conn);
umlDriverUnlock(driver);
@@ -2447,9 +2447,9 @@ umlDomainEventRegister(virConnectPtr conn,
int ret;
umlDriverLock(driver);
- ret = virDomainEventCallbackListAdd(conn,
- driver->domainEventState->callbacks,
- callback, opaque, freecb);
+ ret = virDomainEventStateRegister(conn,
+ driver->domainEventState,
+ callback, opaque, freecb);
umlDriverUnlock(driver);
return ret;
@@ -2483,10 +2483,10 @@ umlDomainEventRegisterAny(virConnectPtr conn,
int ret;
umlDriverLock(driver);
- if (virDomainEventCallbackListAddID(conn,
- driver->domainEventState->callbacks,
- dom, eventID,
- callback, opaque, freecb, &ret) < 0)
+ if (virDomainEventStateRegisterID(conn,
+ driver->domainEventState,
+ dom, eventID,
+ callback, opaque, freecb, &ret) < 0)
ret = -1;
umlDriverUnlock(driver);
diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index 670de9c..1fb369b 100644
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -7188,12 +7188,12 @@ static int vboxDomainEventRegister (virConnectPtr conn,
* later you can iterate over them
*/
- ret = virDomainEventCallbackListAdd(conn,
data->domainEvents->callbacks,
- callback, opaque, freecb);
- VIR_DEBUG("virDomainEventCallbackListAdd (ret = %d) ( conn: %p, "
- "data->domainEvents->callbacks: %p, callback: %p, opaque:
%p, "
- "freecb: %p )", ret, conn,
data->domainEvents->callbacks, callback,
- opaque, freecb);
+ ret = virDomainEventStateRegister(conn, data->domainEvents,
+ callback, opaque, freecb);
+ VIR_DEBUG("virDomainEventStateRegister (ret = %d) ( conn: %p, "
+ "callback: %p, opaque: %p, "
+ "freecb: %p )", ret, conn, callback,
+ opaque, freecb);
}
}
@@ -7280,13 +7280,13 @@ static int vboxDomainEventRegisterAny(virConnectPtr conn,
* later you can iterate over them
*/
- if (virDomainEventCallbackListAddID(conn,
data->domainEvents->callbacks,
- dom, eventID,
- callback, opaque, freecb, &ret) <
0)
+ if (virDomainEventStateRegisterID(conn, data->domainEvents,
+ dom, eventID,
+ callback, opaque, freecb, &ret) <
0)
ret = -1;
- VIR_DEBUG("virDomainEventCallbackListAddID (ret = %d) ( conn: %p,
"
- "data->domainEvents->callbacks: %p, callback: %p,
opaque: %p, "
- "freecb: %p )", ret, conn,
data->domainEvents->callbacks, callback,
+ VIR_DEBUG("virDomainEventStateRegisterID (ret = %d) ( conn: %p, "
+ "callback: %p, opaque: %p, "
+ "freecb: %p )", ret, conn, callback,
opaque, freecb);
}
}
diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
index b11dd9e..a2399dd 100644
--- a/src/xen/xen_driver.c
+++ b/src/xen/xen_driver.c
@@ -1847,8 +1847,8 @@ xenUnifiedDomainEventRegister(virConnectPtr conn,
return -1;
}
- ret = virDomainEventCallbackListAdd(conn, priv->domainEvents->callbacks,
- callback, opaque, freefunc);
+ ret = virDomainEventStateRegister(conn, priv->domainEvents,
+ callback, opaque, freefunc);
xenUnifiedUnlock(priv);
return (ret);
@@ -1897,9 +1897,9 @@ xenUnifiedDomainEventRegisterAny(virConnectPtr conn,
return -1;
}
- if (virDomainEventCallbackListAddID(conn, priv->domainEvents->callbacks,
- dom, eventID,
- callback, opaque, freefunc, &ret) < 0)
+ if (virDomainEventStateRegisterID(conn, priv->domainEvents,
+ dom, eventID,
+ callback, opaque, freefunc, &ret) < 0)
ret = -1;
xenUnifiedUnlock(priv);
--
1.7.7.4
2:15 p.m.
New subject: [libvirt] [PATCH 4/8] Convert drivers to thread safe APIs for adding callbacks
On 12/13/2011 05:38 PM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange(a)redhat.com>
* src/libxl/libxl_driver.c, src/lxc/lxc_driver.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: Convert
to threadsafe APIs
---
src/libxl/libxl_driver.c | 18 ++++++------
src/lxc/lxc_driver.c | 18 ++++++------
src/qemu/qemu_driver.c | 18 ++++++------
src/remote/remote_driver.c | 64 ++++++++++++++++++-------------------------
src/test/test_driver.c | 14 +++++-----
src/uml/uml_driver.c | 18 ++++++------
src/vbox/vbox_tmpl.c | 24 ++++++++--------
src/xen/xen_driver.c | 10 +++---
8 files changed, 87 insertions(+), 97 deletions(-)
Safer and slightly smaller - a nice mix :)
+++ b/src/remote/remote_driver.c
@@ -3124,13 +3124,8 @@ static int remoteDomainEventRegister(virConnectPtr conn,
remoteDriverLock(priv);
- if (priv->domainEventState->timer < 0) {
- remoteError(VIR_ERR_NO_SUPPORT, "%s", _("no event
support"));
- goto done;
- }
Does this hunk belong here, or in a later patch?
@@ -3763,20 +3757,15 @@ static int
remoteDomainEventRegisterAny(virConnectPtr conn,
remoteDriverLock(priv);
- if (priv->domainEventState->timer < 0) {
- remoteError(VIR_ERR_NO_SUPPORT, "%s", _("no event
support"));
Likewise.
+ if ((count = virDomainEventStateRegisterID(conn,
+ priv->domainEventState,
+ dom, eventID,
+ callback, opaque, freecb,
+ &callbackID)) < 0) {
+ remoteError(VIR_ERR_RPC, "%s", _("adding cb to list"));
goto done;
Indentation is off here (9 instead of 8 spaces, on both instances of
this error message).
ACK - the bulk of this patch is mechanical; and while I think you should
rebase the remote_driver.c stuff slightly differently, at least this one
compiled (unlike the rebase disaster on 1/8).
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
4:52 a.m.
New subject: [libvirt] [PATCH 4/8] Convert drivers to thread safe APIs for adding callbacks
On Wed, Dec 14, 2011 at 01:15:58PM -0700, Eric Blake wrote:
On 12/13/2011 05:38 PM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange(a)redhat.com>
>
> * src/libxl/libxl_driver.c, src/lxc/lxc_driver.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: Convert
> to threadsafe APIs
> ---
> src/libxl/libxl_driver.c | 18 ++++++------
> src/lxc/lxc_driver.c | 18 ++++++------
> src/qemu/qemu_driver.c | 18 ++++++------
> src/remote/remote_driver.c | 64 ++++++++++++++++++-------------------------
> src/test/test_driver.c | 14 +++++-----
> src/uml/uml_driver.c | 18 ++++++------
> src/vbox/vbox_tmpl.c | 24 ++++++++--------
> src/xen/xen_driver.c | 10 +++---
> 8 files changed, 87 insertions(+), 97 deletions(-)
Safer and slightly smaller - a nice mix :)
> +++ b/src/remote/remote_driver.c
> @@ -3124,13 +3124,8 @@ static int remoteDomainEventRegister(virConnectPtr conn,
>
> remoteDriverLock(priv);
>
> - if (priv->domainEventState->timer < 0) {
> - remoteError(VIR_ERR_NO_SUPPORT, "%s", _("no event
support"));
> - goto done;
> - }
Does this hunk belong here, or in a later patch?
The next patch removes the contents of virDomainEventState from
public view, so this has to be removed now. A completely different
solution to the problem appears in a later patch, because instead
of registering a timer upfront when virDomainEventStatePtr is
allocated, we will register the timer at first use, so can report
the error directly, instead of having a delayed report as the old
code did.
> + if ((count = virDomainEventStateRegisterID(conn,
> + priv->domainEventState,
> + dom, eventID,
> + callback, opaque, freecb,
> + &callbackID)) < 0) {
> + remoteError(VIR_ERR_RPC, "%s", _("adding cb to
list"));
> goto done;
Indentation is off here (9 instead of 8 spaces, on both instances of
this error message).
ACK - the bulk of this patch is mechanical; and while I think you should
rebase the remote_driver.c stuff slightly differently, at least this one
compiled (unlike the rebase disaster on 1/8).
Regards,
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 :|
6:38 p.m.
New subject: [libvirt] [PATCH 5/8] Remove all domain event structs from header
From: "Daniel P. Berrange" <berrange(a)redhat.com>
No caller of the domain events APIs should need to poke at the
struct internals. Thus they should all be removed from the
header file
* src/conf/domain_event.h: Remove struct definitions
* src/conf/domain_event.c: Add struct definitions
---
src/conf/domain_event.c | 23 +++++++++++++++++++++++
src/conf/domain_event.h | 20 --------------------
2 files changed, 23 insertions(+), 20 deletions(-)
diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c
index c6adae4..bd86bde 100644
--- a/src/conf/domain_event.c
+++ b/src/conf/domain_event.c
@@ -43,6 +43,29 @@ struct _virDomainMeta {
typedef struct _virDomainMeta virDomainMeta;
typedef virDomainMeta *virDomainMetaPtr;
+struct _virDomainEventCallbackList {
+ unsigned int nextID;
+ unsigned int count;
+ virDomainEventCallbackPtr *callbacks;
+};
+
+struct _virDomainEventQueue {
+ unsigned int count;
+ virDomainEventPtr *events;
+};
+
+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;
+ virMutex lock;
+};
+
struct _virDomainEventCallback {
int callbackID;
int eventID;
diff --git a/src/conf/domain_event.h b/src/conf/domain_event.h
index 7eefadb..1845679 100644
--- a/src/conf/domain_event.h
+++ b/src/conf/domain_event.h
@@ -30,11 +30,6 @@
typedef struct _virDomainEventCallback virDomainEventCallback;
typedef virDomainEventCallback *virDomainEventCallbackPtr;
-struct _virDomainEventCallbackList {
- unsigned int nextID;
- unsigned int count;
- virDomainEventCallbackPtr *callbacks;
-};
typedef struct _virDomainEventCallbackList virDomainEventCallbackList;
typedef virDomainEventCallbackList *virDomainEventCallbackListPtr;
@@ -45,24 +40,9 @@ typedef virDomainEventCallbackList *virDomainEventCallbackListPtr;
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;
- virMutex lock;
-};
typedef struct _virDomainEventState virDomainEventState;
typedef virDomainEventState *virDomainEventStatePtr;
--
1.7.7.4
2:27 p.m.
New subject: [libvirt] [PATCH 5/8] Remove all domain event structs from header
On 12/13/2011 05:38 PM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange(a)redhat.com>
No caller of the domain events APIs should need to poke at the
struct internals. Thus they should all be removed from the
header file
* src/conf/domain_event.h: Remove struct definitions
* src/conf/domain_event.c: Add struct definitions
---
src/conf/domain_event.c | 23 +++++++++++++++++++++++
src/conf/domain_event.h | 20 --------------------
2 files changed, 23 insertions(+), 20 deletions(-)
ACK.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
6:38 p.m.
New subject: [libvirt] [PATCH 6/8] Remove decl of all APIs related to domain event callbacks & queues
From: "Daniel P. Berrange" <berrange(a)redhat.com>
The virDomainEventCallbackList and virDomainEventQueue APIs are
now solely helpers used internally by virDomainEventState APIs.
Remove their decls from domain_event.h since no driver code should
need to use them any more.
* src/conf/domain_event.c: Make virDomainEventCallbackList and
virDomainEventQueue APIs static & remove some unused APIs
* src/conf/domain_event.h, src/libvirt_private.syms: Remove
virDomainEventCallbackList and virDomainEventQueue APIs
---
src/conf/domain_event.c | 151 +++++++++++++---------------------------------
src/conf/domain_event.h | 64 +-------------------
src/libvirt_private.syms | 17 -----
3 files changed, 44 insertions(+), 188 deletions(-)
diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c
index bd86bde..bd305ec 100644
--- a/src/conf/domain_event.c
+++ b/src/conf/domain_event.c
@@ -126,7 +126,7 @@ struct _virDomainEvent {
*
* Free the memory in the domain event callback list
*/
-void
+static void
virDomainEventCallbackListFree(virDomainEventCallbackListPtr list)
{
int i;
@@ -151,7 +151,7 @@ virDomainEventCallbackListFree(virDomainEventCallbackListPtr list)
*
* Internal function to remove a callback from a virDomainEventCallbackListPtr
*/
-int
+static int
virDomainEventCallbackListRemove(virConnectPtr conn,
virDomainEventCallbackListPtr cbList,
virConnectDomainEventCallback callback)
@@ -202,7 +202,7 @@ virDomainEventCallbackListRemove(virConnectPtr conn,
*
* Internal function to remove a callback from a virDomainEventCallbackListPtr
*/
-int
+static int
virDomainEventCallbackListRemoveID(virConnectPtr conn,
virDomainEventCallbackListPtr cbList,
int callbackID)
@@ -252,7 +252,7 @@ virDomainEventCallbackListRemoveID(virConnectPtr conn,
* Internal function to remove all of a given connection's callback
* from a virDomainEventCallbackListPtr
*/
-int
+static int
virDomainEventCallbackListRemoveConn(virConnectPtr conn,
virDomainEventCallbackListPtr cbList)
{
@@ -283,9 +283,10 @@ virDomainEventCallbackListRemoveConn(virConnectPtr conn,
}
-int virDomainEventCallbackListMarkDelete(virConnectPtr conn,
- virDomainEventCallbackListPtr cbList,
- virConnectDomainEventCallback callback)
+static int
+virDomainEventCallbackListMarkDelete(virConnectPtr conn,
+ virDomainEventCallbackListPtr cbList,
+ virConnectDomainEventCallback callback)
{
int ret = 0;
int i;
@@ -308,9 +309,10 @@ int virDomainEventCallbackListMarkDelete(virConnectPtr conn,
}
-int virDomainEventCallbackListMarkDeleteID(virConnectPtr conn,
- virDomainEventCallbackListPtr cbList,
- int callbackID)
+static int
+virDomainEventCallbackListMarkDeleteID(virConnectPtr conn,
+ virDomainEventCallbackListPtr cbList,
+ int callbackID)
{
int ret = 0;
int i;
@@ -332,7 +334,8 @@ int virDomainEventCallbackListMarkDeleteID(virConnectPtr conn,
}
-int virDomainEventCallbackListPurgeMarked(virDomainEventCallbackListPtr cbList)
+static int
+virDomainEventCallbackListPurgeMarked(virDomainEventCallbackListPtr cbList)
{
int old_count = cbList->count;
int i;
@@ -362,29 +365,6 @@ int
virDomainEventCallbackListPurgeMarked(virDomainEventCallbackListPtr cbList)
/**
- * virDomainEventCallbackListAdd:
- * @conn: pointer to the connection
- * @cbList: the list
- * @callback: the callback to add
- * @opaque: opaque data tio pass to callback
- *
- * Internal function to add a callback from a virDomainEventCallbackListPtr
- */
-int
-virDomainEventCallbackListAdd(virConnectPtr conn,
- virDomainEventCallbackListPtr cbList,
- virConnectDomainEventCallback callback,
- void *opaque,
- virFreeCallback freecb)
-{
- return virDomainEventCallbackListAddID(conn, cbList, NULL,
- VIR_DOMAIN_EVENT_ID_LIFECYCLE,
- VIR_DOMAIN_EVENT_CALLBACK(callback),
- opaque, freecb, NULL);
-}
-
-
-/**
* virDomainEventCallbackListAddID:
* @conn: pointer to the connection
* @cbList: the list
@@ -395,7 +375,7 @@ virDomainEventCallbackListAdd(virConnectPtr conn,
*
* Internal function to add a callback from a virDomainEventCallbackListPtr
*/
-int
+static int
virDomainEventCallbackListAddID(virConnectPtr conn,
virDomainEventCallbackListPtr cbList,
virDomainPtr dom,
@@ -478,29 +458,34 @@ no_memory:
}
-int virDomainEventCallbackListCountID(virConnectPtr conn,
- virDomainEventCallbackListPtr cbList,
- int eventID)
+/**
+ * virDomainEventCallbackListAdd:
+ * @conn: pointer to the connection
+ * @cbList: the list
+ * @callback: the callback to add
+ * @opaque: opaque data tio pass to callback
+ *
+ * Internal function to add a callback from a virDomainEventCallbackListPtr
+ */
+static int
+virDomainEventCallbackListAdd(virConnectPtr conn,
+ virDomainEventCallbackListPtr cbList,
+ virConnectDomainEventCallback callback,
+ void *opaque,
+ virFreeCallback freecb)
{
- int i;
- int count = 0;
-
- for (i = 0 ; i < cbList->count ; i++) {
- if (cbList->callbacks[i]->deleted)
- continue;
-
- if (cbList->callbacks[i]->eventID == eventID &&
- cbList->callbacks[i]->conn == conn)
- count++;
- }
-
- return count;
+ return virDomainEventCallbackListAddID(conn, cbList, NULL,
+ VIR_DOMAIN_EVENT_ID_LIFECYCLE,
+ VIR_DOMAIN_EVENT_CALLBACK(callback),
+ opaque, freecb, NULL);
}
-int virDomainEventCallbackListEventID(virConnectPtr conn,
- virDomainEventCallbackListPtr cbList,
- int callbackID)
+
+static int
+virDomainEventCallbackListEventID(virConnectPtr conn,
+ virDomainEventCallbackListPtr cbList,
+ int callbackID)
{
int i;
@@ -517,22 +502,6 @@ int virDomainEventCallbackListEventID(virConnectPtr conn,
}
-int virDomainEventCallbackListCount(virDomainEventCallbackListPtr cbList)
-{
- int i;
- int count = 0;
-
- for (i = 0 ; i < cbList->count ; i++) {
- if (cbList->callbacks[i]->deleted)
- continue;
- if (cbList->callbacks[i]->eventID == VIR_DOMAIN_EVENT_ID_LIFECYCLE)
- count++;
- }
-
- return count;
-}
-
-
void virDomainEventFree(virDomainEventPtr event)
{
if (!event)
@@ -589,7 +558,7 @@ void virDomainEventFree(virDomainEventPtr event)
*
* Free the memory in the queue. We process this like a list here
*/
-void
+static void
virDomainEventQueueFree(virDomainEventQueuePtr queue)
{
int i;
@@ -603,7 +572,8 @@ virDomainEventQueueFree(virDomainEventQueuePtr queue)
VIR_FREE(queue);
}
-virDomainEventQueuePtr virDomainEventQueueNew(void)
+static virDomainEventQueuePtr
+virDomainEventQueueNew(void)
{
virDomainEventQueuePtr ret;
@@ -1085,41 +1055,6 @@ virDomainEventPtr virDomainEventDiskChangeNewFromDom(virDomainPtr
dom,
devAlias, reason);
}
-/**
- * virDomainEventQueuePop:
- * @evtQueue: the queue of events
- *
- * Internal function to pop off, and return the front of the queue
- * NOTE: The caller is responsible for freeing the returned object
- *
- * Returns: virDomainEventPtr on success NULL on failure.
- */
-virDomainEventPtr
-virDomainEventQueuePop(virDomainEventQueuePtr evtQueue)
-{
- virDomainEventPtr ret;
-
- if (!evtQueue || evtQueue->count == 0 ) {
- eventReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("event queue is empty, nothing to pop"));
- return NULL;
- }
-
- ret = evtQueue->events[0];
-
- memmove(evtQueue->events,
- evtQueue->events + 1,
- sizeof(*(evtQueue->events)) *
- (evtQueue->count - 1));
-
- if (VIR_REALLOC_N(evtQueue->events,
- evtQueue->count - 1) < 0) {
- ; /* Failure to reduce memory allocation isn't fatal */
- }
- evtQueue->count--;
-
- return ret;
-}
/**
* virDomainEventQueuePush:
@@ -1130,7 +1065,7 @@ virDomainEventQueuePop(virDomainEventQueuePtr evtQueue)
*
* Returns: 0 on success, -1 on failure
*/
-int
+static int
virDomainEventQueuePush(virDomainEventQueuePtr evtQueue,
virDomainEventPtr event)
{
diff --git a/src/conf/domain_event.h b/src/conf/domain_event.h
index 1845679..f37309c 100644
--- a/src/conf/domain_event.h
+++ b/src/conf/domain_event.h
@@ -46,62 +46,6 @@ typedef virDomainEventQueue *virDomainEventQueuePtr;
typedef struct _virDomainEventState virDomainEventState;
typedef virDomainEventState *virDomainEventStatePtr;
-void virDomainEventCallbackListFree(virDomainEventCallbackListPtr list);
-
-int virDomainEventCallbackListAdd(virConnectPtr conn,
- virDomainEventCallbackListPtr cbList,
- virConnectDomainEventCallback callback,
- void *opaque,
- virFreeCallback freecb)
- ATTRIBUTE_NONNULL(1);
-int virDomainEventCallbackListAddID(virConnectPtr conn,
- virDomainEventCallbackListPtr cbList,
- virDomainPtr dom,
- int eventID,
- virConnectDomainEventGenericCallback cb,
- void *opaque,
- virFreeCallback freecb,
- int *callbackID)
- ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(5);
-
-
-int virDomainEventCallbackListRemove(virConnectPtr conn,
- virDomainEventCallbackListPtr cbList,
- virConnectDomainEventCallback callback)
- ATTRIBUTE_NONNULL(1);
-int virDomainEventCallbackListRemoveID(virConnectPtr conn,
- virDomainEventCallbackListPtr cbList,
- int callbackID)
- ATTRIBUTE_NONNULL(1);
-int virDomainEventCallbackListRemoveConn(virConnectPtr conn,
- virDomainEventCallbackListPtr cbList)
- ATTRIBUTE_NONNULL(1);
-
-
-int virDomainEventCallbackListMarkDelete(virConnectPtr conn,
- virDomainEventCallbackListPtr cbList,
- virConnectDomainEventCallback callback)
- ATTRIBUTE_NONNULL(1);
-int virDomainEventCallbackListMarkDeleteID(virConnectPtr conn,
- virDomainEventCallbackListPtr cbList,
- int callbackID)
- ATTRIBUTE_NONNULL(1);
-
-
-int virDomainEventCallbackListPurgeMarked(virDomainEventCallbackListPtr cbList);
-
-int virDomainEventCallbackListCount(virDomainEventCallbackListPtr cbList);
-int virDomainEventCallbackListCountID(virConnectPtr conn,
- virDomainEventCallbackListPtr cbList,
- int eventID)
- ATTRIBUTE_NONNULL(1);
-int virDomainEventCallbackListEventID(virConnectPtr conn,
- virDomainEventCallbackListPtr cbList,
- int callbackID)
- ATTRIBUTE_NONNULL(1);
-
-virDomainEventQueuePtr virDomainEventQueueNew(void);
-
virDomainEventPtr virDomainEventNew(int id, const char *name, const unsigned char *uuid,
int type, int detail);
virDomainEventPtr virDomainEventNewFromDom(virDomainPtr dom, int type, int detail);
virDomainEventPtr virDomainEventNewFromObj(virDomainObjPtr obj, int type, int detail);
@@ -171,14 +115,8 @@ virDomainEventPtr virDomainEventDiskChangeNewFromDom(virDomainPtr
dom,
const char *devAlias,
int reason);
-int virDomainEventQueuePush(virDomainEventQueuePtr evtQueue,
- virDomainEventPtr event);
-
-virDomainEventPtr
-virDomainEventQueuePop(virDomainEventQueuePtr evtQueue);
-
void virDomainEventFree(virDomainEventPtr event);
-void virDomainEventQueueFree(virDomainEventQueuePtr queue);
+
void virDomainEventStateFree(virDomainEventStatePtr state);
virDomainEventStatePtr
virDomainEventStateNew(virEventTimeoutCallback timeout_cb,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 0c664ed..009a77c 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -470,18 +470,6 @@ virDomainWatchdogModelTypeToString;
# domain_event.h
virDomainEventBlockJobNewFromObj;
virDomainEventBlockJobNewFromDom;
-virDomainEventCallbackListAdd;
-virDomainEventCallbackListAddID;
-virDomainEventCallbackListCount;
-virDomainEventCallbackListCountID;
-virDomainEventCallbackListEventID;
-virDomainEventCallbackListFree;
-virDomainEventCallbackListMarkDelete;
-virDomainEventCallbackListMarkDeleteID;
-virDomainEventCallbackListPurgeMarked;
-virDomainEventCallbackListRemove;
-virDomainEventCallbackListRemoveConn;
-virDomainEventCallbackListRemoveID;
virDomainEventControlErrorNewFromDom;
virDomainEventControlErrorNewFromObj;
virDomainEventDiskChangeNewFromDom;
@@ -499,11 +487,6 @@ virDomainEventNew;
virDomainEventNewFromDef;
virDomainEventNewFromDom;
virDomainEventNewFromObj;
-virDomainEventQueueDispatch;
-virDomainEventQueueFree;
-virDomainEventQueueNew;
-virDomainEventQueuePop;
-virDomainEventQueuePush;
virDomainEventRTCChangeNewFromDom;
virDomainEventRTCChangeNewFromObj;
virDomainEventRebootNew;
--
1.7.7.4
2:32 p.m.
New subject: [libvirt] [PATCH 6/8] Remove decl of all APIs related to domain event callbacks & queues
On 12/13/2011 05:38 PM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange(a)redhat.com>
The virDomainEventCallbackList and virDomainEventQueue APIs are
now solely helpers used internally by virDomainEventState APIs.
Remove their decls from domain_event.h since no driver code should
need to use them any more.
* src/conf/domain_event.c: Make virDomainEventCallbackList and
virDomainEventQueue APIs static & remove some unused APIs
* src/conf/domain_event.h, src/libvirt_private.syms: Remove
virDomainEventCallbackList and virDomainEventQueue APIs
---
src/conf/domain_event.c | 151 +++++++++++++---------------------------------
src/conf/domain_event.h | 64 +-------------------
src/libvirt_private.syms | 17 -----
3 files changed, 44 insertions(+), 188 deletions(-)
Fun amounts of deletion :)
+++ b/src/libvirt_private.syms
@@ -470,18 +470,6 @@ virDomainWatchdogModelTypeToString;
# domain_event.h
virDomainEventBlockJobNewFromObj;
virDomainEventBlockJobNewFromDom;
Huh, these lines weren't sorted (pre-existing).
ACK.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
6:38 p.m.
New subject: [libvirt] [PATCH 7/8] Hide use of timers for domain event dispatch
From: "Daniel P. Berrange" <berrange(a)redhat.com>
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(¨_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
2:56 p.m.
New subject: [libvirt] [PATCH 7/8] Hide use of timers for domain event dispatch
On 12/13/2011 05:38 PM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange(a)redhat.com>
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
I think the couple hunks I mentioned in 4/8 remote_driver.c belong in here.
* 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(-)
More impressive deletion numbers.
@@ -1230,10 +1244,11 @@ void virDomainEventDispatch(virDomainEventPtr
event,
}
-void virDomainEventQueueDispatch(virDomainEventQueuePtr queue,
- virDomainEventCallbackListPtr callbacks,
- virDomainEventDispatchFunc dispatch,
- void *opaque)
+static void
+virDomainEventQueueDispatch(virDomainEventQueuePtr queue,
In most cases, you changed things to start the function name in column 1
(with the return type in previous line) - I actually like this style.
@@ -1266,10 +1281,23 @@
virDomainEventStateQueue(virDomainEventStatePtr state,
virDomainEventStateUnlock(state);
}
-void
-virDomainEventStateFlush(virDomainEventStatePtr state,
- virDomainEventDispatchFunc dispatchFunc,
- void *opaque)
+
+static void virDomainEventStateDispatchFunc(virConnectPtr conn,
...but here's a case where the diff makes it look at first glance like
you converted in the opposite direction. A closer look shows that you
kept the layout of virDomainEventStateFlush intact, but added a new
function, with the new function not using consistent layout, but it
would still be worth consistent layout here.
@@ -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);
Why the change from false to true?
@@ -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))) {
Why the change from false (once you fix my comment in 1/8) to true?
ACK; the above are nits, and the bool parameter goes away in 8/8 (but it
would be worth having this one be correct in the meantime).
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
4:54 a.m.
New subject: [libvirt] [PATCH 7/8] Hide use of timers for domain event dispatch
On Wed, Dec 14, 2011 at 01:56:36PM -0700, Eric Blake wrote:
On 12/13/2011 05:38 PM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange(a)redhat.com>
>
> 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
I think the couple hunks I mentioned in 4/8 remote_driver.c belong in here.
>
> * 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(-)
More impressive deletion numbers.
> @@ -1230,10 +1244,11 @@ void virDomainEventDispatch(virDomainEventPtr event,
> }
>
>
> -void virDomainEventQueueDispatch(virDomainEventQueuePtr queue,
> - virDomainEventCallbackListPtr callbacks,
> - virDomainEventDispatchFunc dispatch,
> - void *opaque)
> +static void
> +virDomainEventQueueDispatch(virDomainEventQueuePtr queue,
In most cases, you changed things to start the function name in column 1
(with the return type in previous line) - I actually like this style.
> @@ -1266,10 +1281,23 @@ virDomainEventStateQueue(virDomainEventStatePtr state,
> virDomainEventStateUnlock(state);
> }
>
> -void
> -virDomainEventStateFlush(virDomainEventStatePtr state,
> - virDomainEventDispatchFunc dispatchFunc,
> - void *opaque)
> +
> +static void virDomainEventStateDispatchFunc(virConnectPtr conn,
...but here's a case where the diff makes it look at first glance like
you converted in the opposite direction. A closer look shows that you
kept the layout of virDomainEventStateFlush intact, but added a new
function, with the new function not using consistent layout, but it
would still be worth consistent layout here.
> @@ -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);
Why the change from false to true?
> @@ -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))) {
Why the change from false (once you fix my comment in 1/8) to true?
Both these drivers run inside the daemon where the event loop is always
expected to be present, so using 'false' was wrong.
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 :|
6:38 p.m.
New subject: [libvirt] [PATCH 8/8] Only add the timer when a callback is registered
From: "Daniel P. Berrange" <berrange(a)redhat.com>
The lifetime of the virDomainEventState object is tied to
the lifetime of the driver, which in stateless drivers is
tied to the lifetime of the virConnectPtr.
If we add & remove a timer when allocating/freeing the
virDomainEventState object, we can get a situation where
the timer still triggers once after virDomainEventState
has been freed. The timeout callback can't keep a ref
on the event state though, since that would be a circular
reference.
The trick is to only register the timer when a callback
is registered with the event state & remove the timer
when the callback is unregistered.
The demo for the bug is to run
while true ; do date ; ../tools/virsh -q -c test:///default 'shutdown test; undefine
test; dominfo test' ; done
prior to this fix, it will frequently hang and / or
crash, or corrupt memory
---
src/conf/domain_event.c | 87 ++++++++++++++++++++++++++++++++------------
src/conf/domain_event.h | 2 +-
src/libxl/libxl_driver.c | 2 +-
src/lxc/lxc_driver.c | 2 +-
src/qemu/qemu_driver.c | 2 +-
src/remote/remote_driver.c | 2 +-
src/test/test_driver.c | 2 +-
src/uml/uml_driver.c | 2 +-
src/vbox/vbox_tmpl.c | 2 +-
src/xen/xen_driver.c | 2 +-
10 files changed, 72 insertions(+), 33 deletions(-)
diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c
index da99d9f..26b4967 100644
--- a/src/conf/domain_event.c
+++ b/src/conf/domain_event.c
@@ -631,13 +631,9 @@ static void virDomainEventTimer(int timer ATTRIBUTE_UNUSED, void
*opaque)
/**
* virDomainEventStateNew:
- * @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(bool requireTimer)
+virDomainEventStateNew(void)
{
virDomainEventStatePtr state = NULL;
@@ -658,23 +654,10 @@ virDomainEventStateNew(bool requireTimer)
goto error;
}
- if (!(state->queue = virDomainEventQueueNew())) {
+ if (!(state->queue = virDomainEventQueueNew()))
goto error;
- }
- if ((state->timer = virEventAddTimeout(-1,
- virDomainEventTimer,
- state,
- NULL)) < 0) {
- 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;
- }
- }
+ state->timer = -1;
return state;
@@ -1311,7 +1294,6 @@ virDomainEventStateFlush(virDomainEventStatePtr state)
state->queue->count = 0;
state->queue->events = NULL;
virEventUpdateTimeout(state->timer, -1);
- virDomainEventStateUnlock(state);
virDomainEventQueueDispatch(&tempQueue,
state->callbacks,
@@ -1319,7 +1301,6 @@ virDomainEventStateFlush(virDomainEventStatePtr state)
state);
/* Purge any deleted callbacks */
- virDomainEventStateLock(state);
virDomainEventCallbackListPurgeMarked(state->callbacks);
state->isDispatching = false;
@@ -1346,10 +1327,32 @@ int virDomainEventStateRegister(virConnectPtr conn,
void *opaque,
virFreeCallback freecb)
{
- int ret;
+ int ret = -1;
+
virDomainEventStateLock(state);
+
+ if ((state->callbacks->count == 0) &&
+ (state->timer == -1) &&
+ (state->timer = virEventAddTimeout(-1,
+ virDomainEventTimer,
+ state,
+ NULL)) < 0) {
+ eventReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("could not initialize domain event timer"));
+ goto cleanup;
+ }
+
ret = virDomainEventCallbackListAdd(conn, state->callbacks,
callback, opaque, freecb);
+
+ if (ret == -1 &&
+ state->callbacks->count == 0 &&
+ state->timer != -1) {
+ virEventRemoveTimeout(state->timer);
+ state->timer = -1;
+ }
+
+cleanup:
virDomainEventStateUnlock(state);
return ret;
}
@@ -1379,11 +1382,33 @@ int virDomainEventStateRegisterID(virConnectPtr conn,
virFreeCallback freecb,
int *callbackID)
{
- int ret;
+ int ret = -1;
+
virDomainEventStateLock(state);
+
+ if ((state->callbacks->count == 0) &&
+ (state->timer == -1) &&
+ (state->timer = virEventAddTimeout(-1,
+ virDomainEventTimer,
+ state,
+ NULL)) < 0) {
+ eventReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("could not initialize domain event timer"));
+ goto cleanup;
+ }
+
ret = virDomainEventCallbackListAddID(conn, state->callbacks,
dom, eventID, cb, opaque, freecb,
callbackID);
+
+ if (ret == -1 &&
+ state->callbacks->count == 0 &&
+ state->timer != -1) {
+ virEventRemoveTimeout(state->timer);
+ state->timer = -1;
+ }
+
+cleanup:
virDomainEventStateUnlock(state);
return ret;
}
@@ -1413,6 +1438,13 @@ virDomainEventStateDeregister(virConnectPtr conn,
state->callbacks, callback);
else
ret = virDomainEventCallbackListRemove(conn, state->callbacks, callback);
+
+ if (state->callbacks->count == 0 &&
+ state->timer != -1) {
+ virEventRemoveTimeout(state->timer);
+ state->timer = -1;
+ }
+
virDomainEventStateUnlock(state);
return ret;
}
@@ -1443,6 +1475,13 @@ virDomainEventStateDeregisterID(virConnectPtr conn,
else
ret = virDomainEventCallbackListRemoveID(conn,
state->callbacks, callbackID);
+
+ if (state->callbacks->count == 0 &&
+ state->timer != -1) {
+ virEventRemoveTimeout(state->timer);
+ state->timer = -1;
+ }
+
virDomainEventStateUnlock(state);
return ret;
}
diff --git a/src/conf/domain_event.h b/src/conf/domain_event.h
index a6c3562..0e7cd75 100644
--- a/src/conf/domain_event.h
+++ b/src/conf/domain_event.h
@@ -119,7 +119,7 @@ void virDomainEventFree(virDomainEventPtr event);
void virDomainEventStateFree(virDomainEventStatePtr state);
virDomainEventStatePtr
-virDomainEventStateNew(bool requireTimer);
+virDomainEventStateNew(void);
void
virDomainEventStateQueue(virDomainEventStatePtr state,
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 04392da..0500ed0 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -928,7 +928,7 @@ libxlStartup(int privileged) {
}
VIR_FREE(log_file);
- libxl_driver->domainEventState = virDomainEventStateNew(true);
+ libxl_driver->domainEventState = virDomainEventStateNew();
if (!libxl_driver->domainEventState)
goto error;
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 6d32ed2..3baff19 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -2418,7 +2418,7 @@ static int lxcStartup(int privileged)
if (virDomainObjListInit(&lxc_driver->domains) < 0)
goto cleanup;
- lxc_driver->domainEventState = virDomainEventStateNew(true);
+ lxc_driver->domainEventState = virDomainEventStateNew();
if (!lxc_driver->domainEventState)
goto cleanup;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index b2a8525..5a876de 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -431,7 +431,7 @@ qemudStartup(int privileged) {
goto out_of_memory;
/* Init domain events */
- qemu_driver->domainEventState = virDomainEventStateNew(true);
+ qemu_driver->domainEventState = virDomainEventStateNew();
if (!qemu_driver->domainEventState)
goto error;
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 03bd424..f266574 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -726,7 +726,7 @@ doRemoteOpen (virConnectPtr conn,
}
}
- if (!(priv->domainEventState = virDomainEventStateNew(false)))
+ if (!(priv->domainEventState = virDomainEventStateNew()))
goto failed;
/* Successful. */
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 5ec4190..9e00b1c 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -1137,7 +1137,7 @@ static virDrvOpenStatus testOpen(virConnectPtr conn,
privconn = conn->privateData;
testDriverLock(privconn);
- privconn->domainEventState = virDomainEventStateNew(false);
+ privconn->domainEventState = virDomainEventStateNew();
if (!privconn->domainEventState) {
testDriverUnlock(privconn);
testClose(conn);
diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c
index 360f0ce..671216e 100644
--- a/src/uml/uml_driver.c
+++ b/src/uml/uml_driver.c
@@ -413,7 +413,7 @@ umlStartup(int privileged)
if (virDomainObjListInit(¨_driver->domains) < 0)
goto error;
- uml_driver->domainEventState = virDomainEventStateNew(true);
+ uml_driver->domainEventState = virDomainEventStateNew();
if (!uml_driver->domainEventState)
goto error;
diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index a10f5ae..0339123 100644
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -1034,7 +1034,7 @@ static virDrvOpenStatus vboxOpen(virConnectPtr conn,
#else /* !(VBOX_API_VERSION == 2002) */
- if (!(data->domainEvents = virDomainEventStateNew(true))) {
+ if (!(data->domainEvents = virDomainEventStateNew())) {
vboxUninitialize(data);
return VIR_DRV_OPEN_ERROR;
}
diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
index ab49c2b..20671c0 100644
--- a/src/xen/xen_driver.c
+++ b/src/xen/xen_driver.c
@@ -325,7 +325,7 @@ xenUnifiedOpen (virConnectPtr conn, virConnectAuthPtr auth, unsigned
int flags)
return VIR_DRV_OPEN_ERROR;
}
- if (!(priv->domainEvents = virDomainEventStateNew(true))) {
+ if (!(priv->domainEvents = virDomainEventStateNew())) {
virMutexDestroy(&priv->lock);
VIR_FREE(priv);
return VIR_DRV_OPEN_ERROR;
--
1.7.7.4
4:18 p.m.
New subject: [libvirt] [PATCH 8/8] Only add the timer when a callback is registered
On 12/13/2011 05:38 PM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange(a)redhat.com>
The lifetime of the virDomainEventState object is tied to
the lifetime of the driver, which in stateless drivers is
tied to the lifetime of the virConnectPtr.
If we add & remove a timer when allocating/freeing the
virDomainEventState object, we can get a situation where
the timer still triggers once after virDomainEventState
has been freed. The timeout callback can't keep a ref
on the event state though, since that would be a circular
reference.
The trick is to only register the timer when a callback
is registered with the event state & remove the timer
when the callback is unregistered.
ACK.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
9:05 p.m.
Hi Daniel,
I also met similar issue when I ran libvirt unit test yesterday,
In addition, the case 'read-bufsiz' will be hung forever unless
typing ctrl+c to sends a SIGINT signal to the process.
<snip>
PASS: read-bufsiz
FAIL: read-non-seekable
PASS: start
./undefine: line 49: 26790 Segmentation fault (core dumped)
$abs_top_builddir/tools/virsh -q -c test:///default 'dominfo 1; undefine 1; dominfo
1' > out1 2>&1
--- exp 2011-12-13 15:22:59.076222906 +0800
</snip>
Regards,
Alex
----- Original Message -----
From: "Daniel P. Berrange" <berrange(a)redhat.com>
To: libvir-list(a)redhat.com
Sent: Wednesday, December 14, 2011 8:38:17 AM
Subject: [libvirt] [PATCH 0/8] Refactor event state handling to fix crash
When running
while true ; do date ; ../tools/virsh -q -c test:///default 'shutdown test;
undefine test; dominfo test' ; done
we often experiance hangs or crashes or memory corruption in
glibc. The reason is that when the virConnectPtr is free'd this
in turn frees the event state, which in turn unregisters the
event timer. The timer may, however, still fire one last time
and it is possible this occurs after the event state has been
freed.
The solution in this series is to only register the timer when
the first callback is added and unregister the timer, when the
last callback is removed.
In doing this patch, I decide to unify more of the event handling
across drivers, and simplify the API usage by drivers, and hide
all the impl details.
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
9:36 p.m.
On 12/14/2011 11:05 AM, Alex Jia wrote:
Hi Daniel,
I also met similar issue when I ran libvirt unit test yesterday,
In addition, the case 'read-bufsiz' will be hung forever unless
s/read-bufsiz/read-non-seekable/
typing ctrl+c to sends a SIGINT signal to the process.
<snip>
PASS: read-bufsiz
FAIL: read-non-seekable
PASS: start
./undefine: line 49: 26790 Segmentation fault (core dumped)
$abs_top_builddir/tools/virsh -q -c test:///default 'dominfo 1; undefine 1; dominfo
1'> out1 2>&1
--- exp 2011-12-13 15:22:59.076222906 +0800
</snip>
Regards,
Alex
----- Original Message -----
From: "Daniel P. Berrange"<berrange(a)redhat.com>
To: libvir-list(a)redhat.com
Sent: Wednesday, December 14, 2011 8:38:17 AM
Subject: [libvirt] [PATCH 0/8] Refactor event state handling to fix crash
When running
while true ; do date ; ../tools/virsh -q -c test:///default 'shutdown test;
undefine test; dominfo test' ; done
we often experiance hangs or crashes or memory corruption in
glibc. The reason is that when the virConnectPtr is free'd this
in turn frees the event state, which in turn unregisters the
event timer. The timer may, however, still fire one last time
and it is possible this occurs after the event state has been
freed.
The solution in this series is to only register the timer when
the first callback is added and unregister the timer, when the
last callback is removed.
In doing this patch, I decide to unify more of the event handling
across drivers, and simplify the API usage by drivers, and hide
all the impl details.
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
2:38 p.m.
On 12/13/2011 07:38 PM, Daniel P. Berrange wrote:
When running
while true ; do date ; ../tools/virsh -q -c test:///default 'shutdown test;
undefine test; dominfo test' ; done
I can give a functional ACK on this - my similar test had previously
hung consistently at least once every 80 iterations, and just now ran
for over an hour and a half with 0 hangs and 0 crashes.
4722
days inactive
4727
days old
25 comments
4 participants
participants (4)
-
Alex Jia -
Daniel P. Berrange -
Eric Blake -
Laine Stump