diff mbox series

[v5,02/12] qapi/block-core: add option for io_uring

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

Commit Message

Aarushi Mehta June 10, 2019, 1:48 p.m. UTC
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(-)

Comments

Fam Zheng June 11, 2019, 7:36 a.m. UTC | #1
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
>
Stefan Hajnoczi June 11, 2019, 9:32 a.m. UTC | #2
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
Markus Armbruster July 3, 2019, 6:26 a.m. UTC | #3
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.
diff mbox series

Patch

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: