qemu_kill_report() is already able to tell whether a shutdown
was triggered by guest action (no output) or by a host signal
(a message about termination is printed via error_report); but
this information is then lost. Libvirt would like to be able
to distinguish between a SHUTDOWN event triggered solely by
guest request and one triggered by a SIGTERM on the host.
Enhance the SHUTDOWN event to pass the value of shutdown_signal
through to the monitor client, suitably remapped into a
platform-neutral string. Note that mingw lacks decent signal
support, and will never report a signal because it never calls
qemu_system_killed().
See also https://bugzilla.redhat.com/1384007
Signed-off-by: Eric Blake <eblake@redhat.com>
---
qapi/event.json | 20 +++++++++++++++++++-
vl.c | 21 ++++++++++++++++++---
2 files changed, 37 insertions(+), 4 deletions(-)
diff --git a/qapi/event.json b/qapi/event.json
index e80f3f4..6aad475 100644
--- a/qapi/event.json
+++ b/qapi/event.json
@@ -5,11 +5,29 @@
##
##
+# @ShutdownSignal:
+#
+# The list of host signal types known to cause qemu to shut down a guest.
+#
+# @int: SIGINT
+# @hup: SIGHUP
+# @term: SIGTERM
+#
+# Since: 2.10
+##
+{ 'enum': 'ShutdownSignal', 'data': [ 'int', 'hup', 'term' ] }
+
+##
# @SHUTDOWN:
#
# Emitted when the virtual machine has shut down, indicating that qemu is
# about to exit.
#
+# @signal: If present, the shutdown was (probably) triggered due to
+# the receipt of the given signal in the host, rather than by a guest
+# action (note that there is an inherent race with a guest choosing to
+# shut down near the same time the host sends a signal). (since 2.10)
+#
# Note: If the command-line option "-no-shutdown" has been specified, qemu will
# not exit, and a STOP event will eventually follow the SHUTDOWN event
#
@@ -21,7 +39,7 @@
# "timestamp": { "seconds": 1267040730, "microseconds": 682951 } }
#
##
-{ 'event': 'SHUTDOWN' }
+{ 'event': 'SHUTDOWN', 'data': { '*signal': 'ShutdownSignal' } }
##
# @POWERDOWN:
diff --git a/vl.c b/vl.c
index 0b4ed52..af29b2c 100644
--- a/vl.c
+++ b/vl.c
@@ -1626,9 +1626,23 @@ static int qemu_shutdown_requested(void)
return atomic_xchg(&shutdown_requested, 0);
}
-static void qemu_kill_report(void)
+static ShutdownSignal qemu_kill_report(void)
{
+ ShutdownSignal ss = SHUTDOWN_SIGNAL__MAX;
if (!qtest_driver() && shutdown_signal != -1) {
+ switch (shutdown_signal) {
+ case SIGINT:
+ ss = SHUTDOWN_SIGNAL_INT;
+ break;
+#ifdef SIGHUP
+ case SIGHUP:
+ ss = SHUTDOWN_SIGNAL_HUP;
+ break;
+#endif
+ case SIGTERM:
+ ss = SHUTDOWN_SIGNAL_TERM;
+ break;
+ }
if (shutdown_pid == 0) {
/* This happens for eg ^C at the terminal, so it's worth
* avoiding printing an odd message in that case.
@@ -1644,6 +1658,7 @@ static void qemu_kill_report(void)
}
shutdown_signal = -1;
}
+ return ss;
}
static int qemu_reset_requested(void)
@@ -1852,8 +1867,8 @@ static bool main_loop_should_exit(void)
qemu_system_suspend();
}
if (qemu_shutdown_requested()) {
- qemu_kill_report();
- qapi_event_send_shutdown(&error_abort);
+ ShutdownSignal ss = qemu_kill_report();
+ qapi_event_send_shutdown(ss < SHUTDOWN_SIGNAL__MAX, ss, &error_abort);
if (no_shutdown) {
vm_stop(RUN_STATE_SHUTDOWN);
} else {
--
2.9.3
On Thu, Apr 06, 2017 at 04:09:17PM -0500, Eric Blake wrote:
> qemu_kill_report() is already able to tell whether a shutdown
> was triggered by guest action (no output) or by a host signal
> (a message about termination is printed via error_report); but
> this information is then lost. Libvirt would like to be able
> to distinguish between a SHUTDOWN event triggered solely by
> guest request and one triggered by a SIGTERM on the host.
>
> Enhance the SHUTDOWN event to pass the value of shutdown_signal
> through to the monitor client, suitably remapped into a
> platform-neutral string. Note that mingw lacks decent signal
> support, and will never report a signal because it never calls
> qemu_system_killed().
Is it conceivable that we find a non-signal based way to distinguish
guest initiated shutdown, from host OS initiated kill when on Win32 ?
If so, rather than including a 'signal' field in the event, it might
be better to just have a 'source': 'host|guest' field. ie we would
indicate /who/ initiated the shutdown, rather than /how/ it was
initiated.
Or perhaps we should include both the who & the how ?
> See also https://bugzilla.redhat.com/1384007
>
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
> qapi/event.json | 20 +++++++++++++++++++-
> vl.c | 21 ++++++++++++++++++---
> 2 files changed, 37 insertions(+), 4 deletions(-)
>
> diff --git a/qapi/event.json b/qapi/event.json
> index e80f3f4..6aad475 100644
> --- a/qapi/event.json
> +++ b/qapi/event.json
> @@ -5,11 +5,29 @@
> ##
>
> ##
> +# @ShutdownSignal:
> +#
> +# The list of host signal types known to cause qemu to shut down a guest.
> +#
> +# @int: SIGINT
> +# @hup: SIGHUP
> +# @term: SIGTERM
> +#
> +# Since: 2.10
> +##
> +{ 'enum': 'ShutdownSignal', 'data': [ 'int', 'hup', 'term' ] }
> +
> +##
> # @SHUTDOWN:
> #
> # Emitted when the virtual machine has shut down, indicating that qemu is
> # about to exit.
> #
> +# @signal: If present, the shutdown was (probably) triggered due to
> +# the receipt of the given signal in the host, rather than by a guest
> +# action (note that there is an inherent race with a guest choosing to
> +# shut down near the same time the host sends a signal). (since 2.10)
> +#
> # Note: If the command-line option "-no-shutdown" has been specified, qemu will
> # not exit, and a STOP event will eventually follow the SHUTDOWN event
> #
> @@ -21,7 +39,7 @@
> # "timestamp": { "seconds": 1267040730, "microseconds": 682951 } }
> #
> ##
> -{ 'event': 'SHUTDOWN' }
> +{ 'event': 'SHUTDOWN', 'data': { '*signal': 'ShutdownSignal' } }
>
> ##
> # @POWERDOWN:
> diff --git a/vl.c b/vl.c
> index 0b4ed52..af29b2c 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1626,9 +1626,23 @@ static int qemu_shutdown_requested(void)
> return atomic_xchg(&shutdown_requested, 0);
> }
>
> -static void qemu_kill_report(void)
> +static ShutdownSignal qemu_kill_report(void)
> {
> + ShutdownSignal ss = SHUTDOWN_SIGNAL__MAX;
> if (!qtest_driver() && shutdown_signal != -1) {
> + switch (shutdown_signal) {
> + case SIGINT:
> + ss = SHUTDOWN_SIGNAL_INT;
> + break;
> +#ifdef SIGHUP
> + case SIGHUP:
> + ss = SHUTDOWN_SIGNAL_HUP;
> + break;
> +#endif
> + case SIGTERM:
> + ss = SHUTDOWN_SIGNAL_TERM;
> + break;
> + }
> if (shutdown_pid == 0) {
> /* This happens for eg ^C at the terminal, so it's worth
> * avoiding printing an odd message in that case.
> @@ -1644,6 +1658,7 @@ static void qemu_kill_report(void)
> }
> shutdown_signal = -1;
> }
> + return ss;
> }
>
> static int qemu_reset_requested(void)
> @@ -1852,8 +1867,8 @@ static bool main_loop_should_exit(void)
> qemu_system_suspend();
> }
> if (qemu_shutdown_requested()) {
> - qemu_kill_report();
> - qapi_event_send_shutdown(&error_abort);
> + ShutdownSignal ss = qemu_kill_report();
> + qapi_event_send_shutdown(ss < SHUTDOWN_SIGNAL__MAX, ss, &error_abort);
> if (no_shutdown) {
> vm_stop(RUN_STATE_SHUTDOWN);
> } else {
If any to the above question means we keep '*signal', then consider this
Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://entangle-photo.org -o- http://search.cpan.org/~danberr/ :|
On 04/07/2017 04:35 AM, Daniel P. Berrange wrote:
> On Thu, Apr 06, 2017 at 04:09:17PM -0500, Eric Blake wrote:
>> qemu_kill_report() is already able to tell whether a shutdown
>> was triggered by guest action (no output) or by a host signal
>> (a message about termination is printed via error_report); but
>> this information is then lost. Libvirt would like to be able
>> to distinguish between a SHUTDOWN event triggered solely by
>> guest request and one triggered by a SIGTERM on the host.
>>
>> Enhance the SHUTDOWN event to pass the value of shutdown_signal
>> through to the monitor client, suitably remapped into a
>> platform-neutral string. Note that mingw lacks decent signal
>> support, and will never report a signal because it never calls
>> qemu_system_killed().
>
> Is it conceivable that we find a non-signal based way to distinguish
> guest initiated shutdown, from host OS initiated kill when on Win32 ?
>
> If so, rather than including a 'signal' field in the event, it might
> be better to just have a 'source': 'host|guest' field. ie we would
> indicate /who/ initiated the shutdown, rather than /how/ it was
> initiated.
>
> Or perhaps we should include both the who & the how ?
It's always something that can be extended later (possibly by even
adding an 'unknown' to the how, if there is no real signal name for
mingw). The 'who' is kind of implied by the presence or absence of the
optional 'how', and even though mingw doesn't have SIGHUP, it DOES have
SIGINT (C requires that), so it is still probably that someone
interested enough could wire up Ctrl-C on mingw host to trigger 'int' as
the 'how', rather than the current situation of Ctrl-C presumably doing
nothing and 'how' never being reported. (I haven't ever actually tried
to run a guest with mingw as the host, to know if Ctrl-C even works...)
>> @@ -1852,8 +1867,8 @@ static bool main_loop_should_exit(void)
>> qemu_system_suspend();
>> }
>> if (qemu_shutdown_requested()) {
>> - qemu_kill_report();
>> - qapi_event_send_shutdown(&error_abort);
>> + ShutdownSignal ss = qemu_kill_report();
>> + qapi_event_send_shutdown(ss < SHUTDOWN_SIGNAL__MAX, ss, &error_abort);
>> if (no_shutdown) {
>> vm_stop(RUN_STATE_SHUTDOWN);
>> } else {
>
> If any to the above question means we keep '*signal', then consider this
>
> Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
>
I still think for Unix-y hosts that '*signal' is useful (some people DO
care whether it was SIGTERM vs. SIGHUP that caused qemu to shutdown,
especially if the two signals are sent by two different agents and can
therefore let the management distinguish which agent asked for the guest
to be killed).
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
On 07/04/2017 17:35, Daniel P. Berrange wrote: > Is it conceivable that we find a non-signal based way to distinguish > guest initiated shutdown, from host OS initiated kill when on Win32 ? As far as I know there's no such thing as a "soft kill" (SIGTERM) on Win32, since there is no kill function (only raise). The only interprocess "signals" are Ctrl-C/Ctrl-Break events for the console, which are mapped to SIGINT or SIGBREAK respectively (the difference is that the application cannot trap Ctrl-Break, while it can disable the default Ctrl-C handler). Likewise there are close-window, logoff and system shutdown events that the application can trap, but I'm not sure how much they are worth reporting in the SHUTDOWN event and I'm also not sure that you get them if you don't have a console, as is the case for qemu-system-x86_64w.exe. Paolo > If so, rather than including a 'signal' field in the event, it might > be better to just have a 'source': 'host|guest' field. ie we would > indicate /who/ initiated the shutdown, rather than /how/ it was > initiated.
On 07/04/2017 05:09, Eric Blake wrote:
> ##
> +# @ShutdownSignal:
> +#
> +# The list of host signal types known to cause qemu to shut down a guest.
> +#
> +# @int: SIGINT
> +# @hup: SIGHUP
> +# @term: SIGTERM
> +#
> +# Since: 2.10
> +##
> +{ 'enum': 'ShutdownSignal', 'data': [ 'int', 'hup', 'term' ] }
I follow Markus's suggestion and would even make them uppercase.
Uncommon, but I think justified in this case.
> +##
> # @SHUTDOWN:
> #
> # Emitted when the virtual machine has shut down, indicating that qemu is
> # about to exit.
> #
> +# @signal: If present, the shutdown was (probably) triggered due to
> +# the receipt of the given signal in the host, rather than by a guest
> +# action (note that there is an inherent race with a guest choosing to
> +# shut down near the same time the host sends a signal). (since 2.10)
> +#
> # Note: If the command-line option "-no-shutdown" has been specified, qemu will
> # not exit, and a STOP event will eventually follow the SHUTDOWN event
> #
> @@ -21,7 +39,7 @@
> # "timestamp": { "seconds": 1267040730, "microseconds": 682951 } }
> #
> ##
> -{ 'event': 'SHUTDOWN' }
> +{ 'event': 'SHUTDOWN', 'data': { '*signal': 'ShutdownSignal' } }
>
> ##
> # @POWERDOWN:
> diff --git a/vl.c b/vl.c
> index 0b4ed52..af29b2c 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1626,9 +1626,23 @@ static int qemu_shutdown_requested(void)
> return atomic_xchg(&shutdown_requested, 0);
> }
>
> -static void qemu_kill_report(void)
> +static ShutdownSignal qemu_kill_report(void)
> {
> + ShutdownSignal ss = SHUTDOWN_SIGNAL__MAX;
> if (!qtest_driver() && shutdown_signal != -1) {
> + switch (shutdown_signal) {
> + case SIGINT:
> + ss = SHUTDOWN_SIGNAL_INT;
> + break;
> +#ifdef SIGHUP
> + case SIGHUP:
> + ss = SHUTDOWN_SIGNAL_HUP;
> + break;
> +#endif
> + case SIGTERM:
> + ss = SHUTDOWN_SIGNAL_TERM;
> + break;
> + }
> if (shutdown_pid == 0) {
> /* This happens for eg ^C at the terminal, so it's worth
> * avoiding printing an odd message in that case.
> @@ -1644,6 +1658,7 @@ static void qemu_kill_report(void)
> }
> shutdown_signal = -1;
> }
> + return ss;
> }
>
> static int qemu_reset_requested(void)
> @@ -1852,8 +1867,8 @@ static bool main_loop_should_exit(void)
> qemu_system_suspend();
> }
> if (qemu_shutdown_requested()) {
> - qemu_kill_report();
> - qapi_event_send_shutdown(&error_abort);
> + ShutdownSignal ss = qemu_kill_report();
> + qapi_event_send_shutdown(ss < SHUTDOWN_SIGNAL__MAX, ss, &error_abort);
Maybe call qapi_event_send_shutdown from qemu_kill_report? It is not
obvious that ShutdownSignal is unsigned (except to you and Laszlo of
course).
I think this patch is fine for now; calling qemu_system_killed() from
os-win32.c is an orthogonal improvement.
Paolo
> if (no_shutdown) {
> vm_stop(RUN_STATE_SHUTDOWN);
> } else {
>
© 2016 - 2024 Red Hat, Inc.