Skip to contentSkip to conversation

Allow ipv6 for migration

PATCH In review Juan Quintela 2 participants 7 messages Mar 18, 2011 – Mar 23, 2011

Subsystems (2)
Networking Sockets
lore
Share & tools
lore

Juan Quintela

author
#0 lore raw

v2 cover Allow ipv6 for migration

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(-)
Signature · 1 line
1.7.4
author
#1 lore raw

PATCH v2 1/3 create socket_set_reuseaddr()

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 <[email protected]>
---
 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(-)
gdbstub.c +2 -4
index 1e9f931..b12e20e 100644
@@ -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);
migration-tcp.c +1 -3
index b55f419..2340b55 100644
@@ -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;
nbd.c +1 -4
index d8ebc42..3ecc792 100644
@@ -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;
     }
net/socket.c +4 -9
index 7337f4f..de46506 100644
@@ -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) {
os-posix.c +1 -0
index 38c29d1..e7116c7 100644
@@ -382,3 +382,4 @@ int qemu_create_pidfile(const char *filename)
 
     return 0;
 }
+
oslib-posix.c +8 -0
index 7bc5f7c..2f3383a 100644
@@ -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;
oslib-win32.c +6 -0
index 5f0759f..1641e1f 100644
@@ -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);
qemu-sockets.c +3 -3
index c526324..293dad6 100644
@@ -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,
qemu_socket.h +1 -0
index 180e4db..f9a7f95 100644
@@ -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 */
tests/linux-test.c +1 -3
index 2e4a746..2635d64 100644
@@ -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);
Signature · 1 line
1.7.4
#2 lore raw

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

Quoted 10 lines · 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 <[email protected]>
---
 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

author
#3 lore raw

PATCH v2 2/3 Use getaddrinfo for migration

In reply to 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 <[email protected]>
---
 migration-tcp.c |   51 +++++++--------------------
 net.c           |  106 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 qemu_socket.h   |    3 ++
 3 files changed, 122 insertions(+), 38 deletions(-)
migration-tcp.c +13 -38
index 2340b55..2fa496a 100644
@@ -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;
net.c +106 -0

112 lines not on this page · raw message

qemu_socket.h +3 -0
index f9a7f95..65f7ef0 100644
@@ -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 */
Signature · 1 line
1.7.4
#4 lore raw

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

Quoted 15 lines · 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 <[email protected]>
---
 migration-tcp.c |   51 +++++++--------------------
 net.c           |  106 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 qemu_socket.h   |    3 ++
 3 files changed, 122 insertions(+), 38 deletions(-)
migration-tcp.c +0 -0
index 2340b55..2fa496a 100644

migration-tcp.c static int tcp_close(FdMigrationState *s) · line 51

Quoted 6 lines
migration-tcp.c +0 -2
@@ -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

author
#5 lore raw

PATCH v2 3/3 net/socket: allow ipv6 for net_socket_listen_init and net_socket_connect_init

In reply to Juan Quintela

Remove use of parse_host_port.

Signed-off-by Juan Quintela <[email protected]>
---
 net/socket.c |   55 ++++++++++---------------------------------------------
 1 files changed, 10 insertions(+), 45 deletions(-)
net/socket.c +10 -45
index de46506..aa0b2d8 100644
@@ -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)
Signature · 1 line
1.7.4
#6 lore raw
In reply to Juan Quintela
Quoted 32 lines · 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

Quoted 1 line

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