report that QEMU process was killed by a signal

Currently when rogue script kills QEMU process (using TERM/INT/HUP signal) it looks indistinguishable from system shutdown. Lets report that QEMU was killed and leave some clues about the killer identity.

Signed-off-by Gleb Natapov <[email protected]>

---
v1->v2:
- print message from a main loop instead of signal handler
v2->v3
- use pid_t to store pid instead of int

os-posix.c +4 -3

index 38c29d1..36d937c 100644
@@ -61,9 +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)
 {
-    qemu_system_shutdown_request();
+    qemu_system_killed(info->si_signo, info->si_pid);
 }
 
 static void sigchld_handler(int signal)

@@ -76,7 +76,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);

sysemu.h +2 -0

index 0a83ab9..f868500 100644
@@ -66,6 +66,8 @@ void qemu_system_vmstop_request(int reason);
 int qemu_shutdown_requested(void);
 int qemu_reset_requested(void);
 int qemu_powerdown_requested(void);
+void qemu_system_killed(int signal, pid_t pid);
+void qemu_kill_report(void);
 extern qemu_irq qemu_system_powerdown;
 void qemu_system_reset(void);
 

vl.c +19 -2

index 5e007a7..000c845 100644
@@ -1213,7 +1213,8 @@ typedef struct QEMUResetEntry {
 static QTAILQ_HEAD(reset_handlers, QEMUResetEntry) reset_handlers =
     QTAILQ_HEAD_INITIALIZER(reset_handlers);
 static int reset_requested;
-static int shutdown_requested;
+static int shutdown_requested, shutdown_signal = -1;
+static pid_t shutdown_pid;
 static int powerdown_requested;
 static int debug_requested;
 static int vmstop_requested;

@@ -1225,6 +1226,15 @@ int qemu_shutdown_requested(void)
     return r;
 }
 
+void qemu_kill_report(void)
+{
+    if (shutdown_signal != -1) {
+        fprintf(stderr, "Got signal %d from pid %d\n",
+                         shutdown_signal, shutdown_pid);
+        shutdown_signal = -1;
+    }
+}
+
 int qemu_reset_requested(void)
 {
     int r = reset_requested;

@@ -1298,6 +1308,13 @@ void qemu_system_reset_request(void)
     qemu_notify_event();
 }
 
+void qemu_system_killed(int signal, pid_t pid)
+{
+    shutdown_signal = signal;
+    shutdown_pid = pid;
+    qemu_system_shutdown_request();
+}
+
 void qemu_system_shutdown_request(void)
 {
     shutdown_requested = 1;

@@ -1441,6 +1458,7 @@ static void main_loop(void)
             vm_stop(VMSTOP_DEBUG);
         }
         if (qemu_shutdown_requested()) {
+            qemu_kill_report();
             monitor_protocol_event(QEVENT_SHUTDOWN, NULL);
             if (no_shutdown) {
                 vm_stop(VMSTOP_SHUTDOWN);
--

Gleb.

Ping?

Quoted 1 line · Gleb Natapov

Ping?

Does not work:
INT:
Got signal 951049944 from pid 0
TERM:
Got signal -1553068904 from pid 0

HUP:
Got signal 1 from pid 16185
Even here the pid is not correct, it should be 3098.

Quoted 8 lines · Blue Swirl

Quoted 1 line · Gleb Natapov

Ping?

Does not work:
INT:
Got signal 951049944 from pid 0
TERM:
Got signal -1553068904 from pid 0

You use SDL correct? This is SDL problem and I fixed it in SDL upstream.

Quoted 3 lines

HUP:
Got signal 1 from pid 16185
Even here the pid is not correct, it should be 3098.

HUP should work. Why do you think that pid should be 3098? Bash has its own build in kill command IIRC.

Currently when rogue script kills QEMU process (using TERM/INT/HUP signal) it looks indistinguishable from system shutdown. Lets report that QEMU was killed and leave some clues about the killer identity.

Quoted 3 lines · Gleb Natapov

Currently when rogue script kills QEMU process (using TERM/INT/HUP signal) it looks indistinguishable from system shutdown. Lets report that QEMU was killed and leave some clues about the killer identity.

Unfortunately this patch causes qemu to segfault when killed via ^C (at least on my Ubuntu maverick system). This is because it registers a signal handler with sigaction, but then later the SDL library is initialised and it reinstalls our handler with plain old signal:

        ohandler = signal(SIGINT, SDL_HandleSIG);
        if ( ohandler != SIG_DFL )
                signal(SIGINT, ohandler);

This is clearly buggy but on the other hand SDL is pretty widely deployed and it's the default QEMU video output method, so I think we need to work around it :-(

The most straightforward fix is to get the signal number from argument one and not to bother printing the PID that killed us.

Quoted 3 lines · Gleb Natapov

Currently when rogue script kills QEMU process (using TERM/INT/HUP signal) it looks indistinguishable from system shutdown. Lets report that QEMU was killed and leave some clues about the killer identity.

Unfortunately this patch causes qemu to segfault when killed via ^C (at least on my Ubuntu maverick system). This is because it registers a signal handler with sigaction, but then later the SDL library is initialised and it reinstalls our handler with plain old signal:

         ohandler = signal(SIGINT, SDL_HandleSIG);
         if ( ohandler != SIG_DFL )
                 signal(SIGINT, ohandler);

This is clearly buggy but on the other hand SDL is pretty widely deployed and it's the default QEMU video output method, so I think we need to work around it :-(

The most straightforward fix is to get the signal number from argument one and not to bother printing the PID that killed us.

-- PMM

Quoted 18 lines · Peter Maydell

Quoted 3 lines · Gleb Natapov

Currently when rogue script kills QEMU process (using TERM/INT/HUP signal) it looks indistinguishable from system shutdown. Lets report that QEMU was killed and leave some clues about the killer identity.

Unfortunately this patch causes qemu to segfault when killed via ^C (at least on my Ubuntu maverick system). This is because it registers a signal handler with sigaction, but then later the SDL library is initialised and it reinstalls our handler with plain old signal:

        ohandler = signal(SIGINT, SDL_HandleSIG);
        if ( ohandler != SIG_DFL )
                signal(SIGINT, ohandler);

This is clearly buggy but on the other hand SDL is pretty widely deployed and it's the default QEMU video output method, so I think we need to work around it :-(

Quoted 2 lines

The most straightforward fix is to get the signal number from argument one and not to bother printing the PID that killed us.

Or just #ifdefing this to 0 for now and then once an SDL version is released that works correctly, adding an appropriate guard.

Quoted 6 lines · Peter Maydell

Unfortunately this patch causes qemu to segfault when killed via ^C (at least on my Ubuntu maverick system). This is because it registers a signal handler with sigaction, but then later the SDL library is initialised and it reinstalls our handler with plain old signal:

Quoted 4 lines

        ohandler = signal(SIGINT, SDL_HandleSIG);
        if ( ohandler != SIG_DFL )
                signal(SIGINT, ohandler);

I fixed this in SDL upstream.

Quoted 7 lines

This is clearly buggy but on the other hand SDL is pretty widely deployed and it's the default QEMU video output method, so I think we need to work around it :-(

The most straightforward fix is to get the signal number from argument one and not to bother printing the PID that killed us.

For debugging purposes pid is useful. We cam register signal handler after SDL is initialized though (if waiting for SDL update is not an option).

Quoted 7 lines · Gleb Natapov

Quoted 6 lines · Peter Maydell

Unfortunately this patch causes qemu to segfault when killed via ^C (at least on my Ubuntu maverick system). This is because it registers a signal handler with sigaction, but then later the SDL library is initialised and it reinstalls our handler with plain old signal:

Quoted 5 lines

Quoted 4 lines

        ohandler = signal(SIGINT, SDL_HandleSIG);
        if ( ohandler != SIG_DFL )
                signal(SIGINT, ohandler);

I fixed this in SDL upstream.

Cool, but we can't rely on the SDL we're linked against being a bugfixed version.

Quoted 7 lines

Quoted 7 lines

This is clearly buggy but on the other hand SDL is pretty widely deployed and it's the default QEMU video output method, so I think we need to work around it :-(

The most straightforward fix is to get the signal number from argument one and not to bother printing the PID that killed us.

Quoted 3 lines

For debugging purposes pid is useful. We cam register signal handler after SDL is initialized though (if waiting for SDL update is not an option).

I'm not convinced about the utility of printing the pid, personally. Most programs get along fine without printing anything when they receive a terminal signal. However you're right that it should

be straightforward enough to move the signal init. In fact it looks like this got broken somewhere along the line, the call to os_setup_signal_handling() is already preceded with a comment:

    /* must be after terminal init, SDL library changes signal handlers */
    os_setup_signal_handling();

...we just aren't actually after the terminal init any more :-(

Quoted 3 lines

I'm not convinced about the utility of printing the pid, personally. Most programs get along fine without printing anything when they receive a terminal signal.

Well qemu is a bit of special case. It is long running process that takes huge amount of memory and, as suchm it becomes a target of various monitoring script which, when configured incorrectly, start killing perfectly valid guests. In addition killing of the guest looks exactly like guest shutdown to management software because we call shutdow_request in the signal handler.

Exactly. This should do the trick (not tested).

Quoted 3 lines · Gleb Natapov

Currently when rogue script kills QEMU process (using TERM/INT/HUP signal) it looks indistinguishable from system shutdown. Lets report that QEMU was killed and leave some clues about the killer identity.

Unfortunately this patch causes qemu to segfault when killed via ^C (at least on my Ubuntu maverick system). This is because it registers a signal handler with sigaction, but then later the SDL library is initialised and it reinstalls our handler with plain old signal:

        ohandler = signal(SIGINT, SDL_HandleSIG);
        if ( ohandler != SIG_DFL )
                signal(SIGINT, ohandler);

This is clearly buggy but on the other hand SDL is pretty widely deployed and it's the default QEMU video output method, so I think we need to work around it :-(

The most straightforward fix is to get the signal number from argument one and not to bother printing the PID that killed us.

Quoted 19 lines · Peter Maydell

Quoted 3 lines · Gleb Natapov

Currently when rogue script kills QEMU process (using TERM/INT/HUP signal) it looks indistinguishable from system shutdown. Lets report that QEMU was killed and leave some clues about the killer identity.

Unfortunately this patch causes qemu to segfault when killed via ^C (at least on my Ubuntu maverick system). This is because it registers a signal handler with sigaction, but then later the SDL library is initialised and it reinstalls our handler with plain old signal:

        ohandler = signal(SIGINT, SDL_HandleSIG);
        if ( ohandler != SIG_DFL )
                signal(SIGINT, ohandler);

This is clearly buggy but on the other hand SDL is pretty widely deployed and it's the default QEMU video output method, so I think we need to work around it :-(

Quoted 3 lines

The most straightforward fix is to get the signal number from argument one and not to bother printing the PID that killed us.

Maybe using SDL_INIT_NOPARACHUTE is worth doing?

2011/3/30 Gleb Natapov <[email protected]>:

Quoted 11 lines · Peter Maydell

Quoted 3 lines

I'm not convinced about the utility of printing the pid, personally. Most programs get along fine without printing anything when they receive a terminal signal.

Well qemu is a bit of special case. It is long running process that takes huge amount of memory and, as suchm it becomes a target of various monitoring script which, when configured incorrectly, start killing perfectly valid guests. In addition killing of the guest looks exactly like guest shutdown to management software because we call shutdow_request in the signal handler.

That sounds like a flaw in the communication protocol between qemu and the management software, which would be better fixed by having qemu communicate the reason for exit directly (ie not just by printing to stderr), surely?

Quoted 1 line

Exactly. This should do the trick (not tested).

Looks good, and a test shows I don't get the segfault any more.

Reviewed-by Peter Maydell <[email protected]>

although I guess you'll want to submit it with a sensible git commit message :-)