| Message ID | 1347448378-23915-4-git-send-email-owasserm@redhat.com |
|---|---|
| State | New |
| Headers | show |
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 | 29 +++------- > qemu-sockets.c | 169 +++++++++++++++++++++++++++++++++++++++++-------------- > qemu_socket.h | 9 ++- > 3 files changed, 142 insertions(+), 65 deletions(-) > > diff --git a/migration-tcp.c b/migration-tcp.c > index 7f6ad98..cadea36 100644 > --- a/migration-tcp.c > +++ b/migration-tcp.c > @@ -53,29 +53,18 @@ 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); > } > } > > @@ -88,7 +77,8 @@ int tcp_start_outgoing_migration(MigrationState *s, const char *host_port, > 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, > + &in_progress, errp); > if (error_is_set(errp)) { > migrate_fd_error(s); > return -1; > @@ -96,7 +86,6 @@ int tcp_start_outgoing_migration(MigrationState *s, const char *host_port, > > 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); > } > diff --git a/qemu-sockets.c b/qemu-sockets.c > index 4f5eca8..f5d64c8 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 > @@ -209,41 +210,27 @@ listen: > return slisten; > } > > -static struct addrinfo *inet_parse_connect_opts(QemuOpts *opts, Error **errp) > -{ > - struct addrinfo ai, *res; > - int rc; > - const char *addr; > - 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; > - > - addr = qemu_opt_get(opts, "host"); > - port = qemu_opt_get(opts, "port"); > - if (addr == NULL || port == NULL) { > - fprintf(stderr, > - "inet_parse_connect_opts: host and/or port not specified\n"); > - error_set(errp, QERR_SOCKET_CREATE_FAILED); > - return NULL; > - } > - > - if (qemu_opt_get_bool(opts, "ipv4", 0)) > - ai.ai_family = PF_INET; > - if (qemu_opt_get_bool(opts, "ipv6", 0)) > - ai.ai_family = PF_INET6; > +#ifdef _WIN32 > +#define QEMU_SOCKET_RC_INPROGRESS(rc) \ > + ((rc) == -EINPROGRESS || rc == -EWOULDBLOCK || rc == -WSAEALREADY) > +#else > +#define QEMU_SOCKET_RC_INPROGRESS(rc) \ > + ((rc) == -EINPROGRESS) > +#endif > > - /* lookup */ > - if (0 != (rc = getaddrinfo(addr, port, &ai, &res))) { > - fprintf(stderr,"getaddrinfo(%s,%s): %s\n", addr, port, > - gai_strerror(rc)); > - error_set(errp, QERR_SOCKET_CREATE_FAILED); > - return NULL; > - } > - return res; > -} inet_parse_connect_opts() is only moved, not changed, I think. I like to keep code motion well away from code changes (separate patch) to facilitate review. > +/* 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; > +} ConnectState; > + > +static ConnectState connect_state = { > + .fd = -1, > +}; Why does a single connect_state suffice? > > #ifdef _WIN32 > #define QEMU_SOCKET_RC_INPROGRESS(rc) \ > @@ -254,12 +241,17 @@ static struct addrinfo *inet_parse_connect_opts(QemuOpts *opts, Error **errp) > #endif > > static int inet_connect_addr(struct addrinfo *addr, bool block, > - bool *in_progress, Error **errp) > + IOHandler *handler, bool *in_progress, > + Error **errp) Sure you want a handler parameter? It's always wait_for_connect()... > { > char uaddr[INET6_ADDRSTRLEN + 1]; > char uport[33]; > int sock, rc; > > + if (in_progress) { > + *in_progress = false; > + } > + > if (getnameinfo((struct sockaddr *)addr->ai_addr, addr->ai_addrlen, > uaddr, INET6_ADDRSTRLEN, uport, 32, > NI_NUMERICHOST | NI_NUMERICSERV)) { > @@ -285,6 +277,8 @@ static int inet_connect_addr(struct addrinfo *addr, bool block, > } while (rc == -EINTR); > > if (!block && QEMU_SOCKET_RC_INPROGRESS(rc)) { > + connect_state.fd = sock; > + qemu_set_fd_handler2(sock, NULL, NULL, handler, &connect_state); > if (in_progress) { > *in_progress = true; > } > @@ -295,6 +289,94 @@ static int inet_connect_addr(struct addrinfo *addr, bool block, > return sock; > } > > +static void wait_for_connect(void *opaque) > +{ > + ConnectState *s = opaque; > + int val = 0, rc = 0; > + socklen_t valsize = sizeof(val); > + bool in_progress = false; > + > + do { > + rc = getsockopt(s->fd, SOL_SOCKET, SO_ERROR, (void *) &val, &valsize); > + } while (rc == -1 && (socket_error()) == EINTR); Either parenthesize both operands of && or none. I prefer none. > + > + /* connect succeded */ > + if (!rc && !val) { > + qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL); > + freeaddrinfo(s->addr_list); > + if (s->callback) { > + s->callback(s->fd, s->opaque); > + } > + return; > + } > + > + if (!rc && val) { > + rc = -val; > + } > + > + qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL); > + closesocket(s->fd); > + if (s->current_addr != NULL && s->current_addr->ai_next != NULL) { How can s->current_addr == NULL happen? > + s->current_addr = s->current_addr->ai_next; > + s->fd = inet_connect_addr(s->current_addr, false, wait_for_connect, > + &in_progress, &s->errp); inet_connect_addr() either 1. completes connect (returns valid fd, sets in_progress to false), or 2. starts connect (returns valid fd, sets in_progress to true), or 3. fails (returns -1 and sets in_progress to false). > + if (in_progress) { > + return; Case 2. > + } > + } To reach this point, we either ran out of addresses (if not entered), or connect to the current address completed (case 1), or connect failed (case 3). > + > + qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL); > + > + closesocket(s->fd); > + s->fd = rc; > + freeaddrinfo(s->addr_list); > + /* connect failed */ > + if (s->callback) { > + s->callback(s->fd, s->opaque); > + } > + return; Either I'm confused, or this treats completed connect() as failure, which is wrong. > +} > + > +static struct addrinfo *inet_parse_connect_opts(QemuOpts *opts, Error **errp) > +{ > + struct addrinfo ai, *res; > + int rc; > + const char *addr; > + 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; > + > + addr = qemu_opt_get(opts, "host"); > + port = qemu_opt_get(opts, "port"); > + if (addr == NULL || port == NULL) { > + fprintf(stderr, > + "inet_parse_connect_opts: host and/or port not specified\n"); > + error_set(errp, QERR_SOCKET_CREATE_FAILED); > + return NULL; > + } > + > + if (qemu_opt_get_bool(opts, "ipv4", 0)) { > + ai.ai_family = PF_INET; > + } > + if (qemu_opt_get_bool(opts, "ipv6", 0)) { > + ai.ai_family = PF_INET6; > + } > + > + /* lookup */ > + rc = getaddrinfo(addr, port, &ai, &res); > + if (rc != 0) { > + fprintf(stderr, "getaddrinfo(%s,%s): %s\n", addr, port, > + gai_strerror(rc)); > + error_set(errp, QERR_SOCKET_CREATE_FAILED); > + return NULL; > + } > + return res; > +} > + > + > int inet_connect_opts(QemuOpts *opts, bool *in_progress, Error **errp) > { > struct addrinfo *res, *e; > @@ -306,12 +388,12 @@ int inet_connect_opts(QemuOpts *opts, bool *in_progress, Error **errp) > 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, errp); > + if (!block) { > + connect_state.addr_list = res; > + connect_state.current_addr = e; > + } > + sock = inet_connect_addr(e, block, wait_for_connect, in_progress, errp); > if (in_progress && *in_progress) { > return sock; > } else if (sock >= 0) { > @@ -534,9 +616,8 @@ 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, bool *in_progress, Error **errp) > { > QemuOpts *opts; > int sock = -1; > @@ -544,6 +625,8 @@ int inet_nonblocking_connect(const char *str, bool *in_progress, > opts = qemu_opts_create(&dummy_opts, NULL, 0, NULL); > if (inet_parse(opts, str) == 0) { > qemu_opt_set(opts, "block", "off"); > + connect_state.callback = callback; > + connect_state.opaque = opaque; > sock = inet_connect_opts(opts, in_progress, errp); > } else { > error_set(errp, QERR_SOCKET_CREATE_FAILED); > diff --git a/qemu_socket.h b/qemu_socket.h > index c47f2b0..c8431eb 100644 > --- a/qemu_socket.h > +++ b/qemu_socket.h > @@ -38,14 +38,19 @@ 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); > int inet_connect_opts(QemuOpts *opts, bool *in_progress, Error **errp); > int inet_connect(const char *str, Error **errp); > -int inet_nonblocking_connect(const char *str, bool *in_progress, > - Error **errp); > +int inet_nonblocking_connect(const char *str, ConnectHandler *callback, > + void *opaque, bool *in_progress, Error **errp); > int inet_dgram_opts(QemuOpts *opts); > const char *inet_strfamily(int family); Good stuff!
On Thu, Sep 13, 2012 at 03:22:24PM +0200, Markus Armbruster wrote: > > +/* 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; > > +} ConnectState; > > + > > +static ConnectState connect_state = { > > + .fd = -1, > > +}; > > Why does a single connect_state suffice? Even if it does for the specific use, better not to make assumptions about API use. Let's pass connect state to APIs.
On 13 בספט 2012, at 16:30, "Michael S. Tsirkin" <mst@redhat.com> wrote: > On Thu, Sep 13, 2012 at 03:22:24PM +0200, Markus Armbruster wrote: >>> +/* 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; >>> +} ConnectState; >>> + >>> +static ConnectState connect_state = { >>> + .fd = -1, >>> +}; >> >> Why does a single connect_state suffice? > > Even if it does for the specific use, better not to > make assumptions about API use. Let's pass connect state > to APIs. I agree, Orit
On 09/13/2012 04:22 PM, Markus Armbruster wrote: > 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 | 29 +++------- >> qemu-sockets.c | 169 +++++++++++++++++++++++++++++++++++++++++-------------- >> qemu_socket.h | 9 ++- >> 3 files changed, 142 insertions(+), 65 deletions(-) >> >> diff --git a/migration-tcp.c b/migration-tcp.c >> index 7f6ad98..cadea36 100644 >> --- a/migration-tcp.c >> +++ b/migration-tcp.c >> @@ -53,29 +53,18 @@ 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); >> } >> } >> >> @@ -88,7 +77,8 @@ int tcp_start_outgoing_migration(MigrationState *s, const char *host_port, >> 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, >> + &in_progress, errp); >> if (error_is_set(errp)) { >> migrate_fd_error(s); >> return -1; >> @@ -96,7 +86,6 @@ int tcp_start_outgoing_migration(MigrationState *s, const char *host_port, >> >> 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); >> } >> diff --git a/qemu-sockets.c b/qemu-sockets.c >> index 4f5eca8..f5d64c8 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 >> @@ -209,41 +210,27 @@ listen: >> return slisten; >> } >> >> -static struct addrinfo *inet_parse_connect_opts(QemuOpts *opts, Error **errp) >> -{ >> - struct addrinfo ai, *res; >> - int rc; >> - const char *addr; >> - 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; >> - >> - addr = qemu_opt_get(opts, "host"); >> - port = qemu_opt_get(opts, "port"); >> - if (addr == NULL || port == NULL) { >> - fprintf(stderr, >> - "inet_parse_connect_opts: host and/or port not specified\n"); >> - error_set(errp, QERR_SOCKET_CREATE_FAILED); >> - return NULL; >> - } >> - >> - if (qemu_opt_get_bool(opts, "ipv4", 0)) >> - ai.ai_family = PF_INET; >> - if (qemu_opt_get_bool(opts, "ipv6", 0)) >> - ai.ai_family = PF_INET6; >> +#ifdef _WIN32 >> +#define QEMU_SOCKET_RC_INPROGRESS(rc) \ >> + ((rc) == -EINPROGRESS || rc == -EWOULDBLOCK || rc == -WSAEALREADY) >> +#else >> +#define QEMU_SOCKET_RC_INPROGRESS(rc) \ >> + ((rc) == -EINPROGRESS) >> +#endif >> >> - /* lookup */ >> - if (0 != (rc = getaddrinfo(addr, port, &ai, &res))) { >> - fprintf(stderr,"getaddrinfo(%s,%s): %s\n", addr, port, >> - gai_strerror(rc)); >> - error_set(errp, QERR_SOCKET_CREATE_FAILED); >> - return NULL; >> - } >> - return res; >> -} > > inet_parse_connect_opts() is only moved, not changed, I think. I like > to keep code motion well away from code changes (separate patch) to > facilitate review. > >> +/* 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; >> +} ConnectState; >> + >> +static ConnectState connect_state = { >> + .fd = -1, >> +}; > > Why does a single connect_state suffice? > >> >> #ifdef _WIN32 >> #define QEMU_SOCKET_RC_INPROGRESS(rc) \ >> @@ -254,12 +241,17 @@ static struct addrinfo *inet_parse_connect_opts(QemuOpts *opts, Error **errp) >> #endif >> >> static int inet_connect_addr(struct addrinfo *addr, bool block, >> - bool *in_progress, Error **errp) >> + IOHandler *handler, bool *in_progress, >> + Error **errp) > > Sure you want a handler parameter? It's always wait_for_connect()... > >> { >> char uaddr[INET6_ADDRSTRLEN + 1]; >> char uport[33]; >> int sock, rc; >> >> + if (in_progress) { >> + *in_progress = false; >> + } >> + >> if (getnameinfo((struct sockaddr *)addr->ai_addr, addr->ai_addrlen, >> uaddr, INET6_ADDRSTRLEN, uport, 32, >> NI_NUMERICHOST | NI_NUMERICSERV)) { >> @@ -285,6 +277,8 @@ static int inet_connect_addr(struct addrinfo *addr, bool block, >> } while (rc == -EINTR); >> >> if (!block && QEMU_SOCKET_RC_INPROGRESS(rc)) { >> + connect_state.fd = sock; >> + qemu_set_fd_handler2(sock, NULL, NULL, handler, &connect_state); >> if (in_progress) { >> *in_progress = true; >> } >> @@ -295,6 +289,94 @@ static int inet_connect_addr(struct addrinfo *addr, bool block, >> return sock; >> } >> >> +static void wait_for_connect(void *opaque) >> +{ >> + ConnectState *s = opaque; >> + int val = 0, rc = 0; >> + socklen_t valsize = sizeof(val); >> + bool in_progress = false; >> + >> + do { >> + rc = getsockopt(s->fd, SOL_SOCKET, SO_ERROR, (void *) &val, &valsize); >> + } while (rc == -1 && (socket_error()) == EINTR); > > Either parenthesize both operands of && or none. I prefer none. > >> + >> + /* connect succeded */ >> + if (!rc && !val) { >> + qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL); >> + freeaddrinfo(s->addr_list); >> + if (s->callback) { >> + s->callback(s->fd, s->opaque); >> + } >> + return; >> + } >> + >> + if (!rc && val) { >> + rc = -val; >> + } >> + >> + qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL); >> + closesocket(s->fd); >> + if (s->current_addr != NULL && s->current_addr->ai_next != NULL) { > > How can s->current_addr == NULL happen? can't see a scenario , but it never hurt to check. > >> + s->current_addr = s->current_addr->ai_next; >> + s->fd = inet_connect_addr(s->current_addr, false, wait_for_connect, >> + &in_progress, &s->errp); > > inet_connect_addr() either > > 1. completes connect (returns valid fd, sets in_progress to false), or > > 2. starts connect (returns valid fd, sets in_progress to true), or > > 3. fails (returns -1 and sets in_progress to false). > >> + if (in_progress) { >> + return; > > Case 2. > >> + } >> + } > > To reach this point, we either ran out of addresses (if not entered), or > connect to the current address completed (case 1), or connect failed > (case 3). > >> + >> + qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL); >> + >> + closesocket(s->fd); >> + s->fd = rc; >> + freeaddrinfo(s->addr_list); >> + /* connect failed */ >> + if (s->callback) { >> + s->callback(s->fd, s->opaque); >> + } >> + return; > > Either I'm confused, or this treats completed connect() as failure, > which is wrong. > I think you caught a bug , i will fix it ... Orit >> +} >> + >> +static struct addrinfo *inet_parse_connect_opts(QemuOpts *opts, Error **errp) >> +{ >> + struct addrinfo ai, *res; >> + int rc; >> + const char *addr; >> + 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; >> + >> + addr = qemu_opt_get(opts, "host"); >> + port = qemu_opt_get(opts, "port"); >> + if (addr == NULL || port == NULL) { >> + fprintf(stderr, >> + "inet_parse_connect_opts: host and/or port not specified\n"); >> + error_set(errp, QERR_SOCKET_CREATE_FAILED); >> + return NULL; >> + } >> + >> + if (qemu_opt_get_bool(opts, "ipv4", 0)) { >> + ai.ai_family = PF_INET; >> + } >> + if (qemu_opt_get_bool(opts, "ipv6", 0)) { >> + ai.ai_family = PF_INET6; >> + } >> + >> + /* lookup */ >> + rc = getaddrinfo(addr, port, &ai, &res); >> + if (rc != 0) { >> + fprintf(stderr, "getaddrinfo(%s,%s): %s\n", addr, port, >> + gai_strerror(rc)); >> + error_set(errp, QERR_SOCKET_CREATE_FAILED); >> + return NULL; >> + } >> + return res; >> +} >> + >> + >> int inet_connect_opts(QemuOpts *opts, bool *in_progress, Error **errp) >> { >> struct addrinfo *res, *e; >> @@ -306,12 +388,12 @@ int inet_connect_opts(QemuOpts *opts, bool *in_progress, Error **errp) >> 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, errp); >> + if (!block) { >> + connect_state.addr_list = res; >> + connect_state.current_addr = e; >> + } >> + sock = inet_connect_addr(e, block, wait_for_connect, in_progress, errp); >> if (in_progress && *in_progress) { >> return sock; >> } else if (sock >= 0) { >> @@ -534,9 +616,8 @@ 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, bool *in_progress, Error **errp) >> { >> QemuOpts *opts; >> int sock = -1; >> @@ -544,6 +625,8 @@ int inet_nonblocking_connect(const char *str, bool *in_progress, >> opts = qemu_opts_create(&dummy_opts, NULL, 0, NULL); >> if (inet_parse(opts, str) == 0) { >> qemu_opt_set(opts, "block", "off"); >> + connect_state.callback = callback; >> + connect_state.opaque = opaque; >> sock = inet_connect_opts(opts, in_progress, errp); >> } else { >> error_set(errp, QERR_SOCKET_CREATE_FAILED); >> diff --git a/qemu_socket.h b/qemu_socket.h >> index c47f2b0..c8431eb 100644 >> --- a/qemu_socket.h >> +++ b/qemu_socket.h >> @@ -38,14 +38,19 @@ 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); >> int inet_connect_opts(QemuOpts *opts, bool *in_progress, Error **errp); >> int inet_connect(const char *str, Error **errp); >> -int inet_nonblocking_connect(const char *str, bool *in_progress, >> - Error **errp); >> +int inet_nonblocking_connect(const char *str, ConnectHandler *callback, >> + void *opaque, bool *in_progress, Error **errp); >> int inet_dgram_opts(QemuOpts *opts); >> const char *inet_strfamily(int family); > > Good stuff! >
diff --git a/migration-tcp.c b/migration-tcp.c index 7f6ad98..cadea36 100644 --- a/migration-tcp.c +++ b/migration-tcp.c @@ -53,29 +53,18 @@ 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); } } @@ -88,7 +77,8 @@ int tcp_start_outgoing_migration(MigrationState *s, const char *host_port, 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, + &in_progress, errp); if (error_is_set(errp)) { migrate_fd_error(s); return -1; @@ -96,7 +86,6 @@ int tcp_start_outgoing_migration(MigrationState *s, const char *host_port, 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); } diff --git a/qemu-sockets.c b/qemu-sockets.c index 4f5eca8..f5d64c8 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 @@ -209,41 +210,27 @@ listen: return slisten; } -static struct addrinfo *inet_parse_connect_opts(QemuOpts *opts, Error **errp) -{ - struct addrinfo ai, *res; - int rc; - const char *addr; - 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; - - addr = qemu_opt_get(opts, "host"); - port = qemu_opt_get(opts, "port"); - if (addr == NULL || port == NULL) { - fprintf(stderr, - "inet_parse_connect_opts: host and/or port not specified\n"); - error_set(errp, QERR_SOCKET_CREATE_FAILED); - return NULL; - } - - if (qemu_opt_get_bool(opts, "ipv4", 0)) - ai.ai_family = PF_INET; - if (qemu_opt_get_bool(opts, "ipv6", 0)) - ai.ai_family = PF_INET6; +#ifdef _WIN32 +#define QEMU_SOCKET_RC_INPROGRESS(rc) \ + ((rc) == -EINPROGRESS || rc == -EWOULDBLOCK || rc == -WSAEALREADY) +#else +#define QEMU_SOCKET_RC_INPROGRESS(rc) \ + ((rc) == -EINPROGRESS) +#endif - /* lookup */ - if (0 != (rc = getaddrinfo(addr, port, &ai, &res))) { - fprintf(stderr,"getaddrinfo(%s,%s): %s\n", addr, port, - gai_strerror(rc)); - error_set(errp, QERR_SOCKET_CREATE_FAILED); - return NULL; - } - return res; -} +/* 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; +} ConnectState; + +static ConnectState connect_state = { + .fd = -1, +}; #ifdef _WIN32 #define QEMU_SOCKET_RC_INPROGRESS(rc) \ @@ -254,12 +241,17 @@ static struct addrinfo *inet_parse_connect_opts(QemuOpts *opts, Error **errp) #endif static int inet_connect_addr(struct addrinfo *addr, bool block, - bool *in_progress, Error **errp) + IOHandler *handler, bool *in_progress, + Error **errp) { char uaddr[INET6_ADDRSTRLEN + 1]; char uport[33]; int sock, rc; + if (in_progress) { + *in_progress = false; + } + if (getnameinfo((struct sockaddr *)addr->ai_addr, addr->ai_addrlen, uaddr, INET6_ADDRSTRLEN, uport, 32, NI_NUMERICHOST | NI_NUMERICSERV)) { @@ -285,6 +277,8 @@ static int inet_connect_addr(struct addrinfo *addr, bool block, } while (rc == -EINTR); if (!block && QEMU_SOCKET_RC_INPROGRESS(rc)) { + connect_state.fd = sock; + qemu_set_fd_handler2(sock, NULL, NULL, handler, &connect_state); if (in_progress) { *in_progress = true; } @@ -295,6 +289,94 @@ static int inet_connect_addr(struct addrinfo *addr, bool block, return sock; } +static void wait_for_connect(void *opaque) +{ + ConnectState *s = opaque; + int val = 0, rc = 0; + socklen_t valsize = sizeof(val); + bool in_progress = false; + + do { + rc = getsockopt(s->fd, SOL_SOCKET, SO_ERROR, (void *) &val, &valsize); + } while (rc == -1 && (socket_error()) == EINTR); + + /* connect succeded */ + if (!rc && !val) { + qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL); + freeaddrinfo(s->addr_list); + if (s->callback) { + s->callback(s->fd, s->opaque); + } + return; + } + + if (!rc && val) { + rc = -val; + } + + qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL); + closesocket(s->fd); + if (s->current_addr != NULL && s->current_addr->ai_next != NULL) { + s->current_addr = s->current_addr->ai_next; + s->fd = inet_connect_addr(s->current_addr, false, wait_for_connect, + &in_progress, &s->errp); + if (in_progress) { + return; + } + } + + qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL); + + closesocket(s->fd); + s->fd = rc; + freeaddrinfo(s->addr_list); + /* connect failed */ + if (s->callback) { + s->callback(s->fd, s->opaque); + } + return; +} + +static struct addrinfo *inet_parse_connect_opts(QemuOpts *opts, Error **errp) +{ + struct addrinfo ai, *res; + int rc; + const char *addr; + 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; + + addr = qemu_opt_get(opts, "host"); + port = qemu_opt_get(opts, "port"); + if (addr == NULL || port == NULL) { + fprintf(stderr, + "inet_parse_connect_opts: host and/or port not specified\n"); + error_set(errp, QERR_SOCKET_CREATE_FAILED); + return NULL; + } + + if (qemu_opt_get_bool(opts, "ipv4", 0)) { + ai.ai_family = PF_INET; + } + if (qemu_opt_get_bool(opts, "ipv6", 0)) { + ai.ai_family = PF_INET6; + } + + /* lookup */ + rc = getaddrinfo(addr, port, &ai, &res); + if (rc != 0) { + fprintf(stderr, "getaddrinfo(%s,%s): %s\n", addr, port, + gai_strerror(rc)); + error_set(errp, QERR_SOCKET_CREATE_FAILED); + return NULL; + } + return res; +} + + int inet_connect_opts(QemuOpts *opts, bool *in_progress, Error **errp) { struct addrinfo *res, *e; @@ -306,12 +388,12 @@ int inet_connect_opts(QemuOpts *opts, bool *in_progress, Error **errp) 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, errp); + if (!block) { + connect_state.addr_list = res; + connect_state.current_addr = e; + } + sock = inet_connect_addr(e, block, wait_for_connect, in_progress, errp); if (in_progress && *in_progress) { return sock; } else if (sock >= 0) { @@ -534,9 +616,8 @@ 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, bool *in_progress, Error **errp) { QemuOpts *opts; int sock = -1; @@ -544,6 +625,8 @@ int inet_nonblocking_connect(const char *str, bool *in_progress, opts = qemu_opts_create(&dummy_opts, NULL, 0, NULL); if (inet_parse(opts, str) == 0) { qemu_opt_set(opts, "block", "off"); + connect_state.callback = callback; + connect_state.opaque = opaque; sock = inet_connect_opts(opts, in_progress, errp); } else { error_set(errp, QERR_SOCKET_CREATE_FAILED); diff --git a/qemu_socket.h b/qemu_socket.h index c47f2b0..c8431eb 100644 --- a/qemu_socket.h +++ b/qemu_socket.h @@ -38,14 +38,19 @@ 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); int inet_connect_opts(QemuOpts *opts, bool *in_progress, Error **errp); int inet_connect(const char *str, Error **errp); -int inet_nonblocking_connect(const char *str, bool *in_progress, - Error **errp); +int inet_nonblocking_connect(const char *str, ConnectHandler *callback, + void *opaque, bool *in_progress, Error **errp); int inet_dgram_opts(QemuOpts *opts); const char *inet_strfamily(int family);