Re: [Qemu-devel] [PATCH 3/3] qtest: kill QEMU process on g_assert() failure

2014-02-18 Thread Stefan Hajnoczi
On Mon, Feb 17, 2014 at 06:00:03PM +0100, Markus Armbruster wrote: Paolo Bonzini pbonz...@redhat.com writes: Il 17/02/2014 16:44, Stefan Hajnoczi ha scritto: } +static void sigabrt_handler(int signo) +{ +qtest_end(); +} + void qtest_quit(QTestState *s) { int

Re: [Qemu-devel] [PATCH 3/3] qtest: kill QEMU process on g_assert() failure

2014-02-18 Thread Stefan Hajnoczi
On Mon, Feb 17, 2014 at 05:49:31PM +0100, Markus Armbruster wrote: Stefan Hajnoczi stefa...@redhat.com writes: +/* Catch SIGABRT to clean up on g_assert() failure */ +sigact = (struct sigaction){ +.sa_handler = sigabrt_handler, +.sa_flags = SA_RESETHAND, +};

Re: [Qemu-devel] [PATCH 3/3] qtest: kill QEMU process on g_assert() failure

2014-02-18 Thread Markus Armbruster
Stefan Hajnoczi stefa...@gmail.com writes: On Mon, Feb 17, 2014 at 05:49:31PM +0100, Markus Armbruster wrote: Stefan Hajnoczi stefa...@redhat.com writes: +/* Catch SIGABRT to clean up on g_assert() failure */ +sigact = (struct sigaction){ +.sa_handler = sigabrt_handler,

Re: [Qemu-devel] [PATCH 3/3] qtest: kill QEMU process on g_assert() failure

2014-02-18 Thread Markus Armbruster
Markus Armbruster arm...@redhat.com writes: Stefan Hajnoczi stefa...@redhat.com writes: The QEMU process stays running if the test case fails. This patch fixes the leak by installing a SIGABRT signal handler which invokes qtest_end(). In order to make that work for assertion failures

Re: [Qemu-devel] [PATCH 3/3] qtest: kill QEMU process on g_assert() failure

2014-02-18 Thread Markus Armbruster
Stefan Hajnoczi stefa...@gmail.com writes: On Mon, Feb 17, 2014 at 06:00:03PM +0100, Markus Armbruster wrote: Paolo Bonzini pbonz...@redhat.com writes: Il 17/02/2014 16:44, Stefan Hajnoczi ha scritto: } +static void sigabrt_handler(int signo) +{ +qtest_end(); +} +

Re: [Qemu-devel] [PATCH 3/3] qtest: kill QEMU process on g_assert() failure

2014-02-18 Thread Paolo Bonzini
Il 18/02/2014 10:05, Stefan Hajnoczi ha scritto: SIGABRT is normally synchronous enough: it's sent by abort(). But of course, nothing stops the user from kill -ABRT. Or GLib from calling abort() in some place where an attempt to reenter it crashes burns. Not sure I'd care, but I'm pretty

Re: [Qemu-devel] [PATCH 3/3] qtest: kill QEMU process on g_assert() failure

2014-02-18 Thread Daniel P. Berrange
On Tue, Feb 18, 2014 at 11:07:53AM +0100, Paolo Bonzini wrote: Il 18/02/2014 10:05, Stefan Hajnoczi ha scritto: SIGABRT is normally synchronous enough: it's sent by abort(). But of course, nothing stops the user from kill -ABRT. Or GLib from calling abort() in some place where an attempt

Re: [Qemu-devel] [PATCH 3/3] qtest: kill QEMU process on g_assert() failure

2014-02-18 Thread Paolo Bonzini
Il 18/02/2014 11:17, Daniel P. Berrange ha scritto: Yes, SIGABRT is synchronous for all purposes. So the only danger is that g_string_free() or g_free() could fail while we're in g_assert(false). But they don't, which makes sense because they are totally unrelated to g_assert() and

Re: [Qemu-devel] [PATCH 3/3] qtest: kill QEMU process on g_assert() failure

2014-02-18 Thread Paolo Bonzini
Il 18/02/2014 11:05, Markus Armbruster ha scritto: Yes, SIGABRT is synchronous for all purposes. So the only danger is that g_string_free() or g_free() could fail while we're in g_assert(false). But they don't, which makes sense because they are totally unrelated to g_assert() and

Re: [Qemu-devel] [PATCH 3/3] qtest: kill QEMU process on g_assert() failure

2014-02-18 Thread Markus Armbruster
Paolo Bonzini pbonz...@redhat.com writes: Il 18/02/2014 11:05, Markus Armbruster ha scritto: Yes, SIGABRT is synchronous for all purposes. So the only danger is that g_string_free() or g_free() could fail while we're in g_assert(false). But they don't, which makes sense because they are

Re: [Qemu-devel] [PATCH 3/3] qtest: kill QEMU process on g_assert() failure

2014-02-18 Thread Stefan Hajnoczi
On Tue, Feb 18, 2014 at 11:02:52AM +0100, Markus Armbruster wrote: Markus Armbruster arm...@redhat.com writes: Stefan Hajnoczi stefa...@redhat.com writes: The QEMU process stays running if the test case fails. This patch fixes the leak by installing a SIGABRT signal handler which

Re: [Qemu-devel] [PATCH 3/3] qtest: kill QEMU process on g_assert() failure

2014-02-18 Thread Stefan Hajnoczi
On Tue, Feb 18, 2014 at 10:55:56AM +0100, Markus Armbruster wrote: Stefan Hajnoczi stefa...@gmail.com writes: On Mon, Feb 17, 2014 at 05:49:31PM +0100, Markus Armbruster wrote: Stefan Hajnoczi stefa...@redhat.com writes: if (s-qemu_pid != -1) { kill(s-qemu_pid, SIGTERM);

Re: [Qemu-devel] [PATCH 3/3] qtest: kill QEMU process on g_assert() failure

2014-02-18 Thread Stefan Hajnoczi
On Tue, Feb 18, 2014 at 11:43:10AM +0100, Markus Armbruster wrote: Paolo Bonzini pbonz...@redhat.com writes: Il 18/02/2014 11:05, Markus Armbruster ha scritto: Yes, SIGABRT is synchronous for all purposes. So the only danger is that g_string_free() or g_free() could fail while we're in

Re: [Qemu-devel] [PATCH 3/3] qtest: kill QEMU process on g_assert() failure

2014-02-18 Thread Markus Armbruster
Stefan Hajnoczi stefa...@gmail.com writes: On Tue, Feb 18, 2014 at 11:02:52AM +0100, Markus Armbruster wrote: Markus Armbruster arm...@redhat.com writes: Stefan Hajnoczi stefa...@redhat.com writes: The QEMU process stays running if the test case fails. This patch fixes the leak by

Re: [Qemu-devel] [PATCH 3/3] qtest: kill QEMU process on g_assert() failure

2014-02-18 Thread Peter Maydell
On 18 February 2014 09:17, Stefan Hajnoczi stefa...@gmail.com wrote: When I drive on the highway I stay on the lanes but I guess I could just slide along the side barriers :). It's a style issue but close(-1) annoys me in strace so I try to avoid doing it. As an aside, ever try stracing

Re: [Qemu-devel] [PATCH 3/3] qtest: kill QEMU process on g_assert() failure

2014-02-17 Thread Paolo Bonzini
Il 17/02/2014 16:44, Stefan Hajnoczi ha scritto: } +static void sigabrt_handler(int signo) +{ +qtest_end(); +} + void qtest_quit(QTestState *s) { int status; if (s-qemu_pid != -1) { kill(s-qemu_pid, SIGTERM); waitpid(s-qemu_pid, status, 0); }

Re: [Qemu-devel] [PATCH 3/3] qtest: kill QEMU process on g_assert() failure

2014-02-17 Thread Markus Armbruster
Stefan Hajnoczi stefa...@redhat.com writes: The QEMU process stays running if the test case fails. This patch fixes the leak by installing a SIGABRT signal handler which invokes qtest_end(). In order to make that work for assertion failures during qtest_init(), we need to initialize

Re: [Qemu-devel] [PATCH 3/3] qtest: kill QEMU process on g_assert() failure

2014-02-17 Thread Paolo Bonzini
Il 17/02/2014 17:49, Markus Armbruster ha scritto: Assumes zero-initialization has the same effect as sigemptyset(sigact.sa_mask). Quoting POSIX: The implementation of the sigemptyset() (or sigfillset()) function could quite trivially clear (or set) all the bits in the signal set.

Re: [Qemu-devel] [PATCH 3/3] qtest: kill QEMU process on g_assert() failure

2014-02-17 Thread Markus Armbruster
Paolo Bonzini pbonz...@redhat.com writes: Il 17/02/2014 16:44, Stefan Hajnoczi ha scritto: } +static void sigabrt_handler(int signo) +{ +qtest_end(); +} + void qtest_quit(QTestState *s) { int status; if (s-qemu_pid != -1) { kill(s-qemu_pid, SIGTERM);