| Message ID | 1411660269-11081-1-git-send-email-peter.maydell@linaro.org |
|---|---|
| State | Accepted |
| Commit | 15124e142034d21341ec9f1a304a1dc5a6c25681 |
| Headers | show |
Il 25/09/2014 17:51, Peter Maydell ha scritto: > Add the termination signals SIGINT, SIGHUP and SIGTERM to the > list of signals which we handle synchronously via a signalfd. > This avoids a race condition where if we took the SIGTERM > in the middle of qemu_shutdown_requested: > int r = shutdown_requested; > [SIGTERM here...] > shutdown_requested = 0; > > then the setting of the shutdown_requested flag by > termsig_handler() would be lost and QEMU would fail to > shut down. This was causing 'make check' to hang occasionally. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > Cc: qemu-stable@nongnu.org > --- > main-loop.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/main-loop.c b/main-loop.c > index 3cc79f8..8746abc 100644 > --- a/main-loop.c > +++ b/main-loop.c > @@ -84,6 +84,9 @@ 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); > Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> Paolo
Peter Maydell <peter.maydell@linaro.org> writes: > Add the termination signals SIGINT, SIGHUP and SIGTERM to the > list of signals which we handle synchronously via a signalfd. > This avoids a race condition where if we took the SIGTERM > in the middle of qemu_shutdown_requested: > int r = shutdown_requested; > [SIGTERM here...] > shutdown_requested = 0; > > then the setting of the shutdown_requested flag by > termsig_handler() would be lost and QEMU would fail to > shut down. This was causing 'make check' to hang occasionally. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > Cc: qemu-stable@nongnu.org <snip> I've been testing it with my latest Travis patches (- the make check once patch) and it seems a lot better now: Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Tested-by: Alex Bennée <alex.bennee@linaro.org>
On 26 September 2014 11:37, Alex Bennée <alex.bennee@linaro.org> wrote: > > Peter Maydell <peter.maydell@linaro.org> writes: > >> Add the termination signals SIGINT, SIGHUP and SIGTERM to the >> list of signals which we handle synchronously via a signalfd. >> This avoids a race condition where if we took the SIGTERM >> in the middle of qemu_shutdown_requested: >> int r = shutdown_requested; >> [SIGTERM here...] >> shutdown_requested = 0; >> >> then the setting of the shutdown_requested flag by >> termsig_handler() would be lost and QEMU would fail to >> shut down. This was causing 'make check' to hang occasionally. >> >> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> >> Cc: qemu-stable@nongnu.org > <snip> > > I've been testing it with my latest Travis patches (- the make check > once patch) and it seems a lot better now: > > Reviewed-by: Alex Bennée <alex.bennee@linaro.org> > Tested-by: Alex Bennée <alex.bennee@linaro.org> Thanks; applied to master. -- PMM
Hi, > Subject: [Qemu-devel] [PATCH] main-loop.c: Handle SIGINT, SIGHUP and > SIGTERM synchronously > > Add the termination signals SIGINT, SIGHUP and SIGTERM to the > list of signals which we handle synchronously via a signalfd. > This avoids a race condition where if we took the SIGTERM > in the middle of qemu_shutdown_requested: > int r = shutdown_requested; > [SIGTERM here...] > shutdown_requested = 0; > > then the setting of the shutdown_requested flag by > termsig_handler() would be lost and QEMU would fail to > shut down. This was causing 'make check' to hang occasionally. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> I met a really upset thing after this patch, which I can't using gdb debug Qemu process well. I use gdb attaching a Qemu process, and press 'c' to continue executing. When I want to set a breakpoint for debugging, then press 'ctrl + c', the Qemu will exit, because the SIGINT signal is captured by Qemu now. :( What's your opinion? Thanks. Best regards, -Gonglei > Cc: qemu-stable@nongnu.org > --- > main-loop.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/main-loop.c b/main-loop.c > index 3cc79f8..8746abc 100644 > --- a/main-loop.c > +++ b/main-loop.c > @@ -84,6 +84,9 @@ 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); > -- > 1.9.1 >
On 2014-10-25 13:34, Gonglei wrote: > Hi, > >> Subject: [Qemu-devel] [PATCH] main-loop.c: Handle SIGINT, SIGHUP and >> SIGTERM synchronously >> >> Add the termination signals SIGINT, SIGHUP and SIGTERM to the >> list of signals which we handle synchronously via a signalfd. >> This avoids a race condition where if we took the SIGTERM >> in the middle of qemu_shutdown_requested: >> int r = shutdown_requested; >> [SIGTERM here...] >> shutdown_requested = 0; >> >> then the setting of the shutdown_requested flag by >> termsig_handler() would be lost and QEMU would fail to >> shut down. This was causing 'make check' to hang occasionally. >> >> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > > I met a really upset thing after this patch, which I can't using gdb debug > Qemu process well. I use gdb attaching a Qemu process, and press 'c' to continue > executing. When I want to set a breakpoint for debugging, then press 'ctrl + c', > the Qemu will exit, because the SIGINT signal is captured by Qemu now. :( > > What's your opinion? Thanks. Confirmed, you cannot interrupt a running qemu from gdb anymore. I think this patch has to be reverted and the original issue fixed by making qemu_shutdown_requested signal-safe. Should be simple, just convert shutdown_requested into a counter. Need not be atomic, all happens over the main thread anyway. Jan
diff --git a/main-loop.c b/main-loop.c index 3cc79f8..8746abc 100644 --- a/main-loop.c +++ b/main-loop.c @@ -84,6 +84,9 @@ 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);
Add the termination signals SIGINT, SIGHUP and SIGTERM to the list of signals which we handle synchronously via a signalfd. This avoids a race condition where if we took the SIGTERM in the middle of qemu_shutdown_requested: int r = shutdown_requested; [SIGTERM here...] shutdown_requested = 0; then the setting of the shutdown_requested flag by termsig_handler() would be lost and QEMU would fail to shut down. This was causing 'make check' to hang occasionally. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Cc: qemu-stable@nongnu.org --- main-loop.c | 3 +++ 1 file changed, 3 insertions(+)