[v5] Add support for io_uring

[Qemu-devel] [PATCH v5 02/12] qapi/block-core: add option for io_uring

Posted by Aarushi Mehta 6 years, 8 months ago

Option only enumerates for hosts that support it.

Signed-off-by: Aarushi Mehta <mehta.aaru20@gmail.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 qapi/block-core.json | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 1defcde048..db7eedd058 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2792,11 +2792,13 @@
 #
 # @threads:     Use qemu's thread pool
 # @native:      Use native AIO backend (only Linux and Windows)
+# @io_uring:    Use linux io_uring (since 4.1)
 #
 # Since: 2.9
 ##
 { 'enum': 'BlockdevAioOptions',
-  'data': [ 'threads', 'native' ] }
+  'data': [ 'threads', 'native',
+            { 'name': 'io_uring', 'if': 'defined(CONFIG_LINUX_IO_URING)' } ] }
 
 ##
 # @BlockdevCacheOptions:
-- 
2.17.1

Re: [Qemu-devel] [PATCH v5 02/12] qapi/block-core: add option for io_uring

Posted by Fam Zheng 6 years, 8 months ago

On Mon, 06/10 19:18, Aarushi Mehta wrote:
> Option only enumerates for hosts that support it.
> 
> Signed-off-by: Aarushi Mehta <mehta.aaru20@gmail.com>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  qapi/block-core.json | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 1defcde048..db7eedd058 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -2792,11 +2792,13 @@
>  #
>  # @threads:     Use qemu's thread pool
>  # @native:      Use native AIO backend (only Linux and Windows)
> +# @io_uring:    Use linux io_uring (since 4.1)
>  #
>  # Since: 2.9
>  ##
>  { 'enum': 'BlockdevAioOptions',
> -  'data': [ 'threads', 'native' ] }
> +  'data': [ 'threads', 'native',
> +            { 'name': 'io_uring', 'if': 'defined(CONFIG_LINUX_IO_URING)' } ] }

Question: 'native' has a dependency on libaio but it doesn't have the
condition.  Is the inconsistency intended?

>  
>  ##
>  # @BlockdevCacheOptions:
> -- 
> 2.17.1
>

Re: [Qemu-devel] [PATCH v5 02/12] qapi/block-core: add option for io_uring

Posted by Stefan Hajnoczi 6 years, 8 months ago

On Tue, Jun 11, 2019 at 03:36:53PM +0800, Fam Zheng wrote:
> On Mon, 06/10 19:18, Aarushi Mehta wrote:
> > Option only enumerates for hosts that support it.
> > 
> > Signed-off-by: Aarushi Mehta <mehta.aaru20@gmail.com>
> > Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> > ---
> >  qapi/block-core.json | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/qapi/block-core.json b/qapi/block-core.json
> > index 1defcde048..db7eedd058 100644
> > --- a/qapi/block-core.json
> > +++ b/qapi/block-core.json
> > @@ -2792,11 +2792,13 @@
> >  #
> >  # @threads:     Use qemu's thread pool
> >  # @native:      Use native AIO backend (only Linux and Windows)
> > +# @io_uring:    Use linux io_uring (since 4.1)
> >  #
> >  # Since: 2.9
> >  ##
> >  { 'enum': 'BlockdevAioOptions',
> > -  'data': [ 'threads', 'native' ] }
> > +  'data': [ 'threads', 'native',
> > +            { 'name': 'io_uring', 'if': 'defined(CONFIG_LINUX_IO_URING)' } ] }
> 
> Question: 'native' has a dependency on libaio but it doesn't have the
> condition.  Is the inconsistency intended?

'native' could be conditional too but I guess it's a historical thing.
Either QAPI 'if' didn't exit when BlockdevAioOptions was defined or we
simply forgot to use it :).

It doesn't need to be changed in this patch series.

Stefan

Re: [Qemu-devel] [PATCH v5 02/12] qapi/block-core: add option for io_uring

Posted by Markus Armbruster 6 years, 7 months ago

I apologize for replying so late.

Stefan Hajnoczi <stefanha@redhat.com> writes:

> On Tue, Jun 11, 2019 at 03:36:53PM +0800, Fam Zheng wrote:
>> On Mon, 06/10 19:18, Aarushi Mehta wrote:
>> > Option only enumerates for hosts that support it.
>> > 
>> > Signed-off-by: Aarushi Mehta <mehta.aaru20@gmail.com>
>> > Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
>> > ---
>> >  qapi/block-core.json | 4 +++-
>> >  1 file changed, 3 insertions(+), 1 deletion(-)
>> > 
>> > diff --git a/qapi/block-core.json b/qapi/block-core.json
>> > index 1defcde048..db7eedd058 100644
>> > --- a/qapi/block-core.json
>> > +++ b/qapi/block-core.json
>> > @@ -2792,11 +2792,13 @@
>> >  #
>> >  # @threads:     Use qemu's thread pool
>> >  # @native:      Use native AIO backend (only Linux and Windows)
>> > +# @io_uring:    Use linux io_uring (since 4.1)
>> >  #
>> >  # Since: 2.9
>> >  ##
>> >  { 'enum': 'BlockdevAioOptions',
>> > -  'data': [ 'threads', 'native' ] }
>> > +  'data': [ 'threads', 'native',
>> > +            { 'name': 'io_uring', 'if': 'defined(CONFIG_LINUX_IO_URING)' } ] }
>> 
>> Question: 'native' has a dependency on libaio but it doesn't have the
>> condition.  Is the inconsistency intended?
>
> 'native' could be conditional too but I guess it's a historical thing.
> Either QAPI 'if' didn't exit when BlockdevAioOptions was defined or we
> simply forgot to use it :).

The former.  QAPI supports 'if' in partially since 3.0, and fully since
4.0.

> It doesn't need to be changed in this patch series.

Making the QAPI schema reflect the underlying C code's ifdeffery can
help QMP clients.  Compare:

* query-qmp-schema reports member "native" even when attempting to use
  it will always fail.

* It reports member "io_uring" only when it can work.

This is particularly useful when a QMP client has logic like "do it this
way if X is available, else do it that way".  With accurate
introspection, you can check whether X is available safely and easily.
Without introspection, you'll likely have to try "this way", and if it
fails, figure out why, so you can decide whether to try "that way".
Worse when a multi-step "this way" fails half way through, and you get
to revert its initial steps.