Re: [libvirt] [PATCHv5 00/13] qemu: allow disabling certain virtio revisions
On 09/07/2016 12:40 PM, Daniel P. Berrange wrote:
On Wed, Sep 07, 2016 at 05:52:02PM +0200, Peter Krempa wrote:
> On Wed, Sep 07, 2016 at 16:13:57 +0100, Daniel Berrange wrote:
>> On Wed, Sep 07, 2016 at 05:10:01PM +0200, Peter Krempa wrote:
>>> On Wed, Aug 24, 2016 at 00:20:42 +0200, Ján Tomko wrote:
>>>> https://bugzilla.redhat.com/show_bug.cgi?id=1227354
> [...]
>
>>> Obviously making the user responsible for correct setting is much
>>> easier.
>>>
>>> Sigh. It'd be great if somebody else could also state their opinion on
>>> this. I can't say that I'm a big fan of this approach but it looks
like
>>> as if it would be the only sensible one.
>> I tend to agree. I'd be incredibly happy if we didn't add any of this to
>> the XML and would simply "do the best thing" automatically.
>>
>> I'm particularly not a fan of adding something "just in case there is a
>> bug"
> Yep, that's why I don't see a reason for using --disable-modern in
> libvirt. It does not have any value besides working around "possible
bugs".
>
> On the other hand, I'm kind of worried that with "--disable-legacy" we
> are entering the domain of policy decisions as it's uncertain to me
> whether:
> * Enabling it would break guests with very old drivers.
> * Guest firmware actually supports the modern mode (both ovmf and bios)
> * other possible caveats that I can't think of right now
>
> In such case we'd need to expose a knob to set it, but I'd prefer
> dropping the possibility to use legacy mode only since it's of no value.
We could take a completely different approach to this based on the model.
Effectively virtio-1.0 with disable-legacy is a completely separate hardware
from the original virtio-0.9, with or without disable-modern. You even get
a completely different PCI ID with disable-legacy set. So we could in fact
represent this in the XML as a different model called "virtio1.0", and
remap that to virtio+disable-legacy in the QEMU argv.
This is in fact what we've done with naming in libosinfo already, due to
the fact it has different PCI IDs and so is logically a different device.
IOW,
<sound model="virtio"/> == QEMU virtio
<sound model="virtio1.0"/> == QEMU virtio + disable-legacy
(you worried me for a minute, with your virtual ("nonexistent") virtio
sound device. I wondered if I had missed one!)
Yes, I agree it's a good idea for a different PCI device ID be
represented with a different device model. I just don't like the name
"virtio1.0", which will be obsolete and technically incorrect as soon as
there is a virtio-1.1. I can't think of anything better right now though.
And what about virtio input and video devices, which have a model
"virtio" that has from the beginning meant "legacy-free virtio" (since
they came into existence after virtio-1.0)?
Also, note that qemu's behavior in 2.7.0+ kind of muddies the waters a
bit - if you plug a virtio device into a PCI Express slot, it will
automatically get "disable-legacy=on", thus turning the "virtio"
device
into a "virtio1.0" device (ie changing the PCI device ID). This means
libvirt will forever more need to explicitly add "disable- legacy=off"
for virtio devices in PCI Express slots.
and explicitly don't provide any disable-modern flag mapping, since there
is no compelling use case for that.
Agreed.