From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([208.118.235.92]:52574)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <quintela@redhat.com>) id 1TCS3J-00040m-9J
	for qemu-devel@nongnu.org; Fri, 14 Sep 2012 05:19:15 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <quintela@redhat.com>) id 1TCS3D-00031h-6P
	for qemu-devel@nongnu.org; Fri, 14 Sep 2012 05:19:09 -0400
Received: from mx1.redhat.com ([209.132.183.28]:45740)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <quintela@redhat.com>) id 1TCS3C-00031T-V6
	for qemu-devel@nongnu.org; Fri, 14 Sep 2012 05:19:03 -0400
From: Juan Quintela <quintela@redhat.com>
In-Reply-To: <1347562697-15411-4-git-send-email-owasserm@redhat.com> (Orit
	Wasserman's message of "Thu, 13 Sep 2012 21:58:17 +0300")
References: <1347562697-15411-1-git-send-email-owasserm@redhat.com>
	<1347562697-15411-4-git-send-email-owasserm@redhat.com>
Date: Fri, 14 Sep 2012 11:17:36 +0200
Message-ID: <87vcfhrmj3.fsf@elfo.mitica>
MIME-Version: 1.0
Content-Type: text/plain
Subject: Re: [Qemu-devel] [PATCH v3 3/3] Fix address handling in
	inet_nonblocking_connect
Reply-To: quintela@redhat.com
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Orit Wasserman <owasserm@redhat.com>
Cc: kwolf@redhat.com, aliguori@us.ibm.com, mst@redhat.com, qemu-devel@nongnu.org, armbru@redhat.com, mdroth@linux.vnet.ibm.com, lcapitulino@redhat.com, pbonzini@redhat.com, akong@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.