[Qemu-devel] [PATCH v2 0/9] support to migrate with IPv6 address

[Qemu-devel] [PATCH v2 0/9] support to migrate with IPv6 address

[Amos Kong]

Those patches make migration of IPv6 address work, old code 
only support to parse IPv4 address/port, use getaddrinfo()
to get socket addresses infomation.
Last two patches are about spliting IPv6 host/port.

Changes from v1:
- split different changes to small patches, it will be
  easier to review
- fixed some problem according to Kevin's comment

---

Amos Kong (9):
      net: introduce tcp_server_start()
      net: use tcp_server_start() for tcp server creation
      net: introduce tcp_client_start()
      net: use tcp_client_start for tcp client creation
      net: refector tcp_*_start functions
      net: use getaddrinfo() in tcp_start_common
      net: introduce parse_host_port_info()
      net: split hostname and service by last colon
      net: support to include ipv6 address by brackets


 migration-tcp.c |   62 +++++++------------------
 net.c           |  137 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 net/socket.c    |   64 ++++++--------------------
 qemu_socket.h   |    3 +
 4 files changed, 171 insertions(+), 95 deletions(-)

-- 
Amos Kong

[Qemu-devel] [PATCH v2 1/9] net: introduce tcp_server_start()

[Amos Kong]

Introduce tcp_server_start() by moving original code in
tcp_start_incoming_migration().

Signed-off-by: Amos Kong <akong@redhat.com>
---
 net.c         |   27 +++++++++++++++++++++++++++
 qemu_socket.h |    2 ++
 2 files changed, 29 insertions(+), 0 deletions(-)

diff --git a/net.c b/net.c
index c34474f..0260968 100644
--- a/net.c
+++ b/net.c
@@ -99,6 +99,33 @@ static int get_str_sep(char *buf, int buf_size, const char **pp, int sep)
     return 0;
 }
 
+int tcp_server_start(const char *str, int *fd)
+{
+    int val, ret;
+    struct sockaddr_in saddr;
+
+    if (parse_host_port(&saddr, str) < 0) {
+        return -1;
+    }
+
+    *fd = qemu_socket(PF_INET, SOCK_STREAM, 0);
+    if (fd < 0) {
+        perror("socket");
+        return -1;
+    }
+    socket_set_nonblock(*fd);
+
+    /* allow fast reuse */
+    val = 1;
+    setsockopt(*fd, SOL_SOCKET, SO_REUSEADDR, (const char *)&val, sizeof(val));
+
+    ret = bind(*fd, (struct sockaddr *)&saddr, sizeof(saddr));
+    if (ret < 0) {
+        closesocket(*fd);
+    }
+    return ret;
+}
+
 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 fe4cf6c..d612793 100644
--- a/qemu_socket.h
+++ b/qemu_socket.h
@@ -54,6 +54,8 @@ int unix_listen(const char *path, char *ostr, int olen);
 int unix_connect_opts(QemuOpts *opts);
 int unix_connect(const char *path);
 
+int tcp_server_start(const char *str, int *fd);
+
 /* Old, ipv4 only bits.  Don't use for new code. */
 int parse_host_port(struct sockaddr_in *saddr, const char *str);
 int socket_init(void);

Re: [Qemu-devel] [PATCH v2 1/9] net: introduce tcp_server_start()

[Orit Wasserman]

On 03/05/2012 12:03 PM, Amos Kong wrote:
> Introduce tcp_server_start() by moving original code in
> tcp_start_incoming_migration().
> 
> Signed-off-by: Amos Kong <akong@redhat.com>
> ---
>  net.c         |   27 +++++++++++++++++++++++++++
>  qemu_socket.h |    2 ++
>  2 files changed, 29 insertions(+), 0 deletions(-)
> 
> diff --git a/net.c b/net.c
> index c34474f..0260968 100644
> --- a/net.c
> +++ b/net.c
> @@ -99,6 +99,33 @@ static int get_str_sep(char *buf, int buf_size, const char **pp, int sep)
>      return 0;
>  }
>  
> +int tcp_server_start(const char *str, int *fd)
> +{
> +    int val, ret;
> +    struct sockaddr_in saddr;
> +
> +    if (parse_host_port(&saddr, str) < 0) {

error message would be nice 

> +        return -1;
> +    }
> +
> +    *fd = qemu_socket(PF_INET, SOCK_STREAM, 0);
> +    if (fd < 0) {
> +        perror("socket");
> +        return -1;
> +    }

this is actually net_socket_listen_init version 
tcp_start_incoming_migration returns the error -socket_error().
I prefer not to lose the errno.

I know that when calling net_socket_listen_init for some unknown reason there is an explict check for -1
" if (net_socket_listen_init(vlan, "socket", name, listen) == -1) {"
I think it is a good opportunity to change this check.

Orit
> +    socket_set_nonblock(*fd);
> +
> +    /* allow fast reuse */
> +    val = 1;
> +    setsockopt(*fd, SOL_SOCKET, SO_REUSEADDR, (const char *)&val, sizeof(val));
> +
> +    ret = bind(*fd, (struct sockaddr *)&saddr, sizeof(saddr));
> +    if (ret < 0) {
> +        closesocket(*fd);
> +    }
> +    return ret;
> +}
> +
>  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 fe4cf6c..d612793 100644
> --- a/qemu_socket.h
> +++ b/qemu_socket.h
> @@ -54,6 +54,8 @@ int unix_listen(const char *path, char *ostr, int olen);
>  int unix_connect_opts(QemuOpts *opts);
>  int unix_connect(const char *path);
>  
> +int tcp_server_start(const char *str, int *fd);
> +
>  /* Old, ipv4 only bits.  Don't use for new code. */
>  int parse_host_port(struct sockaddr_in *saddr, const char *str);
>  int socket_init(void);
> 
> 

Re: [Qemu-devel] [PATCH v2 1/9] net: introduce tcp_server_start()

[Amos Kong]

On 05/03/12 21:25, Orit Wasserman wrote:
> On 03/05/2012 12:03 PM, Amos Kong wrote:
>> Introduce tcp_server_start() by moving original code in
>> tcp_start_incoming_migration().
>>
>> Signed-off-by: Amos Kong<akong@redhat.com>
>> ---
>>   net.c         |   27 +++++++++++++++++++++++++++
>>   qemu_socket.h |    2 ++
>>   2 files changed, 29 insertions(+), 0 deletions(-)
>>
>> diff --git a/net.c b/net.c
>> index c34474f..0260968 100644
>> --- a/net.c
>> +++ b/net.c
>> @@ -99,6 +99,33 @@ static int get_str_sep(char *buf, int buf_size, const char **pp, int sep)
>>       return 0;
>>   }
>>
>> +int tcp_server_start(const char *str, int *fd)
>> +{
>> +    int val, ret;
>> +    struct sockaddr_in saddr;
>> +
>> +    if (parse_host_port(&saddr, str)<  0) {
>
> error message would be nice
>
>> +        return -1;
>> +    }
>> +
>> +    *fd = qemu_socket(PF_INET, SOCK_STREAM, 0);
>> +    if (fd<  0) {
>> +        perror("socket");
>> +        return -1;
>> +    }
>
> this is actually net_socket_listen_init version
> tcp_start_incoming_migration returns the error -socket_error().
> I prefer not to lose the errno.

agree.

> I know that when calling net_socket_listen_init for some unknown reason there is an explict check for -1
> " if (net_socket_listen_init(vlan, "socket", name, listen) == -1) {"
> I think it is a good opportunity to change this check.

nod.

>
> Orit
>> +    socket_set_nonblock(*fd);
>> +
>> +    /* allow fast reuse */
>> +    val = 1;
>> +    setsockopt(*fd, SOL_SOCKET, SO_REUSEADDR, (const char *)&val, sizeof(val));
>> +
>> +    ret = bind(*fd, (struct sockaddr *)&saddr, sizeof(saddr));
>> +    if (ret<  0) {
>> +        closesocket(*fd);
>> +    }
>> +    return ret;
>> +}
>> +
>>   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 fe4cf6c..d612793 100644
>> --- a/qemu_socket.h
>> +++ b/qemu_socket.h
>> @@ -54,6 +54,8 @@ int unix_listen(const char *path, char *ostr, int olen);
>>   int unix_connect_opts(QemuOpts *opts);
>>   int unix_connect(const char *path);
>>
>> +int tcp_server_start(const char *str, int *fd);
>> +
>>   /* Old, ipv4 only bits.  Don't use for new code. */
>>   int parse_host_port(struct sockaddr_in *saddr, const char *str);
>>   int socket_init(void);



-- 
			Amos.

[Qemu-devel] [PATCH v2 2/9] net: use tcp_server_start() for tcp server creation

[Amos Kong]

Use tcp_server_start in those two functions:
 tcp_start_incoming_migration()
 net_socket_listen_init()

Signed-off-by: Amos Kong <akong@redhat.com>
---
 migration-tcp.c |   21 +++++----------------
 net/socket.c    |   23 +++--------------------
 2 files changed, 8 insertions(+), 36 deletions(-)

diff --git a/migration-tcp.c b/migration-tcp.c
index 35a5781..ecadd10 100644
--- a/migration-tcp.c
+++ b/migration-tcp.c
@@ -157,28 +157,17 @@ out2:
 
 int tcp_start_incoming_migration(const char *host_port)
 {
-    struct sockaddr_in addr;
-    int val;
+    int ret;
     int s;
 
     DPRINTF("Attempting to start an incoming migration\n");
 
-    if (parse_host_port(&addr, host_port) < 0) {
-        fprintf(stderr, "invalid host/port combination: %s\n", host_port);
-        return -EINVAL;
-    }
-
-    s = qemu_socket(PF_INET, SOCK_STREAM, 0);
-    if (s == -1) {
-        return -socket_error();
+    ret = tcp_server_start(host_port, &s);
+    if (ret < 0) {
+        fprintf(stderr, "tcp_server_start: %s\n", strerror(-ret));
+        return ret;
     }
 
-    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/socket.c b/net/socket.c
index 0bcf229..5feb3d2 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -403,31 +403,14 @@ static int net_socket_listen_init(VLANState *vlan,
                                   const char *host_str)
 {
     NetSocketListenState *s;
-    int fd, val, ret;
-    struct sockaddr_in saddr;
-
-    if (parse_host_port(&saddr, host_str) < 0)
-        return -1;
+    int fd, ret;
 
     s = g_malloc0(sizeof(NetSocketListenState));
 
-    fd = qemu_socket(PF_INET, SOCK_STREAM, 0);
-    if (fd < 0) {
-        perror("socket");
-        g_free(s);
-        return -1;
-    }
-    socket_set_nonblock(fd);
-
-    /* allow fast reuse */
-    val = 1;
-    setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, (const char *)&val, sizeof(val));
-
-    ret = bind(fd, (struct sockaddr *)&saddr, sizeof(saddr));
+    ret = tcp_server_start(host_str, &fd);
     if (ret < 0) {
-        perror("bind");
+        error_report("tcp_server_start: %s", strerror(-ret));
         g_free(s);
-        closesocket(fd);
         return -1;
     }
     ret = listen(fd, 0);

Re: [Qemu-devel] [PATCH v2 2/9] net: use tcp_server_start() for tcp server creation

[Orit Wasserman]

On 03/05/2012 12:03 PM, Amos Kong wrote:
> Use tcp_server_start in those two functions:
>  tcp_start_incoming_migration()
>  net_socket_listen_init()
> 
> Signed-off-by: Amos Kong <akong@redhat.com>
> ---
>  migration-tcp.c |   21 +++++----------------
>  net/socket.c    |   23 +++--------------------
>  2 files changed, 8 insertions(+), 36 deletions(-)
> 
> diff --git a/migration-tcp.c b/migration-tcp.c
> index 35a5781..ecadd10 100644
> --- a/migration-tcp.c
> +++ b/migration-tcp.c
> @@ -157,28 +157,17 @@ out2:
>  
>  int tcp_start_incoming_migration(const char *host_port)
>  {
> -    struct sockaddr_in addr;
> -    int val;
> +    int ret;
>      int s;
>  
>      DPRINTF("Attempting to start an incoming migration\n");
>  
> -    if (parse_host_port(&addr, host_port) < 0) {
> -        fprintf(stderr, "invalid host/port combination: %s\n", host_port);
> -        return -EINVAL;
> -    }
> -
> -    s = qemu_socket(PF_INET, SOCK_STREAM, 0);
> -    if (s == -1) {
> -        return -socket_error();
> +    ret = tcp_server_start(host_port, &s);
> +    if (ret < 0) {
> +        fprintf(stderr, "tcp_server_start: %s\n", strerror(-ret));
> +        return ret;
>      }
>  
> -    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/socket.c b/net/socket.c
> index 0bcf229..5feb3d2 100644
> --- a/net/socket.c
> +++ b/net/socket.c
> @@ -403,31 +403,14 @@ static int net_socket_listen_init(VLANState *vlan,
>                                    const char *host_str)
>  {
>      NetSocketListenState *s;
> -    int fd, val, ret;
> -    struct sockaddr_in saddr;
> -
> -    if (parse_host_port(&saddr, host_str) < 0)
> -        return -1;
> +    int fd, ret;
>  
>      s = g_malloc0(sizeof(NetSocketListenState));
>  
> -    fd = qemu_socket(PF_INET, SOCK_STREAM, 0);
> -    if (fd < 0) {
> -        perror("socket");
> -        g_free(s);
> -        return -1;
> -    }
> -    socket_set_nonblock(fd);
> -
> -    /* allow fast reuse */
> -    val = 1;
> -    setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, (const char *)&val, sizeof(val));
> -
> -    ret = bind(fd, (struct sockaddr *)&saddr, sizeof(saddr));
> +    ret = tcp_server_start(host_str, &fd);
>      if (ret < 0) {
> -        perror("bind");
> +        error_report("tcp_server_start: %s", strerror(-ret));

If the return value is always -1 this has no meaning

Orit
>          g_free(s);
> -        closesocket(fd);
>          return -1;
>      }
>      ret = listen(fd, 0);
> 
> 

Re: [Qemu-devel] [PATCH v2 2/9] net: use tcp_server_start() for tcp server creation

[Amos Kong]

On 05/03/12 21:27, Orit Wasserman wrote:
> On 03/05/2012 12:03 PM, Amos Kong wrote:
>> Use tcp_server_start in those two functions:
>>   tcp_start_incoming_migration()
>>   net_socket_listen_init()
>>
>> Signed-off-by: Amos Kong<akong@redhat.com>
>> ---
>>   migration-tcp.c |   21 +++++----------------
>>   net/socket.c    |   23 +++--------------------
>>   2 files changed, 8 insertions(+), 36 deletions(-)
>>
>> diff --git a/migration-tcp.c b/migration-tcp.c
>> index 35a5781..ecadd10 100644
>> --- a/migration-tcp.c
>> +++ b/migration-tcp.c
>> @@ -157,28 +157,17 @@ out2:
>>
>>   int tcp_start_incoming_migration(const char *host_port)
>>   {
>> -    struct sockaddr_in addr;
>> -    int val;
>> +    int ret;
>>       int s;
>>
>>       DPRINTF("Attempting to start an incoming migration\n");
>>
>> -    if (parse_host_port(&addr, host_port)<  0) {
>> -        fprintf(stderr, "invalid host/port combination: %s\n", host_port);
>> -        return -EINVAL;
>> -    }
>> -
>> -    s = qemu_socket(PF_INET, SOCK_STREAM, 0);
>> -    if (s == -1) {
>> -        return -socket_error();
>> +    ret = tcp_server_start(host_port,&s);
>> +    if (ret<  0) {
>> +        fprintf(stderr, "tcp_server_start: %s\n", strerror(-ret));
>> +        return ret;
>>       }
>>
>> -    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/socket.c b/net/socket.c
>> index 0bcf229..5feb3d2 100644
>> --- a/net/socket.c
>> +++ b/net/socket.c
>> @@ -403,31 +403,14 @@ static int net_socket_listen_init(VLANState *vlan,
>>                                     const char *host_str)
>>   {
>>       NetSocketListenState *s;
>> -    int fd, val, ret;
>> -    struct sockaddr_in saddr;
>> -
>> -    if (parse_host_port(&saddr, host_str)<  0)
>> -        return -1;
>> +    int fd, ret;
>>
>>       s = g_malloc0(sizeof(NetSocketListenState));
>>
>> -    fd = qemu_socket(PF_INET, SOCK_STREAM, 0);
>> -    if (fd<  0) {
>> -        perror("socket");
>> -        g_free(s);
>> -        return -1;
>> -    }
>> -    socket_set_nonblock(fd);
>> -
>> -    /* allow fast reuse */
>> -    val = 1;
>> -    setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, (const char *)&val, sizeof(val));
>> -
>> -    ret = bind(fd, (struct sockaddr *)&saddr, sizeof(saddr));
>> +    ret = tcp_server_start(host_str,&fd);
>>       if (ret<  0) {
>> -        perror("bind");
>> +        error_report("tcp_server_start: %s", strerror(-ret));
>
> If the return value is always -1 this has no meaning

Hi Orit,

"return -1;" is the original code, net_socket_listen_init() is only used 
once in net_init_socket()

         if (net_socket_connect_init(vlan, "socket", name, connect) == -1) {
             return -1;
         }

This patch just replace the server creation code by tcp_server_start().

Amos.

[Qemu-devel] [PATCH v2 3/9] net: introduce tcp_client_start()

[Amos Kong]

Introduce tcp_client_start() by moving original code in
tcp_start_outgoing_migration().

Signed-off-by: Amos Kong <akong@redhat.com>
---
 net.c         |   39 +++++++++++++++++++++++++++++++++++++++
 qemu_socket.h |    1 +
 2 files changed, 40 insertions(+), 0 deletions(-)

diff --git a/net.c b/net.c
index 0260968..5c20e22 100644
--- a/net.c
+++ b/net.c
@@ -126,6 +126,45 @@ int tcp_server_start(const char *str, int *fd)
     return ret;
 }
 
+int tcp_client_start(const char *str, int *fd)
+{
+    struct sockaddr_in saddr;
+    int ret;
+
+    if (parse_host_port(&saddr, str) < 0) {
+        return -EINVAL;
+    }
+
+    *fd = qemu_socket(PF_INET, SOCK_STREAM, 0);
+    if (fd < 0) {
+        perror("socket");
+        return -1;
+    }
+    socket_set_nonblock(*fd);
+
+    for (;;) {
+        ret = connect(*fd, (struct sockaddr *)&saddr, sizeof(saddr));
+        if (ret < 0) {
+            ret = -socket_error();
+            if (ret == -EINPROGRESS) {
+                break;
+#ifdef _WIN32
+            } else if (ret == -WSAEALREADY || ret == -WSAEINVAL) {
+                break;
+#endif
+            } else if (ret != -EINTR && ret != -EWOULDBLOCK) {
+                perror("connect");
+                closesocket(*fd);
+                return -1;
+            }
+        } else {
+            break;
+        }
+    }
+
+    return ret;
+}
+
 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 d612793..9246578 100644
--- a/qemu_socket.h
+++ b/qemu_socket.h
@@ -55,6 +55,7 @@ int unix_connect_opts(QemuOpts *opts);
 int unix_connect(const char *path);
 
 int tcp_server_start(const char *str, int *fd);
+int tcp_client_start(const char *str, int *fd);
 
 /* Old, ipv4 only bits.  Don't use for new code. */
 int parse_host_port(struct sockaddr_in *saddr, const char *str);

Re: [Qemu-devel] [PATCH v2 3/9] net: introduce tcp_client_start()

[Orit Wasserman]

On 03/05/2012 12:03 PM, Amos Kong wrote:
> Introduce tcp_client_start() by moving original code in
> tcp_start_outgoing_migration().
> 
> Signed-off-by: Amos Kong <akong@redhat.com>
> ---
>  net.c         |   39 +++++++++++++++++++++++++++++++++++++++
>  qemu_socket.h |    1 +
>  2 files changed, 40 insertions(+), 0 deletions(-)
> 
> diff --git a/net.c b/net.c
> index 0260968..5c20e22 100644
> --- a/net.c
> +++ b/net.c
> @@ -126,6 +126,45 @@ int tcp_server_start(const char *str, int *fd)
>      return ret;
>  }
>  
> +int tcp_client_start(const char *str, int *fd)
> +{
> +    struct sockaddr_in saddr;
> +    int ret;
> +
> +    if (parse_host_port(&saddr, str) < 0) {
> +        return -EINVAL;
> +    }
> +
> +    *fd = qemu_socket(PF_INET, SOCK_STREAM, 0);
> +    if (fd < 0) {
> +        perror("socket");
> +        return -1;
> +    }
> +    socket_set_nonblock(*fd);
> +
> +    for (;;) {
> +        ret = connect(*fd, (struct sockaddr *)&saddr, sizeof(saddr));
> +        if (ret < 0) {
> +            ret = -socket_error();
> +            if (ret == -EINPROGRESS) {
> +                break;
> +#ifdef _WIN32
> +            } else if (ret == -WSAEALREADY || ret == -WSAEINVAL) {
> +                break;
> +#endif
> +            } else if (ret != -EINTR && ret != -EWOULDBLOCK) {
> +                perror("connect");
> +                closesocket(*fd);
> +                return -1;

I think it should be: return ret (otherwise you lose the error code).
And you need it.

Orit
> +            }
> +        } else {
> +            break;
> +        }
> +    }
> +
> +    return ret;
> +}
> +
>  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 d612793..9246578 100644
> --- a/qemu_socket.h
> +++ b/qemu_socket.h
> @@ -55,6 +55,7 @@ int unix_connect_opts(QemuOpts *opts);
>  int unix_connect(const char *path);
>  
>  int tcp_server_start(const char *str, int *fd);
> +int tcp_client_start(const char *str, int *fd);
>  
>  /* Old, ipv4 only bits.  Don't use for new code. */
>  int parse_host_port(struct sockaddr_in *saddr, const char *str);
> 
> 

Re: [Qemu-devel] [PATCH v2 3/9] net: introduce tcp_client_start()

[Orit Wasserman]

On 03/05/2012 12:03 PM, Amos Kong wrote:
> Introduce tcp_client_start() by moving original code in
> tcp_start_outgoing_migration().
> 
> Signed-off-by: Amos Kong <akong@redhat.com>
> ---
>  net.c         |   39 +++++++++++++++++++++++++++++++++++++++
>  qemu_socket.h |    1 +
>  2 files changed, 40 insertions(+), 0 deletions(-)
> 
> diff --git a/net.c b/net.c
> index 0260968..5c20e22 100644
> --- a/net.c
> +++ b/net.c
> @@ -126,6 +126,45 @@ int tcp_server_start(const char *str, int *fd)
>      return ret;
>  }
>  
> +int tcp_client_start(const char *str, int *fd)
> +{
> +    struct sockaddr_in saddr;
> +    int ret;
> +
> +    if (parse_host_port(&saddr, str) < 0) {
> +        return -EINVAL;

You use this in order to know when to call migrate_fd_error this is problematic as another error can return this error code.
I think that setting *fd = -1 in the beginning of the function would be enough.

Orit
> +    }
> +
> +    *fd = qemu_socket(PF_INET, SOCK_STREAM, 0);
> +    if (fd < 0) {
> +        perror("socket");
> +        return -1;
> +    }
> +    socket_set_nonblock(*fd);
> +
> +    for (;;) {
> +        ret = connect(*fd, (struct sockaddr *)&saddr, sizeof(saddr));
> +        if (ret < 0) {
> +            ret = -socket_error();
> +            if (ret == -EINPROGRESS) {
> +                break;
> +#ifdef _WIN32
> +            } else if (ret == -WSAEALREADY || ret == -WSAEINVAL) {
> +                break;
> +#endif
> +            } else if (ret != -EINTR && ret != -EWOULDBLOCK) {
> +                perror("connect");
> +                closesocket(*fd);
> +                return -1;
should be return ret;
> +            }
> +        } else {
> +            break;
> +        }
> +    }
> +
> +    return ret;
> +}
> +
>  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 d612793..9246578 100644
> --- a/qemu_socket.h
> +++ b/qemu_socket.h
> @@ -55,6 +55,7 @@ int unix_connect_opts(QemuOpts *opts);
>  int unix_connect(const char *path);
>  
>  int tcp_server_start(const char *str, int *fd);
> +int tcp_client_start(const char *str, int *fd);
>  
>  /* Old, ipv4 only bits.  Don't use for new code. */
>  int parse_host_port(struct sockaddr_in *saddr, const char *str);
> 
> 

[Qemu-devel] [PATCH v2 4/9] net: use tcp_client_start for tcp client creation

[Amos Kong]

Use tcp_client_start() in those two functions:
 tcp_start_outgoing_migration()
 net_socket_connect_init()

Signed-off-by: Amos Kong <akong@redhat.com>
---
 migration-tcp.c |   41 +++++++++++++----------------------------
 net/socket.c    |   41 +++++++++++------------------------------
 2 files changed, 24 insertions(+), 58 deletions(-)

diff --git a/migration-tcp.c b/migration-tcp.c
index ecadd10..4f89bff 100644
--- a/migration-tcp.c
+++ b/migration-tcp.c
@@ -81,43 +81,28 @@ static void tcp_wait_for_connect(void *opaque)
 
 int tcp_start_outgoing_migration(MigrationState *s, const char *host_port)
 {
-    struct sockaddr_in addr;
     int ret;
-
-    ret = parse_host_port(&addr, host_port);
-    if (ret < 0) {
-        return ret;
-    }
+    int fd;
 
     s->get_error = socket_errno;
     s->write = socket_write;
     s->close = tcp_close;
 
-    s->fd = qemu_socket(PF_INET, SOCK_STREAM, 0);
-    if (s->fd == -1) {
-        DPRINTF("Unable to open socket");
-        return -socket_error();
-    }
-
-    socket_set_nonblock(s->fd);
-
-    do {
-        ret = connect(s->fd, (struct sockaddr *)&addr, sizeof(addr));
-        if (ret == -1) {
-            ret = -socket_error();
-        }
-        if (ret == -EINPROGRESS || ret == -EWOULDBLOCK) {
-            qemu_set_fd_handler2(s->fd, NULL, NULL, tcp_wait_for_connect, s);
-            return 0;
-        }
-    } while (ret == -EINTR);
-
-    if (ret < 0) {
+    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);
+        if (ret != -EINVAL) {
+            migrate_fd_error(s);
+        }
         return ret;
+    } else {
+        migrate_fd_connect(s);
     }
-    migrate_fd_connect(s);
+
     return 0;
 }
 
diff --git a/net/socket.c b/net/socket.c
index 5feb3d2..b7cd8ec 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -434,41 +434,22 @@ 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");
-        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;
+    ret = tcp_client_start(host_str, &fd);
+    if (ret == -EINPROGRESS || ret == -EWOULDBLOCK) {
+        connected = 0;
 #ifdef _WIN32
-            } else if (err == WSAEALREADY || err == WSAEINVAL) {
-                break;
+    } else if (ret == -WSAEALREADY || ret == -WSAEINVAL) {
+        connected = 0;
 #endif
-            } else {
-                perror("connect");
-                closesocket(fd);
-                return -1;
-            }
-        } else {
-            connected = 1;
-            break;
-        }
+    } else if (ret < 0) {
+        return -1;
+    } else {
+        connected = 1;
     }
+
     s = net_socket_fd_init(vlan, model, name, fd, connected);
     if (!s)
         return -1;

Re: [Qemu-devel] [PATCH v2 4/9] net: use tcp_client_start for tcp client creation

[Orit Wasserman]

On 03/05/2012 12:03 PM, Amos Kong wrote:
> Use tcp_client_start() in those two functions:
>  tcp_start_outgoing_migration()
>  net_socket_connect_init()
> 
> Signed-off-by: Amos Kong <akong@redhat.com>
> ---
>  migration-tcp.c |   41 +++++++++++++----------------------------
>  net/socket.c    |   41 +++++++++++------------------------------
>  2 files changed, 24 insertions(+), 58 deletions(-)
> 
> diff --git a/migration-tcp.c b/migration-tcp.c
> index ecadd10..4f89bff 100644
> --- a/migration-tcp.c
> +++ b/migration-tcp.c
> @@ -81,43 +81,28 @@ static void tcp_wait_for_connect(void *opaque)
>  
>  int tcp_start_outgoing_migration(MigrationState *s, const char *host_port)
>  {
> -    struct sockaddr_in addr;
>      int ret;
> -
> -    ret = parse_host_port(&addr, host_port);
> -    if (ret < 0) {
> -        return ret;
> -    }
> +    int fd;
>  
>      s->get_error = socket_errno;
>      s->write = socket_write;
>      s->close = tcp_close;
>  
> -    s->fd = qemu_socket(PF_INET, SOCK_STREAM, 0);
> -    if (s->fd == -1) {
> -        DPRINTF("Unable to open socket");
> -        return -socket_error();
> -    }
> -
> -    socket_set_nonblock(s->fd);
> -
> -    do {
> -        ret = connect(s->fd, (struct sockaddr *)&addr, sizeof(addr));
> -        if (ret == -1) {
> -            ret = -socket_error();
> -        }
> -        if (ret == -EINPROGRESS || ret == -EWOULDBLOCK) {
> -            qemu_set_fd_handler2(s->fd, NULL, NULL, tcp_wait_for_connect, s);
> -            return 0;
> -        }
> -    } while (ret == -EINTR);
> -
> -    if (ret < 0) {
> +    ret = tcp_client_start(host_port, &fd);
> +    s->fd = fd;

you don't need fd you can pass s->fd to the function.

Orit

> +    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);
> +        if (ret != -EINVAL) {
> +            migrate_fd_error(s);
> +        }
>          return ret;
> +    } else {
> +        migrate_fd_connect(s);
>      }
> -    migrate_fd_connect(s);
> +
>      return 0;
>  }
>  
> diff --git a/net/socket.c b/net/socket.c
> index 5feb3d2..b7cd8ec 100644
> --- a/net/socket.c
> +++ b/net/socket.c
> @@ -434,41 +434,22 @@ 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");
> -        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;
> +    ret = tcp_client_start(host_str, &fd);
> +    if (ret == -EINPROGRESS || ret == -EWOULDBLOCK) {
> +        connected = 0;
>  #ifdef _WIN32
> -            } else if (err == WSAEALREADY || err == WSAEINVAL) {
> -                break;
> +    } else if (ret == -WSAEALREADY || ret == -WSAEINVAL) {
> +        connected = 0;
>  #endif
> -            } else {
> -                perror("connect");
> -                closesocket(fd);
> -                return -1;
> -            }
> -        } else {
> -            connected = 1;
> -            break;
> -        }
> +    } else if (ret < 0) {
> +        return -1;
> +    } else {
> +        connected = 1;
>      }
> +
>      s = net_socket_fd_init(vlan, model, name, fd, connected);
>      if (!s)
>          return -1;
> 
> 

[Qemu-devel] [PATCH v2 5/9] net: refector tcp_*_start functions

[Amos Kong]

There are some repeated code for tcp_server_start()
and tcp_client_start().

Signed-off-by: Amos Kong <akong@redhat.com>
---
 net.c |   82 ++++++++++++++++++++++++++++++++++++-----------------------------
 1 files changed, 46 insertions(+), 36 deletions(-)

diff --git a/net.c b/net.c
index 5c20e22..da2a8d4 100644
--- a/net.c
+++ b/net.c
@@ -99,37 +99,41 @@ static int get_str_sep(char *buf, int buf_size, const char **pp, int sep)
     return 0;
 }
 
-int tcp_server_start(const char *str, int *fd)
+static int tcp_server_bind(int fd, struct sockaddr_in *saddr)
 {
-    int val, ret;
-    struct sockaddr_in saddr;
+    int ret;
+    int val = 1;
 
-    if (parse_host_port(&saddr, str) < 0) {
-        return -1;
-    }
+    /* allow fast reuse */
+    setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, (const char *)&val, sizeof(val));
 
-    *fd = qemu_socket(PF_INET, SOCK_STREAM, 0);
-    if (fd < 0) {
-        perror("socket");
-        return -1;
+    ret = bind(fd, (struct sockaddr *)saddr, sizeof(*saddr));
+
+    if (ret == -1) {
+        ret = -socket_error();
     }
-    socket_set_nonblock(*fd);
+    return ret;
 
-    /* allow fast reuse */
-    val = 1;
-    setsockopt(*fd, SOL_SOCKET, SO_REUSEADDR, (const char *)&val, sizeof(val));
+}
+
+static int tcp_client_connect(int fd, struct sockaddr_in *saddr)
+{
+    int ret;
+
+    do {
+        ret = connect(fd, (struct sockaddr *)saddr, sizeof(*saddr));
+        if (ret == -1) {
+            ret = -socket_error();
+        }
+    } while (ret == -EINTR || ret == -EWOULDBLOCK);
 
-    ret = bind(*fd, (struct sockaddr *)&saddr, sizeof(saddr));
-    if (ret < 0) {
-        closesocket(*fd);
-    }
     return ret;
 }
 
-int tcp_client_start(const char *str, int *fd)
+static int tcp_start_common(const char *str, int *fd, bool server)
 {
+    int ret = -EINVAL;
     struct sockaddr_in saddr;
-    int ret;
 
     if (parse_host_port(&saddr, str) < 0) {
         return -EINVAL;
@@ -142,29 +146,35 @@ int tcp_client_start(const char *str, int *fd)
     }
     socket_set_nonblock(*fd);
 
-    for (;;) {
-        ret = connect(*fd, (struct sockaddr *)&saddr, sizeof(saddr));
-        if (ret < 0) {
-            ret = -socket_error();
-            if (ret == -EINPROGRESS) {
-                break;
+    if (server) {
+        ret = tcp_server_bind(*fd, &saddr);
+    } else {
+        ret = tcp_client_connect(*fd, &saddr);
+    }
+
 #ifdef _WIN32
-            } else if (ret == -WSAEALREADY || ret == -WSAEINVAL) {
-                break;
+    if (ret == -WSAEALREADY || ret == -WSAEINVAL) {
+        return ret;                  /* Success */
+    }
 #endif
-            } else if (ret != -EINTR && ret != -EWOULDBLOCK) {
-                perror("connect");
-                closesocket(*fd);
-                return -1;
-            }
-        } else {
-            break;
-        }
+    if (ret >= 0 || ret == -EINPROGRESS || ret == -EWOULDBLOCK) {
+        return ret;                  /* Success */
     }
 
+    closesocket(*fd);
     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];

[Qemu-devel] [PATCH v2 6/9] net: use getaddrinfo() in tcp_start_common

[Amos Kong]

Migrating with IPv6 address exists problem, gethostbyname()/inet_aton()
could not translate IPv6 address/port simply, so use getaddrinfo()
in tcp_start_common to translate network address and service.
We can get an address list by getaddrinfo().

Userlevel IPv6 Programming Introduction:
http://www.akkadia.org/drepper/userapi-ipv6.html

Reference RFC 3493, Basic Socket Interface Extensions for IPv6

Signed-off-by: Amos Kong <akong@redhat.com>
---
 net.c |   81 ++++++++++++++++++++++++++++++++++++++++++++++++-----------------
 1 files changed, 60 insertions(+), 21 deletions(-)

diff --git a/net.c b/net.c
index da2a8d4..de1db8c 100644
--- a/net.c
+++ b/net.c
@@ -99,7 +99,7 @@ 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 sockaddr_in *saddr)
+static int tcp_server_bind(int fd, struct addrinfo *rp)
 {
     int ret;
     int val = 1;
@@ -107,7 +107,7 @@ static int tcp_server_bind(int fd, struct sockaddr_in *saddr)
     /* allow fast reuse */
     setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, (const char *)&val, sizeof(val));
 
-    ret = bind(fd, (struct sockaddr *)saddr, sizeof(*saddr));
+    ret = bind(fd, rp->ai_addr, rp->ai_addrlen);
 
     if (ret == -1) {
         ret = -socket_error();
@@ -116,12 +116,12 @@ static int tcp_server_bind(int fd, struct sockaddr_in *saddr)
 
 }
 
-static int tcp_client_connect(int fd, struct sockaddr_in *saddr)
+static int tcp_client_connect(int fd, struct addrinfo *rp)
 {
     int ret;
 
     do {
-        ret = connect(fd, (struct sockaddr *)saddr, sizeof(*saddr));
+        ret = connect(fd, rp->ai_addr, rp->ai_addrlen);
         if (ret == -1) {
             ret = -socket_error();
         }
@@ -132,36 +132,75 @@ static int tcp_client_connect(int fd, struct sockaddr_in *saddr)
 
 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;
-    struct sockaddr_in saddr;
 
-    if (parse_host_port(&saddr, str) < 0) {
+    *fd = -1;
+    service = str;
+
+    if (get_str_sep(hostname, sizeof(hostname), &service, ':') < 0) {
+        error_report("invalid host/port combination: %s", str);
         return -EINVAL;
     }
-
-    *fd = qemu_socket(PF_INET, SOCK_STREAM, 0);
-    if (fd < 0) {
-        perror("socket");
-        return -1;
+    if (server && strlen(hostname) == 0) {
+        name = NULL;
+    } else {
+        name = hostname;
     }
-    socket_set_nonblock(*fd);
+
+    /* 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) {
-        ret = tcp_server_bind(*fd, &saddr);
-    } else {
-        ret = tcp_client_connect(*fd, &saddr);
+        hints.ai_flags = AI_PASSIVE;
     }
 
-#ifdef _WIN32
-    if (ret == -WSAEALREADY || ret == -WSAEINVAL) {
-        return ret;                  /* Success */
+    s = getaddrinfo(name, service, &hints, &result);
+    if (s != 0) {
+        error_report("qemu: getaddrinfo: %s", 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);
+        }
+#ifdef _WIN32
+        if (ret == -WSAEALREADY || ret == -WSAEINVAL) {
+            *fd = sfd;
+            break;                  /* Success */
+        }
 #endif
-    if (ret >= 0 || ret == -EINPROGRESS || ret == -EWOULDBLOCK) {
-        return ret;                  /* Success */
+        if (ret >= 0 || ret == -EINPROGRESS || ret == -EWOULDBLOCK) {
+            *fd = sfd;
+            break;                  /* Success */
+        }
+        closesocket(sfd);
     }
 
-    closesocket(*fd);
+    freeaddrinfo(result);
     return ret;
 }
 

[Qemu-devel] [PATCH v2 7/9] net: introduce parse_host_port_info()

[Amos Kong]

int parse_host_port(struct sockaddr_in *saddr, const char *str)
Parsed address info will be restored into 'saddr', it only support ipv4.
This function is used by net_socket_mcast_init() and net_socket_udp_init().

int parse_host_port_info(struct addrinfo *result, const char *str)
Parsed address info will be restored into 'result', it's an address list.
It can be used to parse IPv6 address/port.

Signed-off-by: Amos Kong <akong@redhat.com>
---
 net.c |   26 ++++++++++++++++++++------
 1 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/net.c b/net.c
index de1db8c..2518e5f 100644
--- a/net.c
+++ b/net.c
@@ -130,18 +130,15 @@ static int tcp_client_connect(int fd, struct addrinfo *rp)
     return ret;
 }
 
-static int tcp_start_common(const char *str, int *fd, bool server)
+static int parse_host_port_info(struct addrinfo **result, const char *str,
+                                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) {
@@ -164,12 +161,29 @@ static int tcp_start_common(const char *str, int *fd, bool server)
         hints.ai_flags = AI_PASSIVE;
     }
 
-    s = getaddrinfo(name, service, &hints, &result);
+    s = getaddrinfo(name, service, &hints, result);
     if (s != 0) {
         error_report("qemu: getaddrinfo: %s", gai_strerror(s));
         return -EINVAL;
     }
 
+    return 0;
+}
+
+static int tcp_start_common(const char *str, int *fd, bool server)
+{
+    struct addrinfo *rp;
+    int sfd;
+    int ret = -EINVAL;
+    struct addrinfo *result;
+
+    *fd = -1;
+
+    ret = parse_host_port_info(&result, str, server);
+    if (ret < 0) {
+        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

[Qemu-devel] [PATCH v2 8/9] net: split hostname and service by last colon

[Amos Kong]

IPv6 address contains colons, parse will be wrong.

    [2312::8274]:5200

Signed-off-by: Amos Kong <akong@redhat.com>
---
 net.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net.c b/net.c
index 2518e5f..d6ce1fa 100644
--- a/net.c
+++ b/net.c
@@ -84,7 +84,7 @@ static int get_str_sep(char *buf, int buf_size, const char **pp, int sep)
     const char *p, *p1;
     int len;
     p = *pp;
-    p1 = strchr(p, sep);
+    p1 = strrchr(p, sep);
     if (!p1)
         return -1;
     len = p1 - p;

[Qemu-devel] [PATCH v2 9/9] net: support to include ipv6 address by brackets

[Amos Kong]

That method of representing an IPv6 address with a port is
discouraged because of its ambiguity. Referencing to RFC5952,
the recommended format is:

     [2312::8274]:5200

For IPv6 brackets must be mandatory if you require a port.

test status: Successed
listen side: qemu-kvm .... -incoming tcp:[2312::8274]:5200
client side: qemu-kvm ...
             (qemu) migrate -d tcp:[2312::8274]:5200

Signed-off-by: Amos Kong <akong@redhat.com>
---
 net.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/net.c b/net.c
index d6ce1fa..499ed1d 100644
--- a/net.c
+++ b/net.c
@@ -88,6 +88,12 @@ static int get_str_sep(char *buf, int buf_size, const char **pp, int sep)
     if (!p1)
         return -1;
     len = p1 - p;
+    /* remove brackets which includes hostname */
+    if (*p == '[' && *(p1-1) == ']') {
+        p += 1;
+        len -= 2;
+    }
+
     p1++;
     if (buf_size > 0) {
         if (len > buf_size - 1)