Re: [Qemu-devel] [PATCH v2 04/10] nbd-server: refactor simple reply sending

[Paolo Bonzini <pbonzini@redhat.com>]

On 10/10/2017 10:40, Daniel P. Berrange wrote:
> The I/O channels code does not make guarantees wrt concurrent usage of
> threads or coroutines. It is the callers responsibility to avoid any
> concurrent usage for all APIs. With coroutines you are at least avoiding
> the danger of corrupting memory state, but you still have risk of unexpected
> data ordering.
> 
> IOW, if you have 2 coroutines each doing a series writes on the same QIOChannel
> object, and one does a yield, I/O can certainly be interleaved between the two.
> This is true whether doing multiple qio_channel_writev() calls directly, or
> whether using qio_channel_writev_all().
> 
> Unless I'm misunderstanding though, cork did not offer you protection in
> this scenario either. cork just prevents the data being transmitted onto
> the wire immediately - ie it ensures that if you do 2 writes, they get
> sent in 1 TCP packet instead of many TCP packets.
> 
> If you have multiple coroutines writing to the channel at the same time
> and one yielded in its series of writes, the I/O from multiple coroutines
> will get merged into that 1 single TCP packet when uncorked.
> 
> So if this concurrent usage is a problem NBD was already broken AFAICT.

I don't think there was an issue here.  nbd_co_send_reply is entirely
protected by client->send_lock.

Paolo