From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53697) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1biJLt-0001fo-ND for qemu-devel@nongnu.org; Fri, 09 Sep 2016 06:48:11 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1biJLp-0001yP-FF for qemu-devel@nongnu.org; Fri, 09 Sep 2016 06:48:08 -0400 References: <1473416072-7063-1-git-send-email-maxime.coquelin@redhat.com> <1473416072-7063-2-git-send-email-maxime.coquelin@redhat.com> <20160909123348.4779e59c.cornelia.huck@de.ibm.com> From: Marcel Apfelbaum Message-ID: Date: Fri, 9 Sep 2016 13:48:00 +0300 MIME-Version: 1.0 In-Reply-To: <20160909123348.4779e59c.cornelia.huck@de.ibm.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 1/2] virtio: Add backend feature testing functionnality List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Cornelia Huck , Maxime Coquelin Cc: mst@redhat.com, qemu-devel@nongnu.org, vkaplans@redhat.com, qemu-stable@nongnu.org On 09/09/2016 01:33 PM, Cornelia Huck wrote: > On Fri, 9 Sep 2016 12:14:31 +0200 > Maxime Coquelin 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 >> Cc: Michael S. Tsirkin >> Cc: qemu-stable@nongnu.org >> Signed-off-by: Maxime Coquelin >> --- >> 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? Thanks, Marcel