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

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

[Albert Esteve]

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
implementation violates the specification.

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

Fixes: 043e127a126bb3ceb5fc753deee27d261fd0c5ce
Fixes: 160947666276c5b7f6bca4d746bcac2966635d79
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Signed-off-by: Albert Esteve <aesteve@redhat.com>
---
 hw/virtio/vhost-user.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 00561daa06..96c5e6b95f 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -1607,9 +1607,14 @@ 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);
 }
 
+/*
+ * Handle VHOST_USER_BACKEND_SHARED_OBJECT_REMOVE backend requests.
+ *
+ * Return: 0 on success, 1 on error.
+ */
 static int
 vhost_user_backend_handle_shared_object_remove(struct vhost_dev *dev,
                                                VhostUserShared *object)
@@ -1623,16 +1628,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 1;
         }
         break;
     }
     default:
         /* Not allowed to remove non-owned entries */
-        return 0;
+        return 1;
     }
 
-    return virtio_remove_resource(&uuid);
+    return !virtio_remove_resource(&uuid);
 }
 
 static bool vhost_user_send_resp(QIOChannel *ioc, VhostUserHeader *hdr,
-- 
2.47.0

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

[Michael Tokarev]

22.10.2024 15:46, 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
> implementation violates the specification.
> 
> Revert the logic so that the implementation
> of the vhost-user handling methods matches
> the specification.
> 
> Fixes: 043e127a126bb3ceb5fc753deee27d261fd0c5ce
> Fixes: 160947666276c5b7f6bca4d746bcac2966635d79
> Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
> Signed-off-by: Albert Esteve <aesteve@redhat.com>

Is this worth the effort to pick up for stable
series?

>   hw/virtio/vhost-user.c | 13 +++++++++----
>   1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index 00561daa06..96c5e6b95f 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -1607,9 +1607,14 @@ 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);
>   }
>   
> +/*
> + * Handle VHOST_USER_BACKEND_SHARED_OBJECT_REMOVE backend requests.
> + *
> + * Return: 0 on success, 1 on error.
> + */
>   static int
>   vhost_user_backend_handle_shared_object_remove(struct vhost_dev *dev,
>                                                  VhostUserShared *object)
> @@ -1623,16 +1628,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 1;
>           }
>           break;
>       }
>       default:
>           /* Not allowed to remove non-owned entries */
> -        return 0;
> +        return 1;
>       }
>   
> -    return virtio_remove_resource(&uuid);
> +    return !virtio_remove_resource(&uuid);
>   }
>   
>   static bool vhost_user_send_resp(QIOChannel *ioc, VhostUserHeader *hdr,

-- 
GPG Key transition (from rsa2048 to rsa4096) since 2024-04-24.
New key: rsa4096/61AD3D98ECDF2C8E  9D8B E14E 3F2A 9DD7 9199  28F1 61AD 3D98 ECDF 2C8E
Old key: rsa2048/457CE0A0804465C5  6EE1 95D1 886E 8FFB 810D  4324 457C E0A0 8044 65C5
Transition statement: http://www.corpit.ru/mjt/gpg-transition-2024.txt