Re: [Qemu-devel] [PATCH 10/12] chardev: honour the reconnect setting in tcp_chr_wait_connected

[Marc-André Lureau]

Hi

On Tue, Jan 15, 2019 at 6:54 PM Daniel P. Berrangé <address@hidden> wrote:
>
> If establishing a client connection fails, the tcp_chr_wait_connected
> method should sleep for the reconnect timeout and then retry the
> attempt. This ensures the callers don't immediately abort with an
> error when the initial connection fails.

Obviously, that function is already a bit problematic, since it may
block the thread with no easy way to cancel (well, the _sync functions
in general..).

You change doesn't make it much worse, but it will now loop also in
case of errors. That's what reconnect_time is supposed to do, so

Reviewed-by: Marc-André Lureau <address@hidden>

>
> Signed-off-by: Daniel P. Berrangé <address@hidden>
> ---
>  chardev/char-socket.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/chardev/char-socket.c b/chardev/char-socket.c
> index 96a60eb105..91d775e9c5 100644
> --- a/chardev/char-socket.c
> +++ b/chardev/char-socket.c
> @@ -957,8 +957,15 @@ static int tcp_chr_wait_connected(Chardev *chr, Error 
> **errp)
>          if (s->is_listen) {
>              tcp_chr_accept_server_sync(chr);
>          } else {
> -            if (tcp_chr_connect_client_sync(chr, errp) < 0) {
> -                return -1;
> +            Error *err = NULL;
> +            if (tcp_chr_connect_client_sync(chr, &err) < 0) {
> +                if (s->reconnect_time) {
> +                    error_free(err);
> +                    g_usleep(s->reconnect_time);
> +                } else {
> +                    error_propagate(errp, err);
> +                    return -1;
> +                }
>              }
>          }
>      }
> --
> 2.20.1
>