| Message ID | 20190801234031.29561-8-mehta.aaru20@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add support for io_uring | expand |
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; > +}
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; > > +} >
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; >> > +}
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 --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,