On 18/01/2017 15:58, Stefan Hajnoczi wrote: > On Fri, Jan 13, 2017 at 02:17:20PM +0100, Paolo Bonzini wrote: >> /** >> + * qio_channel_set_aio_context: >> + * @ioc: the channel object >> + * @ctx: the #AioContext to set the handlers on >> + * >> + * Request that qio_channel_yield() sets I/O handlers on >> + * the given #AioContext. If @ctx is %NULL, qio_channel_yield() >> + * uses QEMU's main thread event loop. >> + */ >> +void qio_channel_set_aio_context(QIOChannel *ioc, >> + AioContext *ctx); >> + >> +/** >> + * qio_channel_detach_aio_context: >> + * @ioc: the channel object >> + * >> + * Disable any I/O handlers set by qio_channel_yield(). With the >> + * help of aio_co_schedule(), this allows moving a coroutine that was >> + * paused by qio_channel_yield() to another context. >> + */ >> +void qio_channel_detach_aio_context(QIOChannel *ioc); > > The block layer's bdrv_set_aio_context() has different semantics. It > invokes .detach()/.attach() callbacks and does AioContext locking so the > function can be called safely even while the block driver is waiting for > events. > > It's unfortunate to that the block and io channel APIs act differently > despite having similar names. Was there a reason to choose different > semantics?
Hmm, it's true. I had forgotten that bdrv_set_aio_context exists. set_aio_context can be called from the block layer attach callback, but it's not enough alone (you need aio_co_schedule too) so I didn't want to call the function qio_channel_attach_aio_context. But maybe it *is* a better name, I'll go for it in v2. By the way, v2 will have a better comment on how to use the API: + * You can move a #QIOChannel from an #AioContext to another even if + * I/O handlers are set for a coroutine. However, #QIOChannel provides + * no synchronization between the calls to qio_channel_yield() and + * qio_channel_set_aio_context(). + * + * Therefore you should first call qio_channel_detach_aio_context() + * to ensure that the coroutine is not entered concurrently. Then, + * while the coroutine has yielded, call qio_channel_set_aio_context(), + * and then aio_co_schedule() to place the coroutine on the new + * #AioContext. The calls to qio_channel_detach_aio_context() + * and qio_channel_set_aio_context() should be protected with + * aio_context_acquire() and aio_context_release(). The "while the coroutine has yielded" part is currently handled with aio_context_acquire/aio_context_release (the coroutine cannot run at all between aio_context_acquire and release). When they will be gone, some kind of BDRV_POLL_WHILE at the end of bdrv_detach_aio_context should be enough to ensure that the event loop is quiescent. Paolo >> + >> +/** >> * qio_channel_yield: >> * @ioc: the channel object >> * @condition: the I/O condition to wait for >> * >> - * Yields execution from the current coroutine until >> - * the condition indicated by @condition becomes >> - * available. >> + * Yields execution from the current coroutine until the condition >> + * indicated by @condition becomes available. @condition must >> + * be either %G_IO_IN or %G_IO_OUT; it cannot contain both. In >> + * addition, no two coroutine can be waiting on the same condition > > s/coroutine/coroutines/ >
signature.asc
Description: OpenPGP digital signature
