From 8fbf66121253969e59fe40de66e55bb2f27c5090 Mon Sep 17 00:00:00 2001 From: Felipe Franciosi <felipe@nutanix.com> Date: Thu, 29 Sep 2016 08:52:35 -0700 Subject: [PATCH 01/11] io: Fix double shift usages on QIOChannel features When QIOChannels were introduced in 666a3af9, the feature bits were already defined shifted. However, when using them, the code was shifting them again. The incorrect use was consistent until 74b6ce43, where QIO_CHANNEL_FEATURE_LISTEN was defined shifted but tested unshifted. This patch changes the definition to be unshifted and fixes the incorrect usage introduced on 74b6ce43. Signed-off-by: Felipe Franciosi <felipe@nutanix.com> Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- include/io/channel.h | 6 +++--- io/channel-socket.c | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/include/io/channel.h b/include/io/channel.h index 752e89f4dc..5368604c8e 100644 --- a/include/io/channel.h +++ b/include/io/channel.h @@ -40,9 +40,9 @@ typedef struct QIOChannelClass QIOChannelClass; typedef enum QIOChannelFeature QIOChannelFeature; enum QIOChannelFeature { - QIO_CHANNEL_FEATURE_FD_PASS = (1 << 0), - QIO_CHANNEL_FEATURE_SHUTDOWN = (1 << 1), - QIO_CHANNEL_FEATURE_LISTEN = (1 << 2), + QIO_CHANNEL_FEATURE_FD_PASS, + QIO_CHANNEL_FEATURE_SHUTDOWN, + QIO_CHANNEL_FEATURE_LISTEN, }; diff --git a/io/channel-socket.c b/io/channel-socket.c index 196a4f18f7..6710b2ee96 100644 --- a/io/channel-socket.c +++ b/io/channel-socket.c @@ -403,7 +403,7 @@ static void qio_channel_socket_finalize(Object *obj) QIOChannelSocket *ioc = QIO_CHANNEL_SOCKET(obj); if (ioc->fd != -1) { - if (QIO_CHANNEL(ioc)->features & QIO_CHANNEL_FEATURE_LISTEN) { + if (QIO_CHANNEL(ioc)->features & (1 << QIO_CHANNEL_FEATURE_LISTEN)) { Error *err = NULL; socket_listen_cleanup(ioc->fd, &err); From e413ae0c0492c10d9277a1155ecc21fbbf0e2bc7 Mon Sep 17 00:00:00 2001 From: Felipe Franciosi <felipe@nutanix.com> Date: Thu, 29 Sep 2016 08:52:36 -0700 Subject: [PATCH 02/11] io: Use qio_channel_has_feature() where applicable Parts of the code have been testing QIOChannel features directly with a logical AND. This patch makes it all consistent by using the qio_channel_has_feature() function to test if a feature is present. Signed-off-by: Felipe Franciosi <felipe@nutanix.com> Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- io/channel-socket.c | 3 ++- io/channel-tls.c | 2 +- io/channel-websock.c | 2 +- io/channel.c | 4 ++-- 4 files changed, 6 insertions(+), 5 deletions(-) diff --git a/io/channel-socket.c b/io/channel-socket.c index 6710b2ee96..8fc6e5aaaa 100644 --- a/io/channel-socket.c +++ b/io/channel-socket.c @@ -403,7 +403,8 @@ static void qio_channel_socket_finalize(Object *obj) QIOChannelSocket *ioc = QIO_CHANNEL_SOCKET(obj); if (ioc->fd != -1) { - if (QIO_CHANNEL(ioc)->features & (1 << QIO_CHANNEL_FEATURE_LISTEN)) { + QIOChannel *ioc_local = QIO_CHANNEL(ioc); + if (qio_channel_has_feature(ioc_local, QIO_CHANNEL_FEATURE_LISTEN)) { Error *err = NULL; socket_listen_cleanup(ioc->fd, &err); diff --git a/io/channel-tls.c b/io/channel-tls.c index 9a8525c816..f7bb0e3698 100644 --- a/io/channel-tls.c +++ b/io/channel-tls.c @@ -111,7 +111,7 @@ qio_channel_tls_new_client(QIOChannel *master, ioc = QIO_CHANNEL(tioc); tioc->master = master; - if (master->features & (1 << QIO_CHANNEL_FEATURE_SHUTDOWN)) { + if (qio_channel_has_feature(master, QIO_CHANNEL_FEATURE_SHUTDOWN)) { ioc->features |= (1 << QIO_CHANNEL_FEATURE_SHUTDOWN); } object_ref(OBJECT(master)); diff --git a/io/channel-websock.c b/io/channel-websock.c index 533bd4b3b5..75df03ef29 100644 --- a/io/channel-websock.c +++ b/io/channel-websock.c @@ -497,7 +497,7 @@ qio_channel_websock_new_server(QIOChannel *master) ioc = QIO_CHANNEL(wioc); wioc->master = master; - if (master->features & (1 << QIO_CHANNEL_FEATURE_SHUTDOWN)) { + if (qio_channel_has_feature(master, QIO_CHANNEL_FEATURE_SHUTDOWN)) { ioc->features |= (1 << QIO_CHANNEL_FEATURE_SHUTDOWN); } object_ref(OBJECT(master)); diff --git a/io/channel.c b/io/channel.c index 923c4651ca..e50325c3f7 100644 --- a/io/channel.c +++ b/io/channel.c @@ -40,7 +40,7 @@ ssize_t qio_channel_readv_full(QIOChannel *ioc, QIOChannelClass *klass = QIO_CHANNEL_GET_CLASS(ioc); if ((fds || nfds) && - !(ioc->features & (1 << QIO_CHANNEL_FEATURE_FD_PASS))) { + !qio_channel_has_feature(ioc, QIO_CHANNEL_FEATURE_FD_PASS)) { error_setg_errno(errp, EINVAL, "Channel does not support file descriptor passing"); return -1; @@ -60,7 +60,7 @@ ssize_t qio_channel_writev_full(QIOChannel *ioc, QIOChannelClass *klass = QIO_CHANNEL_GET_CLASS(ioc); if ((fds || nfds) && - !(ioc->features & (1 << QIO_CHANNEL_FEATURE_FD_PASS))) { + !qio_channel_has_feature(ioc, QIO_CHANNEL_FEATURE_FD_PASS)) { error_setg_errno(errp, EINVAL, "Channel does not support file descriptor passing"); return -1; From d8d3c7cc672d89b26180a404d6f0b03494160cf5 Mon Sep 17 00:00:00 2001 From: Felipe Franciosi <felipe@nutanix.com> Date: Thu, 29 Sep 2016 08:52:37 -0700 Subject: [PATCH 03/11] io: Introduce a qio_channel_set_feature() helper Testing QIOChannel feature support can be done with a helper called qio_channel_has_feature(). Setting feature support, however, was done manually with a logical OR. This patch introduces a new helper called qio_channel_set_feature() and makes use of it where applicable. Signed-off-by: Felipe Franciosi <felipe@nutanix.com> Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- include/io/channel.h | 10 ++++++++++ io/channel-socket.c | 9 +++++---- io/channel-tls.c | 2 +- io/channel-websock.c | 2 +- io/channel.c | 7 +++++++ 5 files changed, 24 insertions(+), 6 deletions(-) diff --git a/include/io/channel.h b/include/io/channel.h index 5368604c8e..cf1c6223fa 100644 --- a/include/io/channel.h +++ b/include/io/channel.h @@ -148,6 +148,16 @@ struct QIOChannelClass { bool qio_channel_has_feature(QIOChannel *ioc, QIOChannelFeature feature); +/** + * qio_channel_set_feature: + * @ioc: the channel object + * @feature: the feature to set support for + * + * Add channel support for the feature named in @feature. + */ +void qio_channel_set_feature(QIOChannel *ioc, + QIOChannelFeature feature); + /** * qio_channel_readv_full: * @ioc: the channel object diff --git a/io/channel-socket.c b/io/channel-socket.c index 8fc6e5aaaa..75cbca3c55 100644 --- a/io/channel-socket.c +++ b/io/channel-socket.c @@ -55,7 +55,7 @@ qio_channel_socket_new(void) sioc->fd = -1; ioc = QIO_CHANNEL(sioc); - ioc->features |= (1 << QIO_CHANNEL_FEATURE_SHUTDOWN); + qio_channel_set_feature(ioc, QIO_CHANNEL_FEATURE_SHUTDOWN); #ifdef WIN32 ioc->event = CreateEvent(NULL, FALSE, FALSE, NULL); @@ -107,12 +107,12 @@ qio_channel_socket_set_fd(QIOChannelSocket *sioc, #ifndef WIN32 if (sioc->localAddr.ss_family == AF_UNIX) { QIOChannel *ioc = QIO_CHANNEL(sioc); - ioc->features |= (1 << QIO_CHANNEL_FEATURE_FD_PASS); + qio_channel_set_feature(ioc, QIO_CHANNEL_FEATURE_FD_PASS); } #endif /* WIN32 */ if (getsockopt(fd, SOL_SOCKET, SO_ACCEPTCONN, &val, &len) == 0 && val) { QIOChannel *ioc = QIO_CHANNEL(sioc); - ioc->features |= (1 << QIO_CHANNEL_FEATURE_LISTEN); + qio_channel_set_feature(ioc, QIO_CHANNEL_FEATURE_LISTEN); } return 0; @@ -380,7 +380,8 @@ qio_channel_socket_accept(QIOChannelSocket *ioc, #ifndef WIN32 if (cioc->localAddr.ss_family == AF_UNIX) { - QIO_CHANNEL(cioc)->features |= (1 << QIO_CHANNEL_FEATURE_FD_PASS); + QIOChannel *ioc_local = QIO_CHANNEL(cioc); + qio_channel_set_feature(ioc_local, QIO_CHANNEL_FEATURE_FD_PASS); } #endif /* WIN32 */ diff --git a/io/channel-tls.c b/io/channel-tls.c index f7bb0e3698..d24dc8c613 100644 --- a/io/channel-tls.c +++ b/io/channel-tls.c @@ -112,7 +112,7 @@ qio_channel_tls_new_client(QIOChannel *master, tioc->master = master; if (qio_channel_has_feature(master, QIO_CHANNEL_FEATURE_SHUTDOWN)) { - ioc->features |= (1 << QIO_CHANNEL_FEATURE_SHUTDOWN); + qio_channel_set_feature(ioc, QIO_CHANNEL_FEATURE_SHUTDOWN); } object_ref(OBJECT(master)); diff --git a/io/channel-websock.c b/io/channel-websock.c index 75df03ef29..f45bced82a 100644 --- a/io/channel-websock.c +++ b/io/channel-websock.c @@ -498,7 +498,7 @@ qio_channel_websock_new_server(QIOChannel *master) wioc->master = master; if (qio_channel_has_feature(master, QIO_CHANNEL_FEATURE_SHUTDOWN)) { - ioc->features |= (1 << QIO_CHANNEL_FEATURE_SHUTDOWN); + qio_channel_set_feature(ioc, QIO_CHANNEL_FEATURE_SHUTDOWN); } object_ref(OBJECT(master)); diff --git a/io/channel.c b/io/channel.c index e50325c3f7..d1f1ae5157 100644 --- a/io/channel.c +++ b/io/channel.c @@ -30,6 +30,13 @@ bool qio_channel_has_feature(QIOChannel *ioc, } +void qio_channel_set_feature(QIOChannel *ioc, + QIOChannelFeature feature) +{ + ioc->features |= (1 << feature); +} + + ssize_t qio_channel_readv_full(QIOChannel *ioc, const struct iovec *iov, size_t niov, From bf5352082727df7207c54cf3fc5fb608dc2b1fda Mon Sep 17 00:00:00 2001 From: "Daniel P. Berrange" <berrange@redhat.com> Date: Wed, 26 Oct 2016 18:20:20 +0200 Subject: [PATCH 04/11] io: set LISTEN flag explicitly for listen sockets The SO_ACCEPTCONN ioctl is not portable across OS, with some BSD versions and OS-X not supporting it. There is no viable alternative to this, so instead just set the feature explicitly when creating a listener socket. The current users of qio_channel_socket_new_fd() won't ever be given a listening socket, so there's no problem with no auto-detecting it in this scenario Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- io/channel-socket.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/io/channel-socket.c b/io/channel-socket.c index 75cbca3c55..d7e03f6266 100644 --- a/io/channel-socket.c +++ b/io/channel-socket.c @@ -72,9 +72,6 @@ qio_channel_socket_set_fd(QIOChannelSocket *sioc, int fd, Error **errp) { - int val; - socklen_t len = sizeof(val); - if (sioc->fd != -1) { error_setg(errp, "Socket is already open"); return -1; @@ -110,10 +107,6 @@ qio_channel_socket_set_fd(QIOChannelSocket *sioc, qio_channel_set_feature(ioc, QIO_CHANNEL_FEATURE_FD_PASS); } #endif /* WIN32 */ - if (getsockopt(fd, SOL_SOCKET, SO_ACCEPTCONN, &val, &len) == 0 && val) { - QIOChannel *ioc = QIO_CHANNEL(sioc); - qio_channel_set_feature(ioc, QIO_CHANNEL_FEATURE_LISTEN); - } return 0; @@ -220,6 +213,7 @@ int qio_channel_socket_listen_sync(QIOChannelSocket *ioc, close(fd); return -1; } + qio_channel_set_feature(QIO_CHANNEL(ioc), QIO_CHANNEL_FEATURE_LISTEN); return 0; } From af8096b2c3b16de3f6237f1ead6a657a7565272f Mon Sep 17 00:00:00 2001 From: Felipe Franciosi <felipe@nutanix.com> Date: Thu, 29 Sep 2016 08:52:38 -0700 Subject: [PATCH 05/11] io: Add a QIOChannelSocket cleanup test This patch adds a test to verify that the QIOChannel framework will not unlink a filesystem unix socket unless the _FEATURE_LISTEN bit is set. Due to a bug introduced in 74b6ce43, the framework would unlink the entry if the _FEATURE_SHUTDOWN bit was set, regardless of the presence of _FEATURE_LISTEN. Signed-off-by: Felipe Franciosi <felipe@nutanix.com> Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- tests/test-io-channel-socket.c | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/tests/test-io-channel-socket.c b/tests/test-io-channel-socket.c index f73e063d7d..aa88c3cf45 100644 --- a/tests/test-io-channel-socket.c +++ b/tests/test-io-channel-socket.c @@ -491,6 +491,37 @@ static void test_io_channel_unix_fd_pass(void) } g_free(fdrecv); } + +static void test_io_channel_unix_listen_cleanup(void) +{ + QIOChannelSocket *ioc; + struct sockaddr_un un; + int sock; + +#define TEST_SOCKET "test-io-channel-socket.sock" + + ioc = qio_channel_socket_new(); + + /* Manually bind ioc without calling the qio api to avoid setting + * the LISTEN feature */ + sock = qemu_socket(PF_UNIX, SOCK_STREAM, 0); + memset(&un, 0, sizeof(un)); + un.sun_family = AF_UNIX; + snprintf(un.sun_path, sizeof(un.sun_path), "%s", TEST_SOCKET); + unlink(TEST_SOCKET); + bind(sock, (struct sockaddr *)&un, sizeof(un)); + ioc->fd = sock; + ioc->localAddrLen = sizeof(ioc->localAddr); + getsockname(sock, (struct sockaddr *)&ioc->localAddr, + &ioc->localAddrLen); + + g_assert(g_file_test(TEST_SOCKET, G_FILE_TEST_EXISTS)); + object_unref(OBJECT(ioc)); + g_assert(g_file_test(TEST_SOCKET, G_FILE_TEST_EXISTS)); + + unlink(TEST_SOCKET); +} + #endif /* _WIN32 */ @@ -562,6 +593,8 @@ int main(int argc, char **argv) test_io_channel_unix_async); g_test_add_func("/io/channel/socket/unix-fd-pass", test_io_channel_unix_fd_pass); + g_test_add_func("/io/channel/socket/unix-listen-cleanup", + test_io_channel_unix_listen_cleanup); #endif /* _WIN32 */ return g_test_run(); From 20f4aa265ec8442be66f00ee3986a92018b44b7b Mon Sep 17 00:00:00 2001 From: "Daniel P. Berrange" <berrange@redhat.com> Date: Fri, 30 Sep 2016 11:50:18 +0100 Subject: [PATCH 06/11] io: add ability to set a name for IO channels The GSource object has ability to have a name, which is useful when debugging performance problems with the mainloop event callbacks that take too long. By associating a name with a QIOChannel object, we can then set the name on any GSource associated with the channel. Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- include/glib-compat.h | 7 +++++++ include/io/channel.h | 13 +++++++++++++ io/channel.c | 24 +++++++++++++++++++----- 3 files changed, 39 insertions(+), 5 deletions(-) diff --git a/include/glib-compat.h b/include/glib-compat.h index 8093163bee..9dfe952c5d 100644 --- a/include/glib-compat.h +++ b/include/glib-compat.h @@ -304,4 +304,11 @@ static inline void g_slist_free_full(GSList *list, GDestroyNotify free_func) } #endif +#if !GLIB_CHECK_VERSION(2, 26, 0) +static inline void g_source_set_name(GSource *source, const char *name) +{ + /* This is just a debugging aid, so leaving it a no-op */ +} +#endif + #endif diff --git a/include/io/channel.h b/include/io/channel.h index cf1c6223fa..32a9470794 100644 --- a/include/io/channel.h +++ b/include/io/channel.h @@ -79,6 +79,7 @@ typedef gboolean (*QIOChannelFunc)(QIOChannel *ioc, struct QIOChannel { Object parent; unsigned int features; /* bitmask of QIOChannelFeatures */ + char *name; #ifdef _WIN32 HANDLE event; /* For use with GSource on Win32 */ #endif @@ -158,6 +159,18 @@ bool qio_channel_has_feature(QIOChannel *ioc, void qio_channel_set_feature(QIOChannel *ioc, QIOChannelFeature feature); +/** + * qio_channel_set_name: + * @ioc: the channel object + * @name: the name of the channel + * + * Sets the name of the channel, which serves as an aid + * to debugging. The name is used when creating GSource + * watches for this channel. + */ +void qio_channel_set_name(QIOChannel *ioc, + const char *name); + /** * qio_channel_readv_full: * @ioc: the channel object diff --git a/io/channel.c b/io/channel.c index d1f1ae5157..80924c1772 100644 --- a/io/channel.c +++ b/io/channel.c @@ -37,6 +37,14 @@ void qio_channel_set_feature(QIOChannel *ioc, } +void qio_channel_set_name(QIOChannel *ioc, + const char *name) +{ + g_free(ioc->name); + ioc->name = g_strdup(name); +} + + ssize_t qio_channel_readv_full(QIOChannel *ioc, const struct iovec *iov, size_t niov, @@ -136,7 +144,13 @@ GSource *qio_channel_create_watch(QIOChannel *ioc, GIOCondition condition) { QIOChannelClass *klass = QIO_CHANNEL_GET_CLASS(ioc); - return klass->io_create_watch(ioc, condition); + GSource *ret = klass->io_create_watch(ioc, condition); + + if (ioc->name) { + g_source_set_name(ret, ioc->name); + } + + return ret; } @@ -282,24 +296,24 @@ void qio_channel_wait(QIOChannel *ioc, } -#ifdef _WIN32 static void qio_channel_finalize(Object *obj) { QIOChannel *ioc = QIO_CHANNEL(obj); + g_free(ioc->name); + +#ifdef _WIN32 if (ioc->event) { CloseHandle(ioc->event); } -} #endif +} static const TypeInfo qio_channel_info = { .parent = TYPE_OBJECT, .name = TYPE_QIO_CHANNEL, .instance_size = sizeof(QIOChannel), -#ifdef _WIN32 .instance_finalize = qio_channel_finalize, -#endif .abstract = true, .class_size = sizeof(QIOChannelClass), }; From 0d73f7253e4837603dbcbb7af0706d4d57124b5e Mon Sep 17 00:00:00 2001 From: "Daniel P. Berrange" <berrange@redhat.com> Date: Fri, 30 Sep 2016 11:57:14 +0100 Subject: [PATCH 07/11] nbd: set name for all I/O channels created Ensure that all I/O channels created for NBD are given names to distinguish their respective roles. Acked-by: Paolo Bonzini <pbonzini@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- block/nbd.c | 1 + blockdev-nbd.c | 3 +++ nbd/client.c | 1 + nbd/server.c | 1 + 4 files changed, 6 insertions(+) diff --git a/block/nbd.c b/block/nbd.c index 6bc06d6198..1ec64ab95e 100644 --- a/block/nbd.c +++ b/block/nbd.c @@ -248,6 +248,7 @@ static QIOChannelSocket *nbd_establish_connection(SocketAddress *saddr, Error *local_err = NULL; sioc = qio_channel_socket_new(); + qio_channel_set_name(QIO_CHANNEL(sioc), "nbd-client"); qio_channel_socket_connect_sync(sioc, saddr, diff --git a/blockdev-nbd.c b/blockdev-nbd.c index ca41cc6fdd..81bca1760f 100644 --- a/blockdev-nbd.c +++ b/blockdev-nbd.c @@ -44,6 +44,7 @@ static gboolean nbd_accept(QIOChannel *ioc, GIOCondition condition, return TRUE; } + qio_channel_set_name(QIO_CHANNEL(cioc), "nbd-server"); nbd_client_new(NULL, cioc, nbd_server->tlscreds, NULL, nbd_client_put); @@ -111,6 +112,8 @@ void qmp_nbd_server_start(SocketAddress *addr, nbd_server = g_new0(NBDServerData, 1); nbd_server->watch = -1; nbd_server->listen_ioc = qio_channel_socket_new(); + qio_channel_set_name(QIO_CHANNEL(nbd_server->listen_ioc), + "nbd-listener"); if (qio_channel_socket_listen_sync( nbd_server->listen_ioc, addr, errp) < 0) { goto error; diff --git a/nbd/client.c b/nbd/client.c index a92f1e2275..f6db8369b3 100644 --- a/nbd/client.c +++ b/nbd/client.c @@ -387,6 +387,7 @@ static QIOChannel *nbd_receive_starttls(QIOChannel *ioc, if (!tioc) { return NULL; } + qio_channel_set_name(QIO_CHANNEL(tioc), "nbd-client-tls"); data.loop = g_main_loop_new(g_main_context_default(), FALSE); TRACE("Starting TLS handshake"); qio_channel_tls_handshake(tioc, diff --git a/nbd/server.c b/nbd/server.c index 472f584c32..36bcafcd50 100644 --- a/nbd/server.c +++ b/nbd/server.c @@ -349,6 +349,7 @@ static QIOChannel *nbd_negotiate_handle_starttls(NBDClient *client, return NULL; } + qio_channel_set_name(QIO_CHANNEL(tioc), "nbd-server-tls"); TRACE("Starting TLS handshake"); data.loop = g_main_loop_new(g_main_context_default(), FALSE); qio_channel_tls_handshake(tioc, From e93a68e102ffc8f8316ce24a57f094734dc4d8f7 Mon Sep 17 00:00:00 2001 From: "Daniel P. Berrange" <berrange@redhat.com> Date: Fri, 30 Sep 2016 11:57:14 +0100 Subject: [PATCH 08/11] char: set name for all I/O channels created Ensure that all I/O channels created for character devices are given names to distinguish their respective roles. Acked-by: Paolo Bonzini <pbonzini@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- include/glib-compat.h | 4 +++ qemu-char.c | 77 +++++++++++++++++++++++++++++++++++++++---- 2 files changed, 74 insertions(+), 7 deletions(-) diff --git a/include/glib-compat.h b/include/glib-compat.h index 9dfe952c5d..3f8370b3e4 100644 --- a/include/glib-compat.h +++ b/include/glib-compat.h @@ -309,6 +309,10 @@ static inline void g_source_set_name(GSource *source, const char *name) { /* This is just a debugging aid, so leaving it a no-op */ } +static inline void g_source_set_name_by_id(guint tag, const char *name) +{ + /* This is just a debugging aid, so leaving it a no-op */ +} #endif #endif diff --git a/qemu-char.c b/qemu-char.c index d83a89618e..4d0986ced3 100644 --- a/qemu-char.c +++ b/qemu-char.c @@ -934,7 +934,8 @@ static GSourceFuncs io_watch_poll_funcs = { }; /* Can only be used for read */ -static guint io_add_watch_poll(QIOChannel *ioc, +static guint io_add_watch_poll(CharDriverState *chr, + QIOChannel *ioc, IOCanReadHandler *fd_can_read, QIOChannelFunc fd_read, gpointer user_data, @@ -942,6 +943,7 @@ static guint io_add_watch_poll(QIOChannel *ioc, { IOWatchPoll *iwp; int tag; + char *name; iwp = (IOWatchPoll *) g_source_new(&io_watch_poll_funcs, sizeof(IOWatchPoll)); @@ -952,6 +954,10 @@ static guint io_add_watch_poll(QIOChannel *ioc, iwp->src = NULL; iwp->context = context; + name = g_strdup_printf("chardev-iowatch-%s", chr->label); + g_source_set_name((GSource *)iwp, name); + g_free(name); + tag = g_source_attach(&iwp->parent, context); g_source_unref(&iwp->parent); return tag; @@ -1091,7 +1097,7 @@ static void fd_chr_update_read_handler(CharDriverState *chr, remove_fd_in_watch(chr); if (s->ioc_in) { - chr->fd_in_tag = io_add_watch_poll(s->ioc_in, + chr->fd_in_tag = io_add_watch_poll(chr, s->ioc_in, fd_chr_read_poll, fd_chr_read, chr, context); @@ -1120,6 +1126,7 @@ static CharDriverState *qemu_chr_open_fd(int fd_in, int fd_out, { CharDriverState *chr; FDCharDriver *s; + char *name; chr = qemu_chr_alloc(backend, errp); if (!chr) { @@ -1127,7 +1134,13 @@ static CharDriverState *qemu_chr_open_fd(int fd_in, int fd_out, } s = g_new0(FDCharDriver, 1); s->ioc_in = QIO_CHANNEL(qio_channel_file_new_fd(fd_in)); + name = g_strdup_printf("chardev-file-in-%s", chr->label); + qio_channel_set_name(QIO_CHANNEL(s->ioc_in), name); + g_free(name); s->ioc_out = QIO_CHANNEL(qio_channel_file_new_fd(fd_out)); + name = g_strdup_printf("chardev-file-out-%s", chr->label); + qio_channel_set_name(QIO_CHANNEL(s->ioc_out), name); + g_free(name); qemu_set_nonblock(fd_out); s->chr = chr; chr->opaque = s; @@ -1305,6 +1318,7 @@ static gboolean pty_chr_timer(gpointer opaque) static void pty_chr_rearm_timer(CharDriverState *chr, int ms) { PtyCharDriver *s = chr->opaque; + char *name; if (s->timer_tag) { g_source_remove(s->timer_tag); @@ -1312,10 +1326,14 @@ static void pty_chr_rearm_timer(CharDriverState *chr, int ms) } if (ms == 1000) { + name = g_strdup_printf("pty-timer-secs-%s", chr->label); s->timer_tag = g_timeout_add_seconds(1, pty_chr_timer, chr); } else { + name = g_strdup_printf("pty-timer-ms-%s", chr->label); s->timer_tag = g_timeout_add(ms, pty_chr_timer, chr); } + g_source_set_name_by_id(s->timer_tag, name); + g_free(name); } /* Called with chr_write_lock held. */ @@ -1444,7 +1462,7 @@ static void pty_chr_state(CharDriverState *chr, int connected) s->open_tag = g_idle_add(qemu_chr_be_generic_open_func, chr); } if (!chr->fd_in_tag) { - chr->fd_in_tag = io_add_watch_poll(s->ioc, + chr->fd_in_tag = io_add_watch_poll(chr, s->ioc, pty_chr_read_poll, pty_chr_read, chr, NULL); @@ -1478,6 +1496,7 @@ static CharDriverState *qemu_chr_open_pty(const char *id, int master_fd, slave_fd; char pty_name[PATH_MAX]; ChardevCommon *common = backend->u.pty.data; + char *name; master_fd = qemu_openpty_raw(&slave_fd, pty_name); if (master_fd < 0) { @@ -1510,6 +1529,9 @@ static CharDriverState *qemu_chr_open_pty(const char *id, chr->explicit_be_open = true; s->ioc = QIO_CHANNEL(qio_channel_file_new_fd(master_fd)); + name = g_strdup_printf("chardev-pty-%s", chr->label); + qio_channel_set_name(QIO_CHANNEL(s->ioc), name); + g_free(name); s->timer_tag = 0; return chr; @@ -2596,7 +2618,7 @@ static void udp_chr_update_read_handler(CharDriverState *chr, remove_fd_in_watch(chr); if (s->ioc) { - chr->fd_in_tag = io_add_watch_poll(s->ioc, + chr->fd_in_tag = io_add_watch_poll(chr, s->ioc, udp_chr_read_poll, udp_chr_read, chr, context); @@ -2673,9 +2695,13 @@ static gboolean socket_reconnect_timeout(gpointer opaque); static void qemu_chr_socket_restart_timer(CharDriverState *chr) { TCPCharDriver *s = chr->opaque; + char *name; assert(s->connected == 0); s->reconnect_timer = g_timeout_add_seconds(s->reconnect_time, socket_reconnect_timeout, chr); + name = g_strdup_printf("chardev-socket-reconnect-%s", chr->label); + g_source_set_name_by_id(s->reconnect_timer, name); + g_free(name); } static void check_report_connect_error(CharDriverState *chr, @@ -3000,7 +3026,7 @@ static void tcp_chr_connect(void *opaque) s->connected = 1; if (s->ioc) { - chr->fd_in_tag = io_add_watch_poll(s->ioc, + chr->fd_in_tag = io_add_watch_poll(chr, s->ioc, tcp_chr_read_poll, tcp_chr_read, chr, NULL); @@ -3019,7 +3045,7 @@ static void tcp_chr_update_read_handler(CharDriverState *chr, remove_fd_in_watch(chr); if (s->ioc) { - chr->fd_in_tag = io_add_watch_poll(s->ioc, + chr->fd_in_tag = io_add_watch_poll(chr, s->ioc, tcp_chr_read_poll, tcp_chr_read, chr, context); @@ -3117,6 +3143,7 @@ static void tcp_chr_tls_init(CharDriverState *chr) TCPCharDriver *s = chr->opaque; QIOChannelTLS *tioc; Error *err = NULL; + gchar *name; if (s->is_listen) { tioc = qio_channel_tls_new_server( @@ -3134,6 +3161,11 @@ static void tcp_chr_tls_init(CharDriverState *chr) tcp_chr_disconnect(chr); return; } + name = g_strdup_printf("chardev-tls-%s-%s", + s->is_listen ? "server" : "client", + chr->label); + qio_channel_set_name(QIO_CHANNEL(tioc), name); + g_free(name); object_unref(OBJECT(s->ioc)); s->ioc = QIO_CHANNEL(tioc); @@ -3144,6 +3176,19 @@ static void tcp_chr_tls_init(CharDriverState *chr) } +static void tcp_chr_set_client_ioc_name(CharDriverState *chr, + QIOChannelSocket *sioc) +{ + TCPCharDriver *s = chr->opaque; + char *name; + name = g_strdup_printf("chardev-tcp-%s-%s", + s->is_listen ? "server" : "client", + chr->label); + qio_channel_set_name(QIO_CHANNEL(sioc), name); + g_free(name); + +} + static int tcp_chr_new_client(CharDriverState *chr, QIOChannelSocket *sioc) { TCPCharDriver *s = chr->opaque; @@ -3189,6 +3234,7 @@ static int tcp_chr_add_client(CharDriverState *chr, int fd) if (!sioc) { return -1; } + tcp_chr_set_client_ioc_name(chr, sioc); ret = tcp_chr_new_client(chr, sioc); object_unref(OBJECT(sioc)); return ret; @@ -3230,6 +3276,7 @@ static int tcp_chr_wait_connected(CharDriverState *chr, Error **errp) qio_channel_set_blocking(QIO_CHANNEL(s->listen_ioc), false, NULL); } else { sioc = qio_channel_socket_new(); + tcp_chr_set_client_ioc_name(chr, sioc); if (qio_channel_socket_connect_sync(sioc, s->addr, errp) < 0) { object_unref(OBJECT(sioc)); return -1; @@ -4443,6 +4490,7 @@ static gboolean socket_reconnect_timeout(gpointer opaque) } sioc = qio_channel_socket_new(); + tcp_chr_set_client_ioc_name(chr, sioc); qio_channel_socket_connect_async(sioc, s->addr, qemu_chr_socket_connected, chr, NULL); @@ -4544,12 +4592,19 @@ static CharDriverState *qmp_chardev_open_socket(const char *id, if (s->reconnect_time) { sioc = qio_channel_socket_new(); + tcp_chr_set_client_ioc_name(chr, sioc); qio_channel_socket_connect_async(sioc, s->addr, qemu_chr_socket_connected, chr, NULL); } else { if (s->is_listen) { + char *name; sioc = qio_channel_socket_new(); + + name = g_strdup_printf("chardev-tcp-listener-%s", chr->label); + qio_channel_set_name(QIO_CHANNEL(sioc), name); + g_free(name); + if (qio_channel_socket_listen_sync(sioc, s->addr, errp) < 0) { goto error; } @@ -4590,6 +4645,8 @@ static CharDriverState *qmp_chardev_open_udp(const char *id, ChardevUdp *udp = backend->u.udp.data; ChardevCommon *common = qapi_ChardevUdp_base(udp); QIOChannelSocket *sioc = qio_channel_socket_new(); + char *name; + CharDriverState *chr; if (qio_channel_socket_dgram_sync(sioc, udp->local, udp->remote, @@ -4597,7 +4654,13 @@ static CharDriverState *qmp_chardev_open_udp(const char *id, object_unref(OBJECT(sioc)); return NULL; } - return qemu_chr_open_udp(sioc, common, errp); + chr = qemu_chr_open_udp(sioc, common, errp); + + name = g_strdup_printf("chardev-udp-%s", chr->label); + qio_channel_set_name(QIO_CHANNEL(sioc), name); + g_free(name); + + return chr; } From 6f01f136af7516b180bc14408c56f96826a316b3 Mon Sep 17 00:00:00 2001 From: "Daniel P. Berrange" <berrange@redhat.com> Date: Fri, 30 Sep 2016 11:57:14 +0100 Subject: [PATCH 09/11] migration: set name for all I/O channels created Ensure that all I/O channels created for migration are given names to distinguish their respective roles. Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- migration/exec.c | 2 ++ migration/fd.c | 2 ++ migration/migration.c | 1 + migration/savevm.c | 3 +++ migration/socket.c | 5 +++++ migration/tls.c | 2 ++ 6 files changed, 15 insertions(+) diff --git a/migration/exec.c b/migration/exec.c index 2af63cced6..9157721dfe 100644 --- a/migration/exec.c +++ b/migration/exec.c @@ -38,6 +38,7 @@ void exec_start_outgoing_migration(MigrationState *s, const char *command, Error return; } + qio_channel_set_name(ioc, "migration-exec-outgoing"); migration_channel_connect(s, ioc, NULL); object_unref(OBJECT(ioc)); } @@ -64,6 +65,7 @@ void exec_start_incoming_migration(const char *command, Error **errp) return; } + qio_channel_set_name(ioc, "migration-exec-incoming"); qio_channel_add_watch(ioc, G_IO_IN, exec_accept_incoming_migration, diff --git a/migration/fd.c b/migration/fd.c index 84a10fd68f..58cb51a9e6 100644 --- a/migration/fd.c +++ b/migration/fd.c @@ -38,6 +38,7 @@ void fd_start_outgoing_migration(MigrationState *s, const char *fdname, Error ** return; } + qio_channel_set_name(QIO_CHANNEL(ioc), "migration-fd-outgoing"); migration_channel_connect(s, ioc, NULL); object_unref(OBJECT(ioc)); } @@ -65,6 +66,7 @@ void fd_start_incoming_migration(const char *infd, Error **errp) return; } + qio_channel_set_name(QIO_CHANNEL(ioc), "migration-fd-incoming"); qio_channel_add_watch(ioc, G_IO_IN, fd_accept_incoming_migration, diff --git a/migration/migration.c b/migration/migration.c index 4d417b76cf..156e70791a 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -1567,6 +1567,7 @@ static int postcopy_start(MigrationState *ms, bool *old_vm_running) * to do this we use a qemu_buf to hold the whole of the device state. */ bioc = qio_channel_buffer_new(4096); + qio_channel_set_name(QIO_CHANNEL(bioc), "migration-postcopy-buffer"); fb = qemu_fopen_channel_output(QIO_CHANNEL(bioc)); object_unref(OBJECT(bioc)); diff --git a/migration/savevm.c b/migration/savevm.c index a831ec2d67..0dede9dd61 100644 --- a/migration/savevm.c +++ b/migration/savevm.c @@ -1582,6 +1582,7 @@ static int loadvm_handle_cmd_packaged(MigrationIncomingState *mis) } bioc = qio_channel_buffer_new(length); + qio_channel_set_name(QIO_CHANNEL(bioc), "migration-loadvm-buffer"); ret = qemu_get_buffer(mis->from_src_file, bioc->data, length); @@ -2073,6 +2074,7 @@ void qmp_xen_save_devices_state(const char *filename, Error **errp) if (!ioc) { goto the_end; } + qio_channel_set_name(QIO_CHANNEL(ioc), "migration-xen-save-state"); f = qemu_fopen_channel_output(QIO_CHANNEL(ioc)); ret = qemu_save_device_state(f); qemu_fclose(f); @@ -2105,6 +2107,7 @@ void qmp_xen_load_devices_state(const char *filename, Error **errp) if (!ioc) { return; } + qio_channel_set_name(QIO_CHANNEL(ioc), "migration-xen-load-state"); f = qemu_fopen_channel_input(QIO_CHANNEL(ioc)); migration_incoming_state_new(f); diff --git a/migration/socket.c b/migration/socket.c index a21c0c5c35..11f80b119b 100644 --- a/migration/socket.c +++ b/migration/socket.c @@ -100,6 +100,7 @@ static void socket_start_outgoing_migration(MigrationState *s, data->hostname = g_strdup(saddr->u.inet.data->host); } + qio_channel_set_name(QIO_CHANNEL(sioc), "migration-socket-outgoing"); qio_channel_socket_connect_async(sioc, saddr, socket_outgoing_migration, @@ -146,6 +147,7 @@ static gboolean socket_accept_incoming_migration(QIOChannel *ioc, trace_migration_socket_incoming_accepted(); + qio_channel_set_name(QIO_CHANNEL(sioc), "migration-socket-incoming"); migration_channel_process_incoming(migrate_get_current(), QIO_CHANNEL(sioc)); object_unref(OBJECT(sioc)); @@ -162,6 +164,9 @@ static void socket_start_incoming_migration(SocketAddress *saddr, { QIOChannelSocket *listen_ioc = qio_channel_socket_new(); + qio_channel_set_name(QIO_CHANNEL(listen_ioc), + "migration-socket-listener"); + if (qio_channel_socket_listen_sync(listen_ioc, saddr, errp) < 0) { object_unref(OBJECT(listen_ioc)); qapi_free_SocketAddress(saddr); diff --git a/migration/tls.c b/migration/tls.c index 12c053d15f..49ca9a8930 100644 --- a/migration/tls.c +++ b/migration/tls.c @@ -99,6 +99,7 @@ void migration_tls_channel_process_incoming(MigrationState *s, } trace_migration_tls_incoming_handshake_start(); + qio_channel_set_name(QIO_CHANNEL(tioc), "migration-tls-incoming"); qio_channel_tls_handshake(tioc, migration_tls_incoming_handshake, NULL, @@ -154,6 +155,7 @@ void migration_tls_channel_connect(MigrationState *s, } trace_migration_tls_outgoing_handshake_start(hostname); + qio_channel_set_name(QIO_CHANNEL(tioc), "migration-tls-outgoing"); qio_channel_tls_handshake(tioc, migration_tls_outgoing_handshake, s, From 10bcfe5897b5101b2831b4e2cdbb439ba459d599 Mon Sep 17 00:00:00 2001 From: "Daniel P. Berrange" <berrange@redhat.com> Date: Fri, 30 Sep 2016 11:57:14 +0100 Subject: [PATCH 10/11] vnc: set name for all I/O channels created Ensure that all I/O channels created for VNC are given names to distinguish their respective roles. Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- ui/vnc-auth-vencrypt.c | 1 + ui/vnc-ws.c | 3 +++ ui/vnc.c | 7 +++++++ 3 files changed, 11 insertions(+) diff --git a/ui/vnc-auth-vencrypt.c b/ui/vnc-auth-vencrypt.c index 11c8c9a819..c0c29a5119 100644 --- a/ui/vnc-auth-vencrypt.c +++ b/ui/vnc-auth-vencrypt.c @@ -116,6 +116,7 @@ static int protocol_client_vencrypt_auth(VncState *vs, uint8_t *data, size_t len return 0; } + qio_channel_set_name(QIO_CHANNEL(tls), "vnc-server-tls"); VNC_DEBUG("Start TLS VeNCrypt handshake process\n"); object_unref(OBJECT(vs->ioc)); vs->ioc = QIO_CHANNEL(tls); diff --git a/ui/vnc-ws.c b/ui/vnc-ws.c index 42a8e7be5c..bffb484a8d 100644 --- a/ui/vnc-ws.c +++ b/ui/vnc-ws.c @@ -67,6 +67,8 @@ gboolean vncws_tls_handshake_io(QIOChannel *ioc G_GNUC_UNUSED, return TRUE; } + qio_channel_set_name(QIO_CHANNEL(tls), "vnc-ws-server-tls"); + VNC_DEBUG("Start TLS WS handshake process\n"); object_unref(OBJECT(vs->ioc)); vs->ioc = QIO_CHANNEL(tls); @@ -113,6 +115,7 @@ gboolean vncws_handshake_io(QIOChannel *ioc G_GNUC_UNUSED, } wioc = qio_channel_websock_new_server(vs->ioc); + qio_channel_set_name(QIO_CHANNEL(wioc), "vnc-ws-server-websock"); object_unref(OBJECT(vs->ioc)); vs->ioc = QIO_CHANNEL(wioc); diff --git a/ui/vnc.c b/ui/vnc.c index 1bedc95b57..2c28a59ff7 100644 --- a/ui/vnc.c +++ b/ui/vnc.c @@ -3100,6 +3100,9 @@ static gboolean vnc_listen_io(QIOChannel *ioc, sioc = qio_channel_socket_accept(QIO_CHANNEL_SOCKET(ioc), &err); if (sioc != NULL) { + qio_channel_set_name(QIO_CHANNEL(sioc), + ioc != QIO_CHANNEL(vd->lsock) ? + "vnc-ws-server" : "vnc-server"); qio_channel_set_delay(QIO_CHANNEL(sioc), false); vnc_connect(vd, sioc, false, ioc != QIO_CHANNEL(vd->lsock)); @@ -3788,6 +3791,7 @@ void vnc_display_open(const char *id, Error **errp) } vd->is_unix = saddr->type == SOCKET_ADDRESS_KIND_UNIX; sioc = qio_channel_socket_new(); + qio_channel_set_name(QIO_CHANNEL(sioc), "vnc-reverse"); if (qio_channel_socket_connect_sync(sioc, saddr, errp) < 0) { goto fail; } @@ -3795,6 +3799,7 @@ void vnc_display_open(const char *id, Error **errp) object_unref(OBJECT(sioc)); } else { vd->lsock = qio_channel_socket_new(); + qio_channel_set_name(QIO_CHANNEL(vd->lsock), "vnc-listen"); if (qio_channel_socket_listen_sync(vd->lsock, saddr, errp) < 0) { goto fail; } @@ -3802,6 +3807,7 @@ void vnc_display_open(const char *id, Error **errp) if (ws_enabled) { vd->lwebsock = qio_channel_socket_new(); + qio_channel_set_name(QIO_CHANNEL(vd->lwebsock), "vnc-ws-listen"); if (qio_channel_socket_listen_sync(vd->lwebsock, wsaddr, errp) < 0) { object_unref(OBJECT(vd->lsock)); @@ -3845,6 +3851,7 @@ void vnc_display_add_client(const char *id, int csock, bool skipauth) sioc = qio_channel_socket_new_fd(csock, NULL); if (sioc) { + qio_channel_set_name(QIO_CHANNEL(sioc), "vnc-server"); vnc_connect(vd, sioc, skipauth, false); object_unref(OBJECT(sioc)); } From c3ff757d25115d6a530e8859d7287a862b1dc02d Mon Sep 17 00:00:00 2001 From: "Daniel P. Berrange" <berrange@redhat.com> Date: Fri, 30 Sep 2016 15:34:24 +0100 Subject: [PATCH 11/11] main: set names for main loop sources created The main loop creates two generic sources for the AIO and IO handler systems. Acked-by: Paolo Bonzini <pbonzini@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- main-loop.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/main-loop.c b/main-loop.c index 6a7f8d30bd..66c4eb69a3 100644 --- a/main-loop.c +++ b/main-loop.c @@ -161,9 +161,11 @@ int qemu_init_main_loop(Error **errp) qemu_notify_bh = qemu_bh_new(notify_event_cb, NULL); gpollfds = g_array_new(FALSE, FALSE, sizeof(GPollFD)); src = aio_get_g_source(qemu_aio_context); + g_source_set_name(src, "aio-context"); g_source_attach(src, NULL); g_source_unref(src); src = iohandler_get_g_source(); + g_source_set_name(src, "io-handler"); g_source_attach(src, NULL); g_source_unref(src); return 0;