From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:34315) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TEFhY-0004Mi-3Z for qemu-devel@nongnu.org; Wed, 19 Sep 2012 04:32:09 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TEFhW-00051U-Rs for qemu-devel@nongnu.org; Wed, 19 Sep 2012 04:32:08 -0400 Received: from mx1.redhat.com ([209.132.183.28]:12002) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TEFhW-00051L-JC for qemu-devel@nongnu.org; Wed, 19 Sep 2012 04:32:06 -0400 Message-ID: <505982FD.3020601@redhat.com> Date: Wed, 19 Sep 2012 16:31:57 +0800 From: Amos Kong MIME-Version: 1.0 References: <1347562697-15411-1-git-send-email-owasserm@redhat.com> <1347562697-15411-4-git-send-email-owasserm@redhat.com> In-Reply-To: <1347562697-15411-4-git-send-email-owasserm@redhat.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v3 3/3] Fix address handling in inet_nonblocking_connect List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Orit Wasserman Cc: kwolf@redhat.com, aliguori@us.ibm.com, mdroth@linux.vnet.ibm.com, mst@redhat.com, quintela@redhat.com, armbru@redhat.com, qemu-devel@nongnu.org, pbonzini@redhat.com, lcapitulino@redhat.com On 14/09/12 02:58, 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 diffe= rent > address. > callers on inet_nonblocking_connect register a callback function that w= ill > be called when connect opertion completes, in case of failure the fd wi= ll have > a negative value Hi Orit, We almost get to the destination :) > Signed-off-by: Orit Wasserman > Signed-off-by: Michael S. Tsirkin > --- > migration-tcp.c | 29 +++++----------- > qemu-char.c | 2 +- > qemu-sockets.c | 95 ++++++++++++++++++++++++++++++++++++++++++++++= +------- > qemu_socket.h | 13 ++++++-- > 4 files changed, 102 insertions(+), 37 deletions(-) > ... > diff --git a/qemu-sockets.c b/qemu-sockets.c > index 212075d..d321c58 100644 > --- a/qemu-sockets.c > +++ b/qemu-sockets.c > @@ -24,6 +24,7 @@ > > #include "qemu_socket.h" > #include "qemu-common.h" /* for qemu_isdigit */ > +#include "main-loop.h" > > #ifndef AI_ADDRCONFIG > # define AI_ADDRCONFIG 0 > @@ -217,11 +218,69 @@ listen: > ((rc) =3D=3D -EINPROGRESS) > #endif > > +/* Struct to store connect state for non blocking connect */ > +typedef struct ConnectState { > + int fd; > + struct addrinfo *addr_list; > + struct addrinfo *current_addr; > + ConnectHandler *callback; > + void *opaque; > + Error *errp; This member is not used. > +} ConnectState; # make (gcc-4.4.6-4.el6.x86_64) CC qemu-sockets.o qemu-sockets.c:229: error: redefinition of typedef =91ConnectState=92 qemu_socket.h:46: note: previous declaration of =91ConnectState=92 was he= re make: *** [qemu-sockets.o] Error 1 struct ConnectState { int fd; struct addrinfo *addr_list; struct addrinfo *current_addr; ConnectHandler *callback; void *opaque; }; > + > static int inet_connect_addr(struct addrinfo *addr, bool block, > - bool *in_progress) > + bool *in_progress, ConnectState *connect_= state); > + > +static void wait_for_connect(void *opaque) > +{ > + ConnectState *s =3D opaque; > + int val =3D 0, rc =3D 0; > + socklen_t valsize =3D sizeof(val); > + bool in_progress =3D false; > + > + qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL); > + > + do { > + rc =3D getsockopt(s->fd, SOL_SOCKET, SO_ERROR, (void *) &val, = &valsize); > + } while (rc =3D=3D -1 && socket_error() =3D=3D EINTR); # man getsockopt RETURN VALUE On success, zero is returned. On error, -1 is returned, and=20 errno is set appropriately. .. original code: .. .. do { .. ret =3D getsockopt(s->fd, SOL_SOCKET, SO_ERROR, (void *) &val,= =20 &valsize); .. } while (ret =3D=3D -1 && (socket_error()) =3D=3D EINTR); .. .. if (ret < 0) { // ret equals to -1, and socket_error() !=3D EINTR If ret < 0, just set the error state, but the callback function is not=20 set to NULL. Then tcp_wait_for_connect() will be called again, and retry to checking=20 current socket by getsockopt(). But in this patch, we will abandon current socket, and connect next=20 address on the list. We should reserve that block, and move qemu_set_fd_handler2(...NULL..)=20 below it. .. migrate_fd_error(s); .. return; .. } .. .. qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL); > + /* update rc to contain error details */ > + if (!rc && val) { > + rc =3D -val; another issue: val >=3D 0 all the time? > + } > + > + /* connect error */ > + if (rc < 0) { > + closesocket(s->fd); > + s->fd =3D rc; > + } > + > + /* try to connect to the next address on the list */ > + while (s->current_addr->ai_next !=3D NULL && s->fd < 0) { > + s->current_addr =3D s->current_addr->ai_next; > + s->fd =3D inet_connect_addr(s->current_addr, false, &in_progre= ss, s); > + /* connect in progress */ > + if (in_progress) { > + return; > + } > + } > + > + freeaddrinfo(s->addr_list); > + if (s->callback) { > + s->callback(s->fd, s->opaque); > + } > + g_free(s); > + return; > +} --=20 Amos.