[Qemu-devel] [PATCH v12 0/4] support to migrate with IPv6 address

[Qemu-devel] [PATCH v12 0/4] support to migrate with IPv6 address

[Amos Kong]

Those patches updated help functions in qemu-socket.c,
and used them in migrate-tcp.c to supporting IPv6 migration.

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

Changes from v2:
- fix issue of returning real error 
- set s->fd to -1 when parse fails, won't call migrate_fd_error()

Changes from v3:
- try to use help functions in qemu-socket.c

Changes from v4:
- introduce set_socket_error() to restore real errno
- fix connect error process

Changes from v5:
- use error class to pass socket error

Changes from v6:
- merge error process and nonblock support together
- fix leak of repeatedly error_set()
- coding style fix
- fix EWOULDBLOCK process

Changes from v7:
- posix: let EWOULDBLOCK fall through to CONNECT_FAILED path
- add unknown error process
- fix typo

Changes from v8:
- reuse rc variable
- fix a NULL pointer dereference

Changes from v9:
- handle non-blocking correctly if errp is NULL

Changes from v10:
- send out the whole series, no change
- add 'Reviewed-by'

Changes from v11:
- trivial fix: update error description in qerror.c

---

Amos Kong (4):
      qerror: add five qerror strings
      sockets: change inet_connect() to support nonblock socket
      sockets: use error class to pass listen error
      use inet_listen()/inet_connect() to support ipv6 migration


 migration-tcp.c |   77 +++++++++++++++----------------------------------------
 migration.c     |   14 ++++++----
 migration.h     |    7 +++--
 nbd.c           |    4 +--
 qemu-char.c     |    4 +--
 qemu-sockets.c  |   60 ++++++++++++++++++++++++++++++++++++-------
 qemu_socket.h   |   10 ++++---
 qerror.c        |   20 ++++++++++++++
 qerror.h        |   15 +++++++++++
 ui/vnc.c        |    5 ++--
 vl.c            |    7 ++++-
 11 files changed, 138 insertions(+), 85 deletions(-)

-- 
Amos Kong

[Qemu-devel] [PATCH v12 1/4] qerror: add five qerror strings

[Amos Kong]

Add five new qerror strings, they are about listen/connect socket:
  QERR_SOCKET_CONNECT_IN_PROGRESS
  QERR_SOCKET_CONNECT_FAILED
  QERR_SOCKET_LISTEN_FAILED
  QERR_SOCKET_BIND_FAILED
  QERR_SOCKET_CREATE_FAILED

Signed-off-by: Amos Kong <akong@redhat.com>
Reviewed-by: Orit Wasserman <owasserm@redhat.com>
---
 qerror.c |   20 ++++++++++++++++++++
 qerror.h |   15 +++++++++++++++
 2 files changed, 35 insertions(+), 0 deletions(-)

diff --git a/qerror.c b/qerror.c
index 96fbe71..5092fe7 100644
--- a/qerror.c
+++ b/qerror.c
@@ -304,6 +304,26 @@ static const QErrorStringTable qerror_table[] = {
         .error_fmt = QERR_VNC_SERVER_FAILED,
         .desc      = "Could not start VNC server on %(target)",
     },
+    {
+        .error_fmt = QERR_SOCKET_CONNECT_IN_PROGRESS,
+        .desc      = "Connection can not be completed immediately",
+    },
+    {
+        .error_fmt = QERR_SOCKET_CONNECT_FAILED,
+        .desc      = "Failed to connect to socket",
+    },
+    {
+        .error_fmt = QERR_SOCKET_LISTEN_FAILED,
+        .desc      = "Failed to set socket to listening mode",
+    },
+    {
+        .error_fmt = QERR_SOCKET_BIND_FAILED,
+        .desc      = "Failed to bind socket",
+    },
+    {
+        .error_fmt = QERR_SOCKET_CREATE_FAILED,
+        .desc      = "Failed to create socket",
+    },
     {}
 };
 
diff --git a/qerror.h b/qerror.h
index 5c23c1f..4cbba48 100644
--- a/qerror.h
+++ b/qerror.h
@@ -248,4 +248,19 @@ QError *qobject_to_qerror(const QObject *obj);
 #define QERR_VNC_SERVER_FAILED \
     "{ 'class': 'VNCServerFailed', 'data': { 'target': %s } }"
 
+#define QERR_SOCKET_CONNECT_IN_PROGRESS \
+    "{ 'class': 'SockConnectInprogress', 'data': {} }"
+
+#define QERR_SOCKET_CONNECT_FAILED \
+    "{ 'class': 'SockConnectFailed', 'data': {} }"
+
+#define QERR_SOCKET_LISTEN_FAILED \
+    "{ 'class': 'SockListenFailed', 'data': {} }"
+
+#define QERR_SOCKET_BIND_FAILED \
+    "{ 'class': 'SockBindFailed', 'data': {} }"
+
+#define QERR_SOCKET_CREATE_FAILED \
+    "{ 'class': 'SockCreateFailed', 'data': {} }"
+
 #endif /* QERROR_H */

[Qemu-devel] [PATCH v12 2/4] sockets: change inet_connect() to support nonblock socket

[Amos Kong]

Add a bool argument to inet_connect() to assign if set socket
to block/nonblock, and delete original argument 'socktype'
that is unused.
Add a new argument to inet_connect()/inet_connect_opts(),
to pass back connect error by error class.

Retry to connect when -EINTR is got. Connect's successful
for nonblock socket when following errors are got, user
should wait for connecting by select():
  -EINPROGRESS
  -EWOULDBLOCK (win32)
  -WSAEALREADY (win32)

Change nbd, vnc to use new interface.

Signed-off-by: Amos Kong <akong@redhat.com>
Reviewed-by: Orit Wasserman <owasserm@redhat.com>
Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 nbd.c          |    2 +-
 qemu-char.c    |    2 +-
 qemu-sockets.c |   43 +++++++++++++++++++++++++++++++++++++------
 qemu_socket.h  |    6 ++++--
 ui/vnc.c       |    2 +-
 5 files changed, 44 insertions(+), 11 deletions(-)

diff --git a/nbd.c b/nbd.c
index 153709f..bebfc49 100644
--- a/nbd.c
+++ b/nbd.c
@@ -162,7 +162,7 @@ int tcp_socket_outgoing(const char *address, uint16_t port)
 
 int tcp_socket_outgoing_spec(const char *address_and_port)
 {
-    return inet_connect(address_and_port, SOCK_STREAM);
+    return inet_connect(address_and_port, true, NULL);
 }
 
 int tcp_socket_incoming(const char *address, uint16_t port)
diff --git a/qemu-char.c b/qemu-char.c
index a9fc504..060bf9d 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2446,7 +2446,7 @@ static CharDriverState *qemu_chr_open_socket(QemuOpts *opts)
         if (is_listen) {
             fd = inet_listen_opts(opts, 0);
         } else {
-            fd = inet_connect_opts(opts);
+            fd = inet_connect_opts(opts, NULL);
         }
     }
     if (fd < 0) {
diff --git a/qemu-sockets.c b/qemu-sockets.c
index 6bcb8e3..ce3f06c 100644
--- a/qemu-sockets.c
+++ b/qemu-sockets.c
@@ -51,6 +51,9 @@ static QemuOptsList dummy_opts = {
         },{
             .name = "ipv6",
             .type = QEMU_OPT_BOOL,
+        },{
+            .name = "block",
+            .type = QEMU_OPT_BOOL,
         },
         { /* end if list */ }
     },
@@ -194,7 +197,7 @@ listen:
     return slisten;
 }
 
-int inet_connect_opts(QemuOpts *opts)
+int inet_connect_opts(QemuOpts *opts, Error **errp)
 {
     struct addrinfo ai,*res,*e;
     const char *addr;
@@ -202,6 +205,7 @@ int inet_connect_opts(QemuOpts *opts)
     char uaddr[INET6_ADDRSTRLEN+1];
     char uport[33];
     int sock,rc;
+    bool block;
 
     memset(&ai,0, sizeof(ai));
     ai.ai_flags = AI_CANONNAME | AI_ADDRCONFIG;
@@ -210,8 +214,10 @@ int inet_connect_opts(QemuOpts *opts)
 
     addr = qemu_opt_get(opts, "host");
     port = qemu_opt_get(opts, "port");
+    block = qemu_opt_get_bool(opts, "block", 0);
     if (addr == NULL || port == NULL) {
         fprintf(stderr, "inet_connect: host and/or port not specified\n");
+        error_set(errp, QERR_SOCKET_CREATE_FAILED);
         return -1;
     }
 
@@ -224,6 +230,7 @@ int inet_connect_opts(QemuOpts *opts)
     if (0 != (rc = getaddrinfo(addr, port, &ai, &res))) {
         fprintf(stderr,"getaddrinfo(%s,%s): %s\n", addr, port,
                 gai_strerror(rc));
+        error_set(errp, QERR_SOCKET_CREATE_FAILED);
 	return -1;
     }
 
@@ -241,19 +248,37 @@ int inet_connect_opts(QemuOpts *opts)
             continue;
         }
         setsockopt(sock,SOL_SOCKET,SO_REUSEADDR,(void*)&on,sizeof(on));
-
+        if (!block) {
+            socket_set_nonblock(sock);
+        }
         /* connect to peer */
-        if (connect(sock,e->ai_addr,e->ai_addrlen) < 0) {
+        do {
+            rc = 0;
+            if (connect(sock, e->ai_addr, e->ai_addrlen) < 0) {
+                rc = -socket_error();
+            }
+        } while (rc == -EINTR);
+
+  #ifdef _WIN32
+        if (!block && (rc == -EINPROGRESS || rc == -EWOULDBLOCK
+                       || rc == -WSAEALREADY)) {
+  #else
+        if (!block && (rc == -EINPROGRESS)) {
+  #endif
+            error_set(errp, QERR_SOCKET_CONNECT_IN_PROGRESS);
+        } else if (rc < 0) {
             if (NULL == e->ai_next)
                 fprintf(stderr, "%s: connect(%s,%s,%s,%s): %s\n", __FUNCTION__,
                         inet_strfamily(e->ai_family),
                         e->ai_canonname, uaddr, uport, strerror(errno));
             closesocket(sock);
+            sock = -1;
             continue;
         }
         freeaddrinfo(res);
         return sock;
     }
+    error_set(errp, QERR_SOCKET_CONNECT_FAILED);
     freeaddrinfo(res);
     return -1;
 }
@@ -449,14 +474,20 @@ int inet_listen(const char *str, char *ostr, int olen,
     return sock;
 }
 
-int inet_connect(const char *str, int socktype)
+int inet_connect(const char *str, bool block, Error **errp)
 {
     QemuOpts *opts;
     int sock = -1;
 
     opts = qemu_opts_create(&dummy_opts, NULL, 0);
-    if (inet_parse(opts, str) == 0)
-        sock = inet_connect_opts(opts);
+    if (inet_parse(opts, str) == 0) {
+        if (block) {
+            qemu_opt_set(opts, "block", "on");
+        }
+        sock = inet_connect_opts(opts, errp);
+    } else {
+        error_set(errp, QERR_SOCKET_CREATE_FAILED);
+    }
     qemu_opts_del(opts);
     return sock;
 }
diff --git a/qemu_socket.h b/qemu_socket.h
index a5d0a84..26998ef 100644
--- a/qemu_socket.h
+++ b/qemu_socket.h
@@ -27,6 +27,8 @@ int inet_aton(const char *cp, struct in_addr *ia);
 #endif /* !_WIN32 */
 
 #include "qemu-option.h"
+#include "error.h"
+#include "qerror.h"
 
 /* misc helpers */
 int qemu_socket(int domain, int type, int protocol);
@@ -40,8 +42,8 @@ int send_all(int fd, const void *buf, int len1);
 int inet_listen_opts(QemuOpts *opts, int port_offset);
 int inet_listen(const char *str, char *ostr, int olen,
                 int socktype, int port_offset);
-int inet_connect_opts(QemuOpts *opts);
-int inet_connect(const char *str, int socktype);
+int inet_connect_opts(QemuOpts *opts, Error **errp);
+int inet_connect(const char *str, bool block, Error **errp);
 int inet_dgram_opts(QemuOpts *opts);
 const char *inet_strfamily(int family);
 
diff --git a/ui/vnc.c b/ui/vnc.c
index deb9ecd..3ae7704 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -3068,7 +3068,7 @@ int vnc_display_open(DisplayState *ds, const char *display)
         if (strncmp(display, "unix:", 5) == 0)
             vs->lsock = unix_connect(display+5);
         else
-            vs->lsock = inet_connect(display, SOCK_STREAM);
+            vs->lsock = inet_connect(display, true, NULL);
         if (-1 == vs->lsock) {
             g_free(vs->display);
             vs->display = NULL;

[Qemu-devel] [PATCH v12 3/4] sockets: use error class to pass listen error

[Amos Kong]

Add a new argument in inet_listen()/inet_listen_opts()
to pass back listen error.

Change nbd, qemu-char, vnc to use new interface.

Signed-off-by: Amos Kong <akong@redhat.com>
Reviewed-by: Orit Wasserman <owasserm@redhat.com>
Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 nbd.c          |    2 +-
 qemu-char.c    |    2 +-
 qemu-sockets.c |   17 ++++++++++++++---
 qemu_socket.h  |    4 ++--
 ui/vnc.c       |    3 ++-
 5 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/nbd.c b/nbd.c
index bebfc49..dc0adf9 100644
--- a/nbd.c
+++ b/nbd.c
@@ -176,7 +176,7 @@ int tcp_socket_incoming_spec(const char *address_and_port)
 {
     char *ostr  = NULL;
     int olen = 0;
-    return inet_listen(address_and_port, ostr, olen, SOCK_STREAM, 0);
+    return inet_listen(address_and_port, ostr, olen, SOCK_STREAM, 0, NULL);
 }
 
 int unix_socket_incoming(const char *path)
diff --git a/qemu-char.c b/qemu-char.c
index 060bf9d..fe1126f 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2444,7 +2444,7 @@ static CharDriverState *qemu_chr_open_socket(QemuOpts *opts)
         }
     } else {
         if (is_listen) {
-            fd = inet_listen_opts(opts, 0);
+            fd = inet_listen_opts(opts, 0, NULL);
         } else {
             fd = inet_connect_opts(opts, NULL);
         }
diff --git a/qemu-sockets.c b/qemu-sockets.c
index ce3f06c..46c7619 100644
--- a/qemu-sockets.c
+++ b/qemu-sockets.c
@@ -103,7 +103,7 @@ const char *inet_strfamily(int family)
     return "unknown";
 }
 
-int inet_listen_opts(QemuOpts *opts, int port_offset)
+int inet_listen_opts(QemuOpts *opts, int port_offset, Error **errp)
 {
     struct addrinfo ai,*res,*e;
     const char *addr;
@@ -120,6 +120,7 @@ int inet_listen_opts(QemuOpts *opts, int port_offset)
     if ((qemu_opt_get(opts, "host") == NULL) ||
         (qemu_opt_get(opts, "port") == NULL)) {
         fprintf(stderr, "%s: host and/or port not specified\n", __FUNCTION__);
+        error_set(errp, QERR_SOCKET_CREATE_FAILED);
         return -1;
     }
     pstrcpy(port, sizeof(port), qemu_opt_get(opts, "port"));
@@ -138,6 +139,7 @@ int inet_listen_opts(QemuOpts *opts, int port_offset)
     if (rc != 0) {
         fprintf(stderr,"getaddrinfo(%s,%s): %s\n", addr, port,
                 gai_strerror(rc));
+        error_set(errp, QERR_SOCKET_CREATE_FAILED);
         return -1;
     }
 
@@ -150,6 +152,9 @@ int inet_listen_opts(QemuOpts *opts, int port_offset)
         if (slisten < 0) {
             fprintf(stderr,"%s: socket(%s): %s\n", __FUNCTION__,
                     inet_strfamily(e->ai_family), strerror(errno));
+            if (!e->ai_next) {
+                error_set(errp, QERR_SOCKET_CREATE_FAILED);
+            }
             continue;
         }
 
@@ -173,6 +178,9 @@ int inet_listen_opts(QemuOpts *opts, int port_offset)
                 fprintf(stderr,"%s: bind(%s,%s,%d): %s\n", __FUNCTION__,
                         inet_strfamily(e->ai_family), uaddr, inet_getport(e),
                         strerror(errno));
+                if (!e->ai_next) {
+                    error_set(errp, QERR_SOCKET_BIND_FAILED);
+                }
             }
         }
         closesocket(slisten);
@@ -183,6 +191,7 @@ int inet_listen_opts(QemuOpts *opts, int port_offset)
 
 listen:
     if (listen(slisten,1) != 0) {
+        error_set(errp, QERR_SOCKET_LISTEN_FAILED);
         perror("listen");
         closesocket(slisten);
         freeaddrinfo(res);
@@ -446,7 +455,7 @@ static int inet_parse(QemuOpts *opts, const char *str)
 }
 
 int inet_listen(const char *str, char *ostr, int olen,
-                int socktype, int port_offset)
+                int socktype, int port_offset, Error **errp)
 {
     QemuOpts *opts;
     char *optstr;
@@ -454,7 +463,7 @@ int inet_listen(const char *str, char *ostr, int olen,
 
     opts = qemu_opts_create(&dummy_opts, NULL, 0);
     if (inet_parse(opts, str) == 0) {
-        sock = inet_listen_opts(opts, port_offset);
+        sock = inet_listen_opts(opts, port_offset, errp);
         if (sock != -1 && ostr) {
             optstr = strchr(str, ',');
             if (qemu_opt_get_bool(opts, "ipv6", 0)) {
@@ -469,6 +478,8 @@ int inet_listen(const char *str, char *ostr, int olen,
                          optstr ? optstr : "");
             }
         }
+    } else {
+        error_set(errp, QERR_SOCKET_CREATE_FAILED);
     }
     qemu_opts_del(opts);
     return sock;
diff --git a/qemu_socket.h b/qemu_socket.h
index 26998ef..4689ff3 100644
--- a/qemu_socket.h
+++ b/qemu_socket.h
@@ -39,9 +39,9 @@ void socket_set_nonblock(int fd);
 int send_all(int fd, const void *buf, int len1);
 
 /* New, ipv6-ready socket helper functions, see qemu-sockets.c */
-int inet_listen_opts(QemuOpts *opts, int port_offset);
+int inet_listen_opts(QemuOpts *opts, int port_offset, Error **errp);
 int inet_listen(const char *str, char *ostr, int olen,
-                int socktype, int port_offset);
+                int socktype, int port_offset, Error **errp);
 int inet_connect_opts(QemuOpts *opts, Error **errp);
 int inet_connect(const char *str, bool block, Error **errp);
 int inet_dgram_opts(QemuOpts *opts);
diff --git a/ui/vnc.c b/ui/vnc.c
index 3ae7704..be384a5 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -3088,7 +3088,8 @@ int vnc_display_open(DisplayState *ds, const char *display)
             pstrcpy(dpy, 256, "unix:");
             vs->lsock = unix_listen(display+5, dpy+5, 256-5);
         } else {
-            vs->lsock = inet_listen(display, dpy, 256, SOCK_STREAM, 5900);
+            vs->lsock = inet_listen(display, dpy, 256,
+                                    SOCK_STREAM, 5900, NULL);
         }
         if (-1 == vs->lsock) {
             g_free(dpy);

[Qemu-devel] [PATCH v12 4/4] use inet_listen()/inet_connect() to support ipv6 migration

[Amos Kong]

Use help functions in qemu-socket.c for tcp migration,
which already support ipv6 addresses.

Currently errp will be set to UNDEFINED_ERROR when migration fails,
qemu would output "migration failed: ...", and current user can
see a message("An undefined error has occurred") in monitor.

This patch changed tcp_start_outgoing_migration()/inet_connect()
/inet_connect_opts(), socket error would be passed back,
then current user can see a meaningful err message in monitor.

Qemu will exit if listening fails, so output socket error
to qemu stderr.

For IPv6 brackets must be mandatory if you require a port.
Referencing to RFC5952, the recommended format is:
  [2312::8274]:5200

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>
Reviewed-by: Orit Wasserman <owasserm@redhat.com>
Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 migration-tcp.c |   77 +++++++++++++++----------------------------------------
 migration.c     |   14 ++++++----
 migration.h     |    7 +++--
 vl.c            |    7 ++++-
 4 files changed, 39 insertions(+), 66 deletions(-)

diff --git a/migration-tcp.c b/migration-tcp.c
index 35a5781..440804d 100644
--- a/migration-tcp.c
+++ b/migration-tcp.c
@@ -79,45 +79,32 @@ static void tcp_wait_for_connect(void *opaque)
     }
 }
 
-int tcp_start_outgoing_migration(MigrationState *s, const char *host_port)
+int tcp_start_outgoing_migration(MigrationState *s, const char *host_port,
+                                 Error **errp)
 {
-    struct sockaddr_in addr;
-    int ret;
-
-    ret = parse_host_port(&addr, host_port);
-    if (ret < 0) {
-        return ret;
-    }
-
     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);
+    s->fd = inet_connect(host_port, false, errp);
 
-    if (ret < 0) {
+    if (!error_is_set(errp)) {
+        migrate_fd_connect(s);
+    } else if (error_is_type(*errp, QERR_SOCKET_CONNECT_IN_PROGRESS)) {
+        DPRINTF("connect in progress\n");
+        qemu_set_fd_handler2(s->fd, NULL, NULL, tcp_wait_for_connect, s);
+    } else if (error_is_type(*errp, QERR_SOCKET_CREATE_FAILED)) {
+        DPRINTF("connect failed\n");
+        return -1;
+    } else if (error_is_type(*errp, QERR_SOCKET_CONNECT_FAILED)) {
         DPRINTF("connect failed\n");
         migrate_fd_error(s);
-        return ret;
+        return -1;
+    } else {
+        DPRINTF("unknown error\n");
+        return -1;
     }
-    migrate_fd_connect(s);
+
     return 0;
 }
 
@@ -155,40 +142,18 @@ out2:
     close(s);
 }
 
-int tcp_start_incoming_migration(const char *host_port)
+int tcp_start_incoming_migration(const char *host_port, Error **errp)
 {
-    struct sockaddr_in addr;
-    int val;
     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();
-    }
-
-    val = 1;
-    setsockopt(s, SOL_SOCKET, SO_REUSEADDR, (const char *)&val, sizeof(val));
+    s = inet_listen(host_port, NULL, 256, SOCK_STREAM, 0, errp);
 
-    if (bind(s, (struct sockaddr *)&addr, sizeof(addr)) == -1) {
-        goto err;
-    }
-    if (listen(s, 1) == -1) {
-        goto err;
+    if (s < 0) {
+        return -1;
     }
 
     qemu_set_fd_handler2(s, NULL, tcp_accept_incoming_migration, NULL,
                          (void *)(intptr_t)s);
 
     return 0;
-
-err:
-    close(s);
-    return -socket_error();
 }
diff --git a/migration.c b/migration.c
index f9e968e..3f485d3 100644
--- a/migration.c
+++ b/migration.c
@@ -60,13 +60,13 @@ static MigrationState *migrate_get_current(void)
     return &current_migration;
 }
 
-int qemu_start_incoming_migration(const char *uri)
+int qemu_start_incoming_migration(const char *uri, Error **errp)
 {
     const char *p;
     int ret;
 
     if (strstart(uri, "tcp:", &p))
-        ret = tcp_start_incoming_migration(p);
+        ret = tcp_start_incoming_migration(p, errp);
 #if !defined(WIN32)
     else if (strstart(uri, "exec:", &p))
         ret =  exec_start_incoming_migration(p);
@@ -414,7 +414,7 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
     s = migrate_init(blk, inc);
 
     if (strstart(uri, "tcp:", &p)) {
-        ret = tcp_start_outgoing_migration(s, p);
+        ret = tcp_start_outgoing_migration(s, p, errp);
 #if !defined(WIN32)
     } else if (strstart(uri, "exec:", &p)) {
         ret = exec_start_outgoing_migration(s, p);
@@ -429,9 +429,11 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
     }
 
     if (ret < 0) {
-        DPRINTF("migration failed: %s\n", strerror(-ret));
-        /* FIXME: we should return meaningful errors */
-        error_set(errp, QERR_UNDEFINED_ERROR);
+        if (!error_is_set(errp)) {
+            DPRINTF("migration failed: %s\n", strerror(-ret));
+            /* FIXME: we should return meaningful errors */
+            error_set(errp, QERR_UNDEFINED_ERROR);
+        }
         return;
     }
 
diff --git a/migration.h b/migration.h
index 691b367..2e9ca2e 100644
--- a/migration.h
+++ b/migration.h
@@ -37,7 +37,7 @@ struct MigrationState
 
 void process_incoming_migration(QEMUFile *f);
 
-int qemu_start_incoming_migration(const char *uri);
+int qemu_start_incoming_migration(const char *uri, Error **errp);
 
 uint64_t migrate_max_downtime(void);
 
@@ -49,9 +49,10 @@ int exec_start_incoming_migration(const char *host_port);
 
 int exec_start_outgoing_migration(MigrationState *s, const char *host_port);
 
-int tcp_start_incoming_migration(const char *host_port);
+int tcp_start_incoming_migration(const char *host_port, Error **errp);
 
-int tcp_start_outgoing_migration(MigrationState *s, const char *host_port);
+int tcp_start_outgoing_migration(MigrationState *s, const char *host_port,
+                                 Error **errp);
 
 int unix_start_incoming_migration(const char *path);
 
diff --git a/vl.c b/vl.c
index 5e0080b..4ab690c 100644
--- a/vl.c
+++ b/vl.c
@@ -3631,8 +3631,13 @@ int main(int argc, char **argv, char **envp)
     }
 
     if (incoming) {
-        int ret = qemu_start_incoming_migration(incoming);
+        Error *errp = NULL;
+        int ret = qemu_start_incoming_migration(incoming, &errp);
         if (ret < 0) {
+            if (error_is_set(&errp)) {
+                fprintf(stderr, "Migrate: %s\n", error_get_pretty(errp));
+                error_free(errp);
+            }
             fprintf(stderr, "Migration failed. Exit code %s(%d), exiting.\n",
                     incoming, ret);
             exit(ret);

Re: [Qemu-devel] [PATCH v12 1/4] qerror: add five qerror strings

[Michael Roth]

On Fri, May 11, 2012 at 12:28:08AM +0800, Amos Kong wrote:
> Add five new qerror strings, they are about listen/connect socket:
>   QERR_SOCKET_CONNECT_IN_PROGRESS
>   QERR_SOCKET_CONNECT_FAILED
>   QERR_SOCKET_LISTEN_FAILED
>   QERR_SOCKET_BIND_FAILED
>   QERR_SOCKET_CREATE_FAILED
> 
> Signed-off-by: Amos Kong <akong@redhat.com>
> Reviewed-by: Orit Wasserman <owasserm@redhat.com>

Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>

> ---
>  qerror.c |   20 ++++++++++++++++++++
>  qerror.h |   15 +++++++++++++++
>  2 files changed, 35 insertions(+), 0 deletions(-)
> 
> diff --git a/qerror.c b/qerror.c
> index 96fbe71..5092fe7 100644
> --- a/qerror.c
> +++ b/qerror.c
> @@ -304,6 +304,26 @@ static const QErrorStringTable qerror_table[] = {
>          .error_fmt = QERR_VNC_SERVER_FAILED,
>          .desc      = "Could not start VNC server on %(target)",
>      },
> +    {
> +        .error_fmt = QERR_SOCKET_CONNECT_IN_PROGRESS,
> +        .desc      = "Connection can not be completed immediately",
> +    },
> +    {
> +        .error_fmt = QERR_SOCKET_CONNECT_FAILED,
> +        .desc      = "Failed to connect to socket",
> +    },
> +    {
> +        .error_fmt = QERR_SOCKET_LISTEN_FAILED,
> +        .desc      = "Failed to set socket to listening mode",
> +    },
> +    {
> +        .error_fmt = QERR_SOCKET_BIND_FAILED,
> +        .desc      = "Failed to bind socket",
> +    },
> +    {
> +        .error_fmt = QERR_SOCKET_CREATE_FAILED,
> +        .desc      = "Failed to create socket",
> +    },
>      {}
>  };
> 
> diff --git a/qerror.h b/qerror.h
> index 5c23c1f..4cbba48 100644
> --- a/qerror.h
> +++ b/qerror.h
> @@ -248,4 +248,19 @@ QError *qobject_to_qerror(const QObject *obj);
>  #define QERR_VNC_SERVER_FAILED \
>      "{ 'class': 'VNCServerFailed', 'data': { 'target': %s } }"
> 
> +#define QERR_SOCKET_CONNECT_IN_PROGRESS \
> +    "{ 'class': 'SockConnectInprogress', 'data': {} }"
> +
> +#define QERR_SOCKET_CONNECT_FAILED \
> +    "{ 'class': 'SockConnectFailed', 'data': {} }"
> +
> +#define QERR_SOCKET_LISTEN_FAILED \
> +    "{ 'class': 'SockListenFailed', 'data': {} }"
> +
> +#define QERR_SOCKET_BIND_FAILED \
> +    "{ 'class': 'SockBindFailed', 'data': {} }"
> +
> +#define QERR_SOCKET_CREATE_FAILED \
> +    "{ 'class': 'SockCreateFailed', 'data': {} }"
> +
>  #endif /* QERROR_H */
> 

Re: [Qemu-devel] [PATCH v12 0/4] support to migrate with IPv6 address

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 818 bytes --]

On 05/10/2012 10:27 AM, Amos Kong wrote:
> Those patches updated help functions in qemu-socket.c,
> and used them in migrate-tcp.c to supporting IPv6 migration.
> 

> ---
> 
> Amos Kong (4):
>       qerror: add five qerror strings
>       sockets: change inet_connect() to support nonblock socket
>       sockets: use error class to pass listen error
>       use inet_listen()/inet_connect() to support ipv6 migration

I'm trying to understand the scope of this series, and how it might
impact what libvirt needs to support for migration in an IPv6
environment.  Is the point of this patch that IPv6 migration was
previously not possible, and is now possible using [IP6addr]:port notation?

-- 
Eric Blake   eblake@redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 620 bytes --]

Re: [Qemu-devel] [PATCH v12 0/4] support to migrate with IPv6 address

[Michael Roth]

On Thu, May 10, 2012 at 11:29:48AM -0600, Eric Blake wrote:
> On 05/10/2012 10:27 AM, Amos Kong wrote:
> > Those patches updated help functions in qemu-socket.c,
> > and used them in migrate-tcp.c to supporting IPv6 migration.
> > 
> 
> > ---
> > 
> > Amos Kong (4):
> >       qerror: add five qerror strings
> >       sockets: change inet_connect() to support nonblock socket
> >       sockets: use error class to pass listen error
> >       use inet_listen()/inet_connect() to support ipv6 migration
> 
> I'm trying to understand the scope of this series, and how it might
> impact what libvirt needs to support for migration in an IPv6
> environment.  Is the point of this patch that IPv6 migration was
> previously not possible, and is now possible using [IP6addr]:port notation?

Looks like Amos went offline: but yes, in a nutshell.

addr parsing now relies on qemu-sockets.c:inet_parse(), which has supported
[ip6addr]:port for a while, as opposed to net.c:parse_host_port(), which
didn't.

Additional Error-handling was added to
qemu-sockets.c:inet_connect()/inet_listen(), along with non-blocking support
for inet_connect(), to subsume the ad-hoc client/server setup in
migration-tcp.c

> 
> -- 
> Eric Blake   eblake@redhat.com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
> 

Re: [Qemu-devel] [PATCH v12 0/4] support to migrate with IPv6 address

[Amos Kong]

On 05/11/2012 02:12 AM, Michael Roth wrote:
> On Thu, May 10, 2012 at 11:29:48AM -0600, Eric Blake wrote:
>> On 05/10/2012 10:27 AM, Amos Kong wrote:
>>> Those patches updated help functions in qemu-socket.c,
>>> and used them in migrate-tcp.c to supporting IPv6 migration.
>>>
>>
>>> ---
>>>
>>> Amos Kong (4):
>>>       qerror: add five qerror strings
>>>       sockets: change inet_connect() to support nonblock socket
>>>       sockets: use error class to pass listen error
>>>       use inet_listen()/inet_connect() to support ipv6 migration
>>
>> I'm trying to understand the scope of this series, and how it might
>> impact what libvirt needs to support for migration in an IPv6
>> environment.  Is the point of this patch that IPv6 migration was
>> previously not possible, and is now possible using [IP6addr]:port notation?
> 
> Looks like Amos went offline: but yes, in a nutshell.
> 
> addr parsing now relies on qemu-sockets.c:inet_parse(), which has supported
> [ip6addr]:port for a while, as opposed to net.c:parse_host_port(), which
> didn't.

yeah.

I didn't change qemu monitor cmd interface in this patchset,
and the transport of data is done by qemu, not libvirt.

I guess libvirt only needs to update addr string parse,
for example:

---- GOOD
start a VM:
# qemu-kvm --enable-kvm -boot n -incoming tcp:ipv6alias:16514 -vnc :1
-monitor stdio -name qemu-vm1

try to migrate vm by virsh with addr alias
# virsh migrate libivrt-vm2 tcp:ipv6alias
(connection can establish)

--- FAIL
start a VM:
# qemu-kvm-apply-my-patches --enable-kvm -boot n -incoming
tcp:[2002::3:4]:16514 -vnc :1 -monitor stdio -name qemu-vm1

try to migrate vm by virsh with ipv6 addr:
# virsh migrate libvirt-vm2 tcp:[2002::3:4]
error: invalid argument: could not parse connection URI tcp:[2002::3:4]


Thanks, Amos.

> Additional Error-handling was added to
> qemu-sockets.c:inet_connect()/inet_listen(), along with non-blocking support
> for inet_connect(), to subsume the ad-hoc client/server setup in
> migration-tcp.c


-- 
			Amos.

Re: [Qemu-devel] [PATCH v12 0/4] support to migrate with IPv6 address

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1530 bytes --]

[adding libvirt]

On 05/11/2012 12:46 AM, Amos Kong wrote:
> On 05/11/2012 02:12 AM, Michael Roth wrote:
>> On Thu, May 10, 2012 at 11:29:48AM -0600, Eric Blake wrote:
>>> On 05/10/2012 10:27 AM, Amos Kong wrote:
>>>> Those patches updated help functions in qemu-socket.c,
>>>> and used them in migrate-tcp.c to supporting IPv6 migration.

>> addr parsing now relies on qemu-sockets.c:inet_parse(), which has supported
>> [ip6addr]:port for a while, as opposed to net.c:parse_host_port(), which
>> didn't.
> 
> yeah.
> 
> I didn't change qemu monitor cmd interface in this patchset,
> and the transport of data is done by qemu, not libvirt.
> 
> I guess libvirt only needs to update addr string parse,
> for example:
> 
> ---- GOOD
> start a VM:
> # qemu-kvm --enable-kvm -boot n -incoming tcp:ipv6alias:16514 -vnc :1
> -monitor stdio -name qemu-vm1
> 
> try to migrate vm by virsh with addr alias
> # virsh migrate libivrt-vm2 tcp:ipv6alias
> (connection can establish)
> 
> --- FAIL
> start a VM:
> # qemu-kvm-apply-my-patches --enable-kvm -boot n -incoming
> tcp:[2002::3:4]:16514 -vnc :1 -monitor stdio -name qemu-vm1
> 
> try to migrate vm by virsh with ipv6 addr:
> # virsh migrate libvirt-vm2 tcp:[2002::3:4]
> error: invalid argument: could not parse connection URI tcp:[2002::3:4]

Thanks for researching that.  Looks like it should be fixed in libvirt
to match, then.

-- 
Eric Blake   eblake@redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 620 bytes --]

Re: [Qemu-devel] [PATCH v12 0/4] support to migrate with IPv6 address

[Anthony Liguori]

On 05/10/2012 11:27 AM, Amos Kong wrote:
> Those patches updated help functions in qemu-socket.c,
> and used them in migrate-tcp.c to supporting IPv6 migration.

Applied all.  Thanks.

I tended to agree with Paolo that lack of IPv6 support is a bug at this point...

Regards,

Anthony Liguori

>
> ---
> Changes from v1:
> - split different changes to small patches, it will be easier to review
> - fixed some problem according to Kevin's comment
>
> Changes from v2:
> - fix issue of returning real error
> - set s->fd to -1 when parse fails, won't call migrate_fd_error()
>
> Changes from v3:
> - try to use help functions in qemu-socket.c
>
> Changes from v4:
> - introduce set_socket_error() to restore real errno
> - fix connect error process
>
> Changes from v5:
> - use error class to pass socket error
>
> Changes from v6:
> - merge error process and nonblock support together
> - fix leak of repeatedly error_set()
> - coding style fix
> - fix EWOULDBLOCK process
>
> Changes from v7:
> - posix: let EWOULDBLOCK fall through to CONNECT_FAILED path
> - add unknown error process
> - fix typo
>
> Changes from v8:
> - reuse rc variable
> - fix a NULL pointer dereference
>
> Changes from v9:
> - handle non-blocking correctly if errp is NULL
>
> Changes from v10:
> - send out the whole series, no change
> - add 'Reviewed-by'
>
> Changes from v11:
> - trivial fix: update error description in qerror.c
>
> ---
>
> Amos Kong (4):
>        qerror: add five qerror strings
>        sockets: change inet_connect() to support nonblock socket
>        sockets: use error class to pass listen error
>        use inet_listen()/inet_connect() to support ipv6 migration
>
>
>   migration-tcp.c |   77 +++++++++++++++----------------------------------------
>   migration.c     |   14 ++++++----
>   migration.h     |    7 +++--
>   nbd.c           |    4 +--
>   qemu-char.c     |    4 +--
>   qemu-sockets.c  |   60 ++++++++++++++++++++++++++++++++++++-------
>   qemu_socket.h   |   10 ++++---
>   qerror.c        |   20 ++++++++++++++
>   qerror.h        |   15 +++++++++++
>   ui/vnc.c        |    5 ++--
>   vl.c            |    7 ++++-
>   11 files changed, 138 insertions(+), 85 deletions(-)
>