On Fri, 9 Sep 2016 13:48:00 +0300
Marcel Apfelbaum <mar...@redhat.com> wrote:
> On 09/09/2016 01:33 PM, Cornelia Huck wrote:
> > On Fri, 9 Sep 2016 12:14:31 +0200
> > Maxime Coquelin <maxime.coque...@redhat.com> 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 <mar...@redhat.com>
> >> Cc: Michael S. Tsirkin <m...@redhat.com>
> >> Cc: qemu-sta...@nongnu.org
> >> Signed-off-by: Maxime Coquelin <maxime.coque...@redhat.com>
> >> ---
> >> 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.
> >
>
> While I agree with the feature dependency issue , would the negation be ok?
> What I mean is: if the backend does not support feature X, no
> matter what the depending features are, it will still not support it after
> the negotiation.
>
> Changing the function to virtio_backend_unsupported_feature(x) would be
> better?
I think yes, although that would mean we need a new query function that
pokes through all the layers, no?
