Re: [Qemu-devel] [PATCH] qemu-char: Allow a chardev to reconnect if disconnected

[Corey Minyard]

On 04/10/2014 06:43 AM, address@hidden wrote:
> From: Huangweidong <address@hidden>
>
> Allow a socket chardev reconnect if the connection drops while in use.
>
> Signed-off-by: Huangweidong <address@hidden>
> Signed-off-by: Gonglei <address@hidden>
> ---
> This patch is modified according to corey's patch. Some changes below:
> 1. IMO it's unnecessary that chardev reconnect if it fails to connect at 
> startup.
> Qemu exit in this scene. In this way the patch does not change interface of 
> chardev.
> It would be much more simple.

I believe that it should not stop qemu if it fails at startup. 
Otherwise you constrain the start order and you can prevent a server
from coming up because of a missing resource that may not be that
critical at the moment.  With the current implementation, client sockets
really aren't that useful for a critical system.  Reconnecting makes it
usable in a critical system.

> 2. I set the reconnect timer one second, just like pty.

I'm not too picky about the time.  A couple of things about this:

With this patch, the default behavior changes to reconnect.  That might
cause issues for some users.  Adding a configurable timeout is easy if
you have to specify something on the command line, that's why I did it.

Also, if something is listening to connect/disconnect events from the
device, it will get a connect then disconnect every second.  It's
probably better to wait until the connection is actually established
before you report it up.

-corey
>
>  include/sysemu/char.h |  2 ++
>  qemu-char.c           | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 52 insertions(+)
>
> diff --git a/include/sysemu/char.h b/include/sysemu/char.h
> index b81a6ff..f646ac8 100644
> --- a/include/sysemu/char.h
> +++ b/include/sysemu/char.h
> @@ -19,6 +19,7 @@
>  #define CHR_EVENT_MUX_OUT 4 /* mux-focus will move on */
>  #define CHR_EVENT_CLOSED  5 /* connection closed */
>  
> +#define CHR_SOCK_RECONNECT_TIME 1 /* reconnection time (second) */
>  
>  #define CHR_IOCTL_SERIAL_SET_PARAMS   1
>  typedef struct {
> @@ -82,6 +83,7 @@ struct CharDriverState {
>      guint fd_in_tag;
>      QemuOpts *opts;
>      QTAILQ_ENTRY(CharDriverState) next;
> +    QEMUTimer *recon_timer;
>  };
>  
>  /**
> diff --git a/qemu-char.c b/qemu-char.c
> index 54ed244..a87a345 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -96,9 +96,17 @@ void qemu_chr_be_event(CharDriverState *s, int event)
>      /* Keep track if the char device is open */
>      switch (event) {
>          case CHR_EVENT_OPENED:
> +            if (s->recon_timer) {
> +                timer_del(s->recon_timer);
> +            }
>              s->be_open = 1;
>              break;
>          case CHR_EVENT_CLOSED:
> +            if (s->recon_timer) {
> +                timer_mod(s->recon_timer,
> +                (get_clock() +
> +                 (CHR_SOCK_RECONNECT_TIME * get_ticks_per_sec())));
> +            }
>              s->be_open = 0;
>              break;
>      }
> @@ -2619,6 +2627,43 @@ static void tcp_chr_close(CharDriverState *chr)
>      qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
>  }
>  
> +static void recon_timeout(void *opaque)
> +{
> +    CharDriverState *chr = opaque;
> +    QemuOpts *opts = chr->opts;
> +    TCPCharDriver *tcp = (TCPCharDriver *)chr->opaque;
> +    int fd = -1;
> +    Error *local_err = NULL;
> +
> +    if (chr->be_open) {
> +        return;
> +    }
> +
> +    if (tcp->is_unix) {
> +        fd = unix_connect_opts(opts, &local_err, NULL, NULL);
> +    } else {
> +        fd = inet_connect_opts(opts, &local_err, NULL, NULL);
> +    }
> +
> +    if (fd < 0) {
> +        goto fail;
> +    }
> +
> +    tcp->fd = fd;
> +    socket_set_nodelay(fd);
> +    tcp->chan = io_channel_from_socket(tcp->fd);
> +    tcp_chr_connect(chr);
> +    printf("chardev: socket reconnect sucess\n");
> +    return;
> +
> +fail:
> +    if (local_err) {
> +        qerror_report_err(local_err);
> +        error_free(local_err);
> +    }
> +    qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
> +}
> +
>  static CharDriverState *qemu_chr_open_socket_fd(int fd, bool do_nodelay,
>                                                  bool is_listen, bool 
> is_telnet,
>                                                  bool is_waitconnect,
> @@ -2693,6 +2738,11 @@ static CharDriverState *qemu_chr_open_socket_fd(int 
> fd, bool do_nodelay,
>          socket_set_nodelay(fd);
>          s->chan = io_channel_from_socket(s->fd);
>          tcp_chr_connect(chr);
> +        chr->recon_timer = timer_new(QEMU_CLOCK_REALTIME, SCALE_NS,
> +                                 recon_timeout, chr);
> +        timer_mod(chr->recon_timer,
> +                   (get_clock() +
> +                    (CHR_SOCK_RECONNECT_TIME * get_ticks_per_sec())));
>      }
>  
>      if (is_listen && is_waitconnect) {