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

[Marcel Apfelbaum <marcel@redhat.com>]

On 09/09/2016 04:10 PM, 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);

I think the first initialization of the
legacy/modern variables is not necessary anymore.

Other than that it looks good to me.

Thanks,
Marcel

> +    if (!legacy && !modern) {
> +        error_setg(errp, "PCI device is neither legacy nor modern.");
> +        return;
> +    }
> +
>      config = proxy->pci_dev.config;
>      if (proxy->class_code) {
>          pci_config_set_class(config, proxy->class_code);
> diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h
> index 25fbf8a..4e976b3 100644
> --- a/hw/virtio/virtio-pci.h
> +++ b/hw/virtio/virtio-pci.h
> @@ -172,6 +172,11 @@ static inline void virtio_pci_force_virtio_1(VirtIOPCIProxy *proxy)
>      proxy->disable_legacy = ON_OFF_AUTO_ON;
>  }
>
> +static inline void virtio_pci_disable_modern(VirtIOPCIProxy *proxy)
> +{
> +    proxy->disable_modern = true;
> +}
> +
>  /*
>   * virtio-scsi-pci: This extends VirtioPCIProxy.
>   */
>