Re: [Qemu-stable] [Qemu-devel] [PATCH 1/2] virtio: Add backend feature testing functionnality

[Cornelia Huck]

On Fri,  9 Sep 2016 12:14:31 +0200
Maxime Coquelin <address@hidden> wrote:

> This patch adds virtio_test_backend_feature() function to
> enable checking a backend feature before the negociation
> takes place.
> 
> It may be used, for example, to check whether the backend
> supports VIRTIO_F_VERSION_1 before enabling modern
> capabilities.
> 
> Cc: Marcel Apfelbaum <address@hidden>
> Cc: Michael S. Tsirkin <address@hidden>
> Cc: address@hidden
> Signed-off-by: Maxime Coquelin <address@hidden>
> ---
>  hw/virtio/virtio.c         | 14 ++++++++++++++
>  include/hw/virtio/virtio.h |  2 ++
>  2 files changed, 16 insertions(+)
> 
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 74c085c..7ab91a1 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -1481,6 +1481,20 @@ void virtio_vmstate_save(QEMUFile *f, void *opaque, 
> size_t size)
>      virtio_save(VIRTIO_DEVICE(opaque), f);
>  }
> 
> +bool virtio_test_backend_feature(VirtIODevice *vdev,
> +                                 unsigned int fbit, Error **errp)
> +{
> +    VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
> +    uint64_t feature;
> +
> +    virtio_add_feature(&feature, fbit);
> +
> +    assert(k->get_features != NULL);
> +    feature = k->get_features(vdev, feature, errp);
> +
> +    return virtio_has_feature(feature, fbit);
> +}
> +
>  static int virtio_set_features_nocheck(VirtIODevice *vdev, uint64_t val)
>  {
>      VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);

What happens if you want to test for features that depend upon each
other? The backend may support your feature, but it may withdraw the
feature bit if a dependency is not fullfilled.

You'll probably want to run validation on the whole feature set; but
that is hard if you're too early in the setup process.