Re: [Qemu-devel] [PATCH 04/18] char: add wait support for reconnect

[Yuanhan Liu <yuanhan.liu@linux.intel.com>]

On Fri, Apr 01, 2016 at 01:16:14PM +0200, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> Until now, 'wait' was solely used for listening sockets. However, it can
> also be useful for 'reconnect' socket kind, where the first open must
> succeed before continuing.
> 
> This allows for instance (with other support patches) having vhost-user
> wait for an initial connection to setup the vhost-net, before eventually
> accepting new connections.

I have met a tricky issue about this patch. Assume the socket exist in
before we start QEMU, but somehow we can not connect to it, say due to
permission error. In such case, QEMU would wait there forever, without
any error messages. I would assume it's a bug, right?

	--yliu
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  qemu-char.c | 31 +++++++++++++++++++++++--------
>  1 file changed, 23 insertions(+), 8 deletions(-)
> 
> diff --git a/qemu-char.c b/qemu-char.c
> index 8702931..3e25c08 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -3659,7 +3659,7 @@ static void qemu_chr_parse_socket(QemuOpts *opts, ChardevBackend *backend,
>                                    Error **errp)
>  {
>      bool is_listen      = qemu_opt_get_bool(opts, "server", false);
> -    bool is_waitconnect = is_listen && qemu_opt_get_bool(opts, "wait", true);
> +    bool is_wait        = qemu_opt_get_bool(opts, "wait", is_listen);
>      bool is_telnet      = qemu_opt_get_bool(opts, "telnet", false);
>      bool do_nodelay     = !qemu_opt_get_bool(opts, "delay", true);
>      int64_t reconnect   = qemu_opt_get_number(opts, "reconnect", 0);
> @@ -3696,7 +3696,7 @@ static void qemu_chr_parse_socket(QemuOpts *opts, ChardevBackend *backend,
>      sock->has_telnet = true;
>      sock->telnet = is_telnet;
>      sock->has_wait = true;
> -    sock->wait = is_waitconnect;
> +    sock->wait = is_wait;
>      sock->has_reconnect = true;
>      sock->reconnect = reconnect;
>      sock->tls_creds = g_strdup(tls_creds);
> @@ -4350,7 +4350,7 @@ static CharDriverState *qmp_chardev_open_socket(const char *id,
>      bool do_nodelay     = sock->has_nodelay ? sock->nodelay : false;
>      bool is_listen      = sock->has_server  ? sock->server  : true;
>      bool is_telnet      = sock->has_telnet  ? sock->telnet  : false;
> -    bool is_waitconnect = sock->has_wait    ? sock->wait    : false;
> +    bool is_wait        = sock->has_wait    ? sock->wait    : false;
>      int64_t reconnect   = sock->has_reconnect ? sock->reconnect : 0;
>      ChardevCommon *common = qapi_ChardevSocket_base(sock);
>      QIOChannelSocket *sioc = NULL;
> @@ -4424,7 +4424,7 @@ static CharDriverState *qmp_chardev_open_socket(const char *id,
>      }
>  
>      sioc = qio_channel_socket_new();
> -    if (s->reconnect_time) {
> +    if (s->reconnect_time && !is_wait) {
>          qio_channel_socket_connect_async(sioc, s->addr,
>                                           qemu_chr_socket_connected,
>                                           chr, NULL);
> @@ -4433,7 +4433,7 @@ static CharDriverState *qmp_chardev_open_socket(const char *id,
>              goto error;
>          }
>          s->listen_ioc = sioc;
> -        if (is_waitconnect) {
> +        if (is_wait) {
>              trace_char_socket_waiting(chr->filename);
>              tcp_chr_accept(QIO_CHANNEL(s->listen_ioc), G_IO_IN, chr);
>          }
> @@ -4443,9 +4443,24 @@ static CharDriverState *qmp_chardev_open_socket(const char *id,
>                  QIO_CHANNEL(s->listen_ioc), G_IO_IN, tcp_chr_accept, chr, NULL);
>          }
>      } else {
> -        if (qio_channel_socket_connect_sync(sioc, s->addr, errp) < 0) {
> -            goto error;
> -        }
> +        do {
> +            Error *err = NULL;
> +
> +            if (qio_channel_socket_connect_sync(sioc, s->addr, &err) < 0) {
> +                if (reconnect) {
> +                    trace_char_socket_reconnect_error(chr->label,
> +                                                      error_get_pretty(err));
> +                    error_free(err);
> +                    continue;
> +                } else {
> +                    error_propagate(errp, err);
> +                    goto error;
> +                }
> +            } else {
> +                break;
> +            }
> +        } while (true);
> +
>          tcp_chr_new_client(chr, sioc);
>          object_unref(OBJECT(sioc));
>      }
> -- 
> 2.5.5