Fix xen driver refcounting.

The commit cb51aa48a777ddae6997faa9f28350cb62655ffd "Fix up connection
reference counting." changed the driver closing and virConnectPtr
unref-logic in virConnectClose().

Before this commit virConnectClose() closed all drivers of the given
virConnectPtr and virUnrefConnect()'ed it afterwards. After this
commit the driver-closing is done in virUnrefConnect() if and only if
the ref-count of the virConnectPtr dropped to zero.

This change in execution order leads to a virConnectPtr leak, at least
for connections to Xen.

The relevant call sequences:

virConnectOpen() -> xenUnifiedOpen() ...

... xenInotifyOpen() -> virConnectRef(conn)

... xenStoreOpen() -> xenStoreAddWatch() -> conn->refs++

virConnectClose() -> xenUnifiedClose() ...

... xenInotifyClose() -> virUnrefConnect(conn)

... xenStoreClose() -> xenStoreRemoveWatch() -> virUnrefConnect(conn)

Before the commit this additional virConnectRef/virUnrefConnect calls
where no problem, because virConnectClose() closed the drivers
explicitly and the additional refs added by the Xen subdrivers were
removed properly. After the commit this additional refs result in a
virConnectPtr leak (including a leak of the hypercall file handle;
that's how I noticed this problem), because now the drivers are only
close if and only if the ref-count drops to zero, but this cannot
happen anymore, because the additional refs from the Xen subdrivers
would only be removed if the drivers get closed, but that doesn't
happen because the ref-count cannot drop to zero.

The fix for this problem is simple: remove the
virConnectRef/virUnrefConnect calls from the Xen subdrivers (see
attached patch). Maybe someone could explain why the Xen Inotify and
Xen Store driver do this extra ref-counting, but none of the other Xen
subdrivers. It seems unnecessary to me and can be removed without
problems.

Signed-off-by: Chris Lalancette <clalance@redhat.com>

diff --git a/src/xen/xen_inotify.c b/src/xen/xen_inotify.c
index 7deb1dbe99eda06fd6033956e153fcf77e71eb5b..9e0407f0113f976a4e3e2f602e751075009741b7 100644 (file)
--- a/src/xen/xen_inotify.c
+++ b/src/xen/xen_inotify.c
@@ -463,7 +463,6 @@ xenInotifyOpen(virConnectPtr conn ATTRIBUTE_UNUSED,
         DEBUG0("Failed to add inotify handle, disabling events");
     }
 
-    virConnectRef(conn);
     return 0;
 }
 
@@ -486,7 +485,6 @@ xenInotifyClose(virConnectPtr conn)
     if (priv->inotifyWatch != -1)
         virEventRemoveHandle(priv->inotifyWatch);
     close(priv->inotifyFD);
-    virUnrefConnect(conn);
 
     return 0;
 }

diff --git a/src/xen/xs_internal.c b/src/xen/xs_internal.c
index 54f410f29c2c626af457886f8d684074441ea7e0..0fabcf81abeb79013e54e3b06b598a4f92131b8a 100644 (file)
--- a/src/xen/xs_internal.c
+++ b/src/xen/xs_internal.c
@@ -1141,8 +1141,6 @@ int xenStoreAddWatch(virConnectPtr conn,
     list->watches[n] = watch;
     list->count++;
 
-    conn->refs++;
-
     return xs_watch(priv->xshandle, watch->path, watch->token);
 }
 
@@ -1192,7 +1190,6 @@ int xenStoreRemoveWatch(virConnectPtr conn,
                 ; /* Failure to reduce memory allocation isn't fatal */
             }
             list->count--;
-            virUnrefConnect(conn);
             return 0;
         }
     }