Re: [Qemu-devel] [PATCH v2 1/3] block: block: introduce bdrv_io_plug() and bdrv_io_unplug()

[Paolo Bonzini <pbonzini@redhat.com>]

Il 02/07/2014 10:39, Ming Lei ha scritto:
> Then start to read payload in original path, but no plug/unplug any
> more. Also another request may follows, and another plug&unplug
> comes too, which makes thing more complicated, so I suggest to
> enable plug&unplug only for raw driver now.

That's just a performance issue (and actually one that wasn't in 2.0 
because qcow2 on dataplane wasn't supported there).  In many cases the 
cache hit of the qcow2 metadata cache can be very high, and avoiding 
plug/unplug would prevent an easy performance bonus.

I don't especially like plug/unplug as an API (I think it's better to 
extend aio_multiwrite to include other kind of requests), but:

- either we have qualms on the correctness of it, and then we should 
live with the regressions

- or if the patches are not messy and reverting them is easy, we should 
go for it.  This is what we did for dataplane in the first place, and we 
can keep doing it in the 2.1 dataplane code.

> > If you could make it an assertion, that would also be great.  (BTW, it's
> > probably best to add a nesting count to plug/unplug).
>
> I'd rather to add it in future if we really need that.

I think this is a matter of correctness as Kevin's review pointed out.

Paolo