On 6/9/23 08:04, Philippe Mathieu-Daudé wrote:
> On 2/8/23 11:08, Albert Esteve wrote:
>> Add three new vhost-user protocol
>> `VHOST_USER_BACKEND_SHARED_OBJECT_* messages`.
>> These new messages are sent from vhost-user
>> back-ends to interact with the virtio-dmabuf
>> table in order to add or remove themselves as
>> virtio exporters, or lookup for virtio dma-buf
>> shared objects.
>>
>> The action taken in the front-end depends
>> on the type stored in the virtio shared
>> object hash table.
>>
>> When the table holds a pointer to a vhost
>> backend for a given UUID, the front-end sends
>> a VHOST_USER_GET_SHARED_OBJECT to the
>> backend holding the shared object.
>>
>> In the libvhost-user library we need to add
>> helper functions to allow sending messages to
>> interact with the virtio shared objects
>> hash table.
>>
>> The messages can only be sent after successfully
>> negotiating a new VHOST_USER_PROTOCOL_F_SHARED_OBJECT
>> vhost-user protocol feature bit.
>>
>> Signed-off-by: Albert Esteve <aesteve@redhat.com>
>> ---
>> docs/interop/vhost-user.rst | 57 ++++++++
>> hw/virtio/vhost-user.c | 166 ++++++++++++++++++++++
>> include/hw/virtio/vhost-backend.h | 3 +
>> subprojects/libvhost-user/libvhost-user.c | 118 +++++++++++++++
>> subprojects/libvhost-user/libvhost-user.h | 55 ++++++-
>> 5 files changed, 398 insertions(+), 1 deletion(-)
>
>
>> +static bool
>> +vhost_user_backend_send_dmabuf_fd(QIOChannel *ioc, VhostUserHeader *hdr,
>> + VhostUserPayload *payload)
>> +{
>> + Error *local_err = NULL;
>> + struct iovec iov[2];
>> +
>> + if (hdr->flags & VHOST_USER_NEED_REPLY_MASK) {
>> + hdr->flags &= ~VHOST_USER_NEED_REPLY_MASK;
>> + }
>> + hdr->flags |= VHOST_USER_REPLY_MASK;
>> +
>> + hdr->size = sizeof(payload->u64);
>> +
>> + iov[0].iov_base = hdr;
>> + iov[0].iov_len = VHOST_USER_HDR_SIZE;
>> + iov[1].iov_base = payload;
>> + iov[1].iov_len = hdr->size;
>> +
>> + if (qio_channel_writev_all(ioc, iov, ARRAY_SIZE(iov), &local_err)) {
>> + error_report_err(local_err);
>
> This function could have a 'Error **errp' parameter to propagate
> the error to the caller.
>
>> + return false;
>> + }
>> + return true;
>> +}
>> +
>> +static bool
>> +vhost_user_backend_send_dmabuf_fd(QIOChannel *ioc, VhostUserHeader *hdr,
>> + VhostUserPayload *payload)
>> +{
>> + hdr->size = sizeof(payload->u64);
>> + return vhost_user_send_resp(ioc, hdr, payload);
>> +}
>
> I'm confused by having two vhost_user_backend_send_dmabuf_fd() functions
> with different body...
This patch doesn't compile:
../../hw/virtio/vhost-user.c:1662:1: error: redefinition of
‘vhost_user_backend_send_dmabuf_fd’
1662 | vhost_user_backend_send_dmabuf_fd(QIOChannel *ioc,
VhostUserHeader *hdr,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../hw/virtio/vhost-user.c:1636:1: note: previous definition of
‘vhost_user_backend_send_dmabuf_fd’ with type ‘_Bool(QIOChannel *,
VhostUserHeader *, VhostUserPayload *)’
1636 | vhost_user_backend_send_dmabuf_fd(QIOChannel *ioc,
VhostUserHeader *hdr,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../hw/virtio/vhost-user.c: In function
‘vhost_user_backend_send_dmabuf_fd’:
../../hw/virtio/vhost-user.c:1666:12: error: implicit declaration of
function ‘vhost_user_send_resp’; did you mean ‘vhost_user_set_u64’?
[-Werror=implicit-function-declaration]
1666 | return vhost_user_send_resp(ioc, hdr, payload);
| ^~~~~~~~~~~~~~~~~~~~
| vhost_user_set_u64
../../hw/virtio/vhost-user.c:1666:12: error: nested extern declaration
of ‘vhost_user_send_resp’ [-Werror=nested-externs]
At top level:
../../hw/virtio/vhost-user.c:1636:1: error:
‘vhost_user_backend_send_dmabuf_fd’ defined but not used
[-Werror=unused-function]
1636 | vhost_user_backend_send_dmabuf_fd(QIOChannel *ioc,
VhostUserHeader *hdr,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
Uh, nice catch.
This was not happening before, but I did not try the patches individually for the few last reviews.
I will squash it as suggested with the next patch.
Thanks for checking!