[Qemu-block] [PULL 07/18] block/qapi: Move 'aio' option to file driver

[Kevin Wolf]

The option whether or not to use a native AIO interface really isn't a
generic option for all drivers, but only applies to the native file
protocols. This patch moves the option in blockdev-add to the
appropriate places (raw-posix and raw-win32).

We still have to keep the flag BDRV_O_NATIVE_AIO for compatibility
because so far the AIO option was usually specified on the wrong layer
(the top-level format driver, which didn't even look at it) and then
inherited by the protocol driver (where it was actually used). We can't
forbid this use except in new interfaces.

Signed-off-by: Kevin Wolf <address@hidden>
Reviewed-by: Eric Blake <address@hidden>
Reviewed-by: Max Reitz <address@hidden>
---
 block/raw-posix.c      | 44 ++++++++++++++++++++++++---------------
 block/raw-win32.c      | 56 +++++++++++++++++++++++++++++++++++++++++++++-----
 qapi/block-core.json   |  6 +++---
 tests/qemu-iotests/087 |  4 ++--
 4 files changed, 83 insertions(+), 27 deletions(-)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index 6ed7547..166e9d1 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -143,6 +143,7 @@ typedef struct BDRVRawState {
     bool has_discard:1;
     bool has_write_zeroes:1;
     bool discard_zeroes:1;
+    bool use_linux_aio:1;
     bool has_fallocate;
     bool needs_alignment;
 } BDRVRawState;
@@ -367,18 +368,6 @@ static void raw_parse_flags(int bdrv_flags, int 
*open_flags)
     }
 }
 
-#ifdef CONFIG_LINUX_AIO
-static bool raw_use_aio(int bdrv_flags)
-{
-    /*
-     * Currently Linux do AIO only for files opened with O_DIRECT
-     * specified so check NOCACHE flag too
-     */
-    return (bdrv_flags & (BDRV_O_NOCACHE|BDRV_O_NATIVE_AIO)) ==
-                         (BDRV_O_NOCACHE|BDRV_O_NATIVE_AIO);
-}
-#endif
-
 static void raw_parse_filename(const char *filename, QDict *options,
                                Error **errp)
 {
@@ -399,6 +388,11 @@ static QemuOptsList raw_runtime_opts = {
             .type = QEMU_OPT_STRING,
             .help = "File name of the image",
         },
+        {
+            .name = "aio",
+            .type = QEMU_OPT_STRING,
+            .help = "host AIO implementation (threads, native)",
+        },
         { /* end of list */ }
     },
 };
@@ -410,6 +404,7 @@ static int raw_open_common(BlockDriverState *bs, QDict 
*options,
     QemuOpts *opts;
     Error *local_err = NULL;
     const char *filename = NULL;
+    BlockdevAioOptions aio, aio_default;
     int fd, ret;
     struct stat st;
 
@@ -429,6 +424,18 @@ static int raw_open_common(BlockDriverState *bs, QDict 
*options,
         goto fail;
     }
 
+    aio_default = (bdrv_flags & BDRV_O_NATIVE_AIO)
+                  ? BLOCKDEV_AIO_OPTIONS_NATIVE
+                  : BLOCKDEV_AIO_OPTIONS_THREADS;
+    aio = qapi_enum_parse(BlockdevAioOptions_lookup, qemu_opt_get(opts, "aio"),
+                          BLOCKDEV_AIO_OPTIONS__MAX, aio_default, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        ret = -EINVAL;
+        goto fail;
+    }
+    s->use_linux_aio = (aio == BLOCKDEV_AIO_OPTIONS_NATIVE);
+
     s->open_flags = open_flags;
     raw_parse_flags(bdrv_flags, &s->open_flags);
 
@@ -444,14 +451,15 @@ static int raw_open_common(BlockDriverState *bs, QDict 
*options,
     s->fd = fd;
 
 #ifdef CONFIG_LINUX_AIO
-    if (!raw_use_aio(bdrv_flags) && (bdrv_flags & BDRV_O_NATIVE_AIO)) {
+     /* Currently Linux does AIO only for files opened with O_DIRECT */
+    if (s->use_linux_aio && !(s->open_flags & O_DIRECT)) {
         error_setg(errp, "aio=native was specified, but it requires "
                          "cache.direct=on, which was not specified.");
         ret = -EINVAL;
         goto fail;
     }
 #else
-    if (bdrv_flags & BDRV_O_NATIVE_AIO) {
+    if (s->use_linux_aio) {
         error_setg(errp, "aio=native was specified, but is not supported "
                          "in this build.");
         ret = -EINVAL;
@@ -1256,7 +1264,7 @@ static int coroutine_fn raw_co_prw(BlockDriverState *bs, 
uint64_t offset,
         if (!bdrv_qiov_is_aligned(bs, qiov)) {
             type |= QEMU_AIO_MISALIGNED;
 #ifdef CONFIG_LINUX_AIO
-        } else if (bs->open_flags & BDRV_O_NATIVE_AIO) {
+        } else if (s->use_linux_aio) {
             LinuxAioState *aio = aio_get_linux_aio(bdrv_get_aio_context(bs));
             assert(qiov->size == bytes);
             return laio_co_submit(bs, aio, s->fd, offset, qiov, type);
@@ -1285,7 +1293,8 @@ static int coroutine_fn raw_co_pwritev(BlockDriverState 
*bs, uint64_t offset,
 static void raw_aio_plug(BlockDriverState *bs)
 {
 #ifdef CONFIG_LINUX_AIO
-    if (bs->open_flags & BDRV_O_NATIVE_AIO) {
+    BDRVRawState *s = bs->opaque;
+    if (s->use_linux_aio) {
         LinuxAioState *aio = aio_get_linux_aio(bdrv_get_aio_context(bs));
         laio_io_plug(bs, aio);
     }
@@ -1295,7 +1304,8 @@ static void raw_aio_plug(BlockDriverState *bs)
 static void raw_aio_unplug(BlockDriverState *bs)
 {
 #ifdef CONFIG_LINUX_AIO
-    if (bs->open_flags & BDRV_O_NATIVE_AIO) {
+    BDRVRawState *s = bs->opaque;
+    if (s->use_linux_aio) {
         LinuxAioState *aio = aio_get_linux_aio(bdrv_get_aio_context(bs));
         laio_io_unplug(bs, aio);
     }
diff --git a/block/raw-win32.c b/block/raw-win32.c
index 56f45fe..734bb10 100644
--- a/block/raw-win32.c
+++ b/block/raw-win32.c
@@ -32,6 +32,7 @@
 #include "block/thread-pool.h"
 #include "qemu/iov.h"
 #include "qapi/qmp/qstring.h"
+#include "qapi/util.h"
 #include <windows.h>
 #include <winioctl.h>
 
@@ -252,7 +253,8 @@ static void raw_probe_alignment(BlockDriverState *bs, Error 
**errp)
     }
 }
 
-static void raw_parse_flags(int flags, int *access_flags, DWORD *overlapped)
+static void raw_parse_flags(int flags, bool use_aio, int *access_flags,
+                            DWORD *overlapped)
 {
     assert(access_flags != NULL);
     assert(overlapped != NULL);
@@ -264,7 +266,7 @@ static void raw_parse_flags(int flags, int *access_flags, 
DWORD *overlapped)
     }
 
     *overlapped = FILE_ATTRIBUTE_NORMAL;
-    if (flags & BDRV_O_NATIVE_AIO) {
+    if (use_aio) {
         *overlapped |= FILE_FLAG_OVERLAPPED;
     }
     if (flags & BDRV_O_NOCACHE) {
@@ -292,10 +294,35 @@ static QemuOptsList raw_runtime_opts = {
             .type = QEMU_OPT_STRING,
             .help = "File name of the image",
         },
+        {
+            .name = "aio",
+            .type = QEMU_OPT_STRING,
+            .help = "host AIO implementation (threads, native)",
+        },
         { /* end of list */ }
     },
 };
 
+static bool get_aio_option(QemuOpts *opts, int flags, Error **errp)
+{
+    BlockdevAioOptions aio, aio_default;
+
+    aio_default = (flags & BDRV_O_NATIVE_AIO) ? BLOCKDEV_AIO_OPTIONS_NATIVE
+                                              : BLOCKDEV_AIO_OPTIONS_THREADS;
+    aio = qapi_enum_parse(BlockdevAioOptions_lookup, qemu_opt_get(opts, "aio"),
+                          BLOCKDEV_AIO_OPTIONS__MAX, aio_default, errp);
+
+    switch (aio) {
+    case BLOCKDEV_AIO_OPTIONS_NATIVE:
+        return true;
+    case BLOCKDEV_AIO_OPTIONS_THREADS:
+        return false;
+    default:
+        error_setg(errp, "Invalid AIO option");
+    }
+    return false;
+}
+
 static int raw_open(BlockDriverState *bs, QDict *options, int flags,
                     Error **errp)
 {
@@ -305,6 +332,7 @@ static int raw_open(BlockDriverState *bs, QDict *options, 
int flags,
     QemuOpts *opts;
     Error *local_err = NULL;
     const char *filename;
+    bool use_aio;
     int ret;
 
     s->type = FTYPE_FILE;
@@ -319,7 +347,14 @@ static int raw_open(BlockDriverState *bs, QDict *options, 
int flags,
 
     filename = qemu_opt_get(opts, "filename");
 
-    raw_parse_flags(flags, &access_flags, &overlapped);
+    use_aio = get_aio_option(opts, flags, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        ret = -EINVAL;
+        goto fail;
+    }
+
+    raw_parse_flags(flags, use_aio, &access_flags, &overlapped);
 
     if (filename[0] && filename[1] == ':') {
         snprintf(s->drive_path, sizeof(s->drive_path), "%c:\\", filename[0]);
@@ -346,7 +381,7 @@ static int raw_open(BlockDriverState *bs, QDict *options, 
int flags,
         goto fail;
     }
 
-    if (flags & BDRV_O_NATIVE_AIO) {
+    if (use_aio) {
         s->aio = win32_aio_init();
         if (s->aio == NULL) {
             CloseHandle(s->hfile);
@@ -647,6 +682,7 @@ static int hdev_open(BlockDriverState *bs, QDict *options, 
int flags,
 
     Error *local_err = NULL;
     const char *filename;
+    bool use_aio;
 
     QemuOpts *opts = qemu_opts_create(&raw_runtime_opts, NULL, 0,
                                       &error_abort);
@@ -659,6 +695,16 @@ static int hdev_open(BlockDriverState *bs, QDict *options, 
int flags,
 
     filename = qemu_opt_get(opts, "filename");
 
+    use_aio = get_aio_option(opts, flags, &local_err);
+    if (!local_err && use_aio) {
+        error_setg(&local_err, "AIO is not supported on Windows host devices");
+    }
+    if (local_err) {
+        error_propagate(errp, local_err);
+        ret = -EINVAL;
+        goto done;
+    }
+
     if (strstart(filename, "/dev/cdrom", NULL)) {
         if (find_cdrom(device_name, sizeof(device_name)) < 0) {
             error_setg(errp, "Could not open CD-ROM drive");
@@ -677,7 +723,7 @@ static int hdev_open(BlockDriverState *bs, QDict *options, 
int flags,
     }
     s->type = find_device_type(bs, filename);
 
-    raw_parse_flags(flags, &access_flags, &overlapped);
+    raw_parse_flags(flags, use_aio, &access_flags, &overlapped);
 
     create_flags = OPEN_EXISTING;
 
diff --git a/qapi/block-core.json b/qapi/block-core.json
index b5fdd42..9d797b8 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1724,11 +1724,13 @@
 # Driver specific block device options for the file backend.
 #
 # @filename:    path to the image file
+# @aio:         #optional AIO backend (default: threads) (since: 2.8)
 #
 # Since: 1.7
 ##
 { 'struct': 'BlockdevOptionsFile',
-  'data': { 'filename': 'str' } }
+  'data': { 'filename': 'str',
+            '*aio': 'BlockdevAioOptions' } }
 
 ##
 # @BlockdevOptionsNull
@@ -2232,7 +2234,6 @@
 #                 This option is required on the top level of blockdev-add.
 # @discard:       #optional discard-related options (default: ignore)
 # @cache:         #optional cache-related options
-# @aio:           #optional AIO backend (default: threads)
 # @read-only:     #optional whether the block device should be read-only
 #                 (default: false)
 # @detect-zeroes: #optional detect and optimize zero writes (Since 2.1)
@@ -2247,7 +2248,6 @@
             '*node-name': 'str',
             '*discard': 'BlockdevDiscardOptions',
             '*cache': 'BlockdevCacheOptions',
-            '*aio': 'BlockdevAioOptions',
             '*read-only': 'bool',
             '*detect-zeroes': 'BlockdevDetectZeroesOptions' },
   'discriminator': 'driver',
diff --git a/tests/qemu-iotests/087 b/tests/qemu-iotests/087
index 5c04577..b1ac71f 100755
--- a/tests/qemu-iotests/087
+++ b/tests/qemu-iotests/087
@@ -117,10 +117,10 @@ run_qemu <<EOF
       "options": {
         "driver": "$IMGFMT",
         "node-name": "disk",
-        "aio": "native",
         "file": {
             "driver": "file",
-            "filename": "$TEST_IMG"
+            "filename": "$TEST_IMG",
+            "aio": "native"
         }
       }
     }
-- 
1.8.3.1