Re: [PATCHv5 00/13] qemu: allow disabling certain virtio revisions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxx> wrote:

On 09/07/2016 03:38 PM, Sascha Silbe wrote:
Dear Laine,

Laine Stump <laine@xxxxxxxxx> writes:

On 09/07/2016 02:35 PM, Sascha Silbe wrote:
"Daniel P. Berrange" <berrange@xxxxxxxxxx> 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.

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]