[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] nbd: strict nbd_wr_syncv
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [PATCH] nbd: strict nbd_wr_syncv |
Date: |
Fri, 12 May 2017 18:30:47 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.0 |
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
- [Qemu-devel] [PATCH] nbd: strict nbd_wr_syncv, Vladimir Sementsov-Ogievskiy, 2017/05/12
- Re: [Qemu-devel] [PATCH] nbd: strict nbd_wr_syncv, Paolo Bonzini, 2017/05/12
- Re: [Qemu-devel] [PATCH] nbd: strict nbd_wr_syncv, Vladimir Sementsov-Ogievskiy, 2017/05/12
- Re: [Qemu-devel] [PATCH] nbd: strict nbd_wr_syncv,
Paolo Bonzini <=
- Re: [Qemu-devel] [PATCH] nbd: strict nbd_wr_syncv, Vladimir Sementsov-Ogievskiy, 2017/05/15
- Re: [Qemu-devel] [PATCH] nbd: strict nbd_wr_syncv, Vladimir Sementsov-Ogievskiy, 2017/05/16
- Re: [Qemu-devel] [PATCH] nbd: strict nbd_wr_syncv, Vladimir Sementsov-Ogievskiy, 2017/05/16
- Re: [Qemu-devel] [PATCH] nbd: strict nbd_wr_syncv, Paolo Bonzini, 2017/05/16
- Re: [Qemu-devel] [PATCH] nbd: strict nbd_wr_syncv, Vladimir Sementsov-Ogievskiy, 2017/05/16
- Re: [Qemu-devel] [PATCH] nbd: strict nbd_wr_syncv, Paolo Bonzini, 2017/05/16