Re: [Qemu-devel] [PATCH 2/2] qtest: fix crash if SIGABRT during qtest_init()

[Stefan Hajnoczi]

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