12.02.2019 13:33, Daniel P. Berrangé wrote: > On Mon, Feb 11, 2019 at 03:55:58PM +0300, Vladimir Sementsov-Ogievskiy wrote: >> Expose attached aio context. It will be used in nbd code, to >> understand, in which aio context negotiation should be done. > > I'm not especially objecting to the idea of adding the API to the > QIOChannel class, but I'm surprised that NBD needs this. Surely it > already knows what AIO context it assigned to the channel in the > first place.
Actually not, nbd/client.c don't know it, it gets @ioc as a paramter of nbd_receive_negotiate and nbd_receive_export_list, which may be called from block/nbd-client.c and qemu-nbd.c.. for example, nbd-client.c do nbd_client_attach_aio_context(bs, bdrv_get_aio_context(bs)); where nbd_client_attach_aio_context is void nbd_client_attach_aio_context(BlockDriverState *bs, AioContext *new_context) { NBDClientSession *client = nbd_get_client_session(bs); qio_channel_attach_aio_context(QIO_CHANNEL(client->ioc), new_context); aio_co_schedule(new_context, client->connection_co); } It does it after nbd_receive_negotiate(), but I think, it should be split in these series in nbd_client_connect(), so that we attach aio context before negotiation and schedule connection_co after it. and NBDClientSession is block/* related object. So, nbd/client.c sees only resulting ioc. And it seems better to get aio context from ioc, than pass additional parameter to nbd_receive_negotiate() together with ioc. IOW, it feels like this is papering over a limitation > in NBD not keeping track of what AIO context it should be using. > >> >> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> >> --- >> include/io/channel.h | 9 +++++++++ >> io/channel.c | 5 +++++ >> 2 files changed, 14 insertions(+) >> >> diff --git a/include/io/channel.h b/include/io/channel.h >> index da2f138200..1a1e4a01b0 100644 >> --- a/include/io/channel.h >> +++ b/include/io/channel.h >> @@ -718,6 +718,15 @@ GSource *qio_channel_add_watch_source(QIOChannel *ioc, >> void qio_channel_attach_aio_context(QIOChannel *ioc, >> AioContext *ctx); >> >> +/* >> + * qio_channel_get_aio_context >> + * @ioc: the channel object >> + * >> + * Returns channel AioContext if any attached by >> + * qio_channel_attach_aio_context(), otherwise NULL. >> + */ >> +AioContext *qio_channel_get_attached_aio_context(QIOChannel *ioc); >> + >> /** >> * qio_channel_detach_aio_context: >> * @ioc: the channel object >> diff --git a/io/channel.c b/io/channel.c >> index 8dd0684f5d..a1b937bb6b 100644 >> --- a/io/channel.c >> +++ b/io/channel.c >> @@ -454,6 +454,11 @@ void qio_channel_detach_aio_context(QIOChannel *ioc) >> ioc->ctx = NULL; >> } >> >> +AioContext *qio_channel_get_attached_aio_context(QIOChannel *ioc) >> +{ >> + return ioc->ctx; >> +} >> + >> void coroutine_fn qio_channel_yield(QIOChannel *ioc, >> GIOCondition condition) >> { >> -- >> 2.18.0 >> > > Regards, > Daniel > -- Best regards, Vladimir