qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Max Reitz <mreitz@redhat.com>
To: Kevin Wolf <kwolf@redhat.com>, Stefan Hajnoczi <stefanha@redhat.com>
Cc: Fam Zheng <fam@euphon.net>,
	oleksandr@redhat.com, Maxim Levitsky <maximlevitsky@gmail.com>,
	qemu-block@nongnu.org, Julia Suvorova <jusual@redhat.com>,
	qemu-devel@nongnu.org, Markus Armbruster <armbru@redhat.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Aarushi Mehta <mehta.aaru20@gmail.com>
Subject: Re: [PATCH v2 03/15] block/block: add BDRV flag for io_uring
Date: Mon, 11 Nov 2019 17:25:04 +0100	[thread overview]
Message-ID: <6e8cf473-d3e9-c743-d2bd-75a0d955e6c6@redhat.com> (raw)
In-Reply-To: <20191111105759.GB7297@linux.fritz.box>


[-- Attachment #1.1: Type: text/plain, Size: 2112 bytes --]

On 11.11.19 11:57, Kevin Wolf wrote:
> Am 25.10.2019 um 18:04 hat Stefan Hajnoczi geschrieben:
>> From: Aarushi Mehta <mehta.aaru20@gmail.com>
>>
>> Signed-off-by: Aarushi Mehta <mehta.aaru20@gmail.com>
>> Reviewed-by: Maxim Levitsky <maximlevitsky@gmail.com>
>> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
>> ---
>>  include/block/block.h | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/include/block/block.h b/include/block/block.h
>> index 89606bd9f8..bdb48dcd1b 100644
>> --- a/include/block/block.h
>> +++ b/include/block/block.h
>> @@ -126,6 +126,7 @@ typedef struct HDGeometry {
>>                                        ignoring the format layer */
>>  #define BDRV_O_NO_IO       0x10000 /* don't initialize for I/O */
>>  #define BDRV_O_AUTO_RDONLY 0x20000 /* degrade to read-only if opening read-write fails */
>> +#define BDRV_O_IO_URING    0x40000 /* use io_uring instead of the thread pool */
> 
> Why do we need a new (global!) flag rather than a driver-specific option
> for file-posix? This looks wrong to me, the flag has no sensible meaning
> for any other driver.
> 
> (Yes, BDRV_O_NATIVE_AIO is wrong, too, but compatibility means we can't
> remove it easily.)

To add to that, Kevin and me had a short talk on IRC, and it seemed like
the reason would be that it’s easier to use this way than an option
would be.

To me, the problem seems to be that it’s only easier to use because of
the option inheritance we have in the block layer (so you can set this
flag on a qcow2 node and its file node will have it, too).  But
naturally this inheritance is a bit of magic (because qemu has to guess
where you probably want the flag to end up), so I’d too rather avoid
adding to it.

OTOH, I wonder whether ease of use is really that important here.  Would
people who want to use io_uring really care about a command line that’s
going to be a bit more complicated than just
-drive file=foo.img,format=$imgfmt,aio=io_uring,if=$interface?  (Namely
file.aio in this case, or maybe even a full-on block graph definition.)

Max


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

  reply	other threads:[~2019-11-11 16:36 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-25 16:04 [PATCH v2 00/15] io_uring: add Linux io_uring AIO engine Stefan Hajnoczi
2019-10-25 16:04 ` [PATCH v2 01/15] configure: permit use of io_uring Stefan Hajnoczi
2019-10-25 16:04 ` [PATCH v2 02/15] qapi/block-core: add option for io_uring Stefan Hajnoczi
2019-10-25 19:09   ` Markus Armbruster
2019-10-25 16:04 ` [PATCH v2 03/15] block/block: add BDRV flag " Stefan Hajnoczi
2019-11-11 10:57   ` Kevin Wolf
2019-11-11 16:25     ` Max Reitz [this message]
2019-11-13 11:42       ` Stefan Hajnoczi
2019-10-25 16:04 ` [PATCH v2 04/15] block/io_uring: implements interfaces " Stefan Hajnoczi
2019-10-25 16:04 ` [PATCH v2 05/15] stubs: add stubs for io_uring interface Stefan Hajnoczi
2019-11-11 11:13   ` Kevin Wolf
2019-11-13 11:42     ` Stefan Hajnoczi
2019-12-03 11:16     ` Stefan Hajnoczi
2019-12-03 14:28       ` Kevin Wolf
2019-12-03 15:55         ` Paolo Bonzini
2019-10-25 16:04 ` [PATCH v2 06/15] util/async: add aio interfaces for io_uring Stefan Hajnoczi
2019-10-25 16:04 ` [PATCH v2 07/15] blockdev: adds bdrv_parse_aio to use io_uring Stefan Hajnoczi
2019-10-25 16:04 ` [PATCH v2 08/15] block/file-posix.c: extend " Stefan Hajnoczi
2019-10-25 16:04 ` [PATCH v2 09/15] block: add trace events for io_uring Stefan Hajnoczi
2019-10-25 16:04 ` [PATCH v2 10/15] block/io_uring: adds userspace completion polling Stefan Hajnoczi
2019-10-25 16:04 ` [PATCH v2 11/15] qemu-io: adds option to use aio engine Stefan Hajnoczi
2019-10-25 16:04 ` [PATCH v2 12/15] qemu-img: adds option to use aio engine for benchmarking Stefan Hajnoczi
2019-10-25 16:04 ` [PATCH v2 13/15] qemu-nbd: adds option for aio engines Stefan Hajnoczi
2019-10-25 16:04 ` [PATCH v2 14/15] tests/qemu-iotests: enable testing with aio options Stefan Hajnoczi
2019-10-25 16:04 ` [PATCH v2 15/15] tests/qemu-iotests: use AIOMODE with various tests Stefan Hajnoczi
2019-11-04 10:32 ` [PATCH v2 00/15] io_uring: add Linux io_uring AIO engine Stefan Hajnoczi
2019-11-13 11:43   ` Stefan Hajnoczi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=6e8cf473-d3e9-c743-d2bd-75a0d955e6c6@redhat.com \
    --to=mreitz@redhat.com \
    --cc=armbru@redhat.com \
    --cc=fam@euphon.net \
    --cc=jusual@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=maximlevitsky@gmail.com \
    --cc=mehta.aaru20@gmail.com \
    --cc=oleksandr@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).