All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Daniel P. Berrange" <berrange@redhat.com>
To: Simon <qemu.bugs@whitewinterwolf.com>
Cc: qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [Bug 1217339] [PATCH] Unix signal to send ACPI-shutdown to Guest
Date: Wed, 15 Mar 2017 14:41:24 +0000	[thread overview]
Message-ID: <20170315144124.GP7770@redhat.com> (raw)
In-Reply-To: <11569d75b61122820f186a32cdd20689@whitewinterwolf.com>

On Wed, Mar 15, 2017 at 02:45:57PM +0100, Simon wrote:
> Hello,
> 
> Here is a short patch answering to Qemu wish-list issue 1217339.
> <https://bugs.launchpad.net/qemu/+bug/1217339>
> It makes Qemu to cleanly power off the guest when receiving a SIGHUP
> signal, thus without requiring any monitor access which is currently
> impossible (AFAIK).
> 
> The original issue mentions using SIGQUIT in its title, however as
> Laszlo explained in the bug thread SIGQUIT is designed for debugging
> purposes and is less about quitting than generating a core file. The
> original request also explicitly mentioned that:
> 
> > If, for some reason SIGQUIT would not be accepted as the signal, take
> > any free to use signal, like USR1.
> 
> Qemu currently maps SIGTERM, SIGINT and SIGHUP to a brutal shutdown of
> the guest. This patch does not alter Qemu behavior for SIGTERM and
> SIGQUIT, and maps SIGHUP to the clean power off of the guest.
> 
> IMHO SIGTERM and SIGINT behavior should not be altered:
> 
> - SIGTERM is the preferred alternative to a SIGKILL to shutdown a stuck
>   guest.
> - SIGINT is mostly a matter of user expectation as it handles the Ctrl-C
>   sequence, I suppose that most users expect a brutal shutdown as it is
>   now but this is just a random guess.
> 
> SIGHUP is a more versatile signal, also used for instance to tell a
> daemon to reload its configuration files. From a functional point-of-
> view, for me it makes sense to use a "hang-up" signal to power off a
> guest, more than an impersonal USR1 signal, but this remains easily
> changeable would SIGHUP not be suitable for this purpose.
> 
> With this patch applied, it becomes possible to easily and cleanly shut-
> down Qemu virtual machines system-wide without involving any monitor:
> 
> 1. Send SIGHUP to all Qemu processes so the guests power off cleanly.
> 2. After a few time send SIGTERM to the remaining Qemu processes to
>    forcefully close stuck guests.
> 3. After a few time send SIGKILL to the remaining Qemu processes to
>    forcefully close stuck Qemu hypervisor processes.
> 
> I find this more convenient than having to tinker with Qemu monitor to
> implement step 1 as it must currently be done.
> 
> Signed-off-by: Simon Geusebroek <qemu.bugs@whitewinterwolf.com>
> ---
> diff -ur a/vl.c b/vl.c
> --- a/vl.c	2016-12-20 21:16:54.000000000 +0100
> +++ b/vl.c	2017-03-14 16:02:51.959911847 +0100
> @@ -1871,7 +1871,11 @@
>      /* Cannot call qemu_system_shutdown_request directly because
>       * we are in a signal handler.
>       */
> -    shutdown_requested = 1;
> +    if (signal == SIGHUP) {
> +        powerdown_requested = 1;
> +    } else {
> +        shutdown_requested = 1;
> +    }
>      qemu_notify_event();
>  }

While I understand your motivation this creates a semantic change for
existing users of QEMU. IOW anyone who is currently relying on use
of SIGHUP will experiance a regression when upgrading QEMU.

So if we want to signal to generate a clean shutdown, we need to pick
one that QEMU hasn't already set a specific behaviour for.

SIGQUIT could be a valid option, or the super generic SIGUSR1

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/ :|

  reply	other threads:[~2017-03-15 14:41 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-15 13:45 [Qemu-devel] [Bug 1217339] [PATCH] Unix signal to send ACPI-shutdown to Guest Simon
2017-03-15 14:41 ` Daniel P. Berrange [this message]
2017-03-15 17:46   ` Simon
2017-03-15 18:00     ` Peter Maydell
2017-03-15 18:08       ` Daniel P. Berrange
2017-03-15 18:45         ` Peter Maydell
2017-03-16 10:25           ` Simon
2017-03-16 10:26           ` Daniel P. Berrange
2017-03-18 13:41             ` [Qemu-devel] [Bug 1217339] [PATCH v2] " Simon
2017-03-20  9:55               ` Daniel P. Berrange
2017-03-21 12:30                 ` Simon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170315144124.GP7770@redhat.com \
    --to=berrange@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu.bugs@whitewinterwolf.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.