Patchwork [1/2] Use getaddrinfo for migration

login
register
mail settings
Submitter Juan Quintela
Date March 16, 2011, 9:01 p.m.
Message ID <414060358dedfe8455354ef2fc1a90e6254abe16.1300297944.git.quintela@redhat.com>
Download mbox | patch
Permalink /patch/87306/
State New
Headers show

Comments

Juan Quintela - March 16, 2011, 9:01 p.m.
This allows us to use ipv4/ipv6 for migration addresses.
Once there, it also uses /etc/services names (it came free).

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration-tcp.c |   53 ++++++-------------------
 net.c           |  113 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 qemu_socket.h   |    3 +
 3 files changed, 129 insertions(+), 40 deletions(-)
Peter Maydell - March 16, 2011, 10:06 p.m.
On 16 March 2011 21:01, Juan Quintela <quintela@redhat.com> wrote:
> +static int tcp_server_bind(int fd, struct addrinfo *rp)
> +{
> +    int val = 1;
> +    int ret;
> +
> +    /* allow fast reuse */
> +    setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, (const char *)&val,
> +               sizeof(val));
> +
> +    ret = bind(fd, rp->ai_addr, rp->ai_addrlen);
> +
> +    if (ret == -1) {
> +        ret = -socket_error();
> +    }
> +    return ret;
> +
> +}

I know this is just moved code, but does this do the right thing
on Windows? Windows semantics for SO_REUSEADDR are completely
different from the Unix ones:
http://blogs.msdn.com/b/wndp/archive/2005/08/03/anthony-jones.aspx
http://msdn.microsoft.com/en-us/library/ms740618%28VS.85%29.aspx

Roughly, Unix default behaviour is like Windows SO_EXCLUSIVEADDRUSE;
Unix SO_REUSEADDR is like Windows default behaviour; and Windows
SO_REUSEADDR behaviour is really weird and you don't want it.

(I think Cygwin has a workaround for this to give unix semantics,
but Windows qemu builds as a mingw app, so we get the MS semantics,
right?)

> +static int tcp_start_common(const char *str, int *fd, bool server)
> +{
[...]
> +    s = getaddrinfo(name, service, &hints, &result);
> +    if (s != 0) {
> +        fprintf(stderr, "qemu: getaddrinfo: %s\n", gai_strerror(s));
> +        return -EINVAL;
> +    }

It seems a bit odd to have a low level utility routine printing
diagnostics to stderr rather than just returning the error
details to its caller somehow. Ditto the other fprintf later.

-- PMM
Juan Quintela - March 16, 2011, 10:20 p.m.
Peter Maydell <peter.maydell@linaro.org> wrote:
> On 16 March 2011 21:01, Juan Quintela <quintela@redhat.com> wrote:
>> +static int tcp_server_bind(int fd, struct addrinfo *rp)
>> +{
>> +    int val = 1;
>> +    int ret;
>> +
>> +    /* allow fast reuse */
>> +    setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, (const char *)&val,
>> +               sizeof(val));
>> +
>> +    ret = bind(fd, rp->ai_addr, rp->ai_addrlen);
>> +
>> +    if (ret == -1) {
>> +        ret = -socket_error();
>> +    }
>> +    return ret;
>> +
>> +}
>
> I know this is just moved code, but does this do the right thing
> on Windows? Windows semantics for SO_REUSEADDR are completely
> different from the Unix ones:
> http://blogs.msdn.com/b/wndp/archive/2005/08/03/anthony-jones.aspx
> http://msdn.microsoft.com/en-us/library/ms740618%28VS.85%29.aspx
>
> Roughly, Unix default behaviour is like Windows SO_EXCLUSIVEADDRUSE;
> Unix SO_REUSEADDR is like Windows default behaviour; and Windows
> SO_REUSEADDR behaviour is really weird and you don't want it.
>
> (I think Cygwin has a workaround for this to give unix semantics,
> but Windows qemu builds as a mingw app, so we get the MS semantics,
> right?)

No clue really, but again, this is the code that existed there, we can
fix on top of it.


>> +static int tcp_start_common(const char *str, int *fd, bool server)
>> +{
> [...]
>> +    s = getaddrinfo(name, service, &hints, &result);
>> +    if (s != 0) {
>> +        fprintf(stderr, "qemu: getaddrinfo: %s\n", gai_strerror(s));
>> +        return -EINVAL;
>> +    }
>
> It seems a bit odd to have a low level utility routine printing
> diagnostics to stderr rather than just returning the error
> details to its caller somehow. Ditto the other fprintf later.

Previous code did it.  I am not clear what to do here.  The way to get
an error for getaddrinfo is to return one string with gai_strerror().

Suggestions appreciated.

Later, Jaun.

Patch

diff --git a/migration-tcp.c b/migration-tcp.c
index b55f419..2fa496a 100644
--- a/migration-tcp.c
+++ b/migration-tcp.c
@@ -48,8 +48,6 @@  static int tcp_close(FdMigrationState *s)
     }
     return 0;
 }
-
-
 static void tcp_wait_for_connect(void *opaque)
 {
     FdMigrationState *s = opaque;
@@ -83,13 +81,10 @@  MigrationState *tcp_start_outgoing_migration(Monitor *mon,
 					     int blk,
 					     int inc)
 {
-    struct sockaddr_in addr;
     FdMigrationState *s;
+    int fd;
     int ret;

-    if (parse_host_port(&addr, host_port) < 0)
-        return NULL;
-
     s = qemu_mallocz(sizeof(*s));

     s->get_error = socket_errno;
@@ -105,33 +100,22 @@  MigrationState *tcp_start_outgoing_migration(Monitor *mon,
     s->state = MIG_STATE_ACTIVE;
     s->mon = NULL;
     s->bandwidth_limit = bandwidth_limit;
-    s->fd = qemu_socket(PF_INET, SOCK_STREAM, 0);
-    if (s->fd == -1) {
-        qemu_free(s);
-        return NULL;
-    }
-
-    socket_set_nonblock(s->fd);

     if (!detach) {
         migrate_fd_monitor_suspend(s, mon);
     }

-    do {
-        ret = connect(s->fd, (struct sockaddr *)&addr, sizeof(addr));
-        if (ret == -1)
-            ret = -(s->get_error(s));
-
-        if (ret == -EINPROGRESS || ret == -EWOULDBLOCK)
-            qemu_set_fd_handler2(s->fd, NULL, NULL, tcp_wait_for_connect, s);
-    } while (ret == -EINTR);
-
-    if (ret < 0 && ret != -EINPROGRESS && ret != -EWOULDBLOCK) {
+    ret = tcp_client_start(host_port, &fd);
+    s->fd = fd;
+    if (ret == -EINPROGRESS || ret == -EWOULDBLOCK) {
+        DPRINTF("connect in progress");
+        qemu_set_fd_handler2(s->fd, NULL, NULL, tcp_wait_for_connect, s);
+    } else if (ret < 0) {
         DPRINTF("connect failed\n");
         migrate_fd_error(s);
-    } else if (ret >= 0)
+    } else {
         migrate_fd_connect(s);
-
+    }
     return &s->mig_state;
 }

@@ -171,25 +155,14 @@  out2:

 int tcp_start_incoming_migration(const char *host_port)
 {
-    struct sockaddr_in addr;
-    int val;
+    int ret;
     int s;

-    if (parse_host_port(&addr, host_port) < 0) {
-        fprintf(stderr, "invalid host/port combination: %s\n", host_port);
-        return -EINVAL;
+    ret = tcp_server_start(host_port, &s);
+    if (ret < 0) {
+        return ret;
     }

-    s = qemu_socket(PF_INET, SOCK_STREAM, 0);
-    if (s == -1)
-        return -socket_error();
-
-    val = 1;
-    setsockopt(s, SOL_SOCKET, SO_REUSEADDR, (const char *)&val, sizeof(val));
-
-    if (bind(s, (struct sockaddr *)&addr, sizeof(addr)) == -1)
-        goto err;
-
     if (listen(s, 1) == -1)
         goto err;

diff --git a/net.c b/net.c
index ddcca97..8347e17 100644
--- a/net.c
+++ b/net.c
@@ -94,6 +94,119 @@  static int get_str_sep(char *buf, int buf_size, const char **pp, int sep)
     return 0;
 }

+static int tcp_server_bind(int fd, struct addrinfo *rp)
+{
+    int val = 1;
+    int ret;
+
+    /* allow fast reuse */
+    setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, (const char *)&val,
+               sizeof(val));
+
+    ret = bind(fd, rp->ai_addr, rp->ai_addrlen);
+
+    if (ret == -1) {
+        ret = -socket_error();
+    }
+    return ret;
+
+}
+
+static int tcp_client_connect(int fd, struct addrinfo *rp)
+{
+    int ret;
+
+    do {
+        ret = connect(fd, rp->ai_addr, rp->ai_addrlen);
+        if (ret == -1) {
+            ret = -socket_error();
+        }
+    } while (ret == -EINTR);
+
+    return ret;
+}
+
+
+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;
+    }
+
+    /* 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);
+    }
+
+    if (rp == NULL) {               /* No address succeeded */
+        fprintf(stderr, "Could not connect\n");
+    }
+
+    freeaddrinfo(result);
+    return ret;
+}
+
+int tcp_server_start(const char *str, int *fd)
+{
+    return tcp_start_common(str, fd, true);
+}
+
+int tcp_client_start(const char *str, int *fd)
+{
+    return tcp_start_common(str, fd, false);
+}
+
 int parse_host_port(struct sockaddr_in *saddr, const char *str)
 {
     char buf[512];
diff --git a/qemu_socket.h b/qemu_socket.h
index 180e4db..c399d96 100644
--- a/qemu_socket.h
+++ b/qemu_socket.h
@@ -56,4 +56,7 @@  int unix_connect(const char *path);
 int parse_host_port(struct sockaddr_in *saddr, const char *str);
 int socket_init(void);

+int tcp_client_start(const char *str, int *fd);
+int tcp_server_start(const char *str, int *fd);
+
 #endif /* QEMU_SOCKET_H */