* [PATCH v3 1/8] block-copy: add missing coroutine_fn annotations
2022-11-16 8:50 [PATCH v3 0/8] Still more coroutine and various fixes in block layer Emanuele Giuseppe Esposito
@ 2022-11-16 8:50 ` Emanuele Giuseppe Esposito
2022-11-16 8:50 ` [PATCH v3 2/8] nbd/server.c: " Emanuele Giuseppe Esposito
` (7 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Emanuele Giuseppe Esposito @ 2022-11-16 8:50 UTC (permalink / raw)
To: qemu-block
Cc: Kevin Wolf, Hanna Reitz, John Snow, Vladimir Sementsov-Ogievskiy,
Eric Blake, Fam Zheng, qemu-devel, Emanuele Giuseppe Esposito
block_copy_reset_unallocated and block_copy_is_cluster_allocated are
only called by backup_run, a corotuine_fn itself.
Same applies to block_copy_block_status, called by
block_copy_dirty_clusters.
Therefore mark them as coroutine too.
Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
block/block-copy.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/block/block-copy.c b/block/block-copy.c
index bb947afdda..f33ab1d0b6 100644
--- a/block/block-copy.c
+++ b/block/block-copy.c
@@ -577,8 +577,9 @@ static coroutine_fn int block_copy_task_entry(AioTask *task)
return ret;
}
-static int block_copy_block_status(BlockCopyState *s, int64_t offset,
- int64_t bytes, int64_t *pnum)
+static coroutine_fn int block_copy_block_status(BlockCopyState *s,
+ int64_t offset,
+ int64_t bytes, int64_t *pnum)
{
int64_t num;
BlockDriverState *base;
@@ -613,8 +614,9 @@ static int block_copy_block_status(BlockCopyState *s, int64_t offset,
* Check if the cluster starting at offset is allocated or not.
* return via pnum the number of contiguous clusters sharing this allocation.
*/
-static int block_copy_is_cluster_allocated(BlockCopyState *s, int64_t offset,
- int64_t *pnum)
+static int coroutine_fn block_copy_is_cluster_allocated(BlockCopyState *s,
+ int64_t offset,
+ int64_t *pnum)
{
BlockDriverState *bs = s->source->bs;
int64_t count, total_count = 0;
@@ -669,8 +671,9 @@ void block_copy_reset(BlockCopyState *s, int64_t offset, int64_t bytes)
* @return 0 when the cluster at @offset was unallocated,
* 1 otherwise, and -ret on error.
*/
-int64_t block_copy_reset_unallocated(BlockCopyState *s,
- int64_t offset, int64_t *count)
+int64_t coroutine_fn block_copy_reset_unallocated(BlockCopyState *s,
+ int64_t offset,
+ int64_t *count)
{
int ret;
int64_t clusters, bytes;
--
2.31.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v3 2/8] nbd/server.c: add missing coroutine_fn annotations
2022-11-16 8:50 [PATCH v3 0/8] Still more coroutine and various fixes in block layer Emanuele Giuseppe Esposito
2022-11-16 8:50 ` [PATCH v3 1/8] block-copy: add missing coroutine_fn annotations Emanuele Giuseppe Esposito
@ 2022-11-16 8:50 ` Emanuele Giuseppe Esposito
2022-11-16 8:50 ` [PATCH v3 3/8] block-backend: replace bdrv_*_above with blk_*_above Emanuele Giuseppe Esposito
` (6 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Emanuele Giuseppe Esposito @ 2022-11-16 8:50 UTC (permalink / raw)
To: qemu-block
Cc: Kevin Wolf, Hanna Reitz, John Snow, Vladimir Sementsov-Ogievskiy,
Eric Blake, Fam Zheng, qemu-devel, Emanuele Giuseppe Esposito
There are probably more missing, but right now it is necessary that
we extend coroutine_fn to block{allock/status}_to_extents, because
they use bdrv_* functions calling the generated_co_wrapper API, which
checks for the qemu_in_coroutine() case.
Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
nbd/server.c | 21 ++++++++++++---------
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/nbd/server.c b/nbd/server.c
index ada16089f3..e2eec0cf46 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -2141,8 +2141,9 @@ static int nbd_extent_array_add(NBDExtentArray *ea,
return 0;
}
-static int blockstatus_to_extents(BlockDriverState *bs, uint64_t offset,
- uint64_t bytes, NBDExtentArray *ea)
+static int coroutine_fn blockstatus_to_extents(BlockDriverState *bs,
+ uint64_t offset, uint64_t bytes,
+ NBDExtentArray *ea)
{
while (bytes) {
uint32_t flags;
@@ -2168,8 +2169,9 @@ static int blockstatus_to_extents(BlockDriverState *bs, uint64_t offset,
return 0;
}
-static int blockalloc_to_extents(BlockDriverState *bs, uint64_t offset,
- uint64_t bytes, NBDExtentArray *ea)
+static int coroutine_fn blockalloc_to_extents(BlockDriverState *bs,
+ uint64_t offset, uint64_t bytes,
+ NBDExtentArray *ea)
{
while (bytes) {
int64_t num;
@@ -2220,11 +2222,12 @@ static int nbd_co_send_extents(NBDClient *client, uint64_t handle,
}
/* Get block status from the exported device and send it to the client */
-static int nbd_co_send_block_status(NBDClient *client, uint64_t handle,
- BlockDriverState *bs, uint64_t offset,
- uint32_t length, bool dont_fragment,
- bool last, uint32_t context_id,
- Error **errp)
+static int
+coroutine_fn nbd_co_send_block_status(NBDClient *client, uint64_t handle,
+ BlockDriverState *bs, uint64_t offset,
+ uint32_t length, bool dont_fragment,
+ bool last, uint32_t context_id,
+ Error **errp)
{
int ret;
unsigned int nb_extents = dont_fragment ? 1 : NBD_MAX_BLOCK_STATUS_EXTENTS;
--
2.31.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v3 3/8] block-backend: replace bdrv_*_above with blk_*_above
2022-11-16 8:50 [PATCH v3 0/8] Still more coroutine and various fixes in block layer Emanuele Giuseppe Esposito
2022-11-16 8:50 ` [PATCH v3 1/8] block-copy: add missing coroutine_fn annotations Emanuele Giuseppe Esposito
2022-11-16 8:50 ` [PATCH v3 2/8] nbd/server.c: " Emanuele Giuseppe Esposito
@ 2022-11-16 8:50 ` Emanuele Giuseppe Esposito
2022-11-16 8:50 ` [PATCH v3 4/8] block: distinguish between bdrv_create running in coroutine and not Emanuele Giuseppe Esposito
` (5 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Emanuele Giuseppe Esposito @ 2022-11-16 8:50 UTC (permalink / raw)
To: qemu-block
Cc: Kevin Wolf, Hanna Reitz, John Snow, Vladimir Sementsov-Ogievskiy,
Eric Blake, Fam Zheng, qemu-devel, Emanuele Giuseppe Esposito
Avoid mixing bdrv_* functions with blk_*, so create blk_* counterparts
for:
- bdrv_block_status_above
- bdrv_is_allocated_above
Note that these functions will take the rdlock, so they must always run
in a coroutine.
Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
block/block-backend.c | 21 +++++++++++++++++++++
block/commit.c | 4 ++--
include/sysemu/block-backend-io.h | 9 +++++++++
nbd/server.c | 28 ++++++++++++++--------------
qemu-img.c | 4 ++--
5 files changed, 48 insertions(+), 18 deletions(-)
diff --git a/block/block-backend.c b/block/block-backend.c
index 742efa7955..333d50fb3f 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1424,6 +1424,27 @@ int coroutine_fn blk_co_pwritev(BlockBackend *blk, int64_t offset,
return blk_co_pwritev_part(blk, offset, bytes, qiov, 0, flags);
}
+int coroutine_fn blk_block_status_above(BlockBackend *blk,
+ BlockDriverState *base,
+ int64_t offset, int64_t bytes,
+ int64_t *pnum, int64_t *map,
+ BlockDriverState **file)
+{
+ IO_CODE();
+ return bdrv_block_status_above(blk_bs(blk), base, offset, bytes, pnum, map,
+ file);
+}
+
+int coroutine_fn blk_is_allocated_above(BlockBackend *blk,
+ BlockDriverState *base,
+ bool include_base, int64_t offset,
+ int64_t bytes, int64_t *pnum)
+{
+ IO_CODE();
+ return bdrv_is_allocated_above(blk_bs(blk), base, include_base, offset,
+ bytes, pnum);
+}
+
typedef struct BlkRwCo {
BlockBackend *blk;
int64_t offset;
diff --git a/block/commit.c b/block/commit.c
index 0029b31944..9d4d908344 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -155,8 +155,8 @@ static int coroutine_fn commit_run(Job *job, Error **errp)
break;
}
/* Copy if allocated above the base */
- ret = bdrv_is_allocated_above(blk_bs(s->top), s->base_overlay, true,
- offset, COMMIT_BUFFER_SIZE, &n);
+ ret = blk_is_allocated_above(s->top, s->base_overlay, true,
+ offset, COMMIT_BUFFER_SIZE, &n);
copy = (ret > 0);
trace_commit_one_iteration(s, offset, n, ret);
if (copy) {
diff --git a/include/sysemu/block-backend-io.h b/include/sysemu/block-backend-io.h
index 50f5aa2e07..a47cb825e5 100644
--- a/include/sysemu/block-backend-io.h
+++ b/include/sysemu/block-backend-io.h
@@ -92,6 +92,15 @@ int coroutine_fn blk_co_copy_range(BlockBackend *blk_in, int64_t off_in,
int64_t bytes, BdrvRequestFlags read_flags,
BdrvRequestFlags write_flags);
+int coroutine_fn blk_block_status_above(BlockBackend *blk,
+ BlockDriverState *base,
+ int64_t offset, int64_t bytes,
+ int64_t *pnum, int64_t *map,
+ BlockDriverState **file);
+int coroutine_fn blk_is_allocated_above(BlockBackend *blk,
+ BlockDriverState *base,
+ bool include_base, int64_t offset,
+ int64_t bytes, int64_t *pnum);
/*
* "I/O or GS" API functions. These functions can run without
diff --git a/nbd/server.c b/nbd/server.c
index e2eec0cf46..6389b46396 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -1991,7 +1991,7 @@ static int coroutine_fn nbd_co_send_structured_error(NBDClient *client,
}
/* Do a sparse read and send the structured reply to the client.
- * Returns -errno if sending fails. bdrv_block_status_above() failure is
+ * Returns -errno if sending fails. blk_block_status_above() failure is
* reported to the client, at which point this function succeeds.
*/
static int coroutine_fn nbd_co_send_sparse_read(NBDClient *client,
@@ -2007,10 +2007,10 @@ static int coroutine_fn nbd_co_send_sparse_read(NBDClient *client,
while (progress < size) {
int64_t pnum;
- int status = bdrv_block_status_above(blk_bs(exp->common.blk), NULL,
- offset + progress,
- size - progress, &pnum, NULL,
- NULL);
+ int status = blk_block_status_above(exp->common.blk, NULL,
+ offset + progress,
+ size - progress, &pnum, NULL,
+ NULL);
bool final;
if (status < 0) {
@@ -2141,14 +2141,14 @@ static int nbd_extent_array_add(NBDExtentArray *ea,
return 0;
}
-static int coroutine_fn blockstatus_to_extents(BlockDriverState *bs,
+static int coroutine_fn blockstatus_to_extents(BlockBackend *blk,
uint64_t offset, uint64_t bytes,
NBDExtentArray *ea)
{
while (bytes) {
uint32_t flags;
int64_t num;
- int ret = bdrv_block_status_above(bs, NULL, offset, bytes, &num,
+ int ret = blk_block_status_above(blk, NULL, offset, bytes, &num,
NULL, NULL);
if (ret < 0) {
@@ -2169,13 +2169,13 @@ static int coroutine_fn blockstatus_to_extents(BlockDriverState *bs,
return 0;
}
-static int coroutine_fn blockalloc_to_extents(BlockDriverState *bs,
+static int coroutine_fn blockalloc_to_extents(BlockBackend *blk,
uint64_t offset, uint64_t bytes,
NBDExtentArray *ea)
{
while (bytes) {
int64_t num;
- int ret = bdrv_is_allocated_above(bs, NULL, false, offset, bytes,
+ int ret = blk_is_allocated_above(blk, NULL, false, offset, bytes,
&num);
if (ret < 0) {
@@ -2224,7 +2224,7 @@ static int nbd_co_send_extents(NBDClient *client, uint64_t handle,
/* Get block status from the exported device and send it to the client */
static int
coroutine_fn nbd_co_send_block_status(NBDClient *client, uint64_t handle,
- BlockDriverState *bs, uint64_t offset,
+ BlockBackend *blk, uint64_t offset,
uint32_t length, bool dont_fragment,
bool last, uint32_t context_id,
Error **errp)
@@ -2234,9 +2234,9 @@ coroutine_fn nbd_co_send_block_status(NBDClient *client, uint64_t handle,
g_autoptr(NBDExtentArray) ea = nbd_extent_array_new(nb_extents);
if (context_id == NBD_META_ID_BASE_ALLOCATION) {
- ret = blockstatus_to_extents(bs, offset, length, ea);
+ ret = blockstatus_to_extents(blk, offset, length, ea);
} else {
- ret = blockalloc_to_extents(bs, offset, length, ea);
+ ret = blockalloc_to_extents(blk, offset, length, ea);
}
if (ret < 0) {
return nbd_co_send_structured_error(
@@ -2563,7 +2563,7 @@ static coroutine_fn int nbd_handle_request(NBDClient *client,
if (client->export_meta.base_allocation) {
ret = nbd_co_send_block_status(client, request->handle,
- blk_bs(exp->common.blk),
+ exp->common.blk,
request->from,
request->len, dont_fragment,
!--contexts_remaining,
@@ -2576,7 +2576,7 @@ static coroutine_fn int nbd_handle_request(NBDClient *client,
if (client->export_meta.allocation_depth) {
ret = nbd_co_send_block_status(client, request->handle,
- blk_bs(exp->common.blk),
+ exp->common.blk,
request->from, request->len,
dont_fragment,
!--contexts_remaining,
diff --git a/qemu-img.c b/qemu-img.c
index a3b64c88af..4282a34bc0 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1730,8 +1730,8 @@ static int convert_iteration_sectors(ImgConvertState *s, int64_t sector_num)
do {
count = n * BDRV_SECTOR_SIZE;
- ret = bdrv_block_status_above(src_bs, base, offset, count, &count,
- NULL, NULL);
+ ret = bdrv_block_status_above(src_bs, base, offset, count,
+ &count, NULL, NULL);
if (ret < 0) {
if (s->salvage) {
--
2.31.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v3 4/8] block: distinguish between bdrv_create running in coroutine and not
2022-11-16 8:50 [PATCH v3 0/8] Still more coroutine and various fixes in block layer Emanuele Giuseppe Esposito
` (2 preceding siblings ...)
2022-11-16 8:50 ` [PATCH v3 3/8] block-backend: replace bdrv_*_above with blk_*_above Emanuele Giuseppe Esposito
@ 2022-11-16 8:50 ` Emanuele Giuseppe Esposito
2022-11-16 8:50 ` [PATCH v3 5/8] block/vmdk: add missing coroutine_fn annotations Emanuele Giuseppe Esposito
` (4 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Emanuele Giuseppe Esposito @ 2022-11-16 8:50 UTC (permalink / raw)
To: qemu-block
Cc: Kevin Wolf, Hanna Reitz, John Snow, Vladimir Sementsov-Ogievskiy,
Eric Blake, Fam Zheng, qemu-devel, Emanuele Giuseppe Esposito
Call two different functions depending on whether bdrv_create
is in coroutine or not, following the same pattern as
generated_co_wrapper functions.
This allows to also call the coroutine function directly,
without using CreateCo or relying in bdrv_create().
It will also be useful when we add the graph rdlock to the
coroutine case.
Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
block.c | 74 ++++++++++++++++++++++++++++-----------------------------
1 file changed, 36 insertions(+), 38 deletions(-)
diff --git a/block.c b/block.c
index 577639c7e0..375c8056a3 100644
--- a/block.c
+++ b/block.c
@@ -528,66 +528,64 @@ typedef struct CreateCo {
Error *err;
} CreateCo;
-static void coroutine_fn bdrv_create_co_entry(void *opaque)
+static int coroutine_fn bdrv_co_create(BlockDriver *drv, const char *filename,
+ QemuOpts *opts, Error **errp)
{
- Error *local_err = NULL;
int ret;
+ GLOBAL_STATE_CODE();
+ assert(*errp == NULL);
+
+ if (!drv->bdrv_co_create_opts) {
+ error_setg(errp, "Driver '%s' does not support image creation",
+ drv->format_name);
+ return -ENOTSUP;
+ }
+
+ ret = drv->bdrv_co_create_opts(drv, filename, opts, errp);
+ if (ret < 0 && !*errp) {
+ error_setg_errno(errp, -ret, "Could not create image");
+ }
+
+ return ret;
+}
+
+static void coroutine_fn bdrv_create_co_entry(void *opaque)
+{
CreateCo *cco = opaque;
- assert(cco->drv);
GLOBAL_STATE_CODE();
+ assert(cco->drv);
- ret = cco->drv->bdrv_co_create_opts(cco->drv,
- cco->filename, cco->opts, &local_err);
- error_propagate(&cco->err, local_err);
- cco->ret = ret;
+ 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)
{
- int ret;
-
GLOBAL_STATE_CODE();
- Coroutine *co;
- CreateCo cco = {
- .drv = drv,
- .filename = g_strdup(filename),
- .opts = opts,
- .ret = NOT_DONE,
- .err = NULL,
- };
-
- if (!drv->bdrv_co_create_opts) {
- error_setg(errp, "Driver '%s' does not support image creation", drv->format_name);
- ret = -ENOTSUP;
- goto out;
- }
-
if (qemu_in_coroutine()) {
/* Fast-path if already in coroutine context */
- bdrv_create_co_entry(&cco);
+ return bdrv_co_create(drv, filename, opts, errp);
} else {
+ Coroutine *co;
+ CreateCo cco = {
+ .drv = drv,
+ .filename = g_strdup(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);
+ g_free(cco.filename);
+ return cco.ret;
}
-
- ret = cco.ret;
- if (ret < 0) {
- if (cco.err) {
- error_propagate(errp, cco.err);
- } else {
- error_setg_errno(errp, -ret, "Could not create image");
- }
- }
-
-out:
- g_free(cco.filename);
- return ret;
}
/**
--
2.31.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v3 5/8] block/vmdk: add missing coroutine_fn annotations
2022-11-16 8:50 [PATCH v3 0/8] Still more coroutine and various fixes in block layer Emanuele Giuseppe Esposito
` (3 preceding siblings ...)
2022-11-16 8:50 ` [PATCH v3 4/8] block: distinguish between bdrv_create running in coroutine and not Emanuele Giuseppe Esposito
@ 2022-11-16 8:50 ` Emanuele Giuseppe Esposito
2022-11-16 8:50 ` [PATCH v3 6/8] block: bdrv_create_file is a coroutine_fn Emanuele Giuseppe Esposito
` (3 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Emanuele Giuseppe Esposito @ 2022-11-16 8:50 UTC (permalink / raw)
To: qemu-block
Cc: Kevin Wolf, Hanna Reitz, John Snow, Vladimir Sementsov-Ogievskiy,
Eric Blake, Fam Zheng, qemu-devel, Emanuele Giuseppe Esposito
vmdk_co_create_opts() is a coroutine_fn, and calls vmdk_co_do_create()
which in turn can call two callbacks: vmdk_co_create_opts_cb and
vmdk_co_create_cb.
Mark all these functions as coroutine_fn, since vmdk_co_create_opts()
is the only caller.
Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
block/vmdk.c | 36 +++++++++++++++++++-----------------
1 file changed, 19 insertions(+), 17 deletions(-)
diff --git a/block/vmdk.c b/block/vmdk.c
index 26376352b9..0c32bf2e83 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -2285,10 +2285,11 @@ exit:
return ret;
}
-static int vmdk_create_extent(const char *filename, int64_t filesize,
- bool flat, bool compress, bool zeroed_grain,
- BlockBackend **pbb,
- QemuOpts *opts, Error **errp)
+static int coroutine_fn vmdk_create_extent(const char *filename,
+ int64_t filesize, bool flat,
+ bool compress, bool zeroed_grain,
+ BlockBackend **pbb,
+ QemuOpts *opts, Error **errp)
{
int ret;
BlockBackend *blk = NULL;
@@ -2366,14 +2367,14 @@ static int filename_decompose(const char *filename, char *path, char *prefix,
* non-split format.
* idx >= 1: get the n-th extent if in a split subformat
*/
-typedef BlockBackend *(*vmdk_create_extent_fn)(int64_t size,
- int idx,
- bool flat,
- bool split,
- bool compress,
- bool zeroed_grain,
- void *opaque,
- Error **errp);
+typedef BlockBackend * coroutine_fn (*vmdk_create_extent_fn)(int64_t size,
+ int idx,
+ bool flat,
+ bool split,
+ bool compress,
+ bool zeroed_grain,
+ void *opaque,
+ Error **errp);
static void vmdk_desc_add_extent(GString *desc,
const char *extent_line_fmt,
@@ -2616,7 +2617,7 @@ typedef struct {
QemuOpts *opts;
} VMDKCreateOptsData;
-static BlockBackend *vmdk_co_create_opts_cb(int64_t size, int idx,
+static BlockBackend * coroutine_fn vmdk_co_create_opts_cb(int64_t size, int idx,
bool flat, bool split, bool compress,
bool zeroed_grain, void *opaque,
Error **errp)
@@ -2768,10 +2769,11 @@ exit:
return ret;
}
-static BlockBackend *vmdk_co_create_cb(int64_t size, int idx,
- bool flat, bool split, bool compress,
- bool zeroed_grain, void *opaque,
- Error **errp)
+static BlockBackend * coroutine_fn vmdk_co_create_cb(int64_t size, int idx,
+ bool flat, bool split,
+ bool compress,
+ bool zeroed_grain,
+ void *opaque, Error **errp)
{
int ret;
BlockDriverState *bs;
--
2.31.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v3 6/8] block: bdrv_create_file is a coroutine_fn
2022-11-16 8:50 [PATCH v3 0/8] Still more coroutine and various fixes in block layer Emanuele Giuseppe Esposito
` (4 preceding siblings ...)
2022-11-16 8:50 ` [PATCH v3 5/8] block/vmdk: add missing coroutine_fn annotations Emanuele Giuseppe Esposito
@ 2022-11-16 8:50 ` Emanuele Giuseppe Esposito
2022-11-16 8:50 ` [PATCH v3 7/8] block: bdrv_create is never called in coroutine context Emanuele Giuseppe Esposito
` (2 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Emanuele Giuseppe Esposito @ 2022-11-16 8:50 UTC (permalink / raw)
To: qemu-block
Cc: Kevin Wolf, Hanna Reitz, John Snow, Vladimir Sementsov-Ogievskiy,
Eric Blake, Fam Zheng, qemu-devel, Emanuele Giuseppe Esposito
It is always called in coroutine_fn callbacks, therefore
it can directly call bdrv_co_create().
Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
block.c | 6 ++++--
include/block/block-global-state.h | 3 ++-
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/block.c b/block.c
index 375c8056a3..dcac28756c 100644
--- a/block.c
+++ b/block.c
@@ -533,6 +533,7 @@ static int coroutine_fn bdrv_co_create(BlockDriver *drv, const char *filename,
{
int ret;
GLOBAL_STATE_CODE();
+ assert(qemu_in_coroutine());
assert(*errp == NULL);
if (!drv->bdrv_co_create_opts) {
@@ -723,7 +724,8 @@ out:
return ret;
}
-int bdrv_create_file(const char *filename, QemuOpts *opts, Error **errp)
+int coroutine_fn bdrv_create_file(const char *filename, QemuOpts *opts,
+ Error **errp)
{
QemuOpts *protocol_opts;
BlockDriver *drv;
@@ -764,7 +766,7 @@ int bdrv_create_file(const char *filename, QemuOpts *opts, Error **errp)
goto out;
}
- ret = bdrv_create(drv, filename, protocol_opts, errp);
+ ret = bdrv_co_create(drv, filename, protocol_opts, errp);
out:
qemu_opts_del(protocol_opts);
qobject_unref(qdict);
diff --git a/include/block/block-global-state.h b/include/block/block-global-state.h
index 00e0cf8aea..6f35ed99e3 100644
--- a/include/block/block-global-state.h
+++ b/include/block/block-global-state.h
@@ -57,7 +57,8 @@ BlockDriver *bdrv_find_protocol(const char *filename,
BlockDriver *bdrv_find_format(const char *format_name);
int bdrv_create(BlockDriver *drv, const char* filename,
QemuOpts *opts, Error **errp);
-int bdrv_create_file(const char *filename, QemuOpts *opts, Error **errp);
+int coroutine_fn bdrv_create_file(const char *filename, QemuOpts *opts,
+ Error **errp);
BlockDriverState *bdrv_new(void);
int bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top,
--
2.31.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v3 7/8] block: bdrv_create is never called in coroutine context
2022-11-16 8:50 [PATCH v3 0/8] Still more coroutine and various fixes in block layer Emanuele Giuseppe Esposito
` (5 preceding siblings ...)
2022-11-16 8:50 ` [PATCH v3 6/8] block: bdrv_create_file is a coroutine_fn Emanuele Giuseppe Esposito
@ 2022-11-16 8:50 ` Emanuele Giuseppe Esposito
2022-11-16 8:50 ` [PATCH v3 8/8] block/dirty-bitmap: remove unnecessary qemu_in_coroutine() case Emanuele Giuseppe Esposito
2022-11-16 10:41 ` [PATCH v3 0/8] Still more coroutine and various fixes in block layer Emanuele Giuseppe Esposito
8 siblings, 0 replies; 10+ messages in thread
From: Emanuele Giuseppe Esposito @ 2022-11-16 8:50 UTC (permalink / raw)
To: qemu-block
Cc: Kevin Wolf, Hanna Reitz, John Snow, Vladimir Sementsov-Ogievskiy,
Eric Blake, Fam Zheng, qemu-devel, Emanuele Giuseppe Esposito
Delete the if case and make sure it won't be called again
in coroutines.
Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
block.c | 37 ++++++++++++++++---------------------
1 file changed, 16 insertions(+), 21 deletions(-)
diff --git a/block.c b/block.c
index dcac28756c..7a4ce7948c 100644
--- a/block.c
+++ b/block.c
@@ -563,30 +563,25 @@ static void coroutine_fn bdrv_create_co_entry(void *opaque)
int bdrv_create(BlockDriver *drv, const char* filename,
QemuOpts *opts, Error **errp)
{
+ Coroutine *co;
+ CreateCo cco = {
+ .drv = drv,
+ .filename = g_strdup(filename),
+ .opts = opts,
+ .ret = NOT_DONE,
+ .err = NULL,
+ };
GLOBAL_STATE_CODE();
+ assert(!qemu_in_coroutine());
- 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 = g_strdup(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);
- g_free(cco.filename);
- return cco.ret;
+ 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);
+ g_free(cco.filename);
+ return cco.ret;
}
/**
--
2.31.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v3 8/8] block/dirty-bitmap: remove unnecessary qemu_in_coroutine() case
2022-11-16 8:50 [PATCH v3 0/8] Still more coroutine and various fixes in block layer Emanuele Giuseppe Esposito
` (6 preceding siblings ...)
2022-11-16 8:50 ` [PATCH v3 7/8] block: bdrv_create is never called in coroutine context Emanuele Giuseppe Esposito
@ 2022-11-16 8:50 ` Emanuele Giuseppe Esposito
2022-11-16 10:41 ` [PATCH v3 0/8] Still more coroutine and various fixes in block layer Emanuele Giuseppe Esposito
8 siblings, 0 replies; 10+ messages in thread
From: Emanuele Giuseppe Esposito @ 2022-11-16 8:50 UTC (permalink / raw)
To: qemu-block
Cc: Kevin Wolf, Hanna Reitz, John Snow, Vladimir Sementsov-Ogievskiy,
Eric Blake, Fam Zheng, qemu-devel, Emanuele Giuseppe Esposito
Some functions check if they are running in a coroutine, calling
the coroutine callback directly if it's the case.
Except that no coroutine calls such functions, therefore that case
can be removed.
Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
block/dirty-bitmap.c | 66 +++++++++++++++++++-------------------------
1 file changed, 29 insertions(+), 37 deletions(-)
diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index bf3dc0512a..8092d08261 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -418,24 +418,20 @@ bdrv_co_remove_persistent_dirty_bitmap_entry(void *opaque)
int bdrv_remove_persistent_dirty_bitmap(BlockDriverState *bs, const char *name,
Error **errp)
{
- if (qemu_in_coroutine()) {
- return bdrv_co_remove_persistent_dirty_bitmap(bs, name, errp);
- } else {
- Coroutine *co;
- BdrvRemovePersistentDirtyBitmapCo s = {
- .bs = bs,
- .name = name,
- .errp = errp,
- .ret = -EINPROGRESS,
- };
-
- co = qemu_coroutine_create(bdrv_co_remove_persistent_dirty_bitmap_entry,
- &s);
- bdrv_coroutine_enter(bs, co);
- BDRV_POLL_WHILE(bs, s.ret == -EINPROGRESS);
-
- return s.ret;
- }
+ Coroutine *co;
+ BdrvRemovePersistentDirtyBitmapCo s = {
+ .bs = bs,
+ .name = name,
+ .errp = errp,
+ .ret = -EINPROGRESS,
+ };
+ assert(!qemu_in_coroutine());
+ co = qemu_coroutine_create(bdrv_co_remove_persistent_dirty_bitmap_entry,
+ &s);
+ bdrv_coroutine_enter(bs, co);
+ BDRV_POLL_WHILE(bs, s.ret == -EINPROGRESS);
+
+ return s.ret;
}
bool
@@ -494,25 +490,21 @@ bool bdrv_can_store_new_dirty_bitmap(BlockDriverState *bs, const char *name,
uint32_t granularity, Error **errp)
{
IO_CODE();
- if (qemu_in_coroutine()) {
- return bdrv_co_can_store_new_dirty_bitmap(bs, name, granularity, errp);
- } else {
- Coroutine *co;
- BdrvCanStoreNewDirtyBitmapCo s = {
- .bs = bs,
- .name = name,
- .granularity = granularity,
- .errp = errp,
- .in_progress = true,
- };
-
- co = qemu_coroutine_create(bdrv_co_can_store_new_dirty_bitmap_entry,
- &s);
- bdrv_coroutine_enter(bs, co);
- BDRV_POLL_WHILE(bs, s.in_progress);
-
- return s.ret;
- }
+ Coroutine *co;
+ BdrvCanStoreNewDirtyBitmapCo s = {
+ .bs = bs,
+ .name = name,
+ .granularity = granularity,
+ .errp = errp,
+ .in_progress = true,
+ };
+ assert(!qemu_in_coroutine());
+ co = qemu_coroutine_create(bdrv_co_can_store_new_dirty_bitmap_entry,
+ &s);
+ bdrv_coroutine_enter(bs, co);
+ BDRV_POLL_WHILE(bs, s.in_progress);
+
+ return s.ret;
}
void bdrv_disable_dirty_bitmap(BdrvDirtyBitmap *bitmap)
--
2.31.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v3 0/8] Still more coroutine and various fixes in block layer
2022-11-16 8:50 [PATCH v3 0/8] Still more coroutine and various fixes in block layer Emanuele Giuseppe Esposito
` (7 preceding siblings ...)
2022-11-16 8:50 ` [PATCH v3 8/8] block/dirty-bitmap: remove unnecessary qemu_in_coroutine() case Emanuele Giuseppe Esposito
@ 2022-11-16 10:41 ` Emanuele Giuseppe Esposito
8 siblings, 0 replies; 10+ messages in thread
From: Emanuele Giuseppe Esposito @ 2022-11-16 10:41 UTC (permalink / raw)
To: qemu-block
Cc: Kevin Wolf, Hanna Reitz, John Snow, Vladimir Sementsov-Ogievskiy,
Eric Blake, Fam Zheng, qemu-devel
I apologize, as discussed also in v2 I just realized I could introduce
generated_co_wrapper_simple already here and simplify patches 6 and 8.
Also I think commit messages are the old ones from v1.
I'll resend. Please ignore this serie.
Emanuele
Am 16/11/2022 um 09:50 schrieb Emanuele Giuseppe Esposito:
> This is a dump of all minor coroutine-related fixes found while looking
> around and testing various things in the QEMU block layer.
>
> Patches aim to:
> - add missing coroutine_fn annotation to the functions
> - simplify to avoid the typical "if in coroutine: fn()
> // else create_coroutine(fn)" already present in generated_co_wraper
> functions.
> - make sure that if a BlockDriver callback is defined as coroutine_fn, then
> it is always running in a coroutine.
>
> This serie is based on Kevin Wolf's series "block: Simplify drain".
>
> Based-on: <20221108123738.530873-1-kwolf@redhat.com>
>
> Emanuele
> ---
> v3:
> * Remove patch 1, base on kevin "drain semplification serie"
>
> v2:
> * clarified commit message in patches 2/3/6 on why we add coroutine_fn
>
> Emanuele Giuseppe Esposito (8):
> block-copy: add missing coroutine_fn annotations
> nbd/server.c: add missing coroutine_fn annotations
> block-backend: replace bdrv_*_above with blk_*_above
> block: distinguish between bdrv_create running in coroutine and not
> block/vmdk: add missing coroutine_fn annotations
> block: bdrv_create_file is a coroutine_fn
> block: bdrv_create is never called in coroutine context
> block/dirty-bitmap: remove unnecessary qemu_in_coroutine() case
>
> block.c | 75 ++++++++++++++----------------
> block/block-backend.c | 21 +++++++++
> block/block-copy.c | 15 +++---
> block/commit.c | 4 +-
> block/dirty-bitmap.c | 66 ++++++++++++--------------
> block/vmdk.c | 36 +++++++-------
> include/block/block-global-state.h | 3 +-
> include/sysemu/block-backend-io.h | 9 ++++
> nbd/server.c | 43 +++++++++--------
> qemu-img.c | 4 +-
> 10 files changed, 151 insertions(+), 125 deletions(-)
>
^ permalink raw reply [flat|nested] 10+ messages in thread