qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Marcel Apfelbaum
Subject: Re: [Qemu-devel] [PATCH 1/2] virtio: Add backend feature testing functionnality
Date: Fri, 9 Sep 2016 14:47:00 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.1.1

On 09/09/2016 02:36 PM, Maxime Coquelin wrote:


On 09/09/2016 01:20 PM, Cornelia Huck wrote:
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.

Yes, that could be the solution.
I agree that making it too generic might create confusion
with some features.


Sounds good to me. Let's go with it and we'll re-think it if
we'll find other feature negotiation issues later.

Thanks,
Marcel


Thanks,
Maxime





reply via email to

[Prev in Thread] Current Thread [Next in Thread]