Re: [libvirt] [PATCH 08/10] Re-write virsh console to use streams

2010-11-02 Thread Daniel P. Berrange
On Mon, Nov 01, 2010 at 03:55:08PM -0600, Eric Blake wrote:
> On 11/01/2010 10:12 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
> > ---
> >  .x-sc_avoid_write |1 +
> >  tools/Makefile.am |1 +
> >  tools/console.c   |  330 
> > -
> >  tools/console.h   |2 +-
> >  tools/virsh.c |   76 -
> 
> tools/virsh.pod changes?
> 
> > +if (con->terminalToStream.data[con->terminalToStream.offset] == 
> > CTRL_CLOSE_BRACKET) {
> > +con->quit = true;
> > +return;
> > +}
> 
> Is there any way to type an escape sequence, such as ^v in common stty
> usage, in order to allow sending a literal ^] through to the console
> instead of always making it quit?

Not at this time. This was also true of the old impl, so not a regression

> 
> >  
> > +if (con) {
> > +if (con->st)
> > +virStreamFree(con->st);
> 
> Should virStreamFree tolerate a NULL argument, at which point it should
> be added to the list in cfg.mk of free()-like functions that should not
> have an extra if() preceding usage?

Our public APIs all raise an error if you pass NULL to any of the
virXFree() APIs.

Daniel
-- 
|: Red Hat, Engineering, London-o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org-o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH 08/10] Re-write virsh console to use streams

2010-11-01 Thread Eric Blake
On 11/01/2010 10:12 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
> ---
>  .x-sc_avoid_write |1 +
>  tools/Makefile.am |1 +
>  tools/console.c   |  330 
> -
>  tools/console.h   |2 +-
>  tools/virsh.c |   76 -

tools/virsh.pod changes?

> +if (con->terminalToStream.data[con->terminalToStream.offset] == 
> CTRL_CLOSE_BRACKET) {
> +con->quit = true;
> +return;
> +}

Is there any way to type an escape sequence, such as ^v in common stty
usage, in order to allow sending a literal ^] through to the console
instead of always making it quit?

>  
> +if (con) {
> +if (con->st)
> +virStreamFree(con->st);

Should virStreamFree tolerate a NULL argument, at which point it should
be added to the list in cfg.mk of free()-like functions that should not
have an extra if() preceding usage?

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



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 08/10] Re-write virsh console to use streams

2010-11-01 Thread Daniel P. Berrange
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 |1 +
 tools/Makefile.am |1 +
 tools/console.c   |  330 -
 tools/console.h   |2 +-
 tools/virsh.c |   76 -
 5 files changed, 275 insertions(+), 135 deletions(-)

diff --git a/.x-sc_avoid_write b/.x-sc_avoid_write
index 1f893b8..7bb8078 100644
--- a/.x-sc_avoid_write
+++ b/.x-sc_avoid_write
@@ -5,3 +5,4 @@
 ^src/xen/xend_internal\.c$
 ^daemon/libvirtd.c$
 ^gnulib/
+^tools/console.c$
diff --git a/tools/Makefile.am b/tools/Makefile.am
index bfe4455..f6f19bd 100644
--- a/tools/Makefile.am
+++ b/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)
diff --git a/tools/console.c b/tools/console.c
index 60e62e2..d003ab4 100644
--- a/tools/console.c
+++ b/tools/console.c
@@ -34,15 +34,41 @@
 # include 
 # include 
 # include 
+# include 
 
-# include "console.h"
 # include "internal.h"
+# include "console.h"
 # include "logging.h"
 # include "util.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;
@@ -61,22 +87,191 @@ 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;
+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);
+
+