[Qemu-devel] [PULL 00/29] qemu-sockets error propagation + NBD server

[Qemu-devel] [PULL 00/29] qemu-sockets error propagation + NBD server

[Paolo Bonzini]

Anthony,

The following changes since commit b6348f29d033d5a8a26f633d2ee94362595f32a4:

  target-arm/translate: Fix RRX operands (2012-10-17 19:56:46 +0200)

are available in the git repository at:

  git://github.com/bonzini/qemu.git nbd-next

for you to fetch changes up to 1f493f2c7ee75691c4ad22a7663f908924f464c5:

  qmp: add NBD server commands (2012-10-19 15:29:12 +0200)

I and Luiz agreed that I would take care of sending it to you.  It's been
through his and Markus's review, and in particularly Markus suggested
various improvements to the commit message and further splitting of
some commits; the only remaining sore point was the HMP interface so I
dropped it for now.

Paolo

----------------------------------------------------------------
Paolo Bonzini (29):
      error: add error_set_errno and error_setg_errno
      qemu-sockets: add Error ** to all functions
      qemu-sockets: unix_listen and unix_connect are portable
      qemu-sockets: add nonblocking connect for Unix sockets
      migration: avoid using error_is_set and thus relying on errp != NULL
      migration: centralize call to migrate_fd_error()
      migration: use qemu-sockets to establish Unix sockets
      migration (outgoing): add error propagation for all protocols
      migration (incoming): add error propagation to fd and exec protocols
      qemu-char: ask and print error information from qemu-sockets
      nbd: ask and print error information from qemu-sockets
      qemu-ga: ask and print error information from qemu-sockets
      vnc: avoid Yoda conditionals
      vnc: introduce a single label for error returns
      vnc: reorganize code for reverse mode
      vnc: add error propagation to vnc_display_open
      qemu-sockets: include strerror or gai_strerror output in error messages
      qemu-sockets: add error propagation to inet_connect_addr
      qemu-sockets: add error propagation to inet_dgram_opts
      qemu-sockets: add error propagation to inet_parse
      qemu-sockets: add error propagation to Unix socket functions
      vnc: drop QERR_VNC_SERVER_FAILED
      build: add QAPI files to the tools
      qapi: add socket address types
      qemu-sockets: return InetSocketAddress from inet_parse
      qemu-sockets: add socket_listen, socket_connect, socket_parse
      block: prepare code for adding block notifiers
      block: add close notifiers
      qmp: add NBD server commands

 Makefile.objs       |   9 +-
 block.c             |  19 ++-
 block.h             |   1 +
 block_int.h         |   2 +
 blockdev-nbd.c      | 119 +++++++++++++++
 console.h           |   2 +-
 error.c             |  28 ++++
 error.h             |   9 ++
 migration-exec.c    |  26 +---
 migration-fd.c      |  27 +---
 migration-tcp.c     |  19 +--
 migration-unix.c    |  95 ++----------
 migration.c         |  34 ++---
 migration.h         |  19 ++-
 nbd.c               |  39 ++++-
 qapi-schema.json    |  96 ++++++++++++
 qemu-char.c         |  24 ++-
 qemu-sockets.c      | 424 ++++++++++++++++++++++++++++++++++------------------
 qemu-tool.c         |   6 +
 qemu_socket.h       |  19 ++-
 qerror.h            |   3 -
 qga/channel-posix.c |   8 +-
 qmp-commands.hx     |  16 ++
 qmp.c               |   6 +-
 ui/vnc.c            |  93 ++++++------
 vl.c                |  25 ++--
 26 file modificati, 756 inserzioni(+), 412 rimozioni(-)
 create mode 100644 blockdev-nbd.c
-- 
1.7.12.1

[Qemu-devel] [PATCH 01/29] error: add error_set_errno and error_setg_errno

[Paolo Bonzini]

These functions help maintaining homogeneous formatting of error
messages that include strerror values.

Acked-by: Luiz Capitulino <lcapitulino@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 error.c | 28 ++++++++++++++++++++++++++++
 error.h |  9 +++++++++
 2 file modificati, 37 inserzioni(+)

diff --git a/error.c b/error.c
index 1f05fc4..128d88c 100644
--- a/error.c
+++ b/error.c
@@ -43,6 +43,34 @@ void error_set(Error **errp, ErrorClass err_class, const char *fmt, ...)
     *errp = err;
 }
 
+void error_set_errno(Error **errp, int os_errno, ErrorClass err_class,
+                     const char *fmt, ...)
+{
+    Error *err;
+    char *msg1;
+    va_list ap;
+
+    if (errp == NULL) {
+        return;
+    }
+    assert(*errp == NULL);
+
+    err = g_malloc0(sizeof(*err));
+
+    va_start(ap, fmt);
+    msg1 = g_strdup_vprintf(fmt, ap);
+    if (os_errno != 0) {
+        err->msg = g_strdup_printf("%s: %s", msg1, strerror(os_errno));
+        g_free(msg1);
+    } else {
+        err->msg = msg1;
+    }
+    va_end(ap);
+    err->err_class = err_class;
+
+    *errp = err;
+}
+
 Error *error_copy(const Error *err)
 {
     Error *err_new;
diff --git a/error.h b/error.h
index da7fed3..4d52e73 100644
--- a/error.h
+++ b/error.h
@@ -30,10 +30,19 @@ typedef struct Error Error;
 void error_set(Error **err, ErrorClass err_class, const char *fmt, ...) GCC_FMT_ATTR(3, 4);
 
 /**
+ * Set an indirect pointer to an error given a ErrorClass value and a
+ * printf-style human message, followed by a strerror() string if
+ * @os_error is not zero.
+ */
+void error_set_errno(Error **err, int os_error, ErrorClass err_class, const char *fmt, ...) GCC_FMT_ATTR(4, 5);
+
+/**
  * Same as error_set(), but sets a generic error
  */
 #define error_setg(err, fmt, ...) \
     error_set(err, ERROR_CLASS_GENERIC_ERROR, fmt, ## __VA_ARGS__)
+#define error_setg_errno(err, os_error, fmt, ...) \
+    error_set_errno(err, os_error, ERROR_CLASS_GENERIC_ERROR, fmt, ## __VA_ARGS__)
 
 /**
  * Returns true if an indirect pointer to an error is pointing to a valid
-- 
1.7.12.1

[Qemu-devel] [PATCH 02/29] qemu-sockets: add Error ** to all functions

[Paolo Bonzini]

This lets me adjust the clients to do proper error propagation first,
thus avoiding temporary regressions in the quality of the error messages.

Reviewed-by: Luiz Capitulino <lcapitulino@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 nbd.c               |  4 ++--
 qemu-char.c         |  6 +++---
 qemu-sockets.c      | 22 +++++++++++-----------
 qemu_socket.h       | 10 +++++-----
 qga/channel-posix.c |  2 +-
 ui/vnc.c            |  4 ++--
 6 file modificati, 24 inserzioni(+), 24 rimozioni(-)

diff --git a/nbd.c b/nbd.c
index 6f0db62..f61a288 100644
--- a/nbd.c
+++ b/nbd.c
@@ -230,12 +230,12 @@ int unix_socket_incoming(const char *path)
     char *ostr = NULL;
     int olen = 0;
 
-    return unix_listen(path, ostr, olen);
+    return unix_listen(path, ostr, olen, NULL);
 }
 
 int unix_socket_outgoing(const char *path)
 {
-    return unix_connect(path);
+    return unix_connect(path, NULL);
 }
 
 /* Basic flow for negotiation
diff --git a/qemu-char.c b/qemu-char.c
index b082bae..3cc6cb5 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2102,7 +2102,7 @@ static CharDriverState *qemu_chr_open_udp(QemuOpts *opts)
     chr = g_malloc0(sizeof(CharDriverState));
     s = g_malloc0(sizeof(NetCharDriver));
 
-    fd = inet_dgram_opts(opts);
+    fd = inet_dgram_opts(opts, NULL);
     if (fd < 0) {
         fprintf(stderr, "inet_dgram_opts failed\n");
         goto return_err;
@@ -2448,9 +2448,9 @@ static CharDriverState *qemu_chr_open_socket(QemuOpts *opts)
 
     if (is_unix) {
         if (is_listen) {
-            fd = unix_listen_opts(opts);
+            fd = unix_listen_opts(opts, NULL);
         } else {
-            fd = unix_connect_opts(opts);
+            fd = unix_connect_opts(opts, NULL);
         }
     } else {
         if (is_listen) {
diff --git a/qemu-sockets.c b/qemu-sockets.c
index 2b1ed2f..7f0d4be 100644
--- a/qemu-sockets.c
+++ b/qemu-sockets.c
@@ -403,7 +403,7 @@ int inet_connect_opts(QemuOpts *opts, Error **errp,
     return sock;
 }
 
-int inet_dgram_opts(QemuOpts *opts)
+int inet_dgram_opts(QemuOpts *opts, Error **errp)
 {
     struct addrinfo ai, *peer = NULL, *local = NULL;
     const char *addr;
@@ -653,7 +653,7 @@ int inet_nonblocking_connect(const char *str,
 
 #ifndef _WIN32
 
-int unix_listen_opts(QemuOpts *opts)
+int unix_listen_opts(QemuOpts *opts, Error **errp)
 {
     struct sockaddr_un un;
     const char *path = qemu_opt_get(opts, "path");
@@ -701,7 +701,7 @@ err:
     return -1;
 }
 
-int unix_connect_opts(QemuOpts *opts)
+int unix_connect_opts(QemuOpts *opts, Error **errp)
 {
     struct sockaddr_un un;
     const char *path = qemu_opt_get(opts, "path");
@@ -731,7 +731,7 @@ int unix_connect_opts(QemuOpts *opts)
 }
 
 /* compatibility wrapper */
-int unix_listen(const char *str, char *ostr, int olen)
+int unix_listen(const char *str, char *ostr, int olen, Error **errp)
 {
     QemuOpts *opts;
     char *path, *optstr;
@@ -752,7 +752,7 @@ int unix_listen(const char *str, char *ostr, int olen)
         qemu_opt_set(opts, "path", str);
     }
 
-    sock = unix_listen_opts(opts);
+    sock = unix_listen_opts(opts, errp);
 
     if (sock != -1 && ostr)
         snprintf(ostr, olen, "%s%s", qemu_opt_get(opts, "path"), optstr ? optstr : "");
@@ -760,42 +760,42 @@ int unix_listen(const char *str, char *ostr, int olen)
     return sock;
 }
 
-int unix_connect(const char *path)
+int unix_connect(const char *path, Error **errp)
 {
     QemuOpts *opts;
     int sock;
 
     opts = qemu_opts_create(&dummy_opts, NULL, 0, NULL);
     qemu_opt_set(opts, "path", path);
-    sock = unix_connect_opts(opts);
+    sock = unix_connect_opts(opts, errp);
     qemu_opts_del(opts);
     return sock;
 }
 
 #else
 
-int unix_listen_opts(QemuOpts *opts)
+int unix_listen_opts(QemuOpts *opts, Error **errp)
 {
     fprintf(stderr, "unix sockets are not available on windows\n");
     errno = ENOTSUP;
     return -1;
 }
 
-int unix_connect_opts(QemuOpts *opts)
+int unix_connect_opts(QemuOpts *opts, Error **errp)
 {
     fprintf(stderr, "unix sockets are not available on windows\n");
     errno = ENOTSUP;
     return -1;
 }
 
-int unix_listen(const char *path, char *ostr, int olen)
+int unix_listen(const char *path, char *ostr, int olen, Error **errp)
 {
     fprintf(stderr, "unix sockets are not available on windows\n");
     errno = ENOTSUP;
     return -1;
 }
 
-int unix_connect(const char *path)
+int unix_connect(const char *path, Error **errp)
 {
     fprintf(stderr, "unix sockets are not available on windows\n");
     errno = ENOTSUP;
diff --git a/qemu_socket.h b/qemu_socket.h
index 3e8aee9..ff979b5 100644
--- a/qemu_socket.h
+++ b/qemu_socket.h
@@ -53,13 +53,13 @@ int inet_nonblocking_connect(const char *str,
                              NonBlockingConnectHandler *callback,
                              void *opaque, Error **errp);
 
-int inet_dgram_opts(QemuOpts *opts);
+int inet_dgram_opts(QemuOpts *opts, Error **errp);
 const char *inet_strfamily(int family);
 
-int unix_listen_opts(QemuOpts *opts);
-int unix_listen(const char *path, char *ostr, int olen);
-int unix_connect_opts(QemuOpts *opts);
-int unix_connect(const char *path);
+int unix_listen_opts(QemuOpts *opts, Error **errp);
+int unix_listen(const char *path, char *ostr, int olen, Error **errp);
+int unix_connect_opts(QemuOpts *opts, Error **errp);
+int unix_connect(const char *path, Error **errp);
 
 /* Old, ipv4 only bits.  Don't use for new code. */
 int parse_host_port(struct sockaddr_in *saddr, const char *str);
diff --git a/qga/channel-posix.c b/qga/channel-posix.c
index 57eea06..e22eee6 100644
--- a/qga/channel-posix.c
+++ b/qga/channel-posix.c
@@ -181,7 +181,7 @@ static gboolean ga_channel_open(GAChannel *c, const gchar *path, GAChannelMethod
         break;
     }
     case GA_CHANNEL_UNIX_LISTEN: {
-        int fd = unix_listen(path, NULL, strlen(path));
+        int fd = unix_listen(path, NULL, strlen(path), NULL);
         if (fd == -1) {
             g_critical("error opening path: %s", strerror(errno));
             return false;
diff --git a/ui/vnc.c b/ui/vnc.c
index 33e6386..b893e07 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -3063,7 +3063,7 @@ int vnc_display_open(DisplayState *ds, const char *display)
     if (reverse) {
         /* connect to viewer */
         if (strncmp(display, "unix:", 5) == 0)
-            vs->lsock = unix_connect(display+5);
+            vs->lsock = unix_connect(display+5, NULL);
         else
             vs->lsock = inet_connect(display, NULL);
         if (-1 == vs->lsock) {
@@ -3083,7 +3083,7 @@ int vnc_display_open(DisplayState *ds, const char *display)
         dpy = g_malloc(256);
         if (strncmp(display, "unix:", 5) == 0) {
             pstrcpy(dpy, 256, "unix:");
-            vs->lsock = unix_listen(display+5, dpy+5, 256-5);
+            vs->lsock = unix_listen(display+5, dpy+5, 256-5, NULL);
         } else {
             vs->lsock = inet_listen(display, dpy, 256,
                                     SOCK_STREAM, 5900, NULL);
-- 
1.7.12.1

[Qemu-devel] [PATCH 03/29] qemu-sockets: unix_listen and unix_connect are portable

[Paolo Bonzini]

They are just wrappers and do not need a Win32-specific version.

Reviewed-by: Luiz Capitulino <lcapitulino@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qemu-sockets.c | 49 +++++++++++++++++--------------------------------
 1 file modificato, 17 inserzioni(+), 32 rimozioni(-)

diff --git a/qemu-sockets.c b/qemu-sockets.c
index 7f0d4be..7bf756d 100644
--- a/qemu-sockets.c
+++ b/qemu-sockets.c
@@ -730,6 +730,23 @@ int unix_connect_opts(QemuOpts *opts, Error **errp)
     return sock;
 }
 
+#else
+
+int unix_listen_opts(QemuOpts *opts, Error **errp)
+{
+    fprintf(stderr, "unix sockets are not available on windows\n");
+    errno = ENOTSUP;
+    return -1;
+}
+
+int unix_connect_opts(QemuOpts *opts, Error **errp)
+{
+    fprintf(stderr, "unix sockets are not available on windows\n");
+    errno = ENOTSUP;
+    return -1;
+}
+#endif
+
 /* compatibility wrapper */
 int unix_listen(const char *str, char *ostr, int olen, Error **errp)
 {
@@ -772,38 +789,6 @@ int unix_connect(const char *path, Error **errp)
     return sock;
 }
 
-#else
-
-int unix_listen_opts(QemuOpts *opts, Error **errp)
-{
-    fprintf(stderr, "unix sockets are not available on windows\n");
-    errno = ENOTSUP;
-    return -1;
-}
-
-int unix_connect_opts(QemuOpts *opts, Error **errp)
-{
-    fprintf(stderr, "unix sockets are not available on windows\n");
-    errno = ENOTSUP;
-    return -1;
-}
-
-int unix_listen(const char *path, char *ostr, int olen, Error **errp)
-{
-    fprintf(stderr, "unix sockets are not available on windows\n");
-    errno = ENOTSUP;
-    return -1;
-}
-
-int unix_connect(const char *path, Error **errp)
-{
-    fprintf(stderr, "unix sockets are not available on windows\n");
-    errno = ENOTSUP;
-    return -1;
-}
-
-#endif
-
 #ifdef _WIN32
 static void socket_cleanup(void)
 {
-- 
1.7.12.1

[Qemu-devel] [PATCH 04/29] qemu-sockets: add nonblocking connect for Unix sockets

[Paolo Bonzini]

This patch mostly mimics what was done to TCP sockets, but simpler
because there is only one address to try.  It also includes a free EINTR
bug fix.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qemu-char.c    |  2 +-
 qemu-sockets.c | 77 ++++++++++++++++++++++++++++++++++++++++++++++++----------
 qemu_socket.h  |  6 ++++-
 3 file modificati, 70 inserzioni(+), 15 rimozioni(-)

diff --git a/qemu-char.c b/qemu-char.c
index 3cc6cb5..8ebd582 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2450,7 +2450,7 @@ static CharDriverState *qemu_chr_open_socket(QemuOpts *opts)
         if (is_listen) {
             fd = unix_listen_opts(opts, NULL);
         } else {
-            fd = unix_connect_opts(opts, NULL);
+            fd = unix_connect_opts(opts, NULL, NULL, NULL);
         }
     } else {
         if (is_listen) {
diff --git a/qemu-sockets.c b/qemu-sockets.c
index 7bf756d..d510ad1 100644
--- a/qemu-sockets.c
+++ b/qemu-sockets.c
@@ -252,16 +252,19 @@ static void wait_for_connect(void *opaque)
     }
 
     /* try to connect to the next address on the list */
-    while (s->current_addr->ai_next != NULL && s->fd < 0) {
-        s->current_addr = s->current_addr->ai_next;
-        s->fd = inet_connect_addr(s->current_addr, &in_progress, s);
-        /* connect in progress */
-        if (in_progress) {
-            return;
+    if (s->current_addr) {
+        while (s->current_addr->ai_next != NULL && s->fd < 0) {
+            s->current_addr = s->current_addr->ai_next;
+            s->fd = inet_connect_addr(s->current_addr, &in_progress, s);
+            /* connect in progress */
+            if (in_progress) {
+                return;
+            }
         }
+
+        freeaddrinfo(s->addr_list);
     }
 
-    freeaddrinfo(s->addr_list);
     if (s->callback) {
         s->callback(s->fd, s->opaque);
     }
@@ -701,11 +704,13 @@ err:
     return -1;
 }
 
-int unix_connect_opts(QemuOpts *opts, Error **errp)
+int unix_connect_opts(QemuOpts *opts, Error **errp,
+                      NonBlockingConnectHandler *callback, void *opaque)
 {
     struct sockaddr_un un;
     const char *path = qemu_opt_get(opts, "path");
-    int sock;
+    ConnectState *connect_state = NULL;
+    int sock, rc;
 
     if (NULL == path) {
         fprintf(stderr, "unix connect: no path specified\n");
@@ -717,16 +722,44 @@ int unix_connect_opts(QemuOpts *opts, Error **errp)
         perror("socket(unix)");
         return -1;
     }
+    if (callback != NULL) {
+        connect_state = g_malloc0(sizeof(*connect_state));
+        connect_state->callback = callback;
+        connect_state->opaque = opaque;
+        socket_set_nonblock(sock);
+    }
 
     memset(&un, 0, sizeof(un));
     un.sun_family = AF_UNIX;
     snprintf(un.sun_path, sizeof(un.sun_path), "%s", path);
-    if (connect(sock, (struct sockaddr*) &un, sizeof(un)) < 0) {
+
+    /* connect to peer */
+    do {
+        rc = 0;
+        if (connect(sock, (struct sockaddr *) &un, sizeof(un)) < 0) {
+            rc = -socket_error();
+        }
+    } while (rc == -EINTR);
+
+    if (connect_state != NULL && QEMU_SOCKET_RC_INPROGRESS(rc)) {
+        connect_state->fd = sock;
+        qemu_set_fd_handler2(sock, NULL, NULL, wait_for_connect,
+                             connect_state);
+        return sock;
+    } else if (rc >= 0) {
+        /* non blocking socket immediate success, call callback */
+        if (callback != NULL) {
+            callback(sock, opaque);
+        }
+    }
+
+    if (rc < 0) {
         fprintf(stderr, "connect(unix:%s): %s\n", path, strerror(errno));
         close(sock);
-	return -1;
+        sock = -1;
     }
 
+    g_free(connect_state);
     return sock;
 }
 
@@ -739,7 +772,8 @@ int unix_listen_opts(QemuOpts *opts, Error **errp)
     return -1;
 }
 
-int unix_connect_opts(QemuOpts *opts, Error **errp)
+int unix_connect_opts(QemuOpts *opts, Error **errp,
+                      NonBlockingConnectHandler *callback, void *opaque)
 {
     fprintf(stderr, "unix sockets are not available on windows\n");
     errno = ENOTSUP;
@@ -784,7 +818,24 @@ int unix_connect(const char *path, Error **errp)
 
     opts = qemu_opts_create(&dummy_opts, NULL, 0, NULL);
     qemu_opt_set(opts, "path", path);
-    sock = unix_connect_opts(opts, errp);
+    sock = unix_connect_opts(opts, errp, NULL, NULL);
+    qemu_opts_del(opts);
+    return sock;
+}
+
+
+int unix_nonblocking_connect(const char *path,
+                             NonBlockingConnectHandler *callback,
+                             void *opaque, Error **errp)
+{
+    QemuOpts *opts;
+    int sock = -1;
+
+    g_assert(callback != NULL);
+
+    opts = qemu_opts_create(&dummy_opts, NULL, 0, NULL);
+    qemu_opt_set(opts, "path", path);
+    sock = unix_connect_opts(opts, errp, callback, opaque);
     qemu_opts_del(opts);
     return sock;
 }
diff --git a/qemu_socket.h b/qemu_socket.h
index ff979b5..89a5feb 100644
--- a/qemu_socket.h
+++ b/qemu_socket.h
@@ -58,8 +58,12 @@ const char *inet_strfamily(int family);
 
 int unix_listen_opts(QemuOpts *opts, Error **errp);
 int unix_listen(const char *path, char *ostr, int olen, Error **errp);
-int unix_connect_opts(QemuOpts *opts, Error **errp);
+int unix_connect_opts(QemuOpts *opts, Error **errp,
+                      NonBlockingConnectHandler *callback, void *opaque);
 int unix_connect(const char *path, Error **errp);
+int unix_nonblocking_connect(const char *str,
+                             NonBlockingConnectHandler *callback,
+                             void *opaque, Error **errp);
 
 /* Old, ipv4 only bits.  Don't use for new code. */
 int parse_host_port(struct sockaddr_in *saddr, const char *str);
-- 
1.7.12.1

[Qemu-devel] [PATCH 05/29] migration: avoid using error_is_set and thus relying on errp != NULL

[Paolo Bonzini]

The migration code is using errp to detect "internal" errors, this means
that it relies on errp being non-NULL.

No impact so far because our only QMP clients (the QMP marshaller and HMP)
never pass a NULL Error **.  But if we had others, this patch would make
sure that migration can work with a NULL Error **.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 migration-tcp.c |  8 +++++---
 migration.c     | 13 +++++++------
 2 file modificati, 12 inserzioni(+), 9 rimozioni(-)

diff --git a/migration-tcp.c b/migration-tcp.c
index a15c2b8..78337a3 100644
--- a/migration-tcp.c
+++ b/migration-tcp.c
@@ -71,14 +71,16 @@ static void tcp_wait_for_connect(int fd, void *opaque)
 int tcp_start_outgoing_migration(MigrationState *s, const char *host_port,
                                  Error **errp)
 {
+    Error *local_err = NULL;
+
     s->get_error = socket_errno;
     s->write = socket_write;
     s->close = tcp_close;
 
-    s->fd = inet_nonblocking_connect(host_port, tcp_wait_for_connect, s,
-                                     errp);
-    if (error_is_set(errp)) {
+    s->fd = inet_nonblocking_connect(host_port, tcp_wait_for_connect, s, &local_err);
+    if (local_err != NULL) {
         migrate_fd_error(s);
+        error_propagate(errp, local_err);
         return -1;
     }
 
diff --git a/migration.c b/migration.c
index 22a05c4..8a04174 100644
--- a/migration.c
+++ b/migration.c
@@ -481,6 +481,7 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
                  bool has_inc, bool inc, bool has_detach, bool detach,
                  Error **errp)
 {
+    Error *local_err = NULL;
     MigrationState *s = migrate_get_current();
     MigrationParams params;
     const char *p;
@@ -506,7 +507,7 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
     s = migrate_init(&params);
 
     if (strstart(uri, "tcp:", &p)) {
-        ret = tcp_start_outgoing_migration(s, p, errp);
+        ret = tcp_start_outgoing_migration(s, p, &local_err);
 #if !defined(WIN32)
     } else if (strstart(uri, "exec:", &p)) {
         ret = exec_start_outgoing_migration(s, p);
@@ -520,11 +521,11 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
         return;
     }
 
-    if (ret < 0) {
-        if (!error_is_set(errp)) {
-            DPRINTF("migration failed: %s\n", strerror(-ret));
-            /* FIXME: we should return meaningful errors */
-            error_set(errp, QERR_UNDEFINED_ERROR);
+    if (ret < 0 || local_err) {
+        if (!local_err) {
+            error_set_errno(errp, -ret, QERR_UNDEFINED_ERROR);
+        } else {
+            error_propagate(errp, local_err);
         }
         return;
     }
-- 
1.7.12.1

[Qemu-devel] [PATCH 06/29] migration: centralize call to migrate_fd_error()

[Paolo Bonzini]

The call to migrate_fd_error() was missing for non-socket backends, so
centralize it in qmp_migrate().

Before:

    (qemu) migrate fd:ffff
    migrate: An undefined error has occurred
    (qemu) info migrate
    (qemu)

After:

    (qemu) migrate fd:ffff
    migrate: An undefined error has occurred
    (qemu) info migrate
    capabilities: xbzrle: off
    Migration status: failed
    total time: 0 milliseconds

(The awful error message will be fixed later in the series).

Reviewed-by: Luiz Capitulino <lcapitulino@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 migration-tcp.c  | 1 -
 migration-unix.c | 1 -
 migration.c      | 1 +
 3 file modificati, 1 inserzione(+), 2 rimozioni(-)

diff --git a/migration-tcp.c b/migration-tcp.c
index 78337a3..e8bc76a 100644
--- a/migration-tcp.c
+++ b/migration-tcp.c
@@ -79,7 +79,6 @@ int tcp_start_outgoing_migration(MigrationState *s, const char *host_port,
 
     s->fd = inet_nonblocking_connect(host_port, tcp_wait_for_connect, s, &local_err);
     if (local_err != NULL) {
-        migrate_fd_error(s);
         error_propagate(errp, local_err);
         return -1;
     }
diff --git a/migration-unix.c b/migration-unix.c
index 169de88..d349662 100644
--- a/migration-unix.c
+++ b/migration-unix.c
@@ -111,7 +111,6 @@ int unix_start_outgoing_migration(MigrationState *s, const char *path)
 
     if (ret < 0) {
         DPRINTF("connect failed\n");
-        migrate_fd_error(s);
         return ret;
     }
     migrate_fd_connect(s);
diff --git a/migration.c b/migration.c
index 8a04174..a56358e 100644
--- a/migration.c
+++ b/migration.c
@@ -522,6 +522,7 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
     }
 
     if (ret < 0 || local_err) {
+        migrate_fd_error(s);
         if (!local_err) {
             error_set_errno(errp, -ret, QERR_UNDEFINED_ERROR);
         } else {
-- 
1.7.12.1

[Qemu-devel] [PATCH 07/29] migration: use qemu-sockets to establish Unix sockets

[Paolo Bonzini]

This makes migration-unix.c again a cut-and-paste job from migration-tcp.c,
exactly as it was in the beginning. :)

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 migration-unix.c | 94 ++++++++++----------------------------------------------
 migration.c      |  4 +--
 migration.h      |  4 +--
 3 file modificati, 21 inserzioni(+), 81 rimozioni(-)

diff --git a/migration-unix.c b/migration-unix.c
index d349662..5387c21 100644
--- a/migration-unix.c
+++ b/migration-unix.c
@@ -53,67 +53,34 @@ static int unix_close(MigrationState *s)
     return r;
 }
 
-static void unix_wait_for_connect(void *opaque)
+static void unix_wait_for_connect(int fd, void *opaque)
 {
     MigrationState *s = opaque;
-    int val, ret;
-    socklen_t valsize = sizeof(val);
 
-    DPRINTF("connect completed\n");
-    do {
-        ret = getsockopt(s->fd, SOL_SOCKET, SO_ERROR, (void *) &val, &valsize);
-    } while (ret == -1 && errno == EINTR);
-
-    if (ret < 0) {
+    if (fd < 0) {
+        DPRINTF("migrate connect error\n");
+        s->fd = -1;
         migrate_fd_error(s);
-        return;
-    }
-
-    qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL);
-
-    if (val == 0)
+    } else {
+        DPRINTF("migrate connect success\n");
+        s->fd = fd;
         migrate_fd_connect(s);
-    else {
-        DPRINTF("error connecting %d\n", val);
-        migrate_fd_error(s);
     }
 }
 
-int unix_start_outgoing_migration(MigrationState *s, const char *path)
+int unix_start_outgoing_migration(MigrationState *s, const char *path, Error **errp)
 {
-    struct sockaddr_un addr;
-    int ret;
+    Error *local_err = NULL;
 
-    addr.sun_family = AF_UNIX;
-    snprintf(addr.sun_path, sizeof(addr.sun_path), "%s", path);
     s->get_error = unix_errno;
     s->write = unix_write;
     s->close = unix_close;
 
-    s->fd = qemu_socket(PF_UNIX, SOCK_STREAM, 0);
-    if (s->fd == -1) {
-        DPRINTF("Unable to open socket");
-        return -errno;
-    }
-
-    socket_set_nonblock(s->fd);
-
-    do {
-        ret = connect(s->fd, (struct sockaddr *)&addr, sizeof(addr));
-        if (ret == -1) {
-            ret = -errno;
-        }
-        if (ret == -EINPROGRESS || ret == -EWOULDBLOCK) {
-	    qemu_set_fd_handler2(s->fd, NULL, NULL, unix_wait_for_connect, s);
-            return 0;
-        }
-    } while (ret == -EINTR);
-
-    if (ret < 0) {
-        DPRINTF("connect failed\n");
-        return ret;
+    s->fd = unix_nonblocking_connect(path, unix_wait_for_connect, s, &local_err);
+    if (local_err != NULL) {
+        error_propagate(errp, local_err);
+        return -1;
     }
-    migrate_fd_connect(s);
     return 0;
 }
 
@@ -151,43 +118,16 @@ out2:
     close(s);
 }
 
-int unix_start_incoming_migration(const char *path)
+int unix_start_incoming_migration(const char *path, Error **errp)
 {
-    struct sockaddr_un addr;
     int s;
-    int ret;
-
-    DPRINTF("Attempting to start an incoming migration\n");
-
-    s = qemu_socket(PF_UNIX, SOCK_STREAM, 0);
-    if (s == -1) {
-        fprintf(stderr, "Could not open unix socket: %s\n", strerror(errno));
-        return -errno;
-    }
-
-    memset(&addr, 0, sizeof(addr));
-    addr.sun_family = AF_UNIX;
-    snprintf(addr.sun_path, sizeof(addr.sun_path), "%s", path);
 
-    unlink(addr.sun_path);
-    if (bind(s, (struct sockaddr *) &addr, sizeof(addr)) < 0) {
-        ret = -errno;
-        fprintf(stderr, "bind(unix:%s): %s\n", addr.sun_path, strerror(errno));
-        goto err;
-    }
-    if (listen(s, 1) == -1) {
-        fprintf(stderr, "listen(unix:%s): %s\n", addr.sun_path,
-                strerror(errno));
-        ret = -errno;
-        goto err;
+    s = unix_listen(path, NULL, 0, errp);
+    if (s < 0) {
+        return -1;
     }
 
     qemu_set_fd_handler2(s, NULL, unix_accept_incoming_migration, NULL,
                          (void *)(intptr_t)s);
-
     return 0;
-
-err:
-    close(s);
-    return ret;
 }
diff --git a/migration.c b/migration.c
index a56358e..767e297 100644
--- a/migration.c
+++ b/migration.c
@@ -75,7 +75,7 @@ int qemu_start_incoming_migration(const char *uri, Error **errp)
     else if (strstart(uri, "exec:", &p))
         ret =  exec_start_incoming_migration(p);
     else if (strstart(uri, "unix:", &p))
-        ret = unix_start_incoming_migration(p);
+        ret = unix_start_incoming_migration(p, errp);
     else if (strstart(uri, "fd:", &p))
         ret = fd_start_incoming_migration(p);
 #endif
@@ -512,7 +512,7 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
     } else if (strstart(uri, "exec:", &p)) {
         ret = exec_start_outgoing_migration(s, p);
     } else if (strstart(uri, "unix:", &p)) {
-        ret = unix_start_outgoing_migration(s, p);
+        ret = unix_start_outgoing_migration(s, p, &local_err);
     } else if (strstart(uri, "fd:", &p)) {
         ret = fd_start_outgoing_migration(s, p);
 #endif
diff --git a/migration.h b/migration.h
index a9852fc..e0612a3 100644
--- a/migration.h
+++ b/migration.h
@@ -63,9 +63,9 @@ int tcp_start_incoming_migration(const char *host_port, Error **errp);
 int tcp_start_outgoing_migration(MigrationState *s, const char *host_port,
                                  Error **errp);
 
-int unix_start_incoming_migration(const char *path);
+int unix_start_incoming_migration(const char *path, Error **errp);
 
-int unix_start_outgoing_migration(MigrationState *s, const char *path);
+int unix_start_outgoing_migration(MigrationState *s, const char *path, Error **errp);
 
 int fd_start_incoming_migration(const char *path);
 
-- 
1.7.12.1

[Qemu-devel] [PATCH 08/29] migration (outgoing): add error propagation for all protocols

[Paolo Bonzini]

Error propagation is already there for socket backends.  Add it to other
protocols, simplifying code that tests for errors that will never happen.
With all protocols understanding Error, the code can be simplified
further by removing the return value.

Unfortunately, the quality of error messages varies depending
on where the error is detected, because no Error is passed to the
NonBlockingConnectHandler.  Thus, the exact error message still cannot
be sent to the user if the OS reports it asynchronously via SO_ERROR.
If NonBlockingConnectHandler received an Error**, we could for
example report the error class and/or message via a new field of the
query-migration command even if it is reported asynchronously.

Before:

    (qemu) migrate fd:ffff
    migrate: An undefined error has occurred
    (qemu) info migrate
    (qemu)

After:

    (qemu) migrate fd:ffff
    migrate: File descriptor named 'ffff' has not been found
    (qemu) info migrate
    capabilities: xbzrle: off
    Migration status: failed
    total time: 0 milliseconds

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 migration-exec.c | 18 ++++--------------
 migration-fd.c   | 19 ++++---------------
 migration-tcp.c  | 13 ++-----------
 migration-unix.c | 11 ++---------
 migration.c      | 17 ++++++-----------
 migration.h      |  9 ++++-----
 6 file modificati, 22 inserzioni(+), 65 rimozioni(-)

diff --git a/migration-exec.c b/migration-exec.c
index 6c97db9..5f3f4b2 100644
--- a/migration-exec.c
+++ b/migration-exec.c
@@ -60,22 +60,18 @@ static int exec_close(MigrationState *s)
     return ret;
 }
 
-int exec_start_outgoing_migration(MigrationState *s, const char *command)
+void exec_start_outgoing_migration(MigrationState *s, const char *command, Error **errp)
 {
     FILE *f;
 
     f = popen(command, "w");
     if (f == NULL) {
-        DPRINTF("Unable to popen exec target\n");
-        goto err_after_popen;
+        error_setg_errno(errp, errno, "failed to popen the migration target");
+        return;
     }
 
     s->fd = fileno(f);
-    if (s->fd == -1) {
-        DPRINTF("Unable to retrieve file descriptor for popen'd handle\n");
-        goto err_after_open;
-    }
-
+    assert(s->fd != -1);
     socket_set_nonblock(s->fd);
 
     s->opaque = qemu_popen(f, "w");
@@ -85,12 +81,6 @@ int exec_start_outgoing_migration(MigrationState *s, const char *command)
     s->write = file_write;
 
     migrate_fd_connect(s);
-    return 0;
-
-err_after_open:
-    pclose(f);
-err_after_popen:
-    return -1;
 }
 
 static void exec_accept_incoming_migration(void *opaque)
diff --git a/migration-fd.c b/migration-fd.c
index 7335167..a7c800a 100644
--- a/migration-fd.c
+++ b/migration-fd.c
@@ -73,30 +73,19 @@ static int fd_close(MigrationState *s)
     return 0;
 }
 
-int fd_start_outgoing_migration(MigrationState *s, const char *fdname)
+void fd_start_outgoing_migration(MigrationState *s, const char *fdname, Error **errp)
 {
-    s->fd = monitor_get_fd(cur_mon, fdname, NULL);
+    s->fd = monitor_get_fd(cur_mon, fdname, errp);
     if (s->fd == -1) {
-        DPRINTF("fd_migration: invalid file descriptor identifier\n");
-        goto err_after_get_fd;
-    }
-
-    if (fcntl(s->fd, F_SETFL, O_NONBLOCK) == -1) {
-        DPRINTF("Unable to set nonblocking mode on file descriptor\n");
-        goto err_after_open;
+        return;
     }
 
+    fcntl(s->fd, F_SETFL, O_NONBLOCK);
     s->get_error = fd_errno;
     s->write = fd_write;
     s->close = fd_close;
 
     migrate_fd_connect(s);
-    return 0;
-
-err_after_open:
-    close(s->fd);
-err_after_get_fd:
-    return -1;
 }
 
 static void fd_accept_incoming_migration(void *opaque)
diff --git a/migration-tcp.c b/migration-tcp.c
index e8bc76a..5e54e68 100644
--- a/migration-tcp.c
+++ b/migration-tcp.c
@@ -68,22 +68,13 @@ static void tcp_wait_for_connect(int fd, void *opaque)
     }
 }
 
-int tcp_start_outgoing_migration(MigrationState *s, const char *host_port,
-                                 Error **errp)
+void tcp_start_outgoing_migration(MigrationState *s, const char *host_port, Error **errp)
 {
-    Error *local_err = NULL;
-
     s->get_error = socket_errno;
     s->write = socket_write;
     s->close = tcp_close;
 
-    s->fd = inet_nonblocking_connect(host_port, tcp_wait_for_connect, s, &local_err);
-    if (local_err != NULL) {
-        error_propagate(errp, local_err);
-        return -1;
-    }
-
-    return 0;
+    s->fd = inet_nonblocking_connect(host_port, tcp_wait_for_connect, s, errp);
 }
 
 static void tcp_accept_incoming_migration(void *opaque)
diff --git a/migration-unix.c b/migration-unix.c
index 5387c21..34a413a 100644
--- a/migration-unix.c
+++ b/migration-unix.c
@@ -68,20 +68,13 @@ static void unix_wait_for_connect(int fd, void *opaque)
     }
 }
 
-int unix_start_outgoing_migration(MigrationState *s, const char *path, Error **errp)
+void unix_start_outgoing_migration(MigrationState *s, const char *path, Error **errp)
 {
-    Error *local_err = NULL;
-
     s->get_error = unix_errno;
     s->write = unix_write;
     s->close = unix_close;
 
-    s->fd = unix_nonblocking_connect(path, unix_wait_for_connect, s, &local_err);
-    if (local_err != NULL) {
-        error_propagate(errp, local_err);
-        return -1;
-    }
-    return 0;
+    s->fd = unix_nonblocking_connect(path, unix_wait_for_connect, s, errp);
 }
 
 static void unix_accept_incoming_migration(void *opaque)
diff --git a/migration.c b/migration.c
index 767e297..f7f0138 100644
--- a/migration.c
+++ b/migration.c
@@ -485,7 +485,6 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
     MigrationState *s = migrate_get_current();
     MigrationParams params;
     const char *p;
-    int ret;
 
     params.blk = blk;
     params.shared = inc;
@@ -507,27 +506,23 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
     s = migrate_init(&params);
 
     if (strstart(uri, "tcp:", &p)) {
-        ret = tcp_start_outgoing_migration(s, p, &local_err);
+        tcp_start_outgoing_migration(s, p, &local_err);
 #if !defined(WIN32)
     } else if (strstart(uri, "exec:", &p)) {
-        ret = exec_start_outgoing_migration(s, p);
+        exec_start_outgoing_migration(s, p, &local_err);
     } else if (strstart(uri, "unix:", &p)) {
-        ret = unix_start_outgoing_migration(s, p, &local_err);
+        unix_start_outgoing_migration(s, p, &local_err);
     } else if (strstart(uri, "fd:", &p)) {
-        ret = fd_start_outgoing_migration(s, p);
+        fd_start_outgoing_migration(s, p, &local_err);
 #endif
     } else {
         error_set(errp, QERR_INVALID_PARAMETER_VALUE, "uri", "a valid migration protocol");
         return;
     }
 
-    if (ret < 0 || local_err) {
+    if (local_err) {
         migrate_fd_error(s);
-        if (!local_err) {
-            error_set_errno(errp, -ret, QERR_UNDEFINED_ERROR);
-        } else {
-            error_propagate(errp, local_err);
-        }
+        error_propagate(errp, local_err);
         return;
     }
 
diff --git a/migration.h b/migration.h
index e0612a3..275d2d8 100644
--- a/migration.h
+++ b/migration.h
@@ -56,20 +56,19 @@ void do_info_migrate(Monitor *mon, QObject **ret_data);
 
 int exec_start_incoming_migration(const char *host_port);
 
-int exec_start_outgoing_migration(MigrationState *s, const char *host_port);
+void exec_start_outgoing_migration(MigrationState *s, const char *host_port, Error **errp);
 
 int tcp_start_incoming_migration(const char *host_port, Error **errp);
 
-int tcp_start_outgoing_migration(MigrationState *s, const char *host_port,
-                                 Error **errp);
+void tcp_start_outgoing_migration(MigrationState *s, const char *host_port, Error **errp);
 
 int unix_start_incoming_migration(const char *path, Error **errp);
 
-int unix_start_outgoing_migration(MigrationState *s, const char *path, Error **errp);
+void unix_start_outgoing_migration(MigrationState *s, const char *path, Error **errp);
 
 int fd_start_incoming_migration(const char *path);
 
-int fd_start_outgoing_migration(MigrationState *s, const char *fdname);
+void fd_start_outgoing_migration(MigrationState *s, const char *fdname, Error **errp);
 
 void migrate_fd_error(MigrationState *s);
 
-- 
1.7.12.1

[Qemu-devel] [PATCH 09/29] migration (incoming): add error propagation to fd and exec protocols

[Paolo Bonzini]

And remove the superfluous integer return value.

Reviewed-by: Luiz Capitulino <lcapitulino@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 migration-exec.c |  8 +++-----
 migration-fd.c   |  8 +++-----
 migration-tcp.c  |  7 ++-----
 migration-unix.c |  5 ++---
 migration.c      | 15 ++++++---------
 migration.h      | 10 +++++-----
 vl.c             | 16 ++++++----------
 7 file modificati, 27 inserzioni(+), 42 rimozioni(-)

diff --git a/migration-exec.c b/migration-exec.c
index 5f3f4b2..519af57 100644
--- a/migration-exec.c
+++ b/migration-exec.c
@@ -92,19 +92,17 @@ static void exec_accept_incoming_migration(void *opaque)
     qemu_fclose(f);
 }
 
-int exec_start_incoming_migration(const char *command)
+void exec_start_incoming_migration(const char *command, Error **errp)
 {
     QEMUFile *f;
 
     DPRINTF("Attempting to start an incoming migration\n");
     f = qemu_popen_cmd(command, "r");
     if(f == NULL) {
-        DPRINTF("Unable to apply qemu wrapper to popen file\n");
-        return -errno;
+        error_setg_errno(errp, errno, "failed to popen the migration source");
+        return;
     }
 
     qemu_set_fd_handler2(qemu_stdio_fd(f), NULL,
 			 exec_accept_incoming_migration, NULL, f);
-
-    return 0;
 }
diff --git a/migration-fd.c b/migration-fd.c
index a7c800a..ce6932d 100644
--- a/migration-fd.c
+++ b/migration-fd.c
@@ -97,7 +97,7 @@ static void fd_accept_incoming_migration(void *opaque)
     qemu_fclose(f);
 }
 
-int fd_start_incoming_migration(const char *infd)
+void fd_start_incoming_migration(const char *infd, Error **errp)
 {
     int fd;
     QEMUFile *f;
@@ -107,11 +107,9 @@ int fd_start_incoming_migration(const char *infd)
     fd = strtol(infd, NULL, 0);
     f = qemu_fdopen(fd, "rb");
     if(f == NULL) {
-        DPRINTF("Unable to apply qemu wrapper to file descriptor\n");
-        return -errno;
+        error_setg_errno(errp, errno, "failed to open the source descriptor");
+        return;
     }
 
     qemu_set_fd_handler2(fd, NULL, fd_accept_incoming_migration, NULL, f);
-
-    return 0;
 }
diff --git a/migration-tcp.c b/migration-tcp.c
index 5e54e68..46f6ac5 100644
--- a/migration-tcp.c
+++ b/migration-tcp.c
@@ -111,18 +111,15 @@ out2:
     close(s);
 }
 
-int tcp_start_incoming_migration(const char *host_port, Error **errp)
+void tcp_start_incoming_migration(const char *host_port, Error **errp)
 {
     int s;
 
     s = inet_listen(host_port, NULL, 256, SOCK_STREAM, 0, errp);
-
     if (s < 0) {
-        return -1;
+        return;
     }
 
     qemu_set_fd_handler2(s, NULL, tcp_accept_incoming_migration, NULL,
                          (void *)(intptr_t)s);
-
-    return 0;
 }
diff --git a/migration-unix.c b/migration-unix.c
index 34a413a..ed3db3a 100644
--- a/migration-unix.c
+++ b/migration-unix.c
@@ -111,16 +111,15 @@ out2:
     close(s);
 }
 
-int unix_start_incoming_migration(const char *path, Error **errp)
+void unix_start_incoming_migration(const char *path, Error **errp)
 {
     int s;
 
     s = unix_listen(path, NULL, 0, errp);
     if (s < 0) {
-        return -1;
+        return;
     }
 
     qemu_set_fd_handler2(s, NULL, unix_accept_incoming_migration, NULL,
                          (void *)(intptr_t)s);
-    return 0;
 }
diff --git a/migration.c b/migration.c
index f7f0138..2802918 100644
--- a/migration.c
+++ b/migration.c
@@ -64,26 +64,23 @@ static MigrationState *migrate_get_current(void)
     return &current_migration;
 }
 
-int qemu_start_incoming_migration(const char *uri, Error **errp)
+void 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, errp);
+        tcp_start_incoming_migration(p, errp);
 #if !defined(WIN32)
     else if (strstart(uri, "exec:", &p))
-        ret =  exec_start_incoming_migration(p);
+        exec_start_incoming_migration(p, errp);
     else if (strstart(uri, "unix:", &p))
-        ret = unix_start_incoming_migration(p, errp);
+        unix_start_incoming_migration(p, errp);
     else if (strstart(uri, "fd:", &p))
-        ret = fd_start_incoming_migration(p);
+        fd_start_incoming_migration(p, errp);
 #endif
     else {
-        fprintf(stderr, "unknown migration protocol: %s\n", uri);
-        ret = -EPROTONOSUPPORT;
+        error_setg(errp, "unknown migration protocol: %s\n", uri);
     }
-    return ret;
 }
 
 void process_incoming_migration(QEMUFile *f)
diff --git a/migration.h b/migration.h
index 275d2d8..aa44e1b 100644
--- a/migration.h
+++ b/migration.h
@@ -46,7 +46,7 @@ struct MigrationState
 
 void process_incoming_migration(QEMUFile *f);
 
-int qemu_start_incoming_migration(const char *uri, Error **errp);
+void qemu_start_incoming_migration(const char *uri, Error **errp);
 
 uint64_t migrate_max_downtime(void);
 
@@ -54,19 +54,19 @@ void do_info_migrate_print(Monitor *mon, const QObject *data);
 
 void do_info_migrate(Monitor *mon, QObject **ret_data);
 
-int exec_start_incoming_migration(const char *host_port);
+void exec_start_incoming_migration(const char *host_port, Error **errp);
 
 void exec_start_outgoing_migration(MigrationState *s, const char *host_port, Error **errp);
 
-int tcp_start_incoming_migration(const char *host_port, Error **errp);
+void tcp_start_incoming_migration(const char *host_port, Error **errp);
 
 void tcp_start_outgoing_migration(MigrationState *s, const char *host_port, Error **errp);
 
-int unix_start_incoming_migration(const char *path, Error **errp);
+void unix_start_incoming_migration(const char *path, Error **errp);
 
 void unix_start_outgoing_migration(MigrationState *s, const char *path, Error **errp);
 
-int fd_start_incoming_migration(const char *path);
+void fd_start_incoming_migration(const char *path, Error **errp);
 
 void fd_start_outgoing_migration(MigrationState *s, const char *fdname, Error **errp);
 
diff --git a/vl.c b/vl.c
index 5b357a3..2a072e8 100644
--- a/vl.c
+++ b/vl.c
@@ -3761,16 +3761,12 @@ int main(int argc, char **argv, char **envp)
     }
 
     if (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);
+        Error *local_err = NULL;
+        qemu_start_incoming_migration(incoming, &local_err);
+        if (local_err) {
+            fprintf(stderr, "-incoming %s: %s\n", incoming, error_get_pretty(local_err));
+            error_free(local_err);
+            exit(1);
         }
     } else if (autostart) {
         vm_start();
-- 
1.7.12.1

[Qemu-devel] [PATCH 10/29] qemu-char: ask and print error information from qemu-sockets

[Paolo Bonzini]

Before:

    $ qemu-system-x86_64 -monitor tcp:localhost:6000
    (starts despite error)

    $ qemu-system-x86_64 -monitor tcp:foo.bar:12345
    getaddrinfo(foo.bar,12345): Name or service not known
    chardev: opening backend "socket" failed

    $ qemu-system-x86_64 -monitor tcp:localhost:443,server=on
    inet_listen_opts: bind(ipv4,127.0.0.1,443): Permission denied
    inet_listen_opts: FAILED
    chardev: opening backend "socket" failed

After:

    $ x86_64-softmmu/qemu-system-x86_64 -monitor tcp:localhost:6000
    x86_64-softmmu/qemu-system-x86_64: -monitor tcp:localhost:6000: Failed to connect to socket: Connection refused
    chardev: opening backend "socket" failed

    $ x86_64-softmmu/qemu-system-x86_64 -monitor tcp:foo.bar:12345
    qemu-system-x86_64: -monitor tcp:foo.bar:12345: address resolution failed for foo.bar:12345: Name or service not known
    chardev: opening backend "socket" failed

    $ x86_64-softmmu/qemu-system-x86_64 -monitor tcp:localhost:443,server=on
    qemu-system-x86_64: -monitor tcp:localhost:443,server=on: Failed to bind socket: Permission denied
    chardev: opening backend "socket" failed

Reviewed-by: Luiz Capitulino <lcapitulino@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qemu-char.c | 23 +++++++++++++++++------
 1 file modificato, 17 inserzioni(+), 6 rimozioni(-)

diff --git a/qemu-char.c b/qemu-char.c
index 8ebd582..04b5c23 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2097,12 +2097,13 @@ static CharDriverState *qemu_chr_open_udp(QemuOpts *opts)
 {
     CharDriverState *chr = NULL;
     NetCharDriver *s = NULL;
+    Error *local_err = NULL;
     int fd = -1;
 
     chr = g_malloc0(sizeof(CharDriverState));
     s = g_malloc0(sizeof(NetCharDriver));
 
-    fd = inet_dgram_opts(opts, NULL);
+    fd = inet_dgram_opts(opts, &local_err);
     if (fd < 0) {
         fprintf(stderr, "inet_dgram_opts failed\n");
         goto return_err;
@@ -2118,6 +2119,10 @@ static CharDriverState *qemu_chr_open_udp(QemuOpts *opts)
     return chr;
 
 return_err:
+    if (local_err) {
+        qerror_report_err(local_err);
+        error_free(local_err);
+    }
     g_free(chr);
     g_free(s);
     if (fd >= 0) {
@@ -2428,6 +2433,7 @@ static CharDriverState *qemu_chr_open_socket(QemuOpts *opts)
 {
     CharDriverState *chr = NULL;
     TCPCharDriver *s = NULL;
+    Error *local_err = NULL;
     int fd = -1;
     int is_listen;
     int is_waitconnect;
@@ -2448,15 +2454,15 @@ static CharDriverState *qemu_chr_open_socket(QemuOpts *opts)
 
     if (is_unix) {
         if (is_listen) {
-            fd = unix_listen_opts(opts, NULL);
+            fd = unix_listen_opts(opts, &local_err);
         } else {
-            fd = unix_connect_opts(opts, NULL, NULL, NULL);
+            fd = unix_connect_opts(opts, &local_err, NULL, NULL);
         }
     } else {
         if (is_listen) {
-            fd = inet_listen_opts(opts, 0, NULL);
+            fd = inet_listen_opts(opts, 0, &local_err);
         } else {
-            fd = inet_connect_opts(opts, NULL, NULL, NULL);
+            fd = inet_connect_opts(opts, &local_err, NULL, NULL);
         }
     }
     if (fd < 0) {
@@ -2517,8 +2523,13 @@ static CharDriverState *qemu_chr_open_socket(QemuOpts *opts)
     return chr;
 
  fail:
-    if (fd >= 0)
+    if (local_err) {
+        qerror_report_err(local_err);
+        error_free(local_err);
+    }
+    if (fd >= 0) {
         closesocket(fd);
+    }
     g_free(s);
     g_free(chr);
     return NULL;
-- 
1.7.12.1

[Qemu-devel] [PATCH 11/29] nbd: ask and print error information from qemu-sockets

[Paolo Bonzini]

Before:

    $ qemu-system-x86_64 nbd:localhost:12345
    inet_connect_opts: connect(ipv4,yakj.usersys.redhat.com,127.0.0.1,12345): Connection refused
    qemu-system-x86_64: could not open disk image nbd:localhost:12345: Connection refused

After:

    $ x86_64-softmmu/qemu-system-x86_64 nbd:localhost:12345
    qemu-system-x86_64: Failed to connect to socket: Connection refused
    qemu-system-x86_64: could not open disk image nbd:localhost:12345: Connection refused

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 nbd.c | 39 +++++++++++++++++++++++++++++++--------
 1 file modificato, 31 inserzioni(+), 8 rimozioni(-)

diff --git a/nbd.c b/nbd.c
index f61a288..cec5a94 100644
--- a/nbd.c
+++ b/nbd.c
@@ -208,7 +208,14 @@ 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, NULL);
+    Error *local_err = NULL;
+    int fd = inet_connect(address_and_port, &local_err);
+
+    if (local_err != NULL) {
+        qerror_report_err(local_err);
+        error_free(local_err);
+    }
+    return fd;
 }
 
 int tcp_socket_incoming(const char *address, uint16_t port)
@@ -220,22 +227,38 @@ int tcp_socket_incoming(const char *address, uint16_t port)
 
 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, NULL);
+    Error *local_err = NULL;
+    int fd = inet_listen(address_and_port, NULL, 0, SOCK_STREAM, 0, &local_err);
+
+    if (local_err != NULL) {
+        qerror_report_err(local_err);
+        error_free(local_err);
+    }
+    return fd;
 }
 
 int unix_socket_incoming(const char *path)
 {
-    char *ostr = NULL;
-    int olen = 0;
+    Error *local_err = NULL;
+    int fd = unix_listen(path, NULL, 0, &local_err);
 
-    return unix_listen(path, ostr, olen, NULL);
+    if (local_err != NULL) {
+        qerror_report_err(local_err);
+        error_free(local_err);
+    }
+    return fd;
 }
 
 int unix_socket_outgoing(const char *path)
 {
-    return unix_connect(path, NULL);
+    Error *local_err = NULL;
+    int fd = unix_connect(path, &local_err);
+
+    if (local_err != NULL) {
+        qerror_report_err(local_err);
+        error_free(local_err);
+    }
+    return fd;
 }
 
 /* Basic flow for negotiation
-- 
1.7.12.1

[Qemu-devel] [PATCH 12/29] qemu-ga: ask and print error information from qemu-sockets

[Paolo Bonzini]

Reviewed-by: Luiz Capitulino <lcapitulino@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qga/channel-posix.c | 8 +++++---
 1 file modificato, 5 inserzioni(+), 3 rimozioni(-)

diff --git a/qga/channel-posix.c b/qga/channel-posix.c
index e22eee6..d152827 100644
--- a/qga/channel-posix.c
+++ b/qga/channel-posix.c
@@ -181,9 +181,11 @@ static gboolean ga_channel_open(GAChannel *c, const gchar *path, GAChannelMethod
         break;
     }
     case GA_CHANNEL_UNIX_LISTEN: {
-        int fd = unix_listen(path, NULL, strlen(path), NULL);
-        if (fd == -1) {
-            g_critical("error opening path: %s", strerror(errno));
+        Error *local_err = NULL;
+        int fd = unix_listen(path, NULL, strlen(path), &local_err);
+        if (local_err != NULL) {
+            g_critical("%s", error_get_pretty(local_err));
+            error_free(local_err);
             return false;
         }
         ga_channel_listen_add(c, fd, true);
-- 
1.7.12.1

[Qemu-devel] [PATCH 13/29] vnc: avoid Yoda conditionals

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 ui/vnc.c | 4 ++--
 1 file modificato, 2 inserzioni(+), 2 rimozioni(-)

diff --git a/ui/vnc.c b/ui/vnc.c
index b893e07..2892fb7 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -3066,7 +3066,7 @@ int vnc_display_open(DisplayState *ds, const char *display)
             vs->lsock = unix_connect(display+5, NULL);
         else
             vs->lsock = inet_connect(display, NULL);
-        if (-1 == vs->lsock) {
+        if (vs->lsock < 0) {
             g_free(vs->display);
             vs->display = NULL;
             return -1;
@@ -3088,7 +3088,7 @@ int vnc_display_open(DisplayState *ds, const char *display)
             vs->lsock = inet_listen(display, dpy, 256,
                                     SOCK_STREAM, 5900, NULL);
         }
-        if (-1 == vs->lsock) {
+        if (vs->lsock < 0) {
             g_free(dpy);
             return -1;
         } else {
-- 
1.7.12.1

[Qemu-devel] [PATCH 14/29] vnc: introduce a single label for error returns

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 ui/vnc.c | 42 +++++++++++++++++-------------------------
 1 file modificato, 17 inserzioni(+), 25 rimozioni(-)

diff --git a/ui/vnc.c b/ui/vnc.c
index 2892fb7..6da265f 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -2872,8 +2872,7 @@ int vnc_display_open(DisplayState *ds, const char *display)
     if (strcmp(display, "none") == 0)
         return 0;
 
-    if (!(vs->display = strdup(display)))
-        return -1;
+    vs->display = g_strdup(display);
     vs->share_policy = VNC_SHARE_POLICY_ALLOW_EXCLUSIVE;
 
     options = display;
@@ -2885,9 +2884,7 @@ int vnc_display_open(DisplayState *ds, const char *display)
                         "VNC password auth disabled due to FIPS mode, "
                         "consider using the VeNCrypt or SASL authentication "
                         "methods as an alternative\n");
-                g_free(vs->display);
-                vs->display = NULL;
-                return -1;
+                goto fail;
             }
             password = 1; /* Require password auth */
         } else if (strncmp(options, "reverse", 7) == 0) {
@@ -2919,16 +2916,12 @@ int vnc_display_open(DisplayState *ds, const char *display)
                 if (vnc_tls_set_x509_creds_dir(vs, path) < 0) {
                     fprintf(stderr, "Failed to find x509 certificates/keys in %s\n", path);
                     g_free(path);
-                    g_free(vs->display);
-                    vs->display = NULL;
-                    return -1;
+                    goto fail;
                 }
                 g_free(path);
             } else {
                 fprintf(stderr, "No certificate path provided\n");
-                g_free(vs->display);
-                vs->display = NULL;
-                return -1;
+                goto fail;
             }
 #endif
 #if defined(CONFIG_VNC_TLS) || defined(CONFIG_VNC_SASL)
@@ -2948,9 +2941,7 @@ int vnc_display_open(DisplayState *ds, const char *display)
                 vs->share_policy = VNC_SHARE_POLICY_FORCE_SHARED;
             } else {
                 fprintf(stderr, "unknown vnc share= option\n");
-                g_free(vs->display);
-                vs->display = NULL;
-                return -1;
+                goto fail;
             }
         }
     }
@@ -3053,9 +3044,7 @@ int vnc_display_open(DisplayState *ds, const char *display)
     if ((saslErr = sasl_server_init(NULL, "qemu")) != SASL_OK) {
         fprintf(stderr, "Failed to initialize SASL auth %s",
                 sasl_errstring(saslErr, NULL, NULL));
-        g_free(vs->display);
-        vs->display = NULL;
-        return -1;
+        goto fail;
     }
 #endif
     vs->lock_key_sync = lock_key_sync;
@@ -3067,9 +3056,7 @@ int vnc_display_open(DisplayState *ds, const char *display)
         else
             vs->lsock = inet_connect(display, NULL);
         if (vs->lsock < 0) {
-            g_free(vs->display);
-            vs->display = NULL;
-            return -1;
+            goto fail;
         } else {
             int csock = vs->lsock;
             vs->lsock = -1;
@@ -3090,13 +3077,18 @@ int vnc_display_open(DisplayState *ds, const char *display)
         }
         if (vs->lsock < 0) {
             g_free(dpy);
-            return -1;
-        } else {
-            g_free(vs->display);
-            vs->display = dpy;
+            goto fail;
         }
+        g_free(vs->display);
+        vs->display = dpy;
+        qemu_set_fd_handler2(vs->lsock, NULL, vnc_listen_read, NULL, vs);
     }
-    return qemu_set_fd_handler2(vs->lsock, NULL, vnc_listen_read, NULL, vs);
+    return 0;
+
+fail:
+    g_free(vs->display);
+    vs->display = NULL;
+    return -1;
 }
 
 void vnc_display_add_client(DisplayState *ds, int csock, int skipauth)
-- 
1.7.12.1

[Qemu-devel] [PATCH 15/29] vnc: reorganize code for reverse mode

[Paolo Bonzini]

Avoid the dance between csock and vs->lsock.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 console.h |  2 +-
 qmp.c     |  6 ++----
 ui/vnc.c  | 20 +++++++++-----------
 3 file modificati, 12 inserzioni(+), 16 rimozioni(-)

diff --git a/console.h b/console.h
index f990684..6099d8d 100644
--- a/console.h
+++ b/console.h
@@ -378,7 +378,7 @@ void cocoa_display_init(DisplayState *ds, int full_screen);
 /* vnc.c */
 void vnc_display_init(DisplayState *ds);
 void vnc_display_close(DisplayState *ds);
-int vnc_display_open(DisplayState *ds, const char *display);
+void vnc_display_open(DisplayState *ds, const char *display, Error **errp);
 void vnc_display_add_client(DisplayState *ds, int csock, int skipauth);
 int vnc_display_disable_login(DisplayState *ds);
 char *vnc_display_local_addr(DisplayState *ds);
diff --git a/qmp.c b/qmp.c
index 36c54c5..31bc3bf 100644
--- a/qmp.c
+++ b/qmp.c
@@ -349,11 +349,9 @@ void qmp_change_vnc_password(const char *password, Error **errp)
     }
 }
 
-static void qmp_change_vnc_listen(const char *target, Error **err)
+static void qmp_change_vnc_listen(const char *target, Error **errp)
 {
-    if (vnc_display_open(NULL, target) < 0) {
-        error_set(err, QERR_VNC_SERVER_FAILED, target);
-    }
+    vnc_display_open(NULL, target, errp);
 }
 
 static void qmp_change_vnc(const char *target, bool has_arg, const char *arg,
diff --git a/ui/vnc.c b/ui/vnc.c
index 6da265f..1989620 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -3051,19 +3051,17 @@ int vnc_display_open(DisplayState *ds, const char *display)
 
     if (reverse) {
         /* connect to viewer */
-        if (strncmp(display, "unix:", 5) == 0)
-            vs->lsock = unix_connect(display+5, NULL);
-        else
-            vs->lsock = inet_connect(display, NULL);
-        if (vs->lsock < 0) {
-            goto fail;
+        int csock;
+        vs->lsock = -1;
+        if (strncmp(display, "unix:", 5) == 0) {
+            csock = unix_connect(display+5, NULL);
         } else {
-            int csock = vs->lsock;
-            vs->lsock = -1;
-            vnc_connect(vs, csock, 0);
+            csock = inet_connect(display, NULL);
         }
-        return 0;
-
+        if (csock < 0) {
+            goto fail;
+        }
+        vnc_connect(vs, csock, 0);
     } else {
         /* listen for connects */
         char *dpy;
-- 
1.7.12.1

[Qemu-devel] [PATCH 16/29] vnc: add error propagation to vnc_display_open

[Paolo Bonzini]

Before:

    $ qemu-system-x86_64 -vnc foo.bar:12345
    getaddrinfo(foo.bar,18245): Name or service not known
    Failed to start VNC server on `foo.bar:12345'

    $ qemu-system-x86_64 -vnc localhost:12345,reverse=on
    inet_connect_opts: connect(ipv4,yakj.usersys.redhat.com,127.0.0.1,12345): Connection refused
    Failed to start VNC server on `localhost:12345,reverse=on'

After:

    $ x86_64-softmmu/qemu-system-x86_64 -vnc foo.bar:12345
    Failed to start VNC server on `foo.bar:12345': address resolution failed for foo.bar:18245: Name or service not known

    $ x86_64-softmmu/qemu-system-x86_64 -vnc localhost:12345,reverse=on
    Failed to start VNC server on `localhost:12345,reverse=on': Failed to connect to socket: Connection refused

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 ui/vnc.c | 42 +++++++++++++++++++++++-------------------
 vl.c     |  9 ++++++---
 2 file modificati, 29 inserzioni(+), 22 rimozioni(-)

diff --git a/ui/vnc.c b/ui/vnc.c
index 1989620..ce7f5b7 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -2848,7 +2848,7 @@ char *vnc_display_local_addr(DisplayState *ds)
     return vnc_socket_local_addr("%s:%s", vs->lsock);
 }
 
-int vnc_display_open(DisplayState *ds, const char *display)
+void vnc_display_open(DisplayState *ds, const char *display, Error **errp)
 {
     VncDisplay *vs = ds ? (VncDisplay *)ds->opaque : vnc_display;
     const char *options;
@@ -2866,11 +2866,13 @@ int vnc_display_open(DisplayState *ds, const char *display)
 #endif
     int lock_key_sync = 1;
 
-    if (!vnc_display)
-        return -1;
+    if (!vnc_display) {
+        error_setg(errp, "VNC display not active");
+        return;
+    }
     vnc_display_close(ds);
     if (strcmp(display, "none") == 0)
-        return 0;
+        return;
 
     vs->display = g_strdup(display);
     vs->share_policy = VNC_SHARE_POLICY_ALLOW_EXCLUSIVE;
@@ -2880,10 +2882,10 @@ int vnc_display_open(DisplayState *ds, const char *display)
         options++;
         if (strncmp(options, "password", 8) == 0) {
             if (fips_get_state()) {
-                fprintf(stderr,
-                        "VNC password auth disabled due to FIPS mode, "
-                        "consider using the VeNCrypt or SASL authentication "
-                        "methods as an alternative\n");
+                error_setg(errp,
+                           "VNC password auth disabled due to FIPS mode, "
+                           "consider using the VeNCrypt or SASL authentication "
+                           "methods as an alternative");
                 goto fail;
             }
             password = 1; /* Require password auth */
@@ -2914,13 +2916,13 @@ int vnc_display_open(DisplayState *ds, const char *display)
 
                 VNC_DEBUG("Trying certificate path '%s'\n", path);
                 if (vnc_tls_set_x509_creds_dir(vs, path) < 0) {
-                    fprintf(stderr, "Failed to find x509 certificates/keys in %s\n", path);
+                    error_setg(errp, "Failed to find x509 certificates/keys in %s", path);
                     g_free(path);
                     goto fail;
                 }
                 g_free(path);
             } else {
-                fprintf(stderr, "No certificate path provided\n");
+                error_setg(errp, "No certificate path provided");
                 goto fail;
             }
 #endif
@@ -2940,7 +2942,7 @@ int vnc_display_open(DisplayState *ds, const char *display)
             } else if (strncmp(options+6, "force-shared", 12) == 0) {
                 vs->share_policy = VNC_SHARE_POLICY_FORCE_SHARED;
             } else {
-                fprintf(stderr, "unknown vnc share= option\n");
+                error_setg(errp, "unknown vnc share= option");
                 goto fail;
             }
         }
@@ -3042,8 +3044,8 @@ int vnc_display_open(DisplayState *ds, const char *display)
 
 #ifdef CONFIG_VNC_SASL
     if ((saslErr = sasl_server_init(NULL, "qemu")) != SASL_OK) {
-        fprintf(stderr, "Failed to initialize SASL auth %s",
-                sasl_errstring(saslErr, NULL, NULL));
+        error_setg(errp, "Failed to initialize SASL auth: %s",
+                   sasl_errstring(saslErr, NULL, NULL));
         goto fail;
     }
 #endif
@@ -3054,9 +3056,9 @@ int vnc_display_open(DisplayState *ds, const char *display)
         int csock;
         vs->lsock = -1;
         if (strncmp(display, "unix:", 5) == 0) {
-            csock = unix_connect(display+5, NULL);
+            csock = unix_connect(display+5, errp);
         } else {
-            csock = inet_connect(display, NULL);
+            csock = inet_connect(display, errp);
         }
         if (csock < 0) {
             goto fail;
@@ -3068,10 +3070,10 @@ int vnc_display_open(DisplayState *ds, const char *display)
         dpy = g_malloc(256);
         if (strncmp(display, "unix:", 5) == 0) {
             pstrcpy(dpy, 256, "unix:");
-            vs->lsock = unix_listen(display+5, dpy+5, 256-5, NULL);
+            vs->lsock = unix_listen(display+5, dpy+5, 256-5, errp);
         } else {
             vs->lsock = inet_listen(display, dpy, 256,
-                                    SOCK_STREAM, 5900, NULL);
+                                    SOCK_STREAM, 5900, errp);
         }
         if (vs->lsock < 0) {
             g_free(dpy);
@@ -3081,12 +3083,14 @@ int vnc_display_open(DisplayState *ds, const char *display)
         vs->display = dpy;
         qemu_set_fd_handler2(vs->lsock, NULL, vnc_listen_read, NULL, vs);
     }
-    return 0;
+    return;
 
 fail:
+    if (!error_is_set(errp)) {
+        error_set(errp, QERR_VNC_SERVER_FAILED, display);
+    }
     g_free(vs->display);
     vs->display = NULL;
-    return -1;
 }
 
 void vnc_display_add_client(DisplayState *ds, int csock, int skipauth)
diff --git a/vl.c b/vl.c
index 2a072e8..67a624f 100644
--- a/vl.c
+++ b/vl.c
@@ -3706,10 +3706,13 @@ int main(int argc, char **argv, char **envp)
 #ifdef CONFIG_VNC
     /* init remote displays */
     if (vnc_display) {
+        Error *local_err = NULL;
         vnc_display_init(ds);
-        if (vnc_display_open(ds, vnc_display) < 0) {
-            fprintf(stderr, "Failed to start VNC server on `%s'\n",
-                    vnc_display);
+        vnc_display_open(ds, vnc_display, &local_err);
+        if (local_err != NULL) {
+            fprintf(stderr, "Failed to start VNC server on `%s': %s\n",
+                    vnc_display, error_get_pretty(local_err));
+            error_free(local_err);
             exit(1);
         }
 
-- 
1.7.12.1

[Qemu-devel] [PATCH 17/29] qemu-sockets: include strerror or gai_strerror output in error messages

[Paolo Bonzini]

Among others, before:

    $ qemu-system-x86_64 -chardev socket,port=12345,id=char
    inet_connect: host and/or port not specified
    chardev: opening backend "socket" failed

After:

    $ x86_64-softmmu/qemu-system-x86_64 -chardev socket,port=12345,id=char
    qemu-system-x86_64: -chardev socket,port=12345,id=char: host and/or port not specified
    chardev: opening backend "socket" failed

perror and fprintf can be removed because all clients can now
consume Errors properly.

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qemu-sockets.c | 30 +++++++++---------------------
 1 file modificato, 9 inserzioni(+), 21 rimozioni(-)

diff --git a/qemu-sockets.c b/qemu-sockets.c
index d510ad1..9e3d233 100644
--- a/qemu-sockets.c
+++ b/qemu-sockets.c
@@ -120,8 +120,7 @@ int inet_listen_opts(QemuOpts *opts, int port_offset, Error **errp)
 
     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);
+        error_setg(errp, "host and/or port not specified");
         return -1;
     }
     pstrcpy(port, sizeof(port), qemu_opt_get(opts, "port"));
@@ -138,9 +137,8 @@ int inet_listen_opts(QemuOpts *opts, int port_offset, Error **errp)
         snprintf(port, sizeof(port), "%d", atoi(port) + port_offset);
     rc = getaddrinfo(strlen(addr) ? addr : NULL, port, &ai, &res);
     if (rc != 0) {
-        fprintf(stderr,"getaddrinfo(%s,%s): %s\n", addr, port,
-                gai_strerror(rc));
-        error_set(errp, QERR_SOCKET_CREATE_FAILED);
+        error_setg(errp, "address resolution failed for %s:%s: %s", addr, port,
+                   gai_strerror(rc));
         return -1;
     }
 
@@ -151,10 +149,8 @@ int inet_listen_opts(QemuOpts *opts, int port_offset, Error **errp)
 		        NI_NUMERICHOST | NI_NUMERICSERV);
         slisten = qemu_socket(e->ai_family, e->ai_socktype, e->ai_protocol);
         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);
+                error_set_errno(errp, errno, QERR_SOCKET_CREATE_FAILED);
             }
             continue;
         }
@@ -176,24 +172,19 @@ int inet_listen_opts(QemuOpts *opts, int port_offset, Error **errp)
                 goto listen;
             }
             if (p == port_max) {
-                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);
+                    error_set_errno(errp, errno, QERR_SOCKET_BIND_FAILED);
                 }
             }
         }
         closesocket(slisten);
     }
-    fprintf(stderr, "%s: FAILED\n", __FUNCTION__);
     freeaddrinfo(res);
     return -1;
 
 listen:
     if (listen(slisten,1) != 0) {
-        error_set(errp, QERR_SOCKET_LISTEN_FAILED);
-        perror("listen");
+        error_set_errno(errp, errno, QERR_SOCKET_LISTEN_FAILED);
         closesocket(slisten);
         freeaddrinfo(res);
         return -1;
@@ -324,9 +315,7 @@ static struct addrinfo *inet_parse_connect_opts(QemuOpts *opts, Error **errp)
     addr = qemu_opt_get(opts, "host");
     port = qemu_opt_get(opts, "port");
     if (addr == NULL || port == NULL) {
-        fprintf(stderr,
-                "inet_parse_connect_opts: host and/or port not specified\n");
-        error_set(errp, QERR_SOCKET_CREATE_FAILED);
+        error_setg(errp, "host and/or port not specified");
         return NULL;
     }
 
@@ -340,9 +329,8 @@ static struct addrinfo *inet_parse_connect_opts(QemuOpts *opts, Error **errp)
     /* lookup */
     rc = getaddrinfo(addr, port, &ai, &res);
     if (rc != 0) {
-        fprintf(stderr, "getaddrinfo(%s,%s): %s\n", addr, port,
-                gai_strerror(rc));
-        error_set(errp, QERR_SOCKET_CREATE_FAILED);
+        error_setg(errp, "address resolution failed for %s:%s: %s", addr, port,
+                   gai_strerror(rc));
         return NULL;
     }
     return res;
-- 
1.7.12.1

[Qemu-devel] [PATCH 18/29] qemu-sockets: add error propagation to inet_connect_addr

[Paolo Bonzini]

perror and fprintf can be removed because all clients can now consume
Errors properly.  However, we'll need to change the non-blocking connect
handlers to take an Error, in order to improve error handling for
migration with the TCP protocol.

This is a minor degradation in error reporting for outgoing migration.
However, until 1.2 this case just failed without even attempting to
connect, so it is still an improvement as far as overall QoI is
concerned.

Reviewed-by: Luiz Capitulino <lcapitulino@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qemu-sockets.c | 15 ++++++---------
 1 file modificato, 6 inserzioni(+), 9 rimozioni(-)

diff --git a/qemu-sockets.c b/qemu-sockets.c
index 9e3d233..88d58fe 100644
--- a/qemu-sockets.c
+++ b/qemu-sockets.c
@@ -216,7 +216,7 @@ typedef struct ConnectState {
 } ConnectState;
 
 static int inet_connect_addr(struct addrinfo *addr, bool *in_progress,
-                             ConnectState *connect_state);
+                             ConnectState *connect_state, Error **errp);
 
 static void wait_for_connect(void *opaque)
 {
@@ -246,7 +246,7 @@ static void wait_for_connect(void *opaque)
     if (s->current_addr) {
         while (s->current_addr->ai_next != NULL && s->fd < 0) {
             s->current_addr = s->current_addr->ai_next;
-            s->fd = inet_connect_addr(s->current_addr, &in_progress, s);
+            s->fd = inet_connect_addr(s->current_addr, &in_progress, s, NULL);
             /* connect in progress */
             if (in_progress) {
                 return;
@@ -263,7 +263,7 @@ static void wait_for_connect(void *opaque)
 }
 
 static int inet_connect_addr(struct addrinfo *addr, bool *in_progress,
-                             ConnectState *connect_state)
+                             ConnectState *connect_state, Error **errp)
 {
     int sock, rc;
 
@@ -271,8 +271,7 @@ static int inet_connect_addr(struct addrinfo *addr, bool *in_progress,
 
     sock = qemu_socket(addr->ai_family, addr->ai_socktype, addr->ai_protocol);
     if (sock < 0) {
-        fprintf(stderr, "%s: socket(%s): %s\n", __func__,
-                inet_strfamily(addr->ai_family), strerror(errno));
+        error_set_errno(errp, errno, QERR_SOCKET_CREATE_FAILED);
         return -1;
     }
     qemu_setsockopt(sock, SOL_SOCKET, SO_REUSEADDR, &on, sizeof(on));
@@ -293,6 +292,7 @@ static int inet_connect_addr(struct addrinfo *addr, bool *in_progress,
                              connect_state);
         *in_progress = true;
     } else if (rc < 0) {
+        error_set_errno(errp, errno, QERR_SOCKET_CONNECT_FAILED);
         closesocket(sock);
         return -1;
     }
@@ -375,7 +375,7 @@ int inet_connect_opts(QemuOpts *opts, Error **errp,
         if (connect_state != NULL) {
             connect_state->current_addr = e;
         }
-        sock = inet_connect_addr(e, &in_progress, connect_state);
+        sock = inet_connect_addr(e, &in_progress, connect_state, errp);
         if (in_progress) {
             return sock;
         } else if (sock >= 0) {
@@ -386,9 +386,6 @@ int inet_connect_opts(QemuOpts *opts, Error **errp,
             break;
         }
     }
-    if (sock < 0) {
-        error_set(errp, QERR_SOCKET_CONNECT_FAILED);
-    }
     g_free(connect_state);
     freeaddrinfo(res);
     return sock;
-- 
1.7.12.1

[Qemu-devel] [PATCH 19/29] qemu-sockets: add error propagation to inet_dgram_opts

[Paolo Bonzini]

Before:

    $ qemu-system-x86_64 -monitor udp:localhost:631@localhost:631
    inet_dgram_opts: bind(ipv4,127.0.0.1,631): OK
    inet_dgram_opts failed
    chardev: opening backend "udp" failed

After:

    $ x86_64-softmmu/qemu-system-x86_64 -monitor udp:localhost:631@localhost:631
    qemu-system-x86_64: -monitor udp:localhost:631@localhost:631: Failed to bind socket: Address already in use
    chardev: opening backend "udp" failed

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qemu-char.c    |  1 -
 qemu-sockets.c | 34 ++++++++--------------------------
 2 file modificati, 8 inserzioni(+), 27 rimozioni(-)

diff --git a/qemu-char.c b/qemu-char.c
index 04b5c23..afe2bfb 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2105,7 +2105,6 @@ static CharDriverState *qemu_chr_open_udp(QemuOpts *opts)
 
     fd = inet_dgram_opts(opts, &local_err);
     if (fd < 0) {
-        fprintf(stderr, "inet_dgram_opts failed\n");
         goto return_err;
     }
 
diff --git a/qemu-sockets.c b/qemu-sockets.c
index 88d58fe..078f7c1 100644
--- a/qemu-sockets.c
+++ b/qemu-sockets.c
@@ -396,8 +396,6 @@ int inet_dgram_opts(QemuOpts *opts, Error **errp)
     struct addrinfo ai, *peer = NULL, *local = NULL;
     const char *addr;
     const char *port;
-    char uaddr[INET6_ADDRSTRLEN+1];
-    char uport[33];
     int sock = -1, rc;
 
     /* lookup peer addr */
@@ -412,7 +410,7 @@ int inet_dgram_opts(QemuOpts *opts, Error **errp)
         addr = "localhost";
     }
     if (port == NULL || strlen(port) == 0) {
-        fprintf(stderr, "inet_dgram: port not specified\n");
+        error_setg(errp, "remote port not specified");
         return -1;
     }
 
@@ -422,8 +420,8 @@ int inet_dgram_opts(QemuOpts *opts, Error **errp)
         ai.ai_family = PF_INET6;
 
     if (0 != (rc = getaddrinfo(addr, port, &ai, &peer))) {
-        fprintf(stderr,"getaddrinfo(%s,%s): %s\n", addr, port,
-                gai_strerror(rc));
+        error_setg(errp, "address resolution failed for %s:%s: %s", addr, port,
+                   gai_strerror(rc));
 	return -1;
     }
 
@@ -442,44 +440,28 @@ int inet_dgram_opts(QemuOpts *opts, Error **errp)
         port = "0";
 
     if (0 != (rc = getaddrinfo(addr, port, &ai, &local))) {
-        fprintf(stderr,"getaddrinfo(%s,%s): %s\n", addr, port,
-                gai_strerror(rc));
+        error_setg(errp, "address resolution failed for %s:%s: %s", addr, port,
+                   gai_strerror(rc));
         goto err;
     }
 
     /* create socket */
     sock = qemu_socket(peer->ai_family, peer->ai_socktype, peer->ai_protocol);
     if (sock < 0) {
-        fprintf(stderr,"%s: socket(%s): %s\n", __FUNCTION__,
-                inet_strfamily(peer->ai_family), strerror(errno));
+        error_set_errno(errp, errno, QERR_SOCKET_CREATE_FAILED);
         goto err;
     }
     setsockopt(sock,SOL_SOCKET,SO_REUSEADDR,(void*)&on,sizeof(on));
 
     /* bind socket */
-    if (getnameinfo((struct sockaddr*)local->ai_addr,local->ai_addrlen,
-                    uaddr,INET6_ADDRSTRLEN,uport,32,
-                    NI_NUMERICHOST | NI_NUMERICSERV) != 0) {
-        fprintf(stderr, "%s: getnameinfo: oops\n", __FUNCTION__);
-        goto err;
-    }
     if (bind(sock, local->ai_addr, local->ai_addrlen) < 0) {
-        fprintf(stderr,"%s: bind(%s,%s,%d): OK\n", __FUNCTION__,
-                inet_strfamily(local->ai_family), uaddr, inet_getport(local));
+        error_set_errno(errp, errno, QERR_SOCKET_BIND_FAILED);
         goto err;
     }
 
     /* connect to peer */
-    if (getnameinfo((struct sockaddr*)peer->ai_addr, peer->ai_addrlen,
-                    uaddr, INET6_ADDRSTRLEN, uport, 32,
-                    NI_NUMERICHOST | NI_NUMERICSERV) != 0) {
-        fprintf(stderr, "%s: getnameinfo: oops\n", __FUNCTION__);
-        goto err;
-    }
     if (connect(sock,peer->ai_addr,peer->ai_addrlen) < 0) {
-        fprintf(stderr, "%s: connect(%s,%s,%s,%s): %s\n", __FUNCTION__,
-                inet_strfamily(peer->ai_family),
-                peer->ai_canonname, uaddr, uport, strerror(errno));
+        error_set_errno(errp, errno, QERR_SOCKET_CONNECT_FAILED);
         goto err;
     }
 
-- 
1.7.12.1

[Qemu-devel] [PATCH 20/29] qemu-sockets: add error propagation to inet_parse

[Paolo Bonzini]

Reviewed-by: Luiz Capitulino <lcapitulino@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qemu-sockets.c | 41 +++++++++++++++++++++--------------------
 1 file modificato, 21 inserzioni(+), 20 rimozioni(-)

diff --git a/qemu-sockets.c b/qemu-sockets.c
index 078f7c1..fb9c4c9 100644
--- a/qemu-sockets.c
+++ b/qemu-sockets.c
@@ -480,7 +480,7 @@ err:
 }
 
 /* compatibility wrapper */
-static int inet_parse(QemuOpts *opts, const char *str)
+static void inet_parse(QemuOpts *opts, const char *str, Error **errp)
 {
     const char *optstr, *h;
     char addr[64];
@@ -492,32 +492,28 @@ static int inet_parse(QemuOpts *opts, const char *str)
         /* no host given */
         addr[0] = '\0';
         if (1 != sscanf(str,":%32[^,]%n",port,&pos)) {
-            fprintf(stderr, "%s: portonly parse error (%s)\n",
-                    __FUNCTION__, str);
-            return -1;
+            error_setg(errp, "error parsing port in address '%s'", str);
+            return;
         }
     } else if (str[0] == '[') {
         /* IPv6 addr */
         if (2 != sscanf(str,"[%64[^]]]:%32[^,]%n",addr,port,&pos)) {
-            fprintf(stderr, "%s: ipv6 parse error (%s)\n",
-                    __FUNCTION__, str);
-            return -1;
+            error_setg(errp, "error parsing IPv6 address '%s'", str);
+            return;
         }
         qemu_opt_set(opts, "ipv6", "on");
     } else if (qemu_isdigit(str[0])) {
         /* IPv4 addr */
         if (2 != sscanf(str,"%64[0-9.]:%32[^,]%n",addr,port,&pos)) {
-            fprintf(stderr, "%s: ipv4 parse error (%s)\n",
-                    __FUNCTION__, str);
-            return -1;
+            error_setg(errp, "error parsing IPv4 address '%s'", str);
+            return;
         }
         qemu_opt_set(opts, "ipv4", "on");
     } else {
         /* hostname */
         if (2 != sscanf(str,"%64[^:]:%32[^,]%n",addr,port,&pos)) {
-            fprintf(stderr, "%s: hostname parse error (%s)\n",
-                    __FUNCTION__, str);
-            return -1;
+            error_setg(errp, "error parsing address '%s'", str);
+            return;
         }
     }
     qemu_opt_set(opts, "host", addr);
@@ -532,7 +528,6 @@ static int inet_parse(QemuOpts *opts, const char *str)
         qemu_opt_set(opts, "ipv4", "on");
     if (strstr(optstr, ",ipv6"))
         qemu_opt_set(opts, "ipv6", "on");
-    return 0;
 }
 
 int inet_listen(const char *str, char *ostr, int olen,
@@ -541,9 +536,11 @@ int inet_listen(const char *str, char *ostr, int olen,
     QemuOpts *opts;
     char *optstr;
     int sock = -1;
+    Error *local_err = NULL;
 
     opts = qemu_opts_create(&dummy_opts, NULL, 0, NULL);
-    if (inet_parse(opts, str) == 0) {
+    inet_parse(opts, str, &local_err);
+    if (local_err == NULL) {
         sock = inet_listen_opts(opts, port_offset, errp);
         if (sock != -1 && ostr) {
             optstr = strchr(str, ',');
@@ -560,7 +557,7 @@ int inet_listen(const char *str, char *ostr, int olen,
             }
         }
     } else {
-        error_set(errp, QERR_SOCKET_CREATE_FAILED);
+        error_propagate(errp, local_err);
     }
     qemu_opts_del(opts);
     return sock;
@@ -578,12 +575,14 @@ int inet_connect(const char *str, Error **errp)
 {
     QemuOpts *opts;
     int sock = -1;
+    Error *local_err = NULL;
 
     opts = qemu_opts_create(&dummy_opts, NULL, 0, NULL);
-    if (inet_parse(opts, str) == 0) {
+    inet_parse(opts, str, &local_err);
+    if (local_err == NULL) {
         sock = inet_connect_opts(opts, errp, NULL, NULL);
     } else {
-        error_set(errp, QERR_SOCKET_CREATE_FAILED);
+        error_propagate(errp, local_err);
     }
     qemu_opts_del(opts);
     return sock;
@@ -608,14 +607,16 @@ int inet_nonblocking_connect(const char *str,
 {
     QemuOpts *opts;
     int sock = -1;
+    Error *local_err = NULL;
 
     g_assert(callback != NULL);
 
     opts = qemu_opts_create(&dummy_opts, NULL, 0, NULL);
-    if (inet_parse(opts, str) == 0) {
+    inet_parse(opts, str, &local_err);
+    if (local_err == NULL) {
         sock = inet_connect_opts(opts, errp, callback, opaque);
     } else {
-        error_set(errp, QERR_SOCKET_CREATE_FAILED);
+        error_propagate(errp, local_err);
     }
     qemu_opts_del(opts);
     return sock;
-- 
1.7.12.1

[Qemu-devel] [PATCH 21/29] qemu-sockets: add error propagation to Unix socket functions

[Paolo Bonzini]

Before:

    $ qemu-system-x86_64 -monitor unix:/vvv,server=off
    connect(unix:/vvv): No such file or directory
    chardev: opening backend "socket" failed

After:

    $ x86_64-softmmu/qemu-system-x86_64 -monitor unix:/vvv,server=off
    qemu-system-x86_64: -monitor unix:/vvv,server=off: Failed to connect to socket: No such file or directory
    chardev: opening backend "socket" failed

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qemu-sockets.c | 16 ++++++++--------
 1 file modificato, 8 inserzioni(+), 8 rimozioni(-)

diff --git a/qemu-sockets.c b/qemu-sockets.c
index fb9c4c9..daff8e6 100644
--- a/qemu-sockets.c
+++ b/qemu-sockets.c
@@ -632,7 +632,7 @@ int unix_listen_opts(QemuOpts *opts, Error **errp)
 
     sock = qemu_socket(PF_UNIX, SOCK_STREAM, 0);
     if (sock < 0) {
-        perror("socket(unix)");
+        error_set_errno(errp, errno, QERR_SOCKET_CREATE_FAILED);
         return -1;
     }
 
@@ -657,11 +657,11 @@ int unix_listen_opts(QemuOpts *opts, Error **errp)
 
     unlink(un.sun_path);
     if (bind(sock, (struct sockaddr*) &un, sizeof(un)) < 0) {
-        fprintf(stderr, "bind(unix:%s): %s\n", un.sun_path, strerror(errno));
+        error_set_errno(errp, errno, QERR_SOCKET_BIND_FAILED);
         goto err;
     }
     if (listen(sock, 1) < 0) {
-        fprintf(stderr, "listen(unix:%s): %s\n", un.sun_path, strerror(errno));
+        error_set_errno(errp, errno, QERR_SOCKET_LISTEN_FAILED);
         goto err;
     }
 
@@ -681,13 +681,13 @@ int unix_connect_opts(QemuOpts *opts, Error **errp,
     int sock, rc;
 
     if (NULL == path) {
-        fprintf(stderr, "unix connect: no path specified\n");
+        error_setg(errp, "unix connect: no path specified\n");
         return -1;
     }
 
     sock = qemu_socket(PF_UNIX, SOCK_STREAM, 0);
     if (sock < 0) {
-        perror("socket(unix)");
+        error_set_errno(errp, errno, QERR_SOCKET_CREATE_FAILED);
         return -1;
     }
     if (callback != NULL) {
@@ -722,7 +722,7 @@ int unix_connect_opts(QemuOpts *opts, Error **errp,
     }
 
     if (rc < 0) {
-        fprintf(stderr, "connect(unix:%s): %s\n", path, strerror(errno));
+        error_set_errno(errp, -rc, QERR_SOCKET_CONNECT_FAILED);
         close(sock);
         sock = -1;
     }
@@ -735,7 +735,7 @@ int unix_connect_opts(QemuOpts *opts, Error **errp,
 
 int unix_listen_opts(QemuOpts *opts, Error **errp)
 {
-    fprintf(stderr, "unix sockets are not available on windows\n");
+    error_setg(errp, "unix sockets are not available on windows");
     errno = ENOTSUP;
     return -1;
 }
@@ -743,7 +743,7 @@ int unix_listen_opts(QemuOpts *opts, Error **errp)
 int unix_connect_opts(QemuOpts *opts, Error **errp,
                       NonBlockingConnectHandler *callback, void *opaque)
 {
-    fprintf(stderr, "unix sockets are not available on windows\n");
+    error_setg(errp, "unix sockets are not available on windows");
     errno = ENOTSUP;
     return -1;
 }
-- 
1.7.12.1

[Qemu-devel] [PATCH 22/29] vnc: drop QERR_VNC_SERVER_FAILED

[Paolo Bonzini]

We now always return "nice" error messages in errp when we goto fail.
Drop the default error message.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qerror.h | 3 ---
 ui/vnc.c | 3 ---
 2 file modificati, 6 rimozioni(-)

diff --git a/qerror.h b/qerror.h
index c91708c..f23b978 100644
--- a/qerror.h
+++ b/qerror.h
@@ -237,9 +237,6 @@ void assert_no_error(Error *err);
 #define QERR_VIRTFS_FEATURE_BLOCKS_MIGRATION \
     ERROR_CLASS_GENERIC_ERROR, "Migration is disabled when VirtFS export path '%s' is mounted in the guest using mount_tag '%s'"
 
-#define QERR_VNC_SERVER_FAILED \
-    ERROR_CLASS_GENERIC_ERROR, "Could not start VNC server on %s"
-
 #define QERR_SOCKET_CONNECT_FAILED \
     ERROR_CLASS_GENERIC_ERROR, "Failed to connect to socket"
 
diff --git a/ui/vnc.c b/ui/vnc.c
index ce7f5b7..79f1922 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -3086,9 +3086,6 @@ void vnc_display_open(DisplayState *ds, const char *display, Error **errp)
     return;
 
 fail:
-    if (!error_is_set(errp)) {
-        error_set(errp, QERR_VNC_SERVER_FAILED, display);
-    }
     g_free(vs->display);
     vs->display = NULL;
 }
-- 
1.7.12.1

[Qemu-devel] [PATCH 23/29] build: add QAPI files to the tools

[Paolo Bonzini]

We need them because qemu-sockets will soon be using SocketAddress.

Acked-by: Luiz Capitulino <lcapitulino@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 Makefile.objs | 3 ++-
 1 file modificato, 2 inserzioni(+). 1 rimozione(-)

diff --git a/Makefile.objs b/Makefile.objs
index 74b3542..3f16d67 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -48,6 +48,7 @@ block-obj-y += $(coroutine-obj-y) $(qobject-obj-y) $(version-obj-y)
 block-obj-$(CONFIG_POSIX) += posix-aio-compat.o
 block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
 block-obj-y += block/
+block-obj-y += $(qapi-obj-y) qapi-types.o qapi-visit.o
 
 ifeq ($(CONFIG_VIRTIO)$(CONFIG_VIRTFS)$(CONFIG_PCI),yyy)
 # Lots of the fsdev/9pcode is pulled in by vl.c via qemu_fsdev_add.
@@ -239,9 +240,9 @@ QEMU_CFLAGS+=$(GLIB_CFLAGS)
 
 nested-vars += \
 	qga-obj-y \
-	block-obj-y \
 	qom-obj-y \
 	qapi-obj-y \
+	block-obj-y \
 	user-obj-y \
 	common-obj-y \
 	extra-obj-y
-- 
1.7.12.1

[Qemu-devel] [PATCH 24/29] qapi: add socket address types

[Paolo Bonzini]

Acked-by: Luiz Capitulino <lcapitulino@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qapi-schema.json | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file modificato, 53 inserzioni(+)

diff --git a/qapi-schema.json b/qapi-schema.json
index f9dbdae..a92a33b 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2505,6 +2505,59 @@
     'opts': 'NetClientOptions' } }
 
 ##
+# @InetSocketAddress
+#
+# Captures a socket address or address range in the Internet namespace.
+#
+# @host: host part of the address
+#
+# @port: port part of the address, or lowest port if @to is present
+#
+# @to: highest port to try
+#
+# @ipv4: whether to accept IPv4 addresses, default try both IPv4 and IPv6
+#        #optional
+#
+# @ipv6: whether to accept IPv6 addresses, default try both IPv4 and IPv6
+#        #optional
+#
+# Since 1.3
+##
+{ 'type': 'InetSocketAddress',
+  'data': {
+    'host': 'str',
+    'port': 'str',
+    '*to': 'uint16',
+    '*ipv4': 'bool',
+    '*ipv6': 'bool' } }
+
+##
+# @UnixSocketAddress
+#
+# Captures a socket address in the local ("Unix socket") namespace.
+#
+# @path: filesystem path to use
+#
+# Since 1.3
+##
+{ 'type': 'UnixSocketAddress',
+  'data': {
+    'path': 'str' } }
+
+##
+# @SocketAddress
+#
+# Captures the address of a socket, which could also be a named file descriptor
+#
+# Since 1.3
+##
+{ 'union': 'SocketAddress',
+  'data': {
+    'inet': 'InetSocketAddress',
+    'unix': 'UnixSocketAddress',
+    'fd': 'String' } }
+
+##
 # @getfd:
 #
 # Receive a file descriptor via SCM rights and assign it a name
-- 
1.7.12.1

[Qemu-devel] [PATCH 25/29] qemu-sockets: return InetSocketAddress from inet_parse

[Paolo Bonzini]

Reviewed-by: Luiz Capitulino <lcapitulino@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qemu-sockets.c | 121 +++++++++++++++++++++++++++++++++++++--------------------
 1 file modificato, 79 inserzioni(+), 42 rimozioni(-)

diff --git a/qemu-sockets.c b/qemu-sockets.c
index daff8e6..909c65f 100644
--- a/qemu-sockets.c
+++ b/qemu-sockets.c
@@ -480,54 +480,91 @@ err:
 }
 
 /* compatibility wrapper */
-static void inet_parse(QemuOpts *opts, const char *str, Error **errp)
+static InetSocketAddress *inet_parse(const char *str, Error **errp)
 {
+    InetSocketAddress *addr;
     const char *optstr, *h;
-    char addr[64];
+    char host[64];
     char port[33];
+    int to;
     int pos;
 
+    addr = g_new0(InetSocketAddress, 1);
+
     /* parse address */
     if (str[0] == ':') {
         /* no host given */
-        addr[0] = '\0';
-        if (1 != sscanf(str,":%32[^,]%n",port,&pos)) {
+        host[0] = '\0';
+        if (1 != sscanf(str, ":%32[^,]%n", port, &pos)) {
             error_setg(errp, "error parsing port in address '%s'", str);
-            return;
+            goto fail;
         }
     } else if (str[0] == '[') {
         /* IPv6 addr */
-        if (2 != sscanf(str,"[%64[^]]]:%32[^,]%n",addr,port,&pos)) {
+        if (2 != sscanf(str, "[%64[^]]]:%32[^,]%n", host, port, &pos)) {
             error_setg(errp, "error parsing IPv6 address '%s'", str);
-            return;
+            goto fail;
         }
-        qemu_opt_set(opts, "ipv6", "on");
+        addr->ipv6 = addr->has_ipv6 = true;
     } else if (qemu_isdigit(str[0])) {
         /* IPv4 addr */
-        if (2 != sscanf(str,"%64[0-9.]:%32[^,]%n",addr,port,&pos)) {
+        if (2 != sscanf(str, "%64[0-9.]:%32[^,]%n", host, port, &pos)) {
             error_setg(errp, "error parsing IPv4 address '%s'", str);
-            return;
+            goto fail;
         }
-        qemu_opt_set(opts, "ipv4", "on");
+        addr->ipv4 = addr->has_ipv4 = true;
     } else {
         /* hostname */
-        if (2 != sscanf(str,"%64[^:]:%32[^,]%n",addr,port,&pos)) {
+        if (2 != sscanf(str, "%64[^:]:%32[^,]%n", host, port, &pos)) {
             error_setg(errp, "error parsing address '%s'", str);
-            return;
+            goto fail;
         }
     }
-    qemu_opt_set(opts, "host", addr);
-    qemu_opt_set(opts, "port", port);
+
+    addr->host = g_strdup(host);
+    addr->port = g_strdup(port);
 
     /* parse options */
     optstr = str + pos;
     h = strstr(optstr, ",to=");
-    if (h)
-        qemu_opt_set(opts, "to", h+4);
-    if (strstr(optstr, ",ipv4"))
-        qemu_opt_set(opts, "ipv4", "on");
-    if (strstr(optstr, ",ipv6"))
-        qemu_opt_set(opts, "ipv6", "on");
+    if (h) {
+        if (1 != sscanf(str, "%d%n", &to, &pos) ||
+            (str[pos] != '\0' && str[pos] != ',')) {
+            error_setg(errp, "error parsing to= argument");
+            goto fail;
+        }
+        addr->has_to = true;
+        addr->to = to;
+    }
+    if (strstr(optstr, ",ipv4")) {
+        addr->ipv4 = addr->has_ipv4 = true;
+    }
+    if (strstr(optstr, ",ipv6")) {
+        addr->ipv6 = addr->has_ipv6 = true;
+    }
+    return addr;
+
+fail:
+    qapi_free_InetSocketAddress(addr);
+    return NULL;
+}
+
+static void inet_addr_to_opts(QemuOpts *opts, InetSocketAddress *addr)
+{
+    bool ipv4 = addr->ipv4 || !addr->has_ipv4;
+    bool ipv6 = addr->ipv6 || !addr->has_ipv6;
+
+    if (!ipv4 || !ipv6) {
+        qemu_opt_set_bool(opts, "ipv4", ipv4);
+        qemu_opt_set_bool(opts, "ipv6", ipv6);
+    }
+    if (addr->has_to) {
+        char to[20];
+        snprintf(to, sizeof(to), "%d", addr->to);
+        qemu_opt_set(opts, "to", to);
+    }
+    qemu_opt_set(opts, "host", addr->host);
+    qemu_opt_set(opts, "port", addr->port);
 }
 
 int inet_listen(const char *str, char *ostr, int olen,
@@ -536,11 +573,13 @@ int inet_listen(const char *str, char *ostr, int olen,
     QemuOpts *opts;
     char *optstr;
     int sock = -1;
-    Error *local_err = NULL;
+    InetSocketAddress *addr;
 
-    opts = qemu_opts_create(&dummy_opts, NULL, 0, NULL);
-    inet_parse(opts, str, &local_err);
-    if (local_err == NULL) {
+    addr = inet_parse(str, errp);
+    if (addr != NULL) {
+        opts = qemu_opts_create(&dummy_opts, NULL, 0, NULL);
+        inet_addr_to_opts(opts, addr);
+        qapi_free_InetSocketAddress(addr);
         sock = inet_listen_opts(opts, port_offset, errp);
         if (sock != -1 && ostr) {
             optstr = strchr(str, ',');
@@ -556,10 +595,8 @@ int inet_listen(const char *str, char *ostr, int olen,
                          optstr ? optstr : "");
             }
         }
-    } else {
-        error_propagate(errp, local_err);
+        qemu_opts_del(opts);
     }
-    qemu_opts_del(opts);
     return sock;
 }
 
@@ -575,16 +612,16 @@ int inet_connect(const char *str, Error **errp)
 {
     QemuOpts *opts;
     int sock = -1;
-    Error *local_err = NULL;
+    InetSocketAddress *addr;
 
-    opts = qemu_opts_create(&dummy_opts, NULL, 0, NULL);
-    inet_parse(opts, str, &local_err);
-    if (local_err == NULL) {
+    addr = inet_parse(str, errp);
+    if (addr != NULL) {
+        opts = qemu_opts_create(&dummy_opts, NULL, 0, NULL);
+        inet_addr_to_opts(opts, addr);
+        qapi_free_InetSocketAddress(addr);
         sock = inet_connect_opts(opts, errp, NULL, NULL);
-    } else {
-        error_propagate(errp, local_err);
+        qemu_opts_del(opts);
     }
-    qemu_opts_del(opts);
     return sock;
 }
 
@@ -607,18 +644,18 @@ int inet_nonblocking_connect(const char *str,
 {
     QemuOpts *opts;
     int sock = -1;
-    Error *local_err = NULL;
+    InetSocketAddress *addr;
 
     g_assert(callback != NULL);
 
-    opts = qemu_opts_create(&dummy_opts, NULL, 0, NULL);
-    inet_parse(opts, str, &local_err);
-    if (local_err == NULL) {
+    addr = inet_parse(str, errp);
+    if (addr != NULL) {
+        opts = qemu_opts_create(&dummy_opts, NULL, 0, NULL);
+        inet_addr_to_opts(opts, addr);
+        qapi_free_InetSocketAddress(addr);
         sock = inet_connect_opts(opts, errp, callback, opaque);
-    } else {
-        error_propagate(errp, local_err);
+        qemu_opts_del(opts);
     }
-    qemu_opts_del(opts);
     return sock;
 }
 
-- 
1.7.12.1

[Qemu-devel] [PATCH 26/29] qemu-sockets: add socket_listen, socket_connect, socket_parse

[Paolo Bonzini]

These are QAPI-friendly versions of the qemu-sockets functions.  They
support IP sockets, Unix sockets, and named file descriptors, using a
QAPI union to dispatch to the correct function.

Reviewed-by: Luiz Capitulino <lcapitulino@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qemu-sockets.c | 99 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 qemu-tool.c    |  6 ++++
 qemu_socket.h  |  5 +++
 3 file modificati, 110 inserzioni(+)

diff --git a/qemu-sockets.c b/qemu-sockets.c
index 909c65f..cfed9c5 100644
--- a/qemu-sockets.c
+++ b/qemu-sockets.c
@@ -22,6 +22,7 @@
 #include <errno.h>
 #include <unistd.h>
 
+#include "monitor.h"
 #include "qemu_socket.h"
 #include "qemu-common.h" /* for qemu_isdigit */
 #include "main-loop.h"
@@ -845,6 +846,104 @@ int unix_nonblocking_connect(const char *path,
     return sock;
 }
 
+SocketAddress *socket_parse(const char *str, Error **errp)
+{
+    SocketAddress *addr = NULL;
+
+    addr = g_new(SocketAddress, 1);
+    if (strstart(str, "unix:", NULL)) {
+        if (str[5] == '\0') {
+            error_setg(errp, "invalid Unix socket address\n");
+            goto fail;
+        } else {
+            addr->kind = SOCKET_ADDRESS_KIND_UNIX;
+            addr->q_unix = g_new(UnixSocketAddress, 1);
+            addr->q_unix->path = g_strdup(str + 5);
+        }
+    } else if (strstart(str, "fd:", NULL)) {
+        if (str[3] == '\0') {
+            error_setg(errp, "invalid file descriptor address\n");
+            goto fail;
+        } else {
+            addr->kind = SOCKET_ADDRESS_KIND_FD;
+            addr->fd = g_new(String, 1);
+            addr->fd->str = g_strdup(str + 3);
+        }
+    } else {
+        addr->kind = SOCKET_ADDRESS_KIND_INET;
+        addr->inet = g_new(InetSocketAddress, 1);
+        addr->inet = inet_parse(str, errp);
+        if (addr->inet == NULL) {
+            goto fail;
+        }
+    }
+    return addr;
+
+fail:
+    qapi_free_SocketAddress(addr);
+    return NULL;
+}
+
+int socket_connect(SocketAddress *addr, Error **errp,
+                   NonBlockingConnectHandler *callback, void *opaque)
+{
+    QemuOpts *opts;
+    int fd;
+
+    opts = qemu_opts_create(&dummy_opts, NULL, 0, NULL);
+    switch (addr->kind) {
+    case SOCKET_ADDRESS_KIND_INET:
+        inet_addr_to_opts(opts, addr->inet);
+        fd = inet_connect_opts(opts, errp, callback, opaque);
+        break;
+
+    case SOCKET_ADDRESS_KIND_UNIX:
+        qemu_opt_set(opts, "path", addr->q_unix->path);
+        fd = unix_connect_opts(opts, errp, callback, opaque);
+        break;
+
+    case SOCKET_ADDRESS_KIND_FD:
+        fd = monitor_get_fd(cur_mon, addr->fd->str, errp);
+        if (callback) {
+            callback(fd, opaque);
+        }
+        break;
+
+    default:
+        abort();
+    }
+    qemu_opts_del(opts);
+    return fd;
+}
+
+int socket_listen(SocketAddress *addr, Error **errp)
+{
+    QemuOpts *opts;
+    int fd;
+
+    opts = qemu_opts_create(&dummy_opts, NULL, 0, NULL);
+    switch (addr->kind) {
+    case SOCKET_ADDRESS_KIND_INET:
+        inet_addr_to_opts(opts, addr->inet);
+        fd = inet_listen_opts(opts, 0, errp);
+        break;
+
+    case SOCKET_ADDRESS_KIND_UNIX:
+        qemu_opt_set(opts, "path", addr->q_unix->path);
+        fd = unix_listen_opts(opts, errp);
+        break;
+
+    case SOCKET_ADDRESS_KIND_FD:
+        fd = monitor_get_fd(cur_mon, addr->fd->str, errp);
+        break;
+
+    default:
+        abort();
+    }
+    qemu_opts_del(opts);
+    return fd;
+}
+
 #ifdef _WIN32
 static void socket_cleanup(void)
 {
diff --git a/qemu-tool.c b/qemu-tool.c
index f2f9813..6fe1dd2 100644
--- a/qemu-tool.c
+++ b/qemu-tool.c
@@ -48,6 +48,12 @@ int monitor_cur_is_qmp(void)
     return 0;
 }
 
+int monitor_get_fd(Monitor *mon, const char *name, Error **errp)
+{
+    error_setg(errp, "only QEMU supports file descriptor passing");
+    return -1;
+}
+
 void monitor_set_error(Monitor *mon, QError *qerror)
 {
 }
diff --git a/qemu_socket.h b/qemu_socket.h
index 89a5feb..02490ad 100644
--- a/qemu_socket.h
+++ b/qemu_socket.h
@@ -65,6 +65,11 @@ int unix_nonblocking_connect(const char *str,
                              NonBlockingConnectHandler *callback,
                              void *opaque, Error **errp);
 
+SocketAddress *socket_parse(const char *str, Error **errp);
+int socket_connect(SocketAddress *addr, Error **errp,
+                   NonBlockingConnectHandler *callback, void *opaque);
+int socket_listen(SocketAddress *addr, Error **errp);
+
 /* 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);
-- 
1.7.12.1

[Qemu-devel] [PATCH 27/29] block: prepare code for adding block notifiers

[Paolo Bonzini]

There is no reason in principle to skip job cancellation and draining
of pending I/O when there is no medium in the disk.  Do these unconditionally,
which also prepares the code for the next patch.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block.c | 10 +++++-----
 1 file modificato, 5 inserzioni(+), 5 rimozioni(-)

diff --git a/block.c b/block.c
index e95f613..2e4ddea 100644
--- a/block.c
+++ b/block.c
@@ -1098,12 +1098,12 @@ void bdrv_reopen_abort(BDRVReopenState *reopen_state)
 void bdrv_close(BlockDriverState *bs)
 {
     bdrv_flush(bs);
-    if (bs->drv) {
-        if (bs->job) {
-            block_job_cancel_sync(bs->job);
-        }
-        bdrv_drain_all();
+    if (bs->job) {
+        block_job_cancel_sync(bs->job);
+    }
+    bdrv_drain_all();
 
+    if (bs->drv) {
         if (bs == bs_snapshots) {
             bs_snapshots = NULL;
         }
-- 
1.7.12.1

[Qemu-devel] [PATCH 28/29] block: add close notifiers

[Paolo Bonzini]

The first user of close notifiers will be the embedded NBD server.
It would be possible to use them to do some of the ad hoc processing
(e.g. for block jobs and I/O limits) that is currently done by
bdrv_close.

Acked-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 Makefile.objs | 4 ++--
 block.c       | 9 +++++++++
 block.h       | 1 +
 block_int.h   | 2 ++
 4 file modificati, 14 inserzioni(+), 2 rimozioni(-)

diff --git a/Makefile.objs b/Makefile.objs
index 3f16d67..ca67885 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -43,7 +43,7 @@ coroutine-obj-$(CONFIG_WIN32) += coroutine-win32.o
 
 block-obj-y = cutils.o iov.o cache-utils.o qemu-option.o module.o async.o
 block-obj-y += nbd.o block.o blockjob.o aio.o aes.o qemu-config.o
-block-obj-y += qemu-progress.o qemu-sockets.o uri.o
+block-obj-y += qemu-progress.o qemu-sockets.o uri.o notify.o
 block-obj-y += $(coroutine-obj-y) $(qobject-obj-y) $(version-obj-y)
 block-obj-$(CONFIG_POSIX) += posix-aio-compat.o
 block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
@@ -94,7 +94,7 @@ common-obj-y += bt-host.o bt-vhci.o
 common-obj-y += dma-helpers.o
 common-obj-y += iov.o acl.o
 common-obj-$(CONFIG_POSIX) += compatfd.o
-common-obj-y += notify.o event_notifier.o
+common-obj-y += event_notifier.o
 common-obj-y += qemu-timer.o qemu-timer-common.o
 common-obj-y += qtest.o
 common-obj-y += vl.o
diff --git a/block.c b/block.c
index 2e4ddea..56426a9 100644
--- a/block.c
+++ b/block.c
@@ -30,6 +30,7 @@
 #include "module.h"
 #include "qjson.h"
 #include "sysemu.h"
+#include "notify.h"
 #include "qemu-coroutine.h"
 #include "qmp-commands.h"
 #include "qemu-timer.h"
@@ -312,9 +313,16 @@ BlockDriverState *bdrv_new(const char *device_name)
         QTAILQ_INSERT_TAIL(&bdrv_states, bs, list);
     }
     bdrv_iostatus_disable(bs);
+    notifier_list_init(&bs->close_notifiers);
+
     return bs;
 }
 
+void bdrv_add_close_notifier(BlockDriverState *bs, Notifier *notify)
+{
+    notifier_list_add(&bs->close_notifiers, notify);
+}
+
 BlockDriver *bdrv_find_format(const char *format_name)
 {
     BlockDriver *drv1;
@@ -1102,6 +1110,7 @@ void bdrv_close(BlockDriverState *bs)
         block_job_cancel_sync(bs->job);
     }
     bdrv_drain_all();
+    notifier_list_notify(&bs->close_notifiers, bs);
 
     if (bs->drv) {
         if (bs == bs_snapshots) {
diff --git a/block.h b/block.h
index e2d89d7..aa608a8 100644
--- a/block.h
+++ b/block.h
@@ -144,6 +144,7 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state,
 void bdrv_reopen_commit(BDRVReopenState *reopen_state);
 void bdrv_reopen_abort(BDRVReopenState *reopen_state);
 void bdrv_close(BlockDriverState *bs);
+void bdrv_add_close_notifier(BlockDriverState *bs, Notifier *notify);
 int bdrv_attach_dev(BlockDriverState *bs, void *dev);
 void bdrv_attach_dev_nofail(BlockDriverState *bs, void *dev);
 void bdrv_detach_dev(BlockDriverState *bs, void *dev);
diff --git a/block_int.h b/block_int.h
index f4bae04..cedabbd 100644
--- a/block_int.h
+++ b/block_int.h
@@ -233,6 +233,8 @@ struct BlockDriverState {
     BlockDriverState *backing_hd;
     BlockDriverState *file;
 
+    NotifierList close_notifiers;
+
     /* number of in-flight copy-on-read requests */
     unsigned int copy_on_read_in_flight;
 
-- 
1.7.12.1

[Qemu-devel] [PATCH 29/29] qmp: add NBD server commands

[Paolo Bonzini]

Adding an NBD server inside QEMU is trivial, since all the logic is
in nbd.c and can be shared easily between qemu-nbd and QEMU itself.
The main difference is that qemu-nbd serves a single unnamed export,
while QEMU serves named exports.

Acked-by: Luiz Capitulino <lcapitulino@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 Makefile.objs    |   2 +-
 blockdev-nbd.c   | 119 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 qapi-schema.json |  43 ++++++++++++++++++++
 qmp-commands.hx  |  16 ++++++++
 4 file modificati, 179 inserzioni(+). 1 rimozione(-)
 create mode 100644 blockdev-nbd.c

diff --git a/Makefile.objs b/Makefile.objs
index ca67885..9eca179 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -61,7 +61,7 @@ endif
 # suppress *all* target specific code in case of system emulation, i.e. a
 # single QEMU executable should support all CPUs and machines.
 
-common-obj-y = $(block-obj-y) blockdev.o block/
+common-obj-y = $(block-obj-y) blockdev.o blockdev-nbd.o block/
 common-obj-y += net.o net/
 common-obj-y += qom/
 common-obj-y += readline.o console.o cursor.o
diff --git a/blockdev-nbd.c b/blockdev-nbd.c
new file mode 100644
index 0000000..8031813
--- /dev/null
+++ b/blockdev-nbd.c
@@ -0,0 +1,119 @@
+/*
+ * Serving QEMU block devices via NBD
+ *
+ * Copyright (c) 2012 Red Hat, Inc.
+ *
+ * Author: Paolo Bonzini <pbonzini@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * later.  See the COPYING file in the top-level directory.
+ */
+
+#include "blockdev.h"
+#include "hw/block-common.h"
+#include "monitor.h"
+#include "qerror.h"
+#include "sysemu.h"
+#include "qmp-commands.h"
+#include "trace.h"
+#include "nbd.h"
+#include "qemu_socket.h"
+
+static int server_fd = -1;
+
+static void nbd_accept(void *opaque)
+{
+    struct sockaddr_in addr;
+    socklen_t addr_len = sizeof(addr);
+
+    int fd = accept(server_fd, (struct sockaddr *)&addr, &addr_len);
+    if (fd >= 0) {
+        nbd_client_new(NULL, fd, nbd_client_put);
+    }
+}
+
+void qmp_nbd_server_start(SocketAddress *addr, Error **errp)
+{
+    if (server_fd != -1) {
+        error_setg(errp, "NBD server already running");
+        return;
+    }
+
+    server_fd = socket_listen(addr, errp);
+    if (server_fd != -1) {
+        qemu_set_fd_handler2(server_fd, NULL, nbd_accept, NULL, NULL);
+    }
+}
+
+/* Hook into the BlockDriverState notifiers to close the export when
+ * the file is closed.
+ */
+typedef struct NBDCloseNotifier {
+    Notifier n;
+    NBDExport *exp;
+    QTAILQ_ENTRY(NBDCloseNotifier) next;
+} NBDCloseNotifier;
+
+static QTAILQ_HEAD(, NBDCloseNotifier) close_notifiers =
+    QTAILQ_HEAD_INITIALIZER(close_notifiers);
+
+static void nbd_close_notifier(Notifier *n, void *data)
+{
+    NBDCloseNotifier *cn = DO_UPCAST(NBDCloseNotifier, n, n);
+
+    notifier_remove(&cn->n);
+    QTAILQ_REMOVE(&close_notifiers, cn, next);
+
+    nbd_export_close(cn->exp);
+    nbd_export_put(cn->exp);
+    g_free(cn);
+}
+
+static void nbd_server_put_ref(NBDExport *exp)
+{
+    BlockDriverState *bs = nbd_export_get_blockdev(exp);
+    drive_put_ref(drive_get_by_blockdev(bs));
+}
+
+void qmp_nbd_server_add(const char *device, bool has_writable, bool writable,
+                        Error **errp)
+{
+    BlockDriverState *bs;
+    NBDExport *exp;
+    NBDCloseNotifier *n;
+
+    if (nbd_export_find(device)) {
+        error_setg(errp, "NBD server already exporting device '%s'", device);
+        return;
+    }
+
+    bs = bdrv_find(device);
+    if (!bs) {
+        error_set(errp, QERR_DEVICE_NOT_FOUND, device);
+        return;
+    }
+
+    exp = nbd_export_new(bs, 0, -1, writable ? 0 : NBD_FLAG_READ_ONLY,
+                         nbd_server_put_ref);
+
+    nbd_export_set_name(exp, device);
+    drive_get_ref(drive_get_by_blockdev(bs));
+
+    n = g_malloc0(sizeof(NBDCloseNotifier));
+    n->n.notify = nbd_close_notifier;
+    n->exp = exp;
+    bdrv_add_close_notifier(bs, &n->n);
+    QTAILQ_INSERT_TAIL(&close_notifiers, n, next);
+}
+
+void qmp_nbd_server_stop(Error **errp)
+{
+    while (!QTAILQ_EMPTY(&close_notifiers)) {
+        NBDCloseNotifier *cn = QTAILQ_FIRST(&close_notifiers);
+        nbd_close_notifier(&cn->n, nbd_export_get_blockdev(cn->exp));
+    }
+
+    qemu_set_fd_handler2(server_fd, NULL, NULL, NULL, NULL);
+    close(server_fd);
+    server_fd = -1;
+}
diff --git a/qapi-schema.json b/qapi-schema.json
index a92a33b..4451588 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2849,3 +2849,46 @@
 # Since: 0.14.0
 ##
 { 'command': 'screendump', 'data': {'filename': 'str'} }
+
+##
+# @nbd-server-start:
+#
+# Start an NBD server listening on the given host and port.  Block
+# devices can then be exported using @nbd-server-add.  The NBD
+# server will present them as named exports; for example, another
+# QEMU instance could refer to them as "nbd:HOST:PORT:exportname=NAME".
+#
+# @addr: Address on which to listen.
+#
+# Returns: error if the server is already running.
+#
+# Since: 1.3.0
+##
+{ 'command': 'nbd-server-start',
+  'data': { 'addr': 'SocketAddress' } }
+
+##
+# @nbd-server-add:
+#
+# Export a device to QEMU's embedded NBD server.
+#
+# @device: Block device to be exported
+#
+# @writable: Whether clients should be able to write to the device via the
+#     NBD connection (default false). #optional
+#
+# Returns: error if the device is already marked for export.
+#
+# Since: 1.3.0
+##
+{ 'command': 'nbd-server-add', 'data': {'device': 'str', '*writable': 'bool'} }
+
+##
+# @nbd-server-stop:
+#
+# Stop QEMU's embedded NBD server, and unregister all devices previously
+# added via @nbd-server-add.
+#
+# Since: 1.3.0
+##
+{ 'command': 'nbd-server-stop' }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 2f8477e..8b40902 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -2543,6 +2543,22 @@ EQMP
     },
 
     {
+        .name       = "nbd-server-start",
+        .args_type  = "addr:q",
+        .mhandler.cmd_new = qmp_marshal_input_nbd_server_start,
+    },
+    {
+        .name       = "nbd-server-add",
+        .args_type  = "device:B,writable:b?",
+        .mhandler.cmd_new = qmp_marshal_input_nbd_server_add,
+    },
+    {
+        .name       = "nbd-server-stop",
+        .args_type  = "",
+        .mhandler.cmd_new = qmp_marshal_input_nbd_server_stop,
+    },
+
+    {
         .name       = "change-vnc-password",
         .args_type  = "password:s",
         .mhandler.cmd_new = qmp_marshal_input_change_vnc_password,
-- 
1.7.12.1

Re: [Qemu-devel] [PATCH 15/29] vnc: reorganize code for reverse mode

[Markus Armbruster]

Paolo Bonzini <pbonzini@redhat.com> writes:

> Avoid the dance between csock and vs->lsock.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  console.h |  2 +-
>  qmp.c     |  6 ++----
>  ui/vnc.c  | 20 +++++++++-----------
>  3 file modificati, 12 inserzioni(+), 16 rimozioni(-)
>
> diff --git a/console.h b/console.h
> index f990684..6099d8d 100644
> --- a/console.h
> +++ b/console.h
> @@ -378,7 +378,7 @@ void cocoa_display_init(DisplayState *ds, int full_screen);
>  /* vnc.c */
>  void vnc_display_init(DisplayState *ds);
>  void vnc_display_close(DisplayState *ds);
> -int vnc_display_open(DisplayState *ds, const char *display);
> +void vnc_display_open(DisplayState *ds, const char *display, Error **errp);
>  void vnc_display_add_client(DisplayState *ds, int csock, int skipauth);
>  int vnc_display_disable_login(DisplayState *ds);
>  char *vnc_display_local_addr(DisplayState *ds);
> diff --git a/qmp.c b/qmp.c
> index 36c54c5..31bc3bf 100644
> --- a/qmp.c
> +++ b/qmp.c
> @@ -349,11 +349,9 @@ void qmp_change_vnc_password(const char *password, Error **errp)
>      }
>  }
>  
> -static void qmp_change_vnc_listen(const char *target, Error **err)
> +static void qmp_change_vnc_listen(const char *target, Error **errp)
>  {
> -    if (vnc_display_open(NULL, target) < 0) {
> -        error_set(err, QERR_VNC_SERVER_FAILED, target);
> -    }
> +    vnc_display_open(NULL, target, errp);
>  }
>  

These two hunks looks out of place, and don't compile:

    ui/vnc.c:2851:5: error: conflicting types for ‘vnc_display_open’

    vl.c:3710:9: error: too few arguments to function ‘vnc_display_open’

Did they creep into the wrong patch?

[...]

Re: [Qemu-devel] [PATCH 14/29] vnc: introduce a single label for error returns

[Markus Armbruster]

Paolo Bonzini <pbonzini@redhat.com> writes:

> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  ui/vnc.c | 42 +++++++++++++++++-------------------------
>  1 file modificato, 17 inserzioni(+), 25 rimozioni(-)
>
> diff --git a/ui/vnc.c b/ui/vnc.c
> index 2892fb7..6da265f 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -2872,8 +2872,7 @@ int vnc_display_open(DisplayState *ds, const char *display)
>      if (strcmp(display, "none") == 0)
>          return 0;
>  
> -    if (!(vs->display = strdup(display)))
> -        return -1;
> +    vs->display = g_strdup(display);
>      vs->share_policy = VNC_SHARE_POLICY_ALLOW_EXCLUSIVE;
>  
>      options = display;
> @@ -2885,9 +2884,7 @@ int vnc_display_open(DisplayState *ds, const char *display)
>                          "VNC password auth disabled due to FIPS mode, "
>                          "consider using the VeNCrypt or SASL authentication "
>                          "methods as an alternative\n");
> -                g_free(vs->display);
> -                vs->display = NULL;
> -                return -1;
> +                goto fail;
>              }
>              password = 1; /* Require password auth */
>          } else if (strncmp(options, "reverse", 7) == 0) {
> @@ -2919,16 +2916,12 @@ int vnc_display_open(DisplayState *ds, const char *display)
>                  if (vnc_tls_set_x509_creds_dir(vs, path) < 0) {
>                      fprintf(stderr, "Failed to find x509 certificates/keys in %s\n", path);
>                      g_free(path);
> -                    g_free(vs->display);
> -                    vs->display = NULL;
> -                    return -1;
> +                    goto fail;
>                  }
>                  g_free(path);
>              } else {
>                  fprintf(stderr, "No certificate path provided\n");
> -                g_free(vs->display);
> -                vs->display = NULL;
> -                return -1;
> +                goto fail;
>              }
>  #endif
>  #if defined(CONFIG_VNC_TLS) || defined(CONFIG_VNC_SASL)
> @@ -2948,9 +2941,7 @@ int vnc_display_open(DisplayState *ds, const char *display)
>                  vs->share_policy = VNC_SHARE_POLICY_FORCE_SHARED;
>              } else {
>                  fprintf(stderr, "unknown vnc share= option\n");
> -                g_free(vs->display);
> -                vs->display = NULL;
> -                return -1;
> +                goto fail;
>              }
>          }
>      }
> @@ -3053,9 +3044,7 @@ int vnc_display_open(DisplayState *ds, const char *display)
>      if ((saslErr = sasl_server_init(NULL, "qemu")) != SASL_OK) {
>          fprintf(stderr, "Failed to initialize SASL auth %s",
>                  sasl_errstring(saslErr, NULL, NULL));
> -        g_free(vs->display);
> -        vs->display = NULL;
> -        return -1;
> +        goto fail;
>      }
>  #endif
>      vs->lock_key_sync = lock_key_sync;
> @@ -3067,9 +3056,7 @@ int vnc_display_open(DisplayState *ds, const char *display)
>          else
>              vs->lsock = inet_connect(display, NULL);
>          if (vs->lsock < 0) {
> -            g_free(vs->display);
> -            vs->display = NULL;
> -            return -1;
> +            goto fail;
>          } else {
>              int csock = vs->lsock;
>              vs->lsock = -1;
> @@ -3090,13 +3077,18 @@ int vnc_display_open(DisplayState *ds, const char *display)

Note: I'm reordering the quoted diff lines slightly for clarity.

>          }
>          if (vs->lsock < 0) {
>              g_free(dpy);
> -            return -1;
> +            goto fail;

Now additionally executes

       g_free(vs->display);
       vs->display = NULL;

Silent bug fix?

> -        } else {
> -            g_free(vs->display);
> -            vs->display = dpy;
>          }
> +        g_free(vs->display);
> +        vs->display = dpy;
> +        qemu_set_fd_handler2(vs->lsock, NULL, vnc_listen_read, NULL, vs);
>      }
> -    return qemu_set_fd_handler2(vs->lsock, NULL, vnc_listen_read, NULL, vs);
> +    return 0;
> +
> +fail:
> +    g_free(vs->display);
> +    vs->display = NULL;
> +    return -1;
>  }
>  
>  void vnc_display_add_client(DisplayState *ds, int csock, int skipauth)

Re: [Qemu-devel] [PATCH 08/29] migration (outgoing): add error propagation for all protocols

[Markus Armbruster]

Paolo Bonzini <pbonzini@redhat.com> writes:

> Error propagation is already there for socket backends.  Add it to other
> protocols, simplifying code that tests for errors that will never happen.
> With all protocols understanding Error, the code can be simplified
> further by removing the return value.
>
> Unfortunately, the quality of error messages varies depending
> on where the error is detected, because no Error is passed to the
> NonBlockingConnectHandler.  Thus, the exact error message still cannot
> be sent to the user if the OS reports it asynchronously via SO_ERROR.
> If NonBlockingConnectHandler received an Error**, we could for
> example report the error class and/or message via a new field of the
> query-migration command even if it is reported asynchronously.
>
> Before:
>
>     (qemu) migrate fd:ffff
>     migrate: An undefined error has occurred
>     (qemu) info migrate
>     (qemu)
>
> After:
>
>     (qemu) migrate fd:ffff
>     migrate: File descriptor named 'ffff' has not been found
>     (qemu) info migrate
>     capabilities: xbzrle: off
>     Migration status: failed
>     total time: 0 milliseconds
[...]
> diff --git a/migration-fd.c b/migration-fd.c
> index 7335167..a7c800a 100644
> --- a/migration-fd.c
> +++ b/migration-fd.c
> @@ -73,30 +73,19 @@ static int fd_close(MigrationState *s)
>      return 0;
>  }
>  
> -int fd_start_outgoing_migration(MigrationState *s, const char *fdname)
> +void fd_start_outgoing_migration(MigrationState *s, const char *fdname, Error **errp)
>  {
> -    s->fd = monitor_get_fd(cur_mon, fdname, NULL);
> +    s->fd = monitor_get_fd(cur_mon, fdname, errp);
>      if (s->fd == -1) {
> -        DPRINTF("fd_migration: invalid file descriptor identifier\n");
> -        goto err_after_get_fd;
> -    }
> -
> -    if (fcntl(s->fd, F_SETFL, O_NONBLOCK) == -1) {
> -        DPRINTF("Unable to set nonblocking mode on file descriptor\n");
> -        goto err_after_open;
> +        return;
>      }
>  
> +    fcntl(s->fd, F_SETFL, O_NONBLOCK);

Sure this can't fail?

>      s->get_error = fd_errno;
>      s->write = fd_write;
>      s->close = fd_close;
>  
>      migrate_fd_connect(s);
> -    return 0;
> -
> -err_after_open:
> -    close(s->fd);
> -err_after_get_fd:
> -    return -1;
>  }
>  
>  static void fd_accept_incoming_migration(void *opaque)
[...]

Re: [Qemu-devel] [PATCH 18/29] qemu-sockets: add error propagation to inet_connect_addr

[Markus Armbruster]

Paolo Bonzini <pbonzini@redhat.com> writes:

> perror and fprintf can be removed because all clients can now consume
> Errors properly.  However, we'll need to change the non-blocking connect
> handlers to take an Error, in order to improve error handling for
> migration with the TCP protocol.
>
> This is a minor degradation in error reporting for outgoing migration.
> However, until 1.2 this case just failed without even attempting to
> connect, so it is still an improvement as far as overall QoI is
> concerned.
>
> Reviewed-by: Luiz Capitulino <lcapitulino@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  qemu-sockets.c | 15 ++++++---------
>  1 file modificato, 6 inserzioni(+), 9 rimozioni(-)
>
> diff --git a/qemu-sockets.c b/qemu-sockets.c
> index 9e3d233..88d58fe 100644
> --- a/qemu-sockets.c
> +++ b/qemu-sockets.c
> @@ -216,7 +216,7 @@ typedef struct ConnectState {
>  } ConnectState;
>  
>  static int inet_connect_addr(struct addrinfo *addr, bool *in_progress,
> -                             ConnectState *connect_state);
> +                             ConnectState *connect_state, Error **errp);
>  
>  static void wait_for_connect(void *opaque)
>  {
> @@ -246,7 +246,7 @@ static void wait_for_connect(void *opaque)
>      if (s->current_addr) {
>          while (s->current_addr->ai_next != NULL && s->fd < 0) {
>              s->current_addr = s->current_addr->ai_next;
> -            s->fd = inet_connect_addr(s->current_addr, &in_progress, s);
> +            s->fd = inet_connect_addr(s->current_addr, &in_progress, s, NULL);
>              /* connect in progress */
>              if (in_progress) {
>                  return;
> @@ -263,7 +263,7 @@ static void wait_for_connect(void *opaque)
>  }
>  
>  static int inet_connect_addr(struct addrinfo *addr, bool *in_progress,
> -                             ConnectState *connect_state)
> +                             ConnectState *connect_state, Error **errp)
>  {
>      int sock, rc;
>  
> @@ -271,8 +271,7 @@ static int inet_connect_addr(struct addrinfo *addr, bool *in_progress,
>  
>      sock = qemu_socket(addr->ai_family, addr->ai_socktype, addr->ai_protocol);
>      if (sock < 0) {
> -        fprintf(stderr, "%s: socket(%s): %s\n", __func__,
> -                inet_strfamily(addr->ai_family), strerror(errno));
> +        error_set_errno(errp, errno, QERR_SOCKET_CREATE_FAILED);
>          return -1;
>      }
>      qemu_setsockopt(sock, SOL_SOCKET, SO_REUSEADDR, &on, sizeof(on));
> @@ -293,6 +292,7 @@ static int inet_connect_addr(struct addrinfo *addr, bool *in_progress,
>                               connect_state);
>          *in_progress = true;
>      } else if (rc < 0) {
> +        error_set_errno(errp, errno, QERR_SOCKET_CONNECT_FAILED);
>          closesocket(sock);
>          return -1;
>      }

Looks like this reports a previously unreported error condition.

If it does, it's a fix worth mentioning in commit message.

> @@ -375,7 +375,7 @@ int inet_connect_opts(QemuOpts *opts, Error **errp,
>          if (connect_state != NULL) {
>              connect_state->current_addr = e;
>          }
> -        sock = inet_connect_addr(e, &in_progress, connect_state);
> +        sock = inet_connect_addr(e, &in_progress, connect_state, errp);
>          if (in_progress) {
>              return sock;
>          } else if (sock >= 0) {
> @@ -386,9 +386,6 @@ int inet_connect_opts(QemuOpts *opts, Error **errp,
>              break;
>          }
>      }
> -    if (sock < 0) {
> -        error_set(errp, QERR_SOCKET_CONNECT_FAILED);
> -    }
>      g_free(connect_state);
>      freeaddrinfo(res);
>      return sock;

Re: [Qemu-devel] [PULL 00/29] qemu-sockets error propagation + NBD server

[Markus Armbruster]

Paolo Bonzini <pbonzini@redhat.com> writes:

> Anthony,
>
> The following changes since commit b6348f29d033d5a8a26f633d2ee94362595f32a4:
>
>   target-arm/translate: Fix RRX operands (2012-10-17 19:56:46 +0200)
>
> are available in the git repository at:
>
>   git://github.com/bonzini/qemu.git nbd-next
>
> for you to fetch changes up to 1f493f2c7ee75691c4ad22a7663f908924f464c5:
>
>   qmp: add NBD server commands (2012-10-19 15:29:12 +0200)
>
> I and Luiz agreed that I would take care of sending it to you.  It's been
> through his and Markus's review, and in particularly Markus suggested
> various improvements to the commit message and further splitting of
> some commits; the only remaining sore point was the HMP interface so I
> dropped it for now.

All my review comments have been addressed, except for a few nits.  I
think the patch splitting added one error in 15/29, corrected in the
next patch.  Fix that, and you've amply earned my

Reviewed-by: Markus Armbruster <armbru@redhat.com>

Re: [Qemu-devel] [PULL 00/29] qemu-sockets error propagation + NBD server

[Anthony Liguori]

Paolo Bonzini <pbonzini@redhat.com> writes:

> Anthony,
>
> The following changes since commit b6348f29d033d5a8a26f633d2ee94362595f32a4:
>
>   target-arm/translate: Fix RRX operands (2012-10-17 19:56:46 +0200)
>
> are available in the git repository at:
>
>   git://github.com/bonzini/qemu.git nbd-next
>
> for you to fetch changes up to 1f493f2c7ee75691c4ad22a7663f908924f464c5:
>
>   qmp: add NBD server commands (2012-10-19 15:29:12 +0200)
>
> I and Luiz agreed that I would take care of sending it to you.  It's been
> through his and Markus's review, and in particularly Markus suggested
> various improvements to the commit message and further splitting of
> some commits; the only remaining sore point was the HMP interface so I
> dropped it for now.

anthony@titi:~/build/qemu$ make
  GEN   qmp-commands.h
  GEN   qapi-types.h
  GEN   qapi-visit.h
  GEN   QMP/qmp-commands.txt
  CC    osdep.o
  CC    oslib-posix.o
  CC    trace.o
  CC    trace/default.o
  CC    trace/control.o
  CC    qemu-tool.o
  CC    qemu-timer.o
  CC    main-loop.o
  CC    iohandler.o
  CC    cutils.o
  CC    async.o
  CC    libcacard/vscclient.o
  LINK  vscclient
qemu-tool.o: In function `monitor_get_fd':
/home/anthony/git/qemu/qemu-tool.c:53: undefined reference to `error_set'
collect2: ld returned 1 exit status
make: *** [vscclient] Error 1

Even tried a clean build and get the same error.

Regards,

Anthony Liguori

>
> Paolo
>
> ----------------------------------------------------------------
> Paolo Bonzini (29):
>       error: add error_set_errno and error_setg_errno
>       qemu-sockets: add Error ** to all functions
>       qemu-sockets: unix_listen and unix_connect are portable
>       qemu-sockets: add nonblocking connect for Unix sockets
>       migration: avoid using error_is_set and thus relying on errp != NULL
>       migration: centralize call to migrate_fd_error()
>       migration: use qemu-sockets to establish Unix sockets
>       migration (outgoing): add error propagation for all protocols
>       migration (incoming): add error propagation to fd and exec protocols
>       qemu-char: ask and print error information from qemu-sockets
>       nbd: ask and print error information from qemu-sockets
>       qemu-ga: ask and print error information from qemu-sockets
>       vnc: avoid Yoda conditionals
>       vnc: introduce a single label for error returns
>       vnc: reorganize code for reverse mode
>       vnc: add error propagation to vnc_display_open
>       qemu-sockets: include strerror or gai_strerror output in error messages
>       qemu-sockets: add error propagation to inet_connect_addr
>       qemu-sockets: add error propagation to inet_dgram_opts
>       qemu-sockets: add error propagation to inet_parse
>       qemu-sockets: add error propagation to Unix socket functions
>       vnc: drop QERR_VNC_SERVER_FAILED
>       build: add QAPI files to the tools
>       qapi: add socket address types
>       qemu-sockets: return InetSocketAddress from inet_parse
>       qemu-sockets: add socket_listen, socket_connect, socket_parse
>       block: prepare code for adding block notifiers
>       block: add close notifiers
>       qmp: add NBD server commands
>
>  Makefile.objs       |   9 +-
>  block.c             |  19 ++-
>  block.h             |   1 +
>  block_int.h         |   2 +
>  blockdev-nbd.c      | 119 +++++++++++++++
>  console.h           |   2 +-
>  error.c             |  28 ++++
>  error.h             |   9 ++
>  migration-exec.c    |  26 +---
>  migration-fd.c      |  27 +---
>  migration-tcp.c     |  19 +--
>  migration-unix.c    |  95 ++----------
>  migration.c         |  34 ++---
>  migration.h         |  19 ++-
>  nbd.c               |  39 ++++-
>  qapi-schema.json    |  96 ++++++++++++
>  qemu-char.c         |  24 ++-
>  qemu-sockets.c      | 424 ++++++++++++++++++++++++++++++++++------------------
>  qemu-tool.c         |   6 +
>  qemu_socket.h       |  19 ++-
>  qerror.h            |   3 -
>  qga/channel-posix.c |   8 +-
>  qmp-commands.hx     |  16 ++
>  qmp.c               |   6 +-
>  ui/vnc.c            |  93 ++++++------
>  vl.c                |  25 ++--
>  26 file modificati, 756 inserzioni(+), 412 rimozioni(-)
>  create mode 100644 blockdev-nbd.c
> -- 
> 1.7.12.1

Re: [Qemu-devel] [PATCH 08/29] migration (outgoing): add error propagation for all protocols

[Paolo Bonzini]

Il 22/10/2012 17:52, Markus Armbruster ha scritto:
>> >  
>> > +    fcntl(s->fd, F_SETFL, O_NONBLOCK);
> Sure this can't fail?
> 

Yes, we even assume that when we call socket_set_nonblock.

Paolo

Re: [Qemu-devel] [PATCH 14/29] vnc: introduce a single label for error returns

[Paolo Bonzini]

Il 22/10/2012 17:49, Markus Armbruster ha scritto:
>>          }
>> >          if (vs->lsock < 0) {
>> >              g_free(dpy);
>> > -            return -1;
>> > +            goto fail;
> Now additionally executes
> 
>        g_free(vs->display);
>        vs->display = NULL;
> 
> Silent bug fix?
> 

Possibly... no idea how to reproduce it.

Paolo

Re: [Qemu-devel] [PULL 00/29] qemu-sockets error propagation + NBD server

[Paolo Bonzini]

Il 22/10/2012 21:53, Anthony Liguori ha scritto:
>   LINK  vscclient
> qemu-tool.o: In function `monitor_get_fd':
> /home/anthony/git/qemu/qemu-tool.c:53: undefined reference to `error_set'
> collect2: ld returned 1 exit status
> make: *** [vscclient] Error 1
> 
> Even tried a clean build and get the same error.

Fixed like this and pushed to the same place (tip is now commit
8b5c3381d28ebb1765f17a4cbe1852d3b68b7dc4).  Sorry.

I will try to clean up the dependencies before or during soft feature
freeze.

Paolo

diff --git a/Makefile b/Makefile
index 88285a4..2a33868 100644
--- a/Makefile
+++ b/Makefile
@@ -171,7 +171,8 @@ qemu-io$(EXESUF): qemu-io.o cmd.o $(tools-obj-y) $(block-obj-y)
 
 qemu-bridge-helper$(EXESUF): qemu-bridge-helper.o
 
-vscclient$(EXESUF): $(libcacard-y) $(oslib-obj-y) $(trace-obj-y) $(tools-obj-y) qemu-timer-common.o libcacard/vscclient.o
+vscclient$(EXESUF): $(libcacard-y) $(oslib-obj-y) $(trace-obj-y) \
+	error.o $(tools-obj-y) qemu-timer-common.o libcacard/vscclient.o
 	$(call quiet-command,$(CC) $(LDFLAGS) -o $@ $^ $(libcacard_libs) $(LIBS),"  LINK  $@")
 
 fsdev/virtfs-proxy-helper$(EXESUF): fsdev/virtfs-proxy-helper.o fsdev/virtio-9p-marshal.o oslib-posix.o $(trace-obj-y)

Re: [Qemu-devel] [PULL 00/29] qemu-sockets error propagation + NBD server

[Paolo Bonzini]

Il 23/10/2012 14:34, Paolo Bonzini ha scritto:
> Fixed like this and pushed to the same place (tip is now commit
> 8b5c3381d28ebb1765f17a4cbe1852d3b68b7dc4).  Sorry.
> 
> I will try to clean up the dependencies before or during soft feature
> freeze.

Nevermind, still broken. :(

Paolo

Re: [Qemu-devel] [PULL 00/29] qemu-sockets error propagation + NBD server

[Paolo Bonzini]

On Tue, Oct 23, 2012 at 5:27 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 23/10/2012 14:34, Paolo Bonzini ha scritto:
>> Fixed like this and pushed to the same place (tip is now commit
>> 8b5c3381d28ebb1765f17a4cbe1852d3b68b7dc4).  Sorry.
>>
>> I will try to clean up the dependencies before or during soft feature
>> freeze.
>
> Nevermind, still broken. :(

It's fixed now (actually a couple hours ago :)).

Paolo

Re: [Qemu-devel] [PULL 00/29] qemu-sockets error propagation + NBD server

[Anthony Liguori]

Paolo Bonzini <pbonzini@redhat.com> writes:

> Anthony,
>
> The following changes since commit b6348f29d033d5a8a26f633d2ee94362595f32a4:
>
>   target-arm/translate: Fix RRX operands (2012-10-17 19:56:46 +0200)
>
> are available in the git repository at:
>
>   git://github.com/bonzini/qemu.git nbd-next

Pulled. Thanks.

Regards,

Anthony Liguori

>
> for you to fetch changes up to 1f493f2c7ee75691c4ad22a7663f908924f464c5:
>
>   qmp: add NBD server commands (2012-10-19 15:29:12 +0200)
>
> I and Luiz agreed that I would take care of sending it to you.  It's been
> through his and Markus's review, and in particularly Markus suggested
> various improvements to the commit message and further splitting of
> some commits; the only remaining sore point was the HMP interface so I
> dropped it for now.
>
> Paolo
>
> ----------------------------------------------------------------
> Paolo Bonzini (29):
>       error: add error_set_errno and error_setg_errno
>       qemu-sockets: add Error ** to all functions
>       qemu-sockets: unix_listen and unix_connect are portable
>       qemu-sockets: add nonblocking connect for Unix sockets
>       migration: avoid using error_is_set and thus relying on errp != NULL
>       migration: centralize call to migrate_fd_error()
>       migration: use qemu-sockets to establish Unix sockets
>       migration (outgoing): add error propagation for all protocols
>       migration (incoming): add error propagation to fd and exec protocols
>       qemu-char: ask and print error information from qemu-sockets
>       nbd: ask and print error information from qemu-sockets
>       qemu-ga: ask and print error information from qemu-sockets
>       vnc: avoid Yoda conditionals
>       vnc: introduce a single label for error returns
>       vnc: reorganize code for reverse mode
>       vnc: add error propagation to vnc_display_open
>       qemu-sockets: include strerror or gai_strerror output in error messages
>       qemu-sockets: add error propagation to inet_connect_addr
>       qemu-sockets: add error propagation to inet_dgram_opts
>       qemu-sockets: add error propagation to inet_parse
>       qemu-sockets: add error propagation to Unix socket functions
>       vnc: drop QERR_VNC_SERVER_FAILED
>       build: add QAPI files to the tools
>       qapi: add socket address types
>       qemu-sockets: return InetSocketAddress from inet_parse
>       qemu-sockets: add socket_listen, socket_connect, socket_parse
>       block: prepare code for adding block notifiers
>       block: add close notifiers
>       qmp: add NBD server commands
>
>  Makefile.objs       |   9 +-
>  block.c             |  19 ++-
>  block.h             |   1 +
>  block_int.h         |   2 +
>  blockdev-nbd.c      | 119 +++++++++++++++
>  console.h           |   2 +-
>  error.c             |  28 ++++
>  error.h             |   9 ++
>  migration-exec.c    |  26 +---
>  migration-fd.c      |  27 +---
>  migration-tcp.c     |  19 +--
>  migration-unix.c    |  95 ++----------
>  migration.c         |  34 ++---
>  migration.h         |  19 ++-
>  nbd.c               |  39 ++++-
>  qapi-schema.json    |  96 ++++++++++++
>  qemu-char.c         |  24 ++-
>  qemu-sockets.c      | 424 ++++++++++++++++++++++++++++++++++------------------
>  qemu-tool.c         |   6 +
>  qemu_socket.h       |  19 ++-
>  qerror.h            |   3 -
>  qga/channel-posix.c |   8 +-
>  qmp-commands.hx     |  16 ++
>  qmp.c               |   6 +-
>  ui/vnc.c            |  93 ++++++------
>  vl.c                |  25 ++--
>  26 file modificati, 756 inserzioni(+), 412 rimozioni(-)
>  create mode 100644 blockdev-nbd.c
> -- 
> 1.7.12.1