On Wed, Sep 6, 2023 at 4:27 PM Philippe Mathieu-Daudé <phi...@linaro.org>
wrote:
> Hi Albert,
>
> On 6/9/23 13:15, 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.
> >
> > Finally, refactor code to send response message so
> > that all common parts both for the common REPLY_ACK
> > case, and other data responses, can call it and
> > avoid code repetition.
> >
> > Signed-off-by: Albert Esteve <aest...@redhat.com>
> > ---
> > docs/interop/vhost-user.rst | 57 +++++++
> > hw/virtio/vhost-user.c | 174 ++++++++++++++++++++--
> > include/hw/virtio/vhost-backend.h | 3 +
> > subprojects/libvhost-user/libvhost-user.c | 118 +++++++++++++++
> > subprojects/libvhost-user/libvhost-user.h | 55 ++++++-
> > 5 files changed, 393 insertions(+), 14 deletions(-)
>
> Almost 400 lines of changes is too much for me to review in a
> single patch. Looking at the names, can't we split virtio VS
> libvhost-user?
>
Ack.
>
> > +static bool vhost_user_send_resp(QIOChannel *ioc, VhostUserHeader *hdr,
> > + VhostUserPayload *payload)
> > +{
> > + Error *local_err = NULL;
> > + struct iovec iov[] = {
> > + { .iov_base = hdr, .iov_len = VHOST_USER_HDR_SIZE },
> > + { .iov_base = payload, .iov_len = hdr->size },
> > + };
> > +
> > + hdr->flags &= ~VHOST_USER_NEED_REPLY_MASK;
> > + hdr->flags |= VHOST_USER_REPLY_MASK;
> > +
> > + if (qio_channel_writev_all(ioc, iov, ARRAY_SIZE(iov), &local_err)) {
> > + error_report_err(local_err);
> > + return false;
> > + }
> > +
> > + return true;
> > +}
>
> I note you ignored my comment regarding adding a 'Error **' argument in
> the previous version:
>
> https://lore.kernel.org/qemu-devel/911eef0c-d04f-2fcf-e78b-2475cd7af...@linaro.org/
>
> Sorry I missed those comments somehow :/
I'll check them and resend.
BR,
Albert
