On 10/14/2011 01:08 PM, Kevin Wolf wrote:
Am 14.10.2011 10:41, schrieb Paolo Bonzini:
Add coroutine support for flush and apply the same emulation that
we already do for read/write. bdrv_aio_flush is simplified to always
go through a coroutine.
Signed-off-by: Paolo Bonzini<pbonz...@redhat.com>
To make the implementation more consistent with read/write operations,
wouldn't it make sense to provide a bdrv_co_flush() globally instead of
using the synchronous version as the preferred public interface?
I thought about it, but then it turned out that I would have
bdrv_flush
-> create coroutine or just fast-path to bdrv_flush_co_entry
-> bdrv_flush_co_entry
-> driver
and
bdrv_co_flush
-> bdrv_flush_co_entry
-> driver
In other words, the code would be exactly the same, save for an "if
(qemu_in_coroutine())". The reason is that, unlike read/write, neither
flush nor discard take a qiov.
In general, I think that with Stefan's cleanup having specialized
coroutine versions has in general a much smaller benefit. The code
reading benefit of naming routines like bdrv_co_* is already lost, for
example, since bdrv_read can yield when called for coroutine context.
Let me show how this might go. Right now you have
bdrv_read/write
-> bdrv_rw_co
-> create qiov
-> create coroutine or just fast-path to bdrv_rw_co_entry
-> bdrv_rw_co_entry
-> bdrv_co_do_readv/writev
-> driver
bdrv_co_readv/writev
-> bdrv_co_do_readv/writev
-> driver
But starting from here, you might just as well reorganize it like this:
bdrv_read/writev
-> bdrv_rw_co
-> create qiov
-> bdrv_readv/writev
bdrv_readv/writev
-> create coroutine or just fast-path to bdrv_rw_co_entry
-> bdrv_rw_co_entry
-> bdrv_co_do_readv/writev
-> driver
and just drop bdrv_co_readv, since it would just be hard-coding the
fast-path of bdrv_readv.
Since some amount of synchronous I/O would likely always be there, for
example in qemu-img, I think this unification would make more sense than
providing two separate entrypoints for bdrv_co_flush and bdrv_flush.
Paolo
