| Message ID | 20221116122241.2856527-11-eesposit@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Still more coroutine and various fixes in block layer | expand |
Am 16.11.2022 um 13:22 hat Emanuele Giuseppe Esposito geschrieben: > This function is never called in coroutine context, therefore > instead of manually creating a new coroutine, delegate it to the > block-coroutine-wrapper script, defining it as g_c_w_simple. > > Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> At first I thought that "never called in coroutine context" was not entirely true because of paths like this: qcow2_co_create() -> bdrv_open_blockdev_ref() -> bdrv_open_inherit() -> bdrv_append_temp_snapshot() -> bdrv_create(). The only reason why it doesn't happen is that with a BlockdevRef, it's impossible to get BDRV_O_SNAPSHOT set, so bdrv_append_temp_snapshot() can't actually be called when you come from bdrv_open_blockdev_ref(). Of course, opening images in a coroutine is likely to already do similar things. We should probably drop out of coroutine context for bdrv_open to be safe. In practice, I suspect it might work anyway because nothing is going to wait on the current coroutine, but maybe better not to rely on it. Anyway, not a problem of your patch, it's just something it made me think of. > diff --git a/block.c b/block.c > index 7a4c3eb540..d3e168408a 100644 > --- a/block.c > +++ b/block.c > @@ -528,7 +528,7 @@ typedef struct CreateCo { > Error *err; > } CreateCo; > > -static int coroutine_fn bdrv_co_create(BlockDriver *drv, const char *filename, > +int coroutine_fn bdrv_co_create(BlockDriver *drv, const char *filename, > QemuOpts *opts, Error **errp) The indentation is off now. With this fixed: Reviewed-by: Kevin Wolf <kwolf@redhat.com>
diff --git a/block.c b/block.c index 7a4c3eb540..d3e168408a 100644 --- a/block.c +++ b/block.c @@ -528,7 +528,7 @@ typedef struct CreateCo { Error *err; } CreateCo; -static int coroutine_fn bdrv_co_create(BlockDriver *drv, const char *filename, +int coroutine_fn bdrv_co_create(BlockDriver *drv, const char *filename, QemuOpts *opts, Error **errp) { int ret; @@ -555,42 +555,6 @@ static int coroutine_fn bdrv_co_create(BlockDriver *drv, const char *filename, return ret; } -static void coroutine_fn bdrv_create_co_entry(void *opaque) -{ - CreateCo *cco = opaque; - GLOBAL_STATE_CODE(); - - cco->ret = bdrv_co_create(cco->drv, cco->filename, cco->opts, &cco->err); -} - -int bdrv_create(BlockDriver *drv, const char* filename, - QemuOpts *opts, Error **errp) -{ - GLOBAL_STATE_CODE(); - - if (qemu_in_coroutine()) { - /* Fast-path if already in coroutine context */ - return bdrv_co_create(drv, filename, opts, errp); - } else { - Coroutine *co; - CreateCo cco = { - .drv = drv, - .filename = filename, - .opts = opts, - .ret = NOT_DONE, - .err = NULL, - }; - - co = qemu_coroutine_create(bdrv_create_co_entry, &cco); - qemu_coroutine_enter(co); - while (cco.ret == NOT_DONE) { - aio_poll(qemu_get_aio_context(), true); - } - error_propagate(errp, cco.err); - return cco.ret; - } -} - /** * Helper function for bdrv_create_file_fallback(): Resize @blk to at * least the given @minimum_size. diff --git a/include/block/block-global-state.h b/include/block/block-global-state.h index 6f35ed99e3..305336bdb9 100644 --- a/include/block/block-global-state.h +++ b/include/block/block-global-state.h @@ -55,8 +55,14 @@ BlockDriver *bdrv_find_protocol(const char *filename, bool allow_protocol_prefix, Error **errp); BlockDriver *bdrv_find_format(const char *format_name); -int bdrv_create(BlockDriver *drv, const char* filename, - QemuOpts *opts, Error **errp); + +int coroutine_fn bdrv_co_create(BlockDriver *drv, const char* filename, + QemuOpts *opts, Error **errp); +int generated_co_wrapper_simple bdrv_create(BlockDriver *drv, + const char* filename, + QemuOpts *opts, + Error **errp); + int coroutine_fn bdrv_create_file(const char *filename, QemuOpts *opts, Error **errp);
This function is never called in coroutine context, therefore instead of manually creating a new coroutine, delegate it to the block-coroutine-wrapper script, defining it as g_c_w_simple. Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> --- block.c | 38 +----------------------------- include/block/block-global-state.h | 10 ++++++-- 2 files changed, 9 insertions(+), 39 deletions(-)