[Qemu-devel] [PATCH 1/2] tests/libqtest: Fix possible deadlock in qtest initialization

[Marcel Apfelbaum <marcel.a@redhat.com>]

'socket_accept' waits for Qemu to init its unix socket.
If Qemu encounters an error during command line parsing,
it can exit before initializing the communication channel.
It gets worse as the make check-qtest-* gets stuck without
notifying which test exactly has problems, so debugging can
be a challenge.

The solution has two parts:
 - Use a timeout for the socket.
 - Expose a qtest_state_valid that checks that the connections
   with Qemu are OK.
Asserting qtest_state_valid in each test after qtest_init
is a must, as we need to trace which test failed.

Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
---
 tests/libqtest.c | 26 +++++++++++++++++++++-----
 tests/libqtest.h |  8 ++++++++
 2 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/tests/libqtest.c b/tests/libqtest.c
index f587d36..93dfa81 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -34,6 +34,7 @@
 #include "qapi/qmp/json-parser.h"
 
 #define MAX_IRQ 256
+#define SOCKET_TIMEOUT 5
 
 QTestState *global_qtest;
 
@@ -83,7 +84,6 @@ static int socket_accept(int sock)
     do {
         ret = accept(sock, (struct sockaddr *)&addr, &addrlen);
     } while (ret == -1 && errno == EINTR);
-    g_assert_no_errno(ret);
     close(sock);
 
     return ret;
@@ -111,6 +111,8 @@ QTestState *qtest_init(const char *extra_args)
     gchar *command;
     const char *qemu_binary;
     struct sigaction sigact;
+    struct timeval socket_timeout = { .tv_sec = SOCKET_TIMEOUT,
+                                      .tv_usec = 0 };
 
     qemu_binary = getenv("QTEST_QEMU_BINARY");
     g_assert(qemu_binary != NULL);
@@ -123,6 +125,11 @@ QTestState *qtest_init(const char *extra_args)
     sock = init_socket(socket_path);
     qmpsock = init_socket(qmp_socket_path);
 
+    setsockopt(sock, SOL_SOCKET, SO_RCVTIMEO, (void *)&socket_timeout,
+               sizeof(socket_timeout));
+    setsockopt(qmpsock, SOL_SOCKET, SO_RCVTIMEO, (void *)&socket_timeout,
+               sizeof(socket_timeout));
+
     /* Catch SIGABRT to clean up on g_assert() failure */
     sigact = (struct sigaction){
         .sa_handler = sigabrt_handler,
@@ -147,7 +154,9 @@ QTestState *qtest_init(const char *extra_args)
     }
 
     s->fd = socket_accept(sock);
-    s->qmp_fd = socket_accept(qmpsock);
+    if (s->fd >= 0) {
+        s->qmp_fd = socket_accept(qmpsock);
+    }
     unlink(socket_path);
     unlink(qmp_socket_path);
     g_free(socket_path);
@@ -158,9 +167,11 @@ QTestState *qtest_init(const char *extra_args)
         s->irq_level[i] = false;
     }
 
-    /* Read the QMP greeting and then do the handshake */
-    qtest_qmp_discard_response(s, "");
-    qtest_qmp_discard_response(s, "{ 'execute': 'qmp_capabilities' }");
+    if (qtest_state_valid(s)) {
+        /* Read the QMP greeting and then do the handshake */
+        qtest_qmp_discard_response(s, "");
+        qtest_qmp_discard_response(s, "{ 'execute': 'qmp_capabilities' }");
+    }
 
     if (getenv("QTEST_STOP")) {
         kill(s->qemu_pid, SIGSTOP);
@@ -169,6 +180,11 @@ QTestState *qtest_init(const char *extra_args)
     return s;
 }
 
+bool qtest_state_valid(QTestState *s)
+{
+    return (s->fd >= 0) && (s->qmp_fd >= 0);
+}
+
 void qtest_quit(QTestState *s)
 {
     sigaction(SIGABRT, &s->sigact_old, NULL);
diff --git a/tests/libqtest.h b/tests/libqtest.h
index 9deebdc..39a37b1 100644
--- a/tests/libqtest.h
+++ b/tests/libqtest.h
@@ -45,6 +45,14 @@ QTestState *qtest_init(const char *extra_args);
 void qtest_quit(QTestState *s);
 
 /**
+ * qtest_state_valid:
+ * @state: #QTestState instance to check
+ *
+ * Returns: True if qtest was initialized successfully
+ */
+bool qtest_state_valid(QTestState *s);
+
+/**
  * qtest_qmp_discard_response:
  * @s: #QTestState instance to operate on.
  * @fmt...: QMP message to send to qemu
-- 
1.8.3.1