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

[Kevin Wolf]

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 <address@hidden>
> > > 
> > > Signed-off-by: Aarushi Mehta <address@hidden>
> > > Signed-off-by: Stefan Hajnoczi <address@hidden>
> > 
> > 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

signature.asc
Description: PGP signature