Eric Blake <ebl...@redhat.com> writes: > On 04/12/2017 06:02 AM, Markus Armbruster wrote: >> Eric Blake <ebl...@redhat.com> writes: >> >>> 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 >> >> I understand the desire to distinguish between guest-initiated and >> host-initiated shutdown, but I'm not sure why libvirt (or anyone) would >> care for the exact signal. Can you explain? > > If we don't care about the signal itself, a simple boolean (host vs. > guest) is just as easy to code up. Or even code up a boolean now, and > then add signal information down the road if someone has a use case for > it (as Dan said, libvirt doesn't care, but someone on top of libvirt > might - but I haven't identified such a user at this point in time).
Starting with just a boolean should be safe. Especially attractive if it lets us sidestep Windows complications; more on that below. >> >>> Note that mingw lacks decent signal >>> support, and will never report a signal because it never calls >>> qemu_system_killed(). >> >> Awkward. >> > >> In other words, these three signals are polite requests to terminate >> QEMU. >> >> Stefan, are there equivalent requests under Windows? I guess there >> might be one at least for SIGINT, namely whatever happens when you hit >> ^C on the console. > > Mingw has SIGINT (C99 requires it), and that's presumably what happens > for ^C,... > >> >> Could we arrange to run qemu_system_killed() then? > > ...but I don't know why it is not currently wired up to call > qemu_system_killed(), nor do I have enough Windows programming expertise > to try and write such a patch. But I think that is an orthogonal > improvement. On the other hand, mingw has a definition for SIGTERM (but > I'm not sure how it gets triggered) and no definition at all for SIGHUP > (as evidenced by the #ifdef'fery in the patch to get it to compile under > docker targetting mingw). If all we need is distingishing host- and guest-initiated shutdown, then detecting the latter reliably lets us stay away from OS-specific stuff. Can we do that? >> >> If not, could we at least distinguish between guest-initiated and >> host-initiated shutdown? >> >>> See also https://bugzilla.redhat.com/1384007 >>> >>> Signed-off-by: Eric Blake <ebl...@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' ] } >> >> I'd call them sigint, sighup, sigterm, but it's a matter of taste. > > And it goes away if we are okay with a simpler bool of host vs. guest. > >> >>> + >>> +## >>> # @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) >>> +# >> >> Is the "(probably)" due to just Windows, or are there other reasons for >> uncertainty? > > There are other reasons too: a guest can request shutdown immediately > before the host sends SIGINT. Based on when things are processed, you > could see either the guest or the host as the initiator. And the race > is not entirely implausible - when trying to shut down a guest, libvirt > first tries to inform the guest to initiate things (whether by interrupt > or guest agent), but after a given amount of time, assumes the guest is > unresponsive and resorts to a signal to qemu. A heavily loaded guest > that takes its time in responding could easily overlap with the timeout > resorting to a host-side action. This race doesn't worry me. If both host and guest have initiated a shutdown, then reporting whichever of the two finishes first seems fair. Additional ways to terminate QEMU: HMP and QMP command "quit", and the various GUI controls such "close SDL window". > >>> -static void qemu_kill_report(void) >>> +static ShutdownSignal qemu_kill_report(void) >>> { >>> + ShutdownSignal ss = SHUTDOWN_SIGNAL__MAX; >>> if (!qtest_driver() && shutdown_signal != -1) { >> >> Outside this patch's scope: could just as well use 0 instead of -1, as 0 >> can't be a valid signal number (kill() uses it for "check if we could >> kill"). > > Indeed. > >>> @@ -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 { >> >> Why not send the event within qemu_kill_report()? > > Sure, I can do that in v2.
