On Thu, Mar 13, 2014 at 01:07:05PM +0200, Marcel Apfelbaum wrote: > On Thu, 2014-03-13 at 10:41 +0100, Stefan Hajnoczi wrote: > > If an assertion fails during qtest_init() the SIGABRT handler is > > invoked. This is the correct behavior since we need to kill the QEMU > > process to avoid leaking it when the test dies. > > > > The global_qtest pointer used by the SIGABRT handler is currently only > > assigned after qtest_init() returns. This results in a segfault if an > > assertion failure occurs during qtest_init(). > > > > Move global_qtest assignment inside qtest_init(). Not pretty but let's > > face it - the signal handler dependeds on global state. > Looks OK to me, but it seems that it is symmetrical with my > patch: Mine checked for global_qtest that is not null (not hiding anything :() > and yours increases global_qtest's scope. > > I understand why you preferred it this way, to ensure the QEMU instance > is killed, but as I stated before, from my point of view > qtest_init aborted <=> the qemu machine exited because of on error. > (but I might be wrong)
Think about this case: If we hit an assertion failure in qtest_init() because of socket errors (e.g. QEMU ran for a little bit but closed the socket while we were negotiating), then we *do* need to kill the QEMU process. Stefan
