catch signals
PATCH 1/1 catch signals
Hi,
This patch adds a signal handler to qemu, so fatal signals don't kill off qemu. Instead a shutdown request is issued, like it is done when you close the SDL window. qemu cleans up nicely then, cespecially it calls all atexit handlers.
please apply,
Gerd
Signature · 1 line
http://kraxel.fedorapeople.org/xenner/
Quoted 1 line
From
886775f5229d83d73f3d162b919ea68b386b2a60Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann <[email protected]>
Date: Tue, 5 Aug 2008 17:53:36 +0200
Subject: [PATCH] catch signals, so atexit handlers are called correctly.
--- curses.c | 2 -- sdl.c | 5 ----- vl.c | 39 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 39 insertions(+), 7 deletions(-)
curses.c +0 -2
@@ -346,8 +346,6 @@ void curses_display_init(DisplayState *ds, int full_screen)atexit (curses_atexit);#ifndef _WIN32 -signal (SIGINT ,SIG_DFL ); -signal (SIGQUIT ,SIG_DFL );#if defined(SIGWINCH ) && defined(KEY_RESIZE )/* some curses implementations provide a handler, but we * want to be sure this is handled regardless of the library */
sdl.c +0 -5
@@ -636,11 +636,6 @@ void sdl_display_init(DisplayState *ds, int full_screen, int no_frame)fprintf (stderr,"Could not initialize SDL - exiting\n" );exit (1 ); } -#ifndef _WIN32 -/* NOTE: we still want Ctrl-C to work, so we undo the SDL redirections */ -signal (SIGINT ,SIG_DFL ); -signal (SIGQUIT ,SIG_DFL ); -#endif ds->dpy_update = sdl_update; ds->dpy_resize = sdl_resize;
vl.c +39 -0
@@ -8141,6 +8141,40 @@ static BOOL WINAPI qemu_ctrl_handler(DWORD type)#define MAX_NET_CLIENTS 32 +#ifndef _WIN32 +static void termsig_handler (int signal) +{ +switch (signal) { +case SIGSEGV : +case SIGBUS : +/* returning from signal handler most likely isn't going to work */ +fprintf (stderr,"qemu: got signal %d (%s), taking emergency exit\n" , + signal,strsignal (signal)); +exit (1 ); +break ; +default : +qemu_system_shutdown_request (); +vm_start ();/* In case we're paused */ +break ; + } +} + +static void termsig_setup (void ) +{ +struct sigaction act; + +memset (&act,0 ,sizeof (act)); + act.sa_flags =SA_RESETHAND ; + act.sa_handler = termsig_handler; + +sigaction (SIGINT , &act,NULL ); +sigaction (SIGTERM , &act,NULL ); +sigaction (SIGQUIT , &act,NULL ); +sigaction (SIGSEGV , &act,NULL ); +sigaction (SIGBUS , &act,NULL ); +} +#endif +int main (int argc,char **argv) {#ifdef CONFIG_GDBSTUB
@@ -9031,6 +9065,11 @@ int main(int argc, char **argv)#endif } +#ifndef _WIN32 +/* must be after terminal init, SDL changes signal handlers */ +termsig_setup (); +#endif +/* Maintain compatibility with multiple stdio monitors */ has_monitor =0 ;
Signature · 1 line
1.5.5.1
vl.c static BOOL WINAPI qemu_ctrl_handler(DWORD type) · line 8174
Quoted 5 lines · Gerd Hoffmann, le Tue 05 Aug 2008 18:09:39 +0200, a écrit :
vl.c +5
-0+sigaction (SIGINT , &act,NULL ); +sigaction (SIGTERM , &act,NULL ); +sigaction (SIGQUIT , &act,NULL ); +sigaction (SIGSEGV , &act,NULL ); +sigaction (SIGBUS , &act,NULL );
+ SIGHUP and SIGPIPE?
Samuel
Quoted 8 lines · Samuel Thibault
vl.c static BOOL WINAPI qemu_ctrl_handler(DWORD type) · line 8174
Quoted 5 lines · Gerd Hoffmann, le Tue 05 Aug 2008 18:09:39 +0200, a écrit :
vl.c +5
-0+sigaction (SIGINT , &act,NULL ); +sigaction (SIGTERM , &act,NULL ); +sigaction (SIGQUIT , &act,NULL ); +sigaction (SIGSEGV , &act,NULL ); +sigaction (SIGBUS , &act,NULL );+ SIGHUP and SIGPIPE?
SIGHUP yes. SIGPIPE is handled elsewhere in the code. Also guest shutdown isn't the thing we want to do on SIGPIPE ...
cheers,
Gerd
Signature · 1 line
http://kraxel.fedorapeople.org/xenner/
vl.c static BOOL WINAPI qemu_ctrl_handler(DWORD type) · line 8153
Quoted 11 lines · Gerd Hoffmann
vl.c +10
-0+#ifndef _WIN32 +static void termsig_handler (int signal) +{ +switch (signal) { +case SIGSEGV : +case SIGBUS : +/* returning from signal handler most likely isn't going to work */ +fprintf (stderr,"qemu: got signal %d (%s), taking emergency exit\n" , + signal,strsignal (signal)); +exit (1 );
Neither of these functions are on the POSIX async-signal-safe list, so their use from signal handlers is not a good idea.
'man 7 signal' will detail the safe functions that can be used from signal handlers. http://kerneltrap.org/man/linux/man7/signal.7
vl.c static BOOL WINAPI qemu_ctrl_handler(DWORD type) · line 8156
Quoted 3 lines
vl.c +3
-0+break ; +default : +qemu_system_shutdown_request ();
This is ok it merely sets a flag
Quoted 1 line
+
vm_start(); /* In case we're paused */
I've not checked all the functions this calls to see if they are safe, but I have a feeling this won't be safe in general because the vm_state_notify() will invoke a number of callbacks whose content we can't predict.
I think rather than trying todo anything in the signal handler, it is safest to just set a flag and have its state checked and acted upon in the main loop.
Daniel
Signature · 4 lines
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
Quoted 3 lines · Daniel P. Berrange, le Tue 05 Aug 2008 17:35:19 +0100, a écrit :
I think rather than trying todo anything in the signal handler, it is safest to just set a flag and have its state checked and acted upon in the main loop.
We can not do that for SIGSEGV/SIGBUS, however.
Samuel
Quoted 6 lines · Samuel Thibault
Quoted 3 lines · Daniel P. Berrange, le Tue 05 Aug 2008 17:35:19 +0100, a écrit :
I think rather than trying todo anything in the signal handler, it is safest to just set a flag and have its state checked and acted upon in the main loop.
We can not do that for SIGSEGV/SIGBUS, however.
We shouldn't be trying todo anything for SEGV/BUS. It is basically game over at that point - you've no chance of orderly shutdown. Only QUIT, INT, TERM, HUP should be trying todo graceful shutdown, because those don't imply your process is corrupting its memory/ doing bad stuff.
Daniel
Signature · 4 lines
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
Quoted 6 lines · Daniel P. Berrange
Quoted 5 lines · Samuel Thibault
Quoted 3 lines · Daniel P. Berrange, le Tue 05 Aug 2008 17:35:19 +0100, a écrit :
I think rather than trying todo anything in the signal handler, it is safest to just set a flag and have its state checked and acted upon in the main loop.
Quoted 6 lines
Quoted 1 line
We can not do that for SIGSEGV/SIGBUS, however.
We shouldn't be trying todo anything for SEGV/BUS. It is basically game over at that point - you've no chance of orderly shutdown. Only QUIT, INT, TERM, HUP should be trying todo graceful shutdown, because those don't imply your process is corrupting its memory/ doing bad stuff.
If you can detect _where_ is triggering SIGSEGV/SIGBUS, and it's in generated code, then you've got a good chance of treating it as a special kind of trap and aborting cleanly.
Also, while orderly shutdown is not possible, some subsystems could register an "emergency shutdown" hook which is async-signal-safe.
Logging would have to be disabled during them, and they'd just do things to tidy up, e.g. unlinking temporary files, writing memory/disk state to an emergency snapshot if that's possible, complete pending writes to disk image formats if that makes them safer.
I do something like that in a program of mine: register cleanup handlers, and some of them say "can be called from an emergency signal".
Finally, another reason to have emergency cleanups is when you send SIGTERM but it's wedged, and the main loop isn't responding. After a while, you want to kill the thing, and a bit of signal-safe cleanup then is good.
-- Jamie
Quoted 15 lines · Daniel P. Berrange
vl.c static BOOL WINAPI qemu_ctrl_handler(DWORD type) · line 8153
Quoted 11 lines · Gerd Hoffmann
vl.c +10
-0+#ifndef _WIN32 +static void termsig_handler (int signal) +{ +switch (signal) { +case SIGSEGV : +case SIGBUS : +/* returning from signal handler most likely isn't going to work */ +fprintf (stderr,"qemu: got signal %d (%s), taking emergency exit\n" , + signal,strsignal (signal)); +exit (1 );Neither of these functions are on the POSIX async-signal-safe list, so their use from signal handlers is not a good idea.
We are in dead water already and also don't plan to ever return from the
signal handler. Is it really a problem then?
I want to be able to do cleanups (well, at least attempt) even in case of a segfault. If exit() + atexit handlers isn't going to fill the bill we'll have to create some signal-save emergency cleanup handlers.
Quoted 5 lines
Quoted 1 line
+
vm_start(); /* In case we're paused */I think rather than trying todo anything in the signal handler, it is safest to just set a flag and have its state checked and acted upon in the main loop.
Isn't going to fly for SIGSEGV and SIGBUS. Agree for the other signals.
I'll go check whenever we can handle stopped cpus some other way (guess I just need an additional check in the main_loop() function).
cheers,
Gerd
Signature · 1 line
http://kraxel.fedorapeople.org/xenner/
Quoted 17 lines · Gerd Hoffmann
Quoted 15 lines · Daniel P. Berrange
vl.c static BOOL WINAPI qemu_ctrl_handler(DWORD type) · line 8153
Quoted 11 lines · Gerd Hoffmann
vl.c +10
-0+#ifndef _WIN32 +static void termsig_handler (int signal) +{ +switch (signal) { +case SIGSEGV : +case SIGBUS : +/* returning from signal handler most likely isn't going to work */ +fprintf (stderr,"qemu: got signal %d (%s), taking emergency exit\n" , + signal,strsignal (signal)); +exit (1 );Neither of these functions are on the POSIX async-signal-safe list, so their use from signal handlers is not a good idea.
Quoted 2 lines
We are in dead water already and also don't plan to ever return from the
signal handler. Is it really a problem then?
Yes, because if you further corrupt state in the signal handler it makes debugging what went wrong even harder than it already is. In SEGV/BUS case you really want to be able to get a decent core dump to analyse, so the utmost care should be taken to avoid further messing up state.
The STDIO libs are not re-entrant safe so if the original code was in a STDIO function, and a SEGV comes in on another thread, the signal handler will likely deadlock in a mutex. 'exit' will call into STDIO to flush buffers, so will suffer the same problem.
Quoted 3 lines
I want to be able to do cleanups (well, at least attempt) even in case of a segfault. If
exit()+ atexit handlers isn't going to fill the bill we'll have to create some signal-save emergency cleanup handlers.
You could use 'sigaltstack' to setup an alternate pre-allocated stack and carefully code the handler so it only uses pre-allocated memory, or mem from the alt-stack & signal-safe functions. If you need something really complex, you could 'fork' a cleanup program and communicate any info to it using a pipe.
Regards,
Daniel
Signature · 4 lines
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
Gerd Hoffmann writes ("[Qemu-devel] [PATCH] catch signals"):
Quoted 4 lines
This patch adds a signal handler to qemu, so fatal signals don't kill off qemu. Instead a shutdown request is issued, like it is done when you close the SDL window. qemu cleans up nicely then, cespecially it calls all atexit handlers.
...
vl.c static BOOL WINAPI qemu_ctrl_handler(DWORD type) · line 8174
Quoted 5 lines
vl.c +5
-0+sigaction (SIGINT , &act,NULL ); +sigaction (SIGTERM , &act,NULL ); +sigaction (SIGQUIT , &act,NULL ); +sigaction (SIGSEGV , &act,NULL ); +sigaction (SIGBUS , &act,NULL );
This is wrong, I think. It's fine to take this action for INT and TERM.
It's very wrong to do so for SEGV and BUS. QUIT is used for debugging so should be left set to DFL too.
Ian.
Quoted 1 line · Ian Jackson
It's very wrong to do so for SEGV and BUS.
Why
Note there is a newer version which doesn't call signal-unsafe functions
in the signal handler any more.
cheers,
Gerd
Signature · 1 line
http://kraxel.fedorapeople.org/xenner/