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

[Paolo Bonzini]

On 16/05/2017 11:32, Vladimir Sementsov-Ogievskiy wrote:
> 16.05.2017 12:10, Vladimir Sementsov-Ogievskiy wrote:
>> 15.05.2017 12:43, Vladimir Sementsov-Ogievskiy wrote:
>>> I mean, make negotiation behave like normal nbd communication,
>>> non-blocking socket + yield.. So, some other coroutines may do their
>>> work, while nbd-negotiation coroutine waits for io..

Some callers of bdrv_open may not allow reentrancy.  For example:

        handle_qmp_command
        -> qmp_dispatch
        -> do_qmp_dispatch
        -> qmp_marshal_blockdev_add
        -> qmp_blockdev_add
        -> bds_tree_init
        -> bdrv_open

You cannot return to the monitor before qmp_blockdev_add is done,
otherwise you don't have a return value for handle_qmp_command to pass
to monitor_json_emitter.

>> Also, one more question here: in nbd_negotiate_write(), why do we need
>> qio_channel_add_watch? write_sync will yield with qio_channel_yield()
>> until io complete, why to use 2 mechanisms to wake up a coroutine?
> 
> Hmm, these nbd_negotiate_* functions was introduced in 1a6245a5b, when
> nbd_wr_syncv was working through qemu_co_sendv_recvv, which just yields,
> without setting any handlers. But now, nbd_wr_syncv works through
> qio_channel_yield() which sets handlers, so the code with extra watchers
> looks wrong.

Yes, I think you're right about that.

Paolo