From 680d16dcb79f999fad3a652c5190d6a5c6ea10dd Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 2 Oct 2012 09:00:45 +0200 Subject: [PATCH 01/30] error: add error_set_errno and error_setg_errno These functions help maintaining homogeneous formatting of error messages that include strerror values. Acked-by: Luiz Capitulino Signed-off-by: Paolo Bonzini --- error.c | 28 ++++++++++++++++++++++++++++ error.h | 9 +++++++++ 2 files changed, 37 insertions(+) diff --git a/error.c b/error.c index 1f05fc466e..128d88cd91 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 da7fed399e..4d52e7369e 100644 --- a/error.h +++ b/error.h @@ -29,11 +29,20 @@ 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 From 7fc4e63ec018a0ef6d420ddb7f6cbf68387d4995 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 2 Oct 2012 09:35:32 +0200 Subject: [PATCH 02/30] qemu-sockets: add Error ** to all functions 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 Signed-off-by: Paolo Bonzini --- 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 files changed, 24 insertions(+), 24 deletions(-) diff --git a/nbd.c b/nbd.c index 6f0db62deb..f61a288369 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 b082bae11b..3cc6cb52ee 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 2b1ed2f0e9..7f0d4be323 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 3e8aee9cad..ff979b5cd6 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 57eea06c47..e22eee6f67 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 66ae93010f..ceade57ce3 100644 --- a/ui/vnc.c +++ b/ui/vnc.c @@ -3065,7 +3065,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) { @@ -3085,7 +3085,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); From 0c814709471ac8a9617406e1e2cbd7015e97bfe9 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 18 Oct 2012 08:44:00 +0200 Subject: [PATCH 03/30] qemu-sockets: unix_listen and unix_connect are portable They are just wrappers and do not need a Win32-specific version. Reviewed-by: Luiz Capitulino Signed-off-by: Paolo Bonzini --- qemu-sockets.c | 49 +++++++++++++++++-------------------------------- 1 file changed, 17 insertions(+), 32 deletions(-) diff --git a/qemu-sockets.c b/qemu-sockets.c index 7f0d4be323..7bf756d07e 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) { From 1fc05adfa0f79a1268f7c2b7fb324f15eb63dceb Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Wed, 3 Oct 2012 13:37:46 +0200 Subject: [PATCH 04/30] qemu-sockets: add nonblocking connect for Unix sockets 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 --- qemu-char.c | 2 +- qemu-sockets.c | 81 ++++++++++++++++++++++++++++++++++++++++---------- qemu_socket.h | 6 +++- 3 files changed, 72 insertions(+), 17 deletions(-) diff --git a/qemu-char.c b/qemu-char.c index 3cc6cb52ee..8ebd5827d9 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 7bf756d07e..d510ad1332 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) { - fprintf(stderr, "connect(unix:%s): %s\n", path, strerror(errno)); - close(sock); - return -1; + + /* 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); + 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 ff979b5cd6..89a5feb7ec 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); From be7059cd7f8998d41f0b44ec13907359d04c63d2 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Wed, 3 Oct 2012 14:34:33 +0200 Subject: [PATCH 05/30] migration: avoid using error_is_set and thus relying on errp != NULL 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 --- migration-tcp.c | 8 +++++--- migration.c | 13 +++++++------ 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/migration-tcp.c b/migration-tcp.c index a15c2b87a1..78337a3e29 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 62e030487d..b332dae3a4 100644 --- a/migration.c +++ b/migration.c @@ -483,6 +483,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; @@ -508,7 +509,7 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk, s = migrate_init(¶ms); 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); @@ -522,11 +523,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; } From 342ab8d1e4f8650a22a3a1c9fade31df3fd9c1c1 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 2 Oct 2012 09:59:38 +0200 Subject: [PATCH 06/30] migration: centralize call to migrate_fd_error() 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 Signed-off-by: Paolo Bonzini --- migration-tcp.c | 1 - migration-unix.c | 1 - migration.c | 1 + 3 files changed, 1 insertion(+), 2 deletions(-) diff --git a/migration-tcp.c b/migration-tcp.c index 78337a3e29..e8bc76acc6 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 169de88677..d349662498 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 b332dae3a4..efea21983a 100644 --- a/migration.c +++ b/migration.c @@ -524,6 +524,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 { From e08c95ce8d66276198704f21ed9df6d99b3477e0 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Wed, 3 Oct 2012 14:05:49 +0200 Subject: [PATCH 07/30] migration: use qemu-sockets to establish Unix sockets 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 --- migration-unix.c | 94 +++++++++--------------------------------------- migration.c | 4 +-- migration.h | 4 +-- 3 files changed, 21 insertions(+), 81 deletions(-) diff --git a/migration-unix.c b/migration-unix.c index d349662498..5387c213ef 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; + s->fd = unix_nonblocking_connect(path, unix_wait_for_connect, s, &local_err); + if (local_err != NULL) { + error_propagate(errp, local_err); + return -1; } - - 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; - } - 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 efea21983a..e631f1837f 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 @@ -514,7 +514,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 1c3e9b750e..92e0aa9e72 100644 --- a/migration.h +++ b/migration.h @@ -66,9 +66,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); From f37afb5ab1921f42043b5527a517eef95c36acf8 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 2 Oct 2012 10:02:46 +0200 Subject: [PATCH 08/30] migration (outgoing): add error propagation for all protocols 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 --- migration-exec.c | 18 ++++-------------- migration-fd.c | 19 ++++--------------- migration-tcp.c | 13 ++----------- migration-unix.c | 11 ++--------- migration.c | 17 ++++++----------- migration.h | 9 ++++----- 6 files changed, 22 insertions(+), 65 deletions(-) diff --git a/migration-exec.c b/migration-exec.c index 6c97db973c..5f3f4b2c86 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 73351678e0..a7c800a852 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 e8bc76acc6..5e54e68360 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 5387c213ef..34a413ab46 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 e631f1837f..ef171d2b39 100644 --- a/migration.c +++ b/migration.c @@ -487,7 +487,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; @@ -509,27 +508,23 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk, s = migrate_init(¶ms); 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 92e0aa9e72..c805e2885e 100644 --- a/migration.h +++ b/migration.h @@ -59,20 +59,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); From 43eaae28e0394f8fb80848fb40aa5d28c6360321 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 2 Oct 2012 18:21:18 +0200 Subject: [PATCH 09/30] migration (incoming): add error propagation to fd and exec protocols And remove the superfluous integer return value. Reviewed-by: Luiz Capitulino Signed-off-by: Paolo Bonzini --- 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 files changed, 27 insertions(+), 42 deletions(-) diff --git a/migration-exec.c b/migration-exec.c index 5f3f4b2c86..519af57ac7 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 a7c800a852..ce6932d7c3 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 5e54e68360..46f6ac545c 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 34a413ab46..ed3db3a39a 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 ef171d2b39..bd55a15f82 100644 --- a/migration.c +++ b/migration.c @@ -64,26 +64,23 @@ MigrationState *migrate_get_current(void) return ¤t_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 c805e2885e..c3a23cc6c8 100644 --- a/migration.h +++ b/migration.h @@ -49,7 +49,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); @@ -57,19 +57,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 ee3c43ae2f..0597888449 100644 --- a/vl.c +++ b/vl.c @@ -3766,16 +3766,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(); From 87d5f24f3f5edc4b69d071d5966cea0aec7b571d Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 2 Oct 2012 09:16:49 +0200 Subject: [PATCH 10/30] qemu-char: ask and print error information from qemu-sockets 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 Signed-off-by: Paolo Bonzini --- qemu-char.c | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/qemu-char.c b/qemu-char.c index 8ebd5827d9..04b5c236df 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; From f8430e7621b3319f6b94c735c811b2a2448cd6ea Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 2 Oct 2012 10:07:21 +0200 Subject: [PATCH 11/30] nbd: ask and print error information from qemu-sockets 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 --- nbd.c | 39 +++++++++++++++++++++++++++++++-------- 1 file changed, 31 insertions(+), 8 deletions(-) diff --git a/nbd.c b/nbd.c index f61a288369..cec5a9449b 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 From 90119816e36ba019650214e7efeccdac1d4a9e32 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 2 Oct 2012 10:09:14 +0200 Subject: [PATCH 12/30] qemu-ga: ask and print error information from qemu-sockets Reviewed-by: Luiz Capitulino Signed-off-by: Paolo Bonzini --- qga/channel-posix.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/qga/channel-posix.c b/qga/channel-posix.c index e22eee6f67..d152827bcf 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); From c1c1619c8b100f675bc97198218f3fcb0a740921 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 18 Oct 2012 09:06:21 +0200 Subject: [PATCH 13/30] vnc: avoid Yoda conditionals Signed-off-by: Paolo Bonzini --- ui/vnc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ui/vnc.c b/ui/vnc.c index ceade57ce3..64c4092f1f 100644 --- a/ui/vnc.c +++ b/ui/vnc.c @@ -3068,7 +3068,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; @@ -3090,7 +3090,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 { From 1ce52c78ab90c4303bcb110f2c614410386d79a2 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 18 Oct 2012 09:07:05 +0200 Subject: [PATCH 14/30] vnc: introduce a single label for error returns Signed-off-by: Paolo Bonzini --- ui/vnc.c | 42 +++++++++++++++++------------------------- 1 file changed, 17 insertions(+), 25 deletions(-) diff --git a/ui/vnc.c b/ui/vnc.c index 64c4092f1f..72d6f68897 100644 --- a/ui/vnc.c +++ b/ui/vnc.c @@ -2874,8 +2874,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; @@ -2887,9 +2886,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) { @@ -2921,16 +2918,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) @@ -2950,9 +2943,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; } } } @@ -3055,9 +3046,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; @@ -3069,9 +3058,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; @@ -3092,13 +3079,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) From 007fcd3ee9673b3d00baacf3765bd501296155cd Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 18 Oct 2012 09:01:01 +0200 Subject: [PATCH 15/30] vnc: reorganize code for reverse mode Avoid the dance between csock and vs->lsock. Signed-off-by: Paolo Bonzini --- console.h | 2 +- qmp.c | 6 ++---- ui/vnc.c | 20 +++++++++----------- 3 files changed, 12 insertions(+), 16 deletions(-) diff --git a/console.h b/console.h index f990684ef4..6099d8d710 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 36c54c57cf..31bc3bfdd1 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 72d6f68897..46de820c80 100644 --- a/ui/vnc.c +++ b/ui/vnc.c @@ -3053,19 +3053,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; From 2d55f0e817b6fa260282f5b5c533bcd470c75a32 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 2 Oct 2012 10:17:21 +0200 Subject: [PATCH 16/30] vnc: add error propagation to vnc_display_open 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 --- ui/vnc.c | 42 +++++++++++++++++++++++------------------- vl.c | 9 ++++++--- 2 files changed, 29 insertions(+), 22 deletions(-) diff --git a/ui/vnc.c b/ui/vnc.c index 46de820c80..93775694b2 100644 --- a/ui/vnc.c +++ b/ui/vnc.c @@ -2850,7 +2850,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; @@ -2868,11 +2868,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; @@ -2882,10 +2884,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 */ @@ -2916,13 +2918,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 @@ -2942,7 +2944,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; } } @@ -3044,8 +3046,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 @@ -3056,9 +3058,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; @@ -3070,10 +3072,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); @@ -3083,12 +3085,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 0597888449..9f99ef4763 100644 --- a/vl.c +++ b/vl.c @@ -3711,10 +3711,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); } From a12fb8ad5b444174ecf3c3270f93ec59ad177bf3 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 2 Oct 2012 09:18:02 +0200 Subject: [PATCH 17/30] qemu-sockets: include strerror or gai_strerror output in error messages 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 Signed-off-by: Paolo Bonzini --- qemu-sockets.c | 30 +++++++++--------------------- 1 file changed, 9 insertions(+), 21 deletions(-) diff --git a/qemu-sockets.c b/qemu-sockets.c index d510ad1332..9e3d233624 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; From 11663b553b0815b266b6a9060ab9702cf7a5334c Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 2 Oct 2012 09:19:01 +0200 Subject: [PATCH 18/30] qemu-sockets: add error propagation to inet_connect_addr 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 Signed-off-by: Paolo Bonzini --- qemu-sockets.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/qemu-sockets.c b/qemu-sockets.c index 9e3d233624..88d58fe70c 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; From 4f085c822947068c785be8c1f27d1ad4e3215149 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 2 Oct 2012 09:25:14 +0200 Subject: [PATCH 19/30] qemu-sockets: add error propagation to inet_dgram_opts 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 Signed-off-by: Paolo Bonzini --- qemu-char.c | 1 - qemu-sockets.c | 34 ++++++++-------------------------- 2 files changed, 8 insertions(+), 27 deletions(-) diff --git a/qemu-char.c b/qemu-char.c index 04b5c236df..afe2bfb4dd 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 88d58fe70c..078f7c17c3 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; } From 2f002c43ebded28604c26787c9215c4d3594f0ec Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Wed, 19 Sep 2012 13:22:21 +0200 Subject: [PATCH 20/30] qemu-sockets: add error propagation to inet_parse Reviewed-by: Luiz Capitulino Signed-off-by: Paolo Bonzini --- qemu-sockets.c | 41 +++++++++++++++++++++-------------------- 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/qemu-sockets.c b/qemu-sockets.c index 078f7c17c3..fb9c4c9587 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; From 58899664de29c250229f8d306fcf04f852cf5cc6 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Wed, 19 Sep 2012 13:54:39 +0200 Subject: [PATCH 21/30] qemu-sockets: add error propagation to Unix socket functions 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 Signed-off-by: Paolo Bonzini --- qemu-sockets.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/qemu-sockets.c b/qemu-sockets.c index fb9c4c9587..daff8e6a39 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; } From 3f8b11bc7d86a1952311565f089063d544d5a2f7 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Fri, 19 Oct 2012 11:34:18 +0200 Subject: [PATCH 22/30] vnc: drop QERR_VNC_SERVER_FAILED We now always return "nice" error messages in errp when we goto fail. Drop the default error message. Signed-off-by: Paolo Bonzini --- qerror.h | 3 --- ui/vnc.c | 3 --- 2 files changed, 6 deletions(-) diff --git a/qerror.h b/qerror.h index c91708cc3c..f23b97892d 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 93775694b2..d0ffcc54af 100644 --- a/ui/vnc.c +++ b/ui/vnc.c @@ -3088,9 +3088,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; } From 69758c22e0814f77c7a286ce65a68270ce71284e Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Wed, 19 Sep 2012 15:11:05 +0200 Subject: [PATCH 23/30] build: add QAPI files to the tools We need them because qemu-sockets will soon be using SocketAddress. Acked-by: Luiz Capitulino Signed-off-by: Paolo Bonzini --- Makefile | 3 +-- Makefile.objs | 3 ++- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index 88285a474b..9f599bc255 100644 --- a/Makefile +++ b/Makefile @@ -164,8 +164,7 @@ tools-obj-y = $(oslib-obj-y) $(trace-obj-y) qemu-tool.o qemu-timer.o \ iohandler.o cutils.o iov.o async.o tools-obj-$(CONFIG_POSIX) += compatfd.o -qemu-img$(EXESUF): qemu-img.o $(tools-obj-y) $(block-obj-y) $(qapi-obj-y) \ - qapi-visit.o qapi-types.o +qemu-img$(EXESUF): qemu-img.o $(tools-obj-y) $(block-obj-y) qemu-nbd$(EXESUF): qemu-nbd.o $(tools-obj-y) $(block-obj-y) qemu-io$(EXESUF): qemu-io.o cmd.o $(tools-obj-y) $(block-obj-y) diff --git a/Makefile.objs b/Makefile.objs index 74b35422ce..3f16d67dac 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 From 5be8c759f008947514fbf7d8df9a30eebe496b6d Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Wed, 19 Sep 2012 14:03:35 +0200 Subject: [PATCH 24/30] qapi: add socket address types Acked-by: Luiz Capitulino Signed-off-by: Paolo Bonzini --- qapi-schema.json | 53 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/qapi-schema.json b/qapi-schema.json index c615ee212d..8468567bfb 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -2518,6 +2518,59 @@ 'id': 'str', '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: # From 879e45c72da1569e07fbbc6a1aa2a708ea796044 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Wed, 19 Sep 2012 13:51:46 +0200 Subject: [PATCH 25/30] qemu-sockets: return InetSocketAddress from inet_parse Reviewed-by: Luiz Capitulino Signed-off-by: Paolo Bonzini --- qemu-sockets.c | 121 ++++++++++++++++++++++++++++++++----------------- 1 file changed, 79 insertions(+), 42 deletions(-) diff --git a/qemu-sockets.c b/qemu-sockets.c index daff8e6a39..909c65f3fb 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; } From 0ef3dd6c2de1fa4f80c5aa71fb5fdada0f35cc9a Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 23 Oct 2012 22:36:08 +0200 Subject: [PATCH 26/30] tests: do not include tools-obj-y Signed-off-by: Paolo Bonzini --- tests/Makefile | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/Makefile b/tests/Makefile index 26a67ce6f5..86c9b79ebe 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -42,11 +42,11 @@ test-qapi-obj-y += module.o $(test-obj-y): QEMU_INCLUDES += -Itests -tests/check-qint$(EXESUF): tests/check-qint.o qint.o $(tools-obj-y) -tests/check-qstring$(EXESUF): tests/check-qstring.o qstring.o $(tools-obj-y) -tests/check-qdict$(EXESUF): tests/check-qdict.o qdict.o qfloat.o qint.o qstring.o qbool.o qlist.o $(tools-obj-y) -tests/check-qlist$(EXESUF): tests/check-qlist.o qlist.o qint.o $(tools-obj-y) -tests/check-qfloat$(EXESUF): tests/check-qfloat.o qfloat.o $(tools-obj-y) +tests/check-qint$(EXESUF): tests/check-qint.o qint.o +tests/check-qstring$(EXESUF): tests/check-qstring.o qstring.o +tests/check-qdict$(EXESUF): tests/check-qdict.o qdict.o qfloat.o qint.o qstring.o qbool.o qlist.o +tests/check-qlist$(EXESUF): tests/check-qlist.o qlist.o qint.o +tests/check-qfloat$(EXESUF): tests/check-qfloat.o qfloat.o tests/check-qjson$(EXESUF): tests/check-qjson.o $(qobject-obj-y) $(tools-obj-y) tests/test-coroutine$(EXESUF): tests/test-coroutine.o $(coroutine-obj-y) $(tools-obj-y) tests/test-iov$(EXESUF): tests/test-iov.o iov.o From 101f9cbc2b73340197ec610c095fd69f0b1413ed Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 23 Oct 2012 21:31:53 +0200 Subject: [PATCH 27/30] qemu-sockets: add socket_listen, socket_connect, socket_parse 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 Signed-off-by: Paolo Bonzini --- Makefile | 2 +- qemu-sockets.c | 99 ++++++++++++++++++++++++++++++++++++++++++++++++++ qemu-tool.c | 6 +++ qemu_socket.h | 5 +++ 4 files changed, 111 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 9f599bc255..17e2d58ce5 100644 --- a/Makefile +++ b/Makefile @@ -161,7 +161,7 @@ qemu-img.o: qemu-img-cmds.h tools-obj-y = $(oslib-obj-y) $(trace-obj-y) qemu-tool.o qemu-timer.o \ qemu-timer-common.o main-loop.o notify.o \ - iohandler.o cutils.o iov.o async.o + iohandler.o cutils.o iov.o async.o error.o tools-obj-$(CONFIG_POSIX) += compatfd.o qemu-img$(EXESUF): qemu-img.o $(tools-obj-y) $(block-obj-y) diff --git a/qemu-sockets.c b/qemu-sockets.c index 909c65f3fb..cfed9c5a5b 100644 --- a/qemu-sockets.c +++ b/qemu-sockets.c @@ -22,6 +22,7 @@ #include #include +#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 f2f98138ce..da4c05aaf7 100644 --- a/qemu-tool.c +++ b/qemu-tool.c @@ -38,6 +38,12 @@ const char *qemu_get_vm_name(void) Monitor *cur_mon; +int monitor_get_fd(Monitor *mon, const char *name, Error **errp) +{ + error_setg(errp, "only QEMU supports file descriptor passing"); + return -1; +} + void vm_stop(RunState state) { abort(); diff --git a/qemu_socket.h b/qemu_socket.h index 89a5feb7ec..02490ad06c 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); From 3cbc002c34aa85ea952ee9b169a3ff97d350516a Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Fri, 19 Oct 2012 11:36:48 +0200 Subject: [PATCH 28/30] block: prepare code for adding block notifiers 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 --- block.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/block.c b/block.c index e95f613aa4..2e4ddea222 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; } From d7d512f60979681c27597f1b1277e03505c1de08 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 23 Aug 2012 11:20:36 +0200 Subject: [PATCH 29/30] block: add close notifiers 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 Signed-off-by: Paolo Bonzini --- Makefile.objs | 4 ++-- block.c | 9 +++++++++ block.h | 1 + block_int.h | 2 ++ 4 files changed, 14 insertions(+), 2 deletions(-) diff --git a/Makefile.objs b/Makefile.objs index 3f16d67dac..ca67885778 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 2e4ddea222..56426a936a 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 e2d89d7bc1..aa608a8c99 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 f4bae04401..cedabbdd13 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; From 6dd844db4a49a367cc15cd0e8bfb72cfc6652766 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Wed, 22 Aug 2012 16:43:07 +0200 Subject: [PATCH 30/30] qmp: add NBD server commands 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 Signed-off-by: Paolo Bonzini --- Makefile.objs | 2 +- blockdev-nbd.c | 119 +++++++++++++++++++++++++++++++++++++++++++++++ qapi-schema.json | 43 +++++++++++++++++ qmp-commands.hx | 16 +++++++ 4 files changed, 179 insertions(+), 1 deletion(-) create mode 100644 blockdev-nbd.c diff --git a/Makefile.objs b/Makefile.objs index ca67885778..9eca179903 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 0000000000..8031813071 --- /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 + * + * 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 8468567bfb..6fd263e646 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -2863,3 +2863,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 5ba8c48cb4..ebe9a78ca9 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -2551,6 +2551,22 @@ EQMP .mhandler.cmd_new = qmp_qom_get, }, + { + .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",