Fix reference counting bug in virsh console

The event watches need to be removed before the event loop
terminates, otherwise they cause a dangling reference to
be held on the virStreamPtr, which in turns holds a reference
on virConnectPtr, which in turn causes errors like

  "Failed to disconnect from the hypervisor"

* tools/console.c: Remove watches before event loop quits
* tools/virsh.c: Print out dangling reference count

diff --git a/tools/console.c b/tools/console.c
index f38bbb9a6420229c753b3f07e9279843593b910e..c2971cf1b7574f418c86c05095fb5301324f19be 100644 (file)
--- a/tools/console.c
+++ b/tools/console.c
@@ -88,6 +88,19 @@ cfmakeraw (struct termios *attr)
 }
 # endif /* !HAVE_CFMAKERAW */
 
+static void
+virConsoleShutdown(virConsolePtr con)
+{
+    con->quit = true;
+    virStreamEventRemoveCallback(con->st);
+    if (con->stdinWatch != -1)
+        virEventRemoveHandleImpl(con->stdinWatch);
+    if (con->stdinWatch != -1)
+        virEventRemoveHandleImpl(con->stdoutWatch);
+    con->stdinWatch = -1;
+    con->stdoutWatch = -1;
+}
+
 static void
 virConsoleEventOnStream(virStreamPtr st,
                         int events, void *opaque)
@@ -103,7 +116,7 @@ virConsoleEventOnStream(virStreamPtr st,
             if (VIR_REALLOC_N(con->streamToTerminal.data,
                               con->streamToTerminal.length + 1024) < 0) {
                 virReportOOMError();
-                con->quit = true;
+                virConsoleShutdown(con);
                 return;
             }
             con->streamToTerminal.length += 1024;
@@ -117,7 +130,7 @@ virConsoleEventOnStream(virStreamPtr st,
         if (got == -2)
             return; /* blocking */
         if (got <= 0) {
-            con->quit = true;
+            virConsoleShutdown(con);
             return;
         }
         con->streamToTerminal.offset += got;
@@ -136,7 +149,7 @@ virConsoleEventOnStream(virStreamPtr st,
         if (done == -2)
             return; /* blocking */
         if (done < 0) {
-            con->quit = true;
+            virConsoleShutdown(con);
             return;
         }
         memmove(con->terminalToStream.data,
@@ -158,7 +171,7 @@ virConsoleEventOnStream(virStreamPtr st,
 
     if (events & VIR_STREAM_EVENT_ERROR ||
         events & VIR_STREAM_EVENT_HANGUP) {
-        con->quit = true;
+        virConsoleShutdown(con);
     }
 }
 
@@ -179,7 +192,7 @@ virConsoleEventOnStdin(int watch ATTRIBUTE_UNUSED,
             if (VIR_REALLOC_N(con->terminalToStream.data,
                               con->terminalToStream.length + 1024) < 0) {
                 virReportOOMError();
-                con->quit = true;
+                virConsoleShutdown(con);
                 return;
             }
             con->terminalToStream.length += 1024;
@@ -192,16 +205,16 @@ virConsoleEventOnStdin(int watch ATTRIBUTE_UNUSED,
                    avail);
         if (got < 0) {
             if (errno != EAGAIN) {
-                con->quit = true;
+                virConsoleShutdown(con);
             }
             return;
         }
         if (got == 0) {
-            con->quit = true;
+            virConsoleShutdown(con);
             return;
         }
         if (con->terminalToStream.data[con->terminalToStream.offset] == CTRL_CLOSE_BRACKET) {
-            con->quit = true;
+            virConsoleShutdown(con);
             return;
         }
 
@@ -214,7 +227,7 @@ virConsoleEventOnStdin(int watch ATTRIBUTE_UNUSED,
 
     if (events & VIR_EVENT_HANDLE_ERROR ||
         events & VIR_EVENT_HANDLE_HANGUP) {
-        con->quit = true;
+        virConsoleShutdown(con);
     }
 }
 
@@ -235,7 +248,7 @@ virConsoleEventOnStdout(int watch ATTRIBUTE_UNUSED,
                      con->streamToTerminal.offset);
         if (done < 0) {
             if (errno != EAGAIN) {
-                con->quit = true;
+                virConsoleShutdown(con);
             }
             return;
         }
@@ -258,7 +271,7 @@ virConsoleEventOnStdout(int watch ATTRIBUTE_UNUSED,
 
     if (events & VIR_EVENT_HANDLE_ERROR ||
         events & VIR_EVENT_HANDLE_HANGUP) {
-        con->quit = true;
+        virConsoleShutdown(con);
     }
 }
 
@@ -341,10 +354,6 @@ int vshRunConsole(virDomainPtr dom, const char *devname)
             break;
     }
 
-    virStreamEventRemoveCallback(con->st);
-    virEventRemoveHandleImpl(con->stdinWatch);
-    virEventRemoveHandleImpl(con->stdoutWatch);
-
     ret = 0;
 
  cleanup:

diff --git a/tools/virsh.c b/tools/virsh.c
index 55a36b2fd4b9c31f62730128783ac43ea2c34b97..d15a8df74249e2ab0794c0ec20f495f93900141f 100644 (file)
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -637,8 +637,9 @@ cmdConnect(vshControl *ctl, const vshCmd *cmd)
     char *name;
 
     if (ctl->conn) {
-        if (virConnectClose(ctl->conn) != 0) {
-            vshError(ctl, "%s", _("Failed to disconnect from the hypervisor"));
+        int ret;
+        if ((ret = virConnectClose(ctl->conn)) != 0) {
+            vshError(ctl, _("Failed to disconnect from the hypervisor, %d leaked reference(s)"), ret);
             return FALSE;
         }
         ctl->conn = NULL;
@@ -11463,8 +11464,9 @@ vshDeinit(vshControl *ctl)
     vshCloseLogFile(ctl);
     VIR_FREE(ctl->name);
     if (ctl->conn) {
-        if (virConnectClose(ctl->conn) != 0) {
-            vshError(ctl, "%s", _("failed to disconnect from the hypervisor"));
+        int ret;
+        if ((ret = virConnectClose(ctl->conn)) != 0) {
+            vshError(ctl, _("Failed to disconnect from the hypervisor, %d leaked reference(s)"), ret);
         }
     }
     virResetLastError();