All of lore.kernel.org
 help / color / mirror / Atom feed
From: Laszlo Ersek <lersek@redhat.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: pkrempa@redhat.com, marcel.a@redhat.com, libvir-list@redhat.com,
	hutao@cn.fujitsu.com, mst@redhat.com, qemu-devel@nongnu.org,
	armbru@redhat.com, rhod@redhat.com, kraxel@redhat.com,
	anthony@codemonkey.ws, lcapitulino@redhat.com, afaerber@suse.de
Subject: Re: [Qemu-devel] pvpanic plans?
Date: Thu, 22 Aug 2013 19:15:10 +0200	[thread overview]
Message-ID: <5216471E.7020209@redhat.com> (raw)
In-Reply-To: <1377187852-11192-1-git-send-email-pbonzini@redhat.com>

On 08/22/13 18:10, Paolo Bonzini wrote:
> The thread from yesterday has died off (perhaps also because of
> my inappropriate answer to Michael, for which I apologize to him
> and everyone).  I took some time to discuss the libvirt requirements
> further with Daniel Berrange and Eric Blake on IRC.  If anyone is
> interested, I can give logs.  This is a suggestion for how to
> proceed in both QEMU and libvirt.

The analysis is pretty overwhelming :)

I have read Anthony's response and I'm not trying to argue -- I'm just
spending a few thoughts on this and I'm willing to let them go to waste.

In general I think we should minimize the quirks the user (who edits the
libvirt XML) has to know about. Interactions between some XML elements,
without explicit inter-references (formulated as attributes, like
controller/index) are Bad (TM). So,

> == Builtin pvpanic ==
> 
> QEMU will remove pvpanic from pc-1.5 in 1.6.1 and 1.5.4.  This does not
> break migration.
> 
> 
> == Support in libvirt for current functionality ==
> 
> libvirt will add a <panic-notifier/> element, and possibly a capability
> for it accessible via "virsh capabilities".  There are two possibilities:
> 
> 1) On QEMU 1.5.4/1.6.1 and newer (and on QEMU 1.6.0 with a machine type
>    other than pc-1.5), <on_crash> will only work if the element is there.
>    On QEMU 1.5.0->1.5.3, and on QEMU 1.6.0 with the pc-1.5 machine type,
>    <on_crash> will be obeyed always, and may override e.g. reboot-on-panic
>    if a guest driver exist.

I don't like this because there's some interplay between on_crash and
panic_notifier, which even depends on the qemu version.


> 
> 2) On all versions, <on_crash> will only work if the element is there.

I like this, because, if on_crash doesn't work without panic_notifier
*at all*, then we can just drop panic_notifier, and make on_crash mean
(on_crash && panic_notifier) in the original sense.

IOW, drop "panic_notifier", and make "on_crash" work *always*.

> 
> 
> In turn, there are two ways to implement (2):
> 
> 2a) libvirt will always add -global pvpanic.iobase=0 to neutralize
>     the builtin pvpanic device if present.  <panic-notifier/>
>     will create the device with -device pvpanic,iobase=0x505
> 
>     Advantage: no changes to QEMU
> 
>     Disadvantage 1: writes to port 0 with QEMU 1.{5.0,5.1,5.2,5.3,6.0}
>       and pc-1.5 machine type will write to a pvpanic device instead of
>       the DMA controller.  Probably harmless, and limited to some QEMU
>       versions.
> 
>     Disadvantage 2: libvirt has knowledge of the pvpanic port number

Updating this paragraph with my above suggestion:

- (s/pvpanic.iobase/pvpanic.ioport/g)

- if "on_crash" is absent:
  - for 1.{5.0,5.1,5.2,5.3,6.0}, add -global pvpanic.ioport=0
  - for other versions, do nothing

- if "on_crash" is present:
  - for 1.{5.0,5.1,5.2,5.3,6.0}, do nothing,
  - for other versions, pass -device pvpanic
    (knowledge of 0x505 is unneeded)

"advantage" and "disadvantage 1" remain, "disadvantage 2" is gone.


> 2b) QEMU will provide a way for libvirt to detect that no machine type
>     has the builtin pvpanic.  If some machine type may have the builtin
>     pvpanic, and <panic-notifier/> is absent, libvirt will add
>     "-global pvpanic.iobase=0" to neutralize it.  Otherwise, libvirt
>     will create the device normally.
> 
>     A possible way for libvirt to detect "good" machine types is a
>     dummy property.  This is a bit ugly in that the property would not
>     affect the behavior of the device.  The property would remain in
>     the long term.
> 
>     Another possibility is for QEMU to rename the device, e.g. to
>     isa-pvpanic.  This is also somewhat gross, but not visible in the
>     long term when the "pvpanic" name will be lost in history.
> 
>     Advantage 1: libvirt has no knowledge of the pvpanic port number
> 
>     Disadvantage 1: same as above
> 
>     Disadvantage 2: need a somewhat gross change in QEMU
> 
> 
>     This method also provides an (also somewhat gross on the QEMU side)
>     way to detect other changes in the pvpanic semantics.  One example
>     mentioned below, is making the panicked state temporary.

Too much work in qemu, in order to introduce ugliness, to hide older
ugliness.

> == Possible improvements to pvpanic ==

That's too complex / far out for me now, sorry :)

Thanks,
Laszlo

  parent reply	other threads:[~2013-08-22 17:13 UTC|newest]

Thread overview: 54+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-08-22 16:10 [Qemu-devel] pvpanic plans? Paolo Bonzini
2013-08-22 16:44 ` Anthony Liguori
2013-08-22 17:53   ` Laszlo Ersek
2013-08-22 18:25     ` Anthony Liguori
2013-08-27  8:42       ` Richard W.M. Jones
2013-08-22 19:16     ` Paolo Bonzini
2013-08-22 20:09       ` Anthony Liguori
2013-08-22 20:36         ` Laszlo Ersek
2013-08-22 20:39           ` Anthony Liguori
2013-08-23  8:52             ` Paolo Bonzini
2013-08-22 21:08         ` Peter Maydell
2013-08-27  8:06         ` Richard W.M. Jones
2013-08-27 13:08           ` Ronen Hod
2013-08-27 13:20             ` Richard W.M. Jones
2013-08-27 13:26               ` Anthony Liguori
2013-08-27 13:57                 ` Richard W.M. Jones
2013-08-27 13:13       ` [Qemu-devel] [libvirt] " Daniel P. Berrange
2013-08-27 13:17         ` Anthony Liguori
2013-08-27 13:21         ` Richard W.M. Jones
2013-08-22 17:15 ` Laszlo Ersek [this message]
2013-08-22 18:33   ` [Qemu-devel] " Anthony Liguori
2013-08-22 19:44     ` Christian Borntraeger
2013-08-22 19:19   ` Paolo Bonzini
2013-08-22 19:41     ` Laszlo Ersek
2013-08-22 20:02       ` [Qemu-devel] [libvirt] " Eric Blake
2013-10-24  2:39 ` [Qemu-devel] " Hu Tao
2013-10-29 16:01   ` Markus Armbruster
2013-10-31 14:30   ` Michael S. Tsirkin
2013-10-31 14:32     ` Eric Blake
2013-10-31 14:34       ` Anthony Liguori
2013-10-31 14:39       ` Paolo Bonzini
2013-10-31 14:52         ` Michael S. Tsirkin
2013-10-31 14:56           ` Paolo Bonzini
2013-10-31 15:09             ` Michael S. Tsirkin
2013-10-31 15:26               ` Paolo Bonzini
2013-10-31 15:45                 ` Michael S. Tsirkin
2013-10-31 15:56                   ` Paolo Bonzini
2013-10-31 16:14                     ` Michael S. Tsirkin
2013-10-31 16:17                       ` Paolo Bonzini
2013-10-31 16:26                         ` Michael S. Tsirkin
2013-10-31 16:38                           ` Paolo Bonzini
2013-10-31 16:48                             ` Michael S. Tsirkin
2013-10-31 16:52                               ` Paolo Bonzini
2013-10-31 17:00                                 ` Michael S. Tsirkin
2013-10-31 17:09                                   ` Paolo Bonzini
2013-10-31 17:01                             ` Michael S. Tsirkin
2013-10-31 17:10                               ` Paolo Bonzini
2013-10-31 17:18                                 ` Michael S. Tsirkin
2013-10-31 18:03                                   ` Paolo Bonzini
2013-10-31 16:28                         ` Michael S. Tsirkin
2013-10-31 14:47       ` Michael S. Tsirkin
2013-10-31 14:49         ` Eric Blake
2013-10-31 15:07           ` Michael S. Tsirkin
2013-11-04  9:25     ` Christian Borntraeger

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=5216471E.7020209@redhat.com \
    --to=lersek@redhat.com \
    --cc=afaerber@suse.de \
    --cc=anthony@codemonkey.ws \
    --cc=armbru@redhat.com \
    --cc=hutao@cn.fujitsu.com \
    --cc=kraxel@redhat.com \
    --cc=lcapitulino@redhat.com \
    --cc=libvir-list@redhat.com \
    --cc=marcel.a@redhat.com \
    --cc=mst@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=pkrempa@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=rhod@redhat.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.