[libvirt] [PATCH 05/10] virsh: fix console stream error reporting

[Daniel P. Berrange]

On Wed, Oct 12, 2011 at 03:43:15PM +0200, Peter Krempa wrote:
> This patch subscribes the console stream event callback to handle errors
> (and stream abortion) from the daemon. The functionality was (partly)
> implemented in the callback, but the error events were not registered.
> ---
>  tools/console.c |   21 ++++++++++++++-------
>  1 files changed, 14 insertions(+), 7 deletions(-)
> 
> diff --git a/tools/console.c b/tools/console.c
> index 0f85bc7..3913c42 100644
> --- a/tools/console.c
> +++ b/tools/console.c
> @@ -109,6 +109,12 @@ virConsoleEventOnStream(virStreamPtr st,
>  {
>      virConsolePtr con = opaque;
> 
> +    if (events & VIR_STREAM_EVENT_ERROR ||
> +        events & VIR_STREAM_EVENT_HANGUP) {
> +        virConsoleShutdown(con);
> +        return;
> +    }

There is already code for this further down in virConsoleEventOnStream.

The check for ERROR/HANGUP should come *last* in the method, because
you might also have READABLE set at the same time. We need to make
sure we read any final data.

>      if (events & VIR_STREAM_EVENT_READABLE) {
>          size_t avail = con->streamToTerminal.length -
>              con->streamToTerminal.offset;
> @@ -169,12 +175,10 @@ virConsoleEventOnStream(virStreamPtr st,
>      }
>      if (!con->terminalToStream.offset)
>          virStreamEventUpdateCallback(con->st,
> -                                     VIR_STREAM_EVENT_READABLE);
> +                                     ( VIR_STREAM_EVENT_READABLE |
> +                                       VIR_STREAM_EVENT_HANGUP |
> +                                       VIR_STREAM_EVENT_ERROR ));
> 
> -    if (events & VIR_STREAM_EVENT_ERROR ||
> -        events & VIR_STREAM_EVENT_HANGUP) {
> -        virConsoleShutdown(con);
> -    }
>  }
> 
>  static void
> @@ -346,7 +350,9 @@ int vshRunConsole(virDomainPtr dom, const char *dev_name)
>                                           NULL);
> 
>      virStreamEventAddCallback(con->st,
> -                              VIR_STREAM_EVENT_READABLE,
> +                              (VIR_STREAM_EVENT_READABLE |
> +                               VIR_STREAM_EVENT_HANGUP |
> +                               VIR_STREAM_EVENT_ERROR),
>                                virConsoleEventOnStream,
>                                con,
>                                NULL);

As with the other patch, this is not required. HANGUP/ERROR are
always enabled, if you have READABLE or WRITABLE set.

> @@ -356,7 +362,8 @@ int vshRunConsole(virDomainPtr dom, const char *dev_name)
>              break;
>      }
> 
> -    ret = 0;
> +    if (!virGetLastError())
> +        ret = 0;

This looks like the only bit we might need, but I'm not entirely
convinced. If the user aborts the console with Ctrl-], then I don't
think we need to print the error from virStreamAbort. They know
they just aborted it


Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|