Re: [Qemu-devel] [PATCH v4 3/4] Fix address handling in inet_nonblocking_connect

[Amos Kong <akong@redhat.com>]

On 23/09/12 22:49, Orit Wasserman wrote:
> getaddrinfo can give us a list of addresses, but we only try to
> connect to the first one. If that fails we never proceed to
> the next one.  This is common on desktop setups that often have ipv6
> configured but not actually working.
>
> To fix this make inet_connect_nonblocking retry connection with a different
> address.
> callers on inet_nonblocking_connect register a callback function that will
> be called when connect opertion completes, in case of failure the fd will have
> a negative value
>
> Signed-off-by: Orit Wasserman <owasserm@redhat.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>   migration-tcp.c |   37 +++++--------------
>   qemu-char.c     |    2 +-
>   qemu-sockets.c  |  108 ++++++++++++++++++++++++++++++++++++++++++++++---------
>   qemu_socket.h   |   33 ++++++++++++-----
>   4 files changed, 125 insertions(+), 55 deletions(-)

...

> -int inet_connect_opts(QemuOpts *opts, bool block, bool *in_progress,
> -                      Error **errp)
> +int inet_connect_opts(QemuOpts *opts, Error **errp, ConnectHandler *callback,
> +                      void *opaque)
>   {
>       struct addrinfo *res, *e;
>       int sock = -1;
> +    bool in_progress = false;
> +    ConnectState *connect_state = NULL;
>
>       res = inet_parse_connect_opts(opts, errp);
>       if (!res) {
>           return -1;
>       }
>
> -    if (in_progress) {
> -        *in_progress = false;
> -    }
> -
>       for (e = res; e != NULL; e = e->ai_next) {
> -        sock = inet_connect_addr(e, block, in_progress);
> -        if (sock >= 0) {
> +        if (callback != NULL) {
> +            connect_state = g_malloc0(sizeof(*connect_state));

Repeatedly malloc insider for loop without releasing.

> +            connect_state->addr_list = res;
> +            connect_state->current_addr = e;
> +            connect_state->callback = callback;
> +            connect_state->opaque = opaque;
> +        }
> +        sock = inet_connect_addr(e, &in_progress, connect_state);
> +        if (in_progress) {
> +            return sock;
> +        } else if (sock >= 0) {
> +            /* non blocking socket immediate success, call callback */
> +            if (callback != NULL) {
> +                callback(sock, opaque);
> +            }
>               break;
>           }

We should free() connect_state here, or moving allocate code before for 
loop (don't forget to check callback != NULL ;)

Amos.

>       }
>       if (sock < 0) {
>           error_set(errp, QERR_SOCKET_CONNECT_FAILED);
>       }
> +    g_free(connect_state);
>       freeaddrinfo(res);
>       return sock;
>   }


-- 
			Amos.