On 24/08/2017 17:33, Stefan Hajnoczi wrote:
> This patch enters read_reply_co directly in
> nbd_client_attach_aio_context(). This is safe because new_context is
> acquired by the caller. This ensures that read_reply_co reaches its
> first yield point and its ctx is set up.
I'm not very confident with this patch. aio_context_acquire/release is
going to go away, and this then becomes possible
main context new_context
qemu_aio_coroutine_enter
send request
wait for reply
read first reply
wake coroutine
where the "wake coroutine" part thinks it's running in new_context, and
thus simply enters the coroutine instead of using the bottom half.
But blk_co_preadv() should need the read_reply_co itself, in order to be
woken up after reading the reply header. The core issue here is that
nbd_co_receive_reply was never called, I suspect. And if it was never
called, read_reply_co should not be woken up by nbd_coroutine_end.
So the fix is:
1) assign NULL to s->recv_coroutine[i] when nbd_co_send_request fails
2) move this to nbd_co_receive_reply:
s->recv_coroutine[i] = NULL;
/* Kick the read_reply_co to get the next reply. */
if (s->read_reply_co) {
aio_co_wake(s->read_reply_co);
}
Does this make sense? (Note that the read_reply_co idea actually came
from you, or from my recollections of your proposed design :)).
Paolo
> Note this only happens with UNIX domain sockets on Linux. It doesn't
> seem possible to reproduce this with TCP sockets.
>
> Cc: Paolo Bonzini <pbonz...@redhat.com>
> Signed-off-by: Stefan Hajnoczi <stefa...@redhat.com>
> ---
> block/nbd-client.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/block/nbd-client.c b/block/nbd-client.c
> index 25bcaa2346..0a7f32779e 100644
> --- a/block/nbd-client.c
> +++ b/block/nbd-client.c
> @@ -371,7 +371,7 @@ void nbd_client_attach_aio_context(BlockDriverState *bs,
> {
> NBDClientSession *client = nbd_get_client_session(bs);
> qio_channel_attach_aio_context(QIO_CHANNEL(client->ioc), new_context);
> - aio_co_schedule(new_context, client->read_reply_co);
> + qemu_aio_coroutine_enter(new_context, client->read_reply_co);
> }
