diff mbox

[v4,3/4] Fix address handling in inet_nonblocking_connect

Message ID 1348411747-29804-4-git-send-email-owasserm@redhat.com
State New
Headers show

Commit Message

Orit Wasserman Sept. 23, 2012, 2:49 p.m. UTC
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(-)

Comments

Markus Armbruster Sept. 24, 2012, 9:54 a.m. UTC | #1
Orit Wasserman <owasserm@redhat.com> writes:

> 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(-)
>
> diff --git a/migration-tcp.c b/migration-tcp.c
> index 7f6ad98..a15c2b8 100644
> --- a/migration-tcp.c
> +++ b/migration-tcp.c
> @@ -53,54 +53,35 @@ static int tcp_close(MigrationState *s)
>      return r;
>  }
>  
> -static void tcp_wait_for_connect(void *opaque)
> +static void tcp_wait_for_connect(int fd, void *opaque)
>  {
>      MigrationState *s = opaque;
> -    int val, ret;
> -    socklen_t valsize = sizeof(val);
>  
> -    DPRINTF("connect completed\n");
> -    do {
> -        ret = getsockopt(s->fd, SOL_SOCKET, SO_ERROR, (void *) &val, &valsize);
> -    } while (ret == -1 && (socket_error()) == EINTR);
> -
> -    if (ret < 0) {
> +    if (fd < 0) {
> +        DPRINTF("migrate connect error\n");
> +        s->fd = -1;
>          migrate_fd_error(s);
> -        return;
> -    }
> -
> -    qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL);
> -
> -    if (val == 0)
> +    } else {
> +        DPRINTF("migrate connect success\n");
> +        s->fd = fd;
>          migrate_fd_connect(s);
> -    else {
> -        DPRINTF("error connecting %d\n", val);
> -        migrate_fd_error(s);
>      }
>  }
>  
>  int tcp_start_outgoing_migration(MigrationState *s, const char *host_port,
>                                   Error **errp)
>  {
> -    bool in_progress;
> -
>      s->get_error = socket_errno;
>      s->write = socket_write;
>      s->close = tcp_close;
>  
> -    s->fd = inet_nonblocking_connect(host_port, &in_progress, errp);
> +    s->fd = inet_nonblocking_connect(host_port, tcp_wait_for_connect, s,
> +                                     errp);
>      if (error_is_set(errp)) {
>          migrate_fd_error(s);
>          return -1;
>      }
>  
> -    if (in_progress) {
> -        DPRINTF("connect in progress\n");
> -        qemu_set_fd_handler2(s->fd, NULL, NULL, tcp_wait_for_connect, s);
> -    } else {
> -        migrate_fd_connect(s);
> -    }
> -
>      return 0;
>  }
>  
> diff --git a/qemu-char.c b/qemu-char.c
> index 13b87b5..b082bae 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -2456,7 +2456,7 @@ static CharDriverState *qemu_chr_open_socket(QemuOpts *opts)
>          if (is_listen) {
>              fd = inet_listen_opts(opts, 0, NULL);
>          } else {
> -            fd = inet_connect_opts(opts, true, NULL, NULL);
> +            fd = inet_connect_opts(opts, NULL, NULL, NULL);
>          }
>      }
>      if (fd < 0) {
> diff --git a/qemu-sockets.c b/qemu-sockets.c
> index 99a1500..9a4be15 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
> @@ -214,8 +215,64 @@ listen:
>      ((rc) == -EINPROGRESS)
>  #endif
>  
> -static int inet_connect_addr(struct addrinfo *addr, bool block,
> -                             bool *in_progress)
> +/* Struct to store connect state for non blocking connect */
> +typedef struct ConnectState ConnectState;
> +
> +struct ConnectState {
> +    int fd;
> +    struct addrinfo *addr_list;
> +    struct addrinfo *current_addr;
> +    ConnectHandler *callback;
> +    void *opaque;
> +};

Suggest not to separate the typedef and the struct declaration.

> +
> +static int inet_connect_addr(struct addrinfo *addr, bool *in_progress,
> +                             ConnectState *connect_state);
> +
> +static void wait_for_connect(void *opaque)
> +{
> +    ConnectState *s = opaque;
> +    int val = 0, rc = 0;
> +    socklen_t valsize = sizeof(val);
> +    bool in_progress = false;

Dead initialization.

> +
> +    qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL);
> +
> +    do {
> +        rc = getsockopt(s->fd, SOL_SOCKET, SO_ERROR, (void *) &val, &valsize);
> +    } while (rc == -1 && socket_error() == EINTR);
> +
> +    /* update rc to contain error */
> +    if (!rc && val) {
> +        rc = -1;
> +    }
> +
> +    /* connect error */
> +    if (rc < 0) {
> +        closesocket(s->fd);
> +        s->fd = rc;
> +    }
> +
> +    /* try to connect to the next address on the list */
> +    while (s->current_addr->ai_next != NULL && s->fd < 0) {
> +        s->current_addr = s->current_addr->ai_next;
> +        s->fd = inet_connect_addr(s->current_addr, &in_progress, 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;
> +}
> +
> +static int inet_connect_addr(struct addrinfo *addr, bool *in_progress,
> +                             ConnectState *connect_state)
>  {
>      int sock, rc;
>  
       if (in_progress) {
           *in_progress = false;
       }

All callers always pass non-null in_progress.  Suggest to drop the test.

> @@ -230,7 +287,7 @@ static int inet_connect_addr(struct addrinfo *addr, bool block,
>          return -1;
>      }
>      setsockopt(sock, SOL_SOCKET, SO_REUSEADDR, &on, sizeof(on));
> -    if (!block) {
> +    if (connect_state != NULL) {
>          socket_set_nonblock(sock);
>      }
>      /* connect to peer */
> @@ -241,7 +298,10 @@ static int inet_connect_addr(struct addrinfo *addr, bool block,
>          }
>      } while (rc == -EINTR);
>  
> -    if (!block && QEMU_SOCKET_RC_INPROGRESS(rc)) {
> +    if (connect_state != NULL && QEMU_SOCKET_RC_INPROGRESS(rc)) {
> +        connect_state->fd = sock;
> +        qemu_set_fd_handler2(sock, NULL, NULL, wait_for_connect,
> +                             connect_state);
>          if (in_progress) {
>              *in_progress = true;
>          }

Another superfluous test of in_progress.

> @@ -260,6 +320,7 @@ static struct addrinfo *inet_parse_connect_opts(QemuOpts *opts, Error **errp)
>      const char *port;
>  
>      memset(&ai, 0, sizeof(ai));
> +
>      ai.ai_flags = AI_CANONNAME | AI_ADDRCONFIG;
>      ai.ai_family = PF_UNSPEC;
>      ai.ai_socktype = SOCK_STREAM;
> @@ -291,30 +352,42 @@ static struct addrinfo *inet_parse_connect_opts(QemuOpts *opts, Error **errp)
>      return res;
>  }
>  
> -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));
> +            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;
>          }
>      }
>      if (sock < 0) {
>          error_set(errp, QERR_SOCKET_CONNECT_FAILED);
>      }
> +    g_free(connect_state);
>      freeaddrinfo(res);
>      return sock;
>  }
> @@ -519,7 +592,7 @@ int inet_connect(const char *str, Error **errp)
>  
>      opts = qemu_opts_create(&dummy_opts, NULL, 0, NULL);
>      if (inet_parse(opts, str) == 0) {
> -        sock = inet_connect_opts(opts, true, NULL, errp);
> +        sock = inet_connect_opts(opts, errp, NULL, NULL);
>      } else {
>          error_set(errp, QERR_SOCKET_CREATE_FAILED);
>      }
> @@ -527,16 +600,17 @@ int inet_connect(const char *str, Error **errp)
>      return sock;
>  }
>  
> -
> -int inet_nonblocking_connect(const char *str, bool *in_progress,
> -                             Error **errp)
> +int inet_nonblocking_connect(const char *str, ConnectHandler *callback,
> +                             void *opaque, Error **errp)
>  {
>      QemuOpts *opts;
>      int sock = -1;
>  
> +    g_assert(callback != NULL);
> +
>      opts = qemu_opts_create(&dummy_opts, NULL, 0, NULL);
>      if (inet_parse(opts, str) == 0) {
> -        sock = inet_connect_opts(opts, false, in_progress, errp);
> +        sock = inet_connect_opts(opts, errp, callback, opaque);
>      } else {
>          error_set(errp, QERR_SOCKET_CREATE_FAILED);
>      }
> diff --git a/qemu_socket.h b/qemu_socket.h
> index 3247fb7..da93509 100644
> --- a/qemu_socket.h
> +++ b/qemu_socket.h
> @@ -38,20 +38,31 @@ void socket_set_block(int fd);
>  void socket_set_nonblock(int fd);
>  int send_all(int fd, const void *buf, int len1);
>  
> +/* callback function for nonblocking connect
> + * vaild fd on success, negative error code on failure
> + */
> +typedef void ConnectHandler(int fd, void *opaque);
> +
>  /* New, ipv6-ready socket helper functions, see qemu-sockets.c */
>  int inet_listen_opts(QemuOpts *opts, int port_offset, Error **errp);
>  int inet_listen(const char *str, char *ostr, int olen,
>                  int socktype, int port_offset, Error **errp);
>  /**
>   * Connect function
> + * Use callback for nonblocking connect.
> + *

With all the talk on non-blocking below, this sentence feels redundant.

>   * Returns -1 in case of error, fd in case of success
> + * For non-blocking connect calls the callback function
> + * with fd in case of success or -1 in case of error.

Suggests callback is *always* called, which isn't true: it's only called
when when inet_connect_opts() succeeds and returns the fd.

>   * @opts: QEMU options
> - * @block: set true for blocking socket
> - * @in_progress: set to true in case of on going connect
> + * @callback: callback function that is called when connect completes,
> + *            not NULL for nonblocking.

A few negations too many for my taste %-}

> + * @opaque: opaque for callback function
>   * @errp: set in case of an error
>   **/

Let me try:

/**
 * Create a socket and connect it to an address.
 *
 * @opts: QEMU options, recognized parameters strings "host" and "port",
 *        bools "ipv4" and "ipv6".
 * @errp: set on error
 * @callback: callback function for non-blocking connect
 * @opaque: opaque for callback function
 *
 * Returns: -1 on error, file descriptor on success.
 *
 * If @callback is non-null, the connect is non-blocking.  If this
 * function succeeds, callback will be called when the connection
 * completes, with the file descriptor on success, or -1 on error.
 */

Also closer to Gtk-doc style we seem to be adopting.

I dislike the use of errp here, but let's ignore that for now.

> -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);
> +
>  /**
>   * Blocking connect
>   * Returns -1 in case of error, fd in case of success
> @@ -61,14 +72,18 @@ int inet_connect_opts(QemuOpts *opts, bool block, bool *in_progress,
>  int inet_connect(const char *str, Error **errp);
>  
>  /**
> - * Nonblocking connect
> - * Returns -1 in case of error, fd in case of success
> + * Non-blocking connect

Incorrect spelling "nonblocking" is from PATCH 2/4.  Suggest to fix it
there.

> + * Returns -1 in case of immediate error.
> + * Calls the callback function with fd in case of success or -1 in case of
> + * error.
>   * @str: address string
> - * @in_progress: set to true in case of on going connect
> + * @callback: callback function that is called when connect completes,
> + *            cannot be NULL.
> + * @opaque: opaque for callback function
>   * @errp: set in case of an error
>   **/
> -int inet_nonblocking_connect(const char *str, bool *in_progress,
> -                             Error **errp);
> +int inet_nonblocking_connect(const char *str, ConnectHandler *callback,
> +                             void *opaque, Error **errp);
>  
>  int inet_dgram_opts(QemuOpts *opts);
>  const char *inet_strfamily(int family);
Amos Kong Sept. 24, 2012, 10:05 a.m. UTC | #2
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;
>   }
Orit Wasserman Sept. 24, 2012, 10:11 a.m. UTC | #3
On 09/24/2012 12:10 PM, Michael S. Tsirkin wrote:
> On Sun, Sep 23, 2012 at 04:49:06PM +0200, Orit Wasserman wrote:
>> diff --git a/qemu_socket.h b/qemu_socket.h
>> index 3247fb7..da93509 100644
>> --- a/qemu_socket.h
>> +++ b/qemu_socket.h
>> @@ -38,20 +38,31 @@ void socket_set_block(int fd);
>>  void socket_set_nonblock(int fd);
>>  int send_all(int fd, const void *buf, int len1);
>>  
>> +/* callback function for nonblocking connect
>> + * vaild fd on success, negative error code on failure
> 
> typo
> 
>> + */
>> +typedef void ConnectHandler(int fd, void *opaque);
> 
> Can we rename this NonBlockingConnectHandler?
Of course.
> 
>> +
>>  /* New, ipv6-ready socket helper functions, see qemu-sockets.c */
> 
> BTW let's kill this comment.
>
Orit Wasserman Sept. 24, 2012, 10:12 a.m. UTC | #4
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 <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.
> 
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;
>>   }
> 
>
diff mbox

Patch

diff --git a/migration-tcp.c b/migration-tcp.c
index 7f6ad98..a15c2b8 100644
--- a/migration-tcp.c
+++ b/migration-tcp.c
@@ -53,54 +53,35 @@  static int tcp_close(MigrationState *s)
     return r;
 }
 
-static void tcp_wait_for_connect(void *opaque)
+static void tcp_wait_for_connect(int fd, void *opaque)
 {
     MigrationState *s = opaque;
-    int val, ret;
-    socklen_t valsize = sizeof(val);
 
-    DPRINTF("connect completed\n");
-    do {
-        ret = getsockopt(s->fd, SOL_SOCKET, SO_ERROR, (void *) &val, &valsize);
-    } while (ret == -1 && (socket_error()) == EINTR);
-
-    if (ret < 0) {
+    if (fd < 0) {
+        DPRINTF("migrate connect error\n");
+        s->fd = -1;
         migrate_fd_error(s);
-        return;
-    }
-
-    qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL);
-
-    if (val == 0)
+    } else {
+        DPRINTF("migrate connect success\n");
+        s->fd = fd;
         migrate_fd_connect(s);
-    else {
-        DPRINTF("error connecting %d\n", val);
-        migrate_fd_error(s);
     }
 }
 
 int tcp_start_outgoing_migration(MigrationState *s, const char *host_port,
                                  Error **errp)
 {
-    bool in_progress;
-
     s->get_error = socket_errno;
     s->write = socket_write;
     s->close = tcp_close;
 
-    s->fd = inet_nonblocking_connect(host_port, &in_progress, errp);
+    s->fd = inet_nonblocking_connect(host_port, tcp_wait_for_connect, s,
+                                     errp);
     if (error_is_set(errp)) {
         migrate_fd_error(s);
         return -1;
     }
 
-    if (in_progress) {
-        DPRINTF("connect in progress\n");
-        qemu_set_fd_handler2(s->fd, NULL, NULL, tcp_wait_for_connect, s);
-    } else {
-        migrate_fd_connect(s);
-    }
-
     return 0;
 }
 
diff --git a/qemu-char.c b/qemu-char.c
index 13b87b5..b082bae 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2456,7 +2456,7 @@  static CharDriverState *qemu_chr_open_socket(QemuOpts *opts)
         if (is_listen) {
             fd = inet_listen_opts(opts, 0, NULL);
         } else {
-            fd = inet_connect_opts(opts, true, NULL, NULL);
+            fd = inet_connect_opts(opts, NULL, NULL, NULL);
         }
     }
     if (fd < 0) {
diff --git a/qemu-sockets.c b/qemu-sockets.c
index 99a1500..9a4be15 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
@@ -214,8 +215,64 @@  listen:
     ((rc) == -EINPROGRESS)
 #endif
 
-static int inet_connect_addr(struct addrinfo *addr, bool block,
-                             bool *in_progress)
+/* Struct to store connect state for non blocking connect */
+typedef struct ConnectState ConnectState;
+
+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 *in_progress,
+                             ConnectState *connect_state);
+
+static void wait_for_connect(void *opaque)
+{
+    ConnectState *s = opaque;
+    int val = 0, rc = 0;
+    socklen_t valsize = sizeof(val);
+    bool in_progress = false;
+
+    qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL);
+
+    do {
+        rc = getsockopt(s->fd, SOL_SOCKET, SO_ERROR, (void *) &val, &valsize);
+    } while (rc == -1 && socket_error() == EINTR);
+
+    /* update rc to contain error */
+    if (!rc && val) {
+        rc = -1;
+    }
+
+    /* connect error */
+    if (rc < 0) {
+        closesocket(s->fd);
+        s->fd = rc;
+    }
+
+    /* try to connect to the next address on the list */
+    while (s->current_addr->ai_next != NULL && s->fd < 0) {
+        s->current_addr = s->current_addr->ai_next;
+        s->fd = inet_connect_addr(s->current_addr, &in_progress, 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;
+}
+
+static int inet_connect_addr(struct addrinfo *addr, bool *in_progress,
+                             ConnectState *connect_state)
 {
     int sock, rc;
 
@@ -230,7 +287,7 @@  static int inet_connect_addr(struct addrinfo *addr, bool block,
         return -1;
     }
     setsockopt(sock, SOL_SOCKET, SO_REUSEADDR, &on, sizeof(on));
-    if (!block) {
+    if (connect_state != NULL) {
         socket_set_nonblock(sock);
     }
     /* connect to peer */
@@ -241,7 +298,10 @@  static int inet_connect_addr(struct addrinfo *addr, bool block,
         }
     } while (rc == -EINTR);
 
-    if (!block && QEMU_SOCKET_RC_INPROGRESS(rc)) {
+    if (connect_state != NULL && QEMU_SOCKET_RC_INPROGRESS(rc)) {
+        connect_state->fd = sock;
+        qemu_set_fd_handler2(sock, NULL, NULL, wait_for_connect,
+                             connect_state);
         if (in_progress) {
             *in_progress = true;
         }
@@ -260,6 +320,7 @@  static struct addrinfo *inet_parse_connect_opts(QemuOpts *opts, Error **errp)
     const char *port;
 
     memset(&ai, 0, sizeof(ai));
+
     ai.ai_flags = AI_CANONNAME | AI_ADDRCONFIG;
     ai.ai_family = PF_UNSPEC;
     ai.ai_socktype = SOCK_STREAM;
@@ -291,30 +352,42 @@  static struct addrinfo *inet_parse_connect_opts(QemuOpts *opts, Error **errp)
     return res;
 }
 
-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));
+            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;
         }
     }
     if (sock < 0) {
         error_set(errp, QERR_SOCKET_CONNECT_FAILED);
     }
+    g_free(connect_state);
     freeaddrinfo(res);
     return sock;
 }
@@ -519,7 +592,7 @@  int inet_connect(const char *str, Error **errp)
 
     opts = qemu_opts_create(&dummy_opts, NULL, 0, NULL);
     if (inet_parse(opts, str) == 0) {
-        sock = inet_connect_opts(opts, true, NULL, errp);
+        sock = inet_connect_opts(opts, errp, NULL, NULL);
     } else {
         error_set(errp, QERR_SOCKET_CREATE_FAILED);
     }
@@ -527,16 +600,17 @@  int inet_connect(const char *str, Error **errp)
     return sock;
 }
 
-
-int inet_nonblocking_connect(const char *str, bool *in_progress,
-                             Error **errp)
+int inet_nonblocking_connect(const char *str, ConnectHandler *callback,
+                             void *opaque, Error **errp)
 {
     QemuOpts *opts;
     int sock = -1;
 
+    g_assert(callback != NULL);
+
     opts = qemu_opts_create(&dummy_opts, NULL, 0, NULL);
     if (inet_parse(opts, str) == 0) {
-        sock = inet_connect_opts(opts, false, in_progress, errp);
+        sock = inet_connect_opts(opts, errp, callback, opaque);
     } else {
         error_set(errp, QERR_SOCKET_CREATE_FAILED);
     }
diff --git a/qemu_socket.h b/qemu_socket.h
index 3247fb7..da93509 100644
--- a/qemu_socket.h
+++ b/qemu_socket.h
@@ -38,20 +38,31 @@  void socket_set_block(int fd);
 void socket_set_nonblock(int fd);
 int send_all(int fd, const void *buf, int len1);
 
+/* callback function for nonblocking connect
+ * vaild fd on success, negative error code on failure
+ */
+typedef void ConnectHandler(int fd, void *opaque);
+
 /* New, ipv6-ready socket helper functions, see qemu-sockets.c */
 int inet_listen_opts(QemuOpts *opts, int port_offset, Error **errp);
 int inet_listen(const char *str, char *ostr, int olen,
                 int socktype, int port_offset, Error **errp);
 /**
  * Connect function
+ * Use callback for nonblocking connect.
+ *
  * Returns -1 in case of error, fd in case of success
+ * For non-blocking connect calls the callback function
+ * with fd in case of success or -1 in case of error.
  * @opts: QEMU options
- * @block: set true for blocking socket
- * @in_progress: set to true in case of on going connect
+ * @callback: callback function that is called when connect completes,
+ *            not NULL for nonblocking.
+ * @opaque: opaque for callback function
  * @errp: set in case of an error
  **/
-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);
+
 /**
  * Blocking connect
  * Returns -1 in case of error, fd in case of success
@@ -61,14 +72,18 @@  int inet_connect_opts(QemuOpts *opts, bool block, bool *in_progress,
 int inet_connect(const char *str, Error **errp);
 
 /**
- * Nonblocking connect
- * Returns -1 in case of error, fd in case of success
+ * Non-blocking connect
+ * Returns -1 in case of immediate error.
+ * Calls the callback function with fd in case of success or -1 in case of
+ * error.
  * @str: address string
- * @in_progress: set to true in case of on going connect
+ * @callback: callback function that is called when connect completes,
+ *            cannot be NULL.
+ * @opaque: opaque for callback function
  * @errp: set in case of an error
  **/
-int inet_nonblocking_connect(const char *str, bool *in_progress,
-                             Error **errp);
+int inet_nonblocking_connect(const char *str, ConnectHandler *callback,
+                             void *opaque, Error **errp);
 
 int inet_dgram_opts(QemuOpts *opts);
 const char *inet_strfamily(int family);