[Qemu-devel] [PATCH v2 0/3] Allow ipv6 for migration

[Qemu-devel] [PATCH v2 0/3] Allow ipv6 for migration

[Juan Quintela]

Hi


v2:
- create socket_set_reuseaddr() to have it different for
  Windows and everything else (Peter Maydell)
- use strerror() instead of perror (Peter Maydell)
- fprintf(): one of them removed, the other one ...
  it is complicated, would have to return two errors: getaddrinfo one
  and normal errno.  Not feasible IMHO until we integrate all other users
  of getaddrinfo() and think of a nice API.

v1.

1st patch moves migration to use getaddrinfo() instead of parse_host_port().
This allows us to use ipv6 addresses.  As an extra bonus, now we can use
names from /etc/services.

Code for net_socket_listen_init() and net_socket_connect_init() was
almost identical to migration one, so also changed that ones.
The difference were small based on how error codes were handled.
After discussing with Anthony, it appears that the right ones are
the migration ones.

Tested (the migration code) with:
- "tcp:foo:4444" ipv4 name
- "tcp:foo6:4444" ipv6 name
- "tcp:0:4444"
- "tcp:foo6:iqobject" (this was an unused entry on my machine /etc/services)
   iqobject        48619/tcp               # iqobject
- "tcp::4444": let the kernel make a choice

Please review.

Later, Juan.

Juan Quintela (3):
  create socket_set_reuseaddr()
  Use getaddrinfo for migration
  net/socket: allow ipv6 for net_socket_listen_init and
    net_socket_connect_init

 gdbstub.c          |    6 +--
 migration-tcp.c    |   53 ++++++-------------------
 nbd.c              |    5 +--
 net.c              |  106 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 net/socket.c       |   66 ++++++--------------------------
 os-posix.c         |    1 +
 oslib-posix.c      |    8 ++++
 oslib-win32.c      |    6 +++
 qemu-sockets.c     |    6 +-
 qemu_socket.h      |    4 ++
 tests/linux-test.c |    4 +-
 11 files changed, 158 insertions(+), 107 deletions(-)

-- 
1.7.4

[Qemu-devel] [PATCH 1/3] create socket_set_reuseaddr()

[Juan Quintela]

Windows is different than unix, SO_REUSEADDR is the default value
there.  Create one function to do it and change all callers.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 gdbstub.c          |    6 ++----
 migration-tcp.c    |    4 +---
 nbd.c              |    5 +----
 net/socket.c       |   13 ++++---------
 os-posix.c         |    1 +
 oslib-posix.c      |    8 ++++++++
 oslib-win32.c      |    6 ++++++
 qemu-sockets.c     |    6 +++---
 qemu_socket.h      |    1 +
 tests/linux-test.c |    4 +---
 10 files changed, 28 insertions(+), 26 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index 1e9f931..b12e20e 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -2591,7 +2591,7 @@ static void gdb_accept(void)
 static int gdbserver_open(int port)
 {
     struct sockaddr_in sockaddr;
-    int fd, val, ret;
+    int fd, ret;

     fd = socket(PF_INET, SOCK_STREAM, 0);
     if (fd < 0) {
@@ -2602,9 +2602,7 @@ static int gdbserver_open(int port)
     fcntl(fd, F_SETFD, FD_CLOEXEC);
 #endif

-    /* allow fast reuse */
-    val = 1;
-    setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, (char *)&val, sizeof(val));
+    socket_set_reuseaddr(fd);

     sockaddr.sin_family = AF_INET;
     sockaddr.sin_port = htons(port);
diff --git a/migration-tcp.c b/migration-tcp.c
index b55f419..2340b55 100644
--- a/migration-tcp.c
+++ b/migration-tcp.c
@@ -172,7 +172,6 @@ out2:
 int tcp_start_incoming_migration(const char *host_port)
 {
     struct sockaddr_in addr;
-    int val;
     int s;

     if (parse_host_port(&addr, host_port) < 0) {
@@ -184,8 +183,7 @@ int tcp_start_incoming_migration(const char *host_port)
     if (s == -1)
         return -socket_error();

-    val = 1;
-    setsockopt(s, SOL_SOCKET, SO_REUSEADDR, (const char *)&val, sizeof(val));
+    socket_set_reuseaddr(s);

     if (bind(s, (struct sockaddr *)&addr, sizeof(addr)) == -1)
         goto err;
diff --git a/nbd.c b/nbd.c
index d8ebc42..3ecc792 100644
--- a/nbd.c
+++ b/nbd.c
@@ -148,7 +148,6 @@ int tcp_socket_incoming(const char *address, uint16_t port)
     int s;
     struct in_addr in;
     struct sockaddr_in addr;
-    int opt;

     s = socket(PF_INET, SOCK_STREAM, 0);
     if (s == -1) {
@@ -170,9 +169,7 @@ int tcp_socket_incoming(const char *address, uint16_t port)
     addr.sin_port = htons(port);
     memcpy(&addr.sin_addr.s_addr, &in, sizeof(in));

-    opt = 1;
-    if (setsockopt(s, SOL_SOCKET, SO_REUSEADDR,
-                   (const void *) &opt, sizeof(opt)) == -1) {
+    if (socket_set_reuseaddr(s)) {
         goto error;
     }

diff --git a/net/socket.c b/net/socket.c
index 7337f4f..de46506 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -167,11 +167,9 @@ static int net_socket_mcast_create(struct sockaddr_in *mcastaddr, struct in_addr
         return -1;
     }

-    val = 1;
-    ret=setsockopt(fd, SOL_SOCKET, SO_REUSEADDR,
-                   (const char *)&val, sizeof(val));
+    ret = socket_set_reuseaddr(fd);
     if (ret < 0) {
-	perror("setsockopt(SOL_SOCKET, SO_REUSEADDR)");
+	perror("socket_set_reuseaddr");
 	goto fail;
     }

@@ -392,7 +390,7 @@ static int net_socket_listen_init(VLANState *vlan,
                                   const char *host_str)
 {
     NetSocketListenState *s;
-    int fd, val, ret;
+    int fd, ret;
     struct sockaddr_in saddr;

     if (parse_host_port(&saddr, host_str) < 0)
@@ -406,10 +404,7 @@ static int net_socket_listen_init(VLANState *vlan,
         return -1;
     }
     socket_set_nonblock(fd);
-
-    /* allow fast reuse */
-    val = 1;
-    setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, (const char *)&val, sizeof(val));
+    socket_set_reuseaddr(fd);

     ret = bind(fd, (struct sockaddr *)&saddr, sizeof(saddr));
     if (ret < 0) {
diff --git a/os-posix.c b/os-posix.c
index 38c29d1..e7116c7 100644
--- a/os-posix.c
+++ b/os-posix.c
@@ -382,3 +382,4 @@ int qemu_create_pidfile(const char *filename)

     return 0;
 }
+
diff --git a/oslib-posix.c b/oslib-posix.c
index 7bc5f7c..2f3383a 100644
--- a/oslib-posix.c
+++ b/oslib-posix.c
@@ -72,6 +72,14 @@ void qemu_vfree(void *ptr)
     free(ptr);
 }

+int  socket_set_reuseaddr(int fd)
+{
+    int val = 1;
+    /* allow fast reuse */
+    return setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, (const char *)&val,
+                      sizeof(val));
+}
+
 void socket_set_nonblock(int fd)
 {
     int f;
diff --git a/oslib-win32.c b/oslib-win32.c
index 5f0759f..1641e1f 100644
--- a/oslib-win32.c
+++ b/oslib-win32.c
@@ -79,6 +79,12 @@ void socket_set_nonblock(int fd)
     ioctlsocket(fd, FIONBIO, &opt);
 }

+int socket_set_reuseaddr(int fd)
+{
+    /* Nothing, this is the default windows behaviour */
+    return 0;
+}
+
 int inet_aton(const char *cp, struct in_addr *ia)
 {
     uint32_t addr = inet_addr(cp);
diff --git a/qemu-sockets.c b/qemu-sockets.c
index c526324..293dad6 100644
--- a/qemu-sockets.c
+++ b/qemu-sockets.c
@@ -168,7 +168,7 @@ int inet_listen_opts(QemuOpts *opts, int port_offset)
             continue;
         }

-        setsockopt(slisten,SOL_SOCKET,SO_REUSEADDR,(void*)&on,sizeof(on));
+        socket_set_reuseaddr(slisten);
 #ifdef IPV6_V6ONLY
         if (e->ai_family == PF_INET6) {
             /* listen on both ipv4 and ipv6 */
@@ -265,7 +265,7 @@ int inet_connect_opts(QemuOpts *opts)
             inet_strfamily(e->ai_family), strerror(errno));
             continue;
         }
-        setsockopt(sock,SOL_SOCKET,SO_REUSEADDR,(void*)&on,sizeof(on));
+        socket_set_reuseaddr(sock);

         /* connect to peer */
         if (connect(sock,e->ai_addr,e->ai_addrlen) < 0) {
@@ -358,7 +358,7 @@ int inet_dgram_opts(QemuOpts *opts)
                 inet_strfamily(peer->ai_family), strerror(errno));
         goto err;
     }
-    setsockopt(sock,SOL_SOCKET,SO_REUSEADDR,(void*)&on,sizeof(on));
+    socket_set_reuseaddr(sock);

     /* bind socket */
     if (getnameinfo((struct sockaddr*)local->ai_addr,local->ai_addrlen,
diff --git a/qemu_socket.h b/qemu_socket.h
index 180e4db..f9a7f95 100644
--- a/qemu_socket.h
+++ b/qemu_socket.h
@@ -36,6 +36,7 @@ int inet_aton(const char *cp, struct in_addr *ia);
 int qemu_socket(int domain, int type, int protocol);
 int qemu_accept(int s, struct sockaddr *addr, socklen_t *addrlen);
 void socket_set_nonblock(int fd);
+int socket_set_reuseaddr(int fd);
 int send_all(int fd, const void *buf, int len1);

 /* New, ipv6-ready socket helper functions, see qemu-sockets.c */
diff --git a/tests/linux-test.c b/tests/linux-test.c
index 2e4a746..2635d64 100644
--- a/tests/linux-test.c
+++ b/tests/linux-test.c
@@ -283,9 +283,7 @@ int server_socket(void)

     /* server socket */
     fd = chk_error(socket(PF_INET, SOCK_STREAM, 0));
-
-    val = 1;
-    chk_error(setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &val, sizeof(val)));
+    chk_error(socket_set_reuseaddr(fd));

     sockaddr.sin_family = AF_INET;
     sockaddr.sin_port = htons(TESTPORT);
-- 
1.7.4

[Qemu-devel] [PATCH 2/3] Use getaddrinfo for migration

[Juan Quintela]

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 |   51 +++++++--------------------
 net.c           |  106 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 qemu_socket.h   |    3 ++
 3 files changed, 122 insertions(+), 38 deletions(-)

diff --git a/migration-tcp.c b/migration-tcp.c
index 2340b55..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,23 +155,14 @@ out2:

 int tcp_start_incoming_migration(const char *host_port)
 {
-    struct sockaddr_in addr;
+    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();
-
-    socket_set_reuseaddr(s);
-
-    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..4f61cae 100644
--- a/net.c
+++ b/net.c
@@ -94,6 +94,112 @@ 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 ret;
+
+    socket_set_reuseaddr(fd);
+
+    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);
+    }
+
+    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 f9a7f95..65f7ef0 100644
--- a/qemu_socket.h
+++ b/qemu_socket.h
@@ -57,4 +57,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 */
-- 
1.7.4

[Qemu-devel] [PATCH 3/3] net/socket: allow ipv6 for net_socket_listen_init and net_socket_connect_init

[Juan Quintela]

Remove use of parse_host_port.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 net/socket.c |   55 ++++++++++---------------------------------------------
 1 files changed, 10 insertions(+), 45 deletions(-)

diff --git a/net/socket.c b/net/socket.c
index de46506..aa0b2d8 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -391,24 +391,12 @@ static int net_socket_listen_init(VLANState *vlan,
 {
     NetSocketListenState *s;
     int fd, ret;
-    struct sockaddr_in saddr;
-
-    if (parse_host_port(&saddr, host_str) < 0)
-        return -1;

     s = qemu_mallocz(sizeof(NetSocketListenState));

-    fd = qemu_socket(PF_INET, SOCK_STREAM, 0);
-    if (fd < 0) {
-        perror("socket");
-        return -1;
-    }
-    socket_set_nonblock(fd);
-    socket_set_reuseaddr(fd);
-
-    ret = bind(fd, (struct sockaddr *)&saddr, sizeof(saddr));
+    ret = tcp_server_start(host_str, &fd);
     if (ret < 0) {
-        perror("bind");
+        fprintf(stderr, "tcp_server_start: %s\n", strerror(ret));
         return -1;
     }
     ret = listen(fd, 0);
@@ -430,40 +418,17 @@ static int net_socket_connect_init(VLANState *vlan,
                                    const char *host_str)
 {
     NetSocketState *s;
-    int fd, connected, ret, err;
+    int fd, connected, ret;
     struct sockaddr_in saddr;

-    if (parse_host_port(&saddr, host_str) < 0)
-        return -1;
-
-    fd = qemu_socket(PF_INET, SOCK_STREAM, 0);
-    if (fd < 0) {
-        perror("socket");
+    ret = tcp_client_start(host_str, &fd);
+    if (ret == -EINPROGRESS || ret == -EWOULDBLOCK) {
+        connected = 0;
+    } else if (ret < 0) {
+        closesocket(fd);
         return -1;
-    }
-    socket_set_nonblock(fd);
-
-    connected = 0;
-    for(;;) {
-        ret = connect(fd, (struct sockaddr *)&saddr, sizeof(saddr));
-        if (ret < 0) {
-            err = socket_error();
-            if (err == EINTR || err == EWOULDBLOCK) {
-            } else if (err == EINPROGRESS) {
-                break;
-#ifdef _WIN32
-            } else if (err == WSAEALREADY || err == WSAEINVAL) {
-                break;
-#endif
-            } else {
-                perror("connect");
-                closesocket(fd);
-                return -1;
-            }
-        } else {
-            connected = 1;
-            break;
-        }
+    } else {
+        connected = 1;
     }
     s = net_socket_fd_init(vlan, model, name, fd, connected);
     if (!s)
-- 
1.7.4

Re: [Qemu-devel] [PATCH v2 0/3] Allow ipv6 for migration

[Amit Shah]

On (Fri) 18 Mar 2011 [14:22:47], Juan Quintela wrote:
> Hi
> 
> 
> v2:
> - create socket_set_reuseaddr() to have it different for
>   Windows and everything else (Peter Maydell)
> - use strerror() instead of perror (Peter Maydell)
> - fprintf(): one of them removed, the other one ...
>   it is complicated, would have to return two errors: getaddrinfo one
>   and normal errno.  Not feasible IMHO until we integrate all other users
>   of getaddrinfo() and think of a nice API.
> 
> v1.
> 
> 1st patch moves migration to use getaddrinfo() instead of parse_host_port().
> This allows us to use ipv6 addresses.  As an extra bonus, now we can use
> names from /etc/services.
> 
> Code for net_socket_listen_init() and net_socket_connect_init() was
> almost identical to migration one, so also changed that ones.
> The difference were small based on how error codes were handled.
> After discussing with Anthony, it appears that the right ones are
> the migration ones.
> 
> Tested (the migration code) with:
> - "tcp:foo:4444" ipv4 name
> - "tcp:foo6:4444" ipv6 name
> - "tcp:0:4444"
> - "tcp:foo6:iqobject" (this was an unused entry on my machine /etc/services)
>    iqobject        48619/tcp               # iqobject
> - "tcp::4444": let the kernel make a choice
> 
> Please review.

ACK series.

A couple of whitespace issues; if you're fixing that, you can add my
acked-by to the git log.

		Amit

Re: [Qemu-devel] [PATCH 1/3] create socket_set_reuseaddr()

[Amit Shah]

On (Fri) 18 Mar 2011 [14:22:48], Juan Quintela wrote:
> Windows is different than unix, SO_REUSEADDR is the default value
> there.  Create one function to do it and change all callers.
> 
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
>  gdbstub.c          |    6 ++----
>  migration-tcp.c    |    4 +---
>  nbd.c              |    5 +----
>  net/socket.c       |   13 ++++---------
>  os-posix.c         |    1 +

Nitpick: this addition in os-posix is a newline, can be dropped.

		Amit

Re: [Qemu-devel] [PATCH 2/3] Use getaddrinfo for migration

[Amit Shah]

On (Fri) 18 Mar 2011 [14:22:49], Juan Quintela wrote:
> 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 |   51 +++++++--------------------
>  net.c           |  106 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  qemu_socket.h   |    3 ++
>  3 files changed, 122 insertions(+), 38 deletions(-)
> 
> diff --git a/migration-tcp.c b/migration-tcp.c
> index 2340b55..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)

Another nitpick: one newline should be kept.

		Amit