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

[Boris Fiuczynski]

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