Revert "main-loop.c: Handle SIGINT, SIGHUP and SIGTERM, synchronously"

PATCH Applied Jan Kiszka 4 participants 6 messages Oct 26, 2014 – Oct 27, 2014

lore

Share & tools

lore

Jan Kiszka

author

2014-10-26 09:32 UTC #0 lore raw

PATCH 1/1 Revert "main-loop.c: Handle SIGINT, SIGHUP and SIGTERM, synchronously"

From: Jan Kiszka <[email protected]>

This reverts commit 15124e142034d21341ec9f1a304a1dc5a6c25681. It breaks debuggability of qemu.

Signed-off-by Jan Kiszka <[email protected]>

---

Feel free to apply this before or after "Make qemu_shutdown_requested signal-safe".

 main-loop.c | 3 ---
 1 file changed, 3 deletions(-)

main-loop.c +0 -3

index d2e64f1..53393a4 100644
@@ -84,9 +84,6 @@ static int qemu_signal_init(void)
     sigaddset(&set, SIGIO);
     sigaddset(&set, SIGALRM);
     sigaddset(&set, SIGBUS);
-    sigaddset(&set, SIGINT);
-    sigaddset(&set, SIGHUP);
-    sigaddset(&set, SIGTERM);
     pthread_sigmask(SIG_BLOCK, &set, NULL);
 
     sigdelset(&set, SIG_IPI);

Signature · 1 line

1.8.4.5

Gonglei

Reviewed-by on v1 1/1

2014-10-26 09:53 UTC #1 lore raw

Quoted 8 lines · Jan Kiszka

From: Jan Kiszka <[email protected]>
This reverts commit 15124e142034d21341ec9f1a304a1dc5a6c25681. It breaks debuggability of qemu.
Signed-off-by Jan Kiszka <[email protected]>
---

Thanks, please add my 'Reported-by' tag and

Reviewed-by Gonglei <[email protected]>

Quoted 20 lines

Feel free to apply this before or after "Make qemu_shutdown_requested signal-safe".

 main-loop.c | 3 ---
 1 file changed, 3 deletions(-)

main-loop.c +0 -3

index d2e64f1..53393a4 100644
@@ -84,9 +84,6 @@ static int qemu_signal_init(void)
     sigaddset(&set, SIGIO);
     sigaddset(&set, SIGALRM);
     sigaddset(&set, SIGBUS);
-    sigaddset(&set, SIGINT);
-    sigaddset(&set, SIGHUP);
-    sigaddset(&set, SIGTERM);
     pthread_sigmask(SIG_BLOCK, &set, NULL);
 
     sigdelset(&set, SIG_IPI);

Peter Maydell

2014-10-27 14:19 UTC #2 lore raw

In reply to Jan Kiszka

Quoted 19 lines · Jan Kiszka

From: Jan Kiszka <[email protected]>
This reverts commit 15124e142034d21341ec9f1a304a1dc5a6c25681. It breaks debuggability of qemu.
Signed-off-by Jan Kiszka <[email protected]>
---
Feel free to apply this before or after "Make qemu_shutdown_requested signal-safe".
 main-loop.c | 3 ---
 1 file changed, 3 deletions(-)
main-loop.c +0 -0
index d2e64f1..53393a4 100644

main-loop.c static int qemu_signal_init(void) · line 89

Quoted 6 lines

main-loop.c +0 -3

@@ -84,9 +84,6 @@ static int qemu_signal_init(void)
     sigaddset(&set, SIGIO);
     sigaddset(&set, SIGALRM);
     sigaddset(&set, SIGBUS);
-    sigaddset(&set, SIGINT);
-    sigaddset(&set, SIGHUP);
-    sigaddset(&set, SIGTERM);

I'm planning to apply this patch but with the following comment added here:
/* Note that the SIGINT, SIGTERM and SIGHUP signals are not handled
* via signalfd, and so their handlers will still be invoked
* asynchronously. This is done so that ^C can be used to interrupt
* QEMU when it is being run under gdb.
*/

(which does make the commit not a pure revert).

Quoted 5 lines

main-loop.c +0 -1

     pthread_sigmask(SIG_BLOCK, &set, NULL);
 
     sigdelset(&set, SIG_IPI);
--

1.8.4.5

thanks
-- PMM

Paolo Bonzini

2014-10-27 14:40 UTC #3 lore raw

Quoted 27 lines · Peter Maydell

Quoted 19 lines · Jan Kiszka
From: Jan Kiszka <[email protected]>
This reverts commit 15124e142034d21341ec9f1a304a1dc5a6c25681. It breaks debuggability of qemu.
Signed-off-by Jan Kiszka <[email protected]>
---
Feel free to apply this before or after "Make qemu_shutdown_requested signal-safe".
 main-loop.c | 3 ---
 1 file changed, 3 deletions(-)
main-loop.c +0 -0
index d2e64f1..53393a4 100644
main-loop.c static int qemu_signal_init(void) · line 89
Quoted 6 lines
main-loop.c +0 -3
@@ -84,9 +84,6 @@ static int qemu_signal_init(void)
     sigaddset(&set, SIGIO);
     sigaddset(&set, SIGALRM);
     sigaddset(&set, SIGBUS);
-    sigaddset(&set, SIGINT);
-    sigaddset(&set, SIGHUP);
-    sigaddset(&set, SIGTERM);

Quoted 7 lines

I'm planning to apply this patch but with the following comment added here:
/* Note that the SIGINT, SIGTERM and SIGHUP signals are not handled
* via signalfd, and so their handlers will still be invoked
* asynchronously. This is done so that ^C can be used to interrupt
* QEMU when it is being run under gdb.
*/

What about:

/* SIGINT cannot be handled via signalfd, so that ^C can be used
* to interrupt QEMU when it is being run under gdb. SIGHUP and
* SIGTERM are also handled asynchronously, even though it is not
* strictly necessary, because they use the same handler as SIGINT. */

Quoted 11 lines

(which does make the commit not a pure revert).
Quoted 5 lines
main-loop.c +0 -1
     pthread_sigmask(SIG_BLOCK, &set, NULL);
 
     sigdelset(&set, SIG_IPI);
--
1.8.4.5
thanks
-- PMM

Peter Maydell

2014-10-27 15:06 UTC #4 lore raw

Quoted 38 lines · Paolo Bonzini

Quoted 27 lines · Peter Maydell
Quoted 19 lines · Jan Kiszka
From: Jan Kiszka <[email protected]>
This reverts commit 15124e142034d21341ec9f1a304a1dc5a6c25681. It breaks debuggability of qemu.
Signed-off-by Jan Kiszka <[email protected]>
---
Feel free to apply this before or after "Make qemu_shutdown_requested signal-safe".
 main-loop.c | 3 ---
 1 file changed, 3 deletions(-)
main-loop.c +0 -0
index d2e64f1..53393a4 100644
main-loop.c static int qemu_signal_init(void) · line 89
Quoted 6 lines
main-loop.c +0 -3
@@ -84,9 +84,6 @@ static int qemu_signal_init(void)
     sigaddset(&set, SIGIO);
     sigaddset(&set, SIGALRM);
     sigaddset(&set, SIGBUS);
-    sigaddset(&set, SIGINT);
-    sigaddset(&set, SIGHUP);
-    sigaddset(&set, SIGTERM);
Quoted 7 lines
I'm planning to apply this patch but with the following comment added here:
/* Note that the SIGINT, SIGTERM and SIGHUP signals are not handled
* via signalfd, and so their handlers will still be invoked
* asynchronously. This is done so that ^C can be used to interrupt
* QEMU when it is being run under gdb.
*/

Quoted 7 lines

What about:
/* SIGINT cannot be handled via signalfd, so that ^C can be used
* to interrupt QEMU when it is being run under gdb. SIGHUP and
* SIGTERM are also handled asynchronously, even though it is not
* strictly necessary, because they use the same handler as SIGINT. */

Looks good to me, I'll use that wording.

-- PMM

Peter Maydell

Applied

2014-10-27 16:45 UTC #5 lore raw

Quoted 10 lines · Peter Maydell

Quoted 7 lines · Paolo Bonzini
What about:
/* SIGINT cannot be handled via signalfd, so that ^C can be used
* to interrupt QEMU when it is being run under gdb. SIGHUP and
* SIGTERM are also handled asynchronously, even though it is not
* strictly necessary, because they use the same handler as SIGINT. */
Looks good to me, I'll use that wording.

Applied this and the other patch to master, thanks.

-- PMM