From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:58816) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TG5fh-0007sX-CB for qemu-devel@nongnu.org; Mon, 24 Sep 2012 06:13:55 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TG5fg-0005c1-8Z for qemu-devel@nongnu.org; Mon, 24 Sep 2012 06:13:49 -0400 Received: from mx1.redhat.com ([209.132.183.28]:35344) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TG5fg-0005bu-0G for qemu-devel@nongnu.org; Mon, 24 Sep 2012 06:13:48 -0400 Message-ID: <50603224.3050708@redhat.com> Date: Mon, 24 Sep 2012 12:12:52 +0200 From: Orit Wasserman MIME-Version: 1.0 References: <1348411747-29804-1-git-send-email-owasserm@redhat.com> <1348411747-29804-4-git-send-email-owasserm@redhat.com> <50603070.6090202@redhat.com> In-Reply-To: <50603070.6090202@redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v4 3/4] Fix address handling in inet_nonblocking_connect List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Amos Kong 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 09/24/2012 12:05 PM, Amos Kong wrote: > 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 >> Signed-off-by: Michael S. Tsirkin >> --- >> 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. > oops ... I will move it outside of the loop. >> + 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 ;) > right. > Amos. > >> } >> if (sock < 0) { >> error_set(errp, QERR_SOCKET_CONNECT_FAILED); >> } >> + g_free(connect_state); >> freeaddrinfo(res); >> return sock; >> } > >