On 2024/10/07 21:42, Marc-André Lureau wrote:
HiOn Sat, Oct 5, 2024 at 12:32 PM Akihiko Odaki <akihiko.od...@daynix.com> wrote:On 2024/10/03 20:22, marcandre.lur...@redhat.com wrote:From: Marc-André Lureau <marcandre.lur...@redhat.com> Only check we eventually get a shared memory scanout. Signed-off-by: Marc-André Lureau <marcandre.lur...@redhat.com> --- tests/qtest/dbus-display-test.c | 64 ++++++++++++++++++++++++++++++--- 1 file changed, 59 insertions(+), 5 deletions(-) diff --git a/tests/qtest/dbus-display-test.c b/tests/qtest/dbus-display-test.c index 0390bdcb41..ac92cb00d4 100644 --- a/tests/qtest/dbus-display-test.c +++ b/tests/qtest/dbus-display-test.c @@ -2,9 +2,12 @@ #include "qemu/sockets.h" #include "qemu/dbus.h" #include "qemu/sockets.h" +#include "glib.h" +#include "glibconfig.h" #include <gio/gio.h> #include <gio/gunixfdlist.h> #include "libqtest.h" +#include <sys/mman.h> #include "ui/dbus-display1.h" static GDBusConnection* @@ -82,6 +85,7 @@ typedef struct TestDBusConsoleRegister { GThread *thread; GDBusConnection *listener_conn; GDBusObjectManagerServer *server; + bool with_map; } TestDBusConsoleRegister; static gboolean listener_handle_scanout( @@ -94,13 +98,48 @@ static gboolean listener_handle_scanout( GVariant *arg_data, TestDBusConsoleRegister *test) { + if (!test->with_map) { + g_main_loop_quit(test->loop); + } + + return DBUS_METHOD_INVOCATION_HANDLED; +} + +static gboolean listener_handle_scanout_map( + QemuDBusDisplay1ListenerUnixMap *object, + GDBusMethodInvocation *invocation, + GUnixFDList *fd_list, + GVariant *arg_handle, + guint arg_offset, + guint arg_width, + guint arg_height, + guint arg_stride, + guint arg_pixman_format, + TestDBusConsoleRegister *test) +{ + int fd = -1; + gint32 handle = g_variant_get_handle(arg_handle); + g_autoptr(GError) error = NULL; + void *addr = NULL; + size_t len = arg_height * arg_stride; + + g_assert_cmpuint(g_unix_fd_list_get_length(fd_list), ==, 1); + fd = g_unix_fd_list_get(fd_list, handle, &error); + g_assert_no_error(error); + + addr = mmap(NULL, len, PROT_READ, MAP_PRIVATE, fd, arg_offset); + g_assert_no_errno(GPOINTER_TO_INT(addr));Strictly speaking, this construct is not safe. When void * is 64-bit and int is 32-bit, this assetion will fail if the lower 32 bits of addr are in [0x80000000, 0xffffffff] though addr may still be a valid address. This is because GPOINTER_TO_INT() results in a negative value for such an address, and g_assert_no_errno() asserts that the given value is non-negative. Using g_mapped_file_new_from_fd() and will simplify this function as whole.I thought about that, but g_mapped_file_new_from_fd() doesn't take a len, and it fstat() the fd. This is ok for memfd apparently, and appears to work, but it isn't really compliant with the dbus interface.
That's unfortunate. It seems we need to do: g_assert_no_errno(addr == MAP_FAILED ? -1 : 0)g_assert_nonnull() following this should be unnecessary by the way. It shouldn't happen unless the underlying platform is broken, which is irrelevant from the correctness of our code.
+ g_assert_nonnull(addr); + g_assert_no_errno(munmap(addr, len)); + g_main_loop_quit(test->loop); + close(fd); return DBUS_METHOD_INVOCATION_HANDLED; } static void -test_dbus_console_setup_listener(TestDBusConsoleRegister *test) +test_dbus_console_setup_listener(TestDBusConsoleRegister *test, bool with_map) { g_autoptr(GDBusObjectSkeleton) listener = NULL; g_autoptr(QemuDBusDisplay1ListenerSkeleton) iface = NULL; @@ -114,6 +153,20 @@ test_dbus_console_setup_listener(TestDBusConsoleRegister *test) NULL); g_dbus_object_skeleton_add_interface(listener, G_DBUS_INTERFACE_SKELETON(iface)); + if (with_map) { + g_autoptr(QemuDBusDisplay1ListenerUnixMapSkeleton) iface_map = + QEMU_DBUS_DISPLAY1_LISTENER_UNIX_MAP_SKELETON( + qemu_dbus_display1_listener_unix_map_skeleton_new()); + + g_object_connect(iface_map, + "signal::handle-scanout-map", listener_handle_scanout_map, test, + NULL); + g_dbus_object_skeleton_add_interface(listener, + G_DBUS_INTERFACE_SKELETON(iface_map)); + g_object_set(iface, "interfaces", + (const gchar *[]) { "org.qemu.Display1.Listener.Unix.Map", NULL }, + NULL); + } g_dbus_object_manager_server_export(test->server, listener); g_dbus_object_manager_server_set_connection(test->server, test->listener_conn); @@ -145,7 +198,7 @@ test_dbus_console_registered(GObject *source_object, g_assert_no_error(err); test->listener_conn = g_thread_join(test->thread); - test_dbus_console_setup_listener(test); + test_dbus_console_setup_listener(test, test->with_map); } static gpointer @@ -155,7 +208,7 @@ test_dbus_p2p_server_setup_thread(gpointer data) } static void -test_dbus_display_console(void) +test_dbus_display_console(const void* data) { g_autoptr(GError) err = NULL; g_autoptr(GDBusConnection) conn = NULL; @@ -163,7 +216,7 @@ test_dbus_display_console(void) g_autoptr(GMainLoop) loop = NULL; QTestState *qts = NULL; int pair[2]; - TestDBusConsoleRegister test = { 0, }; + TestDBusConsoleRegister test = { 0, .with_map = GPOINTER_TO_INT(data) }; #ifdef WIN32 WSAPROTOCOL_INFOW info; g_autoptr(GVariant) listener = NULL; @@ -299,7 +352,8 @@ main(int argc, char **argv) g_test_init(&argc, &argv, NULL); qtest_add_func("/dbus-display/vm", test_dbus_display_vm); - qtest_add_func("/dbus-display/console", test_dbus_display_console); + 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); return g_test_run();
