* [Qemu-devel] [PATCH 1/2] Revert "qtest: Fix crash if SIGABRT during qtest_init()"
2014-03-27 14:09 [Qemu-devel] [PATCH 0/2] qtest: fix SIGABRT handler for multiple qtest instances Stefan Hajnoczi
@ 2014-03-27 14:09 ` Stefan Hajnoczi
2014-03-27 14:09 ` [Qemu-devel] [PATCH 2/2] qtest: keep list of qtest instances for SIGABRT handler Stefan Hajnoczi
2014-03-27 14:30 ` [Qemu-devel] [PATCH 0/2] qtest: fix SIGABRT handler for multiple qtest instances Marcel Apfelbaum
2 siblings, 0 replies; 5+ messages in thread
From: Stefan Hajnoczi @ 2014-03-27 14:09 UTC (permalink / raw)
To: qemu-devel
Cc: Marcel Apfelbaum, Markus Armbruster, Stefan Hajnoczi,
Andreas Faerber
It turns out there are test cases that use multiple libqtest instances.
We cannot use a global qtest instance in the SIGABRT handler.
This reverts commit cb201b4872f16dfbce63f8648b2584631e2e965f.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
tests/libqtest.c | 3 +--
tests/libqtest.h | 4 +++-
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/tests/libqtest.c b/tests/libqtest.c
index b03b57a..2b90e4a 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -120,7 +120,7 @@ QTestState *qtest_init(const char *extra_args)
qemu_binary = getenv("QTEST_QEMU_BINARY");
g_assert(qemu_binary != NULL);
- global_qtest = s = g_malloc(sizeof(*s));
+ s = g_malloc(sizeof(*s));
socket_path = g_strdup_printf("/tmp/qtest-%d.sock", getpid());
qmp_socket_path = g_strdup_printf("/tmp/qtest-%d.qmp", getpid());
@@ -181,7 +181,6 @@ QTestState *qtest_init(const char *extra_args)
void qtest_quit(QTestState *s)
{
sigaction(SIGABRT, &s->sigact_old, NULL);
- global_qtest = NULL;
kill_qemu(s);
close(s->fd);
diff --git a/tests/libqtest.h b/tests/libqtest.h
index 27a58fd..8268c09 100644
--- a/tests/libqtest.h
+++ b/tests/libqtest.h
@@ -335,7 +335,8 @@ void qtest_add_func(const char *str, void (*fn));
*/
static inline QTestState *qtest_start(const char *args)
{
- return qtest_init(args);
+ global_qtest = qtest_init(args);
+ return global_qtest;
}
/**
@@ -346,6 +347,7 @@ static inline QTestState *qtest_start(const char *args)
static inline void qtest_end(void)
{
qtest_quit(global_qtest);
+ global_qtest = NULL;
}
/**
--
1.8.5.3
^ permalink raw reply related [flat|nested] 5+ messages in thread* [Qemu-devel] [PATCH 2/2] qtest: keep list of qtest instances for SIGABRT handler
2014-03-27 14:09 [Qemu-devel] [PATCH 0/2] qtest: fix SIGABRT handler for multiple qtest instances Stefan Hajnoczi
2014-03-27 14:09 ` [Qemu-devel] [PATCH 1/2] Revert "qtest: Fix crash if SIGABRT during qtest_init()" Stefan Hajnoczi
@ 2014-03-27 14:09 ` Stefan Hajnoczi
2014-03-27 14:30 ` [Qemu-devel] [PATCH 0/2] qtest: fix SIGABRT handler for multiple qtest instances Marcel Apfelbaum
2 siblings, 0 replies; 5+ messages in thread
From: Stefan Hajnoczi @ 2014-03-27 14:09 UTC (permalink / raw)
To: qemu-devel
Cc: Marcel Apfelbaum, Markus Armbruster, Stefan Hajnoczi,
Andreas Faerber
Keep track of active qtest instances so we can kill them when the test
aborts. This ensures no QEMU processes are left running after test
failure.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
tests/libqtest.c | 47 +++++++++++++++++++++++++++++++++++++----------
1 file changed, 37 insertions(+), 10 deletions(-)
diff --git a/tests/libqtest.c b/tests/libqtest.c
index 2b90e4a..d9e3a33 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -48,6 +48,9 @@ struct QTestState
struct sigaction sigact_old; /* restored on exit */
};
+static GList *qtest_instances;
+static struct sigaction sigact_old;
+
#define g_assert_no_errno(ret) do { \
g_assert_cmpint(ret, !=, -1); \
} while (0)
@@ -104,7 +107,28 @@ static void kill_qemu(QTestState *s)
static void sigabrt_handler(int signo)
{
- kill_qemu(global_qtest);
+ GList *elem;
+ for (elem = qtest_instances; elem; elem = elem->next) {
+ kill_qemu(elem->data);
+ }
+}
+
+static void setup_sigabrt_handler(void)
+{
+ struct sigaction sigact;
+
+ /* Catch SIGABRT to clean up on g_assert() failure */
+ sigact = (struct sigaction){
+ .sa_handler = sigabrt_handler,
+ .sa_flags = SA_RESETHAND,
+ };
+ sigemptyset(&sigact.sa_mask);
+ sigaction(SIGABRT, &sigact, &sigact_old);
+}
+
+static void cleanup_sigabrt_handler(void)
+{
+ sigaction(SIGABRT, &sigact_old, NULL);
}
QTestState *qtest_init(const char *extra_args)
@@ -115,7 +139,6 @@ QTestState *qtest_init(const char *extra_args)
gchar *qmp_socket_path;
gchar *command;
const char *qemu_binary;
- struct sigaction sigact;
qemu_binary = getenv("QTEST_QEMU_BINARY");
g_assert(qemu_binary != NULL);
@@ -128,13 +151,12 @@ QTestState *qtest_init(const char *extra_args)
sock = init_socket(socket_path);
qmpsock = init_socket(qmp_socket_path);
- /* Catch SIGABRT to clean up on g_assert() failure */
- sigact = (struct sigaction){
- .sa_handler = sigabrt_handler,
- .sa_flags = SA_RESETHAND,
- };
- sigemptyset(&sigact.sa_mask);
- sigaction(SIGABRT, &sigact, &s->sigact_old);
+ /* Only install SIGABRT handler once */
+ if (!qtest_instances) {
+ setup_sigabrt_handler();
+ }
+
+ qtest_instances = g_list_prepend(qtest_instances, s);
s->qemu_pid = fork();
if (s->qemu_pid == 0) {
@@ -180,7 +202,12 @@ QTestState *qtest_init(const char *extra_args)
void qtest_quit(QTestState *s)
{
- sigaction(SIGABRT, &s->sigact_old, NULL);
+ /* Uninstall SIGABRT handler on last instance */
+ if (qtest_instances && !qtest_instances->next) {
+ cleanup_sigabrt_handler();
+ }
+
+ qtest_instances = g_list_remove(qtest_instances, s);
kill_qemu(s);
close(s->fd);
--
1.8.5.3
^ permalink raw reply related [flat|nested] 5+ messages in thread