Re: [PATCH v2 05/15] stubs: add stubs for io_uring interface

From: Kevin Wolf <kwolf@redhat.com>
To: Stefan Hajnoczi <stefanha@redhat.com>
Cc: Fam Zheng <fam@euphon.net>,
	oleksandr@redhat.com, qemu-block@nongnu.org,
	Markus Armbruster <armbru@redhat.com>,
	Julia Suvorova <jusual@redhat.com>,
	qemu-devel@nongnu.org, Max Reitz <mreitz@redhat.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Aarushi Mehta <mehta.aaru20@gmail.com>
Subject: Re: [PATCH v2 05/15] stubs: add stubs for io_uring interface
Date: Tue, 3 Dec 2019 15:28:45 +0100	[thread overview]
Message-ID: <20191203142845.GE5637@linux.fritz.box> (raw)
In-Reply-To: <20191203111600.GA167235@stefanha-x1.localdomain>

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

Am 03.12.2019 um 12:16 hat Stefan Hajnoczi geschrieben:
> On Mon, Nov 11, 2019 at 12:13:47PM +0100, 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>
> > > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> > 
> > This commit message needs to answer at least where these stubs are
> > actually used. Aren't all callers protected with #ifdef
> > CONFIG_LINUX_IO_URING? (And if they aren't, why is abort() okay?)
> 
> Okay, I'll clarify in the commit description.
> 
> I didn't find a program that actually requires these stubs today, but in
> theory they are required when:
> 1. A program links against util-obj-y but not block-obj-y (e.g.
>    vhost-user-gpu)
> AND
> 2. It uses util/async.o, which depends on luring_*() APIs
> 
> You can test this by adding a call to qemu_bh_new() into
> vhost-user-gpu's main.c:
> 
>   ld: libqemuutil.a(async.o): in function `aio_ctx_finalize':
>   qemu/util/async.c:281: undefined reference to `luring_detach_aio_context'
>   ld: /home/stefanha/qemu/util/async.c:282: undefined reference to `luring_cleanup'
>   ld: libqemuutil.a(async.o): in function `aio_setup_linux_io_uring':
>   qemu/util/async.c:358: undefined reference to `luring_init'
>   ld: /home/stefanha/qemu/util/async.c:363: undefined reference to `luring_attach_aio_context'
> 
> The program may have no intention of using io_uring, just the QEMU main
> loop and BH, so there is an argument for avoiding linking block-obj-y*.
> That's the purpose of the stubs and why abort() is okay.

Okay, make sense to me then.

> * It's possible to argue against this and personally I'm not that
> convinced by stubs for this scenario.  But io_uring.o should be
> consistent with how things work today with linux-aio.o.  If you feel
> strongly against having stubs then the linux-aio.o stubs should also be
> removed (see commit c2b38b277a788).

I don't really like having block-specific things like Linux AIO or
io_uring in util/async.c, but given that they have per-AioContext state,
it's not clearly wrong either.

Maybe we could consider moving these functions to the file-posix driver,
it's the only caller of them. But it's nothing that should stop this
series.

Kevin

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