On Wed, 2011-05-04 at 11:13 +0100, Daniel P. Berrange wrote: > On Wed, May 04, 2011 at 09:39:02AM +0100, n...@bytemark.co.uk wrote: > > Hi, > > > > Currently migration-tcp.c uses the IPv4-only socket functions, making > > migrations over IPv6 impossible. Following is a tentative patch that > > switches > > it to use inet_connect() and inet_listen(). > > > > However, the patch loses the non-blocking connect() behaviour seen with the > > previous code. I'm not sure how much of an issue this is - if connect() > > blocks > > here, does it block execution of the VM? > > > > If so, I guess we need a non-blocking form of inet_connect(), or some way of > > replicating the behaviour - it would potentially be needed for my NBD > > reconnection patches too? I can see that a blocking connect() might not be > > an > > issue while the KVM process is starting up, but could cause problems if we > > try to reconnect while emulation is ongoing. > > > > Thoughts? > > FWIW, Juan Quintela also posted a set of patches to add IPv6 support for > migration a few weeks back, but unfortunately they don't appear to have > been merged yet: > > http://www.mail-archive.com/qemu-devel@nongnu.org/msg58954.html > > IIUC, Juan's patches don't have the blocking connect() problem you > mention.
Hi, Those patches look closer than mine, yes, although I spotted a problem in tcp_start_common() of that patch series (Juan CC'd): > +static int tcp_start_common(const char *str, int *fd, bool server) > +{ > + char hostname[512]; > + const char *service; > + const char *name; > + struct addrinfo hints; > + struct addrinfo *result, *rp; > + int s; > + int sfd; > + int ret = -EINVAL; > + > + *fd = -1; > + service = str; > + if (get_str_sep(hostname, sizeof(hostname), &service, ':') < 0) { > + return -EINVAL; > + } > + if (server && strlen(hostname) == 0) { > + name = NULL; > + } else { > + name = hostname; > + } I think this will fail when specifying an IPv6 *address* and port, e.g: migrate -d [::1]:5000 We already have code that does this correctly in qemu-sockets.c - inet_parse() - which is primarily what I was trying to get the benefit of, in my patch. > + > + /* Obtain address(es) matching host/port */ > + > + memset(&hints, 0, sizeof(struct addrinfo)); > + hints.ai_family = AF_UNSPEC; /* Allow IPv4 or IPv6 */ > + hints.ai_socktype = SOCK_STREAM; /* Datagram socket */ > + > + if (server) { > + hints.ai_flags = AI_PASSIVE; > + } > + > + s = getaddrinfo(name, service, &hints, &result); > + if (s != 0) { > + fprintf(stderr, "qemu: getaddrinfo: %s\n", gai_strerror(s)); > + return -EINVAL; > + } > + > + /* getaddrinfo() returns a list of address structures. > + Try each address until we successfully bind/connect). > + If socket(2) (or bind/connect(2)) fails, we (close the socket > + and) try the next address. */ > + > + for (rp = result; rp != NULL; rp = rp->ai_next) { > + sfd = qemu_socket(rp->ai_family, rp->ai_socktype, rp->ai_protocol); > + if (sfd == -1) { > + ret = -errno; > + continue; > + } > + socket_set_nonblock(sfd); > + if (server) { > + ret = tcp_server_bind(sfd, rp); > + } else { > + ret = tcp_client_connect(sfd, rp); > + } > + if (ret >= 0 || ret == -EINPROGRESS || ret == -EWOULDBLOCK) { > + *fd = sfd; > + break; /* Success */ > + } > + close(sfd); > + } > + > + freeaddrinfo(result); > + return ret; > +}