From 786c5256d3262518d8805ac2a62eb1c4a3813b80 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Fri, 13 Jan 2023 11:27:14 +0100 Subject: libqtest: split qtest_spawn_qemu function In order to create a function that allows testing of invalid command lines, extract the parts of qtest_init_without_qmp_handshake that do not require any successful set up of sockets. Signed-off-by: Paolo Bonzini --- tests/qtest/libqtest.c | 105 ++++++++++++++++++++++++++----------------------- 1 file changed, 56 insertions(+), 49 deletions(-) (limited to 'tests/qtest') diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c index d658222..4e1f4fb 100644 --- a/tests/qtest/libqtest.c +++ b/tests/qtest/libqtest.c @@ -360,60 +360,25 @@ static pid_t qtest_create_process(char *cmd) } #endif /* _WIN32 */ -QTestState *qtest_init_without_qmp_handshake(const char *extra_args) +static QTestState *G_GNUC_PRINTF(1, 2) qtest_spawn_qemu(const char *fmt, ...) { - QTestState *s; - int sock, qmpsock, i; - gchar *socket_path; - gchar *qmp_socket_path; - gchar *command; - const char *qemu_binary = qtest_qemu_binary(); + va_list ap; + QTestState *s = g_new0(QTestState, 1); const char *trace = g_getenv("QTEST_TRACE"); g_autofree char *tracearg = trace ? g_strdup_printf("-trace %s ", trace) : g_strdup(""); + g_autoptr(GString) command = g_string_new(""); - s = g_new(QTestState, 1); - - socket_path = g_strdup_printf("%s/qtest-%d.sock", - g_get_tmp_dir(), getpid()); - qmp_socket_path = g_strdup_printf("%s/qtest-%d.qmp", - g_get_tmp_dir(), getpid()); - - /* It's possible that if an earlier test run crashed it might - * have left a stale unix socket lying around. Delete any - * stale old socket to avoid spurious test failures with - * tests/libqtest.c:70:init_socket: assertion failed (ret != -1): (-1 != -1) - */ - unlink(socket_path); - unlink(qmp_socket_path); - - socket_init(); - sock = init_socket(socket_path); - qmpsock = init_socket(qmp_socket_path); - - qtest_client_set_rx_handler(s, qtest_client_socket_recv_line); - qtest_client_set_tx_handler(s, qtest_client_socket_send); + va_start(ap, fmt); + g_string_append_printf(command, CMD_EXEC "%s %s", + qtest_qemu_binary(), tracearg); + g_string_append_vprintf(command, fmt, ap); + va_end(ap); qtest_add_abrt_handler(kill_qemu_hook_func, s); - command = g_strdup_printf(CMD_EXEC "%s %s" - "-qtest unix:%s " - "-qtest-log %s " - "-chardev socket,path=%s,id=char0 " - "-mon chardev=char0,mode=control " - "-display none " - "%s" - " -accel qtest", - qemu_binary, tracearg, socket_path, - getenv("QTEST_LOG") ? DEV_STDERR : DEV_NULL, - qmp_socket_path, - extra_args ?: ""); - - g_test_message("starting QEMU: %s", command); - - s->pending_events = NULL; - s->wstatus = 0; - s->expected_status = 0; + g_test_message("starting QEMU: %s", command->str); + #ifndef _WIN32 s->qemu_pid = fork(); if (s->qemu_pid == 0) { @@ -434,14 +399,56 @@ QTestState *qtest_init_without_qmp_handshake(const char *extra_args) if (!g_setenv("QEMU_AUDIO_DRV", "none", true)) { exit(1); } - execlp("/bin/sh", "sh", "-c", command, NULL); + execlp("/bin/sh", "sh", "-c", command->str, NULL); exit(1); } #else - s->qemu_pid = qtest_create_process(command); + s->qemu_pid = qtest_create_process(command->str); #endif /* _WIN32 */ - g_free(command); + return s; +} + +QTestState *qtest_init_without_qmp_handshake(const char *extra_args) +{ + QTestState *s; + int sock, qmpsock, i; + gchar *socket_path; + gchar *qmp_socket_path; + + socket_path = g_strdup_printf("%s/qtest-%d.sock", + g_get_tmp_dir(), getpid()); + qmp_socket_path = g_strdup_printf("%s/qtest-%d.qmp", + g_get_tmp_dir(), getpid()); + + /* + * It's possible that if an earlier test run crashed it might + * have left a stale unix socket lying around. Delete any + * stale old socket to avoid spurious test failures with + * tests/libqtest.c:70:init_socket: assertion failed (ret != -1): (-1 != -1) + */ + unlink(socket_path); + unlink(qmp_socket_path); + + socket_init(); + sock = init_socket(socket_path); + qmpsock = init_socket(qmp_socket_path); + + s = qtest_spawn_qemu("-qtest unix:%s " + "-qtest-log %s " + "-chardev socket,path=%s,id=char0 " + "-mon chardev=char0,mode=control " + "-display none " + "%s" + " -accel qtest", + socket_path, + getenv("QTEST_LOG") ? DEV_STDERR : DEV_NULL, + qmp_socket_path, + extra_args ?: ""); + + qtest_client_set_rx_handler(s, qtest_client_socket_recv_line); + qtest_client_set_tx_handler(s, qtest_client_socket_send); + s->fd = socket_accept(sock); if (s->fd >= 0) { s->qmp_fd = socket_accept(qmpsock); -- cgit v1.1 From 12008ff748d8cfb62fb937559c0fd844371bab5e Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Fri, 13 Jan 2023 12:01:20 +0100 Subject: libqtest: ensure waitpid() is only called once If a test aborts after qtest_wait_qemu() is called, the SIGABRT hooks are still in place and waitpid() is called again. The second time it is called, the process does not exist anymore and the system call fails. Move the s->qemu_pid = -1 assignment to qtest_wait_qemu() to make it idempotent, and anyway remove the SIGABRT hook as well to avoid that qtest_check_status() is called twice. Because of the extra call, qtest_remove_abrt_handler() now has to be made idempotent as well. Signed-off-by: Paolo Bonzini --- tests/qtest/libqtest.c | 55 ++++++++++++++++++++++++++++++-------------------- 1 file changed, 33 insertions(+), 22 deletions(-) (limited to 'tests/qtest') diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c index 4e1f4fb..2bfd460 100644 --- a/tests/qtest/libqtest.c +++ b/tests/qtest/libqtest.c @@ -158,6 +158,7 @@ bool qtest_probe_child(QTestState *s) CloseHandle((HANDLE)pid); #endif s->qemu_pid = -1; + qtest_remove_abrt_handler(s); } return false; } @@ -169,6 +170,8 @@ void qtest_set_expected_status(QTestState *s, int status) static void qtest_check_status(QTestState *s) { + assert(s->qemu_pid == -1); + /* * Check whether qemu exited with expected exit status; anything else is * fishy and should be logged with as much detail as possible. @@ -202,36 +205,40 @@ static void qtest_check_status(QTestState *s) void qtest_wait_qemu(QTestState *s) { + if (s->qemu_pid != -1) { #ifndef _WIN32 - pid_t pid; - uint64_t end; + pid_t pid; + uint64_t end; - /* poll for a while until sending SIGKILL */ - end = g_get_monotonic_time() + WAITPID_TIMEOUT * G_TIME_SPAN_SECOND; + /* poll for a while until sending SIGKILL */ + end = g_get_monotonic_time() + WAITPID_TIMEOUT * G_TIME_SPAN_SECOND; - do { - pid = waitpid(s->qemu_pid, &s->wstatus, WNOHANG); - if (pid != 0) { - break; - } - g_usleep(100 * 1000); - } while (g_get_monotonic_time() < end); + do { + pid = waitpid(s->qemu_pid, &s->wstatus, WNOHANG); + if (pid != 0) { + break; + } + g_usleep(100 * 1000); + } while (g_get_monotonic_time() < end); - if (pid == 0) { - kill(s->qemu_pid, SIGKILL); - pid = RETRY_ON_EINTR(waitpid(s->qemu_pid, &s->wstatus, 0)); - } + if (pid == 0) { + kill(s->qemu_pid, SIGKILL); + pid = RETRY_ON_EINTR(waitpid(s->qemu_pid, &s->wstatus, 0)); + } - assert(pid == s->qemu_pid); + assert(pid == s->qemu_pid); #else - DWORD ret; + DWORD ret; - ret = WaitForSingleObject((HANDLE)s->qemu_pid, INFINITE); - assert(ret == WAIT_OBJECT_0); - GetExitCodeProcess((HANDLE)s->qemu_pid, &s->exit_code); - CloseHandle((HANDLE)s->qemu_pid); + ret = WaitForSingleObject((HANDLE)s->qemu_pid, INFINITE); + assert(ret == WAIT_OBJECT_0); + GetExitCodeProcess((HANDLE)s->qemu_pid, &s->exit_code); + CloseHandle((HANDLE)s->qemu_pid); #endif + s->qemu_pid = -1; + qtest_remove_abrt_handler(s); + } qtest_check_status(s); } @@ -245,7 +252,6 @@ void qtest_kill_qemu(QTestState *s) TerminateProcess((HANDLE)s->qemu_pid, s->expected_status); #endif qtest_wait_qemu(s); - s->qemu_pid = -1; return; } @@ -307,6 +313,11 @@ void qtest_add_abrt_handler(GHookFunc fn, const void *data) void qtest_remove_abrt_handler(void *data) { GHook *hook = g_hook_find_data(&abrt_hooks, TRUE, data); + + if (!hook) { + return; + } + g_hook_destroy_link(&abrt_hooks, hook); /* Uninstall SIGABRT handler on last instance */ -- cgit v1.1