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;
> +}
