Re: [PATCH] vhost-user: fix shared object return values

[Stefano Garzarella]

On Wed, Oct 16, 2024 at 11:06:06AM +0200, Albert Esteve wrote:

VHOST_USER_BACKEND_SHARED_OBJECT_ADD and
VHOST_USER_BACKEND_SHARED_OBJECT_REMOVE state
in the spec that they return 0 for successful
operations, non-zero otherwise. However,
implementation relies on the return types
of the virtio-dmabuf library, with opposite
semantics (true if everything is correct,
false otherwise). Therefore, current implementaion

s/implementaion/implementation

I hadn't seen it ;-P found with:
./scripts/checkpatch.pl --strict --branch master..HEAD --codespell

violates the specification.

Revert the logic so that the implementation
of the vhost-user handling methods matches
the specification.

Fixes: 043e127a126bb3ceb5fc753deee27d261fd0c5ce

This is in from 9.0 ...

Fixes: 160947666276c5b7f6bca4d746bcac2966635d79

... and this from 8.2, so should we consider stable branches?

I think it depends if the backends are checking that return value.

Signed-off-by: Albert Esteve <aest...@redhat.com>
---
hw/virtio/vhost-user.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

Thanks for the fix!

Reviewed-by: Stefano Garzarella <sgarz...@redhat.com>

diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 00561daa06..90917352a4 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -1607,7 +1607,7 @@ vhost_user_backend_handle_shared_object_add(struct 
vhost_dev *dev,
    QemuUUID uuid;

    memcpy(uuid.data, object->uuid, sizeof(object->uuid));
-    return virtio_add_vhost_device(&uuid, dev);
+    return !virtio_add_vhost_device(&uuid, dev);
}

static int
@@ -1623,16 +1623,16 @@ vhost_user_backend_handle_shared_object_remove(struct 
vhost_dev *dev,
        struct vhost_dev *owner = virtio_lookup_vhost_device(&uuid);
        if (dev != owner) {
            /* Not allowed to remove non-owned entries */
-            return 0;
+            return -EPERM;
        }
        break;
    }
    default:
        /* Not allowed to remove non-owned entries */
-        return 0;
+        return -EPERM;
    }

-    return virtio_remove_resource(&uuid);
+    return !virtio_remove_resource(&uuid);
}

static bool vhost_user_send_resp(QIOChannel *ioc, VhostUserHeader *hdr,
--
2.46.1