Re: [libvirt] [PATCHv5 00/13] qemu: allow disabling certain virtio revisions
On 09/13/2016 10:43 AM, Cornelia Huck wrote:
[I've browsed through the thread a bit, but as I'm not a
libvirt
developer I may be missing some basic things]
On Wed, 7 Sep 2016 16:34:17 -0400
Laine Stump <laine(a)laine.org> wrote:
> On 09/07/2016 03:38 PM, Sascha Silbe wrote:
>> Dear Laine,
>>
>> Laine Stump <laine(a)laine.org> writes:
>>
>>> On 09/07/2016 02:35 PM, Sascha Silbe wrote:
>>>> "Daniel P. Berrange" <berrange(a)redhat.com> writes:
>>>> [...]
>>>>> <sound model="virtio"/> == QEMU virtio
>>>>> <sound model="virtio1.0"/> == QEMU virtio +
disable-legacy
I'm not a fan of the virtio1.0 name, but that has already been
commented upon elsewhere.
>>>> What would this do for devices using the virtio-ccw transport?
>>> From libvirt's point of view, the option "disable-legacy=on"
would be
>>> added to the device's commandline argument.
>> Which would break s390x guests. virtio-ccw doesn't have any concept of
>> "legacy" or "modern" devices (that's purely a virtio-pci
concept), so
>> virtio-*-ccw devices don't recognise that switch:
> Okay, so you already know what would happen in qemu. Looking at Jan's
> code in this patch series, (which I didn't do before, but should have)
> when someone tries to set the option for disable-legacy=on when the
> device address is anything except PCI , it logs an error and fails.
Can't you make this a virtio-pci only switch? Or is that problem
occurring when expanding a generic virtio device?
Taking disks as an example (but it's the same for other devices), in
libvirt virtio-blk-pci, virtio-blk-ccw, virtio-blk-s390, and
virtio-blk-device (mmio) devices are all derived from a single base device:
<disk ....>
<target .... bus='virtio'/>
The choice between -pci, -ccw, -device, -s390 is made based on the type
of <address> element in the disk definition.
<address type='pci|ccw|virtio-s390|virtio-mmio' .../>
(no, I don't know why the s390 and mmio address types include "virtio-";
seems a bit redundant to me).
Even if that wasn't the case, we try to maintain consistency between the
formats of the different kinds of devices, so it's always possible
someone would try to set [whatever option we come up with] for the wrong
type of device (since in the end the configuration is just a text file).
If they do that, Jan's patches would log an error; so yesy, it is a
virtio-pci-only switch.
> No code for Dan's suggestion has been written yet, but if there's no
> concept of a legacy mode for virtio-*-ccw, then we would do the same
> thing. And also I would guess that libosinfo would never suggest that
> anyone try to add a "virtio1.0" model device to an s390 virtual machine).
The thing is that, unlike virtio-pci, we don't have any reason to
actually disable support for legacy virtio devices, and therefore don't
have a switch for it.
Yes.
>
>> silbe@oc4731375738:~$ ~/build/qemu-devel/x86_64-softmmu/qemu-system-x86_64
-device virtio-blk,help 2>&1 |grep legacy
>> virtio-blk-pci.disable-legacy=OnOffAuto (on/off/auto)
>> silbe@oc4731375738:~$ ~/build/qemu-devel/s390x-softmmu/qemu-system-s390x -device
virtio-blk,help 2>&1 |grep legacy
>>
>> That nicely illustrates the issue I have with a) mixing virtio-pci
>> legacy/modern into the model name and b) conflating it with virtio
>> 0.9/1.0 (or transitional/non-transitional for that matter).
>>
>> FWIW, the thing closest to virtio-pci legacy/modern is virtio-ccw
>> max_revision. But I doubt there's any reason to set this beyond
>> debugging and testing.
> Definitely - once we've added an option to libvirt, we have to keep it
> there forever - our backward compatibility guarantee requires it. So we
> don't want to add anything unless there's a clear use for it.
I don't see any reason why you'd want to make max_revision configurable
from libvirt. QEMU using it for compat machines is the only reason for
it.
Good.