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

[Cornelia Huck]

On Fri, 9 Sep 2016 14:02:17 +0300
Marcel Apfelbaum <address@hidden> wrote:

> I was thinking to keep the same function proposed by Maxime and change it to 
> negate things:
> 
> /*
>   * A missing feature before all negotiations finished will still be missing 
> at the end.
>   */
> bool virtio_test_backend_unsupported_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);
> }
> 
> We only check if the feature was not there from the start.

I think you'll still end up with the same problem (overindicating
unsupportedness), as you start out with an otherwise empty feature
set, causing features with dependencies to be removed. I fear that
anything starting with an incomplete feature list will have that
problem.

Maybe it would be better to limit this to the one bit we currently want
to test (VERSION_1)? We know the semantics of that one. Less general,
but also less headaches.