Re: [PATCH v2 03/15] block/block: add BDRV flag for io_uring

From: Stefan Hajnoczi <stefanha@redhat.com>
To: Max Reitz <mreitz@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>, Fam Zheng <fam@euphon.net>,
	Maxim Levitsky <maximlevitsky@gmail.com>,
	qemu-block@nongnu.org, oleksandr@redhat.com,
	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: Wed, 13 Nov 2019 11:42:08 +0000	[thread overview]
Message-ID: <20191113114208.GB538417@stefanha-x1.localdomain> (raw)
In-Reply-To: <6e8cf473-d3e9-c743-d2bd-75a0d955e6c6@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 2410 bytes --]

On Mon, Nov 11, 2019 at 05:25:04PM +0100, Max Reitz wrote:
> 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.)

Thanks for the idea.  I agree it's cleaner to avoid a new global flag.

I'll take a look at fixing this and let you know if I hit any problems.

Stefan

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