| Message ID | 20210126012457.39046-10-j@getutm.app (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | iOS and Apple Silicon host support | expand |
On Tue, 26 Jan 2021 at 01:38, Joelle van Dyne <j@getutm.app> wrote: > > macOS 11/iOS 14 added preadv/pwritev APIs. Due to weak linking, configure > will succeed with CONFIG_PREADV even when targeting a lower OS version. I just ran into this this afternoon. It turns out that all our OSX CI configs pass --enable-werror or equivalent to configure, which means that when the configure test provokes the warning that "'preadv' is only available on macOS 11.0 or newer", that is a compile error due to -Werror, and configure decides preadv is not available. But if you do a configure for the default setup that doesn't add -Werror then the test passes and then the binaries fail at runtime... and this is the first time I'd happened to do a build with the newer XCode SDK and without -Werror enabled. So I think that leaves two points for this patch: (1) we need to fix the configure test so that it either succeeds without warnings or fails, so that --enable-werror and --disable-werror configures make the same decision about preadv support. (2) we need to decide whether we want to support the OSX idea that you can compile in support for a function like preadv() and then find that it's not present at runtime, or if we just want to make the choice at configure time. I'm on the fence about this. I'm going to send out a patch which converts the configure test to a meson.build one-liner -- this fixes (1) and (by default) leaves the answer to (2) at "no" (you get preadv() only if you built on macOS 11 for macOS 11; if you build with 10.x support then you dont' get it). I'm agnostic about the final answer to (2) -- we do have the support for the runtime preadv_present flag in this file already, after all -- so I guess I'll leave that to the block maintainers. In the meantime we can fix the non-controversial part. thanks -- PMM
diff --git a/block/file-posix.c b/block/file-posix.c index 666d3e7504..6473f84db8 100644 --- a/block/file-posix.c +++ b/block/file-posix.c @@ -1386,17 +1386,50 @@ static int handle_aiocb_flush(void *opaque) #ifdef CONFIG_PREADV static bool preadv_present = true; +static bool preadv_checked; static ssize_t qemu_preadv(int fd, const struct iovec *iov, int nr_iov, off_t offset) { +#ifdef CONFIG_DARWIN /* preadv introduced in macOS 11 */ + if (unlikely(!preadv_checked)) { + if (__builtin_available(macOS 11, iOS 14, watchOS 7, tvOS 14, *)) { + preadv_checked = true; + } else { + preadv_present = false; + return -ENOSYS; + } + } + /* Now we suppress the availability warning since we use the cached check */ +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wunguarded-availability-new" + return preadv(fd, iov, nr_iov, offset); +#pragma clang diagnostic pop +#else /* CONFIG_DARWIN */ return preadv(fd, iov, nr_iov, offset); +#endif } static ssize_t qemu_pwritev(int fd, const struct iovec *iov, int nr_iov, off_t offset) { +#ifdef CONFIG_DARWIN /* preadv introduced in macOS 11 */ + if (unlikely(!preadv_checked)) { + if (__builtin_available(macOS 11, iOS 14, watchOS 7, tvOS 14, *)) { + preadv_checked = true; + } else { + preadv_present = false; + return -ENOSYS; + } + } + /* Now we suppress the availability warning since we use the cached check */ +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wunguarded-availability-new" + return pwritev(fd, iov, nr_iov, offset); +#pragma clang diagnostic pop +#else /* CONFIG_DARWIN */ return pwritev(fd, iov, nr_iov, offset); +#endif } #else
macOS 11/iOS 14 added preadv/pwritev APIs. Due to weak linking, configure will succeed with CONFIG_PREADV even when targeting a lower OS version. We therefore need to check at run time if we can actually use these APIs. Signed-off-by: Joelle van Dyne <j@getutm.app> --- block/file-posix.c | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+)