On 09/13/2016 05:16 PM, Laine Stump wrote:
On 09/13/2016 10:43 AM, Cornelia Huck wrote:
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.

I totally agree that commonality is good and should tried to be reached. In this case it is already known that the compatibility setting legacy, transition and modern are not "common virtio" but virtio-pci only.

Looking at the current proposal the user finds in the documentation

<dt><code>driver</code></dt>
The <code>compatibility</code> attribute can be used to specify the compatibility of virtio devices. Allowed values are <code>legacy</code>, <code>transitional</code> and <code>modern</code>.
<span class="since">Since 2.2.0</span>.

This reads like a common virtio feature and when the user specifies it on non virtio-pci devices he ends up when starting the domain with an error message:
"... setting the virtio revision is only supported for PCI devices"

Next thing he asks himself: When is e.g. virtio-ccw going to support this feature?

I think that by renaming the driver attribute compatibility into pci-compatibility and also changing the documentation to clearly state that it is used "to specify the compatibility of virtio-pci devices only" would make it much easier for the users.



--
Mit freundlichen Grüßen/Kind regards
   Boris Fiuczynski

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martina Köderitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

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

Reply via email to