diff mbox series

[v9,07/17] blockdev: adds bdrv_parse_aio to use io_uring

Message ID 20190801234031.29561-8-mehta.aaru20@gmail.com
State New, archived
Headers show
Series Add support for io_uring | expand

Commit Message

Aarushi Mehta Aug. 1, 2019, 11:40 p.m. UTC
Signed-off-by: Aarushi Mehta <mehta.aaru20@gmail.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block.c               | 22 ++++++++++++++++++++++
 blockdev.c            | 12 ++++--------
 include/block/block.h |  1 +
 3 files changed, 27 insertions(+), 8 deletions(-)

Comments

McMillan, Erich" via Aug. 7, 2019, 11:44 a.m. UTC | #1
On Fri, Aug 2, 2019 at 1:41 AM Aarushi Mehta <mehta.aaru20@gmail.com> wrote:
> +int bdrv_parse_aio(const char *mode, int *flags)
> +{
> +    if (!strcmp(mode, "threads")) {
> +        /* do nothing, default */
> +    } else if (!strcmp(mode, "native")) {
> +        *flags |= BDRV_O_NATIVE_AIO;

This 'if' should be covered with CONFIG_LINUX_AIO.

Best regards, Julia Suvorova.

> +#ifdef CONFIG_LINUX_IO_URING
> +    } else if (!strcmp(mode, "io_uring")) {
> +        *flags |= BDRV_O_IO_URING;
> +#endif
> +    } else {
> +        return -1;
> +    }
> +
> +    return 0;
> +}
Aarushi Mehta Aug. 7, 2019, 12:05 p.m. UTC | #2
On Wed, 7 Aug, 2019, 17:15 Julia Suvorova, <jusual@mail.ru> wrote:

> On Fri, Aug 2, 2019 at 1:41 AM Aarushi Mehta <mehta.aaru20@gmail.com>
> wrote:
> > +int bdrv_parse_aio(const char *mode, int *flags)
> > +{
> > +    if (!strcmp(mode, "threads")) {
> > +        /* do nothing, default */
> > +    } else if (!strcmp(mode, "native")) {
> > +        *flags |= BDRV_O_NATIVE_AIO;
>
> This 'if' should be covered with CONFIG_LINUX_AIO.
>

The aio=native definition is shared with Windows hosts' native aio and will
break if it was covered.

file-posix handles the case.

Best regards, Julia Suvorova.
>
> > +#ifdef CONFIG_LINUX_IO_URING
> > +    } else if (!strcmp(mode, "io_uring")) {
> > +        *flags |= BDRV_O_IO_URING;
> > +#endif
> > +    } else {
> > +        return -1;
> > +    }
> > +
> > +    return 0;
> > +}
>
McMillan, Erich" via Aug. 7, 2019, 12:49 p.m. UTC | #3
On Wed, Aug 7, 2019 at 2:06 PM Aarushi Mehta <mehta.aaru20@gmail.com> wrote:
>
>
>
> On Wed, 7 Aug, 2019, 17:15 Julia Suvorova, <jusual@mail.ru> wrote:
>>
>> On Fri, Aug 2, 2019 at 1:41 AM Aarushi Mehta <mehta.aaru20@gmail.com> wrote:
>> > +int bdrv_parse_aio(const char *mode, int *flags)
>> > +{
>> > +    if (!strcmp(mode, "threads")) {
>> > +        /* do nothing, default */
>> > +    } else if (!strcmp(mode, "native")) {
>> > +        *flags |= BDRV_O_NATIVE_AIO;
>>
>> This 'if' should be covered with CONFIG_LINUX_AIO.
>
>
> The aio=native definition is shared with Windows hosts' native aio and will break if it was covered.
>
> file-posix handles the case.

Fair enough. Then you can remove all ifdefs for io_uring from
raw_open_common in file-posix.c as this case was already checked here.

Best regards, Julia Suvorova.

>> > +#ifdef CONFIG_LINUX_IO_URING
>> > +    } else if (!strcmp(mode, "io_uring")) {
>> > +        *flags |= BDRV_O_IO_URING;
>> > +#endif
>> > +    } else {
>> > +        return -1;
>> > +    }
>> > +
>> > +    return 0;
>> > +}
Stefan Hajnoczi Oct. 4, 2019, 3:06 p.m. UTC | #4
On Wed, Aug 07, 2019 at 02:49:51PM +0200, Julia Suvorova via Qemu-devel wrote:
> On Wed, Aug 7, 2019 at 2:06 PM Aarushi Mehta <mehta.aaru20@gmail.com> wrote:
> >
> >
> >
> > On Wed, 7 Aug, 2019, 17:15 Julia Suvorova, <jusual@mail.ru> wrote:
> >>
> >> On Fri, Aug 2, 2019 at 1:41 AM Aarushi Mehta <mehta.aaru20@gmail.com> wrote:
> >> > +int bdrv_parse_aio(const char *mode, int *flags)
> >> > +{
> >> > +    if (!strcmp(mode, "threads")) {
> >> > +        /* do nothing, default */
> >> > +    } else if (!strcmp(mode, "native")) {
> >> > +        *flags |= BDRV_O_NATIVE_AIO;
> >>
> >> This 'if' should be covered with CONFIG_LINUX_AIO.
> >
> >
> > The aio=native definition is shared with Windows hosts' native aio and will break if it was covered.
> >
> > file-posix handles the case.
> 
> Fair enough. Then you can remove all ifdefs for io_uring from
> raw_open_common in file-posix.c as this case was already checked here.

This is not possible since the BLOCKDEV_AIO_OPTIONS_IO_URING enum
constant is conditional in the QAPI schema:

  { 'enum': 'BlockdevAioOptions',
    'data': [ 'threads', 'native',
              { 'name': 'io_uring', 'if': 'defined(CONFIG_LINUX_IO_URING)' } ] }

The code can only use BLOCKDEV_AIO_OPTIONS_IO_URING if
CONFIG_LINUX_IO_URING was defined, so we cannot drop the #ifdefs in
raw_open_common().

Stefan
diff mbox series

Patch

diff --git a/block.c b/block.c
index cbd8da5f3b..401831e28d 100644
--- a/block.c
+++ b/block.c
@@ -844,6 +844,28 @@  static BlockdevDetectZeroesOptions bdrv_parse_detect_zeroes(QemuOpts *opts,
     return detect_zeroes;
 }
 
+/**
+ * Set open flags for aio engine
+ *
+ * Return 0 on success, -1 if the engine specified is invalid
+ */
+int bdrv_parse_aio(const char *mode, int *flags)
+{
+    if (!strcmp(mode, "threads")) {
+        /* do nothing, default */
+    } else if (!strcmp(mode, "native")) {
+        *flags |= BDRV_O_NATIVE_AIO;
+#ifdef CONFIG_LINUX_IO_URING
+    } else if (!strcmp(mode, "io_uring")) {
+        *flags |= BDRV_O_IO_URING;
+#endif
+    } else {
+        return -1;
+    }
+
+    return 0;
+}
+
 /**
  * Set open flags for a given discard mode
  *
diff --git a/blockdev.c b/blockdev.c
index 4d141e9a1f..a41623ae9a 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -383,13 +383,9 @@  static void extract_common_blockdev_options(QemuOpts *opts, int *bdrv_flags,
         }
 
         if ((aio = qemu_opt_get(opts, "aio")) != NULL) {
-            if (!strcmp(aio, "native")) {
-                *bdrv_flags |= BDRV_O_NATIVE_AIO;
-            } else if (!strcmp(aio, "threads")) {
-                /* this is the default */
-            } else {
-               error_setg(errp, "invalid aio option");
-               return;
+            if (bdrv_parse_aio(aio, bdrv_flags) < 0) {
+                error_setg(errp, "invalid aio option");
+                return;
             }
         }
     }
@@ -4574,7 +4570,7 @@  QemuOptsList qemu_common_drive_opts = {
         },{
             .name = "aio",
             .type = QEMU_OPT_STRING,
-            .help = "host AIO implementation (threads, native)",
+            .help = "host AIO implementation (threads, native, io_uring)",
         },{
             .name = BDRV_OPT_CACHE_WB,
             .type = QEMU_OPT_BOOL,
diff --git a/include/block/block.h b/include/block/block.h
index e29baa172c..ec6b9ea4c8 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -297,6 +297,7 @@  void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top,
 void bdrv_replace_node(BlockDriverState *from, BlockDriverState *to,
                        Error **errp);
 
+int bdrv_parse_aio(const char *mode, int *flags);
 int bdrv_parse_cache_mode(const char *mode, int *flags, bool *writethrough);
 int bdrv_parse_discard_flags(const char *mode, int *flags);
 BdrvChild *bdrv_open_child(const char *filename,