diff mbox series

[v9,09/11] block: check availablity for preadv/pwritev on mac

Message ID 20210126012457.39046-10-j@getutm.app (mailing list archive)
State New, archived
Headers show
Series iOS and Apple Silicon host support | expand

Commit Message

Joelle van Dyne Jan. 26, 2021, 1:24 a.m. UTC
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(+)

Comments

Peter Maydell Jan. 26, 2021, 3:54 p.m. UTC | #1
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 mbox series

Patch

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