[libvirt] [PATCH 6/7] Re-write virsh console to use streams

[Eric Blake]

On 08/17/2010 11:02 AM, Daniel P. Berrange wrote:
> 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
> ---
>  tools/Makefile.am |    1 +
>  tools/console.c   |  330 ++++++++++++++++++++++++++++++++++++++++-------------
>  tools/console.h   |    2 +-
>  tools/virsh.c     |   76 ++++---------
>  4 files changed, 274 insertions(+), 135 deletions(-)

> +
> +struct virConsoleBuffer {
> +    size_t length;
> +    size_t offset;

s/size_t/off_t/ for offset? (but see below)

> +    char *data;
> +};
> +
> +typedef struct virConsole virConsole;
> +typedef virConsole *virConsolePtr;
> +struct virConsole {
> +    virStreamPtr st;
> +    int quit;

s/int/bool/, and adjust clients to use false/true instead of 0/1

> +
> +    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;
> @@ -61,22 +86,192 @@ cfmakeraw (struct termios *attr)
>  }
>  # endif /* !HAVE_CFMAKERAW */
>  
> -int vshRunConsole(const char *tty) {
> -    int ttyfd, ret = -1;
> +static void
> +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;

Oh, never mind.  size_t for offset is probably just fine after all.

> +        int got;
> +
> +        if (avail < 1024) {
> +            if (VIR_REALLOC_N(con->streamToTerminal.data,
> +                              con->streamToTerminal.length + 1024) < 0) {

/me Note to self - another VIR_EXPAND_N or VIR_RESIZE_N candidate.

> +                virReportOOMError();
> +                con->quit = 1;
> +                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 = 1;
> +            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) {
> +            virErrorPtr err = virGetLastError();
> +            con->quit = 1;
> +            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)
> +            {}

/me Note to self - a VIR_SHRINK_N candidate.

> +            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 = 1;
> +    }
> +}
> +
> +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 = 1;
> +                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 = 1;
> +            }
> +            return;
> +        }
> +        if (got == 0) {
> +            con->quit = 1;
> +            return;
> +        }
> +        if (con->terminalToStream.data[con->terminalToStream.offset] == CTRL_CLOSE_BRACKET) {
> +            con->quit = 1;
> +            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 = 1;
> +    }
> +}
> +
> +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);

Do we want to use safewrite here?

Conditional ACK, if those are deemed easy nits to fix.

-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 619 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20100819/68225201/attachment-0001.sig>