Re-write virsh console to use streams

This re-writes the 'virsh console' command so that it uses
the new streams API. This lets it run remotely and/or as a
non-root user. This requires that virsh be linked against
the simple event loop from libvirtd in daemon/event.c
As an added bonus, it can now connect to any console device,
not just the first one.

* tools/Makefile.am: Link to event.c
* tools/console.c, tools/console.h: Rewrite to use the
  virDomainOpenConsole() APIs with streams
* tools/virsh.c: Support choosing the console name
  via --devname $NAME

.x-sc_avoid_write

@@ -5,3 +5,4 @@
 ^src/xen/xend_internal\.c$
 ^daemon/libvirtd.c$
 ^gnulib/
+^tools/console.c$

tools/Makefile.am

@@ -38,6 +38,7 @@ virt-pki-validate.1: virt-pki-validate
 
 virsh_SOURCES =							\
 		console.c console.h				\
+		../daemon/event.c ../daemon/event.h		\
 		virsh.c
 
 virsh_LDFLAGS = $(WARN_LDFLAGS) $(COVERAGE_LDFLAGS)

tools/console.c

@@ -34,16 +34,42 @@
 # include <errno.h>
 # include <unistd.h>
 # include <signal.h>
+# include <stdbool.h>
 
-# include "console.h"
 # include "internal.h"
+# include "console.h"
 # include "logging.h"
 # include "util.h"
 # include "files.h"
+# include "memory.h"
+# include "virterror_internal.h"
+
+# include "daemon/event.h"
 
 /* ie  Ctrl-]  as per telnet */
 # define CTRL_CLOSE_BRACKET '\35'
 
+# define VIR_FROM_THIS VIR_FROM_NONE
+
+struct virConsoleBuffer {
+    size_t length;
+    size_t offset;
+    char *data;
+};
+
+typedef struct virConsole virConsole;
+typedef virConsole *virConsolePtr;
+struct virConsole {
+    virStreamPtr st;
+    bool quit;
+
+    int stdinWatch;
+    int stdoutWatch;
+
+    struct virConsoleBuffer streamToTerminal;
+    struct virConsoleBuffer terminalToStream;
+};
+
 static int got_signal = 0;
 static void do_signal(int sig ATTRIBUTE_UNUSED) {
     got_signal = 1;
@@ -62,22 +88,191 @@ cfmakeraw (struct termios *attr)
 }
 # endif /* !HAVE_CFMAKERAW */
 
-int vshRunConsole(const char *tty) {
+static void
-    int ttyfd, ret = -1;
+virConsoleEventOnStream(virStreamPtr st,
+                        int events, void *opaque)
+{
+    virConsolePtr con = opaque;
+
+    if (events & VIR_STREAM_EVENT_READABLE) {
+        size_t avail = con->streamToTerminal.length -
+            con->streamToTerminal.offset;
+        int got;
+
+        if (avail < 1024) {
+            if (VIR_REALLOC_N(con->streamToTerminal.data,
+                              con->streamToTerminal.length + 1024) < 0) {
+                virReportOOMError();
+                con->quit = true;
+                return;
+            }
+            con->streamToTerminal.length += 1024;
+            avail += 1024;
+        }
+
+        got = virStreamRecv(st,
+                            con->streamToTerminal.data +
+                            con->streamToTerminal.offset,
+                            avail);
+        if (got == -2)
+            return; /* blocking */
+        if (got <= 0) {
+            con->quit = true;
+            return;
+        }
+        con->streamToTerminal.offset += got;
+        if (con->streamToTerminal.offset)
+            virEventUpdateHandleImpl(con->stdoutWatch,
+                                     VIR_EVENT_HANDLE_WRITABLE);
+    }
+
+    if (events & VIR_STREAM_EVENT_WRITABLE &&
+        con->terminalToStream.offset) {
+        ssize_t done;
+        size_t avail;
+        done = virStreamSend(con->st,
+                             con->terminalToStream.data,
+                             con->terminalToStream.offset);
+        if (done == -2)
+            return; /* blocking */
+        if (done < 0) {
+            con->quit = true;
+            return;
+        }
+        memmove(con->terminalToStream.data,
+                con->terminalToStream.data + done,
+                con->terminalToStream.offset - done);
+        con->terminalToStream.offset -= done;
+
+        avail = con->terminalToStream.length - con->terminalToStream.offset;
+        if (avail > 1024) {
+            if (VIR_REALLOC_N(con->terminalToStream.data,
+                              con->terminalToStream.offset + 1024) < 0)
+            {}
+            con->terminalToStream.length = con->terminalToStream.offset + 1024;
+        }
+    }
+    if (!con->terminalToStream.offset)
+        virStreamEventUpdateCallback(con->st,
+                                     VIR_STREAM_EVENT_READABLE);
+
+    if (events & VIR_STREAM_EVENT_ERROR ||
+        events & VIR_STREAM_EVENT_HANGUP) {
+        con->quit = true;
+    }
+}
+
+static void
+virConsoleEventOnStdin(int watch ATTRIBUTE_UNUSED,
+                       int fd ATTRIBUTE_UNUSED,
+                       int events,
+                       void *opaque)
+{
+    virConsolePtr con = opaque;
+
+    if (events & VIR_EVENT_HANDLE_READABLE) {
+        size_t avail = con->terminalToStream.length -
+            con->terminalToStream.offset;
+        int got;
+
+        if (avail < 1024) {
+            if (VIR_REALLOC_N(con->terminalToStream.data,
+                              con->terminalToStream.length + 1024) < 0) {
+                virReportOOMError();
+                con->quit = true;
+                return;
+            }
+            con->terminalToStream.length += 1024;
+            avail += 1024;
+        }
+
+        got = read(fd,
+                   con->terminalToStream.data +
+                   con->terminalToStream.offset,
+                   avail);
+        if (got < 0) {
+            if (errno != EAGAIN) {
+                con->quit = true;
+            }
+            return;
+        }
+        if (got == 0) {
+            con->quit = true;
+            return;
+        }
+        if (con->terminalToStream.data[con->terminalToStream.offset] == CTRL_CLOSE_BRACKET) {
+            con->quit = true;
+            return;
+        }
+
+        con->terminalToStream.offset += got;
+        if (con->terminalToStream.offset)
+            virStreamEventUpdateCallback(con->st,
+                                         VIR_STREAM_EVENT_READABLE |
+                                         VIR_STREAM_EVENT_WRITABLE);
+    }
+
+    if (events & VIR_EVENT_HANDLE_ERROR ||
+        events & VIR_EVENT_HANDLE_HANGUP) {
+        con->quit = true;
+    }
+}
+
+static void
+virConsoleEventOnStdout(int watch ATTRIBUTE_UNUSED,
+                        int fd,
+                        int events,
+                        void *opaque)
+{
+    virConsolePtr con = opaque;
+
+    if (events & VIR_EVENT_HANDLE_WRITABLE &&
+        con->streamToTerminal.offset) {
+        ssize_t done;
+        size_t avail;
+        done = write(fd,
+                     con->streamToTerminal.data,
+                     con->streamToTerminal.offset);
+        if (done < 0) {
+            if (errno != EAGAIN) {
+                con->quit = true;
+            }
+            return;
+        }
+        memmove(con->streamToTerminal.data,
+                con->streamToTerminal.data + done,
+                con->streamToTerminal.offset - done);
+        con->streamToTerminal.offset -= done;
+
+        avail = con->streamToTerminal.length - con->streamToTerminal.offset;
+        if (avail > 1024) {
+            if (VIR_REALLOC_N(con->streamToTerminal.data,
+                              con->streamToTerminal.offset + 1024) < 0)
+            {}
+            con->streamToTerminal.length = con->streamToTerminal.offset + 1024;
+        }
+    }
+
+    if (!con->streamToTerminal.offset)
+        virEventUpdateHandleImpl(con->stdoutWatch, 0);
+
+    if (events & VIR_EVENT_HANDLE_ERROR ||
+        events & VIR_EVENT_HANDLE_HANGUP) {
+        con->quit = true;
+    }
+}
+
+
+int vshRunConsole(virDomainPtr dom, const char *devname)
+{
+    int ret = -1;
     struct termios ttyattr, rawattr;
     void (*old_sigquit)(int);
     void (*old_sigterm)(int);
     void (*old_sigint)(int);
     void (*old_sighup)(int);
     void (*old_sigpipe)(int);
-
+    virConsolePtr con = NULL;
-
-    /* We do not want this to become the controlling TTY */
-    if ((ttyfd = open(tty, O_NOCTTY | O_RDWR)) < 0) {
-        VIR_ERROR(_("unable to open tty %s: %s"),
-                  tty, strerror(errno));
-        return -1;
-    }
 
     /* Put STDIN into raw mode so that stuff typed
        does not echo to the screen (the TTY reads will
@@ -87,7 +282,7 @@ int vshRunConsole(const char *tty) {
     if (tcgetattr(STDIN_FILENO, &ttyattr) < 0) {
         VIR_ERROR(_("unable to get tty attributes: %s"),
                   strerror(errno));
-        goto closetty;
+        return -1;
     }
 
     rawattr = ttyattr;
@@ -96,7 +291,7 @@ int vshRunConsole(const char *tty) {
     if (tcsetattr(STDIN_FILENO, TCSAFLUSH, &rawattr) < 0) {
         VIR_ERROR(_("unable to set tty attributes: %s"),
                   strerror(errno));
-        goto closetty;
+        goto resettty;
     }
 
 
@@ -111,76 +306,55 @@ int vshRunConsole(const char *tty) {
     old_sigpipe = signal(SIGPIPE, do_signal);
     got_signal = 0;
 
-
+    if (VIR_ALLOC(con) < 0) {
-    /* Now lets process STDIN & tty forever.... */
+        virReportOOMError();
-    for (; !got_signal ;) {
+        goto cleanup;
-        unsigned int i;
-        struct pollfd fds[] = {
-            { STDIN_FILENO, POLLIN, 0 },
-            { ttyfd, POLLIN, 0 },
-        };
-
-        /* Wait for data to be available for reading on
-           STDIN or the tty */
-        if (poll(fds, (sizeof(fds)/sizeof(struct pollfd)), -1) < 0) {
-            if (got_signal)
-                goto cleanup;
-
-            if (errno == EINTR || errno == EAGAIN)
-                continue;
-
-            VIR_ERROR(_("failure waiting for I/O: %s"), strerror(errno));
-            goto cleanup;
-        }
-
-        for (i = 0 ; i < (sizeof(fds)/sizeof(struct pollfd)) ; i++) {
-            if (!fds[i].revents)
-                continue;
-
-            /* Process incoming data available for read */
-            if (fds[i].revents & POLLIN) {
-                char buf[4096];
-                int got, sent = 0, destfd;
-
-                if ((got = read(fds[i].fd, buf, sizeof(buf))) < 0) {
-                    VIR_ERROR(_("failure reading input: %s"),
-                              strerror(errno));
-                    goto cleanup;
-                }
-
-                /* Quit if end of file, or we got the Ctrl-] key */
-                if (!got ||
-                    (got == 1 &&
-                     buf[0] == CTRL_CLOSE_BRACKET))
-                    goto done;
-
-                /* Data from stdin goes to the TTY,
-                   data from the TTY goes to STDOUT */
-                if (fds[i].fd == STDIN_FILENO)
-                    destfd = ttyfd;
-                else
-                    destfd = STDOUT_FILENO;
-
-                while (sent < got) {
-                    int done;
-                    if ((done = safewrite(destfd, buf + sent, got - sent))
-                        <= 0) {
-                        VIR_ERROR(_("failure writing output: %s"),
-                                  strerror(errno));
-                        goto cleanup;
-                    }
-                    sent += done;
-                }
-            } else { /* Any other flag from poll is an error condition */
-                goto cleanup;
-            }
-        }
     }
- done:
+
+    con->st = virStreamNew(virDomainGetConnect(dom),
+                           VIR_STREAM_NONBLOCK);
+    if (!con->st)
+        goto cleanup;
+
+    if (virDomainOpenConsole(dom, devname, con->st, 0) < 0)
+        goto cleanup;
+
+    con->stdinWatch = virEventAddHandleImpl(STDIN_FILENO,
+                                            VIR_EVENT_HANDLE_READABLE,
+                                            virConsoleEventOnStdin,
+                                            con,
+                                            NULL);
+    con->stdoutWatch = virEventAddHandleImpl(STDOUT_FILENO,
+                                             0,
+                                             virConsoleEventOnStdout,
+                                             con,
+                                             NULL);
+
+    virStreamEventAddCallback(con->st,
+                              VIR_STREAM_EVENT_READABLE,
+                              virConsoleEventOnStream,
+                              con,
+                              NULL);
+
+    while (!con->quit) {
+        if (virEventRunOnce() < 0)
+            break;
+    }
+
+    virStreamEventRemoveCallback(con->st);
+    virEventRemoveHandleImpl(con->stdinWatch);
+    virEventRemoveHandleImpl(con->stdoutWatch);
+
     ret = 0;
 
  cleanup:
 
+    if (con) {
+        if (con->st)
+            virStreamFree(con->st);
+        VIR_FREE(con);
+    }
+
     /* Restore original signal handlers */
     signal(SIGQUIT, old_sigpipe);
     signal(SIGQUIT, old_sighup);
@@ -188,13 +362,11 @@ int vshRunConsole(const char *tty) {
     signal(SIGQUIT, old_sigterm);
     signal(SIGQUIT, old_sigquit);
 
+resettty:
     /* Put STDIN back into the (sane?) state we found
        it in before starting */
     tcsetattr(STDIN_FILENO, TCSAFLUSH, &ttyattr);
 
- closetty:
-    VIR_FORCE_CLOSE(ttyfd);
-
     return ret;
 }
 

tools/console.h

@@ -25,7 +25,7 @@
 
 # ifndef WIN32
 
-int vshRunConsole(const char *tty);
+int vshRunConsole(virDomainPtr dom, const char *devname);
 
 # endif /* !WIN32 */
 

tools/virsh.c

@@ -52,6 +52,7 @@
 #include "xml.h"
 #include "libvirt/libvirt-qemu.h"
 #include "files.h"
+#include "../daemon/event.h"
 
 static char *progname;
 
@@ -678,36 +679,16 @@ static const vshCmdInfo info_console[] = {
 
 static const vshCmdOptDef opts_console[] = {
     {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")},
+    {"devname", VSH_OT_STRING, 0, N_("character device name")},
     {NULL, 0, 0, NULL}
 };
 
 static int
-cmdRunConsole(vshControl *ctl, virDomainPtr dom)
+cmdRunConsole(vshControl *ctl, virDomainPtr dom, const char *devname)
 {
-    xmlDocPtr xml = NULL;
-    xmlXPathObjectPtr obj = NULL;
-    xmlXPathContextPtr ctxt = NULL;
     int ret = FALSE;
-    char *doc;
-    char *thatHost = NULL;
-    char *thisHost = NULL;
     virDomainInfo dominfo;
 
-    if (!(thisHost = virGetHostname(ctl->conn))) {
-        vshError(ctl, "%s", _("Failed to get local hostname"));
-        goto cleanup;
-    }
-
-    if (!(thatHost = virConnectGetHostname(ctl->conn))) {
-        vshError(ctl, "%s", _("Failed to get connection hostname"));
-        goto cleanup;
-    }
-
-    if (STRNEQ(thisHost, thatHost)) {
-        vshError(ctl, "%s", _("Cannot connect to a remote console device"));
-        goto cleanup;
-    }
-
     if (virDomainGetInfo(dom, &dominfo) < 0) {
         vshError(ctl, "%s", _("Unable to get domain status"));
         goto cleanup;
@@ -718,38 +699,12 @@ cmdRunConsole(vshControl *ctl, virDomainPtr dom)
         goto cleanup;
     }
 
-    doc = virDomainGetXMLDesc(dom, 0);
+    vshPrintExtra(ctl, _("Connected to domain %s\n"), virDomainGetName(dom));
-    if (!doc)
+    vshPrintExtra(ctl, "%s", _("Escape character is ^]\n"));
-        goto cleanup;
+    if (vshRunConsole(dom, devname) == 0)
-
+        ret = TRUE;
-    xml = xmlReadDoc((const xmlChar *) doc, "domain.xml", NULL,
-                     XML_PARSE_NOENT | XML_PARSE_NONET |
-                     XML_PARSE_NOWARNING);
-    VIR_FREE(doc);
-    if (!xml)
-        goto cleanup;
-    ctxt = xmlXPathNewContext(xml);
-    if (!ctxt)
-        goto cleanup;
-
-    obj = xmlXPathEval(BAD_CAST "string(/domain/devices/console/@tty)", ctxt);
-    if ((obj != NULL) && ((obj->type == XPATH_STRING) &&
-                          (obj->stringval != NULL) && (obj->stringval[0] != 0))) {
-        vshPrintExtra(ctl, _("Connected to domain %s\n"), virDomainGetName(dom));
-        vshPrintExtra(ctl, "%s", _("Escape character is ^]\n"));
-        if (vshRunConsole((const char *)obj->stringval) == 0)
-            ret = TRUE;
-    } else {
-        vshPrintExtra(ctl, "%s", _("No console available for domain\n"));
-    }
-    xmlXPathFreeObject(obj);
 
  cleanup:
-    xmlXPathFreeContext(ctxt);
-    if (xml)
-        xmlFreeDoc(xml);
-    VIR_FREE(thisHost);
-    VIR_FREE(thatHost);
 
     return ret;
 }
@@ -759,6 +714,7 @@ cmdConsole(vshControl *ctl, const vshCmd *cmd)
 {
     virDomainPtr dom;
     int ret;
+    const char *devname;
 
     if (!vshConnectionUsability(ctl, ctl->conn))
         return FALSE;
@@ -766,7 +722,9 @@ cmdConsole(vshControl *ctl, const vshCmd *cmd)
     if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
         return FALSE;
 
-    ret = cmdRunConsole(ctl, dom);
+    devname = vshCommandOptString(cmd, "devname", NULL);
+
+    ret = cmdRunConsole(ctl, dom, devname);
 
     virDomainFree(dom);
     return ret;
@@ -1243,7 +1201,7 @@ cmdCreate(vshControl *ctl, const vshCmd *cmd)
                  virDomainGetName(dom), from);
 #ifndef WIN32
         if (console)
-            cmdRunConsole(ctl, dom);
+            cmdRunConsole(ctl, dom, NULL);
 #endif
         virDomainFree(dom);
     } else {
@@ -1408,7 +1366,7 @@ cmdStart(vshControl *ctl, const vshCmd *cmd)
                  virDomainGetName(dom));
 #ifndef WIN32
         if (console)
-            cmdRunConsole(ctl, dom);
+            cmdRunConsole(ctl, dom, NULL);
 #endif
     } else {
         vshError(ctl, _("Failed to start domain %s"), virDomainGetName(dom));
@@ -11135,6 +11093,14 @@ vshInit(vshControl *ctl)
     /* set up the signals handlers to catch disconnections */
     vshSetupSignals();
 
+    virEventRegisterImpl(virEventAddHandleImpl,
+                         virEventUpdateHandleImpl,
+                         virEventRemoveHandleImpl,
+                         virEventAddTimeoutImpl,
+                         virEventUpdateTimeoutImpl,
+                         virEventRemoveTimeoutImpl);
+    virEventInit();
+
     ctl->conn = virConnectOpenAuth(ctl->name,
                                    virConnectAuthPtrDefault,
                                    ctl->readonly ? VIR_CONNECT_RO : 0);

tools/virsh.pod

@@ -316,9 +316,12 @@ Configure a domain to be automatically started at boot.
 
 The option I<--disable> disables autostarting.
 
-=item B<console> I<domain-id>
+=item B<console> I<domain-id> [I<devname>]
 
-Connect the virtual serial console for the guest.
+Connect the virtual serial console for the guest. The optional
+I<devname> parameter refers to the device alias of an alternate
+console, serial or parallel device configured for the guest.
+If omitted, the primary console will be opened.
 
 =item B<create> I<FILE> optional I<--console> I<--paused>