Re: [libvirt] [PATCH 6/7] qemu: Support newer ivshmem device variants
On Wed, Sep 21, 2016 at 05:10:09PM +0200, Martin Kletzander wrote:
On Wed, Sep 21, 2016 at 04:01:23PM +0100, Daniel P. Berrange wrote:
> I'm not a fan of the idea of silently picking a different device
> for the guest behind the applications back. By not exposing the
> different device types with a "model" attribute, we miss a way
> to report to the application which models are supported by the
> QEMU they're using - eg via domain capabilities.
>
> This in turn means the application doesn't know whether they're
> getting the new or old version, and so don't know if they're going
> to have working migration or not.
>
> If we expanded the XML to include model, then application can
> explicitly request the new model (which supports migration) and
> know that they'll get a startup failure if not supported, as
> opposed to silently falling back to the non-migratable version.
>
> Also, it makes life hard for us if the ivshmem-plain device wants
> to support use of the 'server' attribute in the future, as we will
> then not know which to create.
>
> We've often been burnt in the past by trying todo magic like this,
> instead of explicitly representing stuff in the XML, so I think we
> should be being explicit about ivshmem models here.
>
> Of course, if we do add <model> support, we have to allow for it
> to be missing for sake of upgrades. So there's a question of which
> model we should select as the default, if not seen in the XML.
>
If selecting the newest one whenever the element is missing is fine,
then I'm OK with that. But that would change the device when upgrading
libvirt (without user intervention), which you didn't like IIUC.
Yes, I don't think we can do that - at least note exactly in the way
you do it in this patch. eg Looking at the ivshmem code in QEMU there
is this comment about guest interupt setup:
* Do nothing unless the device actually uses INTx. Here's how
* the device variants signal interrupts, what they put in PCI
* config space:
* Device variant Interrupt Interrupt Pin MSI-X cap.
* ivshmem-plain none 0 no
* ivshmem-doorbell MSI-X 1 yes(1)
* ivshmem,msi=off INTx 1 no
* ivshmem,msi=on MSI-X 1(2) yes(1)
* (1) if guest enabled MSI-X
* (2) the device lies
Note that neither ivshmem-plain or ivshmem-doorbell support use of
INTx for interupts. I'm also concerned about the footnote (2) there,
as that seems to imply that ivshmem,msi=on, is not in fact the
same as ivshmem-doorbell, because ivshmem lies about the interrupt
pin (whatever that means).
I'm inclined so that that we should do
if (ivshmem exists) {
use ivshmem
} else {
if (server) {
if(msi) {
use ivshmem-doorbell
} else {
error config unsupported
}
} else {
use ivshmem-plain
}
}
That way if a distro compiles-out 'ivshmem' we'll use one of the
new devices if they're available, otherwise we'll stick with the
conversative behaviour of using the legacy device for guaranteed
bug compatibility.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|