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

[Juan Quintela <quintela@redhat.com>]

Orit Wasserman <owasserm@redhat.com> 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>

Reviewed-by: Juan Quintela <quintela@redhat.com>

Just thinking out loud to be if I understood this correctly

> +    do {
> +        rc = getsockopt(s->fd, SOL_SOCKET, SO_ERROR, (void *) &val, &valsize);
> +    } while (rc == -1 && socket_error() == EINTR);

"rc"" return the error code of getsockopt()
and "val" returns the error code of the socket if there is one.

> +
> +    /* update rc to contain error details */
> +    if (!rc && val) {
> +        rc = -val;
> +    }

If getsockopt() succeeds, we "reuse" "rc" error code to have the socket
error code.  If you have to resent this, could we improve the comment here?
I have to go to the manual page of getsockopt() that don't likes
SO_ERROR to try to understand what this completely un-intuitive (at
least for me) two lines of code do.


> +
> +    /* connect error */
> +    if (rc < 0) {
> +        closesocket(s->fd);
> +        s->fd = rc;
> +    }

If there is any error (getsockopt or in the socket), we just close the
fd and update the error code.

Head hurts.

Thanks, Juan.