On Thu, Jul 18, 2013 at 11:21:42PM +0200, Charlie Shepherd wrote: > This patch makes bdrv_flush a synchronous function and updates any callers > from > a coroutine context to use bdrv_co_flush instead. > > The motivation for this patch comes from the GSoC Continuation-Passing C > project. When coroutines were introduced, synchronous functions in the block > layer were converted to use asynchronous methods by dynamically detecting if > they were being run from a coroutine context by calling qemu_in_coroutine(), > and > yielding if so. If they were not, they would spawn a new coroutine and poll > until the asynchronous counterpart finished. > > However this approach does not work with CPC as the CPC translator converts > all > functions annotated coroutine_fn to a different (continuation based) calling > convention. This means that coroutine_fn annotated functions cannot be called > from a non-coroutine context. > > This patch is a Request For Comments on the approach of splitting these > "dynamic" functions into synchronous and asynchronous versions. This is easy > for > bdrv_flush as it already has an asynchronous counterpart - bdrv_co_flush. The > only caller of bdrv_flush from a coroutine context is mirror_drain in > block/mirror.c - this should be annotated as a coroutine_fn as it calls > qemu_coroutine_yield(). > > If this approach meets with approval I will develop a patchset splitting the > other "dynamic" functions in the block layer. This will allow all coroutine > functions to have a coroutine_fn annotation that can be statically checked > (CPC > can be used to verify annotations). > > I have audited the other callers of bdrv_flush, they are included below: > > block.c: bdrv_reopen_prepare, bdrv_close, bdrv_commit, bdrv_pwrite_sync
bdrv_pwrite_sync() is a dynamic function. If called from coroutine context it will yield (indirectly from bdrv_pwrite() or bdrv_flush()). > block/qcow2-cache.c: qcow2_cache_entry_flush, qcow2_cache_flush > block/qcow2-refcount.c: qcow2_update_snapshot_refcount > block/qcow2-snapshot.c: qcow2_write_snapshots > block/qcow2.c: qcow2_mark_dirty, qcow2_mark_clean qcow2 runs in coroutine context, the coroutine_fn annotations are just missing. See block/qcow2.c:bdrv_qcow2 for the entry points like qcow2_co_readv. > block/qed-check.c: qed_check_mark_clean > block/qed.c: bdrv_qed_open, bdrv_qed_close > blockdev.c: external_snapshot_prepare, do_drive_del > cpus.c: do_vm_stop > hw/block/nvme.c: nvme_clear_ctrl > qemu-io-cmds.c: flush_f > savevm.c: bdrv_fclose I think the rest are fine and your patch looks good.
