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 <aesteve@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(-)
diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst
index 5a070adbc1..415bb47a19 100644
--- a/docs/interop/vhost-user.rst
+++ b/docs/interop/vhost-user.rst
@@ -1440,6 +1440,18 @@ Front-end message types
query the back-end for its device status as defined in the Virtio
specification.
+``VHOST_USER_GET_SHARED_OBJECT``
+ :id: 41
+ :equivalent ioctl: N/A
+ :request payload: ``struct VhostUserShared``
+ :reply payload: dmabuf fd
+
+ When the ``VHOST_USER_PROTOCOL_F_SHARED_OBJECT`` protocol
+ feature has been successfully negotiated, and the UUID is found
+ in the exporters cache, this message is submitted by the front-end
+ to retrieve a given dma-buf fd from a given back-end, determined by
+ the requested UUID. Back-end will reply passing the fd when the operation
+ is successful, or no fd otherwise.
Back-end message types
----------------------
@@ -1528,6 +1540,51 @@ is sent by the front-end.
The state.num field is currently reserved and must be set to 0.
+``VHOST_USER_BACKEND_SHARED_OBJECT_ADD``
+ :id: 6
+ :equivalent ioctl: N/A
+ :request payload: ``struct VhostUserShared``
+ :reply payload: N/A
+
+ When the ``VHOST_USER_PROTOCOL_F_SHARED_OBJECT`` protocol
+ feature has been successfully negotiated, this message can be submitted
+ by the backends to add themselves as exporters to the virtio shared lookup
+ table. The back-end device gets associated with a UUID in the shared table.
+ The back-end is responsible of keeping its own table with exported dma-buf fds.
+ When another back-end tries to import the resource associated with the UUID,
+ it will send a message to the front-end, which will act as a proxy to the
+ exporter back-end. If ``VHOST_USER_PROTOCOL_F_REPLY_ACK`` is negotiated, and
+ the back-end sets the ``VHOST_USER_NEED_REPLY`` flag, the front-end must
+ respond with zero when operation is successfully completed, or non-zero
+ otherwise.
+
+``VHOST_USER_BACKEND_SHARED_OBJECT_REMOVE``
+ :id: 7
+ :equivalent ioctl: N/A
+ :request payload: ``struct VhostUserShared``
+ :reply payload: N/A
+
+ When the ``VHOST_USER_PROTOCOL_F_SHARED_OBJECT`` protocol
+ feature has been successfully negotiated, this message can be submitted
+ by the backend to remove themselves from to the virtio-dmabuf shared
+ table API. The shared table will remove the back-end device associated with
+ the UUID. If ``VHOST_USER_PROTOCOL_F_REPLY_ACK`` is negotiated, and the
+ back-end sets the ``VHOST_USER_NEED_REPLY`` flag, the front-end must respond
+ with zero when operation is successfully completed, or non-zero otherwise.
+
+``VHOST_USER_BACKEND_SHARED_OBJECT_LOOKUP``
+ :id: 8
+ :equivalent ioctl: N/A
+ :request payload: ``struct VhostUserShared``
+ :reply payload: dmabuf fd and ``u64``
+
+ When the ``VHOST_USER_PROTOCOL_F_SHARED_OBJECT`` protocol
+ feature has been successfully negotiated, this message can be submitted
+ by the backends to retrieve a given dma-buf fd from the virtio-dmabuf
+ shared table given a UUID. Frontend will reply passing the fd and a zero
+ when the operation is successful, or non-zero otherwise. Note that if the
+ operation fails, no fd is sent to the backend.
+
.. _reply_ack:
VHOST_USER_PROTOCOL_F_REPLY_ACK
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 8dcf049d42..28fa0ace42 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -10,6 +10,7 @@
#include "qemu/osdep.h"
#include "qapi/error.h"
+#include "hw/virtio/virtio-dmabuf.h"
#include "hw/virtio/vhost.h"
#include "hw/virtio/virtio-crypto.h"
#include "hw/virtio/vhost-user.h"
@@ -21,6 +22,7 @@
#include "sysemu/kvm.h"
#include "qemu/error-report.h"
#include "qemu/main-loop.h"
+#include "qemu/uuid.h"
#include "qemu/sockets.h"
#include "sysemu/runstate.h"
#include "sysemu/cryptodev.h"
@@ -74,6 +76,7 @@ enum VhostUserProtocolFeature {
/* Feature 14 reserved for VHOST_USER_PROTOCOL_F_INBAND_NOTIFICATIONS. */
VHOST_USER_PROTOCOL_F_CONFIGURE_MEM_SLOTS = 15,
VHOST_USER_PROTOCOL_F_STATUS = 16,
+ VHOST_USER_PROTOCOL_F_SHARED_OBJECT = 17,
VHOST_USER_PROTOCOL_F_MAX
};
@@ -121,6 +124,7 @@ typedef enum VhostUserRequest {
VHOST_USER_REM_MEM_REG = 38,
VHOST_USER_SET_STATUS = 39,
VHOST_USER_GET_STATUS = 40,
+ VHOST_USER_GET_SHARED_OBJECT = 41,
VHOST_USER_MAX
} VhostUserRequest;
@@ -129,6 +133,9 @@ typedef enum VhostUserBackendRequest {
VHOST_USER_BACKEND_IOTLB_MSG = 1,
VHOST_USER_BACKEND_CONFIG_CHANGE_MSG = 2,
VHOST_USER_BACKEND_VRING_HOST_NOTIFIER_MSG = 3,
+ VHOST_USER_BACKEND_SHARED_OBJECT_ADD = 6,
+ VHOST_USER_BACKEND_SHARED_OBJECT_REMOVE = 7,
+ VHOST_USER_BACKEND_SHARED_OBJECT_LOOKUP = 8,
VHOST_USER_BACKEND_MAX
} VhostUserBackendRequest;
@@ -202,6 +209,10 @@ typedef struct VhostUserInflight {
uint16_t queue_size;
} VhostUserInflight;
+typedef struct VhostUserShared {
+ unsigned char uuid[16];
+} VhostUserShared;
+
typedef struct {
VhostUserRequest request;
@@ -226,6 +237,7 @@ typedef union {
VhostUserCryptoSession session;
VhostUserVringArea area;
VhostUserInflight inflight;
+ VhostUserShared object;
} VhostUserPayload;
typedef struct VhostUserMsg {
@@ -1601,6 +1613,144 @@ static int vhost_user_backend_handle_vring_host_notifier(struct vhost_dev *dev,
return 0;
}
+static int
+vhost_user_backend_handle_shared_object_add(struct vhost_dev *dev,
+ VhostUserShared *object)
+{
+ QemuUUID uuid;
+
+ memcpy(uuid.data, object->uuid, sizeof(object->uuid));
+ return virtio_add_vhost_device(&uuid, dev);
+}
+
+static int
+vhost_user_backend_handle_shared_object_remove(VhostUserShared *object)
+{
+ QemuUUID uuid;
+
+ memcpy(uuid.data, object->uuid, sizeof(object->uuid));
+ return virtio_remove_resource(&uuid);
+}
+
+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;
+}
+
+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);
+}
+
+int vhost_user_get_shared_object(struct vhost_dev *dev, unsigned char *uuid,
+ int *dmabuf_fd)
+{
+ struct vhost_user *u = dev->opaque;
+ CharBackend *chr = u->user->chr;
+ int ret;
+ VhostUserMsg msg = {
+ .hdr.request = VHOST_USER_GET_SHARED_OBJECT,
+ .hdr.flags = VHOST_USER_VERSION,
+ };
+ memcpy(msg.payload.object.uuid, uuid, sizeof(msg.payload.object.uuid));
+
+ ret = vhost_user_write(dev, &msg, NULL, 0);
+ if (ret < 0) {
+ return ret;
+ }
+
+ ret = vhost_user_read(dev, &msg);
+ if (ret < 0) {
+ return ret;
+ }
+
+ if (msg.hdr.request != VHOST_USER_GET_SHARED_OBJECT) {
+ error_report("Received unexpected msg type. "
+ "Expected %d received %d",
+ VHOST_USER_GET_SHARED_OBJECT, msg.hdr.request);
+ return -EPROTO;
+ }
+
+ *dmabuf_fd = qemu_chr_fe_get_msgfd(chr);
+ if (*dmabuf_fd < 0) {
+ error_report("Failed to get dmabuf fd");
+ return -EIO;
+ }
+
+ return 0;
+}
+
+static int
+vhost_user_backend_handle_shared_object_lookup(struct vhost_user *u,
+ QIOChannel *ioc,
+ VhostUserHeader *hdr,
+ VhostUserPayload *payload)
+{
+ QemuUUID uuid;
+ CharBackend *chr = u->user->chr;
+ int dmabuf_fd = -1;
+ int fd_num = 0;
+
+ memcpy(uuid.data, payload->object.uuid, sizeof(payload->object.uuid));
+
+ payload->u64 = 0;
+ switch (virtio_object_type(&uuid)) {
+ case TYPE_DMABUF:
+ dmabuf_fd = virtio_lookup_dmabuf(&uuid);
+ break;
+ case TYPE_VHOST_DEV:
+ {
+ struct vhost_dev *dev = virtio_lookup_vhost_device(&uuid);
+ if (dev == NULL) {
+ payload->u64 = -EINVAL;
+ break;
+ }
+ int ret = vhost_user_get_shared_object(dev, uuid.data, &dmabuf_fd);
+ if (ret < 0) {
+ payload->u64 = ret;
+ }
+ break;
+ }
+ case TYPE_INVALID:
+ payload->u64 = -EINVAL;
+ break;
+ }
+
+ if (dmabuf_fd != -1) {
+ fd_num++;
+ }
+
+ if (qemu_chr_fe_set_msgfds(chr, &dmabuf_fd, fd_num) < 0) {
+ error_report("Failed to set msg fds.");
+ payload->u64 = -EINVAL;
+ }
+
+ if (!vhost_user_backend_send_dmabuf_fd(ioc, hdr, payload)) {
+ error_report("Failed to write response msg.");
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
static void close_backend_channel(struct vhost_user *u)
{
g_source_destroy(u->backend_src);
@@ -1658,6 +1808,16 @@ static gboolean backend_read(QIOChannel *ioc, GIOCondition condition,
ret = vhost_user_backend_handle_vring_host_notifier(dev, &payload.area,
fd ? fd[0] : -1);
break;
+ case VHOST_USER_BACKEND_SHARED_OBJECT_ADD:
+ ret = vhost_user_backend_handle_shared_object_add(dev, &payload.object);
+ break;
+ case VHOST_USER_BACKEND_SHARED_OBJECT_REMOVE:
+ ret = vhost_user_backend_handle_shared_object_remove(&payload.object);
+ break;
+ case VHOST_USER_BACKEND_SHARED_OBJECT_LOOKUP:
+ ret = vhost_user_backend_handle_shared_object_lookup(dev->opaque, ioc,
+ &hdr, &payload);
+ break;
default:
error_report("Received unexpected msg type: %d.", hdr.request);
ret = -EINVAL;
@@ -1668,22 +1828,10 @@ static gboolean backend_read(QIOChannel *ioc, GIOCondition condition,
* directly in their request handlers.
*/
if (hdr.flags & VHOST_USER_NEED_REPLY_MASK) {
- struct iovec iovec[2];
-
-
- hdr.flags &= ~VHOST_USER_NEED_REPLY_MASK;
- hdr.flags |= VHOST_USER_REPLY_MASK;
-
payload.u64 = !!ret;
hdr.size = sizeof(payload.u64);
- iovec[0].iov_base = &hdr;
- iovec[0].iov_len = VHOST_USER_HDR_SIZE;
- iovec[1].iov_base = &payload;
- iovec[1].iov_len = hdr.size;
-
- if (qio_channel_writev_all(ioc, iovec, ARRAY_SIZE(iovec), &local_err)) {
- error_report_err(local_err);
+ if (!vhost_user_send_resp(ioc, &hdr, &payload)) {
goto err;
}
}
diff --git a/include/hw/virtio/vhost-backend.h b/include/hw/virtio/vhost-backend.h
index 31a251a9f5..1860b541d8 100644
--- a/include/hw/virtio/vhost-backend.h
+++ b/include/hw/virtio/vhost-backend.h
@@ -196,4 +196,7 @@ int vhost_backend_handle_iotlb_msg(struct vhost_dev *dev,
int vhost_user_gpu_set_socket(struct vhost_dev *dev, int fd);
+int vhost_user_get_shared_object(struct vhost_dev *dev, unsigned char *uuid,
+ int *dmabuf_fd);
+
#endif /* VHOST_BACKEND_H */
diff --git a/subprojects/libvhost-user/libvhost-user.c b/subprojects/libvhost-user/libvhost-user.c
index 0469a50101..432bb9fc0b 100644
--- a/subprojects/libvhost-user/libvhost-user.c
+++ b/subprojects/libvhost-user/libvhost-user.c
@@ -161,6 +161,7 @@ vu_request_to_string(unsigned int req)
REQ(VHOST_USER_GET_MAX_MEM_SLOTS),
REQ(VHOST_USER_ADD_MEM_REG),
REQ(VHOST_USER_REM_MEM_REG),
+ REQ(VHOST_USER_GET_SHARED_OBJECT),
REQ(VHOST_USER_MAX),
};
#undef REQ
@@ -900,6 +901,23 @@ vu_rem_mem_reg(VuDev *dev, VhostUserMsg *vmsg) {
return false;
}
+static bool
+vu_get_shared_object(VuDev *dev, VhostUserMsg *vmsg)
+{
+ int fd_num = 0;
+ int dmabuf_fd = -1;
+ if (dev->iface->get_shared_object) {
+ dmabuf_fd = dev->iface->get_shared_object(dev, &vmsg->payload.object.uuid[0]);
+ }
+ if (dmabuf_fd != -1) {
+ DPRINT("dmabuf_fd found for requested UUID\n");
+ vmsg->fds[fd_num++] = dmabuf_fd;
+ }
+ vmsg->fd_num = fd_num;
+
+ return true;
+}
+
static bool
vu_set_mem_table_exec_postcopy(VuDev *dev, VhostUserMsg *vmsg)
{
@@ -1403,6 +1421,104 @@ bool vu_set_queue_host_notifier(VuDev *dev, VuVirtq *vq, int fd,
return vu_process_message_reply(dev, &vmsg);
}
+bool
+vu_lookup_shared_object(VuDev *dev, unsigned char uuid[UUID_LEN], int *dmabuf_fd)
+{
+ bool result = false;
+ VhostUserMsg msg_reply;
+ VhostUserMsg msg = {
+ .request = VHOST_USER_BACKEND_SHARED_OBJECT_LOOKUP,
+ .size = sizeof(msg.payload.object),
+ .flags = VHOST_USER_VERSION | VHOST_USER_NEED_REPLY_MASK,
+ };
+
+ memcpy(msg.payload.object.uuid, uuid, sizeof(uuid[0]) * UUID_LEN);
+
+ if (!vu_has_protocol_feature(dev, VHOST_USER_PROTOCOL_F_SHARED_OBJECT)) {
+ return false;
+ }
+
+ pthread_mutex_lock(&dev->backend_mutex);
+ if (!vu_message_write(dev, dev->backend_fd, &msg)) {
+ goto out;
+ }
+
+ if (!vu_message_read_default(dev, dev->backend_fd, &msg_reply)) {
+ goto out;
+ }
+
+ if (msg_reply.request != msg.request) {
+ DPRINT("Received unexpected msg type. Expected %d, received %d",
+ msg.request, msg_reply.request);
+ goto out;
+ }
+
+ if (msg_reply.fd_num != 1) {
+ DPRINT("Received unexpected number of fds. Expected 1, received %d",
+ msg_reply.fd_num);
+ goto out;
+ }
+
+ *dmabuf_fd = msg_reply.fds[0];
+ result = *dmabuf_fd > 0 && msg_reply.payload.u64 == 0;
+out:
+ pthread_mutex_unlock(&dev->backend_mutex);
+
+ return result;
+}
+
+static bool
+vu_send_message(VuDev *dev, VhostUserMsg *vmsg)
+{
+ bool result = false;
+ pthread_mutex_lock(&dev->backend_mutex);
+ if (!vu_message_write(dev, dev->backend_fd, vmsg)) {
+ goto out;
+ }
+
+ result = true;
+out:
+ pthread_mutex_unlock(&dev->backend_mutex);
+
+ return result;
+}
+
+bool
+vu_add_shared_object(VuDev *dev, unsigned char uuid[UUID_LEN])
+{
+ VhostUserMsg msg = {
+ .request = VHOST_USER_BACKEND_SHARED_OBJECT_ADD,
+ .size = sizeof(msg.payload.object),
+ .flags = VHOST_USER_VERSION,
+ };
+
+ memcpy(msg.payload.object.uuid, uuid, sizeof(uuid[0]) * UUID_LEN);
+
+ if (!vu_has_protocol_feature(dev, VHOST_USER_PROTOCOL_F_SHARED_OBJECT)) {
+ return false;
+ }
+
+ return vu_send_message(dev, &msg);
+}
+
+bool
+vu_rm_shared_object(VuDev *dev, unsigned char uuid[UUID_LEN])
+{
+ VhostUserMsg msg = {
+ .request = VHOST_USER_BACKEND_SHARED_OBJECT_REMOVE,
+ .size = sizeof(msg.payload.object),
+ .flags = VHOST_USER_VERSION,
+ };
+
+ memcpy(msg.payload.object.uuid, uuid, sizeof(uuid[0]) * UUID_LEN);
+
+ if (!vu_has_protocol_feature(dev, VHOST_USER_PROTOCOL_F_SHARED_OBJECT)) {
+ return false;
+ }
+
+ return vu_send_message(dev, &msg);
+}
+
static bool
vu_set_vring_call_exec(VuDev *dev, VhostUserMsg *vmsg)
{
@@ -1943,6 +2059,8 @@ vu_process_message(VuDev *dev, VhostUserMsg *vmsg)
return vu_add_mem_reg(dev, vmsg);
case VHOST_USER_REM_MEM_REG:
return vu_rem_mem_reg(dev, vmsg);
+ case VHOST_USER_GET_SHARED_OBJECT:
+ return vu_get_shared_object(dev, vmsg);
default:
vmsg_close_fds(vmsg);
vu_panic(dev, "Unhandled request: %d", vmsg->request);
diff --git a/subprojects/libvhost-user/libvhost-user.h b/subprojects/libvhost-user/libvhost-user.h
index 708370c5f5..b36a42a7ca 100644
--- a/subprojects/libvhost-user/libvhost-user.h
+++ b/subprojects/libvhost-user/libvhost-user.h
@@ -64,7 +64,8 @@ enum VhostUserProtocolFeature {
VHOST_USER_PROTOCOL_F_INFLIGHT_SHMFD = 12,
VHOST_USER_PROTOCOL_F_INBAND_NOTIFICATIONS = 14,
VHOST_USER_PROTOCOL_F_CONFIGURE_MEM_SLOTS = 15,
-
+ /* Feature 16 is reserved for VHOST_USER_PROTOCOL_F_STATUS. */
+ VHOST_USER_PROTOCOL_F_SHARED_OBJECT = 17,
VHOST_USER_PROTOCOL_F_MAX
};
@@ -109,6 +110,7 @@ typedef enum VhostUserRequest {
VHOST_USER_GET_MAX_MEM_SLOTS = 36,
VHOST_USER_ADD_MEM_REG = 37,
VHOST_USER_REM_MEM_REG = 38,
+ VHOST_USER_GET_SHARED_OBJECT = 41,
VHOST_USER_MAX
} VhostUserRequest;
@@ -119,6 +121,9 @@ typedef enum VhostUserBackendRequest {
VHOST_USER_BACKEND_VRING_HOST_NOTIFIER_MSG = 3,
VHOST_USER_BACKEND_VRING_CALL = 4,
VHOST_USER_BACKEND_VRING_ERR = 5,
+ VHOST_USER_BACKEND_SHARED_OBJECT_ADD = 6,
+ VHOST_USER_BACKEND_SHARED_OBJECT_REMOVE = 7,
+ VHOST_USER_BACKEND_SHARED_OBJECT_LOOKUP = 8,
VHOST_USER_BACKEND_MAX
} VhostUserBackendRequest;
@@ -172,6 +177,12 @@ typedef struct VhostUserInflight {
uint16_t queue_size;
} VhostUserInflight;
+#define UUID_LEN 16
+
+typedef struct VhostUserShared {
+ unsigned char uuid[UUID_LEN];
+} VhostUserShared;
+
#if defined(_WIN32) && (defined(__x86_64__) || defined(__i386__))
# define VU_PACKED __attribute__((gcc_struct, packed))
#else
@@ -199,6 +210,7 @@ typedef struct VhostUserMsg {
VhostUserConfig config;
VhostUserVringArea area;
VhostUserInflight inflight;
+ VhostUserShared object;
} payload;
int fds[VHOST_MEMORY_BASELINE_NREGIONS];
@@ -232,6 +244,7 @@ typedef int (*vu_get_config_cb) (VuDev *dev, uint8_t *config, uint32_t len);
typedef int (*vu_set_config_cb) (VuDev *dev, const uint8_t *data,
uint32_t offset, uint32_t size,
uint32_t flags);
+typedef int (*vu_get_shared_object_cb) (VuDev *dev, const unsigned char *uuid);
typedef struct VuDevIface {
/* called by VHOST_USER_GET_FEATURES to get the features bitmask */
@@ -258,6 +271,8 @@ typedef struct VuDevIface {
vu_get_config_cb get_config;
/* set the config space of the device */
vu_set_config_cb set_config;
+ /* get virtio shared object from the underlying vhost implementation. */
+ vu_get_shared_object_cb get_shared_object;
} VuDevIface;
typedef void (*vu_queue_handler_cb) (VuDev *dev, int qidx);
@@ -541,6 +556,44 @@ void vu_set_queue_handler(VuDev *dev, VuVirtq *vq,
bool vu_set_queue_host_notifier(VuDev *dev, VuVirtq *vq, int fd,
int size, int offset);
+/**
+ * vu_lookup_shared_object:
+ * @dev: a VuDev context
+ * @uuid: UUID of the shared object
+ * @dmabuf_fd: output dma-buf file descriptor
+ *
+ * Lookup for a virtio shared object (i.e., dma-buf fd) associated with the
+ * received UUID. Result, if found, is stored in the dmabuf_fd argument.
+ *
+ * Returns: whether the virtio object was found.
+ */
+bool vu_lookup_shared_object(VuDev *dev, unsigned char uuid[UUID_LEN],
+ int *dmabuf_fd);
+
+/**
+ * vu_add_shared_object:
+ * @dev: a VuDev context
+ * @uuid: UUID of the shared object
+ *
+ * Registers this back-end as the exporter for the object associated with
+ * the received UUID.
+ *
+ * Returns: TRUE on success, FALSE on failure.
+ */
+bool vu_add_shared_object(VuDev *dev, unsigned char uuid[UUID_LEN]);
+
+/**
+ * vu_rm_shared_object:
+ * @dev: a VuDev context
+ * @uuid: UUID of the shared object
+ *
+ * Removes a shared object entry (i.e., back-end entry) associated with the
+ * received UUID key from the hash table.
+ *
+ * Returns: TRUE on success, FALSE on failure.
+ */
+bool vu_rm_shared_object(VuDev *dev, unsigned char uuid[UUID_LEN]);
+
/**
* vu_queue_set_notification:
* @dev: a VuDev context
--
2.41.0
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 <aesteve@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? > +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-2475cd7af8f0@linaro.org/
On Wed, Sep 6, 2023 at 4:27 PM Philippe Mathieu-Daudé <philmd@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 <aesteve@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-2475cd7af8f0@linaro.org/ > > Sorry I missed those comments somehow :/ I'll check them and resend. BR, Albert
On 6/9/23 16:33, Albert Esteve wrote: > 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-2475cd7af8f0@linaro.org/ <https://lore.kernel.org/qemu-devel/911eef0c-d04f-2fcf-e78b-2475cd7af8f0@linaro.org/> > > Sorry I missed those comments somehow :/ Ah, I see. > I'll check them and resend. You can also object to them, explaining why this isn't really useful, if you think so. But first read the big comment in include/qapi/error.h. Thanks, Phil.
On Wed, Sep 6, 2023 at 4:43 PM Philippe Mathieu-Daudé <philmd@linaro.org> wrote: > On 6/9/23 16:33, Albert Esteve wrote: > > > 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-2475cd7af8f0@linaro.org/ > < > https://lore.kernel.org/qemu-devel/911eef0c-d04f-2fcf-e78b-2475cd7af8f0@linaro.org/ > > > > > > Sorry I missed those comments somehow :/ > > Ah, I see. > > > I'll check them and resend. > > You can also object to them, explaining why this isn't really > useful, if you think so. But first read the big comment in > include/qapi/error.h. > > Sure, I understand. So far I tend to trust the judgement of the more experienced Qemu developers over my own, but if I wouldn't agree with what is suggested I would object :) So: - Regarding the two functions with the same, seems to be solved with the squash before, and it was probably causing the compile error to begin with, so one less thing to worry about! - Regarding splitting the commit, sure, no problem. I'll ensure they do compile separately. - Regarding the error, I read the long comment in the error file and checked surrounding code. I think you are right and will be better propagating the error. And I think I would address all your comments with that! Thanks for the feedback! > Thanks, > > Phil. > >
On Wed, Sep 6, 2023 at 6:01 PM Albert Esteve <aesteve@redhat.com> wrote: > > > On Wed, Sep 6, 2023 at 4:43 PM Philippe Mathieu-Daudé <philmd@linaro.org> > wrote: > >> On 6/9/23 16:33, Albert Esteve wrote: >> >> > 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-2475cd7af8f0@linaro.org/ >> < >> https://lore.kernel.org/qemu-devel/911eef0c-d04f-2fcf-e78b-2475cd7af8f0@linaro.org/ >> > >> > >> > Sorry I missed those comments somehow :/ >> >> Ah, I see. >> >> > I'll check them and resend. >> >> You can also object to them, explaining why this isn't really >> useful, if you think so. But first read the big comment in >> include/qapi/error.h. >> >> > Sure, I understand. So far I tend to trust the judgement of the more > experienced > Qemu developers over my own, but if I wouldn't agree with what is > suggested I would object :) > So: > - Regarding the two functions with the same, seems to be solved with the > squash before, > and it was probably causing the compile error to begin with, so one less > thing to worry about! > - Regarding splitting the commit, sure, no problem. I'll ensure they do > compile separately. > - Regarding the error, I read the long comment in the error file and > checked surrounding code. I think > you are right and will be better propagating the error. > But I think I will omit the Error propagation for `vhost_user_backend_handle_shared_object_lookup`. In this function returning an error code does not necessarily mean that we should log an error. So if we pass the local_err from the backend_read function to the handler, we cannot be sure when we should actually print the log. `vhost_backend_handle_iotlb_msg` has the same issue and does not propagate the error either, relies solely on `error_report` calls. Therefore, I will propagate it for `vhost_user_send_resp` and `vhost_user_backend_send_dmabuf_fd` only. > > And I think I would address all your comments with that! Thanks for the > feedback! > > >> Thanks, >> >> Phil. >> >>
On 6/9/23 18:18, Albert Esteve wrote: > So: > - Regarding the two functions with the same, seems to be solved with > the squash before, > and it was probably causing the compile error to begin with, so > one less thing to worry about! > - Regarding splitting the commit, sure, no problem. I'll ensure they > do compile separately. > - Regarding the error, I read the long comment in the error file and > checked surrounding code. I think > you are right and will be better propagating the error. > > > But I think I will omit the Error propagation for > `vhost_user_backend_handle_shared_object_lookup`. > In this function returning an error code does not necessarily mean that > we should log an error. > So if we pass the local_err from the backend_read function to the > handler, we cannot be sure > when we should actually print the log. > `vhost_backend_handle_iotlb_msg` has the same issue and does not > propagate the error either, > relies solely on `error_report` calls. > > Therefore, I will propagate it for `vhost_user_send_resp` and > `vhost_user_backend_send_dmabuf_fd` only. Sounds good!
© 2016 - 2024 Red Hat, Inc.