Patchwork report that QEMU process was killed by a signal

login
register
mail settings
Submitter Gleb Natapov
Date March 14, 2011, 12:58 p.m.
Message ID <20110314125855.GP10151@redhat.com>
Download mbox | patch
Permalink /patch/86734/
State New
Headers show

Comments

Gleb Natapov - March 14, 2011, 12:58 p.m.
Currently when rogue script kills QEMU process (using TERM/INT/HUP
signal) it looks indistinguishable from system shutdown. Lets report
that QMEU was killed and leave some clues about the killed identity.

Signed-off-by: Gleb Natapov <gleb@redhat.com>
--
			Gleb.
Daniel P. Berrange - March 14, 2011, 1:07 p.m.
On Mon, Mar 14, 2011 at 02:58:55PM +0200, Gleb Natapov wrote:
> Currently when rogue script kills QEMU process (using TERM/INT/HUP
> signal) it looks indistinguishable from system shutdown. Lets report
> that QMEU was killed and leave some clues about the killed identity.

Good idea, but....

> 
> Signed-off-by: Gleb Natapov <gleb@redhat.com>
> diff --git a/os-posix.c b/os-posix.c
> index 38c29d1..7e175e8 100644
> --- a/os-posix.c
> +++ b/os-posix.c
> @@ -61,8 +61,9 @@ void os_setup_early_signal_handling(void)
>      sigaction(SIGPIPE, &act, NULL);
>  }
>  
> -static void termsig_handler(int signal)
> +static void termsig_handler(int signal, siginfo_t *info, void *c)
>  {
> +    fprintf(stderr, "Got signal %d from pid %d\n", info->si_signo, info->si_pid);
>      qemu_system_shutdown_request();

...fprintf() isn't async signal safe. It could deadlock the process
if it malloc()s while doing the arg formatting & the interrupted
thread was already holding a malloc() lock, or indeed if the stdio
impl itself has internal locks which are held. So this data needs
to be manually output using just write()

Regards,
Daniel
Gleb Natapov - March 14, 2011, 1:11 p.m.
On Mon, Mar 14, 2011 at 01:07:40PM +0000, Daniel P. Berrange wrote:
> On Mon, Mar 14, 2011 at 02:58:55PM +0200, Gleb Natapov wrote:
> > Currently when rogue script kills QEMU process (using TERM/INT/HUP
> > signal) it looks indistinguishable from system shutdown. Lets report
> > that QMEU was killed and leave some clues about the killed identity.
> 
> Good idea, but....
> 
> > 
> > Signed-off-by: Gleb Natapov <gleb@redhat.com>
> > diff --git a/os-posix.c b/os-posix.c
> > index 38c29d1..7e175e8 100644
> > --- a/os-posix.c
> > +++ b/os-posix.c
> > @@ -61,8 +61,9 @@ void os_setup_early_signal_handling(void)
> >      sigaction(SIGPIPE, &act, NULL);
> >  }
> >  
> > -static void termsig_handler(int signal)
> > +static void termsig_handler(int signal, siginfo_t *info, void *c)
> >  {
> > +    fprintf(stderr, "Got signal %d from pid %d\n", info->si_signo, info->si_pid);
> >      qemu_system_shutdown_request();
> 
> ...fprintf() isn't async signal safe. It could deadlock the process
> if it malloc()s while doing the arg formatting & the interrupted
> thread was already holding a malloc() lock, or indeed if the stdio
> impl itself has internal locks which are held. So this data needs
> to be manually output using just write()
> 
Or we can save info->si_signo && info->si_pid in global variables and
print this when shutdown is performed by main loop. Will send updated
patch.
 
--
			Gleb.

Patch

diff --git a/os-posix.c b/os-posix.c
index 38c29d1..7e175e8 100644
--- a/os-posix.c
+++ b/os-posix.c
@@ -61,8 +61,9 @@  void os_setup_early_signal_handling(void)
     sigaction(SIGPIPE, &act, NULL);
 }
 
-static void termsig_handler(int signal)
+static void termsig_handler(int signal, siginfo_t *info, void *c)
 {
+    fprintf(stderr, "Got signal %d from pid %d\n", info->si_signo, info->si_pid);
     qemu_system_shutdown_request();
 }
 
@@ -76,7 +77,8 @@  void os_setup_signal_handling(void)
     struct sigaction act;
 
     memset(&act, 0, sizeof(act));
-    act.sa_handler = termsig_handler;
+    act.sa_sigaction = termsig_handler;
+    act.sa_flags = SA_SIGINFO;
     sigaction(SIGINT,  &act, NULL);
     sigaction(SIGHUP,  &act, NULL);
     sigaction(SIGTERM, &act, NULL);