1. Patchew
  2. QEMU
  3. [PATCH v5 00/13] vhost-user: support any POSIX system (tested on macOS, FreeBSD, OpenBSD)
  4. View patch

[PATCH v5 09/13] osdep: move O_DSYNC and O_DIRECT defines from file-posix

Stefano Garzarella posted 13 patches 6 months ago
Maintainers: David Hildenbrand <david@redhat.com>, Igor Mammedov <imammedo@redhat.com>, Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>, Raphael Norwitz <raphael@enfabrica.net>, "Michael S. Tsirkin" <mst@redhat.com>, Gerd Hoffmann <kraxel@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Jason Wang <jasowang@redhat.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Thomas Huth <thuth@redhat.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Eric Blake <eblake@redhat.com>, Markus Armbruster <armbru@redhat.com>, Eduardo Habkost <eduardo@habkost.net>, Coiby Xu <Coiby.Xu@gmail.com>, Laurent Vivier <lvivier@redhat.com>
There is a newer version of this series
[PATCH v5 09/13] osdep: move O_DSYNC and O_DIRECT defines from file-posix
Posted by Stefano Garzarella 6 months ago 
These defines are also useful for vhost-user-blk when it is compiled
in some POSIX systems that do not define them, so let's move them to
“qemu/osdep.h”.

Suggested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
 include/qemu/osdep.h | 14 ++++++++++++++
 block/file-posix.c   | 14 --------------
 2 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index f61edcfdc2..e165b5cb1b 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -325,6 +325,20 @@ void QEMU_ERROR("code path is reachable")
 #define ESHUTDOWN 4099
 #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
+
 #define RETRY_ON_EINTR(expr) \
     (__extension__                                          \
         ({ typeof(expr) __result;                               \
diff --git a/block/file-posix.c b/block/file-posix.c
index 35684f7e21..7a196a2abf 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -110,20 +110,6 @@
 #include <sys/diskslice.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
-
 #define FTYPE_FILE   0
 #define FTYPE_CD     1
 
-- 
2.45.1
Re: [PATCH v5 09/13] osdep: move O_DSYNC and O_DIRECT defines from file-posix
Posted by Daniel P. Berrangé 6 months ago 
On Thu, May 23, 2024 at 04:55:18PM +0200, Stefano Garzarella wrote:
> These defines are also useful for vhost-user-blk when it is compiled
> in some POSIX systems that do not define them, so let's move them to
> “qemu/osdep.h”.
> 
> Suggested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
>  include/qemu/osdep.h | 14 ++++++++++++++
>  block/file-posix.c   | 14 --------------
>  2 files changed, 14 insertions(+), 14 deletions(-)
> 
> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> index f61edcfdc2..e165b5cb1b 100644
> --- a/include/qemu/osdep.h
> +++ b/include/qemu/osdep.h
> @@ -325,6 +325,20 @@ void QEMU_ERROR("code path is reachable")
>  #define ESHUTDOWN 4099
>  #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

Please don't do this - we can't be confident that all code in
QEMU will be OK with O_DIRECT being simulated in this way.

I'm not convinced that the O_DSYNC simulation is a good idea
to do tree-wide either.

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 :|
Re: [PATCH v5 09/13] osdep: move O_DSYNC and O_DIRECT defines from file-posix
Posted by Stefano Garzarella 6 months ago 
On Thu, May 23, 2024 at 04:14:48PM GMT, Daniel P. Berrangé wrote:
>On Thu, May 23, 2024 at 04:55:18PM +0200, Stefano Garzarella wrote:
>> These defines are also useful for vhost-user-blk when it is compiled
>> in some POSIX systems that do not define them, so let's move them to
>> “qemu/osdep.h”.
>>
>> Suggested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
>> ---
>>  include/qemu/osdep.h | 14 ++++++++++++++
>>  block/file-posix.c   | 14 --------------
>>  2 files changed, 14 insertions(+), 14 deletions(-)
>>
>> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
>> index f61edcfdc2..e165b5cb1b 100644
>> --- a/include/qemu/osdep.h
>> +++ b/include/qemu/osdep.h
>> @@ -325,6 +325,20 @@ void QEMU_ERROR("code path is reachable")
>>  #define ESHUTDOWN 4099
>>  #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
>
>Please don't do this - we can't be confident that all code in
>QEMU will be OK with O_DIRECT being simulated in this way.
>
>I'm not convinced that the O_DSYNC simulation is a good idea
>to do tree-wide either.

I was a little scared, and you and the failing tests on win64 convinced 
me to bring this back as in v4 ;-)

Thanks,
Stefano
Re: [PATCH v5 09/13] osdep: move O_DSYNC and O_DIRECT defines from file-posix
Posted by Stefano Garzarella 6 months ago 
On Thu, May 23, 2024 at 04:55:18PM GMT, Stefano Garzarella wrote:
>These defines are also useful for vhost-user-blk when it is compiled
>in some POSIX systems that do not define them, so let's move them to
>“qemu/osdep.h”.
>
>Suggested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
>---
> include/qemu/osdep.h | 14 ++++++++++++++
> block/file-posix.c   | 14 --------------
> 2 files changed, 14 insertions(+), 14 deletions(-)

This seems to break the compilation on win64: 
https://gitlab.com/sgarzarella/qemu/-/jobs/6923403322

     In file included from ../util/osdep.c:24:
     ../util/osdep.c: In function 'qemu_open_internal':
     ../include/qemu/osdep.h:339:18: error: 'O_DSYNC' undeclared (first use in this function)
       339 | #define O_DIRECT O_DSYNC
           |                  ^~~~~~~
     ../util/osdep.c:334:41: note: in expansion of macro 'O_DIRECT'
       334 |         if (errno == EINVAL && (flags & O_DIRECT)) {
           |                                         ^~~~~~~~
     ../include/qemu/osdep.h:339:18: note: each undeclared identifier is reported only once for each function it appears in
       339 | #define O_DIRECT O_DSYNC
           |                  ^~~~~~~
     ../util/osdep.c:334:41: note: in expansion of macro 'O_DIRECT'
       334 |         if (errno == EINVAL && (flags & O_DIRECT)) {
           |                                         ^~~~~~~~
     ../util/osdep.c: In function 'qemu_open_old':
     ../include/qemu/osdep.h:339:18: error: 'O_DSYNC' undeclared (first use in this function)
       339 | #define O_DIRECT O_DSYNC
           |                  ^~~~~~~
     ../util/osdep.c:385:50: note: in expansion of macro 'O_DIRECT'
       385 |     if (ret == -1 && errno == EINVAL && (flags & O_DIRECT)) {
           |

Indeed file-posix.c was not compiled on windows. Oops, I didn't think of 
that :-(

I'm thinking on putting a guard on CONFIG_POSIX, or just checking that 
O_DSYNC is defined. Any suggestion?

Thanks,
Stefano

>
>diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
>index f61edcfdc2..e165b5cb1b 100644
>--- a/include/qemu/osdep.h
>+++ b/include/qemu/osdep.h
>@@ -325,6 +325,20 @@ void QEMU_ERROR("code path is reachable")
> #define ESHUTDOWN 4099
> #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
>+
> #define RETRY_ON_EINTR(expr) \
>     (__extension__                                          \
>         ({ typeof(expr) __result;                               \
>diff --git a/block/file-posix.c b/block/file-posix.c
>index 35684f7e21..7a196a2abf 100644
>--- a/block/file-posix.c
>+++ b/block/file-posix.c
>@@ -110,20 +110,6 @@
> #include <sys/diskslice.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
>-
> #define FTYPE_FILE   0
> #define FTYPE_CD     1
>
>-- 
>2.45.1
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.