diff mbox series

[Bug,1905651,NEW] Tests cannot call g_error

Message ID 160635886967.28413.180075874214780604.malonedeb@chaenomeles.canonical.com
State New
Headers show
Series [Bug,1905651,NEW] Tests cannot call g_error | expand

Commit Message

Doug Evans Nov. 26, 2020, 2:47 a.m. UTC
Public bug reported:

I stumbled on this writing a new test, using tests/qtest/e1000e-test.c
as a template.

g_error() causes SIGTRAP, not SIGABRT, and thus the abort handler doesn't get run.
This in turn means qemu is not killed, which hangs the test because the tap-driver.pl script hangs waiting for more input.
There are a few tests that call g_error().

The SIGABRT handler explicitly kills qemu, e.g.:

    qtest_add_abrt_handler(kill_qemu_hook_func, s);


But not unexpectedly there's no such handler for SIGTRAP.

Apply this patch to trigger a repro:


make check-qtest-i386

check-qtest-i386 will take awhile. To repro faster:

$ grep qtest-i386/qos-test Makefile.mtest
.test.name.229 := qtest-i386/qos-test
$ make run-test-229
Running test qtest-i386/qos-test
** Message: 18:40:49.821: Test g_error hang ...

** (tests/qtest/qos-test:3820728): ERROR **: 18:40:49.821: Pretend something timed out
ERROR qtest-i386/qos-test - Bail out! FATAL-ERROR: Pretend something timed out

At this point things are hung because tap-driver.pl is still waiting for
input because qemu is still running.

** Affects: qemu
     Importance: Undecided
         Status: New


Doug Evans Nov. 26, 2020, 4 a.m. UTC | #1
An alternative is of course to allow g_error to be called.
One might restrict tests to not call it, but it might be impractical to impose that on all code that goes into a test.
One will need to find a way to get glib to not call G_BREAKPOINT for this case.

  if (debugger_present && breakpoint)
    g_abort ();

Doug Evans Nov. 27, 2020, 5:11 a.m. UTC | #2
I don't know QEMU that well yet, but the following question arises: Why
can't QEMU be driven in a way that allows it to see that its controlling
parent has died -> causing QEMU to terminate as well. That way the test
doesn't need to care how it dies (e.g., we don't want a segfault to hang
testing; and nor do we, I think, want to install signal handlers for
every possible signal).
diff mbox series


diff --git a/tests/qtest/e1000e-test.c b/tests/qtest/e1000e-test.c
index fc226fdfeb..e83ace1b5c 100644
--- a/tests/qtest/e1000e-test.c
+++ b/tests/qtest/e1000e-test.c
@@ -87,6 +87,9 @@  static void e1000e_send_verify(QE1000E *d, int *test_sockets, QGuestAllocator *a
     /* Wait for TX WB interrupt */
     e1000e_wait_isr(d, E1000E_TX0_MSG_ID);

+    g_message("Test g_error hang ...");
+    g_error("Pretend something timed out");
     /* Check DD bit */
     g_assert_cmphex(le32_to_cpu(descr.upper.data) & dsta_dd, ==, dsta_dd);