Hi, This series improves handling of dynamic consoles in ui/ (commit 9588d67e72 "console: minimal hotplug suport" added basic unplug & replug). It allows for better hotplug of vfio-pci display devices or bochs-display (the other device supporting hotplug today), with GTK and D-Bus backends. As usual, the first patches are various preliminary cleanups and fixes I hit while developping the rest, they could be merged earlier. I improved the display cleanup to replace the scattered atexit() handler, making leaks more visible to sanitizers. Then console ADDED/REMOVED notifications are added, so display backends can react to hotplug. The GTK code is rework a bit to allow easier hotplug events handling. Finally, D-Bus supports is implemented with a qtest to exercise it. thanks Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- Marc-André Lureau (32): ui/gtk: fix bad widget realize on non-GFX VC build-sys: build with -fno-omit-frame-pointer with ASAN irq: add per-IRQ observer to fix qemu_irq_intercept_in leak scripts/lsan_suppressions: suppress fontconfig leaks vfio/pci: close display console during unrealize, not finalize glib-compat: add fallback for g_clear_fd/g_autofd ui/dbus: remove mouse handler on dispose ui/qmp: keep a reference of console across yield ui: stop ui timer when closing ui/console: init gl_unblock_timer in qemu_console_init ui/spice: remove dead spice_displays ui/spice: add cleanup on shutdown ui: add display cleanup infrastructure ui/curses: implement display cleanup ui/sdl2: implement display cleanup ui/spice-app: implement display cleanup ui/egl: implement display and EGL cleanup ui/cocoa: implement display cleanup ui/dbus: implement display cleanup ui/gtk: implement display cleanup ui/console: add console event notifier infrastructure ui/console: fire console ADDED/REMOVED notifications ui/console-vc: fire ADDED/REMOVED notifications ui/gtk: convert VirtualConsole storage from fixed array to GPtrArray ui/gtk: move global display settings out of per-console init ui/gtk: fix tab re-insertion order on window close ui/gtk: centralize console menu and shortcut management ui/gtk: handle console hotplug/unplug events ui/console: register console in QOM tree dynamically ui/console: unregister console from QOM tree on close ui/dbus: handle console hotplug/unplug events tests/qtest: add D-Bus display hotplug test meson.build | 2 + hw/vfio/pci.h | 1 + include/glib-compat.h | 29 ++++ include/hw/core/irq.h | 1 + include/ui/console.h | 20 +++ include/ui/egl-helpers.h | 1 + include/ui/gtk.h | 6 +- include/ui/qemu-spice-module.h | 1 + include/ui/qemu-spice.h | 9 +- ui/dbus.h | 3 + hw/core/irq.c | 10 +- hw/vfio/display.c | 30 ++-- hw/vfio/pci.c | 1 + system/qtest.c | 3 - system/runstate.c | 4 +- tests/qtest/dbus-display-test.c | 101 ++++++++++- ui/console-vc.c | 13 ++ ui/console.c | 71 +++++++- ui/curses.c | 17 +- ui/dbus-console.c | 9 + ui/dbus.c | 106 ++++++++++-- ui/egl-headless.c | 31 ++++ ui/egl-helpers.c | 19 +++ ui/gtk-clipboard.c | 15 ++ ui/gtk.c | 369 +++++++++++++++++++++++++++++++--------- ui/sdl2.c | 23 ++- ui/spice-app.c | 10 +- ui/spice-core.c | 25 ++- ui/spice-display.c | 52 ++++++ ui/spice-input.c | 52 ++++-- ui/spice-module.c | 5 + ui/ui-qmp-cmds.c | 3 + scripts/lsan_suppressions.txt | 15 +- ui/cocoa.m | 19 ++- ui/meson.build | 4 +- 35 files changed, 901 insertions(+), 179 deletions(-) --- base-commit: 2db91528542672cf0db78b3f2cc0e22b36302b38 change-id: 20260529-b4-ui-909bfa695735 Best regards, -- Marc-André Lureau <marcandre.lureau@redhat.com> The GTK VirtualConsole is a union, it may be .gfx or .vte depending on the type. Fixes: 565f85a9c2 ("ui/gtk: force realization of drawing area") Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- ui/gtk.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ui/gtk.c b/ui/gtk.c index XXXXXXX..XXXXXXX 100644 --- a/ui/gtk.c +++ b/ui/gtk.c @@ -XXX,XX +XXX,XX @@ static void gtk_display_init(DisplayState *ds, DisplayOptions *opts) if (!con) { break; } - gtk_widget_realize(s->vc[idx].gfx.drawing_area); + if (s->vc[idx].type == GD_VC_GFX) { + gtk_widget_realize(s->vc[idx].gfx.drawing_area); + } } if (opts->u.gtk.has_show_menubar && -- 2.54.0 On fc44, LSan fails to suppress leak:qemu_irq_intercept_in, because the backtrace isn't deep enough. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- meson.build | 2 ++ 1 file changed, 2 insertions(+) diff --git a/meson.build b/meson.build index XXXXXXX..XXXXXXX 100644 --- a/meson.build +++ b/meson.build @@ -XXX,XX +XXX,XX @@ if get_option('asan') if cc.has_argument('-fsanitize=address') qemu_cflags = ['-fsanitize=address'] + qemu_cflags qemu_ldflags = ['-fsanitize=address'] + qemu_ldflags + # Ensure complete stack traces for LSan suppressions to match correctly. + qemu_cflags += ['-fno-omit-frame-pointer'] else error('Your compiler does not support -fsanitize=address') endif -- 2.54.0 qemu_irq_intercept_in() saves original IRQ handlers by allocating new QOM objects, which are never freed. On a PC machine, this leaks IRQ objects (one per IOAPIC pin) on every qtest run. Rather than tracking allocations to free later, avoid them: add an "observer" field to IRQState, called by qemu_set_irq() after the real handler. Interception sets the observer instead of rewriting handlers, so there's nothing to save and nothing to leak. Fix qemu_notirq() to route through qemu_set_irq() so inverted IRQs trigger observers too. Drop the LSan suppression. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- include/hw/core/irq.h | 1 + hw/core/irq.c | 10 +++++----- system/qtest.c | 3 --- scripts/lsan_suppressions.txt | 8 -------- 4 files changed, 6 insertions(+), 16 deletions(-) diff --git a/include/hw/core/irq.h b/include/hw/core/irq.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/core/irq.h +++ b/include/hw/core/irq.h @@ -XXX,XX +XXX,XX @@ struct IRQState { qemu_irq_handler handler; void *opaque; int n; + qemu_irq_handler observer; }; void qemu_set_irq(qemu_irq irq, int level); diff --git a/hw/core/irq.c b/hw/core/irq.c index XXXXXXX..XXXXXXX 100644 --- a/hw/core/irq.c +++ b/hw/core/irq.c @@ -XXX,XX +XXX,XX @@ void qemu_set_irq(qemu_irq irq, int level) return; irq->handler(irq->opaque, irq->n, level); + if (unlikely(irq->observer)) { + irq->observer(irq->opaque, irq->n, level); + } } static void init_irq_fields(IRQState *irq, qemu_irq_handler handler, @@ -XXX,XX +XXX,XX @@ static void qemu_notirq(void *opaque, int line, int level) { IRQState *irq = opaque; - irq->handler(irq->opaque, irq->n, !level); + qemu_set_irq(irq, !level); } qemu_irq qemu_irq_invert(qemu_irq irq) @@ -XXX,XX +XXX,XX @@ qemu_irq qemu_irq_invert(qemu_irq irq) void qemu_irq_intercept_in(qemu_irq *gpio_in, qemu_irq_handler handler, int n) { int i; - qemu_irq *old_irqs = qemu_allocate_irqs(NULL, NULL, n); for (i = 0; i < n; i++) { - *old_irqs[i] = *gpio_in[i]; - gpio_in[i]->handler = handler; - gpio_in[i]->opaque = &old_irqs[i]; + gpio_in[i]->observer = handler; } } diff --git a/system/qtest.c b/system/qtest.c index XXXXXXX..XXXXXXX 100644 --- a/system/qtest.c +++ b/system/qtest.c @@ -XXX,XX +XXX,XX @@ void qtest_sendf(CharFrontend *chr, const char *fmt, ...) static void qtest_irq_handler(void *opaque, int n, int level) { - qemu_irq old_irq = *(qemu_irq *)opaque; - qemu_set_irq(old_irq, level); - if (irq_levels[n] != level) { CharFrontend *chr = &qtest->qtest_chr; irq_levels[n] = level; diff --git a/scripts/lsan_suppressions.txt b/scripts/lsan_suppressions.txt index XXXXXXX..XXXXXXX 100644 --- a/scripts/lsan_suppressions.txt +++ b/scripts/lsan_suppressions.txt @@ -XXX,XX +XXX,XX @@ leak:/lib64/libxkbcommon.so.0 # https://github.com/GNOME/glib/blob/main/tools/glib.supp # This avoids false positive leak reports for the qga-ssh-test. leak:g_set_user_dirs - -# qemu_irq_intercept_in is only used by the qtest harness, and -# its API inherently involves a leak. -# While we could keep track of the old IRQ data structure -# in order to free it, it doesn't seem very important to fix -# since it is only used by the qtest test harness. -# Just ignore the leak, at least for the moment. -leak:qemu_irq_intercept_in -- 2.54.0 Those are annoying reports for gtk/sdl etc. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- scripts/lsan_suppressions.txt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/scripts/lsan_suppressions.txt b/scripts/lsan_suppressions.txt index XXXXXXX..XXXXXXX 100644 --- a/scripts/lsan_suppressions.txt +++ b/scripts/lsan_suppressions.txt @@ -XXX,XX +XXX,XX @@ leak:/lib64/libtcmalloc_minimal.so.4 # libxkbcommon also leaks in qemu-keymap leak:/lib64/libxkbcommon.so.0 +# libfontconfig leaks are notorious, for ex +# https://gitlab.freedesktop.org/fontconfig/fontconfig/-/work_items/519 +leak:libfontconfig.so + # g_set_user_dirs() deliberately leaks the previous cached g_get_user_* # values. This is documented in upstream glib's valgrind-format # suppression file: -- 2.54.0 The QemuGraphicConsole holds a strong QOM link back to the device via its "device" property (OBJ_PROP_LINK_STRONG). When graphic_console_close() is only called from vfio_display_finalize() during object finalize, this creates a ref-cycle deadlock: the device can't reach refcount 0 because the console holds a strong ref, but the console's ref is only dropped by graphic_console_close() which runs inside finalize. Split the display teardown into two phases: - vfio_display_exit(): called during unrealize (vfio_exitfn), closes the graphic console to break the ref cycle, and removes display region subregions while the parent memory regions are still alive. - vfio_display_finalize(): remains in finalize (vfio_pci_put_device), frees display region memory, dmabuf, and edid resources. The region memory contains QOM child objects (MemoryRegions) that must stay alive until QOM finalization has processed them. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- hw/vfio/pci.h | 1 + hw/vfio/display.c | 30 +++++++++++++++++++----------- hw/vfio/pci.c | 1 + 3 files changed, 21 insertions(+), 11 deletions(-) diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h index XXXXXXX..XXXXXXX 100644 --- a/hw/vfio/pci.h +++ b/hw/vfio/pci.h @@ -XXX,XX +XXX,XX @@ bool vfio_populate_vga(VFIOPCIDevice *vdev, Error **errp); void vfio_display_reset(VFIOPCIDevice *vdev); bool vfio_display_probe(VFIOPCIDevice *vdev, Error **errp); +void vfio_display_exit(VFIOPCIDevice *vdev); void vfio_display_finalize(VFIOPCIDevice *vdev); extern const VMStateDescription vfio_display_vmstate; diff --git a/hw/vfio/display.c b/hw/vfio/display.c index XXXXXXX..XXXXXXX 100644 --- a/hw/vfio/display.c +++ b/hw/vfio/display.c @@ -XXX,XX +XXX,XX @@ static bool vfio_display_region_init(VFIOPCIDevice *vdev, Error **errp) return true; } -static void vfio_display_region_exit(VFIODisplay *dpy) -{ - if (!dpy->region.buffer.size) { - return; - } - - vfio_region_exit(&dpy->region.buffer); - vfio_region_finalize(&dpy->region.buffer); -} /* ---------------------------------------------------------------------- */ @@ -XXX,XX +XXX,XX @@ bool vfio_display_probe(VFIOPCIDevice *vdev, Error **errp) return false; } -void vfio_display_finalize(VFIOPCIDevice *vdev) +void vfio_display_exit(VFIOPCIDevice *vdev) { if (!vdev->dpy) { return; } + if (display_opengl) { + qemu_console_set_display_gl_ctx(vdev->dpy->con, NULL); + } qemu_graphic_console_close(vdev->dpy->con); + if (vdev->dpy->region.buffer.size) { + vfio_region_exit(&vdev->dpy->region.buffer); + } +} + +void vfio_display_finalize(VFIOPCIDevice *vdev) +{ + if (!vdev->dpy) { + return; + } + vfio_display_dmabuf_exit(vdev->dpy); - vfio_display_region_exit(vdev->dpy); + if (vdev->dpy->region.buffer.size) { + vfio_region_finalize(&vdev->dpy->region.buffer); + } vfio_display_edid_exit(vdev->dpy); g_free(vdev->dpy); + vdev->dpy = NULL; } static bool migrate_needed(void *opaque) diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c index XXXXXXX..XXXXXXX 100644 --- a/hw/vfio/pci.c +++ b/hw/vfio/pci.c @@ -XXX,XX +XXX,XX @@ static void vfio_exitfn(PCIDevice *pdev) VFIOPCIDevice *vdev = VFIO_PCI_DEVICE(pdev); VFIODevice *vbasedev = &vdev->vbasedev; + vfio_display_exit(vdev); vfio_unregister_req_notifier(vdev); vfio_unregister_err_notifier(vdev); pci_device_set_intx_routing_notifier(pdev, NULL); -- 2.54.0 Those helpers were added in glib 2.76. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- include/glib-compat.h | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/include/glib-compat.h b/include/glib-compat.h index XXXXXXX..XXXXXXX 100644 --- a/include/glib-compat.h +++ b/include/glib-compat.h @@ -XXX,XX +XXX,XX @@ #pragma GCC diagnostic ignored "-Wdeprecated-declarations" #include <glib.h> +#include <glib/gstdio.h> #if defined(G_OS_UNIX) #include <glib-unix.h> #include <sys/types.h> @@ -XXX,XX +XXX,XX @@ qemu_g_test_slow(void) #define g_test_thorough() qemu_g_test_slow() #define g_test_quick() (!qemu_g_test_slow()) +static inline gboolean g_clear_fd_qemu(int *fd_ptr, GError **error) +{ +#if GLIB_CHECK_VERSION(2, 76, 0) + return g_clear_fd(fd_ptr, error); +#else + int fd = *fd_ptr; + + *fd_ptr = -1; + + if (fd < 0) { + return TRUE; + } + + return g_close(fd, error); +#endif +} +#define g_clear_fd(fd, err) g_clear_fd_qemu(fd, err) + +#if !GLIB_CHECK_VERSION(2, 76, 0) +static inline void _g_clear_fd_ignore_error(int *fd_ptr) +{ + int errsv = errno; + g_clear_fd(fd_ptr, NULL); + errno = errsv; +} +#define g_autofd __attribute__((cleanup(_g_clear_fd_ignore_error))) +#endif + #pragma GCC diagnostic pop #ifndef G_NORETURN -- 2.54.0 Fixes: 142ca628a7 ("ui: add a D-Bus display backend") Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- ui/dbus-console.c | 1 + 1 file changed, 1 insertion(+) diff --git a/ui/dbus-console.c b/ui/dbus-console.c index XXXXXXX..XXXXXXX 100644 --- a/ui/dbus-console.c +++ b/ui/dbus-console.c @@ -XXX,XX +XXX,XX @@ dbus_display_console_dispose(GObject *object) DBusDisplayConsole *ddc = DBUS_DISPLAY_CONSOLE(object); qemu_console_unregister_listener(&ddc->dcl); + qemu_remove_mouse_mode_change_notifier(&ddc->mouse_mode_notifier); g_clear_object(&ddc->iface_touch); g_clear_object(&ddc->iface_mouse); g_clear_object(&ddc->iface_kbd); -- 2.54.0 While the coroutine is waiting, the console could be finalized. Keep a reference to prevent this. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- ui/ui-qmp-cmds.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ui/ui-qmp-cmds.c b/ui/ui-qmp-cmds.c index XXXXXXX..XXXXXXX 100644 --- a/ui/ui-qmp-cmds.c +++ b/ui/ui-qmp-cmds.c @@ -XXX,XX +XXX,XX @@ qmp_screendump(const char *filename, const char *device, } } + object_ref(con); qemu_console_co_wait_update(con); /* @@ -XXX,XX +XXX,XX @@ qmp_screendump(const char *filename, const char *device, surface = qemu_console_surface(con); if (!surface) { error_setg(errp, "no surface"); + object_unref(con); return; } image = pixman_image_ref(surface->image); + object_unref(con); fd = qemu_create(filename, O_WRONLY | O_TRUNC | O_BINARY, 0666, errp); if (fd == -1) { -- 2.54.0 hwops is reset, so if the UI timer is pending it will crash. Fixes: 9588d67e72 ("console: minimal hotplug suport") Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- ui/console.c | 1 + 1 file changed, 1 insertion(+) diff --git a/ui/console.c b/ui/console.c index XXXXXXX..XXXXXXX 100644 --- a/ui/console.c +++ b/ui/console.c @@ -XXX,XX +XXX,XX @@ void qemu_graphic_console_close(QemuConsole *con) trace_console_gfx_close(con->index); object_property_set_link(OBJECT(con), "device", NULL, &error_abort); qemu_graphic_console_set_hwops(con, &unused_ops, NULL); + timer_del(con->ui_timer); if (con->gl) { qemu_console_gl_scanout_disable(con); -- 2.54.0 Move gl_unblock_timer allocation from graphic_console_init() to qemu_console_init(), similar to what was done in commit cfde05d15b ("ui/console: allocate ui_timer in QemuConsole"). This fixes leaking timers on console recycling. Fixes: a9b1e471e17 ("ui: add a gl-unblock warning timer") Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- ui/console.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ui/console.c b/ui/console.c index XXXXXXX..XXXXXXX 100644 --- a/ui/console.c +++ b/ui/console.c @@ -XXX,XX +XXX,XX @@ qemu_console_init(Object *obj) c->window_id = -1; c->ui_timer = timer_new_ms(QEMU_CLOCK_REALTIME, dpy_set_ui_info_timer, c); + c->gl_unblock_timer = timer_new_ms(QEMU_CLOCK_REALTIME, + console_hw_gl_unblock_timer, c); qemu_console_register(c); } @@ -XXX,XX +XXX,XX @@ QemuConsole *qemu_graphic_console_create(DeviceState *dev, uint32_t head, surface = qemu_create_placeholder_surface(width, height, noinit); qemu_console_set_surface(s, surface); - s->gl_unblock_timer = timer_new_ms(QEMU_CLOCK_REALTIME, - console_hw_gl_unblock_timer, s); return s; } -- 2.54.0 This is left-over from commit 9fa032866da ("spice: fix multihead support") Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- include/ui/qemu-spice.h | 7 ------- 1 file changed, 7 deletions(-) diff --git a/include/ui/qemu-spice.h b/include/ui/qemu-spice.h index XXXXXXX..XXXXXXX 100644 --- a/include/ui/qemu-spice.h +++ b/include/ui/qemu-spice.h @@ -XXX,XX +XXX,XX @@ bool qemu_spice_have_display_interface(QemuConsole *con); int qemu_spice_add_display_interface(QXLInstance *qxlin, QemuConsole *con); int qemu_spice_migrate_info(const char *hostname, int port, int tls_port, const char *subject); - -#else /* CONFIG_SPICE */ - -#include "qemu/error-report.h" - -#define spice_displays 0 - #endif /* CONFIG_SPICE */ static inline bool qemu_using_spice(Error **errp) -- 2.54.0 SPICE resources were never freed on shutdown. Add per-subsystem cleanup (display, input, core) and call it from qemu_cleanup(). Move spice-module.c into libui so the qemu_spice ops table links with the rest of the UI code. Add an LSan suppression for a known spice-server leak. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- include/ui/qemu-spice-module.h | 1 + include/ui/qemu-spice.h | 2 ++ system/runstate.c | 4 ++++ ui/spice-core.c | 25 ++++++++++++++++++-- ui/spice-display.c | 52 ++++++++++++++++++++++++++++++++++++++++++ ui/spice-input.c | 52 ++++++++++++++++++++++++++++-------------- ui/spice-module.c | 5 ++++ scripts/lsan_suppressions.txt | 5 ++++ ui/meson.build | 4 ++-- 9 files changed, 129 insertions(+), 21 deletions(-) diff --git a/include/ui/qemu-spice-module.h b/include/ui/qemu-spice-module.h index XXXXXXX..XXXXXXX 100644 --- a/include/ui/qemu-spice-module.h +++ b/include/ui/qemu-spice-module.h @@ -XXX,XX +XXX,XX @@ typedef struct SpiceInfo SpiceInfo; struct QemuSpiceOps { void (*init)(void); + void (*cleanup)(void); void (*display_init)(void); int (*migrate_info)(const char *h, int p, int t, const char *s); int (*set_passwd)(const char *passwd, diff --git a/include/ui/qemu-spice.h b/include/ui/qemu-spice.h index XXXXXXX..XXXXXXX 100644 --- a/include/ui/qemu-spice.h +++ b/include/ui/qemu-spice.h @@ -XXX,XX +XXX,XX @@ #include "qemu/config-file.h" void qemu_spice_input_init(void); +void qemu_spice_input_cleanup(void); void qemu_spice_display_init(void); +void qemu_spice_display_cleanup(void); void qemu_spice_display_init_done(void); bool qemu_spice_have_display_interface(QemuConsole *con); int qemu_spice_add_display_interface(QXLInstance *qxlin, QemuConsole *con); diff --git a/system/runstate.c b/system/runstate.c index XXXXXXX..XXXXXXX 100644 --- a/system/runstate.c +++ b/system/runstate.c @@ -XXX,XX +XXX,XX @@ #include "system/system.h" #include "system/tpm.h" #include "ui/console.h" +#include "ui/qemu-spice-module.h" #include "trace.h" @@ -XXX,XX +XXX,XX @@ void qemu_cleanup(int status) user_creatable_cleanup(); #ifdef CONFIG_VNC vnc_cleanup(); +#endif +#ifdef CONFIG_SPICE + qemu_spice.cleanup(); #endif /* TODO: unref root container, check all devices are ok */ } diff --git a/ui/spice-core.c b/ui/spice-core.c index XXXXXXX..XXXXXXX 100644 --- a/ui/spice-core.c +++ b/ui/spice-core.c @@ -XXX,XX +XXX,XX @@ static void vm_change_state_handler(void *opaque, bool running, } } +static VMChangeStateEntry *vm_change_entry; + void qemu_spice_display_init_done(void) { if (runstate_is_running()) { qemu_spice_display_start(); } - qemu_add_vm_change_state_handler(vm_change_state_handler, NULL); + vm_change_entry = + qemu_add_vm_change_state_handler(vm_change_state_handler, NULL); } static void qemu_spice_init(void) @@ -XXX,XX +XXX,XX @@ static int qemu_spice_add_interface(SpiceBaseInstance *sin) spice_server = spice_server_new(); spice_server_set_sasl_appname(spice_server, "qemu"); spice_server_init(spice_server, &core_interface); - qemu_add_vm_change_state_handler(vm_change_state_handler, NULL); + vm_change_entry = + qemu_add_vm_change_state_handler(vm_change_state_handler, NULL); } return spice_server_add_interface(spice_server, sin); @@ -XXX,XX +XXX,XX @@ int qemu_spice_display_is_running(SimpleSpiceDisplay *ssd) return spice_display_is_running; } +static void qemu_spice_cleanup(void) +{ + if (!spice_server) { + return; + } + + qemu_spice_display_cleanup(); + qemu_spice_input_cleanup(); + migration_remove_notifier(&migration_state); + g_clear_pointer(&spice_consoles, g_slist_free); + g_clear_pointer(&auth_passwd, g_free); + g_clear_pointer(&spice_server, spice_server_destroy); + g_clear_pointer(&vm_change_entry, qemu_del_vm_change_state_handler); + using_spice = 0; +} + static struct QemuSpiceOps real_spice_ops = { .init = qemu_spice_init, + .cleanup = qemu_spice_cleanup, .display_init = qemu_spice_display_init, .migrate_info = qemu_spice_migrate_info, .set_passwd = qemu_spice_set_passwd, diff --git a/ui/spice-display.c b/ui/spice-display.c index XXXXXXX..XXXXXXX 100644 --- a/ui/spice-display.c +++ b/ui/spice-display.c @@ -XXX,XX +XXX,XX @@ bool spice_opengl; bool spice_remote_client; int spice_max_refresh_rate; +static GPtrArray *spice_displays; + int qemu_spice_rect_is_empty(const QXLRect* r) { return r->top == r->bottom || r->left == r->right; @@ -XXX,XX +XXX,XX @@ static void qemu_spice_display_init_one(QemuConsole *con) qemu_console_set_display_gl_ctx(con, &ssd->dgc); } qemu_console_register_listener(con, &ssd->dcl, ops); + g_ptr_array_add(spice_displays, ssd); +} + +void qemu_spice_display_cleanup(void) +{ + if (!spice_displays) { + return; + } + + for (guint i = 0; i < spice_displays->len; i++) { + SimpleSpiceDisplay *ssd = g_ptr_array_index(spice_displays, i); + SimpleSpiceUpdate *update; + + qemu_console_unregister_listener(&ssd->dcl); +#ifdef HAVE_SPICE_GL + if (spice_opengl) { + qemu_console_set_display_gl_ctx(ssd->dcl.con, NULL); + } +#endif + + if (ssd->ds) { + qemu_spice_destroy_host_primary(ssd); + } + qemu_spice_del_memslot(ssd, MEMSLOT_GROUP_HOST, 0); + spice_server_remove_interface(&ssd->qxl.base); + + while ((update = QTAILQ_FIRST(&ssd->updates)) != NULL) { + QTAILQ_REMOVE(&ssd->updates, update, next); + qemu_spice_destroy_update(ssd, update); + } + g_clear_pointer(&ssd->ptr_define, g_free); + g_clear_pointer(&ssd->ptr_move, g_free); + g_clear_pointer(&ssd->cursor, cursor_unref); + g_clear_pointer(&ssd->surface, pixman_image_unref); + g_clear_pointer(&ssd->mirror, pixman_image_unref); + g_clear_pointer(&ssd->buf, g_free); +#ifdef HAVE_SPICE_GL + g_clear_pointer(&ssd->gl_unblock_bh, qemu_bh_delete); + g_clear_pointer(&ssd->gl_unblock_timer, timer_free); + g_clear_pointer(&ssd->gls, qemu_gl_fini_shader); + egl_fb_destroy(&ssd->guest_fb); + egl_fb_destroy(&ssd->blit_fb); + egl_fb_destroy(&ssd->cursor_fb); +#endif + qemu_mutex_destroy(&ssd->lock); + g_free(ssd); + } + g_clear_pointer(&spice_displays, g_ptr_array_unref); } void qemu_spice_display_init(void) @@ -XXX,XX +XXX,XX @@ void qemu_spice_display_init(void) const char *str; int i; + spice_displays = g_ptr_array_new(); + str = qemu_opt_get(opts, "display"); if (str) { int head = qemu_opt_get_number(opts, "head", 0); diff --git a/ui/spice-input.c b/ui/spice-input.c index XXXXXXX..XXXXXXX 100644 --- a/ui/spice-input.c +++ b/ui/spice-input.c @@ -XXX,XX +XXX,XX @@ static void mouse_mode_notifier(Notifier *notifier, void *data) pointer->absolute = is_absolute; } +static QemuSpiceKbd *spice_kbd; +static QemuSpicePointer *spice_pointer; +static QEMUPutLEDEntry *spice_led; + void qemu_spice_input_init(void) { - QemuSpiceKbd *kbd; - QemuSpicePointer *pointer; - - kbd = g_malloc0(sizeof(*kbd)); - kbd->sin.base.sif = &kbd_interface.base; - qemu_spice.add_interface(&kbd->sin.base); - qemu_add_led_event_handler(kbd_leds, kbd); - - pointer = g_malloc0(sizeof(*pointer)); - pointer->mouse.base.sif = &mouse_interface.base; - pointer->tablet.base.sif = &tablet_interface.base; - qemu_spice.add_interface(&pointer->mouse.base); - - pointer->absolute = false; - pointer->mouse_mode.notify = mouse_mode_notifier; - qemu_add_mouse_mode_change_notifier(&pointer->mouse_mode); - mouse_mode_notifier(&pointer->mouse_mode, NULL); + spice_kbd = g_new0(QemuSpiceKbd, 1); + spice_kbd->sin.base.sif = &kbd_interface.base; + qemu_spice.add_interface(&spice_kbd->sin.base); + spice_led = qemu_add_led_event_handler(kbd_leds, spice_kbd); + + spice_pointer = g_new0(QemuSpicePointer, 1); + spice_pointer->mouse.base.sif = &mouse_interface.base; + spice_pointer->tablet.base.sif = &tablet_interface.base; + qemu_spice.add_interface(&spice_pointer->mouse.base); + + spice_pointer->absolute = false; + spice_pointer->mouse_mode.notify = mouse_mode_notifier; + qemu_add_mouse_mode_change_notifier(&spice_pointer->mouse_mode); + mouse_mode_notifier(&spice_pointer->mouse_mode, NULL); +} + +void qemu_spice_input_cleanup(void) +{ + g_clear_pointer(&spice_led, qemu_remove_led_event_handler); + if (spice_pointer) { + qemu_remove_mouse_mode_change_notifier(&spice_pointer->mouse_mode); + if (spice_pointer->absolute) { + spice_server_remove_interface(&spice_pointer->tablet.base); + } + spice_server_remove_interface(&spice_pointer->mouse.base); + g_clear_pointer(&spice_pointer, g_free); + } + if (spice_kbd) { + spice_server_remove_interface(&spice_kbd->sin.base); + g_clear_pointer(&spice_kbd, g_free); + } } diff --git a/ui/spice-module.c b/ui/spice-module.c index XXXXXXX..XXXXXXX 100644 --- a/ui/spice-module.c +++ b/ui/spice-module.c @@ -XXX,XX +XXX,XX @@ static int qemu_spice_display_add_client_stub(int csock, int skipauth, return -1; } +static void qemu_spice_cleanup_stub(void) +{ +} + struct QemuSpiceOps qemu_spice = { .init = qemu_spice_init_stub, .display_init = qemu_spice_display_init_stub, @@ -XXX,XX +XXX,XX @@ struct QemuSpiceOps qemu_spice = { .set_passwd = qemu_spice_set_passwd_stub, .set_pw_expire = qemu_spice_set_pw_expire_stub, .display_add_client = qemu_spice_display_add_client_stub, + .cleanup = qemu_spice_cleanup_stub, }; #ifdef CONFIG_SPICE diff --git a/scripts/lsan_suppressions.txt b/scripts/lsan_suppressions.txt index XXXXXXX..XXXXXXX 100644 --- a/scripts/lsan_suppressions.txt +++ b/scripts/lsan_suppressions.txt @@ -XXX,XX +XXX,XX @@ leak:libfontconfig.so # https://github.com/GNOME/glib/blob/main/tools/glib.supp # This avoids false positive leak reports for the qga-ssh-test. leak:g_set_user_dirs + +# spice_server_add_interface allocates internal channel data that +# spice_server_destroy does not free +# https://gitlab.freedesktop.org/spice/spice/-/merge_requests/246 +leak:spice_server_add_interface diff --git a/ui/meson.build b/ui/meson.build index XXXXXXX..XXXXXXX 100644 --- a/ui/meson.build +++ b/ui/meson.build @@ -XXX,XX +XXX,XX @@ libui_sources = files( 'kbd-state.c', 'keymaps.c', 'qemu-pixman.c', + 'spice-module.c', 'vgafont.c', ) if pixman.found() libui_sources += files('cp437.c', 'vt100.c') endif libui = static_library('qemuui', libui_sources + genh, - dependencies: [pixman], + dependencies: [pixman, spice_headers], build_by_default: false) ui = declare_dependency(objects: libui.extract_all_objects(recursive: false), dependencies: [pixman]) system_ss.add(png) @@ -XXX,XX +XXX,XX @@ system_ss.add(when: pixman, if_true: files('console-vc.c'), if_false: files('con if dbus_display system_ss.add(files('dbus-module.c')) endif -system_ss.add([spice_headers, files('spice-module.c')]) system_ss.add(when: spice_protocol, if_true: files('vdagent.c')) if host_os == 'linux' -- 2.54.0 Add a cleanup callback to QemuDisplay and a qemu_display_cleanup() function that iterates all registered display types and calls their cleanup handler. Wire it into qemu_cleanup() in runstate.c, replacing the ad-hoc vnc_cleanup() call. This provides a structured alternative to atexit() handlers, giving deterministic teardown ordering and making resource leaks visible to sanitizers. The cleanup should happen before user_creatable_cleanup(), since some display have weak user-creatable references to cleanup before. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- include/ui/console.h | 2 ++ system/runstate.c | 8 +------- ui/console.c | 20 +++++++++++++++++++- 3 files changed, 22 insertions(+), 8 deletions(-) diff --git a/include/ui/console.h b/include/ui/console.h index XXXXXXX..XXXXXXX 100644 --- a/include/ui/console.h +++ b/include/ui/console.h @@ -XXX,XX +XXX,XX @@ struct QemuDisplay { DisplayType type; void (*early_init)(DisplayOptions *opts); void (*init)(DisplayState *ds, DisplayOptions *opts); + void (*cleanup)(void); const char *vc; }; @@ -XXX,XX +XXX,XX @@ bool qemu_display_find_default(DisplayOptions *opts); void qemu_display_early_init(DisplayOptions *opts); void qemu_display_init(DisplayState *ds, DisplayOptions *opts); const char *qemu_display_get_vc(DisplayOptions *opts); +void qemu_display_cleanup(void); void qemu_display_help(void); /* vnc.c */ diff --git a/system/runstate.c b/system/runstate.c index XXXXXXX..XXXXXXX 100644 --- a/system/runstate.c +++ b/system/runstate.c @@ -XXX,XX +XXX,XX @@ #include "system/system.h" #include "system/tpm.h" #include "ui/console.h" -#include "ui/qemu-spice-module.h" #include "trace.h" @@ -XXX,XX +XXX,XX @@ void qemu_cleanup(int status) audio_cleanup(); monitor_cleanup(); qemu_chr_cleanup(); + qemu_display_cleanup(); user_creatable_cleanup(); -#ifdef CONFIG_VNC - vnc_cleanup(); -#endif -#ifdef CONFIG_SPICE - qemu_spice.cleanup(); -#endif /* TODO: unref root container, check all devices are ok */ } diff --git a/ui/console.c b/ui/console.c index XXXXXXX..XXXXXXX 100644 --- a/ui/console.c +++ b/ui/console.c @@ -XXX,XX +XXX,XX @@ #include "qemu/memfd.h" #include "ui/vt100.h" #include "vgafont.h" +#include "ui/qemu-spice.h" #include "console-priv.h" @@ -XXX,XX +XXX,XX @@ void qemu_console_set_display_gl_ctx(QemuConsole *con, DisplayGLCtx *gl) { /* display has opengl support */ assert(con); - if (con->gl) { + if (gl && con->gl) { error_report("The console already has an OpenGL context."); exit(1); } @@ -XXX,XX +XXX,XX @@ void qemu_display_init(DisplayState *ds, DisplayOptions *opts) dpys[opts->type]->init(ds, opts); } +void qemu_display_cleanup(void) +{ + int i; + + for (i = 0; i < DISPLAY_TYPE__MAX; i++) { + if (dpys[i] && dpys[i]->cleanup) { + dpys[i]->cleanup(); + } + } +#ifdef CONFIG_VNC + vnc_cleanup(); +#endif +#ifdef CONFIG_SPICE + qemu_spice.cleanup(); +#endif +} + const char *qemu_display_get_vc(DisplayOptions *opts) { #ifdef CONFIG_PIXMAN -- 2.54.0 Replace the atexit() handler with a proper cleanup callback. The new curses_cleanup() unregisters the display listener, destroy & free the allocated resources. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- ui/curses.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/ui/curses.c b/ui/curses.c index XXXXXXX..XXXXXXX 100644 --- a/ui/curses.c +++ b/ui/curses.c @@ -XXX,XX +XXX,XX @@ static void curses_refresh(DisplayChangeListener *dcl) } } -static void curses_atexit(void) +static void curses_cleanup(void) { + if (!dcl) { + return; + } + endwin(); - g_free(vga_to_curses); - g_free(screen); + qemu_console_unregister_listener(dcl); + g_clear_pointer(&dcl, g_free); + g_clear_pointer(&screenpad, delwin); + g_clear_pointer(&vga_to_curses, g_free); + g_clear_pointer(&screen, g_free); + g_clear_pointer(&kbd_layout, kbd_layout_free); } /* @@ -XXX,XX +XXX,XX @@ static void curses_display_init(DisplayState *ds, DisplayOptions *opts) vga_to_curses = g_new0(cchar_t, 256); curses_setup(); curses_keyboard_setup(); - atexit(curses_atexit); - curses_winch_init(); dcl = g_new0(DisplayChangeListener, 1); @@ -XXX,XX +XXX,XX @@ static void curses_display_init(DisplayState *ds, DisplayOptions *opts) static QemuDisplay qemu_display_curses = { .type = DISPLAY_TYPE_CURSES, .init = curses_display_init, + .cleanup = curses_cleanup, }; static void register_curses(void) -- 2.54.0 Replace the atexit() handler with a proper cleanup callback. Extend sdl_cleanup() to unregister display listeners, free keyboard state, destroy windows, and clean up all cursor resources. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- ui/sdl2.c | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/ui/sdl2.c b/ui/sdl2.c index XXXXXXX..XXXXXXX 100644 --- a/ui/sdl2.c +++ b/ui/sdl2.c @@ -XXX,XX +XXX,XX @@ static void sdl_mouse_define(DisplayChangeListener *dcl, static void sdl_cleanup(void) { - if (guest_sprite) { - SDL_FreeCursor(guest_sprite); + int i; + + if (!sdl2_console) { + return; } + + qemu_remove_mouse_mode_change_notifier(&mouse_mode_notifier); + + for (i = 0; i < sdl2_num_outputs; i++) { + qemu_console_unregister_listener(&sdl2_console[i].dcl); + qkbd_state_free(sdl2_console[i].kbd); + sdl2_window_destroy(&sdl2_console[i]); + } + g_clear_pointer(&sdl2_console, g_free); + sdl2_num_outputs = 0; + + g_clear_pointer(&guest_sprite, SDL_FreeCursor); + g_clear_pointer(&guest_sprite_surface, SDL_FreeSurface); + g_clear_pointer(&sdl_cursor_hidden, SDL_FreeCursor); SDL_QuitSubSystem(SDL_INIT_VIDEO); } @@ -XXX,XX +XXX,XX @@ static void sdl2_display_init(DisplayState *ds, DisplayOptions *o) sdl_grab_start(&sdl2_console[0]); } - atexit(sdl_cleanup); - /* SDL's event polling (in dpy_refresh) must happen on the main thread. */ qemu_main = NULL; } @@ -XXX,XX +XXX,XX @@ static QemuDisplay qemu_display_sdl2 = { .type = DISPLAY_TYPE_SDL, .early_init = sdl2_display_early_init, .init = sdl2_display_init, + .cleanup = sdl_cleanup, }; static void register_sdl1(void) -- 2.54.0 Replace the atexit() handler with the display cleanup callback, reusing the existing spice_app_atexit() function. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- ui/spice-app.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/ui/spice-app.c b/ui/spice-app.c index XXXXXXX..XXXXXXX 100644 --- a/ui/spice-app.c +++ b/ui/spice-app.c @@ -XXX,XX +XXX,XX @@ static const TypeInfo char_vc_type_info = { .class_size = sizeof(VCChardevClass), }; -static void spice_app_atexit(void) +static void spice_app_cleanup(void) { if (sock_path) { unlink(sock_path); + g_clear_pointer(&sock_path, g_free); } if (tmp_dir) { rmdir(tmp_dir); + tmp_dir = NULL; } - g_free(sock_path); - g_free(app_dir); + g_clear_pointer(&app_dir, g_free); } static void spice_app_display_early_init(DisplayOptions *opts) @@ -XXX,XX +XXX,XX @@ static void spice_app_display_early_init(DisplayOptions *opts) exit(1); } - atexit(spice_app_atexit); - if (qemu_name) { app_dir = g_build_filename(g_get_user_runtime_dir(), "qemu", qemu_name, NULL); @@ -XXX,XX +XXX,XX @@ static QemuDisplay qemu_display_spice_app = { .type = DISPLAY_TYPE_SPICE_APP, .early_init = spice_app_display_early_init, .init = spice_app_display_init, + .cleanup = spice_app_cleanup, .vc = "vc", }; -- 2.54.0 Add egl_cleanup() to tear down the EGL render node context, GBM device, and EGL display. Add egl_headless_cleanup() to unregister listeners, destroy framebuffers, and free per-console state tracked via a new GPtrArray. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- include/ui/egl-helpers.h | 1 + ui/egl-headless.c | 31 +++++++++++++++++++++++++++++++ ui/egl-helpers.c | 19 +++++++++++++++++++ 3 files changed, 51 insertions(+) diff --git a/include/ui/egl-helpers.h b/include/ui/egl-helpers.h index XXXXXXX..XXXXXXX 100644 --- a/include/ui/egl-helpers.h +++ b/include/ui/egl-helpers.h @@ -XXX,XX +XXX,XX @@ EGLContext qemu_egl_init_ctx(void); bool qemu_egl_has_dmabuf(void); bool egl_init(const char *rendernode, DisplayGLMode mode, Error **errp); +void egl_cleanup(void); const char *qemu_egl_get_error_string(void); diff --git a/ui/egl-headless.c b/ui/egl-headless.c index XXXXXXX..XXXXXXX 100644 --- a/ui/egl-headless.c +++ b/ui/egl-headless.c @@ -XXX,XX +XXX,XX @@ typedef struct egl_dpy { DisplayChangeListener dcl; + DisplayGLCtx *ctx; DisplaySurface *ds; QemuGLShader *gls; egl_fb guest_fb; @@ -XXX,XX +XXX,XX @@ typedef struct egl_dpy { uint32_t pos_y; } egl_dpy; +static GPtrArray *egl_dpys; + /* ------------------------------------------------------------------ */ static void egl_refresh(DisplayChangeListener *dcl) @@ -XXX,XX +XXX,XX @@ static void egl_headless_init(DisplayState *ds, DisplayOptions *opts) egl_dpy *edpy; int idx; + egl_dpys = g_ptr_array_new(); + for (idx = 0;; idx++) { DisplayGLCtx *ctx; @@ -XXX,XX +XXX,XX @@ static void egl_headless_init(DisplayState *ds, DisplayOptions *opts) edpy->gls = qemu_gl_init_shader(); ctx = g_new0(DisplayGLCtx, 1); ctx->ops = &eglctx_ops; + edpy->ctx = ctx; qemu_console_set_display_gl_ctx(con, ctx); qemu_console_register_listener(con, &edpy->dcl, &egl_ops); + g_ptr_array_add(egl_dpys, edpy); + } +} + +static void egl_headless_cleanup(void) +{ + if (!egl_dpys) { + return; } + + for (guint i = 0; i < egl_dpys->len; i++) { + egl_dpy *edpy = g_ptr_array_index(egl_dpys, i); + + qemu_console_unregister_listener(&edpy->dcl); + qemu_console_set_display_gl_ctx(edpy->dcl.con, NULL); + egl_fb_destroy(&edpy->guest_fb); + egl_fb_destroy(&edpy->cursor_fb); + egl_fb_destroy(&edpy->blit_fb); + qemu_gl_fini_shader(edpy->gls); + g_free(edpy->ctx); + g_free(edpy); + } + g_clear_pointer(&egl_dpys, g_ptr_array_unref); + + egl_cleanup(); } static QemuDisplay qemu_display_egl = { .type = DISPLAY_TYPE_EGL_HEADLESS, .early_init = early_egl_headless_init, .init = egl_headless_init, + .cleanup = egl_headless_cleanup, }; static void register_egl(void) diff --git a/ui/egl-helpers.c b/ui/egl-helpers.c index XXXXXXX..XXXXXXX 100644 --- a/ui/egl-helpers.c +++ b/ui/egl-helpers.c @@ -XXX,XX +XXX,XX @@ bool egl_init(const char *rendernode, DisplayGLMode mode, Error **errp) display_opengl = 1; return true; } + +void egl_cleanup(void) +{ + if (qemu_egl_rn_ctx) { + eglDestroyContext(qemu_egl_display, qemu_egl_rn_ctx); + qemu_egl_rn_ctx = NULL; + } + +#ifdef CONFIG_GBM + g_clear_pointer(&qemu_egl_rn_gbm_dev, gbm_device_destroy); + g_clear_fd(&qemu_egl_rn_fd, NULL); +#endif + + if (qemu_egl_display) { + eglReleaseThread(); + eglTerminate(qemu_egl_display); + qemu_egl_display = NULL; + } +} -- 2.54.0 Move cbowner release from QemuCocoaAppController -dealloc to cocoa_display_cleanup(), since cbowner is allocated in cocoa_display_init() and cleanup is the symmetric teardown path. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- ui/cocoa.m | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/ui/cocoa.m b/ui/cocoa.m index XXXXXXX..XXXXXXX 100644 --- a/ui/cocoa.m +++ b/ui/cocoa.m @@ -XXX,XX +XXX,XX @@ - (void) dealloc COCOA_DEBUG("QemuCocoaAppController: dealloc\n"); [cocoaView release]; - [cbowner release]; - cbowner = nil; [super dealloc]; } @@ -XXX,XX +XXX,XX @@ static void cocoa_display_init(DisplayState *ds, DisplayOptions *opts) qemu_main = cocoa_main; } +static void cocoa_display_cleanup(void) +{ + if (!kbd) { + return; + } + + qemu_console_unregister_listener(&dcl); + g_clear_pointer(&kbd, qkbd_state_free); + qemu_remove_mouse_mode_change_notifier(&mouse_mode_change_notifier); + qemu_clipboard_peer_unregister(&cbpeer); + g_clear_pointer(&cbinfo, qemu_clipboard_info_unref); + qemu_event_destroy(&cbevent); + [cbowner release]; + cbowner = nil; +} + static QemuDisplay qemu_display_cocoa = { .type = DISPLAY_TYPE_COCOA, .init = cocoa_display_init, + .cleanup = cocoa_display_cleanup, }; static void register_cocoa(void) -- 2.54.0 Add dbus_cleanup() to unparent the D-Bus display object, ensuring proper teardown before user_creatable_cleanup() runs. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- ui/dbus-console.c | 3 +++ ui/dbus.c | 13 +++++++++++++ 2 files changed, 16 insertions(+) diff --git a/ui/dbus-console.c b/ui/dbus-console.c index XXXXXXX..XXXXXXX 100644 --- a/ui/dbus-console.c +++ b/ui/dbus-console.c @@ -XXX,XX +XXX,XX @@ dbus_display_console_dispose(GObject *object) DBusDisplayConsole *ddc = DBUS_DISPLAY_CONSOLE(object); qemu_console_unregister_listener(&ddc->dcl); + if (ddc->dcl.con) { + qemu_console_set_display_gl_ctx(ddc->dcl.con, NULL); + } qemu_remove_mouse_mode_change_notifier(&ddc->mouse_mode_notifier); g_clear_object(&ddc->iface_touch); g_clear_object(&ddc->iface_mouse); diff --git a/ui/dbus.c b/ui/dbus.c index XXXXXXX..XXXXXXX 100644 --- a/ui/dbus.c +++ b/ui/dbus.c @@ -XXX,XX +XXX,XX @@ static const TypeInfo dbus_display_info = { } }; +static void +dbus_cleanup(void) +{ + Object *o; + + o = object_resolve_path_component(object_get_objects_root(), + "dbus-display"); + if (o) { + object_unparent(o); + } +} + static QemuDisplay qemu_display_dbus = { .type = DISPLAY_TYPE_DBUS, .early_init = early_dbus_init, .init = dbus_init, + .cleanup = dbus_cleanup, .vc = "vc", }; -- 2.54.0 Add gtk_display_cleanup() to properly tear down GTK display state: remove console and mouse notifiers, unregister clipboard peer, destroy the main window and virtual consoles. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- include/ui/gtk.h | 1 + ui/gtk-clipboard.c | 15 +++++++++++++++ ui/gtk.c | 18 ++++++++++++++++++ 3 files changed, 34 insertions(+) diff --git a/include/ui/gtk.h b/include/ui/gtk.h index XXXXXXX..XXXXXXX 100644 --- a/include/ui/gtk.h +++ b/include/ui/gtk.h @@ -XXX,XX +XXX,XX @@ int gd_gl_area_make_current(DisplayGLCtx *dgc, /* gtk-clipboard.c */ void gd_clipboard_init(GtkDisplayState *gd); +void gd_clipboard_cleanup(GtkDisplayState *gd); void gd_update_scale(VirtualConsole *vc, int ww, int wh, int fbw, int fbh); diff --git a/ui/gtk-clipboard.c b/ui/gtk-clipboard.c index XXXXXXX..XXXXXXX 100644 --- a/ui/gtk-clipboard.c +++ b/ui/gtk-clipboard.c @@ -XXX,XX +XXX,XX @@ void gd_clipboard_init(GtkDisplayState *gd) g_signal_connect(gd->gtkcb[QEMU_CLIPBOARD_SELECTION_SECONDARY], "owner-change", G_CALLBACK(gd_owner_change), gd); } + +void gd_clipboard_cleanup(GtkDisplayState *gd) +{ + if (!gd->cbpeer.name) { + return; + } + qemu_clipboard_peer_unregister(&gd->cbpeer); + g_signal_handlers_disconnect_by_data( + gd->gtkcb[QEMU_CLIPBOARD_SELECTION_CLIPBOARD], gd); + g_signal_handlers_disconnect_by_data( + gd->gtkcb[QEMU_CLIPBOARD_SELECTION_PRIMARY], gd); + g_signal_handlers_disconnect_by_data( + gd->gtkcb[QEMU_CLIPBOARD_SELECTION_SECONDARY], gd); + gd->cbpeer.name = NULL; +} diff --git a/ui/gtk.c b/ui/gtk.c index XXXXXXX..XXXXXXX 100644 --- a/ui/gtk.c +++ b/ui/gtk.c @@ -XXX,XX +XXX,XX @@ static void gd_create_menus(GtkDisplayState *s, DisplayOptions *opts) } +static GtkDisplayState *gtk_display_state; static gboolean gtkinit; static void gtk_display_init(DisplayState *ds, DisplayOptions *opts) @@ -XXX,XX +XXX,XX @@ static void gtk_display_init(DisplayState *ds, DisplayOptions *opts) } assert(opts->type == DISPLAY_TYPE_GTK); s = g_malloc0(sizeof(*s)); + gtk_display_state = s; s->opts = opts; theme = gtk_icon_theme_get_default(); @@ -XXX,XX +XXX,XX @@ static void early_gtk_display_init(DisplayOptions *opts) #endif } +static void gtk_display_cleanup(void) +{ + GtkDisplayState *s = gtk_display_state; + + if (!s) { + return; + } + + qemu_remove_mouse_mode_change_notifier(&s->mouse_mode_notifier); + gd_clipboard_cleanup(s); + g_clear_pointer(&s->window, gtk_widget_destroy); + g_clear_object(&s->null_cursor); + g_clear_pointer(&gtk_display_state, g_free); +} + static QemuDisplay qemu_display_gtk = { .type = DISPLAY_TYPE_GTK, .early_init = early_gtk_display_init, .init = gtk_display_init, + .cleanup = gtk_display_cleanup, .vc = "vc", }; -- 2.54.0 Add a NotifierList to DisplayState so display backends can be notified when consoles are added or removed at runtime. No events are fired yet, that follows in the next commits. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- include/ui/console.h | 18 ++++++++++++++++++ ui/console.c | 23 +++++++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/include/ui/console.h b/include/ui/console.h index XXXXXXX..XXXXXXX 100644 --- a/include/ui/console.h +++ b/include/ui/console.h @@ -XXX,XX +XXX,XX @@ const char *qemu_display_get_vc(DisplayOptions *opts); void qemu_display_cleanup(void); void qemu_display_help(void); +/* + * Console notifications: + * Handlers must be idempotent, it may notify multiple times. + */ +typedef enum { + QEMU_CONSOLE_ADDED, + QEMU_CONSOLE_REMOVED, +} QemuConsoleEventType; + +typedef struct QemuConsoleEvent { + QemuConsoleEventType type; + QemuConsole *con; +} QemuConsoleEvent; + +void qemu_console_add_notifier(Notifier *notifier); +void qemu_console_remove_notifier(Notifier *notifier); +void qemu_console_notify(QemuConsoleEventType type, QemuConsole *con); + /* vnc.c */ void vnc_display_add_client(const char *id, int csock, bool skipauth); int vnc_display_password(const char *id, const char *password, Error **errp); diff --git a/ui/console.c b/ui/console.c index XXXXXXX..XXXXXXX 100644 --- a/ui/console.c +++ b/ui/console.c @@ -XXX,XX +XXX,XX @@ struct DisplayState { bool refreshing; QLIST_HEAD(, DisplayChangeListener) listeners; + NotifierList console_notifiers; }; static DisplayState *display_state; @@ -XXX,XX +XXX,XX @@ static bool console_compatible_with(QemuConsole *con, static QemuConsole *qemu_graphic_console_lookup_unused(void); static void dpy_set_ui_info_timer(void *opaque); +void qemu_console_notify(QemuConsoleEventType type, QemuConsole *con) +{ + DisplayState *ds = get_alloc_displaystate(); + QemuConsoleEvent event = { + .type = type, + .con = con, + }; + notifier_list_notify(&ds->console_notifiers, &event); +} + static void gui_update(void *opaque) { uint64_t interval = GUI_REFRESH_INTERVAL_IDLE; @@ -XXX,XX +XXX,XX @@ static DisplayState *get_alloc_displaystate(void) { if (!display_state) { display_state = g_new0(DisplayState, 1); + notifier_list_init(&display_state->console_notifiers); } return display_state; } +void qemu_console_add_notifier(Notifier *notifier) +{ + DisplayState *ds = get_alloc_displaystate(); + notifier_list_add(&ds->console_notifiers, notifier); +} + +void qemu_console_remove_notifier(Notifier *notifier) +{ + notifier_remove(notifier); +} + /* * Called by main(), after creating QemuConsoles * and before initializing ui (sdl/vnc/...). -- 2.54.0 Fire CONSOLE_ADDED at the end of graphic_console_init() and CONSOLE_REMOVED at the start of graphic_console_close(), so display backends can react to console hotplug/unplug events. REMOVED fires before the device link is cleared and before the placeholder surface swap, so handlers can unregister their DCL while the console is still in a known state. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- ui/console.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ui/console.c b/ui/console.c index XXXXXXX..XXXXXXX 100644 --- a/ui/console.c +++ b/ui/console.c @@ -XXX,XX +XXX,XX @@ QemuConsole *qemu_graphic_console_create(DeviceState *dev, uint32_t head, surface = qemu_create_placeholder_surface(width, height, noinit); qemu_console_set_surface(s, surface); + qemu_console_notify(QEMU_CONSOLE_ADDED, s); return s; } @@ -XXX,XX +XXX,XX @@ void qemu_graphic_console_close(QemuConsole *con) int height = qemu_console_get_height(con, 480); trace_console_gfx_close(con->index); + qemu_console_notify(QEMU_CONSOLE_REMOVED, con); object_property_set_link(OBJECT(con), "device", NULL, &error_abort); qemu_graphic_console_set_hwops(con, &unused_ops, NULL); timer_del(con->ui_timer); -- 2.54.0 Fire CONSOLE_ADDED when the chardev is opened. Fire CONSOLE_REMOVED in char_vc_finalize() before dropping the console reference, so the console is still in a valid state when listeners handle the event. Also fixes a console object leak by adding the missing object_unref(). Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- ui/console-vc.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/ui/console-vc.c b/ui/console-vc.c index XXXXXXX..XXXXXXX 100644 --- a/ui/console-vc.c +++ b/ui/console-vc.c @@ -XXX,XX +XXX,XX @@ static bool vc_chr_open(Chardev *chr, ChardevBackend *backend, Error **errp) } qemu_chr_be_event(chr, CHR_EVENT_OPENED); + qemu_console_notify(QEMU_CONSOLE_ADDED, QEMU_CONSOLE(s)); return true; } @@ -XXX,XX +XXX,XX @@ static void char_vc_init(Object *obj) vc->encoding = CHARDEV_VC_ENCODING_UTF8; } +static void char_vc_finalize(Object *obj) +{ + VCChardev *vc = VC_CHARDEV(obj); + QemuConsole *con = QEMU_CONSOLE(vc->console); + + if (con) { + qemu_console_notify(QEMU_CONSOLE_REMOVED, con); + object_unref(con); + } +} + static const TypeInfo char_vc_type_info = { .name = TYPE_CHARDEV_VC, .parent = TYPE_CHARDEV, .instance_size = sizeof(VCChardev), .instance_init = char_vc_init, .instance_post_init = object_apply_compat_props, + .instance_finalize = char_vc_finalize, .class_init = char_vc_class_init, }; -- 2.54.0 Replace the fixed-size vc[MAX_VCS] with GPtrArray. This is a preparatory refactoring for console hotplug support, which needs to add/remove VCs dynamically. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- include/ui/gtk.h | 3 +- ui/gtk.c | 102 +++++++++++++++++++++++++++++++++++++++++++------------ 2 files changed, 81 insertions(+), 24 deletions(-) diff --git a/include/ui/gtk.h b/include/ui/gtk.h index XXXXXXX..XXXXXXX 100644 --- a/include/ui/gtk.h +++ b/include/ui/gtk.h @@ -XXX,XX +XXX,XX @@ struct GtkDisplayState { GtkWidget *grab_item; GtkWidget *grab_on_hover_item; - int nb_vcs; - VirtualConsole vc[MAX_VCS]; + GPtrArray *vcs; GtkWidget *show_tabs_item; GtkWidget *untabify_item; diff --git a/ui/gtk.c b/ui/gtk.c index XXXXXXX..XXXXXXX 100644 --- a/ui/gtk.c +++ b/ui/gtk.c @@ -XXX,XX +XXX,XX @@ static VirtualConsole *gd_vc_find_by_menu(GtkDisplayState *s) VirtualConsole *vc; gint i; - for (i = 0; i < s->nb_vcs; i++) { - vc = &s->vc[i]; + for (i = 0; i < s->vcs->len; i++) { + vc = g_ptr_array_index(s->vcs, i); if (gtk_check_menu_item_get_active (GTK_CHECK_MENU_ITEM(vc->menu_item))) { return vc; @@ -XXX,XX +XXX,XX @@ static VirtualConsole *gd_vc_find_by_page(GtkDisplayState *s, gint page) VirtualConsole *vc; gint i, p; - for (i = 0; i < s->nb_vcs; i++) { - vc = &s->vc[i]; + if (!s->vcs) { + return NULL; + } + for (i = 0; i < s->vcs->len; i++) { + vc = g_ptr_array_index(s->vcs, i); p = gtk_notebook_page_num(GTK_NOTEBOOK(s->notebook), vc->tab_item); if (p == page) { return vc; @@ -XXX,XX +XXX,XX @@ static void gd_update_caption(GtkDisplayState *s) gtk_window_set_title(GTK_WINDOW(s->window), title); g_free(title); - for (i = 0; i < s->nb_vcs; i++) { - VirtualConsole *vc = &s->vc[i]; + for (i = 0; i < s->vcs->len; i++) { + VirtualConsole *vc = g_ptr_array_index(s->vcs, i); if (!vc->window) { continue; @@ -XXX,XX +XXX,XX @@ static void gtk_release_modifiers(GtkDisplayState *s) { VirtualConsole *vc = gd_vc_find_current(s); - if (vc->type != GD_VC_GFX || + if (!vc || vc->type != GD_VC_GFX || !qemu_console_is_graphic(vc->gfx.dcl.con)) { return; } @@ -XXX,XX +XXX,XX @@ static void gd_mouse_mode_change(Notifier *notify, void *data) gd_ungrab_pointer(s); } } - for (i = 0; i < s->nb_vcs; i++) { - VirtualConsole *vc = &s->vc[i]; + for (i = 0; i < s->vcs->len; i++) { + VirtualConsole *vc = g_ptr_array_index(s->vcs, i); gd_update_cursor(vc); } } @@ -XXX,XX +XXX,XX @@ static void gd_vcs_init(GtkDisplayState *s, GSList *group, int i; for (i = 0; i < nb_vcs; i++) { - VirtualConsole *vc = &s->vc[s->nb_vcs]; - group = gd_vc_vte_init(s, vc, vcs[i], s->nb_vcs, group, view_menu); - s->nb_vcs++; + VirtualConsole *vc = g_new0(VirtualConsole, 1); + g_ptr_array_add(s->vcs, vc); + group = gd_vc_vte_init(s, vc, vcs[i], s->vcs->len - 1, + group, view_menu); } } #endif /* CONFIG_VTE */ @@ -XXX,XX +XXX,XX @@ static GtkWidget *gd_create_menu_view(GtkDisplayState *s, DisplayOptions *opts) /* gfx */ for (vc = 0;; vc++) { + VirtualConsole *v; con = qemu_console_lookup_by_index(vc); if (!con) { break; } - group = gd_vc_gfx_init(s, &s->vc[vc], con, - vc, group, view_menu); - s->nb_vcs++; + v = g_new0(VirtualConsole, 1); + g_ptr_array_add(s->vcs, v); + group = gd_vc_gfx_init(s, v, con, vc, group, view_menu); } #if defined(CONFIG_VTE) @@ -XXX,XX +XXX,XX @@ static void gd_create_menus(GtkDisplayState *s, DisplayOptions *opts) } +static void gd_vc_free(void *p) +{ + VirtualConsole *vc = p; + + switch (vc->type) { + case GD_VC_GFX: + qemu_console_unregister_listener(&vc->gfx.dcl); +#if defined(CONFIG_OPENGL) + if (display_opengl) { + qemu_console_set_display_gl_ctx(vc->gfx.dcl.con, NULL); + } + if (vc->gfx.esurface) { + eglDestroySurface(qemu_egl_display, vc->gfx.esurface); + } + if (vc->gfx.ectx) { + eglDestroyContext(qemu_egl_display, vc->gfx.ectx); + } + if (vc->gfx.gls) { + surface_gl_destroy_texture(vc->gfx.gls, vc->gfx.ds); + qemu_gl_fini_shader(vc->gfx.gls); + } + egl_fb_destroy(&vc->gfx.guest_fb); + egl_fb_destroy(&vc->gfx.win_fb); + egl_fb_destroy(&vc->gfx.cursor_fb); +#endif + qkbd_state_free(vc->gfx.kbd); + if (vc->gfx.surface) { + cairo_surface_destroy(vc->gfx.surface); + } + if (vc->gfx.convert) { + pixman_image_unref(vc->gfx.convert); + } + break; +#if defined(CONFIG_VTE) + case GD_VC_VTE: + fifo8_destroy(&vc->vte.out_fifo); + break; +#endif + } + + if (vc->window) { + gtk_widget_destroy(vc->window); + } else if (vc->tab_item) { + gtk_widget_destroy(vc->tab_item); + } + if (vc->menu_item) { + gtk_widget_destroy(vc->menu_item); + } + g_free(vc->label); + g_free(vc); +} + static GtkDisplayState *gtk_display_state; static gboolean gtkinit; @@ -XXX,XX +XXX,XX @@ static void gtk_display_init(DisplayState *ds, DisplayOptions *opts) assert(opts->type == DISPLAY_TYPE_GTK); s = g_malloc0(sizeof(*s)); gtk_display_state = s; + s->vcs = g_ptr_array_new_with_free_func(gd_vc_free); s->opts = opts; theme = gtk_icon_theme_get_default(); @@ -XXX,XX +XXX,XX @@ static void gtk_display_init(DisplayState *ds, DisplayOptions *opts) gtk_widget_show_all(s->window); - for (idx = 0;; idx++) { - QemuConsole *con = qemu_console_lookup_by_index(idx); - if (!con) { - break; - } - if (s->vc[idx].type == GD_VC_GFX) { - gtk_widget_realize(s->vc[idx].gfx.drawing_area); + for (idx = 0; idx < s->vcs->len; idx++) { + VirtualConsole *v = g_ptr_array_index(s->vcs, idx); + if (v->type == GD_VC_GFX) { + gtk_widget_realize(v->gfx.drawing_area); } } @@ -XXX,XX +XXX,XX @@ static void gtk_display_cleanup(void) qemu_remove_mouse_mode_change_notifier(&s->mouse_mode_notifier); gd_clipboard_cleanup(s); + g_signal_handlers_disconnect_by_func(s->notebook, + G_CALLBACK(gd_change_page), s); + g_clear_pointer(&s->vcs, g_ptr_array_unref); g_clear_pointer(&s->window, gtk_widget_destroy); g_clear_object(&s->null_cursor); g_clear_pointer(&gtk_display_state, g_free); -- 2.54.0 Move zoom_to_fit, keep_aspect_ratio and touch_slots initialisation from gd_vc_gfx_init() to gd_create_menu_view(). These are global display settings that should be set once after all consoles are created, not repeated on every per-console init (where the last iteration's values silently win). Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- ui/gtk.c | 46 +++++++++++++++++++++++----------------------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/ui/gtk.c b/ui/gtk.c index XXXXXXX..XXXXXXX 100644 --- a/ui/gtk.c +++ b/ui/gtk.c @@ -XXX,XX +XXX,XX @@ static GSList *gd_vc_gfx_init(GtkDisplayState *s, VirtualConsole *vc, GSList *group, GtkWidget *view_menu) { const DisplayChangeListenerOps *ops = &dcl_ops; - bool zoom_to_fit = false; - int i; vc->label = qemu_console_get_label(con); vc->s = s; @@ -XXX,XX +XXX,XX @@ static GSList *gd_vc_gfx_init(GtkDisplayState *s, VirtualConsole *vc, gd_connect_vc_gfx_signals(vc); group = gd_vc_menu_init(s, vc, idx, group, view_menu); - if (qemu_console_ui_info_supported(vc->gfx.dcl.con)) { - zoom_to_fit = true; - } - if (s->opts->u.gtk.has_zoom_to_fit) { - zoom_to_fit = s->opts->u.gtk.zoom_to_fit; - } - if (zoom_to_fit) { - gtk_menu_item_activate(GTK_MENU_ITEM(s->zoom_fit_item)); - s->free_scale = true; - } - - s->keep_aspect_ratio = true; - if (s->opts->u.gtk.has_keep_aspect_ratio) - s->keep_aspect_ratio = s->opts->u.gtk.keep_aspect_ratio; - - for (i = 0; i < INPUT_EVENT_SLOTS_MAX; i++) { - struct touch_slot *slot = &touch_slots[i]; - slot->tracking_id = -1; - } - return group; } @@ -XXX,XX +XXX,XX @@ static GtkWidget *gd_create_menu_view(GtkDisplayState *s, DisplayOptions *opts) GtkWidget *view_menu; GtkWidget *separator; QemuConsole *con; - int vc; + bool zoom_to_fit = false; + int vc, i; view_menu = gtk_menu_new(); gtk_menu_set_accel_group(GTK_MENU(view_menu), s->accel_group); @@ -XXX,XX +XXX,XX @@ static GtkWidget *gd_create_menu_view(GtkDisplayState *s, DisplayOptions *opts) v = g_new0(VirtualConsole, 1); g_ptr_array_add(s->vcs, v); group = gd_vc_gfx_init(s, v, con, vc, group, view_menu); + if (qemu_console_ui_info_supported(con)) { + zoom_to_fit = true; + } + } + + if (s->opts->u.gtk.has_zoom_to_fit) { + zoom_to_fit = s->opts->u.gtk.zoom_to_fit; + } + if (zoom_to_fit) { + gtk_menu_item_activate(GTK_MENU_ITEM(s->zoom_fit_item)); + s->free_scale = true; + } + + s->keep_aspect_ratio = true; + if (s->opts->u.gtk.has_keep_aspect_ratio) { + s->keep_aspect_ratio = s->opts->u.gtk.keep_aspect_ratio; + } + + for (i = 0; i < INPUT_EVENT_SLOTS_MAX; i++) { + struct touch_slot *slot = &touch_slots[i]; + slot->tracking_id = -1; } #if defined(CONFIG_VTE) -- 2.54.0 Add gd_vc_notebook_pos() which computes the correct notebook position for a console by counting only non-detached (non-windowed) tabs before it. Use it in gd_tab_window_close() so a re-attached tab is inserted at its logical position rather than appended at the end. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- ui/gtk.c | 27 ++++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/ui/gtk.c b/ui/gtk.c index XXXXXXX..XXXXXXX 100644 --- a/ui/gtk.c +++ b/ui/gtk.c @@ -XXX,XX +XXX,XX @@ static void gd_menu_show_tabs(GtkMenuItem *item, void *opaque) gd_update_windowsize(vc); } +static int gd_vc_notebook_pos(GtkDisplayState *s, VirtualConsole *target) +{ + int pos = 0; + guint i; + + for (i = 0; i < s->vcs->len; i++) { + VirtualConsole *vc = g_ptr_array_index(s->vcs, i); + if (vc == target) { + return pos; + } + if (!vc->window) { + pos++; + } + } + g_assert_not_reached(); +} + static gboolean gd_tab_window_close(GtkWidget *widget, GdkEvent *event, void *opaque) { VirtualConsole *vc = opaque; GtkDisplayState *s = vc->s; + int page; gtk_widget_set_sensitive(vc->menu_item, true); - gd_widget_reparent(vc->window, s->notebook, vc->tab_item); - gtk_notebook_set_tab_label_text(GTK_NOTEBOOK(s->notebook), - vc->tab_item, vc->label); + g_object_ref(vc->tab_item); + gtk_container_remove(GTK_CONTAINER(vc->window), vc->tab_item); + page = gd_vc_notebook_pos(s, vc); + gtk_notebook_insert_page(GTK_NOTEBOOK(s->notebook), + vc->tab_item, gtk_label_new(vc->label), page); + g_object_unref(vc->tab_item); gtk_widget_destroy(vc->window); vc->window = NULL; #if defined(CONFIG_OPENGL) -- 2.54.0 Replace the per-console gd_vc_menu_init() with gd_rebuild_vc_menu() that tears down and rebuilds all console radio menu items and Ctrl+Alt+N accelerators at once. This is called from initialization and whenever consoles are detached or reattached. Shortcuts now skip detached (windowed) consoles, so they always map to reachable tabs. Rename gd_vc_gfx_init() to add_gfx_console() and simplify the init function signatures now that menu creation is decoupled. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- include/ui/gtk.h | 1 + ui/gtk.c | 127 ++++++++++++++++++++++++++++++++++++------------------- 2 files changed, 85 insertions(+), 43 deletions(-) diff --git a/include/ui/gtk.h b/include/ui/gtk.h index XXXXXXX..XXXXXXX 100644 --- a/include/ui/gtk.h +++ b/include/ui/gtk.h @@ -XXX,XX +XXX,XX @@ struct GtkDisplayState { GPtrArray *vcs; + GtkWidget *vc_menu_separator; GtkWidget *show_tabs_item; GtkWidget *untabify_item; GtkWidget *show_menubar_item; diff --git a/ui/gtk.c b/ui/gtk.c index XXXXXXX..XXXXXXX 100644 --- a/ui/gtk.c +++ b/ui/gtk.c @@ -XXX,XX +XXX,XX @@ static void gd_grab_pointer(VirtualConsole *vc, const char *reason); static void gd_ungrab_pointer(GtkDisplayState *s); static void gd_grab_keyboard(VirtualConsole *vc, const char *reason); static void gd_ungrab_keyboard(GtkDisplayState *s); +static void gd_rebuild_vc_menu(GtkDisplayState *s); /** Utility Functions **/ @@ -XXX,XX +XXX,XX @@ static gboolean gd_tab_window_close(GtkWidget *widget, GdkEvent *event, GtkDisplayState *s = vc->s; int page; - gtk_widget_set_sensitive(vc->menu_item, true); g_object_ref(vc->tab_item); gtk_container_remove(GTK_CONTAINER(vc->window), vc->tab_item); page = gd_vc_notebook_pos(s, vc); @@ -XXX,XX +XXX,XX @@ static gboolean gd_tab_window_close(GtkWidget *widget, GdkEvent *event, } #endif + gd_rebuild_vc_menu(s); + if (vc == gd_vc_find_by_menu(s)) { gtk_widget_grab_focus(vc->focus); } @@ -XXX,XX +XXX,XX @@ static void gd_menu_untabify(GtkMenuItem *item, void *opaque) FALSE); } if (!vc->window) { - gtk_widget_set_sensitive(vc->menu_item, false); vc->window = gtk_window_new(GTK_WINDOW_TOPLEVEL); #if defined(CONFIG_OPENGL) if (vc->gfx.esurface) { @@ -XXX,XX +XXX,XX @@ static void gd_menu_untabify(GtkMenuItem *item, void *opaque) gtk_accel_group_connect(ag, GDK_KEY_g, HOTKEY_MODIFIERS, 0, cb); } + gd_rebuild_vc_menu(s); gd_update_geometry_hints(vc); gd_update_caption(s); } @@ -XXX,XX +XXX,XX @@ static gboolean gd_configure(GtkWidget *widget, /** Virtual Console Callbacks **/ -static GSList *gd_vc_menu_init(GtkDisplayState *s, VirtualConsole *vc, - int idx, GSList *group, GtkWidget *view_menu) +static void gd_rebuild_vc_menu(GtkDisplayState *s) { - vc->menu_item = gtk_radio_menu_item_new_with_mnemonic(group, vc->label); - gtk_accel_group_connect(s->accel_group, GDK_KEY_1 + idx, - HOTKEY_MODIFIERS, 0, - g_cclosure_new_swap(G_CALLBACK(gd_accel_switch_vc), vc, NULL)); - gtk_accel_label_set_accel( - GTK_ACCEL_LABEL(gtk_bin_get_child(GTK_BIN(vc->menu_item))), - GDK_KEY_1 + idx, HOTKEY_MODIFIERS); + GSList *group = NULL; + VirtualConsole *vc; + GList *children; + gint insert_pos; + int shortcut_idx = 0; + guint i; + + for (i = 0; i < s->vcs->len; i++) { + vc = g_ptr_array_index(s->vcs, i); + if (vc->menu_item) { + gtk_widget_destroy(vc->menu_item); + vc->menu_item = NULL; + } + } + + for (i = 0; i < 9; i++) { + gtk_accel_group_disconnect_key(s->accel_group, + GDK_KEY_1 + i, HOTKEY_MODIFIERS); + } + + /* find insertion position (just before vc_menu_separator) */ + children = gtk_container_get_children(GTK_CONTAINER(s->view_menu)); + insert_pos = g_list_index(children, s->vc_menu_separator); + g_list_free(children); + + /* create new menu items for each console */ + for (i = 0; i < s->vcs->len; i++) { + vc = g_ptr_array_index(s->vcs, i); - g_signal_connect(vc->menu_item, "activate", - G_CALLBACK(gd_menu_switch_vc), s); - gtk_menu_shell_append(GTK_MENU_SHELL(view_menu), vc->menu_item); + vc->menu_item = gtk_radio_menu_item_new_with_mnemonic(group, + vc->label); + group = gtk_radio_menu_item_get_group( + GTK_RADIO_MENU_ITEM(vc->menu_item)); + + if (vc->window) { + gtk_widget_set_sensitive(vc->menu_item, false); + } else if (shortcut_idx < 9) { + guint key = GDK_KEY_1 + shortcut_idx; + gtk_accel_group_connect(s->accel_group, key, + HOTKEY_MODIFIERS, 0, + g_cclosure_new_swap(G_CALLBACK(gd_accel_switch_vc), + vc, NULL)); + gtk_accel_label_set_accel( + GTK_ACCEL_LABEL(gtk_bin_get_child(GTK_BIN(vc->menu_item))), + key, HOTKEY_MODIFIERS); + shortcut_idx++; + } - return gtk_radio_menu_item_get_group(GTK_RADIO_MENU_ITEM(vc->menu_item)); + g_signal_connect(vc->menu_item, "activate", + G_CALLBACK(gd_menu_switch_vc), s); + gtk_menu_shell_insert(GTK_MENU_SHELL(s->view_menu), + vc->menu_item, insert_pos + i); + gtk_widget_show(vc->menu_item); + } + + /* sync active menu item with current notebook page */ + vc = gd_vc_find_current(s); + if (vc && vc->menu_item) { + g_signal_handlers_block_by_func(vc->menu_item, + gd_menu_switch_vc, s); + gtk_check_menu_item_set_active( + GTK_CHECK_MENU_ITEM(vc->menu_item), TRUE); + g_signal_handlers_unblock_by_func(vc->menu_item, + gd_menu_switch_vc, s); + } } #if defined(CONFIG_VTE) @@ -XXX,XX +XXX,XX @@ static gboolean gd_vc_in(VteTerminal *terminal, gchar *text, guint size, return TRUE; } -static GSList *gd_vc_vte_init(GtkDisplayState *s, VirtualConsole *vc, - Chardev *chr, int idx, - GSList *group, GtkWidget *view_menu) +static void gd_vc_vte_init(GtkDisplayState *s, VirtualConsole *vc, + Chardev *chr, int idx) { char buffer[32]; GtkWidget *box; @@ -XXX,XX +XXX,XX @@ static GSList *gd_vc_vte_init(GtkDisplayState *s, VirtualConsole *vc, snprintf(buffer, sizeof(buffer), "vc%d", idx); vc->label = g_strdup(vc->vte.chr->label ? : buffer); - group = gd_vc_menu_init(s, vc, idx, group, view_menu); vc->vte.terminal = vte_terminal_new(); g_signal_connect(vc->vte.terminal, "commit", G_CALLBACK(gd_vc_in), vc); @@ -XXX,XX +XXX,XX @@ static GSList *gd_vc_vte_init(GtkDisplayState *s, VirtualConsole *vc, gtk_label_new(vc->label)); qemu_chr_be_event(vc->vte.chr, CHR_EVENT_OPENED); - - return group; } -static void gd_vcs_init(GtkDisplayState *s, GSList *group, - GtkWidget *view_menu) +static void gd_vcs_init(GtkDisplayState *s) { int i; for (i = 0; i < nb_vcs; i++) { VirtualConsole *vc = g_new0(VirtualConsole, 1); g_ptr_array_add(s->vcs, vc); - group = gd_vc_vte_init(s, vc, vcs[i], s->vcs->len - 1, - group, view_menu); + gd_vc_vte_init(s, vc, vcs[i], s->vcs->len - 1); } } #endif /* CONFIG_VTE */ @@ -XXX,XX +XXX,XX @@ static bool gd_scale_valid(double scale) return scale >= VC_SCALE_MIN && scale <= VC_SCALE_MAX; } -static GSList *gd_vc_gfx_init(GtkDisplayState *s, VirtualConsole *vc, - QemuConsole *con, int idx, - GSList *group, GtkWidget *view_menu) +static void add_gfx_console(GtkDisplayState *s, QemuConsole *con) { + VirtualConsole *vc = g_new0(VirtualConsole, 1); const DisplayChangeListenerOps *ops = &dcl_ops; + g_ptr_array_add(s->vcs, vc); vc->label = qemu_console_get_label(con); vc->s = s; vc->gfx.preferred_scale = 1.0; @@ -XXX,XX +XXX,XX @@ static GSList *gd_vc_gfx_init(GtkDisplayState *s, VirtualConsole *vc, qemu_console_register_listener(con, &vc->gfx.dcl, ops); gd_connect_vc_gfx_signals(vc); - group = gd_vc_menu_init(s, vc, idx, group, view_menu); - - return group; } -static GtkWidget *gd_create_menu_view(GtkDisplayState *s, DisplayOptions *opts) +static void gd_create_menu_view(GtkDisplayState *s, DisplayOptions *opts) { - GSList *group = NULL; GtkWidget *view_menu; GtkWidget *separator; QemuConsole *con; @@ -XXX,XX +XXX,XX @@ static GtkWidget *gd_create_menu_view(GtkDisplayState *s, DisplayOptions *opts) int vc, i; view_menu = gtk_menu_new(); + s->view_menu = view_menu; gtk_menu_set_accel_group(GTK_MENU(view_menu), s->accel_group); s->full_screen_item = gtk_menu_item_new_with_mnemonic(_("_Fullscreen")); @@ -XXX,XX +XXX,XX @@ static GtkWidget *gd_create_menu_view(GtkDisplayState *s, DisplayOptions *opts) /* gfx */ for (vc = 0;; vc++) { - VirtualConsole *v; con = qemu_console_lookup_by_index(vc); if (!con) { break; } - v = g_new0(VirtualConsole, 1); - g_ptr_array_add(s->vcs, v); - group = gd_vc_gfx_init(s, v, con, vc, group, view_menu); + add_gfx_console(s, con); if (qemu_console_ui_info_supported(con)) { zoom_to_fit = true; } @@ -XXX,XX +XXX,XX @@ static GtkWidget *gd_create_menu_view(GtkDisplayState *s, DisplayOptions *opts) #if defined(CONFIG_VTE) /* vte */ - gd_vcs_init(s, group, view_menu); + gd_vcs_init(s); #endif - separator = gtk_separator_menu_item_new(); - gtk_menu_shell_append(GTK_MENU_SHELL(view_menu), separator); + s->vc_menu_separator = gtk_separator_menu_item_new(); + gtk_menu_shell_append(GTK_MENU_SHELL(view_menu), s->vc_menu_separator); + + gd_rebuild_vc_menu(s); s->show_tabs_item = gtk_check_menu_item_new_with_mnemonic(_("Show _Tabs")); gtk_menu_shell_append(GTK_MENU_SHELL(view_menu), s->show_tabs_item); @@ -XXX,XX +XXX,XX @@ static GtkWidget *gd_create_menu_view(GtkDisplayState *s, DisplayOptions *opts) GTK_ACCEL_LABEL(gtk_bin_get_child(GTK_BIN(s->show_menubar_item))), GDK_KEY_m, HOTKEY_MODIFIERS); gtk_menu_shell_append(GTK_MENU_SHELL(view_menu), s->show_menubar_item); - - return view_menu; } static void gd_create_menus(GtkDisplayState *s, DisplayOptions *opts) @@ -XXX,XX +XXX,XX @@ static void gd_create_menus(GtkDisplayState *s, DisplayOptions *opts) s->accel_group = gtk_accel_group_new(); s->machine_menu = gd_create_menu_machine(s); - s->view_menu = gd_create_menu_view(s, opts); + gd_create_menu_view(s, opts); s->machine_menu_item = gtk_menu_item_new_with_mnemonic(_("_Machine")); gtk_menu_item_set_submenu(GTK_MENU_ITEM(s->machine_menu_item), -- 2.54.0 Register a console notifier so the GTK display dynamically creates and destroys VirtualConsole tabs when graphic consoles are added or removed at runtime (e.g. vfio-pci with display=on hotplug). Add skips consoles that already have a VC binding, remove skips unknown consoles. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- include/ui/gtk.h | 1 + ui/gtk.c | 77 +++++++++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 77 insertions(+), 1 deletion(-) diff --git a/include/ui/gtk.h b/include/ui/gtk.h index XXXXXXX..XXXXXXX 100644 --- a/include/ui/gtk.h +++ b/include/ui/gtk.h @@ -XXX,XX +XXX,XX @@ struct GtkDisplayState { GdkCursor *null_cursor; Notifier mouse_mode_notifier; + Notifier console_notifier; gboolean free_scale; gboolean keep_aspect_ratio; diff --git a/ui/gtk.c b/ui/gtk.c index XXXXXXX..XXXXXXX 100644 --- a/ui/gtk.c +++ b/ui/gtk.c @@ -XXX,XX +XXX,XX @@ static bool gd_scale_valid(double scale) return scale >= VC_SCALE_MIN && scale <= VC_SCALE_MAX; } -static void add_gfx_console(GtkDisplayState *s, QemuConsole *con) +static VirtualConsole * +add_gfx_console(GtkDisplayState *s, QemuConsole *con) { VirtualConsole *vc = g_new0(VirtualConsole, 1); const DisplayChangeListenerOps *ops = &dcl_ops; @@ -XXX,XX +XXX,XX @@ static void add_gfx_console(GtkDisplayState *s, QemuConsole *con) qemu_console_register_listener(con, &vc->gfx.dcl, ops); gd_connect_vc_gfx_signals(vc); + return vc; +} + +static void gd_vc_add_gfx(GtkDisplayState *s, QemuConsole *con) +{ + VirtualConsole *vc; + int i; + + for (i = 0; i < (int)s->vcs->len; i++) { + VirtualConsole *v = g_ptr_array_index(s->vcs, i); + if (v->type == GD_VC_GFX && v->gfx.dcl.con == con) { + return; + } + } + + vc = add_gfx_console(s, con); + gtk_widget_show_all(vc->tab_item); + gtk_widget_realize(vc->gfx.drawing_area); + + if (s->free_scale) { + gd_update_windowsize(vc); + } + + gd_update_caption(s); + gd_rebuild_vc_menu(s); + + gtk_notebook_set_show_tabs(GTK_NOTEBOOK(s->notebook), true); +} + +static void gd_vc_remove_gfx(GtkDisplayState *s, QemuConsole *con) +{ + VirtualConsole *vc = NULL; + guint idx; + + for (idx = 0; idx < s->vcs->len; idx++) { + VirtualConsole *v = g_ptr_array_index(s->vcs, idx); + if (v->type == GD_VC_GFX && v->gfx.dcl.con == con) { + vc = v; + break; + } + } + if (!vc) { + return; + } + + if (s->kbd_owner == vc) { + gd_ungrab_keyboard(s); + } + if (s->ptr_owner == vc) { + gd_ungrab_pointer(s); + } + + g_ptr_array_remove_index(s->vcs, idx); + gd_rebuild_vc_menu(s); + gd_update_caption(s); +} + +static void gd_console_notify(Notifier *n, void *data) +{ + GtkDisplayState *s = container_of(n, GtkDisplayState, console_notifier); + QemuConsoleEvent *event = data; + + switch (event->type) { + case QEMU_CONSOLE_ADDED: + gd_vc_add_gfx(s, event->con); + break; + case QEMU_CONSOLE_REMOVED: + gd_vc_remove_gfx(s, event->con); + break; + } } static void gd_create_menu_view(GtkDisplayState *s, DisplayOptions *opts) @@ -XXX,XX +XXX,XX @@ static void gtk_display_init(DisplayState *ds, DisplayOptions *opts) gd_create_menus(s, opts); + s->console_notifier.notify = gd_console_notify; + qemu_console_add_notifier(&s->console_notifier); + gd_connect_signals(s); gtk_notebook_set_show_tabs(GTK_NOTEBOOK(s->notebook), FALSE); @@ -XXX,XX +XXX,XX @@ static void gtk_display_cleanup(void) return; } + qemu_console_remove_notifier(&s->console_notifier); qemu_remove_mouse_mode_change_notifier(&s->mouse_mode_notifier); gd_clipboard_cleanup(s); g_signal_handlers_disconnect_by_func(s->notebook, -- 2.54.0 Consoles created after init_displaystate() (e.g. hotplugged display devices) were never added to the /backend/console[N] QOM tree. Extract qemu_console_add_to_qom() and call it from qemu_console_register() when the display is already initialized. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- ui/console.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/ui/console.c b/ui/console.c index XXXXXXX..XXXXXXX 100644 --- a/ui/console.c +++ b/ui/console.c @@ -XXX,XX +XXX,XX @@ void qemu_text_console_put_string(QemuTextConsole *s, const char *str, int len) } } +static void qemu_console_add_to_qom(QemuConsole *con) +{ + g_autofree gchar *name = g_strdup_printf("console[%d]", con->index); + object_property_add_child(object_get_container("backend"), + name, OBJECT(con)); +} + static void qemu_console_register(QemuConsole *c) { @@ -XXX,XX +XXX,XX @@ qemu_console_register(QemuConsole *c) } } } + + if (phase_check(PHASE_MACHINE_READY)) { + qemu_console_add_to_qom(c); + } } static void @@ -XXX,XX +XXX,XX @@ void qemu_console_remove_notifier(Notifier *notifier) */ DisplayState *init_displaystate(void) { - gchar *name; QemuConsole *con; QTAILQ_FOREACH(con, &consoles, next) { /* Hook up into the qom tree here (not in object_new()), once * all QemuConsoles are created and the order / numbering * doesn't change any more */ - name = g_strdup_printf("console[%d]", con->index); - object_property_add_child(object_get_container("backend"), - name, OBJECT(con)); - g_free(name); + qemu_console_add_to_qom(con); } return display_state; -- 2.54.0 Call object_unparent() in graphic_console_close() to remove the console from /backend/console[N]. This drops the QOM tree reference, while the initial object_new() reference keeps the console alive for potential reuse. When graphic_console_init() reuses a closed console, re-register it in the QOM tree. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- ui/console.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ui/console.c b/ui/console.c index XXXXXXX..XXXXXXX 100644 --- a/ui/console.c +++ b/ui/console.c @@ -XXX,XX +XXX,XX @@ QemuConsole *qemu_graphic_console_create(DeviceState *dev, uint32_t head, trace_console_gfx_reuse(s->index); width = qemu_console_get_width(s, 0); height = qemu_console_get_height(s, 0); + if (phase_check(PHASE_MACHINE_READY)) { + qemu_console_add_to_qom(s); + } } else { trace_console_gfx_new(); s = (QemuConsole *)object_new(TYPE_QEMU_GRAPHIC_CONSOLE); @@ -XXX,XX +XXX,XX @@ void qemu_graphic_console_close(QemuConsole *con) } surface = qemu_create_placeholder_surface(width, height, unplugged); qemu_console_set_surface(con, surface); + object_unparent(OBJECT(con)); } QemuConsole *qemu_console_lookup_default(void) -- 2.54.0 Subscribe to QemuConsoleEvent notifications to dynamically add and remove D-Bus display consoles. This mirrors the GTK backend's handling added in the previous commits. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- ui/dbus.h | 3 ++ ui/dbus-console.c | 5 +++ ui/dbus.c | 93 +++++++++++++++++++++++++++++++++++++++++++++---------- 3 files changed, 84 insertions(+), 17 deletions(-) diff --git a/ui/dbus.h b/ui/dbus.h index XXXXXXX..XXXXXXX 100644 --- a/ui/dbus.h +++ b/ui/dbus.h @@ -XXX,XX +XXX,XX @@ struct DBusDisplay { DBusClipboardRequest clipboard_request[QEMU_CLIPBOARD_SELECTION__COUNT]; Notifier notifier; + Notifier console_notifier; }; #ifdef WIN32 @@ -XXX,XX +XXX,XX @@ dbus_display_console_new(DBusDisplay *display, QemuConsole *con); int dbus_display_console_get_index(DBusDisplayConsole *ddc); +QemuConsole * +dbus_display_console_get_qemu_console(DBusDisplayConsole *ddc); extern const DisplayChangeListenerOps dbus_console_dcl_ops; diff --git a/ui/dbus-console.c b/ui/dbus-console.c index XXXXXXX..XXXXXXX 100644 --- a/ui/dbus-console.c +++ b/ui/dbus-console.c @@ -XXX,XX +XXX,XX @@ int dbus_display_console_get_index(DBusDisplayConsole *ddc) return qemu_console_get_index(ddc->dcl.con); } +QemuConsole *dbus_display_console_get_qemu_console(DBusDisplayConsole *ddc) +{ + return ddc->dcl.con; +} + DBusDisplayConsole * dbus_display_console_new(DBusDisplay *display, QemuConsole *con) { diff --git a/ui/dbus.c b/ui/dbus.c index XXXXXXX..XXXXXXX 100644 --- a/ui/dbus.c +++ b/ui/dbus.c @@ -XXX,XX +XXX,XX @@ dbus_display_finalize(Object *o) { DBusDisplay *dd = DBUS_DISPLAY(o); + if (dd->console_notifier.notify) { + qemu_console_remove_notifier(&dd->console_notifier); + } if (dd->notifier.notify) { dbus_display_notifier_remove(&dd->notifier); } @@ -XXX,XX +XXX,XX @@ dbus_display_finalize(Object *o) dbus_display = NULL; } +static void +dbus_update_console_ids(DBusDisplay *dd) +{ + g_autoptr(GArray) arr = g_array_new(FALSE, FALSE, sizeof(guint32)); + + for (guint i = 0; i < dd->consoles->len; i++) { + DBusDisplayConsole *ddc = g_ptr_array_index(dd->consoles, i); + guint32 idx = dbus_display_console_get_index(ddc); + g_array_append_val(arr, idx); + } + + g_object_set(dd->iface, "console-ids", + g_variant_new_fixed_array(G_VARIANT_TYPE("u"), + arr->data, arr->len, + sizeof(guint32)), + NULL); +} + static bool -dbus_display_add_console(DBusDisplay *dd, int idx, Error **errp) +dbus_display_add_console(DBusDisplay *dd, QemuConsole *con, Error **errp) { - QemuConsole *con; DBusDisplayConsole *dbus_console; - con = qemu_console_lookup_by_index(idx); - assert(con); + for (guint i = 0; i < dd->consoles->len; i++) { + DBusDisplayConsole *ddc = g_ptr_array_index(dd->consoles, i); + if (dbus_display_console_get_qemu_console(ddc) == con) { + return true; + } + } if (qemu_console_is_graphic(con) && dd->gl_mode != DISPLAY_GL_MODE_OFF) { @@ -XXX,XX +XXX,XX @@ dbus_display_add_console(DBusDisplay *dd, int idx, Error **errp) } dbus_console = dbus_display_console_new(dd, con); - g_ptr_array_insert(dd->consoles, idx, dbus_console); + g_ptr_array_add(dd->consoles, dbus_console); g_dbus_object_manager_server_export(dd->server, G_DBUS_OBJECT_SKELETON(dbus_console)); + dbus_update_console_ids(dd); return true; } +static void +dbus_display_remove_console(DBusDisplay *dd, QemuConsole *con) +{ + for (guint i = 0; i < dd->consoles->len; i++) { + DBusDisplayConsole *ddc = g_ptr_array_index(dd->consoles, i); + if (dbus_display_console_get_qemu_console(ddc) == con) { + if (display_opengl) { + qemu_console_set_display_gl_ctx(con, NULL); + } + g_dbus_object_manager_server_unexport( + dd->server, + g_dbus_object_get_object_path(G_DBUS_OBJECT(ddc))); + g_ptr_array_remove_index(dd->consoles, i); + dbus_update_console_ids(dd); + break; + } + } +} + +static void +dbus_console_notify(Notifier *n, void *data) +{ + DBusDisplay *dd = container_of(n, DBusDisplay, console_notifier); + QemuConsoleEvent *event = data; + + switch (event->type) { + case QEMU_CONSOLE_ADDED: { + Error *err = NULL; + if (!dbus_display_add_console(dd, event->con, &err)) { + error_report_err(err); + } + break; + } + case QEMU_CONSOLE_REMOVED: + dbus_display_remove_console(dd, event->con); + break; + } +} + static void dbus_display_complete(UserCreatable *uc, Error **errp) { DBusDisplay *dd = DBUS_DISPLAY(uc); g_autoptr(GError) err = NULL; g_autofree char *uuid = qemu_uuid_unparse_strdup(&qemu_uuid); - g_autoptr(GArray) consoles = NULL; - GVariant *console_ids; int idx; if (!object_resolve_path_type("", TYPE_DBUS_DISPLAY, NULL)) { @@ -XXX,XX +XXX,XX @@ dbus_display_complete(UserCreatable *uc, Error **errp) } } - consoles = g_array_new(FALSE, FALSE, sizeof(guint32)); for (idx = 0;; idx++) { - if (!qemu_console_lookup_by_index(idx)) { + QemuConsole *con = qemu_console_lookup_by_index(idx); + if (!con) { break; } - if (!dbus_display_add_console(dd, idx, errp)) { + if (!dbus_display_add_console(dd, con, errp)) { return; } - g_array_append_val(consoles, idx); } - console_ids = g_variant_new_from_data( - G_VARIANT_TYPE("au"), - consoles->data, consoles->len * sizeof(guint32), TRUE, - (GDestroyNotify)g_array_unref, consoles); - g_steal_pointer(&consoles); g_object_set(dd->iface, "name", qemu_name ?: "QEMU " QEMU_VERSION, "uuid", uuid, - "console-ids", console_ids, NULL); + dbus_update_console_ids(dd); + + dd->console_notifier.notify = dbus_console_notify; + qemu_console_add_notifier(&dd->console_notifier); if (dd->bus) { g_dbus_object_manager_server_set_connection(dd->server, dd->bus); -- 2.54.0 Add a qtest that verifies display consoles are dynamically added and removed over D-Bus when a bochs-display device is hotplugged and unplugged on a q35 machine. The test plugs device_add a bochs-display, waits for the DEVICE_ADDED QMP event, and checks that the D-Bus VM interface reports a second console. It then device_del it, forces a system reset (q35 removal is ACPI-based and needs guest cooperation qtest cannot provide), waits for DEVICE_DELETED, and checks the console count again. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- tests/qtest/dbus-display-test.c | 101 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 99 insertions(+), 2 deletions(-) diff --git a/tests/qtest/dbus-display-test.c b/tests/qtest/dbus-display-test.c index XXXXXXX..XXXXXXX 100644 --- a/tests/qtest/dbus-display-test.c +++ b/tests/qtest/dbus-display-test.c @@ -XXX,XX +XXX,XX @@ #include <gio/gio.h> #include <gio/gunixfdlist.h> #include "libqtest.h" +#include "qobject/qdict.h" +#include "qobject/qstring.h" #include "ui/dbus-display1.h" static GDBusConnection* @@ -XXX,XX +XXX,XX @@ test_dbus_p2p_from_fd(int fd) } static void -test_setup(QTestState **qts, GDBusConnection **conn) +test_setup_args(QTestState **qts, GDBusConnection **conn, const char *args) { int pair[2]; - *qts = qtest_init("-display dbus,p2p=yes -name dbus-test"); + *qts = qtest_init(args); g_assert_cmpint(qemu_socketpair(AF_UNIX, SOCK_STREAM, 0, pair), ==, 0); @@ -XXX,XX +XXX,XX @@ test_setup(QTestState **qts, GDBusConnection **conn) g_dbus_connection_start_message_processing(*conn); } +static void +test_setup(QTestState **qts, GDBusConnection **conn) +{ + test_setup_args(qts, conn, "-display dbus,p2p=yes -name dbus-test"); +} + static void test_dbus_display_vm(void) { @@ -XXX,XX +XXX,XX @@ test_dbus_display_keyboard(void) qtest_quit(qts); } +static gsize +get_console_ids_count(GDBusConnection *conn) +{ + g_autoptr(GError) err = NULL; + g_autoptr(QemuDBusDisplay1VMProxy) vm = NULL; + GVariant *console_ids; + gsize n_ids = 0; + + vm = QEMU_DBUS_DISPLAY1_VM_PROXY( + qemu_dbus_display1_vm_proxy_new_sync( + conn, + G_DBUS_PROXY_FLAGS_NONE, + NULL, + DBUS_DISPLAY1_ROOT "/VM", + NULL, + &err)); + g_assert_no_error(err); + + console_ids = qemu_dbus_display1_vm_get_console_ids( + QEMU_DBUS_DISPLAY1_VM(vm)); + if (console_ids) { + n_ids = g_variant_n_children(console_ids); + } + return n_ids; +} + +static void +wait_device_event(QTestState *qts, const char *event_name, const char *id) +{ + QDict *resp, *data; + QString *qstr; + + for (;;) { + resp = qtest_qmp_eventwait_ref(qts, event_name); + data = qdict_get_qdict(resp, "data"); + if (!data || !qdict_get(data, "device")) { + qobject_unref(resp); + continue; + } + qstr = qobject_to(QString, qdict_get(data, "device")); + if (!strcmp(qstring_get_str(qstr), id)) { + qobject_unref(resp); + break; + } + qobject_unref(resp); + } +} + +static void +test_dbus_display_hotplug(void) +{ + g_autoptr(GDBusConnection) conn = NULL; + QTestState *qts = NULL; + gsize n; + + test_setup_args(&qts, &conn, + "-machine q35" + " -device pcie-root-port,id=rp0" + " -display dbus,p2p=yes" + " -name dbus-test"); + + n = get_console_ids_count(conn); + g_assert_cmpuint(n, ==, 1); + + qtest_qmp_device_add(qts, "bochs-display", "bochs0", + "{'bus': 'rp0'}"); + + n = get_console_ids_count(conn); + g_assert_cmpuint(n, ==, 2); + + /* + * On q35, PCI device removal is ACPI-based and requires guest + * acknowledgement. Since qtest has no guest OS, issue the delete + * request and force removal via system reset. + */ + qtest_qmp_device_del_send(qts, "bochs0"); + qtest_system_reset_nowait(qts); + wait_device_event(qts, "DEVICE_DELETED", "bochs0"); + + n = get_console_ids_count(conn); + g_assert_cmpuint(n, ==, 1); + + g_clear_object(&conn); + qtest_quit(qts); +} + int main(int argc, char **argv) { @@ -XXX,XX +XXX,XX @@ main(int argc, char **argv) qtest_add_data_func("/dbus-display/console", GINT_TO_POINTER(false), test_dbus_display_console); qtest_add_data_func("/dbus-display/console/map", GINT_TO_POINTER(true), test_dbus_display_console); qtest_add_func("/dbus-display/keyboard", test_dbus_display_keyboard); + if (qtest_has_machine("q35") && qtest_has_device("bochs-display")) { + qtest_add_func("/dbus-display/hotplug", test_dbus_display_hotplug); + } return g_test_run(); } -- 2.54.0 Hi, This series improves handling of dynamic consoles in ui/ (commit 9588d67e72 "console: minimal hotplug suport" added basic unplug & replug). It allows for better hotplug of vfio-pci display devices or bochs-display (the other device supporting hotplug today), with GTK and D-Bus backends. As usual, the first patches are various preliminary cleanups and fixes I hit while developping the rest, they could be merged earlier. I improved the display cleanup to replace the scattered atexit() handler, making leaks more visible to sanitizers. Then console ADDED/REMOVED notifications are added, so display backends can react to hotplug. The GTK code is rework a bit to allow easier hotplug events handling. Finally, D-Bus supports is implemented with a qtest to exercise it. thanks Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- Changes in v3: - added "docs: start a vfio-pci document" patch - gtk: some cleanup order fixes in gd_vc_free() - gtk: drop unwanted show-tabs on console-add - added "util: make notifer_remove() safer" - made dbus led handler removal idempotent - vfio: close dmabufs before closing console - vfio: removed bad set_display_gl_ctx(NULL) call - vfio: fixed dpy->region exit/finalize with pre-check - fix console registration after x-exit-preconfig - rebased & collected patch trailers - Link to v2: https://lore.kernel.org/qemu-devel/20260608-b4-ui-v2-0-9ea6355ea08f@redhat.com Changes in v2: - add "ui/dbus: remove led handler", suggested by Akihiko - fix compilation with -Dvte=disabled - rebased, collect trailers - Link to v1: https://lore.kernel.org/qemu-devel/20260529-b4-ui-v1-0-8db3068421ef@redhat.com To: qemu-devel@nongnu.org Cc: Marc-André Lureau <marcandre.lureau@redhat.com> Cc: Paolo Bonzini <pbonzini@redhat.com> Cc: "Daniel P. Berrangé" <berrange@redhat.com> Cc: Philippe Mathieu-Daudé <philmd@mailo.com> Cc: Pierrick Bouvier <pierrick.bouvier@oss.qualcomm.com> Cc: Fabiano Rosas <farosas@suse.de> Cc: Laurent Vivier <lvivier@redhat.com> Cc: Alex Williamson <alex@shazbot.org> Cc: Cédric Le Goater <clg@redhat.com> Cc: Peter Maydell <peter.maydell@linaro.org> Cc: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp> --- Marc-André Lureau (35): ui/gtk: fix bad widget realize on non-GFX VC build-sys: build with -fno-omit-frame-pointer with ASAN irq: add per-IRQ observer to fix qemu_irq_intercept_in leak scripts/lsan_suppressions: suppress fontconfig leaks vfio/pci: close display console during unrealize, not finalize docs: start a vfio-pci document glib-compat: add fallback for g_clear_fd/g_autofd util: make notifer_remove() safer ui/dbus: remove mouse handler on dispose ui/dbus: remove led handler ui/qmp: keep a reference of console across yield ui: stop ui timer when closing ui/console: init gl_unblock_timer in qemu_console_init ui/spice: remove dead spice_displays ui/spice: add cleanup on shutdown ui: add display cleanup infrastructure ui/curses: implement display cleanup ui/sdl2: implement display cleanup ui/spice-app: implement display cleanup ui/egl: implement display and EGL cleanup ui/cocoa: implement display cleanup ui/dbus: implement display cleanup ui/gtk: implement display cleanup ui/console: add console event notifier infrastructure ui/console: fire console ADDED/REMOVED notifications ui/console-vc: fire ADDED/REMOVED notifications ui/gtk: convert VirtualConsole storage from fixed array to GPtrArray ui/gtk: move global display settings out of per-console init ui/gtk: fix tab re-insertion order on window close ui/gtk: centralize console menu and shortcut management ui/gtk: handle console hotplug/unplug events ui/console: register console in QOM tree dynamically ui/console: unregister console from QOM tree on close ui/dbus: handle console hotplug/unplug events tests/qtest: add D-Bus display hotplug test MAINTAINERS | 1 + docs/devel/index-internals.rst | 1 + docs/devel/vfio-mdpy.rst | 83 +++++++++ meson.build | 2 + hw/vfio/pci.h | 1 + include/glib-compat.h | 29 ++++ include/hw/core/irq.h | 6 +- include/ui/console.h | 20 +++ include/ui/egl-helpers.h | 1 + include/ui/gtk.h | 7 +- include/ui/qemu-spice-module.h | 1 + include/ui/qemu-spice.h | 9 +- ui/dbus.h | 3 + hw/core/irq.c | 12 +- hw/vfio/display.c | 29 ++-- hw/vfio/pci.c | 1 + system/qtest.c | 5 +- system/runstate.c | 4 +- tests/qtest/dbus-display-test.c | 101 ++++++++++- ui/console-vc.c | 13 ++ ui/console.c | 77 ++++++++- ui/curses.c | 17 +- ui/dbus-console.c | 14 +- ui/dbus.c | 106 ++++++++++-- ui/egl-headless.c | 31 ++++ ui/egl-helpers.c | 19 ++ ui/gtk-clipboard.c | 15 ++ ui/gtk.c | 375 +++++++++++++++++++++++++++++++--------- ui/sdl2.c | 23 ++- ui/spice-app.c | 10 +- ui/spice-core.c | 25 ++- ui/spice-display.c | 52 ++++++ ui/spice-input.c | 52 ++++-- ui/spice-module.c | 5 + ui/ui-qmp-cmds.c | 3 + util/notify.c | 2 +- scripts/lsan_suppressions.txt | 15 +- ui/cocoa.m | 19 +- ui/meson.build | 4 +- 39 files changed, 1004 insertions(+), 189 deletions(-) --- base-commit: de5d8bfd6105d3dd3ae668df9762df244a6d1506 change-id: 20260529-b4-ui-909bfa695735 Best regards, -- Marc-André Lureau <marcandre.lureau@redhat.com> The GTK VirtualConsole is a union, it may be .gfx or .vte depending on the type. Fixes: 565f85a9c2 ("ui/gtk: force realization of drawing area") Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- ui/gtk.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ui/gtk.c b/ui/gtk.c index XXXXXXX..XXXXXXX 100644 --- a/ui/gtk.c +++ b/ui/gtk.c @@ -XXX,XX +XXX,XX @@ static void gtk_display_init(DisplayState *ds, DisplayOptions *opts) if (!con) { break; } - gtk_widget_realize(s->vc[idx].gfx.drawing_area); + if (s->vc[idx].type == GD_VC_GFX) { + gtk_widget_realize(s->vc[idx].gfx.drawing_area); + } } if (opts->u.gtk.has_show_menubar && -- 2.54.0 On fc44, LSan fails to suppress leak:qemu_irq_intercept_in, because the backtrace isn't deep enough. Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- meson.build | 2 ++ 1 file changed, 2 insertions(+) diff --git a/meson.build b/meson.build index XXXXXXX..XXXXXXX 100644 --- a/meson.build +++ b/meson.build @@ -XXX,XX +XXX,XX @@ if get_option('asan') if cc.has_argument('-fsanitize=address') qemu_cflags = ['-fsanitize=address'] + qemu_cflags qemu_ldflags = ['-fsanitize=address'] + qemu_ldflags + # Ensure complete stack traces for LSan suppressions to match correctly. + qemu_cflags += ['-fno-omit-frame-pointer'] else error('Your compiler does not support -fsanitize=address') endif -- 2.54.0 qemu_irq_intercept_in() saves original IRQ handlers by allocating new QOM objects, which are never freed. On a PC machine, this leaks IRQ objects (one per IOAPIC pin) on every qtest run. Rather than tracking allocations to free later, avoid them: add an "observer" field to IRQState, called by qemu_set_irq() after the real handler. Interception sets the observer instead of rewriting handlers, so there's nothing to save and nothing to leak. Fix qemu_notirq() to route through qemu_set_irq() so inverted IRQs trigger observers too. Drop the LSan suppression. Reviewed-by: Fabiano Rosas <farosas@suse.de> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- include/hw/core/irq.h | 6 +++--- hw/core/irq.c | 12 ++++++------ system/qtest.c | 5 +---- scripts/lsan_suppressions.txt | 8 -------- 4 files changed, 10 insertions(+), 21 deletions(-) diff --git a/include/hw/core/irq.h b/include/hw/core/irq.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/core/irq.h +++ b/include/hw/core/irq.h @@ -XXX,XX +XXX,XX @@ struct IRQState { qemu_irq_handler handler; void *opaque; int n; + qemu_irq_handler observer; }; void qemu_set_irq(qemu_irq irq, int level); @@ -XXX,XX +XXX,XX @@ void qemu_free_irq(qemu_irq irq); /* Returns a new IRQ with opposite polarity. */ qemu_irq qemu_irq_invert(qemu_irq irq); -/* For internal use in qtest. Similar to qemu_irq_split, but operating - on an existing vector of qemu_irq. */ -void qemu_irq_intercept_in(qemu_irq *gpio_in, qemu_irq_handler handler, int n); +/* For internal use in qtest. */ +void qemu_irq_set_observer(qemu_irq *gpio_in, qemu_irq_handler handler, int n); /** * qemu_irq_is_connected: Return true if IRQ line is wired up diff --git a/hw/core/irq.c b/hw/core/irq.c index XXXXXXX..XXXXXXX 100644 --- a/hw/core/irq.c +++ b/hw/core/irq.c @@ -XXX,XX +XXX,XX @@ void qemu_set_irq(qemu_irq irq, int level) return; irq->handler(irq->opaque, irq->n, level); + if (unlikely(irq->observer)) { + irq->observer(irq->opaque, irq->n, level); + } } static void init_irq_fields(IRQState *irq, qemu_irq_handler handler, @@ -XXX,XX +XXX,XX @@ static void qemu_notirq(void *opaque, int line, int level) { IRQState *irq = opaque; - irq->handler(irq->opaque, irq->n, !level); + qemu_set_irq(irq, !level); } qemu_irq qemu_irq_invert(qemu_irq irq) @@ -XXX,XX +XXX,XX @@ qemu_irq qemu_irq_invert(qemu_irq irq) return qemu_allocate_irq(qemu_notirq, irq, 0); } -void qemu_irq_intercept_in(qemu_irq *gpio_in, qemu_irq_handler handler, int n) +void qemu_irq_set_observer(qemu_irq *gpio_in, qemu_irq_handler handler, int n) { int i; - qemu_irq *old_irqs = qemu_allocate_irqs(NULL, NULL, n); for (i = 0; i < n; i++) { - *old_irqs[i] = *gpio_in[i]; - gpio_in[i]->handler = handler; - gpio_in[i]->opaque = &old_irqs[i]; + gpio_in[i]->observer = handler; } } diff --git a/system/qtest.c b/system/qtest.c index XXXXXXX..XXXXXXX 100644 --- a/system/qtest.c +++ b/system/qtest.c @@ -XXX,XX +XXX,XX @@ void qtest_sendf(CharFrontend *chr, const char *fmt, ...) static void qtest_irq_handler(void *opaque, int n, int level) { - qemu_irq old_irq = *(qemu_irq *)opaque; - qemu_set_irq(old_irq, level); - if (irq_levels[n] != level) { CharFrontend *chr = &qtest->qtest_chr; irq_levels[n] = level; @@ -XXX,XX +XXX,XX @@ static void qtest_process_command(CharFrontend *chr, gchar **words) interception_succeeded = true; } } else { - qemu_irq_intercept_in(ngl->in, qtest_irq_handler, + qemu_irq_set_observer(ngl->in, qtest_irq_handler, ngl->num_in); interception_succeeded = true; } diff --git a/scripts/lsan_suppressions.txt b/scripts/lsan_suppressions.txt index XXXXXXX..XXXXXXX 100644 --- a/scripts/lsan_suppressions.txt +++ b/scripts/lsan_suppressions.txt @@ -XXX,XX +XXX,XX @@ leak:/lib64/libxkbcommon.so.0 # https://github.com/GNOME/glib/blob/main/tools/glib.supp # This avoids false positive leak reports for the qga-ssh-test. leak:g_set_user_dirs - -# qemu_irq_intercept_in is only used by the qtest harness, and -# its API inherently involves a leak. -# While we could keep track of the old IRQ data structure -# in order to free it, it doesn't seem very important to fix -# since it is only used by the qtest test harness. -# Just ignore the leak, at least for the moment. -leak:qemu_irq_intercept_in -- 2.54.0 Those are annoying reports for gtk/sdl etc. Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- scripts/lsan_suppressions.txt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/scripts/lsan_suppressions.txt b/scripts/lsan_suppressions.txt index XXXXXXX..XXXXXXX 100644 --- a/scripts/lsan_suppressions.txt +++ b/scripts/lsan_suppressions.txt @@ -XXX,XX +XXX,XX @@ leak:/lib64/libtcmalloc_minimal.so.4 # libxkbcommon also leaks in qemu-keymap leak:/lib64/libxkbcommon.so.0 +# libfontconfig leaks are notorious, for ex +# https://gitlab.freedesktop.org/fontconfig/fontconfig/-/work_items/519 +leak:libfontconfig.so + # g_set_user_dirs() deliberately leaks the previous cached g_get_user_* # values. This is documented in upstream glib's valgrind-format # suppression file: -- 2.54.0 The QemuGraphicConsole holds a strong QOM link back to the device via its "device" property (OBJ_PROP_LINK_STRONG). When graphic_console_close() is only called from vfio_display_finalize() during object finalize, this creates a ref-cycle deadlock: the device can't reach refcount 0 because the console holds a strong ref, but the console's ref is only dropped by graphic_console_close() which runs inside finalize. Split the display teardown into two phases: - vfio_display_exit(): called during unrealize (vfio_exitfn), closes the graphic console to break the ref cycle, and removes display region subregions while the parent memory regions are still alive. - vfio_display_finalize(): remains in finalize (vfio_pci_put_device), frees display region memory, dmabuf, and edid resources. The region memory contains QOM child objects (MemoryRegions) that must stay alive until QOM finalization has processed them. Acked-by: Cédric Le Goater <clg@redhat.com> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- MAINTAINERS | 1 + hw/vfio/pci.h | 1 + hw/vfio/display.c | 29 +++++++++++++++++------------ hw/vfio/pci.c | 1 + 4 files changed, 20 insertions(+), 12 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index XXXXXXX..XXXXXXX 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -XXX,XX +XXX,XX @@ S: Supported F: hw/vfio/* F: util/vfio-helpers.c F: include/hw/vfio/ +F: docs/devel/vfio-mdpy.rst F: docs/devel/migration/vfio.rst F: qapi/vfio.json F: migration/vfio-stub.c diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h index XXXXXXX..XXXXXXX 100644 --- a/hw/vfio/pci.h +++ b/hw/vfio/pci.h @@ -XXX,XX +XXX,XX @@ bool vfio_populate_vga(VFIOPCIDevice *vdev, Error **errp); void vfio_display_reset(VFIOPCIDevice *vdev); bool vfio_display_probe(VFIOPCIDevice *vdev, Error **errp); +void vfio_display_exit(VFIOPCIDevice *vdev); void vfio_display_finalize(VFIOPCIDevice *vdev); extern const VMStateDescription vfio_display_vmstate; diff --git a/hw/vfio/display.c b/hw/vfio/display.c index XXXXXXX..XXXXXXX 100644 --- a/hw/vfio/display.c +++ b/hw/vfio/display.c @@ -XXX,XX +XXX,XX @@ static bool vfio_display_region_init(VFIOPCIDevice *vdev, Error **errp) return true; } -static void vfio_display_region_exit(VFIODisplay *dpy) -{ - if (!dpy->region.buffer.size) { - return; - } - - vfio_region_exit(&dpy->region.buffer); - vfio_region_finalize(&dpy->region.buffer); -} /* ---------------------------------------------------------------------- */ @@ -XXX,XX +XXX,XX @@ bool vfio_display_probe(VFIOPCIDevice *vdev, Error **errp) return false; } -void vfio_display_finalize(VFIOPCIDevice *vdev) +void vfio_display_exit(VFIOPCIDevice *vdev) { if (!vdev->dpy) { return; } - qemu_graphic_console_close(vdev->dpy->con); vfio_display_dmabuf_exit(vdev->dpy); - vfio_display_region_exit(vdev->dpy); + qemu_graphic_console_close(vdev->dpy->con); + if (vdev->dpy->region.buffer.size) { + vfio_region_exit(&vdev->dpy->region.buffer); + } +} + +void vfio_display_finalize(VFIOPCIDevice *vdev) +{ + if (!vdev->dpy) { + return; + } + + if (vdev->dpy->region.buffer.size) { + vfio_region_finalize(&vdev->dpy->region.buffer); + } vfio_display_edid_exit(vdev->dpy); g_free(vdev->dpy); + vdev->dpy = NULL; } static bool migrate_needed(void *opaque) diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c index XXXXXXX..XXXXXXX 100644 --- a/hw/vfio/pci.c +++ b/hw/vfio/pci.c @@ -XXX,XX +XXX,XX @@ static void vfio_exitfn(PCIDevice *pdev) VFIOPCIDevice *vdev = VFIO_PCI_DEVICE(pdev); VFIODevice *vbasedev = &vdev->vbasedev; + vfio_display_exit(vdev); vfio_unregister_req_notifier(vdev); vfio_unregister_err_notifier(vdev); pci_device_set_intx_routing_notifier(pdev, NULL); -- 2.54.0 Add a section to help me remember how to test vfio display hotplug. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- docs/devel/index-internals.rst | 1 + docs/devel/vfio-mdpy.rst | 83 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 84 insertions(+) diff --git a/docs/devel/index-internals.rst b/docs/devel/index-internals.rst index XXXXXXX..XXXXXXX 100644 --- a/docs/devel/index-internals.rst +++ b/docs/devel/index-internals.rst @@ -XXX,XX +XXX,XX @@ Details about QEMU's various subsystems including how to add features to them. tracing uefi-vars vfio-iommufd + vfio-mdpy writing-monitor-commands virtio-backends crypto diff --git a/docs/devel/vfio-mdpy.rst b/docs/devel/vfio-mdpy.rst new file mode 100644 index XXXXXXX..XXXXXXX --- /dev/null +++ b/docs/devel/vfio-mdpy.rst @@ -XXX,XX +XXX,XX @@ +.. SPDX-License-Identifier: GPL-2.0-or-later + +=================================== +Testing VFIO display with mdev mdpy +=================================== + +.. contents:: Table of Contents + +The kernel provides a sample mediated device driver, ``mdpy`` +(``samples/vfio-mdev/mdpy.c``), that exposes a fake framebuffer through the VFIO +display region interface. It can be used to test VFIO display support, including +hotplug, without any real GPU hardware. + +The kernel modules +================== + +The ``mdpy`` driver depends on the ``mdev`` subsystem. Enable, build and load +the modules. + +Verify that the driver registered successfully: + +.. code-block:: bash + + ls /sys/devices/virtual/mdpy/mdpy/mdev_supported_types/ + +Creating an mdev instance +========================= + +Available types correspond to different resolutions (e.g. ``mdpy-vga`` +for 640x480, ``mdpy-xga`` for 1024x768, ``mdpy-hd`` for 1920x1080). + +Each mdev instance is identified by a UUID: + +.. code-block:: bash + + uuid=$(uuidgen) + echo "$uuid" > /sys/devices/virtual/mdpy/mdpy/mdev_supported_types/mdpy-xga/create + +To remove the instance later: + +.. code-block:: bash + + echo 1 > /sys/bus/mdev/devices/$uuid/remove + +Make sure your user has the necessary permissions to access the vfio group. +(ex: chmod 666 /dev/vfio/16) + +Starting QEMU +============= + +Boot-time attachment +-------------------- + +.. code-block:: bash + + qemu-system-x86_64 -machine q35 -m 1G \ + -device vfio-pci,sysfsdev=/sys/bus/mdev/devices/$uuid,display=on \ + -display gtk,gl=on + +Hotplug via HMP +--------------- + +Start QEMU with a PCIe root port (required for PCIe hotplug) and a +monitor: + +.. code-block:: bash + + qemu-system-x86_64 -machine q35 -m 1G \ + -device pcie-root-port,id=rp0,slot=1 \ + -display gtk,gl=on \ + -monitor stdio + +Then at the ``(qemu)`` prompt: + +.. code-block:: none + + device_add vfio-pci,sysfsdev=/sys/bus/mdev/devices/<uuid>,display=on,bus=rp0,id=mdpy0 + +To hot-unplug: + +.. code-block:: none + + device_del mdpy0 -- 2.54.0 Those helpers were added in glib 2.76. Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- include/glib-compat.h | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/include/glib-compat.h b/include/glib-compat.h index XXXXXXX..XXXXXXX 100644 --- a/include/glib-compat.h +++ b/include/glib-compat.h @@ -XXX,XX +XXX,XX @@ #pragma GCC diagnostic ignored "-Wdeprecated-declarations" #include <glib.h> +#include <glib/gstdio.h> #if defined(G_OS_UNIX) #include <glib-unix.h> #include <sys/types.h> @@ -XXX,XX +XXX,XX @@ qemu_g_test_slow(void) #define g_test_thorough() qemu_g_test_slow() #define g_test_quick() (!qemu_g_test_slow()) +static inline gboolean g_clear_fd_qemu(int *fd_ptr, GError **error) +{ +#if GLIB_CHECK_VERSION(2, 76, 0) + return g_clear_fd(fd_ptr, error); +#else + int fd = *fd_ptr; + + *fd_ptr = -1; + + if (fd < 0) { + return TRUE; + } + + return g_close(fd, error); +#endif +} +#define g_clear_fd(fd, err) g_clear_fd_qemu(fd, err) + +#if !GLIB_CHECK_VERSION(2, 76, 0) +static inline void _g_clear_fd_ignore_error(int *fd_ptr) +{ + int errsv = errno; + g_clear_fd(fd_ptr, NULL); + errno = errsv; +} +#define g_autofd __attribute__((cleanup(_g_clear_fd_ignore_error))) +#endif + #pragma GCC diagnostic pop #ifndef G_NORETURN -- 2.54.0 Allow to call multiple time notifier_remove() safely. This shoudn't impact performance in any measurable way... Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- util/notify.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/util/notify.c b/util/notify.c index XXXXXXX..XXXXXXX 100644 --- a/util/notify.c +++ b/util/notify.c @@ -XXX,XX +XXX,XX @@ void notifier_list_add(NotifierList *list, Notifier *notifier) void notifier_remove(Notifier *notifier) { - QLIST_REMOVE(notifier, node); + QLIST_SAFE_REMOVE(notifier, node); } void notifier_list_notify(NotifierList *list, void *data) -- 2.54.0 Fixes: 142ca628a7 ("ui: add a D-Bus display backend") Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- ui/dbus-console.c | 1 + 1 file changed, 1 insertion(+) diff --git a/ui/dbus-console.c b/ui/dbus-console.c index XXXXXXX..XXXXXXX 100644 --- a/ui/dbus-console.c +++ b/ui/dbus-console.c @@ -XXX,XX +XXX,XX @@ dbus_display_console_dispose(GObject *object) DBusDisplayConsole *ddc = DBUS_DISPLAY_CONSOLE(object); qemu_console_unregister_listener(&ddc->dcl); + qemu_remove_mouse_mode_change_notifier(&ddc->mouse_mode_notifier); g_clear_object(&ddc->iface_touch); g_clear_object(&ddc->iface_mouse); g_clear_object(&ddc->iface_kbd); -- 2.54.0 Reported-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- ui/dbus-console.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/ui/dbus-console.c b/ui/dbus-console.c index XXXXXXX..XXXXXXX 100644 --- a/ui/dbus-console.c +++ b/ui/dbus-console.c @@ -XXX,XX +XXX,XX @@ struct _DBusDisplayConsole { guint last_x; guint last_y; Notifier mouse_mode_notifier; + QEMUPutLEDEntry *kbd_leds_updated; }; G_DEFINE_TYPE(DBusDisplayConsole, @@ -XXX,XX +XXX,XX @@ dbus_display_console_dispose(GObject *object) qemu_console_unregister_listener(&ddc->dcl); qemu_remove_mouse_mode_change_notifier(&ddc->mouse_mode_notifier); + g_clear_pointer(&ddc->kbd_leds_updated, qemu_remove_led_event_handler); g_clear_object(&ddc->iface_touch); g_clear_object(&ddc->iface_mouse); g_clear_object(&ddc->iface_kbd); @@ -XXX,XX +XXX,XX @@ dbus_display_console_new(DBusDisplay *display, QemuConsole *con) ddc->kbd = qkbd_state_init(con); ddc->iface_kbd = qemu_dbus_display1_keyboard_skeleton_new(); - qemu_add_led_event_handler(dbus_kbd_qemu_leds_updated, ddc); + ddc->kbd_leds_updated = qemu_add_led_event_handler( + dbus_kbd_qemu_leds_updated, ddc); g_object_connect(ddc->iface_kbd, "swapped-signal::handle-press", dbus_kbd_press, ddc, "swapped-signal::handle-release", dbus_kbd_release, ddc, -- 2.54.0 While the coroutine is waiting, the console could be finalized. Keep a reference to prevent this. Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- ui/ui-qmp-cmds.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ui/ui-qmp-cmds.c b/ui/ui-qmp-cmds.c index XXXXXXX..XXXXXXX 100644 --- a/ui/ui-qmp-cmds.c +++ b/ui/ui-qmp-cmds.c @@ -XXX,XX +XXX,XX @@ qmp_screendump(const char *filename, const char *device, } } + object_ref(con); qemu_console_co_wait_update(con); /* @@ -XXX,XX +XXX,XX @@ qmp_screendump(const char *filename, const char *device, surface = qemu_console_surface(con); if (!surface) { error_setg(errp, "no surface"); + object_unref(con); return; } image = pixman_image_ref(surface->image); + object_unref(con); fd = qemu_create(filename, O_WRONLY | O_TRUNC | O_BINARY, 0666, errp); if (fd == -1) { -- 2.54.0 hwops is reset, so if the UI timer is pending it will crash. Fixes: 9588d67e72 ("console: minimal hotplug suport") Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- ui/console.c | 1 + 1 file changed, 1 insertion(+) diff --git a/ui/console.c b/ui/console.c index XXXXXXX..XXXXXXX 100644 --- a/ui/console.c +++ b/ui/console.c @@ -XXX,XX +XXX,XX @@ void qemu_graphic_console_close(QemuConsole *con) trace_console_gfx_close(con->index); object_property_set_link(OBJECT(con), "device", NULL, &error_abort); qemu_graphic_console_set_hwops(con, &unused_ops, NULL); + timer_del(con->ui_timer); if (con->gl) { qemu_console_gl_scanout_disable(con); -- 2.54.0 Move gl_unblock_timer allocation from graphic_console_init() to qemu_console_init(), similar to what was done in commit cfde05d15b ("ui/console: allocate ui_timer in QemuConsole"). This fixes leaking timers on console recycling. Fixes: a9b1e471e17 ("ui: add a gl-unblock warning timer") Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- ui/console.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ui/console.c b/ui/console.c index XXXXXXX..XXXXXXX 100644 --- a/ui/console.c +++ b/ui/console.c @@ -XXX,XX +XXX,XX @@ qemu_console_init(Object *obj) c->window_id = -1; c->ui_timer = timer_new_ms(QEMU_CLOCK_REALTIME, dpy_set_ui_info_timer, c); + c->gl_unblock_timer = timer_new_ms(QEMU_CLOCK_REALTIME, + console_hw_gl_unblock_timer, c); qemu_console_register(c); } @@ -XXX,XX +XXX,XX @@ QemuConsole *qemu_graphic_console_create(DeviceState *dev, uint32_t head, surface = qemu_create_placeholder_surface(width, height, noinit); qemu_console_set_surface(s, surface); - s->gl_unblock_timer = timer_new_ms(QEMU_CLOCK_REALTIME, - console_hw_gl_unblock_timer, s); return s; } -- 2.54.0 This is left-over from commit 9fa032866da ("spice: fix multihead support") Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- include/ui/qemu-spice.h | 7 ------- 1 file changed, 7 deletions(-) diff --git a/include/ui/qemu-spice.h b/include/ui/qemu-spice.h index XXXXXXX..XXXXXXX 100644 --- a/include/ui/qemu-spice.h +++ b/include/ui/qemu-spice.h @@ -XXX,XX +XXX,XX @@ bool qemu_spice_have_display_interface(QemuConsole *con); int qemu_spice_add_display_interface(QXLInstance *qxlin, QemuConsole *con); int qemu_spice_migrate_info(const char *hostname, int port, int tls_port, const char *subject); - -#else /* CONFIG_SPICE */ - -#include "qemu/error-report.h" - -#define spice_displays 0 - #endif /* CONFIG_SPICE */ static inline bool qemu_using_spice(Error **errp) -- 2.54.0 SPICE resources were never freed on shutdown. Add per-subsystem cleanup (display, input, core) and call it from qemu_cleanup(). Move spice-module.c into libui so the qemu_spice ops table links with the rest of the UI code. Add an LSan suppression for a known spice-server leak. Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- include/ui/qemu-spice-module.h | 1 + include/ui/qemu-spice.h | 2 ++ system/runstate.c | 4 ++++ ui/spice-core.c | 25 ++++++++++++++++++-- ui/spice-display.c | 52 ++++++++++++++++++++++++++++++++++++++++++ ui/spice-input.c | 52 ++++++++++++++++++++++++++++-------------- ui/spice-module.c | 5 ++++ scripts/lsan_suppressions.txt | 5 ++++ ui/meson.build | 4 ++-- 9 files changed, 129 insertions(+), 21 deletions(-) diff --git a/include/ui/qemu-spice-module.h b/include/ui/qemu-spice-module.h index XXXXXXX..XXXXXXX 100644 --- a/include/ui/qemu-spice-module.h +++ b/include/ui/qemu-spice-module.h @@ -XXX,XX +XXX,XX @@ typedef struct SpiceInfo SpiceInfo; struct QemuSpiceOps { void (*init)(void); + void (*cleanup)(void); void (*display_init)(void); int (*migrate_info)(const char *h, int p, int t, const char *s); int (*set_passwd)(const char *passwd, diff --git a/include/ui/qemu-spice.h b/include/ui/qemu-spice.h index XXXXXXX..XXXXXXX 100644 --- a/include/ui/qemu-spice.h +++ b/include/ui/qemu-spice.h @@ -XXX,XX +XXX,XX @@ #include "qemu/config-file.h" void qemu_spice_input_init(void); +void qemu_spice_input_cleanup(void); void qemu_spice_display_init(void); +void qemu_spice_display_cleanup(void); void qemu_spice_display_init_done(void); bool qemu_spice_have_display_interface(QemuConsole *con); int qemu_spice_add_display_interface(QXLInstance *qxlin, QemuConsole *con); diff --git a/system/runstate.c b/system/runstate.c index XXXXXXX..XXXXXXX 100644 --- a/system/runstate.c +++ b/system/runstate.c @@ -XXX,XX +XXX,XX @@ #include "system/system.h" #include "system/tpm.h" #include "ui/console.h" +#include "ui/qemu-spice-module.h" #include "trace.h" @@ -XXX,XX +XXX,XX @@ void qemu_cleanup(int status) user_creatable_cleanup(); #ifdef CONFIG_VNC vnc_cleanup(); +#endif +#ifdef CONFIG_SPICE + qemu_spice.cleanup(); #endif /* TODO: unref root container, check all devices are ok */ } diff --git a/ui/spice-core.c b/ui/spice-core.c index XXXXXXX..XXXXXXX 100644 --- a/ui/spice-core.c +++ b/ui/spice-core.c @@ -XXX,XX +XXX,XX @@ static void vm_change_state_handler(void *opaque, bool running, } } +static VMChangeStateEntry *vm_change_entry; + void qemu_spice_display_init_done(void) { if (runstate_is_running()) { qemu_spice_display_start(); } - qemu_add_vm_change_state_handler(vm_change_state_handler, NULL); + vm_change_entry = + qemu_add_vm_change_state_handler(vm_change_state_handler, NULL); } static void qemu_spice_init(void) @@ -XXX,XX +XXX,XX @@ static int qemu_spice_add_interface(SpiceBaseInstance *sin) spice_server = spice_server_new(); spice_server_set_sasl_appname(spice_server, "qemu"); spice_server_init(spice_server, &core_interface); - qemu_add_vm_change_state_handler(vm_change_state_handler, NULL); + vm_change_entry = + qemu_add_vm_change_state_handler(vm_change_state_handler, NULL); } return spice_server_add_interface(spice_server, sin); @@ -XXX,XX +XXX,XX @@ int qemu_spice_display_is_running(SimpleSpiceDisplay *ssd) return spice_display_is_running; } +static void qemu_spice_cleanup(void) +{ + if (!spice_server) { + return; + } + + qemu_spice_display_cleanup(); + qemu_spice_input_cleanup(); + migration_remove_notifier(&migration_state); + g_clear_pointer(&spice_consoles, g_slist_free); + g_clear_pointer(&auth_passwd, g_free); + g_clear_pointer(&spice_server, spice_server_destroy); + g_clear_pointer(&vm_change_entry, qemu_del_vm_change_state_handler); + using_spice = 0; +} + static struct QemuSpiceOps real_spice_ops = { .init = qemu_spice_init, + .cleanup = qemu_spice_cleanup, .display_init = qemu_spice_display_init, .migrate_info = qemu_spice_migrate_info, .set_passwd = qemu_spice_set_passwd, diff --git a/ui/spice-display.c b/ui/spice-display.c index XXXXXXX..XXXXXXX 100644 --- a/ui/spice-display.c +++ b/ui/spice-display.c @@ -XXX,XX +XXX,XX @@ bool spice_opengl; bool spice_remote_client; int spice_max_refresh_rate; +static GPtrArray *spice_displays; + int qemu_spice_rect_is_empty(const QXLRect* r) { return r->top == r->bottom || r->left == r->right; @@ -XXX,XX +XXX,XX @@ static void qemu_spice_display_init_one(QemuConsole *con) qemu_console_set_display_gl_ctx(con, &ssd->dgc); } qemu_console_register_listener(con, &ssd->dcl, ops); + g_ptr_array_add(spice_displays, ssd); +} + +void qemu_spice_display_cleanup(void) +{ + if (!spice_displays) { + return; + } + + for (guint i = 0; i < spice_displays->len; i++) { + SimpleSpiceDisplay *ssd = g_ptr_array_index(spice_displays, i); + SimpleSpiceUpdate *update; + + qemu_console_unregister_listener(&ssd->dcl); +#ifdef HAVE_SPICE_GL + if (spice_opengl) { + qemu_console_set_display_gl_ctx(ssd->dcl.con, NULL); + } +#endif + + if (ssd->ds) { + qemu_spice_destroy_host_primary(ssd); + } + qemu_spice_del_memslot(ssd, MEMSLOT_GROUP_HOST, 0); + spice_server_remove_interface(&ssd->qxl.base); + + while ((update = QTAILQ_FIRST(&ssd->updates)) != NULL) { + QTAILQ_REMOVE(&ssd->updates, update, next); + qemu_spice_destroy_update(ssd, update); + } + g_clear_pointer(&ssd->ptr_define, g_free); + g_clear_pointer(&ssd->ptr_move, g_free); + g_clear_pointer(&ssd->cursor, cursor_unref); + g_clear_pointer(&ssd->surface, pixman_image_unref); + g_clear_pointer(&ssd->mirror, pixman_image_unref); + g_clear_pointer(&ssd->buf, g_free); +#ifdef HAVE_SPICE_GL + g_clear_pointer(&ssd->gl_unblock_bh, qemu_bh_delete); + g_clear_pointer(&ssd->gl_unblock_timer, timer_free); + g_clear_pointer(&ssd->gls, qemu_gl_fini_shader); + egl_fb_destroy(&ssd->guest_fb); + egl_fb_destroy(&ssd->blit_fb); + egl_fb_destroy(&ssd->cursor_fb); +#endif + qemu_mutex_destroy(&ssd->lock); + g_free(ssd); + } + g_clear_pointer(&spice_displays, g_ptr_array_unref); } void qemu_spice_display_init(void) @@ -XXX,XX +XXX,XX @@ void qemu_spice_display_init(void) const char *str; int i; + spice_displays = g_ptr_array_new(); + str = qemu_opt_get(opts, "display"); if (str) { int head = qemu_opt_get_number(opts, "head", 0); diff --git a/ui/spice-input.c b/ui/spice-input.c index XXXXXXX..XXXXXXX 100644 --- a/ui/spice-input.c +++ b/ui/spice-input.c @@ -XXX,XX +XXX,XX @@ static void mouse_mode_notifier(Notifier *notifier, void *data) pointer->absolute = is_absolute; } +static QemuSpiceKbd *spice_kbd; +static QemuSpicePointer *spice_pointer; +static QEMUPutLEDEntry *spice_led; + void qemu_spice_input_init(void) { - QemuSpiceKbd *kbd; - QemuSpicePointer *pointer; - - kbd = g_malloc0(sizeof(*kbd)); - kbd->sin.base.sif = &kbd_interface.base; - qemu_spice.add_interface(&kbd->sin.base); - qemu_add_led_event_handler(kbd_leds, kbd); - - pointer = g_malloc0(sizeof(*pointer)); - pointer->mouse.base.sif = &mouse_interface.base; - pointer->tablet.base.sif = &tablet_interface.base; - qemu_spice.add_interface(&pointer->mouse.base); - - pointer->absolute = false; - pointer->mouse_mode.notify = mouse_mode_notifier; - qemu_add_mouse_mode_change_notifier(&pointer->mouse_mode); - mouse_mode_notifier(&pointer->mouse_mode, NULL); + spice_kbd = g_new0(QemuSpiceKbd, 1); + spice_kbd->sin.base.sif = &kbd_interface.base; + qemu_spice.add_interface(&spice_kbd->sin.base); + spice_led = qemu_add_led_event_handler(kbd_leds, spice_kbd); + + spice_pointer = g_new0(QemuSpicePointer, 1); + spice_pointer->mouse.base.sif = &mouse_interface.base; + spice_pointer->tablet.base.sif = &tablet_interface.base; + qemu_spice.add_interface(&spice_pointer->mouse.base); + + spice_pointer->absolute = false; + spice_pointer->mouse_mode.notify = mouse_mode_notifier; + qemu_add_mouse_mode_change_notifier(&spice_pointer->mouse_mode); + mouse_mode_notifier(&spice_pointer->mouse_mode, NULL); +} + +void qemu_spice_input_cleanup(void) +{ + g_clear_pointer(&spice_led, qemu_remove_led_event_handler); + if (spice_pointer) { + qemu_remove_mouse_mode_change_notifier(&spice_pointer->mouse_mode); + if (spice_pointer->absolute) { + spice_server_remove_interface(&spice_pointer->tablet.base); + } + spice_server_remove_interface(&spice_pointer->mouse.base); + g_clear_pointer(&spice_pointer, g_free); + } + if (spice_kbd) { + spice_server_remove_interface(&spice_kbd->sin.base); + g_clear_pointer(&spice_kbd, g_free); + } } diff --git a/ui/spice-module.c b/ui/spice-module.c index XXXXXXX..XXXXXXX 100644 --- a/ui/spice-module.c +++ b/ui/spice-module.c @@ -XXX,XX +XXX,XX @@ static int qemu_spice_display_add_client_stub(int csock, int skipauth, return -1; } +static void qemu_spice_cleanup_stub(void) +{ +} + struct QemuSpiceOps qemu_spice = { .init = qemu_spice_init_stub, .display_init = qemu_spice_display_init_stub, @@ -XXX,XX +XXX,XX @@ struct QemuSpiceOps qemu_spice = { .set_passwd = qemu_spice_set_passwd_stub, .set_pw_expire = qemu_spice_set_pw_expire_stub, .display_add_client = qemu_spice_display_add_client_stub, + .cleanup = qemu_spice_cleanup_stub, }; #ifdef CONFIG_SPICE diff --git a/scripts/lsan_suppressions.txt b/scripts/lsan_suppressions.txt index XXXXXXX..XXXXXXX 100644 --- a/scripts/lsan_suppressions.txt +++ b/scripts/lsan_suppressions.txt @@ -XXX,XX +XXX,XX @@ leak:libfontconfig.so # https://github.com/GNOME/glib/blob/main/tools/glib.supp # This avoids false positive leak reports for the qga-ssh-test. leak:g_set_user_dirs + +# spice_server_add_interface allocates internal channel data that +# spice_server_destroy does not free +# https://gitlab.freedesktop.org/spice/spice/-/merge_requests/246 +leak:spice_server_add_interface diff --git a/ui/meson.build b/ui/meson.build index XXXXXXX..XXXXXXX 100644 --- a/ui/meson.build +++ b/ui/meson.build @@ -XXX,XX +XXX,XX @@ libui_sources = files( 'kbd-state.c', 'keymaps.c', 'qemu-pixman.c', + 'spice-module.c', 'vgafont.c', ) if pixman.found() libui_sources += files('cp437.c', 'vt100.c') endif libui = static_library('qemuui', libui_sources + genh, - dependencies: [pixman], + dependencies: [pixman, spice_headers], build_by_default: false) ui = declare_dependency(objects: libui.extract_all_objects(recursive: false), dependencies: [pixman]) system_ss.add(png) @@ -XXX,XX +XXX,XX @@ system_ss.add(when: pixman, if_true: files('console-vc.c'), if_false: files('con if dbus_display system_ss.add(files('dbus-module.c')) endif -system_ss.add([spice_headers, files('spice-module.c')]) system_ss.add(when: spice_protocol, if_true: files('vdagent.c')) if host_os == 'linux' -- 2.54.0 Add a cleanup callback to QemuDisplay and a qemu_display_cleanup() function that iterates all registered display types and calls their cleanup handler. Wire it into qemu_cleanup() in runstate.c, replacing the ad-hoc vnc_cleanup() call. This provides a structured alternative to atexit() handlers, giving deterministic teardown ordering and making resource leaks visible to sanitizers. The cleanup should happen before user_creatable_cleanup(), since some display have weak user-creatable references to cleanup before. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- include/ui/console.h | 2 ++ system/runstate.c | 8 +------- ui/console.c | 20 +++++++++++++++++++- 3 files changed, 22 insertions(+), 8 deletions(-) diff --git a/include/ui/console.h b/include/ui/console.h index XXXXXXX..XXXXXXX 100644 --- a/include/ui/console.h +++ b/include/ui/console.h @@ -XXX,XX +XXX,XX @@ struct QemuDisplay { DisplayType type; void (*early_init)(DisplayOptions *opts); void (*init)(DisplayState *ds, DisplayOptions *opts); + void (*cleanup)(void); const char *vc; }; @@ -XXX,XX +XXX,XX @@ bool qemu_display_find_default(DisplayOptions *opts); void qemu_display_early_init(DisplayOptions *opts); void qemu_display_init(DisplayState *ds, DisplayOptions *opts); const char *qemu_display_get_vc(DisplayOptions *opts); +void qemu_display_cleanup(void); void qemu_display_help(void); /* vnc.c */ diff --git a/system/runstate.c b/system/runstate.c index XXXXXXX..XXXXXXX 100644 --- a/system/runstate.c +++ b/system/runstate.c @@ -XXX,XX +XXX,XX @@ #include "system/system.h" #include "system/tpm.h" #include "ui/console.h" -#include "ui/qemu-spice-module.h" #include "trace.h" @@ -XXX,XX +XXX,XX @@ void qemu_cleanup(int status) audio_cleanup(); monitor_cleanup(); qemu_chr_cleanup(); + qemu_display_cleanup(); user_creatable_cleanup(); -#ifdef CONFIG_VNC - vnc_cleanup(); -#endif -#ifdef CONFIG_SPICE - qemu_spice.cleanup(); -#endif /* TODO: unref root container, check all devices are ok */ } diff --git a/ui/console.c b/ui/console.c index XXXXXXX..XXXXXXX 100644 --- a/ui/console.c +++ b/ui/console.c @@ -XXX,XX +XXX,XX @@ #include "qemu/memfd.h" #include "ui/vt100.h" #include "vgafont.h" +#include "ui/qemu-spice.h" #include "console-priv.h" @@ -XXX,XX +XXX,XX @@ void qemu_console_set_display_gl_ctx(QemuConsole *con, DisplayGLCtx *gl) { /* display has opengl support */ assert(con); - if (con->gl) { + if (gl && con->gl) { error_report("The console already has an OpenGL context."); exit(1); } @@ -XXX,XX +XXX,XX @@ void qemu_display_init(DisplayState *ds, DisplayOptions *opts) dpys[opts->type]->init(ds, opts); } +void qemu_display_cleanup(void) +{ + int i; + + for (i = 0; i < DISPLAY_TYPE__MAX; i++) { + if (dpys[i] && dpys[i]->cleanup) { + dpys[i]->cleanup(); + } + } +#ifdef CONFIG_VNC + vnc_cleanup(); +#endif +#ifdef CONFIG_SPICE + qemu_spice.cleanup(); +#endif +} + const char *qemu_display_get_vc(DisplayOptions *opts) { #ifdef CONFIG_PIXMAN -- 2.54.0 Replace the atexit() handler with a proper cleanup callback. The new curses_cleanup() unregisters the display listener, destroy & free the allocated resources. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- ui/curses.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/ui/curses.c b/ui/curses.c index XXXXXXX..XXXXXXX 100644 --- a/ui/curses.c +++ b/ui/curses.c @@ -XXX,XX +XXX,XX @@ static void curses_refresh(DisplayChangeListener *dcl) } } -static void curses_atexit(void) +static void curses_cleanup(void) { + if (!dcl) { + return; + } + endwin(); - g_free(vga_to_curses); - g_free(screen); + qemu_console_unregister_listener(dcl); + g_clear_pointer(&dcl, g_free); + g_clear_pointer(&screenpad, delwin); + g_clear_pointer(&vga_to_curses, g_free); + g_clear_pointer(&screen, g_free); + g_clear_pointer(&kbd_layout, kbd_layout_free); } /* @@ -XXX,XX +XXX,XX @@ static void curses_display_init(DisplayState *ds, DisplayOptions *opts) vga_to_curses = g_new0(cchar_t, 256); curses_setup(); curses_keyboard_setup(); - atexit(curses_atexit); - curses_winch_init(); dcl = g_new0(DisplayChangeListener, 1); @@ -XXX,XX +XXX,XX @@ static void curses_display_init(DisplayState *ds, DisplayOptions *opts) static QemuDisplay qemu_display_curses = { .type = DISPLAY_TYPE_CURSES, .init = curses_display_init, + .cleanup = curses_cleanup, }; static void register_curses(void) -- 2.54.0 Replace the atexit() handler with a proper cleanup callback. Extend sdl_cleanup() to unregister display listeners, free keyboard state, destroy windows, and clean up all cursor resources. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- ui/sdl2.c | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/ui/sdl2.c b/ui/sdl2.c index XXXXXXX..XXXXXXX 100644 --- a/ui/sdl2.c +++ b/ui/sdl2.c @@ -XXX,XX +XXX,XX @@ static void sdl_mouse_define(DisplayChangeListener *dcl, static void sdl_cleanup(void) { - if (guest_sprite) { - SDL_FreeCursor(guest_sprite); + int i; + + if (!sdl2_console) { + return; } + + qemu_remove_mouse_mode_change_notifier(&mouse_mode_notifier); + + for (i = 0; i < sdl2_num_outputs; i++) { + qemu_console_unregister_listener(&sdl2_console[i].dcl); + qkbd_state_free(sdl2_console[i].kbd); + sdl2_window_destroy(&sdl2_console[i]); + } + g_clear_pointer(&sdl2_console, g_free); + sdl2_num_outputs = 0; + + g_clear_pointer(&guest_sprite, SDL_FreeCursor); + g_clear_pointer(&guest_sprite_surface, SDL_FreeSurface); + g_clear_pointer(&sdl_cursor_hidden, SDL_FreeCursor); SDL_QuitSubSystem(SDL_INIT_VIDEO); } @@ -XXX,XX +XXX,XX @@ static void sdl2_display_init(DisplayState *ds, DisplayOptions *o) sdl_grab_start(&sdl2_console[0]); } - atexit(sdl_cleanup); - /* SDL's event polling (in dpy_refresh) must happen on the main thread. */ qemu_main = NULL; } @@ -XXX,XX +XXX,XX @@ static QemuDisplay qemu_display_sdl2 = { .type = DISPLAY_TYPE_SDL, .early_init = sdl2_display_early_init, .init = sdl2_display_init, + .cleanup = sdl_cleanup, }; static void register_sdl1(void) -- 2.54.0 Replace the atexit() handler with the display cleanup callback, reusing the existing spice_app_atexit() function. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- ui/spice-app.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/ui/spice-app.c b/ui/spice-app.c index XXXXXXX..XXXXXXX 100644 --- a/ui/spice-app.c +++ b/ui/spice-app.c @@ -XXX,XX +XXX,XX @@ static const TypeInfo char_vc_type_info = { .class_size = sizeof(VCChardevClass), }; -static void spice_app_atexit(void) +static void spice_app_cleanup(void) { if (sock_path) { unlink(sock_path); + g_clear_pointer(&sock_path, g_free); } if (tmp_dir) { rmdir(tmp_dir); + tmp_dir = NULL; } - g_free(sock_path); - g_free(app_dir); + g_clear_pointer(&app_dir, g_free); } static void spice_app_display_early_init(DisplayOptions *opts) @@ -XXX,XX +XXX,XX @@ static void spice_app_display_early_init(DisplayOptions *opts) exit(1); } - atexit(spice_app_atexit); - if (qemu_name) { app_dir = g_build_filename(g_get_user_runtime_dir(), "qemu", qemu_name, NULL); @@ -XXX,XX +XXX,XX @@ static QemuDisplay qemu_display_spice_app = { .type = DISPLAY_TYPE_SPICE_APP, .early_init = spice_app_display_early_init, .init = spice_app_display_init, + .cleanup = spice_app_cleanup, .vc = "vc", }; -- 2.54.0 Add egl_cleanup() to tear down the EGL render node context, GBM device, and EGL display. Add egl_headless_cleanup() to unregister listeners, destroy framebuffers, and free per-console state tracked via a new GPtrArray. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- include/ui/egl-helpers.h | 1 + ui/egl-headless.c | 31 +++++++++++++++++++++++++++++++ ui/egl-helpers.c | 19 +++++++++++++++++++ 3 files changed, 51 insertions(+) diff --git a/include/ui/egl-helpers.h b/include/ui/egl-helpers.h index XXXXXXX..XXXXXXX 100644 --- a/include/ui/egl-helpers.h +++ b/include/ui/egl-helpers.h @@ -XXX,XX +XXX,XX @@ EGLContext qemu_egl_init_ctx(void); bool qemu_egl_has_dmabuf(void); bool egl_init(const char *rendernode, DisplayGLMode mode, Error **errp); +void egl_cleanup(void); const char *qemu_egl_get_error_string(void); diff --git a/ui/egl-headless.c b/ui/egl-headless.c index XXXXXXX..XXXXXXX 100644 --- a/ui/egl-headless.c +++ b/ui/egl-headless.c @@ -XXX,XX +XXX,XX @@ typedef struct egl_dpy { DisplayChangeListener dcl; + DisplayGLCtx *ctx; DisplaySurface *ds; QemuGLShader *gls; egl_fb guest_fb; @@ -XXX,XX +XXX,XX @@ typedef struct egl_dpy { uint32_t pos_y; } egl_dpy; +static GPtrArray *egl_dpys; + /* ------------------------------------------------------------------ */ static void egl_refresh(DisplayChangeListener *dcl) @@ -XXX,XX +XXX,XX @@ static void egl_headless_init(DisplayState *ds, DisplayOptions *opts) egl_dpy *edpy; int idx; + egl_dpys = g_ptr_array_new(); + for (idx = 0;; idx++) { DisplayGLCtx *ctx; @@ -XXX,XX +XXX,XX @@ static void egl_headless_init(DisplayState *ds, DisplayOptions *opts) edpy->gls = qemu_gl_init_shader(); ctx = g_new0(DisplayGLCtx, 1); ctx->ops = &eglctx_ops; + edpy->ctx = ctx; qemu_console_set_display_gl_ctx(con, ctx); qemu_console_register_listener(con, &edpy->dcl, &egl_ops); + g_ptr_array_add(egl_dpys, edpy); + } +} + +static void egl_headless_cleanup(void) +{ + if (!egl_dpys) { + return; } + + for (guint i = 0; i < egl_dpys->len; i++) { + egl_dpy *edpy = g_ptr_array_index(egl_dpys, i); + + qemu_console_unregister_listener(&edpy->dcl); + qemu_console_set_display_gl_ctx(edpy->dcl.con, NULL); + egl_fb_destroy(&edpy->guest_fb); + egl_fb_destroy(&edpy->cursor_fb); + egl_fb_destroy(&edpy->blit_fb); + qemu_gl_fini_shader(edpy->gls); + g_free(edpy->ctx); + g_free(edpy); + } + g_clear_pointer(&egl_dpys, g_ptr_array_unref); + + egl_cleanup(); } static QemuDisplay qemu_display_egl = { .type = DISPLAY_TYPE_EGL_HEADLESS, .early_init = early_egl_headless_init, .init = egl_headless_init, + .cleanup = egl_headless_cleanup, }; static void register_egl(void) diff --git a/ui/egl-helpers.c b/ui/egl-helpers.c index XXXXXXX..XXXXXXX 100644 --- a/ui/egl-helpers.c +++ b/ui/egl-helpers.c @@ -XXX,XX +XXX,XX @@ bool egl_init(const char *rendernode, DisplayGLMode mode, Error **errp) display_opengl = 1; return true; } + +void egl_cleanup(void) +{ + if (qemu_egl_rn_ctx) { + eglDestroyContext(qemu_egl_display, qemu_egl_rn_ctx); + qemu_egl_rn_ctx = NULL; + } + +#ifdef CONFIG_GBM + g_clear_pointer(&qemu_egl_rn_gbm_dev, gbm_device_destroy); + g_clear_fd(&qemu_egl_rn_fd, NULL); +#endif + + if (qemu_egl_display) { + eglReleaseThread(); + eglTerminate(qemu_egl_display); + qemu_egl_display = NULL; + } +} -- 2.54.0 Move cbowner release from QemuCocoaAppController -dealloc to cocoa_display_cleanup(), since cbowner is allocated in cocoa_display_init() and cleanup is the symmetric teardown path. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- ui/cocoa.m | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/ui/cocoa.m b/ui/cocoa.m index XXXXXXX..XXXXXXX 100644 --- a/ui/cocoa.m +++ b/ui/cocoa.m @@ -XXX,XX +XXX,XX @@ - (void) dealloc COCOA_DEBUG("QemuCocoaAppController: dealloc\n"); [cocoaView release]; - [cbowner release]; - cbowner = nil; [super dealloc]; } @@ -XXX,XX +XXX,XX @@ static void cocoa_display_init(DisplayState *ds, DisplayOptions *opts) qemu_main = cocoa_main; } +static void cocoa_display_cleanup(void) +{ + if (!kbd) { + return; + } + + qemu_console_unregister_listener(&dcl); + g_clear_pointer(&kbd, qkbd_state_free); + qemu_remove_mouse_mode_change_notifier(&mouse_mode_change_notifier); + qemu_clipboard_peer_unregister(&cbpeer); + g_clear_pointer(&cbinfo, qemu_clipboard_info_unref); + qemu_event_destroy(&cbevent); + [cbowner release]; + cbowner = nil; +} + static QemuDisplay qemu_display_cocoa = { .type = DISPLAY_TYPE_COCOA, .init = cocoa_display_init, + .cleanup = cocoa_display_cleanup, }; static void register_cocoa(void) -- 2.54.0 Add dbus_cleanup() to unparent the D-Bus display object, ensuring proper teardown before user_creatable_cleanup() runs. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- ui/dbus-console.c | 3 +++ ui/dbus.c | 13 +++++++++++++ 2 files changed, 16 insertions(+) diff --git a/ui/dbus-console.c b/ui/dbus-console.c index XXXXXXX..XXXXXXX 100644 --- a/ui/dbus-console.c +++ b/ui/dbus-console.c @@ -XXX,XX +XXX,XX @@ dbus_display_console_dispose(GObject *object) DBusDisplayConsole *ddc = DBUS_DISPLAY_CONSOLE(object); qemu_console_unregister_listener(&ddc->dcl); + if (ddc->dcl.con) { + qemu_console_set_display_gl_ctx(ddc->dcl.con, NULL); + } qemu_remove_mouse_mode_change_notifier(&ddc->mouse_mode_notifier); g_clear_pointer(&ddc->kbd_leds_updated, qemu_remove_led_event_handler); g_clear_object(&ddc->iface_touch); diff --git a/ui/dbus.c b/ui/dbus.c index XXXXXXX..XXXXXXX 100644 --- a/ui/dbus.c +++ b/ui/dbus.c @@ -XXX,XX +XXX,XX @@ static const TypeInfo dbus_display_info = { } }; +static void +dbus_cleanup(void) +{ + Object *o; + + o = object_resolve_path_component(object_get_objects_root(), + "dbus-display"); + if (o) { + object_unparent(o); + } +} + static QemuDisplay qemu_display_dbus = { .type = DISPLAY_TYPE_DBUS, .early_init = early_dbus_init, .init = dbus_init, + .cleanup = dbus_cleanup, .vc = "vc", }; -- 2.54.0 Add gtk_display_cleanup() to properly tear down GTK display state: remove console and mouse notifiers, unregister clipboard peer, destroy the main window and virtual consoles. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- include/ui/gtk.h | 2 ++ ui/gtk-clipboard.c | 15 +++++++++++++++ ui/gtk.c | 20 +++++++++++++++++++- 3 files changed, 36 insertions(+), 1 deletion(-) diff --git a/include/ui/gtk.h b/include/ui/gtk.h index XXXXXXX..XXXXXXX 100644 --- a/include/ui/gtk.h +++ b/include/ui/gtk.h @@ -XXX,XX +XXX,XX @@ struct GtkDisplayState { GdkCursor *null_cursor; Notifier mouse_mode_notifier; + VMChangeStateEntry *vmse; gboolean free_scale; gboolean keep_aspect_ratio; @@ -XXX,XX +XXX,XX @@ int gd_gl_area_make_current(DisplayGLCtx *dgc, /* gtk-clipboard.c */ void gd_clipboard_init(GtkDisplayState *gd); +void gd_clipboard_cleanup(GtkDisplayState *gd); void gd_update_scale(VirtualConsole *vc, int ww, int wh, int fbw, int fbh); diff --git a/ui/gtk-clipboard.c b/ui/gtk-clipboard.c index XXXXXXX..XXXXXXX 100644 --- a/ui/gtk-clipboard.c +++ b/ui/gtk-clipboard.c @@ -XXX,XX +XXX,XX @@ void gd_clipboard_init(GtkDisplayState *gd) g_signal_connect(gd->gtkcb[QEMU_CLIPBOARD_SELECTION_SECONDARY], "owner-change", G_CALLBACK(gd_owner_change), gd); } + +void gd_clipboard_cleanup(GtkDisplayState *gd) +{ + if (!gd->cbpeer.name) { + return; + } + qemu_clipboard_peer_unregister(&gd->cbpeer); + g_signal_handlers_disconnect_by_data( + gd->gtkcb[QEMU_CLIPBOARD_SELECTION_CLIPBOARD], gd); + g_signal_handlers_disconnect_by_data( + gd->gtkcb[QEMU_CLIPBOARD_SELECTION_PRIMARY], gd); + g_signal_handlers_disconnect_by_data( + gd->gtkcb[QEMU_CLIPBOARD_SELECTION_SECONDARY], gd); + gd->cbpeer.name = NULL; +} diff --git a/ui/gtk.c b/ui/gtk.c index XXXXXXX..XXXXXXX 100644 --- a/ui/gtk.c +++ b/ui/gtk.c @@ -XXX,XX +XXX,XX @@ static void gd_create_menus(GtkDisplayState *s, DisplayOptions *opts) } +static GtkDisplayState *gtk_display_state; static gboolean gtkinit; static void gtk_display_init(DisplayState *ds, DisplayOptions *opts) @@ -XXX,XX +XXX,XX @@ static void gtk_display_init(DisplayState *ds, DisplayOptions *opts) } assert(opts->type == DISPLAY_TYPE_GTK); s = g_malloc0(sizeof(*s)); + gtk_display_state = s; s->opts = opts; theme = gtk_icon_theme_get_default(); @@ -XXX,XX +XXX,XX @@ static void gtk_display_init(DisplayState *ds, DisplayOptions *opts) s->mouse_mode_notifier.notify = gd_mouse_mode_change; qemu_add_mouse_mode_change_notifier(&s->mouse_mode_notifier); - qemu_add_vm_change_state_handler(gd_change_runstate, s); + s->vmse = qemu_add_vm_change_state_handler(gd_change_runstate, s); gtk_window_set_icon_name(GTK_WINDOW(s->window), "qemu"); @@ -XXX,XX +XXX,XX @@ static void early_gtk_display_init(DisplayOptions *opts) #endif } +static void gtk_display_cleanup(void) +{ + GtkDisplayState *s = gtk_display_state; + + if (!s) { + return; + } + qemu_del_vm_change_state_handler(s->vmse); + qemu_remove_mouse_mode_change_notifier(&s->mouse_mode_notifier); + gd_clipboard_cleanup(s); + g_clear_pointer(&s->window, gtk_widget_destroy); + g_clear_object(&s->null_cursor); + g_clear_pointer(&gtk_display_state, g_free); +} + static QemuDisplay qemu_display_gtk = { .type = DISPLAY_TYPE_GTK, .early_init = early_gtk_display_init, .init = gtk_display_init, + .cleanup = gtk_display_cleanup, .vc = "vc", }; -- 2.54.0 Add a NotifierList to DisplayState so display backends can be notified when consoles are added or removed at runtime. No events are fired yet, that follows in the next commits. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- include/ui/console.h | 18 ++++++++++++++++++ ui/console.c | 23 +++++++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/include/ui/console.h b/include/ui/console.h index XXXXXXX..XXXXXXX 100644 --- a/include/ui/console.h +++ b/include/ui/console.h @@ -XXX,XX +XXX,XX @@ const char *qemu_display_get_vc(DisplayOptions *opts); void qemu_display_cleanup(void); void qemu_display_help(void); +/* + * Console notifications: + * Handlers must be idempotent, it may notify multiple times. + */ +typedef enum { + QEMU_CONSOLE_ADDED, + QEMU_CONSOLE_REMOVED, +} QemuConsoleEventType; + +typedef struct QemuConsoleEvent { + QemuConsoleEventType type; + QemuConsole *con; +} QemuConsoleEvent; + +void qemu_console_add_notifier(Notifier *notifier); +void qemu_console_remove_notifier(Notifier *notifier); +void qemu_console_notify(QemuConsoleEventType type, QemuConsole *con); + /* vnc.c */ void vnc_display_add_client(const char *id, int csock, bool skipauth); int vnc_display_password(const char *id, const char *password, Error **errp); diff --git a/ui/console.c b/ui/console.c index XXXXXXX..XXXXXXX 100644 --- a/ui/console.c +++ b/ui/console.c @@ -XXX,XX +XXX,XX @@ struct DisplayState { bool refreshing; QLIST_HEAD(, DisplayChangeListener) listeners; + NotifierList console_notifiers; }; static DisplayState *display_state; @@ -XXX,XX +XXX,XX @@ static bool console_compatible_with(QemuConsole *con, static QemuConsole *qemu_graphic_console_lookup_unused(void); static void dpy_set_ui_info_timer(void *opaque); +void qemu_console_notify(QemuConsoleEventType type, QemuConsole *con) +{ + DisplayState *ds = get_alloc_displaystate(); + QemuConsoleEvent event = { + .type = type, + .con = con, + }; + notifier_list_notify(&ds->console_notifiers, &event); +} + static void gui_update(void *opaque) { uint64_t interval = GUI_REFRESH_INTERVAL_IDLE; @@ -XXX,XX +XXX,XX @@ static DisplayState *get_alloc_displaystate(void) { if (!display_state) { display_state = g_new0(DisplayState, 1); + notifier_list_init(&display_state->console_notifiers); } return display_state; } +void qemu_console_add_notifier(Notifier *notifier) +{ + DisplayState *ds = get_alloc_displaystate(); + notifier_list_add(&ds->console_notifiers, notifier); +} + +void qemu_console_remove_notifier(Notifier *notifier) +{ + notifier_remove(notifier); +} + /* * Called by main(), after creating QemuConsoles * and before initializing ui (sdl/vnc/...). -- 2.54.0 Fire CONSOLE_ADDED at the end of graphic_console_init() and CONSOLE_REMOVED at the start of graphic_console_close(), so display backends can react to console hotplug/unplug events. REMOVED fires before the device link is cleared and before the placeholder surface swap, so handlers can unregister their DCL while the console is still in a known state. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- ui/console.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ui/console.c b/ui/console.c index XXXXXXX..XXXXXXX 100644 --- a/ui/console.c +++ b/ui/console.c @@ -XXX,XX +XXX,XX @@ QemuConsole *qemu_graphic_console_create(DeviceState *dev, uint32_t head, surface = qemu_create_placeholder_surface(width, height, noinit); qemu_console_set_surface(s, surface); + qemu_console_notify(QEMU_CONSOLE_ADDED, s); return s; } @@ -XXX,XX +XXX,XX @@ void qemu_graphic_console_close(QemuConsole *con) int height = qemu_console_get_height(con, 480); trace_console_gfx_close(con->index); + qemu_console_notify(QEMU_CONSOLE_REMOVED, con); object_property_set_link(OBJECT(con), "device", NULL, &error_abort); qemu_graphic_console_set_hwops(con, &unused_ops, NULL); timer_del(con->ui_timer); -- 2.54.0 Fire CONSOLE_ADDED when the chardev is opened. Fire CONSOLE_REMOVED in char_vc_finalize() before dropping the console reference, so the console is still in a valid state when listeners handle the event. Also fixes a console object leak by adding the missing object_unref(). Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- ui/console-vc.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/ui/console-vc.c b/ui/console-vc.c index XXXXXXX..XXXXXXX 100644 --- a/ui/console-vc.c +++ b/ui/console-vc.c @@ -XXX,XX +XXX,XX @@ static bool vc_chr_open(Chardev *chr, ChardevBackend *backend, Error **errp) } qemu_chr_be_event(chr, CHR_EVENT_OPENED); + qemu_console_notify(QEMU_CONSOLE_ADDED, QEMU_CONSOLE(s)); return true; } @@ -XXX,XX +XXX,XX @@ static void char_vc_init(Object *obj) vc->encoding = CHARDEV_VC_ENCODING_UTF8; } +static void char_vc_finalize(Object *obj) +{ + VCChardev *vc = VC_CHARDEV(obj); + QemuConsole *con = QEMU_CONSOLE(vc->console); + + if (con) { + qemu_console_notify(QEMU_CONSOLE_REMOVED, con); + object_unref(con); + } +} + static const TypeInfo char_vc_type_info = { .name = TYPE_CHARDEV_VC, .parent = TYPE_CHARDEV, .instance_size = sizeof(VCChardev), .instance_init = char_vc_init, .instance_post_init = object_apply_compat_props, + .instance_finalize = char_vc_finalize, .class_init = char_vc_class_init, }; -- 2.54.0 Replace the fixed-size vc[MAX_VCS] with GPtrArray. This is a preparatory refactoring for console hotplug support, which needs to add/remove VCs dynamically. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- include/ui/gtk.h | 3 +- ui/gtk.c | 108 +++++++++++++++++++++++++++++++++++++++++++------------ 2 files changed, 87 insertions(+), 24 deletions(-) diff --git a/include/ui/gtk.h b/include/ui/gtk.h index XXXXXXX..XXXXXXX 100644 --- a/include/ui/gtk.h +++ b/include/ui/gtk.h @@ -XXX,XX +XXX,XX @@ struct GtkDisplayState { GtkWidget *grab_item; GtkWidget *grab_on_hover_item; - int nb_vcs; - VirtualConsole vc[MAX_VCS]; + GPtrArray *vcs; GtkWidget *show_tabs_item; GtkWidget *untabify_item; diff --git a/ui/gtk.c b/ui/gtk.c index XXXXXXX..XXXXXXX 100644 --- a/ui/gtk.c +++ b/ui/gtk.c @@ -XXX,XX +XXX,XX @@ static VirtualConsole *gd_vc_find_by_menu(GtkDisplayState *s) VirtualConsole *vc; gint i; - for (i = 0; i < s->nb_vcs; i++) { - vc = &s->vc[i]; + for (i = 0; i < s->vcs->len; i++) { + vc = g_ptr_array_index(s->vcs, i); if (gtk_check_menu_item_get_active (GTK_CHECK_MENU_ITEM(vc->menu_item))) { return vc; @@ -XXX,XX +XXX,XX @@ static VirtualConsole *gd_vc_find_by_page(GtkDisplayState *s, gint page) VirtualConsole *vc; gint i, p; - for (i = 0; i < s->nb_vcs; i++) { - vc = &s->vc[i]; + if (!s->vcs) { + return NULL; + } + for (i = 0; i < s->vcs->len; i++) { + vc = g_ptr_array_index(s->vcs, i); p = gtk_notebook_page_num(GTK_NOTEBOOK(s->notebook), vc->tab_item); if (p == page) { return vc; @@ -XXX,XX +XXX,XX @@ static void gd_update_caption(GtkDisplayState *s) gtk_window_set_title(GTK_WINDOW(s->window), title); g_free(title); - for (i = 0; i < s->nb_vcs; i++) { - VirtualConsole *vc = &s->vc[i]; + for (i = 0; i < s->vcs->len; i++) { + VirtualConsole *vc = g_ptr_array_index(s->vcs, i); if (!vc->window) { continue; @@ -XXX,XX +XXX,XX @@ static void gtk_release_modifiers(GtkDisplayState *s) { VirtualConsole *vc = gd_vc_find_current(s); - if (vc->type != GD_VC_GFX || + if (!vc || vc->type != GD_VC_GFX || !qemu_console_is_graphic(vc->gfx.dcl.con)) { return; } @@ -XXX,XX +XXX,XX @@ static void gd_mouse_mode_change(Notifier *notify, void *data) gd_ungrab_pointer(s); } } - for (i = 0; i < s->nb_vcs; i++) { - VirtualConsole *vc = &s->vc[i]; + for (i = 0; i < s->vcs->len; i++) { + VirtualConsole *vc = g_ptr_array_index(s->vcs, i); gd_update_cursor(vc); } } @@ -XXX,XX +XXX,XX @@ static void gd_vcs_init(GtkDisplayState *s, GSList *group, int i; for (i = 0; i < nb_vcs; i++) { - VirtualConsole *vc = &s->vc[s->nb_vcs]; - group = gd_vc_vte_init(s, vc, vcs[i], s->nb_vcs, group, view_menu); - s->nb_vcs++; + VirtualConsole *vc = g_new0(VirtualConsole, 1); + g_ptr_array_add(s->vcs, vc); + group = gd_vc_vte_init(s, vc, vcs[i], s->vcs->len - 1, + group, view_menu); } } #endif /* CONFIG_VTE */ @@ -XXX,XX +XXX,XX @@ static GtkWidget *gd_create_menu_view(GtkDisplayState *s, DisplayOptions *opts) /* gfx */ for (vc = 0;; vc++) { + VirtualConsole *v; con = qemu_console_lookup_by_index(vc); if (!con) { break; } - group = gd_vc_gfx_init(s, &s->vc[vc], con, - vc, group, view_menu); - s->nb_vcs++; + v = g_new0(VirtualConsole, 1); + g_ptr_array_add(s->vcs, v); + group = gd_vc_gfx_init(s, v, con, vc, group, view_menu); } #if defined(CONFIG_VTE) @@ -XXX,XX +XXX,XX @@ static void gd_create_menus(GtkDisplayState *s, DisplayOptions *opts) } +static void gd_vc_free(void *p) +{ + VirtualConsole *vc = p; + + switch (vc->type) { + case GD_VC_GFX: + qemu_console_unregister_listener(&vc->gfx.dcl); +#if defined(CONFIG_OPENGL) + if (display_opengl) { + qemu_console_set_display_gl_ctx(vc->gfx.dcl.con, NULL); + } + if (vc->gfx.ectx) { + eglMakeCurrent(qemu_egl_display, vc->gfx.esurface, + vc->gfx.esurface, vc->gfx.ectx); + } else if (gtk_use_gl_area) { + gtk_gl_area_make_current(GTK_GL_AREA(vc->gfx.drawing_area)); + } + if (vc->gfx.gls) { + surface_gl_destroy_texture(vc->gfx.gls, vc->gfx.ds); + qemu_gl_fini_shader(vc->gfx.gls); + } + egl_fb_destroy(&vc->gfx.guest_fb); + egl_fb_destroy(&vc->gfx.win_fb); + egl_fb_destroy(&vc->gfx.cursor_fb); + if (vc->gfx.esurface) { + eglDestroySurface(qemu_egl_display, vc->gfx.esurface); + } + if (vc->gfx.ectx) { + eglDestroyContext(qemu_egl_display, vc->gfx.ectx); + } +#endif + qkbd_state_free(vc->gfx.kbd); + if (vc->gfx.surface) { + cairo_surface_destroy(vc->gfx.surface); + } + if (vc->gfx.convert) { + pixman_image_unref(vc->gfx.convert); + } + break; + case GD_VC_VTE: +#ifdef CONFIG_VTE + fifo8_destroy(&vc->vte.out_fifo); +#endif + break; + } + + if (vc->window) { + gtk_widget_destroy(vc->window); + } else if (vc->tab_item) { + gtk_widget_destroy(vc->tab_item); + } + if (vc->menu_item) { + gtk_widget_destroy(vc->menu_item); + } + g_free(vc->label); + g_free(vc); +} + static GtkDisplayState *gtk_display_state; static gboolean gtkinit; @@ -XXX,XX +XXX,XX @@ static void gtk_display_init(DisplayState *ds, DisplayOptions *opts) assert(opts->type == DISPLAY_TYPE_GTK); s = g_malloc0(sizeof(*s)); gtk_display_state = s; + s->vcs = g_ptr_array_new_with_free_func(gd_vc_free); s->opts = opts; theme = gtk_icon_theme_get_default(); @@ -XXX,XX +XXX,XX @@ static void gtk_display_init(DisplayState *ds, DisplayOptions *opts) gtk_widget_show_all(s->window); - for (idx = 0;; idx++) { - QemuConsole *con = qemu_console_lookup_by_index(idx); - if (!con) { - break; - } - if (s->vc[idx].type == GD_VC_GFX) { - gtk_widget_realize(s->vc[idx].gfx.drawing_area); + for (idx = 0; idx < s->vcs->len; idx++) { + VirtualConsole *v = g_ptr_array_index(s->vcs, idx); + if (v->type == GD_VC_GFX) { + gtk_widget_realize(v->gfx.drawing_area); } } @@ -XXX,XX +XXX,XX @@ static void gtk_display_cleanup(void) qemu_del_vm_change_state_handler(s->vmse); qemu_remove_mouse_mode_change_notifier(&s->mouse_mode_notifier); gd_clipboard_cleanup(s); + g_signal_handlers_disconnect_by_func(s->notebook, + G_CALLBACK(gd_change_page), s); + g_clear_pointer(&s->vcs, g_ptr_array_unref); g_clear_pointer(&s->window, gtk_widget_destroy); g_clear_object(&s->null_cursor); g_clear_pointer(&gtk_display_state, g_free); -- 2.54.0 Move zoom_to_fit, keep_aspect_ratio and touch_slots initialisation from gd_vc_gfx_init() to gd_create_menu_view(). These are global display settings that should be set once after all consoles are created, not repeated on every per-console init (where the last iteration's values silently win). Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- ui/gtk.c | 46 +++++++++++++++++++++++----------------------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/ui/gtk.c b/ui/gtk.c index XXXXXXX..XXXXXXX 100644 --- a/ui/gtk.c +++ b/ui/gtk.c @@ -XXX,XX +XXX,XX @@ static GSList *gd_vc_gfx_init(GtkDisplayState *s, VirtualConsole *vc, GSList *group, GtkWidget *view_menu) { const DisplayChangeListenerOps *ops = &dcl_ops; - bool zoom_to_fit = false; - int i; vc->label = qemu_console_get_label(con); vc->s = s; @@ -XXX,XX +XXX,XX @@ static GSList *gd_vc_gfx_init(GtkDisplayState *s, VirtualConsole *vc, gd_connect_vc_gfx_signals(vc); group = gd_vc_menu_init(s, vc, idx, group, view_menu); - if (qemu_console_ui_info_supported(vc->gfx.dcl.con)) { - zoom_to_fit = true; - } - if (s->opts->u.gtk.has_zoom_to_fit) { - zoom_to_fit = s->opts->u.gtk.zoom_to_fit; - } - if (zoom_to_fit) { - gtk_menu_item_activate(GTK_MENU_ITEM(s->zoom_fit_item)); - s->free_scale = true; - } - - s->keep_aspect_ratio = true; - if (s->opts->u.gtk.has_keep_aspect_ratio) - s->keep_aspect_ratio = s->opts->u.gtk.keep_aspect_ratio; - - for (i = 0; i < INPUT_EVENT_SLOTS_MAX; i++) { - struct touch_slot *slot = &touch_slots[i]; - slot->tracking_id = -1; - } - return group; } @@ -XXX,XX +XXX,XX @@ static GtkWidget *gd_create_menu_view(GtkDisplayState *s, DisplayOptions *opts) GtkWidget *view_menu; GtkWidget *separator; QemuConsole *con; - int vc; + bool zoom_to_fit = false; + int vc, i; view_menu = gtk_menu_new(); gtk_menu_set_accel_group(GTK_MENU(view_menu), s->accel_group); @@ -XXX,XX +XXX,XX @@ static GtkWidget *gd_create_menu_view(GtkDisplayState *s, DisplayOptions *opts) v = g_new0(VirtualConsole, 1); g_ptr_array_add(s->vcs, v); group = gd_vc_gfx_init(s, v, con, vc, group, view_menu); + if (qemu_console_ui_info_supported(con)) { + zoom_to_fit = true; + } + } + + if (s->opts->u.gtk.has_zoom_to_fit) { + zoom_to_fit = s->opts->u.gtk.zoom_to_fit; + } + if (zoom_to_fit) { + gtk_menu_item_activate(GTK_MENU_ITEM(s->zoom_fit_item)); + s->free_scale = true; + } + + s->keep_aspect_ratio = true; + if (s->opts->u.gtk.has_keep_aspect_ratio) { + s->keep_aspect_ratio = s->opts->u.gtk.keep_aspect_ratio; + } + + for (i = 0; i < INPUT_EVENT_SLOTS_MAX; i++) { + struct touch_slot *slot = &touch_slots[i]; + slot->tracking_id = -1; } #if defined(CONFIG_VTE) -- 2.54.0 Add gd_vc_notebook_pos() which computes the correct notebook position for a console by counting only non-detached (non-windowed) tabs before it. Use it in gd_tab_window_close() so a re-attached tab is inserted at its logical position rather than appended at the end. Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- ui/gtk.c | 27 ++++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/ui/gtk.c b/ui/gtk.c index XXXXXXX..XXXXXXX 100644 --- a/ui/gtk.c +++ b/ui/gtk.c @@ -XXX,XX +XXX,XX @@ static void gd_menu_show_tabs(GtkMenuItem *item, void *opaque) gd_update_windowsize(vc); } +static int gd_vc_notebook_pos(GtkDisplayState *s, VirtualConsole *target) +{ + int pos = 0; + guint i; + + for (i = 0; i < s->vcs->len; i++) { + VirtualConsole *vc = g_ptr_array_index(s->vcs, i); + if (vc == target) { + return pos; + } + if (!vc->window) { + pos++; + } + } + g_assert_not_reached(); +} + static gboolean gd_tab_window_close(GtkWidget *widget, GdkEvent *event, void *opaque) { VirtualConsole *vc = opaque; GtkDisplayState *s = vc->s; + int page; gtk_widget_set_sensitive(vc->menu_item, true); - gd_widget_reparent(vc->window, s->notebook, vc->tab_item); - gtk_notebook_set_tab_label_text(GTK_NOTEBOOK(s->notebook), - vc->tab_item, vc->label); + g_object_ref(vc->tab_item); + gtk_container_remove(GTK_CONTAINER(vc->window), vc->tab_item); + page = gd_vc_notebook_pos(s, vc); + gtk_notebook_insert_page(GTK_NOTEBOOK(s->notebook), + vc->tab_item, gtk_label_new(vc->label), page); + g_object_unref(vc->tab_item); gtk_widget_destroy(vc->window); vc->window = NULL; #if defined(CONFIG_OPENGL) -- 2.54.0 Replace the per-console gd_vc_menu_init() with gd_rebuild_vc_menu() that tears down and rebuilds all console radio menu items and Ctrl+Alt+N accelerators at once. This is called from initialization and whenever consoles are detached or reattached. Shortcuts now skip detached (windowed) consoles, so they always map to reachable tabs. Rename gd_vc_gfx_init() to add_gfx_console() and simplify the init function signatures now that menu creation is decoupled. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- include/ui/gtk.h | 1 + ui/gtk.c | 127 ++++++++++++++++++++++++++++++++++++------------------- 2 files changed, 85 insertions(+), 43 deletions(-) diff --git a/include/ui/gtk.h b/include/ui/gtk.h index XXXXXXX..XXXXXXX 100644 --- a/include/ui/gtk.h +++ b/include/ui/gtk.h @@ -XXX,XX +XXX,XX @@ struct GtkDisplayState { GPtrArray *vcs; + GtkWidget *vc_menu_separator; GtkWidget *show_tabs_item; GtkWidget *untabify_item; GtkWidget *show_menubar_item; diff --git a/ui/gtk.c b/ui/gtk.c index XXXXXXX..XXXXXXX 100644 --- a/ui/gtk.c +++ b/ui/gtk.c @@ -XXX,XX +XXX,XX @@ static void gd_grab_pointer(VirtualConsole *vc, const char *reason); static void gd_ungrab_pointer(GtkDisplayState *s); static void gd_grab_keyboard(VirtualConsole *vc, const char *reason); static void gd_ungrab_keyboard(GtkDisplayState *s); +static void gd_rebuild_vc_menu(GtkDisplayState *s); /** Utility Functions **/ @@ -XXX,XX +XXX,XX @@ static gboolean gd_tab_window_close(GtkWidget *widget, GdkEvent *event, GtkDisplayState *s = vc->s; int page; - gtk_widget_set_sensitive(vc->menu_item, true); g_object_ref(vc->tab_item); gtk_container_remove(GTK_CONTAINER(vc->window), vc->tab_item); page = gd_vc_notebook_pos(s, vc); @@ -XXX,XX +XXX,XX @@ static gboolean gd_tab_window_close(GtkWidget *widget, GdkEvent *event, } #endif + gd_rebuild_vc_menu(s); + if (vc == gd_vc_find_by_menu(s)) { gtk_widget_grab_focus(vc->focus); } @@ -XXX,XX +XXX,XX @@ static void gd_menu_untabify(GtkMenuItem *item, void *opaque) FALSE); } if (!vc->window) { - gtk_widget_set_sensitive(vc->menu_item, false); vc->window = gtk_window_new(GTK_WINDOW_TOPLEVEL); #if defined(CONFIG_OPENGL) if (vc->gfx.esurface) { @@ -XXX,XX +XXX,XX @@ static void gd_menu_untabify(GtkMenuItem *item, void *opaque) gtk_accel_group_connect(ag, GDK_KEY_g, HOTKEY_MODIFIERS, 0, cb); } + gd_rebuild_vc_menu(s); gd_update_geometry_hints(vc); gd_update_caption(s); } @@ -XXX,XX +XXX,XX @@ static gboolean gd_configure(GtkWidget *widget, /** Virtual Console Callbacks **/ -static GSList *gd_vc_menu_init(GtkDisplayState *s, VirtualConsole *vc, - int idx, GSList *group, GtkWidget *view_menu) +static void gd_rebuild_vc_menu(GtkDisplayState *s) { - vc->menu_item = gtk_radio_menu_item_new_with_mnemonic(group, vc->label); - gtk_accel_group_connect(s->accel_group, GDK_KEY_1 + idx, - HOTKEY_MODIFIERS, 0, - g_cclosure_new_swap(G_CALLBACK(gd_accel_switch_vc), vc, NULL)); - gtk_accel_label_set_accel( - GTK_ACCEL_LABEL(gtk_bin_get_child(GTK_BIN(vc->menu_item))), - GDK_KEY_1 + idx, HOTKEY_MODIFIERS); + GSList *group = NULL; + VirtualConsole *vc; + GList *children; + gint insert_pos; + int shortcut_idx = 0; + guint i; + + for (i = 0; i < s->vcs->len; i++) { + vc = g_ptr_array_index(s->vcs, i); + if (vc->menu_item) { + gtk_widget_destroy(vc->menu_item); + vc->menu_item = NULL; + } + } + + for (i = 0; i < 9; i++) { + gtk_accel_group_disconnect_key(s->accel_group, + GDK_KEY_1 + i, HOTKEY_MODIFIERS); + } + + /* find insertion position (just before vc_menu_separator) */ + children = gtk_container_get_children(GTK_CONTAINER(s->view_menu)); + insert_pos = g_list_index(children, s->vc_menu_separator); + g_list_free(children); + + /* create new menu items for each console */ + for (i = 0; i < s->vcs->len; i++) { + vc = g_ptr_array_index(s->vcs, i); - g_signal_connect(vc->menu_item, "activate", - G_CALLBACK(gd_menu_switch_vc), s); - gtk_menu_shell_append(GTK_MENU_SHELL(view_menu), vc->menu_item); + vc->menu_item = gtk_radio_menu_item_new_with_mnemonic(group, + vc->label); + group = gtk_radio_menu_item_get_group( + GTK_RADIO_MENU_ITEM(vc->menu_item)); + + if (vc->window) { + gtk_widget_set_sensitive(vc->menu_item, false); + } else if (shortcut_idx < 9) { + guint key = GDK_KEY_1 + shortcut_idx; + gtk_accel_group_connect(s->accel_group, key, + HOTKEY_MODIFIERS, 0, + g_cclosure_new_swap(G_CALLBACK(gd_accel_switch_vc), + vc, NULL)); + gtk_accel_label_set_accel( + GTK_ACCEL_LABEL(gtk_bin_get_child(GTK_BIN(vc->menu_item))), + key, HOTKEY_MODIFIERS); + shortcut_idx++; + } - return gtk_radio_menu_item_get_group(GTK_RADIO_MENU_ITEM(vc->menu_item)); + g_signal_connect(vc->menu_item, "activate", + G_CALLBACK(gd_menu_switch_vc), s); + gtk_menu_shell_insert(GTK_MENU_SHELL(s->view_menu), + vc->menu_item, insert_pos + i); + gtk_widget_show(vc->menu_item); + } + + /* sync active menu item with current notebook page */ + vc = gd_vc_find_current(s); + if (vc && vc->menu_item) { + g_signal_handlers_block_by_func(vc->menu_item, + gd_menu_switch_vc, s); + gtk_check_menu_item_set_active( + GTK_CHECK_MENU_ITEM(vc->menu_item), TRUE); + g_signal_handlers_unblock_by_func(vc->menu_item, + gd_menu_switch_vc, s); + } } #if defined(CONFIG_VTE) @@ -XXX,XX +XXX,XX @@ static gboolean gd_vc_in(VteTerminal *terminal, gchar *text, guint size, return TRUE; } -static GSList *gd_vc_vte_init(GtkDisplayState *s, VirtualConsole *vc, - Chardev *chr, int idx, - GSList *group, GtkWidget *view_menu) +static void gd_vc_vte_init(GtkDisplayState *s, VirtualConsole *vc, + Chardev *chr, int idx) { char buffer[32]; GtkWidget *box; @@ -XXX,XX +XXX,XX @@ static GSList *gd_vc_vte_init(GtkDisplayState *s, VirtualConsole *vc, snprintf(buffer, sizeof(buffer), "vc%d", idx); vc->label = g_strdup(vc->vte.chr->label ? : buffer); - group = gd_vc_menu_init(s, vc, idx, group, view_menu); vc->vte.terminal = vte_terminal_new(); g_signal_connect(vc->vte.terminal, "commit", G_CALLBACK(gd_vc_in), vc); @@ -XXX,XX +XXX,XX @@ static GSList *gd_vc_vte_init(GtkDisplayState *s, VirtualConsole *vc, gtk_label_new(vc->label)); qemu_chr_be_event(vc->vte.chr, CHR_EVENT_OPENED); - - return group; } -static void gd_vcs_init(GtkDisplayState *s, GSList *group, - GtkWidget *view_menu) +static void gd_vcs_init(GtkDisplayState *s) { int i; for (i = 0; i < nb_vcs; i++) { VirtualConsole *vc = g_new0(VirtualConsole, 1); g_ptr_array_add(s->vcs, vc); - group = gd_vc_vte_init(s, vc, vcs[i], s->vcs->len - 1, - group, view_menu); + gd_vc_vte_init(s, vc, vcs[i], s->vcs->len - 1); } } #endif /* CONFIG_VTE */ @@ -XXX,XX +XXX,XX @@ static bool gd_scale_valid(double scale) return scale >= VC_SCALE_MIN && scale <= VC_SCALE_MAX; } -static GSList *gd_vc_gfx_init(GtkDisplayState *s, VirtualConsole *vc, - QemuConsole *con, int idx, - GSList *group, GtkWidget *view_menu) +static void add_gfx_console(GtkDisplayState *s, QemuConsole *con) { + VirtualConsole *vc = g_new0(VirtualConsole, 1); const DisplayChangeListenerOps *ops = &dcl_ops; + g_ptr_array_add(s->vcs, vc); vc->label = qemu_console_get_label(con); vc->s = s; vc->gfx.preferred_scale = 1.0; @@ -XXX,XX +XXX,XX @@ static GSList *gd_vc_gfx_init(GtkDisplayState *s, VirtualConsole *vc, qemu_console_register_listener(con, &vc->gfx.dcl, ops); gd_connect_vc_gfx_signals(vc); - group = gd_vc_menu_init(s, vc, idx, group, view_menu); - - return group; } -static GtkWidget *gd_create_menu_view(GtkDisplayState *s, DisplayOptions *opts) +static void gd_create_menu_view(GtkDisplayState *s, DisplayOptions *opts) { - GSList *group = NULL; GtkWidget *view_menu; GtkWidget *separator; QemuConsole *con; @@ -XXX,XX +XXX,XX @@ static GtkWidget *gd_create_menu_view(GtkDisplayState *s, DisplayOptions *opts) int vc, i; view_menu = gtk_menu_new(); + s->view_menu = view_menu; gtk_menu_set_accel_group(GTK_MENU(view_menu), s->accel_group); s->full_screen_item = gtk_menu_item_new_with_mnemonic(_("_Fullscreen")); @@ -XXX,XX +XXX,XX @@ static GtkWidget *gd_create_menu_view(GtkDisplayState *s, DisplayOptions *opts) /* gfx */ for (vc = 0;; vc++) { - VirtualConsole *v; con = qemu_console_lookup_by_index(vc); if (!con) { break; } - v = g_new0(VirtualConsole, 1); - g_ptr_array_add(s->vcs, v); - group = gd_vc_gfx_init(s, v, con, vc, group, view_menu); + add_gfx_console(s, con); if (qemu_console_ui_info_supported(con)) { zoom_to_fit = true; } @@ -XXX,XX +XXX,XX @@ static GtkWidget *gd_create_menu_view(GtkDisplayState *s, DisplayOptions *opts) #if defined(CONFIG_VTE) /* vte */ - gd_vcs_init(s, group, view_menu); + gd_vcs_init(s); #endif - separator = gtk_separator_menu_item_new(); - gtk_menu_shell_append(GTK_MENU_SHELL(view_menu), separator); + s->vc_menu_separator = gtk_separator_menu_item_new(); + gtk_menu_shell_append(GTK_MENU_SHELL(view_menu), s->vc_menu_separator); + + gd_rebuild_vc_menu(s); s->show_tabs_item = gtk_check_menu_item_new_with_mnemonic(_("Show _Tabs")); gtk_menu_shell_append(GTK_MENU_SHELL(view_menu), s->show_tabs_item); @@ -XXX,XX +XXX,XX @@ static GtkWidget *gd_create_menu_view(GtkDisplayState *s, DisplayOptions *opts) GTK_ACCEL_LABEL(gtk_bin_get_child(GTK_BIN(s->show_menubar_item))), GDK_KEY_m, HOTKEY_MODIFIERS); gtk_menu_shell_append(GTK_MENU_SHELL(view_menu), s->show_menubar_item); - - return view_menu; } static void gd_create_menus(GtkDisplayState *s, DisplayOptions *opts) @@ -XXX,XX +XXX,XX @@ static void gd_create_menus(GtkDisplayState *s, DisplayOptions *opts) s->accel_group = gtk_accel_group_new(); s->machine_menu = gd_create_menu_machine(s); - s->view_menu = gd_create_menu_view(s, opts); + gd_create_menu_view(s, opts); s->machine_menu_item = gtk_menu_item_new_with_mnemonic(_("_Machine")); gtk_menu_item_set_submenu(GTK_MENU_ITEM(s->machine_menu_item), -- 2.54.0 Register a console notifier so the GTK display dynamically creates and destroys VirtualConsole tabs when graphic consoles are added or removed at runtime (e.g. vfio-pci with display=on hotplug). Add skips consoles that already have a VC binding, remove skips unknown consoles. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- include/ui/gtk.h | 1 + ui/gtk.c | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 75 insertions(+), 1 deletion(-) diff --git a/include/ui/gtk.h b/include/ui/gtk.h index XXXXXXX..XXXXXXX 100644 --- a/include/ui/gtk.h +++ b/include/ui/gtk.h @@ -XXX,XX +XXX,XX @@ struct GtkDisplayState { GdkCursor *null_cursor; Notifier mouse_mode_notifier; VMChangeStateEntry *vmse; + Notifier console_notifier; gboolean free_scale; gboolean keep_aspect_ratio; diff --git a/ui/gtk.c b/ui/gtk.c index XXXXXXX..XXXXXXX 100644 --- a/ui/gtk.c +++ b/ui/gtk.c @@ -XXX,XX +XXX,XX @@ static bool gd_scale_valid(double scale) return scale >= VC_SCALE_MIN && scale <= VC_SCALE_MAX; } -static void add_gfx_console(GtkDisplayState *s, QemuConsole *con) +static VirtualConsole * +add_gfx_console(GtkDisplayState *s, QemuConsole *con) { VirtualConsole *vc = g_new0(VirtualConsole, 1); const DisplayChangeListenerOps *ops = &dcl_ops; @@ -XXX,XX +XXX,XX @@ static void add_gfx_console(GtkDisplayState *s, QemuConsole *con) qemu_console_register_listener(con, &vc->gfx.dcl, ops); gd_connect_vc_gfx_signals(vc); + return vc; +} + +static void gd_vc_add_gfx(GtkDisplayState *s, QemuConsole *con) +{ + VirtualConsole *vc; + int i; + + for (i = 0; i < (int)s->vcs->len; i++) { + VirtualConsole *v = g_ptr_array_index(s->vcs, i); + if (v->type == GD_VC_GFX && v->gfx.dcl.con == con) { + return; + } + } + + vc = add_gfx_console(s, con); + gtk_widget_show_all(vc->tab_item); + gtk_widget_realize(vc->gfx.drawing_area); + + if (s->free_scale) { + gd_update_windowsize(vc); + } + + gd_update_caption(s); + gd_rebuild_vc_menu(s); +} + +static void gd_vc_remove_gfx(GtkDisplayState *s, QemuConsole *con) +{ + VirtualConsole *vc = NULL; + guint idx; + + for (idx = 0; idx < s->vcs->len; idx++) { + VirtualConsole *v = g_ptr_array_index(s->vcs, idx); + if (v->type == GD_VC_GFX && v->gfx.dcl.con == con) { + vc = v; + break; + } + } + if (!vc) { + return; + } + + if (s->kbd_owner == vc) { + gd_ungrab_keyboard(s); + } + if (s->ptr_owner == vc) { + gd_ungrab_pointer(s); + } + + g_ptr_array_remove_index(s->vcs, idx); + gd_rebuild_vc_menu(s); + gd_update_caption(s); +} + +static void gd_console_notify(Notifier *n, void *data) +{ + GtkDisplayState *s = container_of(n, GtkDisplayState, console_notifier); + QemuConsoleEvent *event = data; + + switch (event->type) { + case QEMU_CONSOLE_ADDED: + gd_vc_add_gfx(s, event->con); + break; + case QEMU_CONSOLE_REMOVED: + gd_vc_remove_gfx(s, event->con); + break; + } } static void gd_create_menu_view(GtkDisplayState *s, DisplayOptions *opts) @@ -XXX,XX +XXX,XX @@ static void gtk_display_init(DisplayState *ds, DisplayOptions *opts) gd_create_menus(s, opts); + s->console_notifier.notify = gd_console_notify; + qemu_console_add_notifier(&s->console_notifier); + gd_connect_signals(s); gtk_notebook_set_show_tabs(GTK_NOTEBOOK(s->notebook), FALSE); @@ -XXX,XX +XXX,XX @@ static void gtk_display_cleanup(void) return; } qemu_del_vm_change_state_handler(s->vmse); + qemu_console_remove_notifier(&s->console_notifier); qemu_remove_mouse_mode_change_notifier(&s->mouse_mode_notifier); gd_clipboard_cleanup(s); g_signal_handlers_disconnect_by_func(s->notebook, -- 2.54.0 Consoles created after init_displaystate() (e.g. hotplugged display devices) were never added to the /backend/console[N] QOM tree. Extract qemu_console_add_to_qom() and call it from qemu_console_register() when the display is already initialized. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- ui/console.c | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/ui/console.c b/ui/console.c index XXXXXXX..XXXXXXX 100644 --- a/ui/console.c +++ b/ui/console.c @@ -XXX,XX +XXX,XX @@ struct DisplayState { uint64_t last_update; uint64_t update_interval; bool refreshing; + bool initialized; QLIST_HEAD(, DisplayChangeListener) listeners; NotifierList console_notifiers; @@ -XXX,XX +XXX,XX @@ void qemu_text_console_put_string(QemuTextConsole *s, const char *str, int len) } } +static void qemu_console_add_to_qom(QemuConsole *con) +{ + g_autofree gchar *name = g_strdup_printf("console[%d]", con->index); + object_property_add_child(object_get_container("backend"), + name, OBJECT(con)); +} + static void qemu_console_register(QemuConsole *c) { @@ -XXX,XX +XXX,XX @@ qemu_console_register(QemuConsole *c) } } } + + if (c->ds->initialized) { + qemu_console_add_to_qom(c); + } } static void @@ -XXX,XX +XXX,XX @@ void qemu_console_remove_notifier(Notifier *notifier) */ DisplayState *init_displaystate(void) { - gchar *name; + DisplayState *ds = get_alloc_displaystate(); QemuConsole *con; QTAILQ_FOREACH(con, &consoles, next) { /* Hook up into the qom tree here (not in object_new()), once * all QemuConsoles are created and the order / numbering * doesn't change any more */ - name = g_strdup_printf("console[%d]", con->index); - object_property_add_child(object_get_container("backend"), - name, OBJECT(con)); - g_free(name); + qemu_console_add_to_qom(con); } - return display_state; + ds->initialized = true; + + return ds; } void qemu_graphic_console_set_hwops(QemuConsole *con, -- 2.54.0 Call object_unparent() in graphic_console_close() to remove the console from /backend/console[N]. This drops the QOM tree reference, while the initial object_new() reference keeps the console alive for potential reuse. When graphic_console_init() reuses a closed console, re-register it in the QOM tree. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- ui/console.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ui/console.c b/ui/console.c index XXXXXXX..XXXXXXX 100644 --- a/ui/console.c +++ b/ui/console.c @@ -XXX,XX +XXX,XX @@ QemuConsole *qemu_graphic_console_create(DeviceState *dev, uint32_t head, trace_console_gfx_reuse(s->index); width = qemu_console_get_width(s, 0); height = qemu_console_get_height(s, 0); + if (s->ds->initialized) { + qemu_console_add_to_qom(s); + } } else { trace_console_gfx_new(); s = (QemuConsole *)object_new(TYPE_QEMU_GRAPHIC_CONSOLE); @@ -XXX,XX +XXX,XX @@ void qemu_graphic_console_close(QemuConsole *con) } surface = qemu_create_placeholder_surface(width, height, unplugged); qemu_console_set_surface(con, surface); + object_unparent(OBJECT(con)); } QemuConsole *qemu_console_lookup_default(void) -- 2.54.0 Subscribe to QemuConsoleEvent notifications to dynamically add and remove D-Bus display consoles. This mirrors the GTK backend's handling added in the previous commits. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- ui/dbus.h | 3 ++ ui/dbus-console.c | 5 +++ ui/dbus.c | 93 +++++++++++++++++++++++++++++++++++++++++++++---------- 3 files changed, 84 insertions(+), 17 deletions(-) diff --git a/ui/dbus.h b/ui/dbus.h index XXXXXXX..XXXXXXX 100644 --- a/ui/dbus.h +++ b/ui/dbus.h @@ -XXX,XX +XXX,XX @@ struct DBusDisplay { DBusClipboardRequest clipboard_request[QEMU_CLIPBOARD_SELECTION__COUNT]; Notifier notifier; + Notifier console_notifier; }; #ifdef WIN32 @@ -XXX,XX +XXX,XX @@ dbus_display_console_new(DBusDisplay *display, QemuConsole *con); int dbus_display_console_get_index(DBusDisplayConsole *ddc); +QemuConsole * +dbus_display_console_get_qemu_console(DBusDisplayConsole *ddc); extern const DisplayChangeListenerOps dbus_console_dcl_ops; diff --git a/ui/dbus-console.c b/ui/dbus-console.c index XXXXXXX..XXXXXXX 100644 --- a/ui/dbus-console.c +++ b/ui/dbus-console.c @@ -XXX,XX +XXX,XX @@ int dbus_display_console_get_index(DBusDisplayConsole *ddc) return qemu_console_get_index(ddc->dcl.con); } +QemuConsole *dbus_display_console_get_qemu_console(DBusDisplayConsole *ddc) +{ + return ddc->dcl.con; +} + DBusDisplayConsole * dbus_display_console_new(DBusDisplay *display, QemuConsole *con) { diff --git a/ui/dbus.c b/ui/dbus.c index XXXXXXX..XXXXXXX 100644 --- a/ui/dbus.c +++ b/ui/dbus.c @@ -XXX,XX +XXX,XX @@ dbus_display_finalize(Object *o) { DBusDisplay *dd = DBUS_DISPLAY(o); + if (dd->console_notifier.notify) { + qemu_console_remove_notifier(&dd->console_notifier); + } if (dd->notifier.notify) { dbus_display_notifier_remove(&dd->notifier); } @@ -XXX,XX +XXX,XX @@ dbus_display_finalize(Object *o) dbus_display = NULL; } +static void +dbus_update_console_ids(DBusDisplay *dd) +{ + g_autoptr(GArray) arr = g_array_new(FALSE, FALSE, sizeof(guint32)); + + for (guint i = 0; i < dd->consoles->len; i++) { + DBusDisplayConsole *ddc = g_ptr_array_index(dd->consoles, i); + guint32 idx = dbus_display_console_get_index(ddc); + g_array_append_val(arr, idx); + } + + g_object_set(dd->iface, "console-ids", + g_variant_new_fixed_array(G_VARIANT_TYPE("u"), + arr->data, arr->len, + sizeof(guint32)), + NULL); +} + static bool -dbus_display_add_console(DBusDisplay *dd, int idx, Error **errp) +dbus_display_add_console(DBusDisplay *dd, QemuConsole *con, Error **errp) { - QemuConsole *con; DBusDisplayConsole *dbus_console; - con = qemu_console_lookup_by_index(idx); - assert(con); + for (guint i = 0; i < dd->consoles->len; i++) { + DBusDisplayConsole *ddc = g_ptr_array_index(dd->consoles, i); + if (dbus_display_console_get_qemu_console(ddc) == con) { + return true; + } + } if (qemu_console_is_graphic(con) && dd->gl_mode != DISPLAY_GL_MODE_OFF) { @@ -XXX,XX +XXX,XX @@ dbus_display_add_console(DBusDisplay *dd, int idx, Error **errp) } dbus_console = dbus_display_console_new(dd, con); - g_ptr_array_insert(dd->consoles, idx, dbus_console); + g_ptr_array_add(dd->consoles, dbus_console); g_dbus_object_manager_server_export(dd->server, G_DBUS_OBJECT_SKELETON(dbus_console)); + dbus_update_console_ids(dd); return true; } +static void +dbus_display_remove_console(DBusDisplay *dd, QemuConsole *con) +{ + for (guint i = 0; i < dd->consoles->len; i++) { + DBusDisplayConsole *ddc = g_ptr_array_index(dd->consoles, i); + if (dbus_display_console_get_qemu_console(ddc) == con) { + if (display_opengl) { + qemu_console_set_display_gl_ctx(con, NULL); + } + g_dbus_object_manager_server_unexport( + dd->server, + g_dbus_object_get_object_path(G_DBUS_OBJECT(ddc))); + g_ptr_array_remove_index(dd->consoles, i); + dbus_update_console_ids(dd); + break; + } + } +} + +static void +dbus_console_notify(Notifier *n, void *data) +{ + DBusDisplay *dd = container_of(n, DBusDisplay, console_notifier); + QemuConsoleEvent *event = data; + + switch (event->type) { + case QEMU_CONSOLE_ADDED: { + Error *err = NULL; + if (!dbus_display_add_console(dd, event->con, &err)) { + error_report_err(err); + } + break; + } + case QEMU_CONSOLE_REMOVED: + dbus_display_remove_console(dd, event->con); + break; + } +} + static void dbus_display_complete(UserCreatable *uc, Error **errp) { DBusDisplay *dd = DBUS_DISPLAY(uc); g_autoptr(GError) err = NULL; g_autofree char *uuid = qemu_uuid_unparse_strdup(&qemu_uuid); - g_autoptr(GArray) consoles = NULL; - GVariant *console_ids; int idx; if (!object_resolve_path_type("", TYPE_DBUS_DISPLAY, NULL)) { @@ -XXX,XX +XXX,XX @@ dbus_display_complete(UserCreatable *uc, Error **errp) } } - consoles = g_array_new(FALSE, FALSE, sizeof(guint32)); for (idx = 0;; idx++) { - if (!qemu_console_lookup_by_index(idx)) { + QemuConsole *con = qemu_console_lookup_by_index(idx); + if (!con) { break; } - if (!dbus_display_add_console(dd, idx, errp)) { + if (!dbus_display_add_console(dd, con, errp)) { return; } - g_array_append_val(consoles, idx); } - console_ids = g_variant_new_from_data( - G_VARIANT_TYPE("au"), - consoles->data, consoles->len * sizeof(guint32), TRUE, - (GDestroyNotify)g_array_unref, consoles); - g_steal_pointer(&consoles); g_object_set(dd->iface, "name", qemu_name ?: "QEMU " QEMU_VERSION, "uuid", uuid, - "console-ids", console_ids, NULL); + dbus_update_console_ids(dd); + + dd->console_notifier.notify = dbus_console_notify; + qemu_console_add_notifier(&dd->console_notifier); if (dd->bus) { g_dbus_object_manager_server_set_connection(dd->server, dd->bus); -- 2.54.0 Add a qtest that verifies display consoles are dynamically added and removed over D-Bus when a bochs-display device is hotplugged and unplugged on a q35 machine. The test plugs device_add a bochs-display, waits for the DEVICE_ADDED QMP event, and checks that the D-Bus VM interface reports a second console. It then device_del it, forces a system reset (q35 removal is ACPI-based and needs guest cooperation qtest cannot provide), waits for DEVICE_DELETED, and checks the console count again. Reviewed-by: Fabiano Rosas <farosas@suse.de> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- tests/qtest/dbus-display-test.c | 101 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 99 insertions(+), 2 deletions(-) diff --git a/tests/qtest/dbus-display-test.c b/tests/qtest/dbus-display-test.c index XXXXXXX..XXXXXXX 100644 --- a/tests/qtest/dbus-display-test.c +++ b/tests/qtest/dbus-display-test.c @@ -XXX,XX +XXX,XX @@ #include <gio/gio.h> #include <gio/gunixfdlist.h> #include "libqtest.h" +#include "qobject/qdict.h" +#include "qobject/qstring.h" #include "ui/dbus-display1.h" static GDBusConnection* @@ -XXX,XX +XXX,XX @@ test_dbus_p2p_from_fd(int fd) } static void -test_setup(QTestState **qts, GDBusConnection **conn) +test_setup_args(QTestState **qts, GDBusConnection **conn, const char *args) { int pair[2]; - *qts = qtest_init("-display dbus,p2p=yes -name dbus-test"); + *qts = qtest_init(args); g_assert_cmpint(qemu_socketpair(AF_UNIX, SOCK_STREAM, 0, pair), ==, 0); @@ -XXX,XX +XXX,XX @@ test_setup(QTestState **qts, GDBusConnection **conn) g_dbus_connection_start_message_processing(*conn); } +static void +test_setup(QTestState **qts, GDBusConnection **conn) +{ + test_setup_args(qts, conn, "-display dbus,p2p=yes -name dbus-test"); +} + static void test_dbus_display_vm(void) { @@ -XXX,XX +XXX,XX @@ test_dbus_display_keyboard(void) qtest_quit(qts); } +static gsize +get_console_ids_count(GDBusConnection *conn) +{ + g_autoptr(GError) err = NULL; + g_autoptr(QemuDBusDisplay1VMProxy) vm = NULL; + GVariant *console_ids; + gsize n_ids = 0; + + vm = QEMU_DBUS_DISPLAY1_VM_PROXY( + qemu_dbus_display1_vm_proxy_new_sync( + conn, + G_DBUS_PROXY_FLAGS_NONE, + NULL, + DBUS_DISPLAY1_ROOT "/VM", + NULL, + &err)); + g_assert_no_error(err); + + console_ids = qemu_dbus_display1_vm_get_console_ids( + QEMU_DBUS_DISPLAY1_VM(vm)); + if (console_ids) { + n_ids = g_variant_n_children(console_ids); + } + return n_ids; +} + +static void +wait_device_event(QTestState *qts, const char *event_name, const char *id) +{ + QDict *resp, *data; + QString *qstr; + + for (;;) { + resp = qtest_qmp_eventwait_ref(qts, event_name); + data = qdict_get_qdict(resp, "data"); + if (!data || !qdict_get(data, "device")) { + qobject_unref(resp); + continue; + } + qstr = qobject_to(QString, qdict_get(data, "device")); + if (!strcmp(qstring_get_str(qstr), id)) { + qobject_unref(resp); + break; + } + qobject_unref(resp); + } +} + +static void +test_dbus_display_hotplug(void) +{ + g_autoptr(GDBusConnection) conn = NULL; + QTestState *qts = NULL; + gsize n; + + test_setup_args(&qts, &conn, + "-machine q35" + " -device pcie-root-port,id=rp0" + " -display dbus,p2p=yes" + " -name dbus-test"); + + n = get_console_ids_count(conn); + g_assert_cmpuint(n, ==, 1); + + qtest_qmp_device_add(qts, "bochs-display", "bochs0", + "{'bus': 'rp0'}"); + + n = get_console_ids_count(conn); + g_assert_cmpuint(n, ==, 2); + + /* + * On q35, PCI device removal is ACPI-based and requires guest + * acknowledgement. Since qtest has no guest OS, issue the delete + * request and force removal via system reset. + */ + qtest_qmp_device_del_send(qts, "bochs0"); + qtest_system_reset_nowait(qts); + wait_device_event(qts, "DEVICE_DELETED", "bochs0"); + + n = get_console_ids_count(conn); + g_assert_cmpuint(n, ==, 1); + + g_clear_object(&conn); + qtest_quit(qts); +} + int main(int argc, char **argv) { @@ -XXX,XX +XXX,XX @@ main(int argc, char **argv) qtest_add_data_func("/dbus-display/console", GINT_TO_POINTER(false), test_dbus_display_console); qtest_add_data_func("/dbus-display/console/map", GINT_TO_POINTER(true), test_dbus_display_console); qtest_add_func("/dbus-display/keyboard", test_dbus_display_keyboard); + if (qtest_has_machine("q35") && qtest_has_device("bochs-display")) { + qtest_add_func("/dbus-display/hotplug", test_dbus_display_hotplug); + } return g_test_run(); } -- 2.54.0