[v4] vhost-user: support any POSIX system (tested on macOS, FreeBSD, OpenBSD)

[PATCH v4 09/12] contrib/vhost-user-blk: enable it on any POSIX system

Posted by Stefano Garzarella 6 months, 3 weeks ago

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

Re: [PATCH v4 09/12] contrib/vhost-user-blk: enable it on any POSIX system

Posted by Philippe Mathieu-Daudé 6 months, 3 weeks ago

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>

Re: [PATCH v4 09/12] contrib/vhost-user-blk: enable it on any POSIX system

Posted by Stefano Garzarella 6 months, 2 weeks ago

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