Re: [Qemu-devel] [PATCH v2] event: Add source information to SHUTDOWN

["Daniel P. Berrange" <berrange@redhat.com>]

On Thu, Apr 20, 2017 at 01:59:29PM +0200, Markus Armbruster wrote:
> Eric Blake <eblake@redhat.com> writes:
> 
> > Libvirt would like to be able to distinguish between a SHUTDOWN
> > event triggered solely by guest request and one triggered by a
> > SIGTERM or other action on the host.  qemu_kill_report() is
> > already able to tell whether a shutdown was triggered by a host
> > signal (but NOT by a host UI event, such as clicking the X on
> > the window), but that information was then lost after being
> > printed to stderr.
> >
> > Enhance the shutdown request path to take a parameter of which
> > way it is being triggered, and update ALL callers.  It would have
> > been less churn to keep the common case with no arguments as
> > meaning guest-triggered, and only modified the host-triggered
> > code paths, via a wrapper function, but then we'd still have to
> > audit that I didn't miss any host-triggered spots; changing the
> > signature forces us to double-check that I correctly categorized
> > all callers.
> >
> > Here is output from 'virsh qemu-monitor-event --loop' with the
> > patch installed:
> >
> > event SHUTDOWN at 1492639680.731251 for domain fedora_13: {"guest":true}
> > event STOP at 1492639680.732116 for domain fedora_13: <null>
> > event SHUTDOWN at 1492639680.732830 for domain fedora_13: {"guest":false}
> >
> > Note that libvirt runs qemu with -no-quit: the first SHUTDOWN event
> > was triggered by an action I took directly in the guest (shutdown -h),
> > at which point qemu stops the vcpus and waits for libvirt to do any
> > final cleanups; the second SHUTDOWN event is the result of libvirt
> > sending SIGTERM now that it has completed cleanup.
> >
> > See also https://bugzilla.redhat.com/1384007
> >
> > Signed-off-by: Eric Blake <eblake@redhat.com>
> >
> > ---
> >
> > I did not wire up the RESET event to report guest-triggered, although
> > I had to plumb that through all the guests since qemu has options that
> > allow remapping reset to shutdown.  It's easy to add that if we want
> > it in v3.
> >
> > The replay driver needs a followup patch if we want to be able to
> > faithfully replay the difference between a host- and guest-initiated
> > shutdown (for now, the replayed event is always attributed to host).
> >
> >
> > v2: retitle (was "event: Add signal information to SHUTDOWN"),
> > completely rework to post bool based on whether it is guest-initiated
> > v1: initial submission, exposing just Unix signals from host
> > ---
> >  qapi/event.json             |  8 ++++++--
> >  include/sysemu/sysemu.h     |  4 ++--
> >  vl.c                        | 19 +++++++++++--------
> >  hw/acpi/core.c              |  4 ++--
> >  hw/arm/highbank.c           |  4 ++--
> >  hw/arm/integratorcp.c       |  2 +-
> >  hw/arm/musicpal.c           |  2 +-
> >  hw/arm/omap1.c              |  6 +++---
> >  hw/arm/omap2.c              |  2 +-
> >  hw/arm/spitz.c              |  2 +-
> >  hw/arm/stellaris.c          |  2 +-
> >  hw/arm/tosa.c               |  2 +-
> >  hw/i386/pc.c                |  2 +-
> >  hw/input/pckbd.c            |  4 ++--
> >  hw/ipmi/ipmi.c              |  4 ++--
> >  hw/isa/lpc_ich9.c           |  2 +-
> >  hw/mips/boston.c            |  2 +-
> >  hw/mips/mips_malta.c        |  2 +-
> >  hw/mips/mips_r4k.c          |  4 ++--
> >  hw/misc/arm_sysctl.c        |  8 ++++----
> >  hw/misc/cbus.c              |  2 +-
> >  hw/misc/macio/cuda.c        |  4 ++--
> >  hw/misc/slavio_misc.c       |  4 ++--
> >  hw/misc/zynq_slcr.c         |  2 +-
> >  hw/pci-host/apb.c           |  4 ++--
> >  hw/pci-host/bonito.c        |  2 +-
> >  hw/pci-host/piix.c          |  2 +-
> >  hw/ppc/e500.c               |  2 +-
> >  hw/ppc/mpc8544_guts.c       |  2 +-
> >  hw/ppc/ppc.c                |  2 +-
> >  hw/ppc/ppc405_uc.c          |  2 +-
> >  hw/ppc/spapr_hcall.c        |  2 +-
> >  hw/ppc/spapr_rtas.c         |  4 ++--
> >  hw/s390x/ipl.c              |  2 +-
> >  hw/sh4/r2d.c                |  2 +-
> >  hw/timer/etraxfs_timer.c    |  2 +-
> >  hw/timer/m48t59.c           |  4 ++--
> >  hw/timer/milkymist-sysctl.c |  4 ++--
> >  hw/timer/pxa2xx_timer.c     |  2 +-
> >  hw/watchdog/watchdog.c      |  2 +-
> >  hw/xenpv/xen_domainbuild.c  |  2 +-
> >  hw/xtensa/xtfpga.c          |  2 +-
> >  kvm-all.c                   |  6 +++---
> >  os-win32.c                  |  2 +-
> >  qmp.c                       |  4 ++--
> >  replay/replay.c             |  5 ++++-
> >  target/alpha/sys_helper.c   |  4 ++--
> >  target/arm/psci.c           |  4 ++--
> >  target/i386/excp_helper.c   |  2 +-
> >  target/i386/hax-all.c       |  6 +++---
> >  target/i386/helper.c        |  2 +-
> >  target/i386/kvm.c           |  2 +-
> >  target/s390x/helper.c       |  2 +-
> >  target/s390x/kvm.c          |  4 ++--
> >  target/s390x/misc_helper.c  |  4 ++--
> >  target/sparc/int32_helper.c |  2 +-
> >  ui/sdl.c                    |  2 +-
> >  ui/sdl2.c                   |  4 ++--
> >  xen-hvm.c                   |  2 +-
> >  trace-events                |  2 +-
> >  ui/cocoa.m                  |  2 +-
> >  61 files changed, 106 insertions(+), 96 deletions(-)
> >
> > diff --git a/qapi/event.json b/qapi/event.json
> > index e80f3f4..c230265 100644
> > --- a/qapi/event.json
> > +++ b/qapi/event.json
> > @@ -10,6 +10,10 @@
> >  # Emitted when the virtual machine has shut down, indicating that qemu is
> >  # about to exit.
> >  #
> > +# @guest: If true, the shutdown was triggered by a guest request (such as
> > +# executing a halt instruction) rather than a host request (such as sending
> > +# qemu a SIGINT). (since 2.10)
> > +#
> 
> "executing a halt instruction" suggests "halt" is a machine instruction.
> I think you mean /usr/sbin/halt.  Suggest something like "executing a
> halt command".

Well technically /usr/sbin/halt just terminates all processes / kernel and
halts CPUs, but the virtual machine is still active (and a 'reset' in the
monitor can start it again. /usr/sbin/poweroff is what actually does the
ACPI poweroff to trigger QEMU to exit[1]

Regards,
Daniel

[1] assuming ACPI poweroff is exposed to guest and supported by the guest
    kernel.
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|