| Message ID | 20240523145522.313012-10-sgarzare@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | vhost-user: support any POSIX system (tested on macOS, FreeBSD, OpenBSD) | expand |
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 >
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
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
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
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(-)