diff mbox series

[for-9.1,v3,08/11] contrib/vhost-user-blk: enable it on any POSIX system

Message ID 20240404122330.92710-9-sgarzare@redhat.com
State New
Headers show
Series vhost-user: support any POSIX system (tested on macOS, FreeBSD, OpenBSD) | expand

Commit Message

Stefano Garzarella April 4, 2024, 12:23 p.m. UTC 
Let's make the code more portable by using the "qemu/bswap.h" API
and 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>
---
 meson.build                             |  2 --
 contrib/vhost-user-blk/vhost-user-blk.c | 19 +++++++++++++++++--
 util/meson.build                        |  4 +++-
 3 files changed, 20 insertions(+), 5 deletions(-)

Comments

Philippe Mathieu-Daudé April 4, 2024, 2 p.m. UTC | #1 
Hi Stefano,

On 4/4/24 14:23, Stefano Garzarella wrote:
> Let's make the code more portable by using the "qemu/bswap.h" API
> and 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>
> ---
>   meson.build                             |  2 --
>   contrib/vhost-user-blk/vhost-user-blk.c | 19 +++++++++++++++++--
>   util/meson.build                        |  4 +++-
>   3 files changed, 20 insertions(+), 5 deletions(-)


> diff --git a/contrib/vhost-user-blk/vhost-user-blk.c b/contrib/vhost-user-blk/vhost-user-blk.c
> index a8ab9269a2..462e584857 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"


> @@ -267,13 +282,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 {
Can we switch to the bswap API in a preliminary patch,
converting all the source files?
Stefano Garzarella April 8, 2024, 7:41 a.m. UTC | #2 
On Thu, Apr 04, 2024 at 04:00:38PM +0200, Philippe Mathieu-Daudé wrote:
>Hi Stefano,

Hi Phil!

>
>On 4/4/24 14:23, Stefano Garzarella wrote:
>>Let's make the code more portable by using the "qemu/bswap.h" API
>>and 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>
>>---
>>  meson.build                             |  2 --
>>  contrib/vhost-user-blk/vhost-user-blk.c | 19 +++++++++++++++++--
>>  util/meson.build                        |  4 +++-
>>  3 files changed, 20 insertions(+), 5 deletions(-)
>
>
>>diff --git a/contrib/vhost-user-blk/vhost-user-blk.c b/contrib/vhost-user-blk/vhost-user-blk.c
>>index a8ab9269a2..462e584857 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"
>
>
>>@@ -267,13 +282,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 {
>Can we switch to the bswap API in a preliminary patch,

Sure, I tried to minimize the patches because it's already big,
but I can split this.

>converting all the source files?
>

What do you mean with "all the source files"?

"le64toh" is used here and in some subprojects (e.g. libvduse,
libvhost-user), where IIUC we can't use QEMU's bswap.h because we
don't want to put a dependency with the QEMU code.

BTW I'll check for other *toh() usage in QEMU code and change in the
preliminary patch you suggested to add.

Thanks for the review,
Stefano
diff mbox series

Patch

diff --git a/meson.build b/meson.build
index 3197a2f62e..b541e5c875 100644
--- a/meson.build
+++ b/meson.build
@@ -1956,8 +1956,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 a8ab9269a2..462e584857 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"
 
@@ -24,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,
 };
@@ -267,13 +282,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/util/meson.build b/util/meson.build
index 0ef9886be0..f52682ce96 100644
--- a/util/meson.build
+++ b/util/meson.build
@@ -113,10 +113,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
 endif
 
 if cpu == 'aarch64'