[libvirt] [PATCH] Fix event-test segfault for mixed remote/local driver usage

Matthias Bolte matthias.bolte at googlemail.com
Thu Jan 14 02:19:06 UTC 2010


If examples/domain-events/events-c/event-test.c uses a local driver
for the hyperviror and a remote driver for storage then the segfault
occurs in remoteDomainEventDispatchFunc because conn->privateData is
used.

The problem is as follows. The doRemoteOpen function registers
remoteDomainEventFired as event handle and remoteDomainEventQueueFlush
as event timeout. The doRemoteOpen function is used in remoteOpen and
remoteOpenSecondaryDriver. So both handlers are registered for all
types of remote drivers, but remoteDomainEventQueueFlush always uses
conn->privateData. That's wrong when remoteDomainEventQueueFlush is
called for any other driver than the hypervisor driver, it should use
conn->storagePrivateData for the storage driver for example.

In the common case this doesn't result in a segfault because in the
common case all drivers for a connection are either all remote or all
local and conn->privateData and conn->storagePrivateData are the same.
But in the local Xen case ('event-test xen:///' executed in Domain-0,
with a libvirtd running in Domain-0) the hypervisor driver is local
and the storage driver is remote. This results in a mismatch between
conn->privateData and conn->storagePrivateData. Now the call to
remoteDomainEventQueueFlush in the context of the storage driver
results in a segfault, because it assumes that conn->privateData points
to remote driver private data, but it points to Xen driver private
data.

To fix this, the pointer to the private driver data gets passed as
opaque parameter instead of the connection pointer. Because some
functions called by the event handler functions need the connection
pointer for other purposes than error reporting the connection
pointer is passed as member of the private remote driver data struct.

Now remoteDomainEventFired and remoteDomainEventQueueFlush have the
correct private data pointer for the respective calling context.
---
 src/remote/remote_driver.c |   16 ++++++++--------
 1 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index d6f5fce..582b85c 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -152,6 +152,7 @@ struct private_stream_data {
 
 struct private_data {
     virMutex lock;
+    virConnectPtr conn;
 
     int sock;                   /* Socket. */
     int watch;                  /* File handle watch */
@@ -893,7 +894,7 @@ doRemoteOpen (virConnectPtr conn,
     if ((priv->watch = virEventAddHandle(priv->sock,
                                          VIR_EVENT_HANDLE_READABLE,
                                          remoteDomainEventFired,
-                                         conn, NULL)) < 0) {
+                                         priv, NULL)) < 0) {
         DEBUG0("virEventAddHandle failed: No addHandleImpl defined."
                " continuing without events.");
     } else {
@@ -901,7 +902,7 @@ doRemoteOpen (virConnectPtr conn,
         DEBUG0("Adding Timeout for remote event queue flushing");
         if ( (priv->eventFlushTimer = virEventAddTimeout(-1,
                                                          remoteDomainEventQueueFlush,
-                                                         conn, NULL)) < 0) {
+                                                         priv, NULL)) < 0) {
             DEBUG0("virEventAddTimeout failed: No addTimeoutImpl defined. "
                     "continuing without events.");
             virEventRemoveHandle(priv->watch);
@@ -980,6 +981,7 @@ remoteAllocPrivateData(virConnectPtr conn)
         return NULL;
     }
     remoteDriverLock(priv);
+    priv->conn = conn;
     priv->localUses = 1;
     priv->watch = -1;
     priv->sock = -1;
@@ -8659,8 +8661,7 @@ remoteDomainEventFired(int watch,
                        int event,
                        void *opaque)
 {
-    virConnectPtr        conn = opaque;
-    struct private_data *priv = conn->privateData;
+    struct private_data *priv = opaque;
 
     remoteDriverLock(priv);
 
@@ -8684,7 +8685,7 @@ remoteDomainEventFired(int watch,
         goto done;
     }
 
-    if (remoteIOHandleInput(conn, priv, 0) < 0)
+    if (remoteIOHandleInput(priv->conn, priv, 0) < 0)
         DEBUG0("Something went wrong during async message processing");
 
 done:
@@ -8708,8 +8709,7 @@ static void remoteDomainEventDispatchFunc(virConnectPtr conn,
 void
 remoteDomainEventQueueFlush(int timer ATTRIBUTE_UNUSED, void *opaque)
 {
-    virConnectPtr conn = opaque;
-    struct private_data *priv = conn->privateData;
+    struct private_data *priv = opaque;
     virDomainEventQueue tempQueue;
 
     remoteDriverLock(priv);
@@ -8731,7 +8731,7 @@ remoteDomainEventQueueFlush(int timer ATTRIBUTE_UNUSED, void *opaque)
 
     if ( priv->callbackList->count == 0 ) {
         /* Tell the server when we are the last callback deregistering */
-        if (call (conn, priv, 0, REMOTE_PROC_DOMAIN_EVENTS_DEREGISTER,
+        if (call (priv->conn, priv, 0, REMOTE_PROC_DOMAIN_EVENTS_DEREGISTER,
                   (xdrproc_t) xdr_void, (char *) NULL,
                   (xdrproc_t) xdr_void, (char *) NULL) == -1)
             VIR_WARN0("Failed to de-register events");
-- 
1.6.3.3




More information about the libvir-list mailing list