Re: [Qemu-devel] [PATCH] nbd: strict nbd_wr_syncv

[Paolo Bonzini]

On 12/05/2017 17:57, Vladimir Sementsov-Ogievskiy wrote:
> 12.05.2017 18:46, Paolo Bonzini wrote:
>>
>> On 12/05/2017 16:17, Vladimir Sementsov-Ogievskiy wrote:
>>> nbd_wr_syncv is called either from coroutine or from client negotiation
>>> code, when socket is in blocking mode. So, -EAGAIN is impossible.
>>>
>>> Furthermore, EAGAIN is confusing, as, what to read/write again? With
>>> EAGAIN as a return code we don't know how much data is already
>>> read or written by the function, so in case of EAGAIN the whole
>>> communication is broken.
>>>
>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
>>> ---
>>>
>>> Hi all!
>>>
>>> If I understand right, nbd_wr_syncv is called either from coroutine
>>> or from client negotiation code, when socket is in blocking mode.
>>> So, some thoughts
>>>
>>> 1. let's establish this with an assert, because returning EAGAIN is
>>>     confusing (see above)
>> Yes, this seems like a good idea.
>>
>>> 2. should we in case of non-coroutine context start this coroutine in
>>>     nbd_wr_syncv, like in bdrv_prwv_co, and use non-blocking mode?
>>> 3. is there any problems or disadvantages of moving  client negotiation
>>>     to coroutine too?
>> When you move code to coroutines you need to be aware of what code can
>> now run concurrently, for example the monitor.  I'm not sure that it's
>> possible to do this.
> 
> Hmm, can you please give some example of a problem? qcow2_open starts
> coroutines to read it's header, why nbd_open can't start
> coroutine/coroutines to read/write some negotiation data?

Ah, it's not a problem if you use synchronous I/O (aio_poll) within the
coroutines.  But then it's still blocking I/O in every way (except
you've fcntl-ed the descriptor to make it non-blocking); it's simply
hidden underneath coroutines and aio_poll.

Paolo