On Wed, Mar 30, 2022 at 10:12:42AM -0700, Si-Wei Liu wrote:
On 3/30/2022 9:24 AM, Stefano Garzarella wrote:
On Tue, Mar 29, 2022 at 11:33:17PM -0700, Si-Wei Liu wrote:
The vhost_vdpa_one_time_request() branch in
vhost_vdpa_set_backend_cap() incorrectly sends down
iotls on vhost_dev with non-zero index. This may
Little typo s/iotls/ioctls
Thanks! Will correct it in v2.
end up with multiple VHOST_SET_BACKEND_FEATURES
ioctl calls sent down on the vhost-vdpa fd that is
shared between all these vhost_dev's.
To fix it, send down ioctl only once via the first
vhost_dev with index 0. Toggle the polarity of the
vhost_vdpa_one_time_request() test would do the trick.
Signed-off-by: Si-Wei Liu <si-wei....@oracle.com>
---
hw/virtio/vhost-vdpa.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index c5ed7a3..27ea706 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -665,7 +665,7 @@ static int vhost_vdpa_set_backend_cap(struct
vhost_dev *dev)
features &= f;
- if (vhost_vdpa_one_time_request(dev)) {
+ if (!vhost_vdpa_one_time_request(dev)) {
r = vhost_vdpa_call(dev, VHOST_SET_BACKEND_FEATURES, &features);
if (r) {
return -EFAULT;
--
1.8.3.1
With that:
Reviewed-by: Stefano Garzarella <sgarz...@redhat.com>
Unrelated to this patch, but the name vhost_vdpa_one_time_request()
is confusing IMHO.
Coincidentally I got the same feeling and wanted to rename it to
something else, too.
Not that I'm good with names, but how about we change it to
vhost_vdpa_skip_one_time_request()?
How about vhost_vdpa_request_already_applied()? seems to be more
readable in the context.
That's fine too, except you can't discern that it's a single request per
device, so maybe I'd add "dev," but I don't know if it gets too long:
vhost_vdpa_dev_request_already_applied()
And I would also add a comment to the function to explain that we use
that function for requests that only need to be applied once, regardless
of the number of queues.
Thanks,
Stefano
