12.02.2019 1:02, Eric Blake wrote: > On 2/11/19 6:56 AM, Vladimir Sementsov-Ogievskiy wrote: >> Now negotiation is done in coroutine, so to take benefit of it let's >> use non-blocking model. >> >> Note that QIOChannel handle synchronous io calls correctly anyway, so > > s/handle/handles/ > >> it's not a problem to send final NBD_CMD_DISC to non-blocking channel >> but using sync qio interface, even not in coroutine. > > s/not in/when not in a/ > >> >> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> >> --- >> block/nbd-client.c | 10 +++------- >> 1 file changed, 3 insertions(+), 7 deletions(-) > > This fixes qemu as NBD client for use in block devices, but what about > qemu as NBD client in qemu-nbd? Does that need any updates? Then > again, your observation about QIO doing the right thing for both > blocking (qemu-nbd) and non-blocking (block layer) channels seems to > cover that.
In qemu-nbd client works in a separate thread, so, I think, we don't need nonblocking: we have a thread anyway. On the other hand, is my code in nbd_reaceive_common safe, keeping in mind that we call it from separate thread? Are qio_channel_attach_aio_context, qemu_aio_coroutine_enter, AIO_WAIT_WHILE thread-safe? > >> @@ -1072,9 +1072,6 @@ static int nbd_client_connect(BlockDriverState *bs, >> object_ref(OBJECT(client->ioc)); >> } >> >> - /* Now that we're connected, set the socket to be non-blocking and >> - * kick the reply mechanism. */ >> - qio_channel_set_blocking(QIO_CHANNEL(sioc), false, NULL); >> client->connection_co = qemu_coroutine_create(nbd_connection_entry, >> client); >> nbd_client_attach_aio_context(bs, bdrv_get_aio_context(bs)); >> >> @@ -1083,9 +1080,8 @@ static int nbd_client_connect(BlockDriverState *bs, >> >> fail: >> /* >> - * We have connected, but must fail for other reasons. The >> - * connection is still blocking; send NBD_CMD_DISC as a courtesy >> - * to the server. >> + * We have connected, but must fail for other reasons. >> + * Send NBD_CMD_DISC as a courtesy to the server. >> */ >> { >> NBDRequest request = { .type = NBD_CMD_DISC }; >> > -- Best regards, Vladimir