Re: [Qemu-devel] [PATCH v2 2/2] virtio-pci: Disable modern interface if backend without VIRTIO_F_VERSION_1

[Maxime Coquelin <maxime.coquelin@redhat.com>]

On 09/09/2016 07:48 PM, Michael S. Tsirkin wrote:
> On Fri, Sep 09, 2016 at 03:10:07PM +0200, Maxime Coquelin wrote:
>> This patch makes pci devices plugging more robust, by not confusing
>> guest with modern interface when the backend doesn't support
>> VIRTIO_F_VERSION_1.
>>
>> Cc: Cornelia Huck <cornelia.huck@de.ibm.com>
>> Cc: Marcel Apfelbaum <marcel@redhat.com>
>> Cc: Michael S. Tsirkin <mst@redhat.com>
>> Cc: qemu-stable@nongnu.org
>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>> ---
>>  hw/virtio/virtio-pci.c | 15 +++++++++++++++
>>  hw/virtio/virtio-pci.h |  5 +++++
>>  2 files changed, 20 insertions(+)
>>
>> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
>> index 755f921..9e88d7b 100644
>> --- a/hw/virtio/virtio-pci.c
>> +++ b/hw/virtio/virtio-pci.c
>> @@ -1581,6 +1581,21 @@ static void virtio_pci_device_plugged(DeviceState *d, Error **errp)
>>      uint32_t size;
>>      VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
>>
>> +    /*
>> +     * Virtio capabilities present without
>> +     * VIRTIO_F_VERSION_1 confuses guests
>> +     */
>> +    if (!virtio_test_backend_virtio_1(vdev, errp)) {
>> +        virtio_pci_disable_modern(proxy);
>> +    }
>> +
>> +    legacy = virtio_pci_legacy(proxy);
>> +    modern = virtio_pci_modern(proxy);
>> +    if (!legacy && !modern) {
>> +        error_setg(errp, "PCI device is neither legacy nor modern.");
>> +        return;
>> +    }
>> +
>
> How does this interact with
> 	virtio-pci: error out when both legacy and modern modes are disabled
> ?
> If it's the same, I'd rather pick that one and apply your
> change on top.
Not exactly, since with my patch modern can be disabled after realize
callback has been called.

But I will base my next revision on top of Greg's patch,
and use the same pattern as he did use.

Thanks,
Maxime