qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 00/12] vhost-user: support any POSIX system (tested on macOS, FreeBSD, OpenBSD)
@ 2024-05-08  7:44 Stefano Garzarella
  2024-05-08  7:44 ` [PATCH v4 01/12] libvhost-user: set msg.msg_control to NULL when it is empty Stefano Garzarella
                   ` (13 more replies)
  0 siblings, 14 replies; 37+ messages in thread
From: Stefano Garzarella @ 2024-05-08  7:44 UTC (permalink / raw)
  To: qemu-devel
  Cc: Jason Wang, Coiby Xu, Paolo Bonzini, qemu-block,
	Daniel P. Berrangé, Gerd Hoffmann, slp,
	Philippe Mathieu-Daudé, Brad Smith, Eduardo Habkost,
	Thomas Huth, Eric Blake, Kevin Wolf, Markus Armbruster,
	Raphael Norwitz, gmaglione, Laurent Vivier, stefanha,
	David Hildenbrand, Hanna Reitz, Michael S. Tsirkin, Igor Mammedov,
	Marc-André Lureau, Stefano Garzarella

v1: https://patchew.org/QEMU/20240228114759.44758-1-sgarzare@redhat.com/
v2: https://patchew.org/QEMU/20240326133936.125332-1-sgarzare@redhat.com/
v3: https://patchew.org/QEMU/20240404122330.92710-1-sgarzare@redhat.com/
v4:
  - rebased on master (commit e116b92d01c2cd75957a9f8ad1d4932292867b81)
  - added patch 6 to move using QEMU bswap helper functions in a separate
    patch (Phil)
  - fail if we find "share=off" in shm_backend_memory_alloc() (David)
  - added Phil's R-b and David's A-b

The vhost-user protocol is not really Linux-specific, so let's try support
QEMU's frontends and backends (including libvhost-user) in any POSIX system
with this series. The main use case is to be able to use virtio devices that
we don't have built-in in QEMU (e.g. virtiofsd, vhost-user-vsock, etc.) even
in non-Linux systems.

The first 5 patches are more like fixes discovered at runtime on macOS or
FreeBSD that could go even independently of this series.

Patches 6, 7, 8, and 9 enable building of frontends and backends (including
libvhost-user) with associated code changes to succeed in compilation.

Patch 10 adds `memory-backend-shm` that uses the POSIX shm_open() API to
create shared memory which is identified by an fd that can be shared with
vhost-user backends. This is useful on those systems (like macOS) where
we don't have memfd_create() or special filesystems like "/dev/shm".

Patches 11 and 12 use `memory-backend-shm` in some vhost-user tests.

Maybe the first 5 patches can go separately, but I only discovered those
problems after testing patches 6 - 9, so I have included them in this series
for now. Please let me know if you prefer that I send them separately.

I tested this series using vhost-user-blk and QSD on macOS Sonoma 14.4
(aarch64), FreeBSD 14 (x86_64), OpenBSD 7.4 (x86_64), and Fedora 39 (x86_64)
in this way:

- Start vhost-user-blk or QSD (same commands for all systems)

  vhost-user-blk -s /tmp/vhost.socket \
    -b Fedora-Cloud-Base-39-1.5.x86_64.raw

  qemu-storage-daemon \
    --blockdev file,filename=Fedora-Cloud-Base-39-1.5.x86_64.qcow2,node-name=file \
    --blockdev qcow2,file=file,node-name=qcow2 \
    --export vhost-user-blk,addr.type=unix,addr.path=/tmp/vhost.socket,id=vub,num-queues=1,node-name=qcow2,writable=on

- macOS (aarch64): start QEMU (using hvf accelerator)

  qemu-system-aarch64 -smp 2 -cpu host -M virt,accel=hvf,memory-backend=mem \
    -drive file=./build/pc-bios/edk2-aarch64-code.fd,if=pflash,format=raw,readonly=on \
    -device virtio-net-device,netdev=net0 -netdev user,id=net0 \
    -device ramfb -device usb-ehci -device usb-kbd \
    -object memory-backend-shm,id=mem,size=512M \
    -device vhost-user-blk-pci,num-queues=1,disable-legacy=on,chardev=char0 \
    -chardev socket,id=char0,path=/tmp/vhost.socket

- FreeBSD/OpenBSD (x86_64): start QEMU (no accelerators available)

  qemu-system-x86_64 -smp 2 -M q35,memory-backend=mem \
    -object memory-backend-shm,id=mem,size="512M" \
    -device vhost-user-blk-pci,num-queues=1,chardev=char0 \
    -chardev socket,id=char0,path=/tmp/vhost.socket

- Fedora (x86_64): start QEMU (using kvm accelerator)

  qemu-system-x86_64 -smp 2 -M q35,accel=kvm,memory-backend=mem \
    -object memory-backend-shm,size="512M" \
    -device vhost-user-blk-pci,num-queues=1,chardev=char0 \
    -chardev socket,id=char0,path=/tmp/vhost.socket

Branch pushed (and CI started) at https://gitlab.com/sgarzarella/qemu/-/tree/macos-vhost-user?ref_type=heads

Thanks,
Stefano

Stefano Garzarella (12):
  libvhost-user: set msg.msg_control to NULL when it is empty
  libvhost-user: fail vu_message_write() if sendmsg() is failing
  libvhost-user: mask F_INFLIGHT_SHMFD if memfd is not supported
  vhost-user-server: do not set memory fd non-blocking
  contrib/vhost-user-blk: fix bind() using the right size of the address
  contrib/vhost-user-*: use QEMU bswap helper functions
  vhost-user: enable frontends on any POSIX system
  libvhost-user: enable it on any POSIX system
  contrib/vhost-user-blk: enable it on any POSIX system
  hostmem: add a new memory backend based on POSIX shm_open()
  tests/qtest/vhost-user-blk-test: use memory-backend-shm
  tests/qtest/vhost-user-test: add a test case for memory-backend-shm

 docs/system/devices/vhost-user.rst        |   5 +-
 meson.build                               |   5 +-
 qapi/qom.json                             |  17 +++
 subprojects/libvhost-user/libvhost-user.h |   2 +-
 backends/hostmem-shm.c                    | 123 ++++++++++++++++++++++
 contrib/vhost-user-blk/vhost-user-blk.c   |  27 +++--
 contrib/vhost-user-input/main.c           |  16 +--
 hw/net/vhost_net.c                        |   5 +
 subprojects/libvhost-user/libvhost-user.c |  76 ++++++++++++-
 tests/qtest/vhost-user-blk-test.c         |   2 +-
 tests/qtest/vhost-user-test.c             |  23 ++++
 util/vhost-user-server.c                  |  12 +++
 backends/meson.build                      |   1 +
 hw/block/Kconfig                          |   2 +-
 qemu-options.hx                           |  13 +++
 util/meson.build                          |   4 +-
 16 files changed, 305 insertions(+), 28 deletions(-)
 create mode 100644 backends/hostmem-shm.c

-- 
2.45.0



^ permalink raw reply	[flat|nested] 37+ messages in thread

* [PATCH v4 01/12] libvhost-user: set msg.msg_control to NULL when it is empty
  2024-05-08  7:44 [PATCH v4 00/12] vhost-user: support any POSIX system (tested on macOS, FreeBSD, OpenBSD) Stefano Garzarella
@ 2024-05-08  7:44 ` Stefano Garzarella
  2024-05-08  8:57   ` Daniel P. Berrangé
  2024-05-08 10:23   ` Philippe Mathieu-Daudé
  2024-05-08  7:44 ` [PATCH v4 02/12] libvhost-user: fail vu_message_write() if sendmsg() is failing Stefano Garzarella
                   ` (12 subsequent siblings)
  13 siblings, 2 replies; 37+ messages in thread
From: Stefano Garzarella @ 2024-05-08  7:44 UTC (permalink / raw)
  To: qemu-devel
  Cc: Jason Wang, Coiby Xu, Paolo Bonzini, qemu-block,
	Daniel P. Berrangé, Gerd Hoffmann, slp,
	Philippe Mathieu-Daudé, Brad Smith, Eduardo Habkost,
	Thomas Huth, Eric Blake, Kevin Wolf, Markus Armbruster,
	Raphael Norwitz, gmaglione, Laurent Vivier, stefanha,
	David Hildenbrand, Hanna Reitz, Michael S. Tsirkin, Igor Mammedov,
	Marc-André Lureau, Stefano Garzarella

On some OS (e.g. macOS) sendmsg() returns -1 (errno EINVAL) if
the `struct msghdr` has the field `msg_controllen` set to 0, but
`msg_control` is not NULL.

Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
 subprojects/libvhost-user/libvhost-user.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/subprojects/libvhost-user/libvhost-user.c b/subprojects/libvhost-user/libvhost-user.c
index a879149fef..22bea0c775 100644
--- a/subprojects/libvhost-user/libvhost-user.c
+++ b/subprojects/libvhost-user/libvhost-user.c
@@ -632,6 +632,7 @@ vu_message_write(VuDev *dev, int conn_fd, VhostUserMsg *vmsg)
         memcpy(CMSG_DATA(cmsg), vmsg->fds, fdsize);
     } else {
         msg.msg_controllen = 0;
+        msg.msg_control = NULL;
     }
 
     do {
-- 
2.45.0



^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH v4 02/12] libvhost-user: fail vu_message_write() if sendmsg() is failing
  2024-05-08  7:44 [PATCH v4 00/12] vhost-user: support any POSIX system (tested on macOS, FreeBSD, OpenBSD) Stefano Garzarella
  2024-05-08  7:44 ` [PATCH v4 01/12] libvhost-user: set msg.msg_control to NULL when it is empty Stefano Garzarella
@ 2024-05-08  7:44 ` Stefano Garzarella
  2024-05-08  8:59   ` Daniel P. Berrangé
  2024-05-08 10:23   ` Philippe Mathieu-Daudé
  2024-05-08  7:44 ` [PATCH v4 03/12] libvhost-user: mask F_INFLIGHT_SHMFD if memfd is not supported Stefano Garzarella
                   ` (11 subsequent siblings)
  13 siblings, 2 replies; 37+ messages in thread
From: Stefano Garzarella @ 2024-05-08  7:44 UTC (permalink / raw)
  To: qemu-devel
  Cc: Jason Wang, Coiby Xu, Paolo Bonzini, qemu-block,
	Daniel P. Berrangé, Gerd Hoffmann, slp,
	Philippe Mathieu-Daudé, Brad Smith, Eduardo Habkost,
	Thomas Huth, Eric Blake, Kevin Wolf, Markus Armbruster,
	Raphael Norwitz, gmaglione, Laurent Vivier, stefanha,
	David Hildenbrand, Hanna Reitz, Michael S. Tsirkin, Igor Mammedov,
	Marc-André Lureau, Stefano Garzarella

In vu_message_write() we use sendmsg() to send the message header,
then a write() to send the payload.

If sendmsg() fails we should avoid sending the payload, since we
were unable to send the header.

Discovered before fixing the issue with the previous patch, where
sendmsg() failed on macOS due to wrong parameters, but the frontend
still sent the payload which the backend incorrectly interpreted
as a wrong header.

Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
 subprojects/libvhost-user/libvhost-user.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/subprojects/libvhost-user/libvhost-user.c b/subprojects/libvhost-user/libvhost-user.c
index 22bea0c775..a11afd1960 100644
--- a/subprojects/libvhost-user/libvhost-user.c
+++ b/subprojects/libvhost-user/libvhost-user.c
@@ -639,6 +639,11 @@ vu_message_write(VuDev *dev, int conn_fd, VhostUserMsg *vmsg)
         rc = sendmsg(conn_fd, &msg, 0);
     } while (rc < 0 && (errno == EINTR || errno == EAGAIN));
 
+    if (rc <= 0) {
+        vu_panic(dev, "Error while writing: %s", strerror(errno));
+        return false;
+    }
+
     if (vmsg->size) {
         do {
             if (vmsg->data) {
-- 
2.45.0



^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH v4 03/12] libvhost-user: mask F_INFLIGHT_SHMFD if memfd is not supported
  2024-05-08  7:44 [PATCH v4 00/12] vhost-user: support any POSIX system (tested on macOS, FreeBSD, OpenBSD) Stefano Garzarella
  2024-05-08  7:44 ` [PATCH v4 01/12] libvhost-user: set msg.msg_control to NULL when it is empty Stefano Garzarella
  2024-05-08  7:44 ` [PATCH v4 02/12] libvhost-user: fail vu_message_write() if sendmsg() is failing Stefano Garzarella
@ 2024-05-08  7:44 ` Stefano Garzarella
  2024-05-08 10:39   ` Philippe Mathieu-Daudé
  2024-05-08  7:44 ` [PATCH v4 04/12] vhost-user-server: do not set memory fd non-blocking Stefano Garzarella
                   ` (10 subsequent siblings)
  13 siblings, 1 reply; 37+ messages in thread
From: Stefano Garzarella @ 2024-05-08  7:44 UTC (permalink / raw)
  To: qemu-devel
  Cc: Jason Wang, Coiby Xu, Paolo Bonzini, qemu-block,
	Daniel P. Berrangé, Gerd Hoffmann, slp,
	Philippe Mathieu-Daudé, Brad Smith, Eduardo Habkost,
	Thomas Huth, Eric Blake, Kevin Wolf, Markus Armbruster,
	Raphael Norwitz, gmaglione, Laurent Vivier, stefanha,
	David Hildenbrand, Hanna Reitz, Michael S. Tsirkin, Igor Mammedov,
	Marc-André Lureau, Stefano Garzarella

libvhost-user will panic when receiving VHOST_USER_GET_INFLIGHT_FD
message if MFD_ALLOW_SEALING is not defined, since it's not able
to create a memfd.

VHOST_USER_GET_INFLIGHT_FD is used only if
VHOST_USER_PROTOCOL_F_INFLIGHT_SHMFD is negotiated. So, let's mask
that feature if the backend is not able to properly handle these
messages.

Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
 subprojects/libvhost-user/libvhost-user.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/subprojects/libvhost-user/libvhost-user.c b/subprojects/libvhost-user/libvhost-user.c
index a11afd1960..1c361ffd51 100644
--- a/subprojects/libvhost-user/libvhost-user.c
+++ b/subprojects/libvhost-user/libvhost-user.c
@@ -1674,6 +1674,16 @@ vu_get_protocol_features_exec(VuDev *dev, VhostUserMsg *vmsg)
         features |= dev->iface->get_protocol_features(dev);
     }
 
+    /*
+     * If MFD_ALLOW_SEALING is not defined, we are not able to handle
+     * VHOST_USER_GET_INFLIGHT_FD messages, since we can't create a memfd.
+     * Those messages are used only if VHOST_USER_PROTOCOL_F_INFLIGHT_SHMFD
+     * is negotiated. A device implementation can enable it, so let's mask
+     * it to avoid a runtime panic.
+     */
+#ifndef MFD_ALLOW_SEALING
+    features &= ~(1ULL << VHOST_USER_PROTOCOL_F_INFLIGHT_SHMFD);
+#endif
     vmsg_set_reply_u64(vmsg, features);
     return true;
 }
-- 
2.45.0



^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH v4 04/12] vhost-user-server: do not set memory fd non-blocking
  2024-05-08  7:44 [PATCH v4 00/12] vhost-user: support any POSIX system (tested on macOS, FreeBSD, OpenBSD) Stefano Garzarella
                   ` (2 preceding siblings ...)
  2024-05-08  7:44 ` [PATCH v4 03/12] libvhost-user: mask F_INFLIGHT_SHMFD if memfd is not supported Stefano Garzarella
@ 2024-05-08  7:44 ` Stefano Garzarella
  2024-05-08  9:00   ` Daniel P. Berrangé
  2024-05-08  7:44 ` [PATCH v4 05/12] contrib/vhost-user-blk: fix bind() using the right size of the address Stefano Garzarella
                   ` (9 subsequent siblings)
  13 siblings, 1 reply; 37+ messages in thread
From: Stefano Garzarella @ 2024-05-08  7:44 UTC (permalink / raw)
  To: qemu-devel
  Cc: Jason Wang, Coiby Xu, Paolo Bonzini, qemu-block,
	Daniel P. Berrangé, Gerd Hoffmann, slp,
	Philippe Mathieu-Daudé, Brad Smith, Eduardo Habkost,
	Thomas Huth, Eric Blake, Kevin Wolf, Markus Armbruster,
	Raphael Norwitz, gmaglione, Laurent Vivier, stefanha,
	David Hildenbrand, Hanna Reitz, Michael S. Tsirkin, Igor Mammedov,
	Marc-André Lureau, Stefano Garzarella

In vhost-user-server we set all fd received from the other peer
in non-blocking mode. For some of them (e.g. memfd, shm_open, etc.)
it's not really needed, because we don't use these fd with blocking
operations, but only to map memory.

In addition, in some systems this operation can fail (e.g. in macOS
setting an fd returned by shm_open() non-blocking fails with errno
= ENOTTY).

So, let's avoid setting fd non-blocking for those messages that we
know carry memory fd (e.g. VHOST_USER_ADD_MEM_REG,
VHOST_USER_SET_MEM_TABLE).

Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
v3:
- avoiding setting fd non-blocking for messages where we have memory fd
  (Eric)
---
 util/vhost-user-server.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/util/vhost-user-server.c b/util/vhost-user-server.c
index 3bfb1ad3ec..b19229074a 100644
--- a/util/vhost-user-server.c
+++ b/util/vhost-user-server.c
@@ -65,6 +65,18 @@ static void vmsg_close_fds(VhostUserMsg *vmsg)
 static void vmsg_unblock_fds(VhostUserMsg *vmsg)
 {
     int i;
+
+    /*
+     * These messages carry fd used to map memory, not to send/receive messages,
+     * so this operation is useless. In addition, in some systems this
+     * operation can fail (e.g. in macOS setting an fd returned by shm_open()
+     * non-blocking fails with errno = ENOTTY)
+     */
+    if (vmsg->request == VHOST_USER_ADD_MEM_REG ||
+        vmsg->request == VHOST_USER_SET_MEM_TABLE) {
+        return;
+    }
+
     for (i = 0; i < vmsg->fd_num; i++) {
         qemu_socket_set_nonblock(vmsg->fds[i]);
     }
-- 
2.45.0



^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH v4 05/12] contrib/vhost-user-blk: fix bind() using the right size of the address
  2024-05-08  7:44 [PATCH v4 00/12] vhost-user: support any POSIX system (tested on macOS, FreeBSD, OpenBSD) Stefano Garzarella
                   ` (3 preceding siblings ...)
  2024-05-08  7:44 ` [PATCH v4 04/12] vhost-user-server: do not set memory fd non-blocking Stefano Garzarella
@ 2024-05-08  7:44 ` Stefano Garzarella
  2024-05-08 10:25   ` Philippe Mathieu-Daudé
  2024-05-08  7:44 ` [PATCH v4 06/12] contrib/vhost-user-*: use QEMU bswap helper functions Stefano Garzarella
                   ` (8 subsequent siblings)
  13 siblings, 1 reply; 37+ messages in thread
From: Stefano Garzarella @ 2024-05-08  7:44 UTC (permalink / raw)
  To: qemu-devel
  Cc: Jason Wang, Coiby Xu, Paolo Bonzini, qemu-block,
	Daniel P. Berrangé, Gerd Hoffmann, slp,
	Philippe Mathieu-Daudé, Brad Smith, Eduardo Habkost,
	Thomas Huth, Eric Blake, Kevin Wolf, Markus Armbruster,
	Raphael Norwitz, gmaglione, Laurent Vivier, stefanha,
	David Hildenbrand, Hanna Reitz, Michael S. Tsirkin, Igor Mammedov,
	Marc-André Lureau, Stefano Garzarella

On macOS passing `-s /tmp/vhost.socket` parameter to the vhost-user-blk
application, the bind was done on `/tmp/vhost.socke` pathname,
missing the last character.

This sounds like one of the portability problems described in the
unix(7) manpage:

   Pathname sockets
       When  binding  a socket to a pathname, a few rules should
       be observed for maximum portability and ease of coding:

       •  The pathname in sun_path should be null-terminated.

       •  The length of the pathname, including the  terminating
          null byte, should not exceed the size of sun_path.

       •  The  addrlen  argument  that  describes  the enclosing
          sockaddr_un structure should have a value of at least:

              offsetof(struct sockaddr_un, sun_path) +
              strlen(addr.sun_path)+1

          or,  more  simply,  addrlen  can   be   specified   as
          sizeof(struct sockaddr_un).

So let's follow the last advice and simplify the code as well.

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
 contrib/vhost-user-blk/vhost-user-blk.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/contrib/vhost-user-blk/vhost-user-blk.c b/contrib/vhost-user-blk/vhost-user-blk.c
index 89e5f11a64..a8ab9269a2 100644
--- a/contrib/vhost-user-blk/vhost-user-blk.c
+++ b/contrib/vhost-user-blk/vhost-user-blk.c
@@ -469,7 +469,6 @@ static int unix_sock_new(char *unix_fn)
 {
     int sock;
     struct sockaddr_un un;
-    size_t len;
 
     assert(unix_fn);
 
@@ -481,10 +480,9 @@ static int unix_sock_new(char *unix_fn)
 
     un.sun_family = AF_UNIX;
     (void)snprintf(un.sun_path, sizeof(un.sun_path), "%s", unix_fn);
-    len = sizeof(un.sun_family) + strlen(un.sun_path);
 
     (void)unlink(unix_fn);
-    if (bind(sock, (struct sockaddr *)&un, len) < 0) {
+    if (bind(sock, (struct sockaddr *)&un, sizeof(un)) < 0) {
         perror("bind");
         goto fail;
     }
-- 
2.45.0



^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH v4 06/12] contrib/vhost-user-*: use QEMU bswap helper functions
  2024-05-08  7:44 [PATCH v4 00/12] vhost-user: support any POSIX system (tested on macOS, FreeBSD, OpenBSD) Stefano Garzarella
                   ` (4 preceding siblings ...)
  2024-05-08  7:44 ` [PATCH v4 05/12] contrib/vhost-user-blk: fix bind() using the right size of the address Stefano Garzarella
@ 2024-05-08  7:44 ` Stefano Garzarella
  2024-05-08 10:13   ` Philippe Mathieu-Daudé
  2024-05-08  7:44 ` [PATCH v4 07/12] vhost-user: enable frontends on any POSIX system Stefano Garzarella
                   ` (7 subsequent siblings)
  13 siblings, 1 reply; 37+ messages in thread
From: Stefano Garzarella @ 2024-05-08  7:44 UTC (permalink / raw)
  To: qemu-devel
  Cc: Jason Wang, Coiby Xu, Paolo Bonzini, qemu-block,
	Daniel P. Berrangé, Gerd Hoffmann, slp,
	Philippe Mathieu-Daudé, Brad Smith, Eduardo Habkost,
	Thomas Huth, Eric Blake, Kevin Wolf, Markus Armbruster,
	Raphael Norwitz, gmaglione, Laurent Vivier, stefanha,
	David Hildenbrand, Hanna Reitz, Michael S. Tsirkin, Igor Mammedov,
	Marc-André Lureau, Stefano Garzarella

Let's replace the calls to le*toh() and htole*() with qemu/bswap.h
helpers to make the code more portable.

Suggested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
 contrib/vhost-user-blk/vhost-user-blk.c |  9 +++++----
 contrib/vhost-user-input/main.c         | 16 ++++++++--------
 2 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/contrib/vhost-user-blk/vhost-user-blk.c b/contrib/vhost-user-blk/vhost-user-blk.c
index a8ab9269a2..9492146855 100644
--- a/contrib/vhost-user-blk/vhost-user-blk.c
+++ b/contrib/vhost-user-blk/vhost-user-blk.c
@@ -16,6 +16,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/bswap.h"
 #include "standard-headers/linux/virtio_blk.h"
 #include "libvhost-user-glib.h"
 
@@ -194,8 +195,8 @@ vub_discard_write_zeroes(VubReq *req, struct iovec *iov, uint32_t iovcnt,
     #if defined(__linux__) && defined(BLKDISCARD) && defined(BLKZEROOUT)
     VubDev *vdev_blk = req->vdev_blk;
     desc = buf;
-    uint64_t range[2] = { le64toh(desc->sector) << 9,
-                          le32toh(desc->num_sectors) << 9 };
+    uint64_t range[2] = { le64_to_cpu(desc->sector) << 9,
+                          le32_to_cpu(desc->num_sectors) << 9 };
     if (type == VIRTIO_BLK_T_DISCARD) {
         if (ioctl(vdev_blk->blk_fd, BLKDISCARD, range) == 0) {
             g_free(buf);
@@ -267,13 +268,13 @@ static int vub_virtio_process_req(VubDev *vdev_blk,
     req->in = (struct virtio_blk_inhdr *)elem->in_sg[in_num - 1].iov_base;
     in_num--;
 
-    type = le32toh(req->out->type);
+    type = le32_to_cpu(req->out->type);
     switch (type & ~VIRTIO_BLK_T_BARRIER) {
     case VIRTIO_BLK_T_IN:
     case VIRTIO_BLK_T_OUT: {
         ssize_t ret = 0;
         bool is_write = type & VIRTIO_BLK_T_OUT;
-        req->sector_num = le64toh(req->out->sector);
+        req->sector_num = le64_to_cpu(req->out->sector);
         if (is_write) {
             ret  = vub_writev(req, &elem->out_sg[1], out_num);
         } else {
diff --git a/contrib/vhost-user-input/main.c b/contrib/vhost-user-input/main.c
index 081230da54..f3362d41ac 100644
--- a/contrib/vhost-user-input/main.c
+++ b/contrib/vhost-user-input/main.c
@@ -51,8 +51,8 @@ static void vi_input_send(VuInput *vi, struct virtio_input_event *event)
     vi->queue[vi->qindex++].event = *event;
 
     /* ... until we see a report sync ... */
-    if (event->type != htole16(EV_SYN) ||
-        event->code != htole16(SYN_REPORT)) {
+    if (event->type != cpu_to_le16(EV_SYN) ||
+        event->code != cpu_to_le16(SYN_REPORT)) {
         return;
     }
 
@@ -103,9 +103,9 @@ vi_evdev_watch(VuDev *dev, int condition, void *data)
 
         g_debug("input %d %d %d", evdev.type, evdev.code, evdev.value);
 
-        virtio.type  = htole16(evdev.type);
-        virtio.code  = htole16(evdev.code);
-        virtio.value = htole32(evdev.value);
+        virtio.type  = cpu_to_le16(evdev.type);
+        virtio.code  = cpu_to_le16(evdev.code);
+        virtio.value = cpu_to_le32(evdev.value);
         vi_input_send(vi, &virtio);
     }
 }
@@ -124,9 +124,9 @@ static void vi_handle_status(VuInput *vi, virtio_input_event *event)
 
     evdev.input_event_sec = tval.tv_sec;
     evdev.input_event_usec = tval.tv_usec;
-    evdev.type = le16toh(event->type);
-    evdev.code = le16toh(event->code);
-    evdev.value = le32toh(event->value);
+    evdev.type = le16_to_cpu(event->type);
+    evdev.code = le16_to_cpu(event->code);
+    evdev.value = le32_to_cpu(event->value);
 
     rc = write(vi->evdevfd, &evdev, sizeof(evdev));
     if (rc == -1) {
-- 
2.45.0



^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH v4 07/12] vhost-user: enable frontends on any POSIX system
  2024-05-08  7:44 [PATCH v4 00/12] vhost-user: support any POSIX system (tested on macOS, FreeBSD, OpenBSD) Stefano Garzarella
                   ` (5 preceding siblings ...)
  2024-05-08  7:44 ` [PATCH v4 06/12] contrib/vhost-user-*: use QEMU bswap helper functions Stefano Garzarella
@ 2024-05-08  7:44 ` Stefano Garzarella
  2024-05-08 10:26   ` Philippe Mathieu-Daudé
  2024-05-08  7:44 ` [PATCH v4 08/12] libvhost-user: enable it " Stefano Garzarella
                   ` (6 subsequent siblings)
  13 siblings, 1 reply; 37+ messages in thread
From: Stefano Garzarella @ 2024-05-08  7:44 UTC (permalink / raw)
  To: qemu-devel
  Cc: Jason Wang, Coiby Xu, Paolo Bonzini, qemu-block,
	Daniel P. Berrangé, Gerd Hoffmann, slp,
	Philippe Mathieu-Daudé, Brad Smith, Eduardo Habkost,
	Thomas Huth, Eric Blake, Kevin Wolf, Markus Armbruster,
	Raphael Norwitz, gmaglione, Laurent Vivier, stefanha,
	David Hildenbrand, Hanna Reitz, Michael S. Tsirkin, Igor Mammedov,
	Marc-André Lureau, Stefano Garzarella

The vhost-user protocol is not really Linux-specific so let's enable
vhost-user frontends for any POSIX system.

In vhost_net.c we use VHOST_FILE_UNBIND which is defined in a Linux
specific header, let's define it for other systems as well.

Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
 meson.build        | 1 -
 hw/net/vhost_net.c | 5 +++++
 hw/block/Kconfig   | 2 +-
 3 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/meson.build b/meson.build
index 43da492372..955921dcb8 100644
--- a/meson.build
+++ b/meson.build
@@ -151,7 +151,6 @@ have_tpm = get_option('tpm') \
 
 # vhost
 have_vhost_user = get_option('vhost_user') \
-  .disable_auto_if(host_os != 'linux') \
   .require(host_os != 'windows',
            error_message: 'vhost-user is not available on Windows').allowed()
 have_vhost_vdpa = get_option('vhost_vdpa') \
diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index fd1a93701a..fced429813 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -34,8 +34,13 @@
 #include "standard-headers/linux/virtio_ring.h"
 #include "hw/virtio/vhost.h"
 #include "hw/virtio/virtio-bus.h"
+#if defined(__linux__)
 #include "linux-headers/linux/vhost.h"
+#endif
 
+#ifndef VHOST_FILE_UNBIND
+#define VHOST_FILE_UNBIND -1
+#endif
 
 /* Features supported by host kernel. */
 static const int kernel_feature_bits[] = {
diff --git a/hw/block/Kconfig b/hw/block/Kconfig
index 9e8f28f982..29ee09e434 100644
--- a/hw/block/Kconfig
+++ b/hw/block/Kconfig
@@ -40,7 +40,7 @@ config VHOST_USER_BLK
     bool
     # Only PCI devices are provided for now
     default y if VIRTIO_PCI
-    depends on VIRTIO && VHOST_USER && LINUX
+    depends on VIRTIO && VHOST_USER
 
 config SWIM
     bool
-- 
2.45.0



^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH v4 08/12] libvhost-user: enable it on any POSIX system
  2024-05-08  7:44 [PATCH v4 00/12] vhost-user: support any POSIX system (tested on macOS, FreeBSD, OpenBSD) Stefano Garzarella
                   ` (6 preceding siblings ...)
  2024-05-08  7:44 ` [PATCH v4 07/12] vhost-user: enable frontends on any POSIX system Stefano Garzarella
@ 2024-05-08  7:44 ` Stefano Garzarella
  2024-05-08 10:36   ` Philippe Mathieu-Daudé
  2024-05-08  7:44 ` [PATCH v4 09/12] contrib/vhost-user-blk: " Stefano Garzarella
                   ` (5 subsequent siblings)
  13 siblings, 1 reply; 37+ messages in thread
From: Stefano Garzarella @ 2024-05-08  7:44 UTC (permalink / raw)
  To: qemu-devel
  Cc: Jason Wang, Coiby Xu, Paolo Bonzini, qemu-block,
	Daniel P. Berrangé, Gerd Hoffmann, slp,
	Philippe Mathieu-Daudé, Brad Smith, Eduardo Habkost,
	Thomas Huth, Eric Blake, Kevin Wolf, Markus Armbruster,
	Raphael Norwitz, gmaglione, Laurent Vivier, stefanha,
	David Hildenbrand, Hanna Reitz, Michael S. Tsirkin, Igor Mammedov,
	Marc-André Lureau, Stefano Garzarella

The vhost-user protocol is not really Linux-specific so let's enable
libvhost-user for any POSIX system.

Compiling it on macOS and FreeBSD some problems came up:
- avoid to include linux/vhost.h which is avaibale only on Linux
  (vhost_types.h contains many of the things we need)
- macOS doesn't provide sys/endian.h, so let's define them
  (note: libvhost-user doesn't include qemu's headers, so we can't use
   use "qemu/bswap.h")
- define eventfd_[write|read] as write/read wrapper when system doesn't
  provide those (e.g. macOS)
- copy SEAL defines from include/qemu/memfd.h to make the code works
  on FreeBSD where MFD_ALLOW_SEALING is defined
- define MAP_NORESERVE if it's not defined (e.g. on FreeBSD)

Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
 meson.build                               |  2 +-
 subprojects/libvhost-user/libvhost-user.h |  2 +-
 subprojects/libvhost-user/libvhost-user.c | 60 +++++++++++++++++++++--
 3 files changed, 59 insertions(+), 5 deletions(-)

diff --git a/meson.build b/meson.build
index 955921dcb8..7954da5971 100644
--- a/meson.build
+++ b/meson.build
@@ -3168,7 +3168,7 @@ endif
 config_host_data.set('CONFIG_FDT', fdt.found())
 
 vhost_user = not_found
-if host_os == 'linux' and have_vhost_user
+if have_vhost_user
   libvhost_user = subproject('libvhost-user')
   vhost_user = libvhost_user.get_variable('vhost_user_dep')
 endif
diff --git a/subprojects/libvhost-user/libvhost-user.h b/subprojects/libvhost-user/libvhost-user.h
index deb40e77b3..e13e1d3931 100644
--- a/subprojects/libvhost-user/libvhost-user.h
+++ b/subprojects/libvhost-user/libvhost-user.h
@@ -18,9 +18,9 @@
 #include <stdbool.h>
 #include <stddef.h>
 #include <poll.h>
-#include <linux/vhost.h>
 #include <pthread.h>
 #include "standard-headers/linux/virtio_ring.h"
+#include "standard-headers/linux/vhost_types.h"
 
 /* Based on qemu/hw/virtio/vhost-user.c */
 #define VHOST_USER_F_PROTOCOL_FEATURES 30
diff --git a/subprojects/libvhost-user/libvhost-user.c b/subprojects/libvhost-user/libvhost-user.c
index 1c361ffd51..03edb4bf64 100644
--- a/subprojects/libvhost-user/libvhost-user.c
+++ b/subprojects/libvhost-user/libvhost-user.c
@@ -28,9 +28,7 @@
 #include <inttypes.h>
 #include <sys/types.h>
 #include <sys/socket.h>
-#include <sys/eventfd.h>
 #include <sys/mman.h>
-#include <endian.h>
 
 /* Necessary to provide VIRTIO_F_VERSION_1 on system
  * with older linux headers. Must appear before
@@ -39,8 +37,8 @@
 #include "standard-headers/linux/virtio_config.h"
 
 #if defined(__linux__)
+#include <endian.h>
 #include <sys/syscall.h>
-#include <fcntl.h>
 #include <sys/ioctl.h>
 #include <linux/vhost.h>
 #include <sys/vfs.h>
@@ -52,6 +50,62 @@
 
 #endif
 
+#if defined(__APPLE__) && (__MACH__)
+#include <libkern/OSByteOrder.h>
+#define htobe16(x) OSSwapHostToBigInt16(x)
+#define htole16(x) OSSwapHostToLittleInt16(x)
+#define be16toh(x) OSSwapBigToHostInt16(x)
+#define le16toh(x) OSSwapLittleToHostInt16(x)
+
+#define htobe32(x) OSSwapHostToBigInt32(x)
+#define htole32(x) OSSwapHostToLittleInt32(x)
+#define be32toh(x) OSSwapBigToHostInt32(x)
+#define le32toh(x) OSSwapLittleToHostInt32(x)
+
+#define htobe64(x) OSSwapHostToBigInt64(x)
+#define htole64(x) OSSwapHostToLittleInt64(x)
+#define be64toh(x) OSSwapBigToHostInt64(x)
+#define le64toh(x) OSSwapLittleToHostInt64(x)
+#endif
+
+#ifdef CONFIG_EVENTFD
+#include <sys/eventfd.h>
+#else
+#define eventfd_t uint64_t
+
+int eventfd_write(int fd, eventfd_t value)
+{
+    return (write(fd, &value, sizeof(value)) == sizeof(value)) ? 0 : -1;
+}
+
+int eventfd_read(int fd, eventfd_t *value)
+{
+    return (read(fd, value, sizeof(*value)) == sizeof(*value)) ? 0 : -1;
+}
+#endif
+
+#ifdef MFD_ALLOW_SEALING
+#include <fcntl.h>
+
+#ifndef F_LINUX_SPECIFIC_BASE
+#define F_LINUX_SPECIFIC_BASE 1024
+#endif
+
+#ifndef F_ADD_SEALS
+#define F_ADD_SEALS (F_LINUX_SPECIFIC_BASE + 9)
+#define F_GET_SEALS (F_LINUX_SPECIFIC_BASE + 10)
+
+#define F_SEAL_SEAL     0x0001  /* prevent further seals from being set */
+#define F_SEAL_SHRINK   0x0002  /* prevent file from shrinking */
+#define F_SEAL_GROW     0x0004  /* prevent file from growing */
+#define F_SEAL_WRITE    0x0008  /* prevent writes */
+#endif
+#endif
+
+#ifndef MAP_NORESERVE
+#define MAP_NORESERVE 0
+#endif
+
 #include "include/atomic.h"
 
 #include "libvhost-user.h"
-- 
2.45.0



^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH v4 09/12] contrib/vhost-user-blk: enable it on any POSIX system
  2024-05-08  7:44 [PATCH v4 00/12] vhost-user: support any POSIX system (tested on macOS, FreeBSD, OpenBSD) Stefano Garzarella
                   ` (7 preceding siblings ...)
  2024-05-08  7:44 ` [PATCH v4 08/12] libvhost-user: enable it " Stefano Garzarella
@ 2024-05-08  7:44 ` Stefano Garzarella
  2024-05-08 10:32   ` Philippe Mathieu-Daudé
  2024-05-08  7:44 ` [PATCH v4 10/12] hostmem: add a new memory backend based on POSIX shm_open() Stefano Garzarella
                   ` (4 subsequent siblings)
  13 siblings, 1 reply; 37+ messages in thread
From: Stefano Garzarella @ 2024-05-08  7:44 UTC (permalink / raw)
  To: qemu-devel
  Cc: Jason Wang, Coiby Xu, Paolo Bonzini, qemu-block,
	Daniel P. Berrangé, Gerd Hoffmann, slp,
	Philippe Mathieu-Daudé, Brad Smith, Eduardo Habkost,
	Thomas Huth, Eric Blake, Kevin Wolf, Markus Armbruster,
	Raphael Norwitz, gmaglione, Laurent Vivier, stefanha,
	David Hildenbrand, Hanna Reitz, Michael S. Tsirkin, Igor Mammedov,
	Marc-André Lureau, Stefano Garzarella

Let's make the code more portable by adding defines from
block/file-posix.c to support O_DIRECT in other systems (e.g. macOS).

vhost-user-server.c is a dependency, let's enable it for any POSIX
system.

Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
v4:
- moved using of "qemu/bswap.h" API in a separate patch [Phil]
---
 meson.build                             |  2 --
 contrib/vhost-user-blk/vhost-user-blk.c | 14 ++++++++++++++
 util/meson.build                        |  4 +++-
 3 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/meson.build b/meson.build
index 7954da5971..25047db3c1 100644
--- a/meson.build
+++ b/meson.build
@@ -1960,8 +1960,6 @@ has_statx = cc.has_header_symbol('sys/stat.h', 'STATX_BASIC_STATS', prefix: gnu_
 has_statx_mnt_id = cc.has_header_symbol('sys/stat.h', 'STATX_MNT_ID', prefix: gnu_source_prefix)
 
 have_vhost_user_blk_server = get_option('vhost_user_blk_server') \
-  .require(host_os == 'linux',
-           error_message: 'vhost_user_blk_server requires linux') \
   .require(have_vhost_user,
            error_message: 'vhost_user_blk_server requires vhost-user support') \
   .disable_auto_if(not have_tools and not have_system) \
diff --git a/contrib/vhost-user-blk/vhost-user-blk.c b/contrib/vhost-user-blk/vhost-user-blk.c
index 9492146855..a450337685 100644
--- a/contrib/vhost-user-blk/vhost-user-blk.c
+++ b/contrib/vhost-user-blk/vhost-user-blk.c
@@ -25,6 +25,20 @@
 #include <sys/ioctl.h>
 #endif
 
+/* OS X does not have O_DSYNC */
+#ifndef O_DSYNC
+#ifdef O_SYNC
+#define O_DSYNC O_SYNC
+#elif defined(O_FSYNC)
+#define O_DSYNC O_FSYNC
+#endif
+#endif
+
+/* Approximate O_DIRECT with O_DSYNC if O_DIRECT isn't available */
+#ifndef O_DIRECT
+#define O_DIRECT O_DSYNC
+#endif
+
 enum {
     VHOST_USER_BLK_MAX_QUEUES = 8,
 };
diff --git a/util/meson.build b/util/meson.build
index 2ad57b10ba..93054f2340 100644
--- a/util/meson.build
+++ b/util/meson.build
@@ -112,10 +112,12 @@ if have_block
     util_ss.add(files('filemonitor-stub.c'))
   endif
   if host_os == 'linux'
-    util_ss.add(files('vhost-user-server.c'), vhost_user)
     util_ss.add(files('vfio-helpers.c'))
     util_ss.add(files('chardev_open.c'))
   endif
+  if host_os != 'windows'
+    util_ss.add(files('vhost-user-server.c'), vhost_user)
+  endif
   util_ss.add(files('yank.c'))
 endif
 
-- 
2.45.0



^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH v4 10/12] hostmem: add a new memory backend based on POSIX shm_open()
  2024-05-08  7:44 [PATCH v4 00/12] vhost-user: support any POSIX system (tested on macOS, FreeBSD, OpenBSD) Stefano Garzarella
                   ` (8 preceding siblings ...)
  2024-05-08  7:44 ` [PATCH v4 09/12] contrib/vhost-user-blk: " Stefano Garzarella
@ 2024-05-08  7:44 ` Stefano Garzarella
  2024-05-08 11:59   ` Markus Armbruster
  2024-05-08  7:44 ` [PATCH v4 11/12] tests/qtest/vhost-user-blk-test: use memory-backend-shm Stefano Garzarella
                   ` (3 subsequent siblings)
  13 siblings, 1 reply; 37+ messages in thread
From: Stefano Garzarella @ 2024-05-08  7:44 UTC (permalink / raw)
  To: qemu-devel
  Cc: Jason Wang, Coiby Xu, Paolo Bonzini, qemu-block,
	Daniel P. Berrangé, Gerd Hoffmann, slp,
	Philippe Mathieu-Daudé, Brad Smith, Eduardo Habkost,
	Thomas Huth, Eric Blake, Kevin Wolf, Markus Armbruster,
	Raphael Norwitz, gmaglione, Laurent Vivier, stefanha,
	David Hildenbrand, Hanna Reitz, Michael S. Tsirkin, Igor Mammedov,
	Marc-André Lureau, Stefano Garzarella

shm_open() creates and opens a new POSIX shared memory object.
A POSIX shared memory object allows creating memory backend with an
associated file descriptor that can be shared with external processes
(e.g. vhost-user).

The new `memory-backend-shm` can be used as an alternative when
`memory-backend-memfd` is not available (Linux only), since shm_open()
should be provided by any POSIX-compliant operating system.

This backend mimics memfd, allocating memory that is practically
anonymous. In theory shm_open() requires a name, but this is allocated
for a short time interval and shm_unlink() is called right after
shm_open(). After that, only fd is shared with external processes
(e.g., vhost-user) as if it were associated with anonymous memory.

In the future we may also allow the user to specify the name to be
passed to shm_open(), but for now we keep the backend simple, mimicking
anonymous memory such as memfd.

Acked-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
v4
- fail if we find "share=off" in shm_backend_memory_alloc() [David]
v3
- enriched commit message and documentation to highlight that we
  want to mimic memfd (David)
---
 docs/system/devices/vhost-user.rst |   5 +-
 qapi/qom.json                      |  17 ++++
 backends/hostmem-shm.c             | 123 +++++++++++++++++++++++++++++
 backends/meson.build               |   1 +
 qemu-options.hx                    |  13 +++
 5 files changed, 157 insertions(+), 2 deletions(-)
 create mode 100644 backends/hostmem-shm.c

diff --git a/docs/system/devices/vhost-user.rst b/docs/system/devices/vhost-user.rst
index 9b2da106ce..35259d8ec7 100644
--- a/docs/system/devices/vhost-user.rst
+++ b/docs/system/devices/vhost-user.rst
@@ -98,8 +98,9 @@ Shared memory object
 
 In order for the daemon to access the VirtIO queues to process the
 requests it needs access to the guest's address space. This is
-achieved via the ``memory-backend-file`` or ``memory-backend-memfd``
-objects. A reference to a file-descriptor which can access this object
+achieved via the ``memory-backend-file``, ``memory-backend-memfd``, or
+``memory-backend-shm`` objects.
+A reference to a file-descriptor which can access this object
 will be passed via the socket as part of the protocol negotiation.
 
 Currently the shared memory object needs to match the size of the main
diff --git a/qapi/qom.json b/qapi/qom.json
index 38dde6d785..52df052df8 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -721,6 +721,19 @@
             '*hugetlbsize': 'size',
             '*seal': 'bool' } }
 
+##
+# @MemoryBackendShmProperties:
+#
+# Properties for memory-backend-shm objects.
+#
+# The @share boolean option is true by default with shm.
+#
+# Since: 9.1
+##
+{ 'struct': 'MemoryBackendShmProperties',
+  'base': 'MemoryBackendProperties',
+  'data': { } }
+
 ##
 # @MemoryBackendEpcProperties:
 #
@@ -985,6 +998,8 @@
     { 'name': 'memory-backend-memfd',
       'if': 'CONFIG_LINUX' },
     'memory-backend-ram',
+    { 'name': 'memory-backend-shm',
+      'if': 'CONFIG_POSIX' },
     'pef-guest',
     { 'name': 'pr-manager-helper',
       'if': 'CONFIG_LINUX' },
@@ -1056,6 +1071,8 @@
       'memory-backend-memfd':       { 'type': 'MemoryBackendMemfdProperties',
                                       'if': 'CONFIG_LINUX' },
       'memory-backend-ram':         'MemoryBackendProperties',
+      'memory-backend-shm':         { 'type': 'MemoryBackendShmProperties',
+                                      'if': 'CONFIG_POSIX' },
       'pr-manager-helper':          { 'type': 'PrManagerHelperProperties',
                                       'if': 'CONFIG_LINUX' },
       'qtest':                      'QtestProperties',
diff --git a/backends/hostmem-shm.c b/backends/hostmem-shm.c
new file mode 100644
index 0000000000..374edc3db8
--- /dev/null
+++ b/backends/hostmem-shm.c
@@ -0,0 +1,123 @@
+/*
+ * QEMU host POSIX shared memory object backend
+ *
+ * Copyright (C) 2024 Red Hat Inc
+ *
+ * Authors:
+ *   Stefano Garzarella <sgarzare@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "sysemu/hostmem.h"
+#include "qapi/error.h"
+
+#define TYPE_MEMORY_BACKEND_SHM "memory-backend-shm"
+
+OBJECT_DECLARE_SIMPLE_TYPE(HostMemoryBackendShm, MEMORY_BACKEND_SHM)
+
+struct HostMemoryBackendShm {
+    HostMemoryBackend parent_obj;
+};
+
+static bool
+shm_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
+{
+    g_autoptr(GString) shm_name = g_string_new(NULL);
+    g_autofree char *backend_name = NULL;
+    uint32_t ram_flags;
+    int fd, oflag;
+    mode_t mode;
+
+    if (!backend->size) {
+        error_setg(errp, "can't create shm backend with size 0");
+        return false;
+    }
+
+    if (!backend->share) {
+        error_setg(errp, "can't create shm backend with `share=off`");
+        return false;
+    }
+
+    /*
+     * Let's use `mode = 0` because we don't want other processes to open our
+     * memory unless we share the file descriptor with them.
+     */
+    mode = 0;
+    oflag = O_RDWR | O_CREAT | O_EXCL;
+    backend_name = host_memory_backend_get_name(backend);
+
+    /*
+     * Some operating systems allow creating anonymous POSIX shared memory
+     * objects (e.g. FreeBSD provides the SHM_ANON constant), but this is not
+     * defined by POSIX, so let's create a unique name.
+     *
+     * From Linux's shm_open(3) man-page:
+     *   For  portable  use,  a shared  memory  object should be identified
+     *   by a name of the form /somename;"
+     */
+    g_string_printf(shm_name, "/qemu-" FMT_pid "-shm-%s", getpid(),
+                    backend_name);
+
+    fd = shm_open(shm_name->str, oflag, mode);
+    if (fd < 0) {
+        error_setg_errno(errp, errno,
+                         "failed to create POSIX shared memory");
+        return false;
+    }
+
+    /*
+     * We have the file descriptor, so we no longer need to expose the
+     * POSIX shared memory object. However it will remain allocated as long as
+     * there are file descriptors pointing to it.
+     */
+    shm_unlink(shm_name->str);
+
+    if (ftruncate(fd, backend->size) == -1) {
+        error_setg_errno(errp, errno,
+                         "failed to resize POSIX shared memory to %" PRIu64,
+                         backend->size);
+        close(fd);
+        return false;
+    }
+
+    ram_flags = RAM_SHARED;
+    ram_flags |= backend->reserve ? 0 : RAM_NORESERVE;
+
+    return memory_region_init_ram_from_fd(&backend->mr, OBJECT(backend),
+                                              backend_name, backend->size,
+                                              ram_flags, fd, 0, errp);
+}
+
+static void
+shm_backend_instance_init(Object *obj)
+{
+    HostMemoryBackendShm *m = MEMORY_BACKEND_SHM(obj);
+
+    MEMORY_BACKEND(m)->share = true;
+}
+
+static void
+shm_backend_class_init(ObjectClass *oc, void *data)
+{
+    HostMemoryBackendClass *bc = MEMORY_BACKEND_CLASS(oc);
+
+    bc->alloc = shm_backend_memory_alloc;
+}
+
+static const TypeInfo shm_backend_info = {
+    .name = TYPE_MEMORY_BACKEND_SHM,
+    .parent = TYPE_MEMORY_BACKEND,
+    .instance_init = shm_backend_instance_init,
+    .class_init = shm_backend_class_init,
+    .instance_size = sizeof(HostMemoryBackendShm),
+};
+
+static void register_types(void)
+{
+    type_register_static(&shm_backend_info);
+}
+
+type_init(register_types);
diff --git a/backends/meson.build b/backends/meson.build
index 8b2b111497..3867b0d363 100644
--- a/backends/meson.build
+++ b/backends/meson.build
@@ -13,6 +13,7 @@ system_ss.add([files(
 if host_os != 'windows'
   system_ss.add(files('rng-random.c'))
   system_ss.add(files('hostmem-file.c'))
+  system_ss.add([files('hostmem-shm.c'), rt])
 endif
 if host_os == 'linux'
   system_ss.add(files('hostmem-memfd.c'))
diff --git a/qemu-options.hx b/qemu-options.hx
index cf61f6b863..2226172fb0 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -5227,6 +5227,19 @@ SRST
 
         The ``share`` boolean option is on by default with memfd.
 
+    ``-object memory-backend-shm,id=id,merge=on|off,dump=on|off,share=on|off,prealloc=on|off,size=size,host-nodes=host-nodes,policy=default|preferred|bind|interleave``
+        Creates a POSIX shared memory backend object, which allows
+        QEMU to share the memory with an external process (e.g. when
+        using vhost-user). This backend mimics memfd, allocating memory that is
+        practically anonymous. This is useful when memfd is not available.
+
+        Please refer to ``memory-backend-file`` for a description of the
+        options.
+
+        The ``share`` boolean option is on by default with shm. Setting it to
+        off will cause a failure during allocation because it is not supported
+        by this backend.
+
     ``-object iommufd,id=id[,fd=fd]``
         Creates an iommufd backend which allows control of DMA mapping
         through the ``/dev/iommu`` device.
-- 
2.45.0



^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH v4 11/12] tests/qtest/vhost-user-blk-test: use memory-backend-shm
  2024-05-08  7:44 [PATCH v4 00/12] vhost-user: support any POSIX system (tested on macOS, FreeBSD, OpenBSD) Stefano Garzarella
                   ` (9 preceding siblings ...)
  2024-05-08  7:44 ` [PATCH v4 10/12] hostmem: add a new memory backend based on POSIX shm_open() Stefano Garzarella
@ 2024-05-08  7:44 ` Stefano Garzarella
  2024-05-10  5:57   ` Thomas Huth
  2024-05-10 10:54   ` Philippe Mathieu-Daudé
  2024-05-08  7:44 ` [PATCH v4 12/12] tests/qtest/vhost-user-test: add a test case for memory-backend-shm Stefano Garzarella
                   ` (2 subsequent siblings)
  13 siblings, 2 replies; 37+ messages in thread
From: Stefano Garzarella @ 2024-05-08  7:44 UTC (permalink / raw)
  To: qemu-devel
  Cc: Jason Wang, Coiby Xu, Paolo Bonzini, qemu-block,
	Daniel P. Berrangé, Gerd Hoffmann, slp,
	Philippe Mathieu-Daudé, Brad Smith, Eduardo Habkost,
	Thomas Huth, Eric Blake, Kevin Wolf, Markus Armbruster,
	Raphael Norwitz, gmaglione, Laurent Vivier, stefanha,
	David Hildenbrand, Hanna Reitz, Michael S. Tsirkin, Igor Mammedov,
	Marc-André Lureau, Stefano Garzarella

`memory-backend-memfd` is available only on Linux while the new
`memory-backend-shm` can be used on any POSIX-compliant operating
system. Let's use it so we can run the test in multiple environments.

Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
 tests/qtest/vhost-user-blk-test.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qtest/vhost-user-blk-test.c b/tests/qtest/vhost-user-blk-test.c
index 117b9acd10..e945f6abf2 100644
--- a/tests/qtest/vhost-user-blk-test.c
+++ b/tests/qtest/vhost-user-blk-test.c
@@ -906,7 +906,7 @@ static void start_vhost_user_blk(GString *cmd_line, int vus_instances,
                            vhost_user_blk_bin);
 
     g_string_append_printf(cmd_line,
-            " -object memory-backend-memfd,id=mem,size=256M,share=on "
+            " -object memory-backend-shm,id=mem,size=256M,share=on "
             " -M memory-backend=mem -m 256M ");
 
     for (i = 0; i < vus_instances; i++) {
-- 
2.45.0



^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH v4 12/12] tests/qtest/vhost-user-test: add a test case for memory-backend-shm
  2024-05-08  7:44 [PATCH v4 00/12] vhost-user: support any POSIX system (tested on macOS, FreeBSD, OpenBSD) Stefano Garzarella
                   ` (10 preceding siblings ...)
  2024-05-08  7:44 ` [PATCH v4 11/12] tests/qtest/vhost-user-blk-test: use memory-backend-shm Stefano Garzarella
@ 2024-05-08  7:44 ` Stefano Garzarella
  2024-05-10  5:58   ` Thomas Huth
  2024-05-08 10:39 ` [PATCH v4 00/12] vhost-user: support any POSIX system (tested on macOS, FreeBSD, OpenBSD) Philippe Mathieu-Daudé
  2024-05-09 19:20 ` Stefan Hajnoczi
  13 siblings, 1 reply; 37+ messages in thread
From: Stefano Garzarella @ 2024-05-08  7:44 UTC (permalink / raw)
  To: qemu-devel
  Cc: Jason Wang, Coiby Xu, Paolo Bonzini, qemu-block,
	Daniel P. Berrangé, Gerd Hoffmann, slp,
	Philippe Mathieu-Daudé, Brad Smith, Eduardo Habkost,
	Thomas Huth, Eric Blake, Kevin Wolf, Markus Armbruster,
	Raphael Norwitz, gmaglione, Laurent Vivier, stefanha,
	David Hildenbrand, Hanna Reitz, Michael S. Tsirkin, Igor Mammedov,
	Marc-André Lureau, Stefano Garzarella

`memory-backend-shm` can be used with vhost-user devices, so let's
add a new test case for it.

Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
 tests/qtest/vhost-user-test.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/tests/qtest/vhost-user-test.c b/tests/qtest/vhost-user-test.c
index d4e437265f..8c1d903b2a 100644
--- a/tests/qtest/vhost-user-test.c
+++ b/tests/qtest/vhost-user-test.c
@@ -44,6 +44,8 @@
                         "mem-path=%s,share=on -numa node,memdev=mem"
 #define QEMU_CMD_MEMFD  " -m %d -object memory-backend-memfd,id=mem,size=%dM," \
                         " -numa node,memdev=mem"
+#define QEMU_CMD_SHM    " -m %d -object memory-backend-shm,id=mem,size=%dM," \
+                        " -numa node,memdev=mem"
 #define QEMU_CMD_CHR    " -chardev socket,id=%s,path=%s%s"
 #define QEMU_CMD_NETDEV " -netdev vhost-user,id=hs0,chardev=%s,vhostforce=on"
 
@@ -195,6 +197,7 @@ enum test_memfd {
     TEST_MEMFD_AUTO,
     TEST_MEMFD_YES,
     TEST_MEMFD_NO,
+    TEST_MEMFD_SHM,
 };
 
 static void append_vhost_net_opts(TestServer *s, GString *cmd_line,
@@ -228,6 +231,8 @@ static void append_mem_opts(TestServer *server, GString *cmd_line,
 
     if (memfd == TEST_MEMFD_YES) {
         g_string_append_printf(cmd_line, QEMU_CMD_MEMFD, size, size);
+    } else if (memfd == TEST_MEMFD_SHM) {
+        g_string_append_printf(cmd_line, QEMU_CMD_SHM, size, size);
     } else {
         const char *root = init_hugepagefs() ? : server->tmpfs;
 
@@ -788,6 +793,19 @@ static void *vhost_user_test_setup_memfd(GString *cmd_line, void *arg)
     return server;
 }
 
+static void *vhost_user_test_setup_shm(GString *cmd_line, void *arg)
+{
+    TestServer *server = test_server_new("vhost-user-test", arg);
+    test_server_listen(server);
+
+    append_mem_opts(server, cmd_line, 256, TEST_MEMFD_SHM);
+    server->vu_ops->append_opts(server, cmd_line, "");
+
+    g_test_queue_destroy(vhost_user_test_cleanup, server);
+
+    return server;
+}
+
 static void test_read_guest_mem(void *obj, void *arg, QGuestAllocator *alloc)
 {
     TestServer *server = arg;
@@ -1081,6 +1099,11 @@ static void register_vhost_user_test(void)
                  "virtio-net",
                  test_read_guest_mem, &opts);
 
+    opts.before = vhost_user_test_setup_shm;
+    qos_add_test("vhost-user/read-guest-mem/shm",
+                 "virtio-net",
+                 test_read_guest_mem, &opts);
+
     if (qemu_memfd_check(MFD_ALLOW_SEALING)) {
         opts.before = vhost_user_test_setup_memfd;
         qos_add_test("vhost-user/read-guest-mem/memfd",
-- 
2.45.0



^ permalink raw reply related	[flat|nested] 37+ messages in thread

* Re: [PATCH v4 01/12] libvhost-user: set msg.msg_control to NULL when it is empty
  2024-05-08  7:44 ` [PATCH v4 01/12] libvhost-user: set msg.msg_control to NULL when it is empty Stefano Garzarella
@ 2024-05-08  8:57   ` Daniel P. Berrangé
  2024-05-08  9:33     ` Stefano Garzarella
  2024-05-08 10:23   ` Philippe Mathieu-Daudé
  1 sibling, 1 reply; 37+ messages in thread
From: Daniel P. Berrangé @ 2024-05-08  8:57 UTC (permalink / raw)
  To: Stefano Garzarella
  Cc: qemu-devel, Jason Wang, Coiby Xu, Paolo Bonzini, qemu-block,
	Gerd Hoffmann, slp, Philippe Mathieu-Daudé, Brad Smith,
	Eduardo Habkost, Thomas Huth, Eric Blake, Kevin Wolf,
	Markus Armbruster, Raphael Norwitz, gmaglione, Laurent Vivier,
	stefanha, David Hildenbrand, Hanna Reitz, Michael S. Tsirkin,
	Igor Mammedov, Marc-André Lureau

On Wed, May 08, 2024 at 09:44:45AM +0200, Stefano Garzarella wrote:
> On some OS (e.g. macOS) sendmsg() returns -1 (errno EINVAL) if
> the `struct msghdr` has the field `msg_controllen` set to 0, but
> `msg_control` is not NULL.
> 
> Reviewed-by: Eric Blake <eblake@redhat.com>
> Reviewed-by: David Hildenbrand <david@redhat.com>
> Reviewed-by: Philippe Mathieu-Daud?? <philmd@linaro.org>

Philippe's name has got mangled here

> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
>  subprojects/libvhost-user/libvhost-user.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/subprojects/libvhost-user/libvhost-user.c b/subprojects/libvhost-user/libvhost-user.c
> index a879149fef..22bea0c775 100644
> --- a/subprojects/libvhost-user/libvhost-user.c
> +++ b/subprojects/libvhost-user/libvhost-user.c
> @@ -632,6 +632,7 @@ vu_message_write(VuDev *dev, int conn_fd, VhostUserMsg *vmsg)
>          memcpy(CMSG_DATA(cmsg), vmsg->fds, fdsize);
>      } else {
>          msg.msg_controllen = 0;
> +        msg.msg_control = NULL;
>      }
>  
>      do {
> -- 
> 2.45.0
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v4 02/12] libvhost-user: fail vu_message_write() if sendmsg() is failing
  2024-05-08  7:44 ` [PATCH v4 02/12] libvhost-user: fail vu_message_write() if sendmsg() is failing Stefano Garzarella
@ 2024-05-08  8:59   ` Daniel P. Berrangé
  2024-05-08 10:23   ` Philippe Mathieu-Daudé
  1 sibling, 0 replies; 37+ messages in thread
From: Daniel P. Berrangé @ 2024-05-08  8:59 UTC (permalink / raw)
  To: Stefano Garzarella
  Cc: qemu-devel, Jason Wang, Coiby Xu, Paolo Bonzini, qemu-block,
	Gerd Hoffmann, slp, Philippe Mathieu-Daudé, Brad Smith,
	Eduardo Habkost, Thomas Huth, Eric Blake, Kevin Wolf,
	Markus Armbruster, Raphael Norwitz, gmaglione, Laurent Vivier,
	stefanha, David Hildenbrand, Hanna Reitz, Michael S. Tsirkin,
	Igor Mammedov, Marc-André Lureau

On Wed, May 08, 2024 at 09:44:46AM +0200, Stefano Garzarella wrote:
> In vu_message_write() we use sendmsg() to send the message header,
> then a write() to send the payload.
> 
> If sendmsg() fails we should avoid sending the payload, since we
> were unable to send the header.
> 
> Discovered before fixing the issue with the previous patch, where
> sendmsg() failed on macOS due to wrong parameters, but the frontend
> still sent the payload which the backend incorrectly interpreted
> as a wrong header.
> 
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
>  subprojects/libvhost-user/libvhost-user.c | 5 +++++
>  1 file changed, 5 insertions(+)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v4 04/12] vhost-user-server: do not set memory fd non-blocking
  2024-05-08  7:44 ` [PATCH v4 04/12] vhost-user-server: do not set memory fd non-blocking Stefano Garzarella
@ 2024-05-08  9:00   ` Daniel P. Berrangé
  0 siblings, 0 replies; 37+ messages in thread
From: Daniel P. Berrangé @ 2024-05-08  9:00 UTC (permalink / raw)
  To: Stefano Garzarella
  Cc: qemu-devel, Jason Wang, Coiby Xu, Paolo Bonzini, qemu-block,
	Gerd Hoffmann, slp, Philippe Mathieu-Daudé, Brad Smith,
	Eduardo Habkost, Thomas Huth, Eric Blake, Kevin Wolf,
	Markus Armbruster, Raphael Norwitz, gmaglione, Laurent Vivier,
	stefanha, David Hildenbrand, Hanna Reitz, Michael S. Tsirkin,
	Igor Mammedov, Marc-André Lureau

On Wed, May 08, 2024 at 09:44:48AM +0200, Stefano Garzarella wrote:
> In vhost-user-server we set all fd received from the other peer
> in non-blocking mode. For some of them (e.g. memfd, shm_open, etc.)
> it's not really needed, because we don't use these fd with blocking
> operations, but only to map memory.
> 
> In addition, in some systems this operation can fail (e.g. in macOS
> setting an fd returned by shm_open() non-blocking fails with errno
> = ENOTTY).
> 
> So, let's avoid setting fd non-blocking for those messages that we
> know carry memory fd (e.g. VHOST_USER_ADD_MEM_REG,
> VHOST_USER_SET_MEM_TABLE).
> 
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
> v3:
> - avoiding setting fd non-blocking for messages where we have memory fd
>   (Eric)
> ---
>  util/vhost-user-server.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v4 01/12] libvhost-user: set msg.msg_control to NULL when it is empty
  2024-05-08  8:57   ` Daniel P. Berrangé
@ 2024-05-08  9:33     ` Stefano Garzarella
  0 siblings, 0 replies; 37+ messages in thread
From: Stefano Garzarella @ 2024-05-08  9:33 UTC (permalink / raw)
  To: Daniel P. Berrangé
  Cc: qemu-devel, Jason Wang, Coiby Xu, Paolo Bonzini, qemu-block,
	Gerd Hoffmann, slp, Philippe Mathieu-Daudé, Brad Smith,
	Eduardo Habkost, Thomas Huth, Eric Blake, Kevin Wolf,
	Markus Armbruster, Raphael Norwitz, gmaglione, Laurent Vivier,
	stefanha, David Hildenbrand, Hanna Reitz, Michael S. Tsirkin,
	Igor Mammedov, Marc-André Lureau

On Wed, May 08, 2024 at 09:57:13AM GMT, Daniel P. Berrangé wrote:
>On Wed, May 08, 2024 at 09:44:45AM +0200, Stefano Garzarella wrote:
>> On some OS (e.g. macOS) sendmsg() returns -1 (errno EINVAL) if
>> the `struct msghdr` has the field `msg_controllen` set to 0, but
>> `msg_control` is not NULL.
>>
>> Reviewed-by: Eric Blake <eblake@redhat.com>
>> Reviewed-by: David Hildenbrand <david@redhat.com>
>> Reviewed-by: Philippe Mathieu-Daud?? <philmd@linaro.org>
>
>Philippe's name has got mangled here

Thank you for bringing this to my attention and helping me solve it
off-list.

It should be fixed with the next posting!

Stefano

>
>> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
>> ---
>>  subprojects/libvhost-user/libvhost-user.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/subprojects/libvhost-user/libvhost-user.c b/subprojects/libvhost-user/libvhost-user.c
>> index a879149fef..22bea0c775 100644
>> --- a/subprojects/libvhost-user/libvhost-user.c
>> +++ b/subprojects/libvhost-user/libvhost-user.c
>> @@ -632,6 +632,7 @@ vu_message_write(VuDev *dev, int conn_fd, VhostUserMsg *vmsg)
>>          memcpy(CMSG_DATA(cmsg), vmsg->fds, fdsize);
>>      } else {
>>          msg.msg_controllen = 0;
>> +        msg.msg_control = NULL;
>>      }
>>
>>      do {
>> --
>> 2.45.0
>>
>
>With regards,
>Daniel
>-- 
>|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
>|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
>|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
>



^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v4 06/12] contrib/vhost-user-*: use QEMU bswap helper functions
  2024-05-08  7:44 ` [PATCH v4 06/12] contrib/vhost-user-*: use QEMU bswap helper functions Stefano Garzarella
@ 2024-05-08 10:13   ` Philippe Mathieu-Daudé
  2024-05-08 10:25     ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 37+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-05-08 10:13 UTC (permalink / raw)
  To: Stefano Garzarella, qemu-devel
  Cc: Jason Wang, Coiby Xu, Paolo Bonzini, qemu-block,
	Daniel P. Berrangé, Gerd Hoffmann, slp, Brad Smith,
	Eduardo Habkost, Thomas Huth, Eric Blake, Kevin Wolf,
	Markus Armbruster, Raphael Norwitz, gmaglione, Laurent Vivier,
	stefanha, David Hildenbrand, Hanna Reitz, Michael S. Tsirkin,
	Igor Mammedov, Marc-André Lureau

On 8/5/24 09:44, Stefano Garzarella wrote:
> Let's replace the calls to le*toh() and htole*() with qemu/bswap.h
> helpers to make the code more portable.
> 
> Suggested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
>   contrib/vhost-user-blk/vhost-user-blk.c |  9 +++++----
>   contrib/vhost-user-input/main.c         | 16 ++++++++--------
>   2 files changed, 13 insertions(+), 12 deletions(-)

Thanks,

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>



^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v4 02/12] libvhost-user: fail vu_message_write() if sendmsg() is failing
  2024-05-08  7:44 ` [PATCH v4 02/12] libvhost-user: fail vu_message_write() if sendmsg() is failing Stefano Garzarella
  2024-05-08  8:59   ` Daniel P. Berrangé
@ 2024-05-08 10:23   ` Philippe Mathieu-Daudé
  1 sibling, 0 replies; 37+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-05-08 10:23 UTC (permalink / raw)
  To: Stefano Garzarella, qemu-devel
  Cc: Jason Wang, Coiby Xu, Paolo Bonzini, qemu-block,
	Daniel P. Berrangé, Gerd Hoffmann, slp, Brad Smith,
	Eduardo Habkost, Thomas Huth, Eric Blake, Kevin Wolf,
	Markus Armbruster, Raphael Norwitz, gmaglione, Laurent Vivier,
	stefanha, David Hildenbrand, Hanna Reitz, Michael S. Tsirkin,
	Igor Mammedov, Marc-André Lureau

On 8/5/24 09:44, Stefano Garzarella wrote:
> In vu_message_write() we use sendmsg() to send the message header,
> then a write() to send the payload.
> 
> If sendmsg() fails we should avoid sending the payload, since we
> were unable to send the header.
> 
> Discovered before fixing the issue with the previous patch, where
> sendmsg() failed on macOS due to wrong parameters, but the frontend
> still sent the payload which the backend incorrectly interpreted
> as a wrong header.
> 
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
>   subprojects/libvhost-user/libvhost-user.c | 5 +++++
>   1 file changed, 5 insertions(+)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>



^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v4 01/12] libvhost-user: set msg.msg_control to NULL when it is empty
  2024-05-08  7:44 ` [PATCH v4 01/12] libvhost-user: set msg.msg_control to NULL when it is empty Stefano Garzarella
  2024-05-08  8:57   ` Daniel P. Berrangé
@ 2024-05-08 10:23   ` Philippe Mathieu-Daudé
  1 sibling, 0 replies; 37+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-05-08 10:23 UTC (permalink / raw)
  To: Stefano Garzarella, qemu-devel
  Cc: Jason Wang, Coiby Xu, Paolo Bonzini, qemu-block,
	Daniel P. Berrangé, Gerd Hoffmann, slp, Brad Smith,
	Eduardo Habkost, Thomas Huth, Eric Blake, Kevin Wolf,
	Markus Armbruster, Raphael Norwitz, gmaglione, Laurent Vivier,
	stefanha, David Hildenbrand, Hanna Reitz, Michael S. Tsirkin,
	Igor Mammedov, Marc-André Lureau

On 8/5/24 09:44, Stefano Garzarella wrote:
> On some OS (e.g. macOS) sendmsg() returns -1 (errno EINVAL) if
> the `struct msghdr` has the field `msg_controllen` set to 0, but
> `msg_control` is not NULL.
> 
> Reviewed-by: Eric Blake <eblake@redhat.com>
> Reviewed-by: David Hildenbrand <david@redhat.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
>   subprojects/libvhost-user/libvhost-user.c | 1 +
>   1 file changed, 1 insertion(+)

Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>




^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v4 05/12] contrib/vhost-user-blk: fix bind() using the right size of the address
  2024-05-08  7:44 ` [PATCH v4 05/12] contrib/vhost-user-blk: fix bind() using the right size of the address Stefano Garzarella
@ 2024-05-08 10:25   ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 37+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-05-08 10:25 UTC (permalink / raw)
  To: Stefano Garzarella, qemu-devel
  Cc: Jason Wang, Coiby Xu, Paolo Bonzini, qemu-block,
	Daniel P. Berrangé, Gerd Hoffmann, slp, Brad Smith,
	Eduardo Habkost, Thomas Huth, Eric Blake, Kevin Wolf,
	Markus Armbruster, Raphael Norwitz, gmaglione, Laurent Vivier,
	stefanha, David Hildenbrand, Hanna Reitz, Michael S. Tsirkin,
	Igor Mammedov, Marc-André Lureau

On 8/5/24 09:44, Stefano Garzarella wrote:
> On macOS passing `-s /tmp/vhost.socket` parameter to the vhost-user-blk
> application, the bind was done on `/tmp/vhost.socke` pathname,
> missing the last character.
> 
> This sounds like one of the portability problems described in the
> unix(7) manpage:
> 
>     Pathname sockets
>         When  binding  a socket to a pathname, a few rules should
>         be observed for maximum portability and ease of coding:
> 
>         •  The pathname in sun_path should be null-terminated.
> 
>         •  The length of the pathname, including the  terminating
>            null byte, should not exceed the size of sun_path.
> 
>         •  The  addrlen  argument  that  describes  the enclosing
>            sockaddr_un structure should have a value of at least:
> 
>                offsetof(struct sockaddr_un, sun_path) +
>                strlen(addr.sun_path)+1
> 
>            or,  more  simply,  addrlen  can   be   specified   as
>            sizeof(struct sockaddr_un).
> 
> So let's follow the last advice and simplify the code as well.
> 
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
>   contrib/vhost-user-blk/vhost-user-blk.c | 4 +---
>   1 file changed, 1 insertion(+), 3 deletions(-)

Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>




^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v4 06/12] contrib/vhost-user-*: use QEMU bswap helper functions
  2024-05-08 10:13   ` Philippe Mathieu-Daudé
@ 2024-05-08 10:25     ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 37+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-05-08 10:25 UTC (permalink / raw)
  To: Stefano Garzarella, qemu-devel
  Cc: Jason Wang, Coiby Xu, Paolo Bonzini, qemu-block,
	Daniel P. Berrangé, Gerd Hoffmann, slp, Brad Smith,
	Eduardo Habkost, Thomas Huth, Eric Blake, Kevin Wolf,
	Markus Armbruster, Raphael Norwitz, gmaglione, Laurent Vivier,
	stefanha, David Hildenbrand, Hanna Reitz, Michael S. Tsirkin,
	Igor Mammedov, Marc-André Lureau

On 8/5/24 12:13, Philippe Mathieu-Daudé wrote:
> On 8/5/24 09:44, Stefano Garzarella wrote:
>> Let's replace the calls to le*toh() and htole*() with qemu/bswap.h
>> helpers to make the code more portable.
>>
>> Suggested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
>> ---
>>   contrib/vhost-user-blk/vhost-user-blk.c |  9 +++++----
>>   contrib/vhost-user-input/main.c         | 16 ++++++++--------
>>   2 files changed, 13 insertions(+), 12 deletions(-)
> 
> Thanks,
> 
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>




^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v4 07/12] vhost-user: enable frontends on any POSIX system
  2024-05-08  7:44 ` [PATCH v4 07/12] vhost-user: enable frontends on any POSIX system Stefano Garzarella
@ 2024-05-08 10:26   ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 37+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-05-08 10:26 UTC (permalink / raw)
  To: Stefano Garzarella, qemu-devel
  Cc: Jason Wang, Coiby Xu, Paolo Bonzini, qemu-block,
	Daniel P. Berrangé, Gerd Hoffmann, slp, Brad Smith,
	Eduardo Habkost, Thomas Huth, Eric Blake, Kevin Wolf,
	Markus Armbruster, Raphael Norwitz, gmaglione, Laurent Vivier,
	stefanha, David Hildenbrand, Hanna Reitz, Michael S. Tsirkin,
	Igor Mammedov, Marc-André Lureau

On 8/5/24 09:44, Stefano Garzarella wrote:
> The vhost-user protocol is not really Linux-specific so let's enable
> vhost-user frontends for any POSIX system.
> 
> In vhost_net.c we use VHOST_FILE_UNBIND which is defined in a Linux
> specific header, let's define it for other systems as well.
> 
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
>   meson.build        | 1 -
>   hw/net/vhost_net.c | 5 +++++
>   hw/block/Kconfig   | 2 +-
>   3 files changed, 6 insertions(+), 2 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

on macOS:
Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>



^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v4 09/12] contrib/vhost-user-blk: enable it on any POSIX system
  2024-05-08  7:44 ` [PATCH v4 09/12] contrib/vhost-user-blk: " Stefano Garzarella
@ 2024-05-08 10:32   ` Philippe Mathieu-Daudé
  2024-05-10  9:02     ` Stefano Garzarella
  0 siblings, 1 reply; 37+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-05-08 10:32 UTC (permalink / raw)
  To: Stefano Garzarella, qemu-devel
  Cc: Jason Wang, Coiby Xu, Paolo Bonzini, qemu-block,
	Daniel P. Berrangé, Gerd Hoffmann, slp, Brad Smith,
	Eduardo Habkost, Thomas Huth, Eric Blake, Kevin Wolf,
	Markus Armbruster, Raphael Norwitz, gmaglione, Laurent Vivier,
	stefanha, David Hildenbrand, Hanna Reitz, Michael S. Tsirkin,
	Igor Mammedov, Marc-André Lureau

On 8/5/24 09:44, Stefano Garzarella wrote:
> Let's make the code more portable by adding defines from
> block/file-posix.c to support O_DIRECT in other systems (e.g. macOS).
> 
> vhost-user-server.c is a dependency, let's enable it for any POSIX
> system.
> 
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
> v4:
> - moved using of "qemu/bswap.h" API in a separate patch [Phil]
> ---
>   meson.build                             |  2 --
>   contrib/vhost-user-blk/vhost-user-blk.c | 14 ++++++++++++++
>   util/meson.build                        |  4 +++-
>   3 files changed, 17 insertions(+), 3 deletions(-)


> diff --git a/contrib/vhost-user-blk/vhost-user-blk.c b/contrib/vhost-user-blk/vhost-user-blk.c
> index 9492146855..a450337685 100644
> --- a/contrib/vhost-user-blk/vhost-user-blk.c
> +++ b/contrib/vhost-user-blk/vhost-user-blk.c
> @@ -25,6 +25,20 @@
>   #include <sys/ioctl.h>
>   #endif
>   
> +/* OS X does not have O_DSYNC */
> +#ifndef O_DSYNC
> +#ifdef O_SYNC
> +#define O_DSYNC O_SYNC
> +#elif defined(O_FSYNC)
> +#define O_DSYNC O_FSYNC
> +#endif
> +#endif
> +
> +/* Approximate O_DIRECT with O_DSYNC if O_DIRECT isn't available */
> +#ifndef O_DIRECT
> +#define O_DIRECT O_DSYNC
> +#endif

Could we add that in "qemu/osdep.h" instead?

Otherwise,
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>



^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v4 08/12] libvhost-user: enable it on any POSIX system
  2024-05-08  7:44 ` [PATCH v4 08/12] libvhost-user: enable it " Stefano Garzarella
@ 2024-05-08 10:36   ` Philippe Mathieu-Daudé
  2024-05-10  8:56     ` Stefano Garzarella
  0 siblings, 1 reply; 37+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-05-08 10:36 UTC (permalink / raw)
  To: Stefano Garzarella, qemu-devel
  Cc: Jason Wang, Coiby Xu, Paolo Bonzini, qemu-block,
	Daniel P. Berrangé, Gerd Hoffmann, slp, Brad Smith,
	Eduardo Habkost, Thomas Huth, Eric Blake, Kevin Wolf,
	Markus Armbruster, Raphael Norwitz, gmaglione, Laurent Vivier,
	stefanha, David Hildenbrand, Hanna Reitz, Michael S. Tsirkin,
	Igor Mammedov, Marc-André Lureau

On 8/5/24 09:44, Stefano Garzarella wrote:
> The vhost-user protocol is not really Linux-specific so let's enable
> libvhost-user for any POSIX system.
> 
> Compiling it on macOS and FreeBSD some problems came up:
> - avoid to include linux/vhost.h which is avaibale only on Linux

"available"

>    (vhost_types.h contains many of the things we need)
> - macOS doesn't provide sys/endian.h, so let's define them
>    (note: libvhost-user doesn't include qemu's headers, so we can't use

"QEMU"

>     use "qemu/bswap.h")
> - define eventfd_[write|read] as write/read wrapper when system doesn't
>    provide those (e.g. macOS)
> - copy SEAL defines from include/qemu/memfd.h to make the code works
>    on FreeBSD where MFD_ALLOW_SEALING is defined

Alternatively add in subprojects/libvhost-user/include/osdep.h.

> - define MAP_NORESERVE if it's not defined (e.g. on FreeBSD)
> 
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
>   meson.build                               |  2 +-
>   subprojects/libvhost-user/libvhost-user.h |  2 +-
>   subprojects/libvhost-user/libvhost-user.c | 60 +++++++++++++++++++++--
>   3 files changed, 59 insertions(+), 5 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>




^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v4 03/12] libvhost-user: mask F_INFLIGHT_SHMFD if memfd is not supported
  2024-05-08  7:44 ` [PATCH v4 03/12] libvhost-user: mask F_INFLIGHT_SHMFD if memfd is not supported Stefano Garzarella
@ 2024-05-08 10:39   ` Philippe Mathieu-Daudé
  2024-05-10  8:25     ` Stefano Garzarella
  0 siblings, 1 reply; 37+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-05-08 10:39 UTC (permalink / raw)
  To: Stefano Garzarella, qemu-devel
  Cc: Jason Wang, Coiby Xu, Paolo Bonzini, qemu-block,
	Daniel P. Berrangé, Gerd Hoffmann, slp, Brad Smith,
	Eduardo Habkost, Thomas Huth, Eric Blake, Kevin Wolf,
	Markus Armbruster, Raphael Norwitz, gmaglione, Laurent Vivier,
	stefanha, David Hildenbrand, Hanna Reitz, Michael S. Tsirkin,
	Igor Mammedov, Marc-André Lureau

On 8/5/24 09:44, Stefano Garzarella wrote:
> libvhost-user will panic when receiving VHOST_USER_GET_INFLIGHT_FD
> message if MFD_ALLOW_SEALING is not defined, since it's not able
> to create a memfd.
> 
> VHOST_USER_GET_INFLIGHT_FD is used only if
> VHOST_USER_PROTOCOL_F_INFLIGHT_SHMFD is negotiated. So, let's mask
> that feature if the backend is not able to properly handle these
> messages.
> 
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
>   subprojects/libvhost-user/libvhost-user.c | 10 ++++++++++
>   1 file changed, 10 insertions(+)
> 
> diff --git a/subprojects/libvhost-user/libvhost-user.c b/subprojects/libvhost-user/libvhost-user.c
> index a11afd1960..1c361ffd51 100644
> --- a/subprojects/libvhost-user/libvhost-user.c
> +++ b/subprojects/libvhost-user/libvhost-user.c
> @@ -1674,6 +1674,16 @@ vu_get_protocol_features_exec(VuDev *dev, VhostUserMsg *vmsg)
>           features |= dev->iface->get_protocol_features(dev);
>       }
>   

Maybe move the #ifndef here?

> +    /*
> +     * If MFD_ALLOW_SEALING is not defined, we are not able to handle
> +     * VHOST_USER_GET_INFLIGHT_FD messages, since we can't create a memfd.
> +     * Those messages are used only if VHOST_USER_PROTOCOL_F_INFLIGHT_SHMFD
> +     * is negotiated. A device implementation can enable it, so let's mask
> +     * it to avoid a runtime panic.
> +     */
> +#ifndef MFD_ALLOW_SEALING
> +    features &= ~(1ULL << VHOST_USER_PROTOCOL_F_INFLIGHT_SHMFD);
> +#endif
>       vmsg_set_reply_u64(vmsg, features);
>       return true;
>   }

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>



^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v4 00/12] vhost-user: support any POSIX system (tested on macOS, FreeBSD, OpenBSD)
  2024-05-08  7:44 [PATCH v4 00/12] vhost-user: support any POSIX system (tested on macOS, FreeBSD, OpenBSD) Stefano Garzarella
                   ` (11 preceding siblings ...)
  2024-05-08  7:44 ` [PATCH v4 12/12] tests/qtest/vhost-user-test: add a test case for memory-backend-shm Stefano Garzarella
@ 2024-05-08 10:39 ` Philippe Mathieu-Daudé
  2024-05-09 19:20 ` Stefan Hajnoczi
  13 siblings, 0 replies; 37+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-05-08 10:39 UTC (permalink / raw)
  To: Stefano Garzarella, qemu-devel
  Cc: Jason Wang, Coiby Xu, Paolo Bonzini, qemu-block,
	Daniel P. Berrangé, Gerd Hoffmann, slp, Brad Smith,
	Eduardo Habkost, Thomas Huth, Eric Blake, Kevin Wolf,
	Markus Armbruster, Raphael Norwitz, gmaglione, Laurent Vivier,
	stefanha, David Hildenbrand, Hanna Reitz, Michael S. Tsirkin,
	Igor Mammedov, Marc-André Lureau

On 8/5/24 09:44, Stefano Garzarella wrote:

> The vhost-user protocol is not really Linux-specific, so let's try support
> QEMU's frontends and backends (including libvhost-user) in any POSIX system
> with this series. The main use case is to be able to use virtio devices that
> we don't have built-in in QEMU (e.g. virtiofsd, vhost-user-vsock, etc.) even
> in non-Linux systems.
> 
> The first 5 patches are more like fixes discovered at runtime on macOS or
> FreeBSD that could go even independently of this series.
> 
> Patches 6, 7, 8, and 9 enable building of frontends and backends (including
> libvhost-user) with associated code changes to succeed in compilation.

Indeed, diffing ./configure on macOS:

+Executing subproject libvhost-user
+
+libvhost-user| Project name: libvhost-user
+libvhost-user| Project version: undefined
+libvhost-user| C compiler for the host machine: cc (clang 15.0.0 "Apple 
clang version 15.0.0 (clang-1500.3.9.4)")
+libvhost-user| C linker for the host machine: cc ld64 1053.12
+libvhost-user| Compiler for C supports arguments -Wsign-compare: YES
+libvhost-user| Compiler for C supports arguments 
-Wdeclaration-after-statement: YES
+libvhost-user| Compiler for C supports arguments -Wstrict-aliasing: YES
+libvhost-user| Dependency threads found: YES unknown (cached)
+libvhost-user| Dependency glib-2.0 found: YES 2.80.0 (overridden)
+libvhost-user| Build targets in project: 6
+libvhost-user| Subproject libvhost-user finished.

-Build targets in project: 707
+Build targets in project: 713

      QOM debugging                                : YES
      Relocatable install                          : YES
      vhost-kernel support                         : NO
-    vhost-net support                            : NO
-    vhost-user support                           : NO
-    vhost-user-crypto support                    : NO
-    vhost-user-blk server support                : NO
+    vhost-net support                            : YES
+    vhost-user support                           : YES
+    vhost-user-crypto support                    : YES
+    vhost-user-blk server support                : YES
      vhost-vdpa support                           : NO
      build guest agent                            : NO

      berkeley-testfloat-3                         : YES
      dtc                                          : YES
      keycodemapdb                                 : YES
+    libvhost-user                                : YES

---


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v4 10/12] hostmem: add a new memory backend based on POSIX shm_open()
  2024-05-08  7:44 ` [PATCH v4 10/12] hostmem: add a new memory backend based on POSIX shm_open() Stefano Garzarella
@ 2024-05-08 11:59   ` Markus Armbruster
  2024-05-10  9:37     ` Stefano Garzarella
  0 siblings, 1 reply; 37+ messages in thread
From: Markus Armbruster @ 2024-05-08 11:59 UTC (permalink / raw)
  To: Stefano Garzarella
  Cc: qemu-devel, Jason Wang, Coiby Xu, Paolo Bonzini, qemu-block,
	Daniel P. Berrangé, Gerd Hoffmann, slp,
	Philippe Mathieu-Daudé, Brad Smith, Eduardo Habkost,
	Thomas Huth, Eric Blake, Kevin Wolf, Raphael Norwitz, gmaglione,
	Laurent Vivier, stefanha, David Hildenbrand, Hanna Reitz,
	Michael S. Tsirkin, Igor Mammedov, Marc-André Lureau

Stefano Garzarella <sgarzare@redhat.com> writes:

> shm_open() creates and opens a new POSIX shared memory object.
> A POSIX shared memory object allows creating memory backend with an
> associated file descriptor that can be shared with external processes
> (e.g. vhost-user).
>
> The new `memory-backend-shm` can be used as an alternative when
> `memory-backend-memfd` is not available (Linux only), since shm_open()
> should be provided by any POSIX-compliant operating system.
>
> This backend mimics memfd, allocating memory that is practically
> anonymous. In theory shm_open() requires a name, but this is allocated
> for a short time interval and shm_unlink() is called right after
> shm_open(). After that, only fd is shared with external processes
> (e.g., vhost-user) as if it were associated with anonymous memory.
>
> In the future we may also allow the user to specify the name to be
> passed to shm_open(), but for now we keep the backend simple, mimicking
> anonymous memory such as memfd.
>
> Acked-by: David Hildenbrand <david@redhat.com>
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
> v4
> - fail if we find "share=off" in shm_backend_memory_alloc() [David]
> v3
> - enriched commit message and documentation to highlight that we
>   want to mimic memfd (David)
> ---
>  docs/system/devices/vhost-user.rst |   5 +-
>  qapi/qom.json                      |  17 ++++
>  backends/hostmem-shm.c             | 123 +++++++++++++++++++++++++++++
>  backends/meson.build               |   1 +
>  qemu-options.hx                    |  13 +++
>  5 files changed, 157 insertions(+), 2 deletions(-)
>  create mode 100644 backends/hostmem-shm.c
>
> diff --git a/docs/system/devices/vhost-user.rst b/docs/system/devices/vhost-user.rst
> index 9b2da106ce..35259d8ec7 100644
> --- a/docs/system/devices/vhost-user.rst
> +++ b/docs/system/devices/vhost-user.rst
> @@ -98,8 +98,9 @@ Shared memory object
>  
>  In order for the daemon to access the VirtIO queues to process the
>  requests it needs access to the guest's address space. This is
> -achieved via the ``memory-backend-file`` or ``memory-backend-memfd``
> -objects. A reference to a file-descriptor which can access this object
> +achieved via the ``memory-backend-file``, ``memory-backend-memfd``, or
> +``memory-backend-shm`` objects.
> +A reference to a file-descriptor which can access this object
>  will be passed via the socket as part of the protocol negotiation.
>  
>  Currently the shared memory object needs to match the size of the main
> diff --git a/qapi/qom.json b/qapi/qom.json
> index 38dde6d785..52df052df8 100644
> --- a/qapi/qom.json
> +++ b/qapi/qom.json
> @@ -721,6 +721,19 @@
>              '*hugetlbsize': 'size',
>              '*seal': 'bool' } }
>  
> +##
> +# @MemoryBackendShmProperties:
> +#
> +# Properties for memory-backend-shm objects.
> +#
> +# The @share boolean option is true by default with shm.

This contradicts the doc comment for @share:

   # @share: if false, the memory is private to QEMU; if true, it is
   #     shared (default: false)

Your intention is to override that text.  But that's less than clear.
Moreover, the documentation of @share is pretty far from this override.
John Snow is working on patches that'll pull it closer.

Hmm, MemoryBackendMemfdProperties has the same override.

I think we should change the doc comment for @share to something like

   # @share: if false, the memory is private to QEMU; if true, it is
   #     shared (default depends on the backend type)

and then document the actual default with each backend type.

> +#
> +# Since: 9.1
> +##
> +{ 'struct': 'MemoryBackendShmProperties',
> +  'base': 'MemoryBackendProperties',
> +  'data': { } }

Let's add 'if': 'CONFIG_POSIX' here.

> +
>  ##
>  # @MemoryBackendEpcProperties:
>  #
> @@ -985,6 +998,8 @@
>      { 'name': 'memory-backend-memfd',
>        'if': 'CONFIG_LINUX' },
>      'memory-backend-ram',
> +    { 'name': 'memory-backend-shm',
> +      'if': 'CONFIG_POSIX' },
>      'pef-guest',
>      { 'name': 'pr-manager-helper',
>        'if': 'CONFIG_LINUX' },
> @@ -1056,6 +1071,8 @@
>        'memory-backend-memfd':       { 'type': 'MemoryBackendMemfdProperties',
>                                        'if': 'CONFIG_LINUX' },
>        'memory-backend-ram':         'MemoryBackendProperties',
> +      'memory-backend-shm':         { 'type': 'MemoryBackendShmProperties',
> +                                      'if': 'CONFIG_POSIX' },
>        'pr-manager-helper':          { 'type': 'PrManagerHelperProperties',
>                                        'if': 'CONFIG_LINUX' },
>        'qtest':                      'QtestProperties',

[...]

> diff --git a/qemu-options.hx b/qemu-options.hx
> index cf61f6b863..2226172fb0 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -5227,6 +5227,19 @@ SRST
>  
>          The ``share`` boolean option is on by default with memfd.
>  
> +    ``-object memory-backend-shm,id=id,merge=on|off,dump=on|off,share=on|off,prealloc=on|off,size=size,host-nodes=host-nodes,policy=default|preferred|bind|interleave``
> +        Creates a POSIX shared memory backend object, which allows
> +        QEMU to share the memory with an external process (e.g. when
> +        using vhost-user). This backend mimics memfd, allocating memory that is
> +        practically anonymous. This is useful when memfd is not available.

This actually explains the purpose, unlike the doc comment in qom.json.
Same for the existing memory backends; can't fault you for doing your
new one the same way.  We ought to fix them all.  I'm not demanding you
do it.

The text could perhaps a bit clearer.  What does "practically anonymous"
mean?  As far as I understand, memory-backend-shm is a more portable and
less featureful version of memory-backend-memfd.  Say that?

> +
> +        Please refer to ``memory-backend-file`` for a description of the
> +        options.
> +
> +        The ``share`` boolean option is on by default with shm. Setting it to
> +        off will cause a failure during allocation because it is not supported
> +        by this backend.
> +

The "will case a failure" part is documented only here, and not in
qom.json.

Not this patch's fault: documentation for -object memory-backend-epc is
missing.

>      ``-object iommufd,id=id[,fd=fd]``
>          Creates an iommufd backend which allows control of DMA mapping
>          through the ``/dev/iommu`` device.



^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v4 00/12] vhost-user: support any POSIX system (tested on macOS, FreeBSD, OpenBSD)
  2024-05-08  7:44 [PATCH v4 00/12] vhost-user: support any POSIX system (tested on macOS, FreeBSD, OpenBSD) Stefano Garzarella
                   ` (12 preceding siblings ...)
  2024-05-08 10:39 ` [PATCH v4 00/12] vhost-user: support any POSIX system (tested on macOS, FreeBSD, OpenBSD) Philippe Mathieu-Daudé
@ 2024-05-09 19:20 ` Stefan Hajnoczi
  13 siblings, 0 replies; 37+ messages in thread
From: Stefan Hajnoczi @ 2024-05-09 19:20 UTC (permalink / raw)
  To: Stefano Garzarella
  Cc: qemu-devel, Jason Wang, Coiby Xu, Paolo Bonzini, qemu-block,
	Daniel P. Berrangé, Gerd Hoffmann, slp,
	Philippe Mathieu-Daudé, Brad Smith, Eduardo Habkost,
	Thomas Huth, Eric Blake, Kevin Wolf, Markus Armbruster,
	Raphael Norwitz, gmaglione, Laurent Vivier, David Hildenbrand,
	Hanna Reitz, Michael S. Tsirkin, Igor Mammedov,
	Marc-André Lureau

[-- Attachment #1: Type: text/plain, Size: 5667 bytes --]

On Wed, May 08, 2024 at 09:44:44AM +0200, Stefano Garzarella wrote:
> v1: https://patchew.org/QEMU/20240228114759.44758-1-sgarzare@redhat.com/
> v2: https://patchew.org/QEMU/20240326133936.125332-1-sgarzare@redhat.com/
> v3: https://patchew.org/QEMU/20240404122330.92710-1-sgarzare@redhat.com/
> v4:
>   - rebased on master (commit e116b92d01c2cd75957a9f8ad1d4932292867b81)
>   - added patch 6 to move using QEMU bswap helper functions in a separate
>     patch (Phil)
>   - fail if we find "share=off" in shm_backend_memory_alloc() (David)
>   - added Phil's R-b and David's A-b
> 
> The vhost-user protocol is not really Linux-specific, so let's try support
> QEMU's frontends and backends (including libvhost-user) in any POSIX system
> with this series. The main use case is to be able to use virtio devices that
> we don't have built-in in QEMU (e.g. virtiofsd, vhost-user-vsock, etc.) even
> in non-Linux systems.
> 
> The first 5 patches are more like fixes discovered at runtime on macOS or
> FreeBSD that could go even independently of this series.
> 
> Patches 6, 7, 8, and 9 enable building of frontends and backends (including
> libvhost-user) with associated code changes to succeed in compilation.
> 
> Patch 10 adds `memory-backend-shm` that uses the POSIX shm_open() API to
> create shared memory which is identified by an fd that can be shared with
> vhost-user backends. This is useful on those systems (like macOS) where
> we don't have memfd_create() or special filesystems like "/dev/shm".
> 
> Patches 11 and 12 use `memory-backend-shm` in some vhost-user tests.
> 
> Maybe the first 5 patches can go separately, but I only discovered those
> problems after testing patches 6 - 9, so I have included them in this series
> for now. Please let me know if you prefer that I send them separately.
> 
> I tested this series using vhost-user-blk and QSD on macOS Sonoma 14.4
> (aarch64), FreeBSD 14 (x86_64), OpenBSD 7.4 (x86_64), and Fedora 39 (x86_64)
> in this way:
> 
> - Start vhost-user-blk or QSD (same commands for all systems)
> 
>   vhost-user-blk -s /tmp/vhost.socket \
>     -b Fedora-Cloud-Base-39-1.5.x86_64.raw
> 
>   qemu-storage-daemon \
>     --blockdev file,filename=Fedora-Cloud-Base-39-1.5.x86_64.qcow2,node-name=file \
>     --blockdev qcow2,file=file,node-name=qcow2 \
>     --export vhost-user-blk,addr.type=unix,addr.path=/tmp/vhost.socket,id=vub,num-queues=1,node-name=qcow2,writable=on
> 
> - macOS (aarch64): start QEMU (using hvf accelerator)
> 
>   qemu-system-aarch64 -smp 2 -cpu host -M virt,accel=hvf,memory-backend=mem \
>     -drive file=./build/pc-bios/edk2-aarch64-code.fd,if=pflash,format=raw,readonly=on \
>     -device virtio-net-device,netdev=net0 -netdev user,id=net0 \
>     -device ramfb -device usb-ehci -device usb-kbd \
>     -object memory-backend-shm,id=mem,size=512M \
>     -device vhost-user-blk-pci,num-queues=1,disable-legacy=on,chardev=char0 \
>     -chardev socket,id=char0,path=/tmp/vhost.socket
> 
> - FreeBSD/OpenBSD (x86_64): start QEMU (no accelerators available)
> 
>   qemu-system-x86_64 -smp 2 -M q35,memory-backend=mem \
>     -object memory-backend-shm,id=mem,size="512M" \
>     -device vhost-user-blk-pci,num-queues=1,chardev=char0 \
>     -chardev socket,id=char0,path=/tmp/vhost.socket
> 
> - Fedora (x86_64): start QEMU (using kvm accelerator)
> 
>   qemu-system-x86_64 -smp 2 -M q35,accel=kvm,memory-backend=mem \
>     -object memory-backend-shm,size="512M" \
>     -device vhost-user-blk-pci,num-queues=1,chardev=char0 \
>     -chardev socket,id=char0,path=/tmp/vhost.socket
> 
> Branch pushed (and CI started) at https://gitlab.com/sgarzarella/qemu/-/tree/macos-vhost-user?ref_type=heads
> 
> Thanks,
> Stefano
> 
> Stefano Garzarella (12):
>   libvhost-user: set msg.msg_control to NULL when it is empty
>   libvhost-user: fail vu_message_write() if sendmsg() is failing
>   libvhost-user: mask F_INFLIGHT_SHMFD if memfd is not supported
>   vhost-user-server: do not set memory fd non-blocking
>   contrib/vhost-user-blk: fix bind() using the right size of the address
>   contrib/vhost-user-*: use QEMU bswap helper functions
>   vhost-user: enable frontends on any POSIX system
>   libvhost-user: enable it on any POSIX system
>   contrib/vhost-user-blk: enable it on any POSIX system
>   hostmem: add a new memory backend based on POSIX shm_open()
>   tests/qtest/vhost-user-blk-test: use memory-backend-shm
>   tests/qtest/vhost-user-test: add a test case for memory-backend-shm
> 
>  docs/system/devices/vhost-user.rst        |   5 +-
>  meson.build                               |   5 +-
>  qapi/qom.json                             |  17 +++
>  subprojects/libvhost-user/libvhost-user.h |   2 +-
>  backends/hostmem-shm.c                    | 123 ++++++++++++++++++++++
>  contrib/vhost-user-blk/vhost-user-blk.c   |  27 +++--
>  contrib/vhost-user-input/main.c           |  16 +--
>  hw/net/vhost_net.c                        |   5 +
>  subprojects/libvhost-user/libvhost-user.c |  76 ++++++++++++-
>  tests/qtest/vhost-user-blk-test.c         |   2 +-
>  tests/qtest/vhost-user-test.c             |  23 ++++
>  util/vhost-user-server.c                  |  12 +++
>  backends/meson.build                      |   1 +
>  hw/block/Kconfig                          |   2 +-
>  qemu-options.hx                           |  13 +++
>  util/meson.build                          |   4 +-
>  16 files changed, 305 insertions(+), 28 deletions(-)
>  create mode 100644 backends/hostmem-shm.c
> 
> -- 
> 2.45.0
> 

Acked-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v4 11/12] tests/qtest/vhost-user-blk-test: use memory-backend-shm
  2024-05-08  7:44 ` [PATCH v4 11/12] tests/qtest/vhost-user-blk-test: use memory-backend-shm Stefano Garzarella
@ 2024-05-10  5:57   ` Thomas Huth
  2024-05-10 10:54   ` Philippe Mathieu-Daudé
  1 sibling, 0 replies; 37+ messages in thread
From: Thomas Huth @ 2024-05-10  5:57 UTC (permalink / raw)
  To: Stefano Garzarella, qemu-devel
  Cc: Jason Wang, Coiby Xu, Paolo Bonzini, qemu-block,
	Daniel P. Berrangé, Gerd Hoffmann, slp,
	Philippe Mathieu-Daudé, Brad Smith, Eduardo Habkost,
	Eric Blake, Kevin Wolf, Markus Armbruster, Raphael Norwitz,
	gmaglione, Laurent Vivier, stefanha, David Hildenbrand,
	Hanna Reitz, Michael S. Tsirkin, Igor Mammedov,
	Marc-André Lureau

On 08/05/2024 09.44, Stefano Garzarella wrote:
> `memory-backend-memfd` is available only on Linux while the new
> `memory-backend-shm` can be used on any POSIX-compliant operating
> system. Let's use it so we can run the test in multiple environments.
> 
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
>   tests/qtest/vhost-user-blk-test.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/qtest/vhost-user-blk-test.c b/tests/qtest/vhost-user-blk-test.c
> index 117b9acd10..e945f6abf2 100644
> --- a/tests/qtest/vhost-user-blk-test.c
> +++ b/tests/qtest/vhost-user-blk-test.c
> @@ -906,7 +906,7 @@ static void start_vhost_user_blk(GString *cmd_line, int vus_instances,
>                              vhost_user_blk_bin);
>   
>       g_string_append_printf(cmd_line,
> -            " -object memory-backend-memfd,id=mem,size=256M,share=on "
> +            " -object memory-backend-shm,id=mem,size=256M,share=on "
>               " -M memory-backend=mem -m 256M ");
>   
>       for (i = 0; i < vus_instances; i++) {

Acked-by: Thomas Huth <thuth@redhat.com>



^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v4 12/12] tests/qtest/vhost-user-test: add a test case for memory-backend-shm
  2024-05-08  7:44 ` [PATCH v4 12/12] tests/qtest/vhost-user-test: add a test case for memory-backend-shm Stefano Garzarella
@ 2024-05-10  5:58   ` Thomas Huth
  0 siblings, 0 replies; 37+ messages in thread
From: Thomas Huth @ 2024-05-10  5:58 UTC (permalink / raw)
  To: Stefano Garzarella, qemu-devel
  Cc: Jason Wang, Coiby Xu, Paolo Bonzini, qemu-block,
	Daniel P. Berrangé, Gerd Hoffmann, slp,
	Philippe Mathieu-Daudé, Brad Smith, Eduardo Habkost,
	Eric Blake, Kevin Wolf, Markus Armbruster, Raphael Norwitz,
	gmaglione, Laurent Vivier, stefanha, David Hildenbrand,
	Hanna Reitz, Michael S. Tsirkin, Igor Mammedov,
	Marc-André Lureau

On 08/05/2024 09.44, Stefano Garzarella wrote:
> `memory-backend-shm` can be used with vhost-user devices, so let's
> add a new test case for it.
> 
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
>   tests/qtest/vhost-user-test.c | 23 +++++++++++++++++++++++
>   1 file changed, 23 insertions(+)
> 
> diff --git a/tests/qtest/vhost-user-test.c b/tests/qtest/vhost-user-test.c
> index d4e437265f..8c1d903b2a 100644
> --- a/tests/qtest/vhost-user-test.c
> +++ b/tests/qtest/vhost-user-test.c
> @@ -44,6 +44,8 @@
>                           "mem-path=%s,share=on -numa node,memdev=mem"
>   #define QEMU_CMD_MEMFD  " -m %d -object memory-backend-memfd,id=mem,size=%dM," \
>                           " -numa node,memdev=mem"
> +#define QEMU_CMD_SHM    " -m %d -object memory-backend-shm,id=mem,size=%dM," \
> +                        " -numa node,memdev=mem"
>   #define QEMU_CMD_CHR    " -chardev socket,id=%s,path=%s%s"
>   #define QEMU_CMD_NETDEV " -netdev vhost-user,id=hs0,chardev=%s,vhostforce=on"
>   
> @@ -195,6 +197,7 @@ enum test_memfd {
>       TEST_MEMFD_AUTO,
>       TEST_MEMFD_YES,
>       TEST_MEMFD_NO,
> +    TEST_MEMFD_SHM,
>   };
>   
>   static void append_vhost_net_opts(TestServer *s, GString *cmd_line,
> @@ -228,6 +231,8 @@ static void append_mem_opts(TestServer *server, GString *cmd_line,
>   
>       if (memfd == TEST_MEMFD_YES) {
>           g_string_append_printf(cmd_line, QEMU_CMD_MEMFD, size, size);
> +    } else if (memfd == TEST_MEMFD_SHM) {
> +        g_string_append_printf(cmd_line, QEMU_CMD_SHM, size, size);
>       } else {
>           const char *root = init_hugepagefs() ? : server->tmpfs;
>   
> @@ -788,6 +793,19 @@ static void *vhost_user_test_setup_memfd(GString *cmd_line, void *arg)
>       return server;
>   }
>   
> +static void *vhost_user_test_setup_shm(GString *cmd_line, void *arg)
> +{
> +    TestServer *server = test_server_new("vhost-user-test", arg);
> +    test_server_listen(server);
> +
> +    append_mem_opts(server, cmd_line, 256, TEST_MEMFD_SHM);
> +    server->vu_ops->append_opts(server, cmd_line, "");
> +
> +    g_test_queue_destroy(vhost_user_test_cleanup, server);
> +
> +    return server;
> +}
> +
>   static void test_read_guest_mem(void *obj, void *arg, QGuestAllocator *alloc)
>   {
>       TestServer *server = arg;
> @@ -1081,6 +1099,11 @@ static void register_vhost_user_test(void)
>                    "virtio-net",
>                    test_read_guest_mem, &opts);
>   
> +    opts.before = vhost_user_test_setup_shm;
> +    qos_add_test("vhost-user/read-guest-mem/shm",
> +                 "virtio-net",
> +                 test_read_guest_mem, &opts);
> +
>       if (qemu_memfd_check(MFD_ALLOW_SEALING)) {
>           opts.before = vhost_user_test_setup_memfd;
>           qos_add_test("vhost-user/read-guest-mem/memfd",

Acked-by: Thomas Huth <thuth@redhat.com>



^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v4 03/12] libvhost-user: mask F_INFLIGHT_SHMFD if memfd is not supported
  2024-05-08 10:39   ` Philippe Mathieu-Daudé
@ 2024-05-10  8:25     ` Stefano Garzarella
  0 siblings, 0 replies; 37+ messages in thread
From: Stefano Garzarella @ 2024-05-10  8:25 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: qemu-devel, Jason Wang, Coiby Xu, Paolo Bonzini, qemu-block,
	Daniel P. Berrangé, Gerd Hoffmann, slp, Brad Smith,
	Eduardo Habkost, Thomas Huth, Eric Blake, Kevin Wolf,
	Markus Armbruster, Raphael Norwitz, gmaglione, Laurent Vivier,
	stefanha, David Hildenbrand, Hanna Reitz, Michael S. Tsirkin,
	Igor Mammedov, Marc-André Lureau

On Wed, May 08, 2024 at 12:39:33PM GMT, Philippe Mathieu-Daudé wrote:
>On 8/5/24 09:44, Stefano Garzarella wrote:
>>libvhost-user will panic when receiving VHOST_USER_GET_INFLIGHT_FD
>>message if MFD_ALLOW_SEALING is not defined, since it's not able
>>to create a memfd.
>>
>>VHOST_USER_GET_INFLIGHT_FD is used only if
>>VHOST_USER_PROTOCOL_F_INFLIGHT_SHMFD is negotiated. So, let's mask
>>that feature if the backend is not able to properly handle these
>>messages.
>>
>>Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
>>---
>>  subprojects/libvhost-user/libvhost-user.c | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
>>
>>diff --git a/subprojects/libvhost-user/libvhost-user.c b/subprojects/libvhost-user/libvhost-user.c
>>index a11afd1960..1c361ffd51 100644
>>--- a/subprojects/libvhost-user/libvhost-user.c
>>+++ b/subprojects/libvhost-user/libvhost-user.c
>>@@ -1674,6 +1674,16 @@ vu_get_protocol_features_exec(VuDev *dev, VhostUserMsg *vmsg)
>>          features |= dev->iface->get_protocol_features(dev);
>>      }
>
>Maybe move the #ifndef here?

Yep, I'll do.

>
>>+    /*
>>+     * If MFD_ALLOW_SEALING is not defined, we are not able to handle
>>+     * VHOST_USER_GET_INFLIGHT_FD messages, since we can't create a memfd.
>>+     * Those messages are used only if VHOST_USER_PROTOCOL_F_INFLIGHT_SHMFD
>>+     * is negotiated. A device implementation can enable it, so let's mask
>>+     * it to avoid a runtime panic.
>>+     */
>>+#ifndef MFD_ALLOW_SEALING
>>+    features &= ~(1ULL << VHOST_USER_PROTOCOL_F_INFLIGHT_SHMFD);
>>+#endif
>>      vmsg_set_reply_u64(vmsg, features);
>>      return true;
>>  }
>
>Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>

Thanks,
Stefano



^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v4 08/12] libvhost-user: enable it on any POSIX system
  2024-05-08 10:36   ` Philippe Mathieu-Daudé
@ 2024-05-10  8:56     ` Stefano Garzarella
  2024-05-10  9:56       ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 37+ messages in thread
From: Stefano Garzarella @ 2024-05-10  8:56 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: qemu-devel, Jason Wang, Coiby Xu, Paolo Bonzini, qemu-block,
	Daniel P. Berrangé, Gerd Hoffmann, slp, Brad Smith,
	Eduardo Habkost, Thomas Huth, Eric Blake, Kevin Wolf,
	Markus Armbruster, Raphael Norwitz, gmaglione, Laurent Vivier,
	stefanha, David Hildenbrand, Hanna Reitz, Michael S. Tsirkin,
	Igor Mammedov, Marc-André Lureau

On Wed, May 08, 2024 at 12:36:30PM GMT, Philippe Mathieu-Daudé wrote:
>On 8/5/24 09:44, Stefano Garzarella wrote:
>>The vhost-user protocol is not really Linux-specific so let's enable
>>libvhost-user for any POSIX system.
>>
>>Compiling it on macOS and FreeBSD some problems came up:
>>- avoid to include linux/vhost.h which is avaibale only on Linux
>
>"available"
>
>>   (vhost_types.h contains many of the things we need)
>>- macOS doesn't provide sys/endian.h, so let's define them
>>   (note: libvhost-user doesn't include qemu's headers, so we can't use
>
>"QEMU"
>

Good catches, I'll fix them!

>>    use "qemu/bswap.h")
>>- define eventfd_[write|read] as write/read wrapper when system doesn't
>>   provide those (e.g. macOS)
>>- copy SEAL defines from include/qemu/memfd.h to make the code works
>>   on FreeBSD where MFD_ALLOW_SEALING is defined
>
>Alternatively add in subprojects/libvhost-user/include/osdep.h.

I like the idea, but we also have other things already present before
this patch (e.g. G_GNUC_PRINTF, MIN, etc.) so do you think it's better
to add 2 patches (move everything to osdep.h, add things from this
patch), or after this series is merged, send a patch to introduce
osdep.h?

I'm tempted for the last option just to prevent this series from
becoming too big, but I don't have a strong opinion.

Thanks,
Stefano

>
>>- define MAP_NORESERVE if it's not defined (e.g. on FreeBSD)
>>
>>Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
>>---
>>  meson.build                               |  2 +-
>>  subprojects/libvhost-user/libvhost-user.h |  2 +-
>>  subprojects/libvhost-user/libvhost-user.c | 60 +++++++++++++++++++++--
>>  3 files changed, 59 insertions(+), 5 deletions(-)
>
>Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>
>



^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v4 09/12] contrib/vhost-user-blk: enable it on any POSIX system
  2024-05-08 10:32   ` Philippe Mathieu-Daudé
@ 2024-05-10  9:02     ` Stefano Garzarella
  0 siblings, 0 replies; 37+ messages in thread
From: Stefano Garzarella @ 2024-05-10  9:02 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, Kevin Wolf, Hanna Czenczek
  Cc: qemu-devel, Jason Wang, Coiby Xu, Paolo Bonzini, qemu-block,
	Daniel P. Berrangé, Gerd Hoffmann, slp, Brad Smith,
	Eduardo Habkost, Thomas Huth, Eric Blake, Kevin Wolf,
	Markus Armbruster, Raphael Norwitz, gmaglione, Laurent Vivier,
	stefanha, David Hildenbrand, Hanna Reitz, Michael S. Tsirkin,
	Igor Mammedov, Marc-André Lureau

On Wed, May 08, 2024 at 12:32:08PM GMT, Philippe Mathieu-Daudé wrote:
>On 8/5/24 09:44, Stefano Garzarella wrote:
>>Let's make the code more portable by adding defines from
>>block/file-posix.c to support O_DIRECT in other systems (e.g. macOS).
>>
>>vhost-user-server.c is a dependency, let's enable it for any POSIX
>>system.
>>
>>Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
>>---
>>v4:
>>- moved using of "qemu/bswap.h" API in a separate patch [Phil]
>>---
>>  meson.build                             |  2 --
>>  contrib/vhost-user-blk/vhost-user-blk.c | 14 ++++++++++++++
>>  util/meson.build                        |  4 +++-
>>  3 files changed, 17 insertions(+), 3 deletions(-)
>
>
>>diff --git a/contrib/vhost-user-blk/vhost-user-blk.c b/contrib/vhost-user-blk/vhost-user-blk.c
>>index 9492146855..a450337685 100644
>>--- a/contrib/vhost-user-blk/vhost-user-blk.c
>>+++ b/contrib/vhost-user-blk/vhost-user-blk.c
>>@@ -25,6 +25,20 @@
>>  #include <sys/ioctl.h>
>>  #endif
>>+/* OS X does not have O_DSYNC */
>>+#ifndef O_DSYNC
>>+#ifdef O_SYNC
>>+#define O_DSYNC O_SYNC
>>+#elif defined(O_FSYNC)
>>+#define O_DSYNC O_FSYNC
>>+#endif
>>+#endif
>>+
>>+/* Approximate O_DIRECT with O_DSYNC if O_DIRECT isn't available */
>>+#ifndef O_DIRECT
>>+#define O_DIRECT O_DSYNC
>>+#endif
>
>Could we add that in "qemu/osdep.h" instead?

Since "qemu/osdep.h" includes fcntl.h, I think it could be fine.

@Hanna, @Kevin WDYT?

>
>Otherwise,
>Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>

Thanks,
Stefano



^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v4 10/12] hostmem: add a new memory backend based on POSIX shm_open()
  2024-05-08 11:59   ` Markus Armbruster
@ 2024-05-10  9:37     ` Stefano Garzarella
  0 siblings, 0 replies; 37+ messages in thread
From: Stefano Garzarella @ 2024-05-10  9:37 UTC (permalink / raw)
  To: Markus Armbruster
  Cc: qemu-devel, Jason Wang, Coiby Xu, Paolo Bonzini, qemu-block,
	Daniel P. Berrangé, Gerd Hoffmann, slp,
	Philippe Mathieu-Daudé, Brad Smith, Eduardo Habkost,
	Thomas Huth, Eric Blake, Kevin Wolf, Raphael Norwitz, gmaglione,
	Laurent Vivier, stefanha, David Hildenbrand, Hanna Reitz,
	Michael S. Tsirkin, Igor Mammedov, Marc-André Lureau

On Wed, May 08, 2024 at 01:59:33PM GMT, Markus Armbruster wrote:
>Stefano Garzarella <sgarzare@redhat.com> writes:
>
>> shm_open() creates and opens a new POSIX shared memory object.
>> A POSIX shared memory object allows creating memory backend with an
>> associated file descriptor that can be shared with external processes
>> (e.g. vhost-user).
>>
>> The new `memory-backend-shm` can be used as an alternative when
>> `memory-backend-memfd` is not available (Linux only), since shm_open()
>> should be provided by any POSIX-compliant operating system.
>>
>> This backend mimics memfd, allocating memory that is practically
>> anonymous. In theory shm_open() requires a name, but this is allocated
>> for a short time interval and shm_unlink() is called right after
>> shm_open(). After that, only fd is shared with external processes
>> (e.g., vhost-user) as if it were associated with anonymous memory.
>>
>> In the future we may also allow the user to specify the name to be
>> passed to shm_open(), but for now we keep the backend simple, mimicking
>> anonymous memory such as memfd.
>>
>> Acked-by: David Hildenbrand <david@redhat.com>
>> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
>> ---
>> v4
>> - fail if we find "share=off" in shm_backend_memory_alloc() [David]
>> v3
>> - enriched commit message and documentation to highlight that we
>>   want to mimic memfd (David)
>> ---
>>  docs/system/devices/vhost-user.rst |   5 +-
>>  qapi/qom.json                      |  17 ++++
>>  backends/hostmem-shm.c             | 123 +++++++++++++++++++++++++++++
>>  backends/meson.build               |   1 +
>>  qemu-options.hx                    |  13 +++
>>  5 files changed, 157 insertions(+), 2 deletions(-)
>>  create mode 100644 backends/hostmem-shm.c
>>
>> diff --git a/docs/system/devices/vhost-user.rst b/docs/system/devices/vhost-user.rst
>> index 9b2da106ce..35259d8ec7 100644
>> --- a/docs/system/devices/vhost-user.rst
>> +++ b/docs/system/devices/vhost-user.rst
>> @@ -98,8 +98,9 @@ Shared memory object
>>
>>  In order for the daemon to access the VirtIO queues to process the
>>  requests it needs access to the guest's address space. This is
>> -achieved via the ``memory-backend-file`` or ``memory-backend-memfd``
>> -objects. A reference to a file-descriptor which can access this object
>> +achieved via the ``memory-backend-file``, ``memory-backend-memfd``, or
>> +``memory-backend-shm`` objects.
>> +A reference to a file-descriptor which can access this object
>>  will be passed via the socket as part of the protocol negotiation.
>>
>>  Currently the shared memory object needs to match the size of the main
>> diff --git a/qapi/qom.json b/qapi/qom.json
>> index 38dde6d785..52df052df8 100644
>> --- a/qapi/qom.json
>> +++ b/qapi/qom.json
>> @@ -721,6 +721,19 @@
>>              '*hugetlbsize': 'size',
>>              '*seal': 'bool' } }
>>
>> +##
>> +# @MemoryBackendShmProperties:
>> +#
>> +# Properties for memory-backend-shm objects.
>> +#
>> +# The @share boolean option is true by default with shm.
>
>This contradicts the doc comment for @share:
>
>   # @share: if false, the memory is private to QEMU; if true, it is
>   #     shared (default: false)
>
>Your intention is to override that text.  But that's less than clear.
>Moreover, the documentation of @share is pretty far from this override.
>John Snow is working on patches that'll pull it closer.
>
>Hmm, MemoryBackendMemfdProperties has the same override.

Yep, I followed @MemoryBackendMemfdProperties and 
@MemoryBackendEpcProperties.

>
>I think we should change the doc comment for @share to something like
>
>   # @share: if false, the memory is private to QEMU; if true, it is
>   #     shared (default depends on the backend type)
>
>and then document the actual default with each backend type.

I agree on that, but I think we should do in a separate series/patch.
If you prefer, I can send that patch.

>
>> +#
>> +# Since: 9.1
>> +##
>> +{ 'struct': 'MemoryBackendShmProperties',
>> +  'base': 'MemoryBackendProperties',
>> +  'data': { } }
>
>Let's add 'if': 'CONFIG_POSIX' here.
>

Do you mean something like this:

{ 'struct': 'MemoryBackendShmProperties',
   'if': 'CONFIG_POSIX',
   'base': 'MemoryBackendProperties',
   'data': { } }

I didn't because for MemoryBackendMemfdProperties and
MemoryBackendEpcProperties we have 'if': 'CONFIG_POSIX' only later in
the ObjectOptions union, so I did the same.

Should we fix them as well?

>> +
>>  ##
>>  # @MemoryBackendEpcProperties:
>>  #
>> @@ -985,6 +998,8 @@
>>      { 'name': 'memory-backend-memfd',
>>        'if': 'CONFIG_LINUX' },
>>      'memory-backend-ram',
>> +    { 'name': 'memory-backend-shm',
>> +      'if': 'CONFIG_POSIX' },
>>      'pef-guest',
>>      { 'name': 'pr-manager-helper',
>>        'if': 'CONFIG_LINUX' },
>> @@ -1056,6 +1071,8 @@
>>        'memory-backend-memfd':       { 'type': 'MemoryBackendMemfdProperties',
>>                                        'if': 'CONFIG_LINUX' },
>>        'memory-backend-ram':         'MemoryBackendProperties',
>> +      'memory-backend-shm':         { 'type': 'MemoryBackendShmProperties',
>> +                                      'if': 'CONFIG_POSIX' },
>>        'pr-manager-helper':          { 'type': 'PrManagerHelperProperties',
>>                                        'if': 'CONFIG_LINUX' },
>>        'qtest':                      'QtestProperties',
>
>[...]
>
>> diff --git a/qemu-options.hx b/qemu-options.hx
>> index cf61f6b863..2226172fb0 100644
>> --- a/qemu-options.hx
>> +++ b/qemu-options.hx
>> @@ -5227,6 +5227,19 @@ SRST
>>
>>          The ``share`` boolean option is on by default with memfd.
>>
>> +    ``-object memory-backend-shm,id=id,merge=on|off,dump=on|off,share=on|off,prealloc=on|off,size=size,host-nodes=host-nodes,policy=default|preferred|bind|interleave``
>> +        Creates a POSIX shared memory backend object, which allows
>> +        QEMU to share the memory with an external process (e.g. when
>> +        using vhost-user). This backend mimics memfd, allocating memory that is
>> +        practically anonymous. This is useful when memfd is not available.
>
>This actually explains the purpose, unlike the doc comment in qom.json.
>Same for the existing memory backends; can't fault you for doing your
>new one the same way.  We ought to fix them all.  I'm not demanding you
>do it.
>
>The text could perhaps a bit clearer.  What does "practically anonymous"
>mean?  As far as I understand, memory-backend-shm is a more portable and
>less featureful version of memory-backend-memfd.  Say that?

Yeah, I see, I'll fix it with your suggestion!

>
>> +
>> +        Please refer to ``memory-backend-file`` for a description of the
>> +        options.
>> +
>> +        The ``share`` boolean option is on by default with shm. Setting it to
>> +        off will cause a failure during allocation because it is not supported
>> +        by this backend.
>> +
>
>The "will case a failure" part is documented only here, and not in
>qom.json.

I'll add it also in qom.json.

>
>Not this patch's fault: documentation for -object memory-backend-epc is
>missing.

Oh right, I don't know anything about it. Should we ping Sean 
Christopherson <sean.j.christopherson@intel.com> who added it?

Thanks,
Stefano

>
>>      ``-object iommufd,id=id[,fd=fd]``
>>          Creates an iommufd backend which allows control of DMA mapping
>>          through the ``/dev/iommu`` device.
>



^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v4 08/12] libvhost-user: enable it on any POSIX system
  2024-05-10  8:56     ` Stefano Garzarella
@ 2024-05-10  9:56       ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 37+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-05-10  9:56 UTC (permalink / raw)
  To: Stefano Garzarella
  Cc: qemu-devel, Jason Wang, Coiby Xu, Paolo Bonzini, qemu-block,
	Daniel P. Berrangé, Gerd Hoffmann, slp, Brad Smith,
	Eduardo Habkost, Thomas Huth, Eric Blake, Kevin Wolf,
	Markus Armbruster, Raphael Norwitz, gmaglione, Laurent Vivier,
	stefanha, David Hildenbrand, Hanna Reitz, Michael S. Tsirkin,
	Igor Mammedov, Marc-André Lureau

On 10/5/24 10:56, Stefano Garzarella wrote:
> On Wed, May 08, 2024 at 12:36:30PM GMT, Philippe Mathieu-Daudé wrote:
>> On 8/5/24 09:44, Stefano Garzarella wrote:
>>> The vhost-user protocol is not really Linux-specific so let's enable
>>> libvhost-user for any POSIX system.


>> Alternatively add in subprojects/libvhost-user/include/osdep.h.
> 
> I like the idea, but we also have other things already present before
> this patch (e.g. G_GNUC_PRINTF, MIN, etc.) so do you think it's better
> to add 2 patches (move everything to osdep.h, add things from this
> patch), or after this series is merged, send a patch to introduce
> osdep.h?
> 
> I'm tempted for the last option just to prevent this series from
> becoming too big, but I don't have a strong opinion.

Whichever is simpler for your workflow works for me :)

Regards,

Phil.


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v4 11/12] tests/qtest/vhost-user-blk-test: use memory-backend-shm
  2024-05-08  7:44 ` [PATCH v4 11/12] tests/qtest/vhost-user-blk-test: use memory-backend-shm Stefano Garzarella
  2024-05-10  5:57   ` Thomas Huth
@ 2024-05-10 10:54   ` Philippe Mathieu-Daudé
  1 sibling, 0 replies; 37+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-05-10 10:54 UTC (permalink / raw)
  To: Stefano Garzarella, qemu-devel
  Cc: Jason Wang, Coiby Xu, Paolo Bonzini, qemu-block,
	Daniel P. Berrangé, Gerd Hoffmann, slp, Brad Smith,
	Eduardo Habkost, Thomas Huth, Eric Blake, Kevin Wolf,
	Markus Armbruster, Raphael Norwitz, gmaglione, Laurent Vivier,
	stefanha, David Hildenbrand, Hanna Reitz, Michael S. Tsirkin,
	Igor Mammedov, Marc-André Lureau

On 8/5/24 09:44, Stefano Garzarella wrote:
> `memory-backend-memfd` is available only on Linux while the new
> `memory-backend-shm` can be used on any POSIX-compliant operating
> system. Let's use it so we can run the test in multiple environments.
> 
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
>   tests/qtest/vhost-user-blk-test.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
(on macOS)


^ permalink raw reply	[flat|nested] 37+ messages in thread

end of thread, other threads:[~2024-05-10 10:54 UTC | newest]

Thread overview: 37+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-05-08  7:44 [PATCH v4 00/12] vhost-user: support any POSIX system (tested on macOS, FreeBSD, OpenBSD) Stefano Garzarella
2024-05-08  7:44 ` [PATCH v4 01/12] libvhost-user: set msg.msg_control to NULL when it is empty Stefano Garzarella
2024-05-08  8:57   ` Daniel P. Berrangé
2024-05-08  9:33     ` Stefano Garzarella
2024-05-08 10:23   ` Philippe Mathieu-Daudé
2024-05-08  7:44 ` [PATCH v4 02/12] libvhost-user: fail vu_message_write() if sendmsg() is failing Stefano Garzarella
2024-05-08  8:59   ` Daniel P. Berrangé
2024-05-08 10:23   ` Philippe Mathieu-Daudé
2024-05-08  7:44 ` [PATCH v4 03/12] libvhost-user: mask F_INFLIGHT_SHMFD if memfd is not supported Stefano Garzarella
2024-05-08 10:39   ` Philippe Mathieu-Daudé
2024-05-10  8:25     ` Stefano Garzarella
2024-05-08  7:44 ` [PATCH v4 04/12] vhost-user-server: do not set memory fd non-blocking Stefano Garzarella
2024-05-08  9:00   ` Daniel P. Berrangé
2024-05-08  7:44 ` [PATCH v4 05/12] contrib/vhost-user-blk: fix bind() using the right size of the address Stefano Garzarella
2024-05-08 10:25   ` Philippe Mathieu-Daudé
2024-05-08  7:44 ` [PATCH v4 06/12] contrib/vhost-user-*: use QEMU bswap helper functions Stefano Garzarella
2024-05-08 10:13   ` Philippe Mathieu-Daudé
2024-05-08 10:25     ` Philippe Mathieu-Daudé
2024-05-08  7:44 ` [PATCH v4 07/12] vhost-user: enable frontends on any POSIX system Stefano Garzarella
2024-05-08 10:26   ` Philippe Mathieu-Daudé
2024-05-08  7:44 ` [PATCH v4 08/12] libvhost-user: enable it " Stefano Garzarella
2024-05-08 10:36   ` Philippe Mathieu-Daudé
2024-05-10  8:56     ` Stefano Garzarella
2024-05-10  9:56       ` Philippe Mathieu-Daudé
2024-05-08  7:44 ` [PATCH v4 09/12] contrib/vhost-user-blk: " Stefano Garzarella
2024-05-08 10:32   ` Philippe Mathieu-Daudé
2024-05-10  9:02     ` Stefano Garzarella
2024-05-08  7:44 ` [PATCH v4 10/12] hostmem: add a new memory backend based on POSIX shm_open() Stefano Garzarella
2024-05-08 11:59   ` Markus Armbruster
2024-05-10  9:37     ` Stefano Garzarella
2024-05-08  7:44 ` [PATCH v4 11/12] tests/qtest/vhost-user-blk-test: use memory-backend-shm Stefano Garzarella
2024-05-10  5:57   ` Thomas Huth
2024-05-10 10:54   ` Philippe Mathieu-Daudé
2024-05-08  7:44 ` [PATCH v4 12/12] tests/qtest/vhost-user-test: add a test case for memory-backend-shm Stefano Garzarella
2024-05-10  5:58   ` Thomas Huth
2024-05-08 10:39 ` [PATCH v4 00/12] vhost-user: support any POSIX system (tested on macOS, FreeBSD, OpenBSD) Philippe Mathieu-Daudé
2024-05-09 19:20 ` Stefan Hajnoczi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).