* [PATCH v5 00/15] Still more coroutine and various fixes in block layer
@ 2022-11-23 11:42 Emanuele Giuseppe Esposito
2022-11-23 11:42 ` [PATCH v5 01/15] block-io: introduce coroutine_fn duplicates for bdrv_common_block_status_above callers Emanuele Giuseppe Esposito
` (14 more replies)
0 siblings, 15 replies; 28+ messages in thread
From: Emanuele Giuseppe Esposito @ 2022-11-23 11:42 UTC (permalink / raw)
To: qemu-block
Cc: Kevin Wolf, Hanna Reitz, John Snow, Vladimir Sementsov-Ogievskiy,
Eric Blake, Fam Zheng, Stefan Hajnoczi, Denis V. Lunev,
Stefan Weil, Jeff Cody, Cleber Rosa, qemu-devel,
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
---
v5:
* add missing reviewed-by from Paolo
* minor indentation fixes
* use when possible _co_, but do not create new g_c_w. It will be done in
future series
* introduce QEMU_IN_COROUTINE
* reorder patches
* rebase on kevin block branch + v2 from "block: Simplify drain"
v4:
* use v2 commit messages
* introduce generated_co_wrapper_simple to simplify patches
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 (15):
block-io: introduce coroutine_fn duplicates for
bdrv_common_block_status_above callers
block-copy: add missing coroutine_fn annotations
nbd/server.c: add missing coroutine_fn annotations
block-backend: replace bdrv_*_above with blk_*_above
block/vmdk: add missing coroutine_fn annotations
block: avoid duplicating filename string in bdrv_create
block: introduce QEMU_IN_COROUTINE macro
block: distinguish between bdrv_create running in coroutine and not
block: bdrv_create_file is a coroutine_fn
block-coroutine-wrapper.py: introduce generated_co_wrapper_simple
block-coroutine-wrapper.py: default to main loop aiocontext if
function does not have a BlockDriverState parameter
block-coroutine-wrapper.py: default to main loop aiocontext if
function does not have a BlockDriverState parameter
block-coroutine-wrapper.py: support also basic return types
block: convert bdrv_create to generated_co_wrapper_simple
block/dirty-bitmap: convert coroutine-only functions to
generated_co_wrapper_simple
block.c | 66 ++++------------
block/block-backend.c | 21 +++++
block/block-copy.c | 21 ++---
block/block-gen.h | 11 +--
block/commit.c | 4 +-
block/crypto.c | 2 +-
block/dirty-bitmap.c | 88 +--------------------
block/io.c | 64 +++++++++++++++-
block/meson.build | 1 +
block/parallels.c | 2 +-
block/qcow.c | 2 +-
block/qcow2.c | 4 +-
block/qed.c | 2 +-
block/raw-format.c | 2 +-
block/vdi.c | 2 +-
block/vhdx.c | 2 +-
block/vmdk.c | 38 +++++-----
block/vpc.c | 2 +-
include/block/block-common.h | 17 ++++-
include/block/block-copy.h | 5 +-
include/block/block-global-state.h | 13 +++-
include/block/block-io.h | 24 +++++-
include/block/dirty-bitmap.h | 10 ++-
include/sysemu/block-backend-io.h | 9 +++
nbd/server.c | 47 ++++++------
scripts/block-coroutine-wrapper.py | 118 ++++++++++++++++++++---------
26 files changed, 322 insertions(+), 255 deletions(-)
--
2.31.1
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v5 01/15] block-io: introduce coroutine_fn duplicates for bdrv_common_block_status_above callers
2022-11-23 11:42 [PATCH v5 00/15] Still more coroutine and various fixes in block layer Emanuele Giuseppe Esposito
@ 2022-11-23 11:42 ` Emanuele Giuseppe Esposito
2022-11-23 16:29 ` Kevin Wolf
2022-11-23 11:42 ` [PATCH v5 02/15] block-copy: add missing coroutine_fn annotations Emanuele Giuseppe Esposito
` (13 subsequent siblings)
14 siblings, 1 reply; 28+ messages in thread
From: Emanuele Giuseppe Esposito @ 2022-11-23 11:42 UTC (permalink / raw)
To: qemu-block
Cc: Kevin Wolf, Hanna Reitz, John Snow, Vladimir Sementsov-Ogievskiy,
Eric Blake, Fam Zheng, Stefan Hajnoczi, Denis V. Lunev,
Stefan Weil, Jeff Cody, Cleber Rosa, qemu-devel,
Emanuele Giuseppe Esposito
bdrv_common_block_status_above() is a g_c_w, and it is being called by
many "wrapper" functions like bdrv_is_allocated(),
bdrv_is_allocated_above() and bdrv_block_status_above().
Because we want to eventually split the coroutine from non-coroutine
case in g_c_w, create duplicate wrappers that take care of directly
calling the same coroutine functions called in the g_c_w.
Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
block/io.c | 64 ++++++++++++++++++++++++++++++++++++++--
include/block/block-io.h | 15 ++++++++++
2 files changed, 76 insertions(+), 3 deletions(-)
diff --git a/block/io.c b/block/io.c
index 38e57d1f67..1bc05c8282 100644
--- a/block/io.c
+++ b/block/io.c
@@ -2533,6 +2533,19 @@ bdrv_co_common_block_status_above(BlockDriverState *bs,
return ret;
}
+int coroutine_fn bdrv_co_block_status_above(BlockDriverState *bs,
+ BlockDriverState *base,
+ int64_t offset, int64_t bytes,
+ int64_t *pnum, int64_t *map,
+ BlockDriverState **file)
+{
+ IO_CODE();
+ /* If QEMU_IN_COROUTINE() fails, use bdrv_block_status_above() */
+ QEMU_IN_COROUTINE();
+ return bdrv_co_common_block_status_above(bs, base, false, true, offset,
+ bytes, pnum, map, file, NULL);
+}
+
int bdrv_block_status_above(BlockDriverState *bs, BlockDriverState *base,
int64_t offset, int64_t bytes, int64_t *pnum,
int64_t *map, BlockDriverState **file)
@@ -2578,6 +2591,24 @@ int coroutine_fn bdrv_co_is_zero_fast(BlockDriverState *bs, int64_t offset,
return (pnum == bytes) && (ret & BDRV_BLOCK_ZERO);
}
+int coroutine_fn bdrv_co_is_allocated(BlockDriverState *bs, int64_t offset,
+ int64_t bytes, int64_t *pnum)
+{
+ int ret;
+ int64_t dummy;
+ IO_CODE();
+ /* If QEMU_IN_COROUTINE() fails, use bdrv_is_allocated() */
+ QEMU_IN_COROUTINE();
+
+ ret = bdrv_co_common_block_status_above(bs, bs, true, false, offset,
+ bytes, pnum ? pnum : &dummy, NULL,
+ NULL, NULL);
+ if (ret < 0) {
+ return ret;
+ }
+ return !!(ret & BDRV_BLOCK_ALLOCATED);
+}
+
int bdrv_is_allocated(BlockDriverState *bs, int64_t offset, int64_t bytes,
int64_t *pnum)
{
@@ -2594,6 +2625,31 @@ int bdrv_is_allocated(BlockDriverState *bs, int64_t offset, int64_t bytes,
return !!(ret & BDRV_BLOCK_ALLOCATED);
}
+/* See bdrv_is_allocated_above for documentation */
+int coroutine_fn bdrv_co_is_allocated_above(BlockDriverState *top,
+ BlockDriverState *base,
+ bool include_base, int64_t offset,
+ int64_t bytes, int64_t *pnum)
+{
+ int depth;
+ int ret;
+ IO_CODE();
+ /* If QEMU_IN_COROUTINE() fails, use bdrv_is_allocated_above() */
+ QEMU_IN_COROUTINE();
+
+ ret = bdrv_co_common_block_status_above(top, base, include_base, false,
+ offset, bytes, pnum, NULL, NULL,
+ &depth);
+ if (ret < 0) {
+ return ret;
+ }
+
+ if (ret & BDRV_BLOCK_ALLOCATED) {
+ return depth;
+ }
+ return 0;
+}
+
/*
* Given an image chain: ... -> [BASE] -> [INTER1] -> [INTER2] -> [TOP]
*
@@ -2617,10 +2673,12 @@ int bdrv_is_allocated_above(BlockDriverState *top,
int64_t bytes, int64_t *pnum)
{
int depth;
- int ret = bdrv_common_block_status_above(top, base, include_base, false,
- offset, bytes, pnum, NULL, NULL,
- &depth);
+ int ret;
IO_CODE();
+
+ ret = bdrv_common_block_status_above(top, base, include_base, false,
+ offset, bytes, pnum, NULL, NULL,
+ &depth);
if (ret < 0) {
return ret;
}
diff --git a/include/block/block-io.h b/include/block/block-io.h
index 92aaa7c1e9..72919254cd 100644
--- a/include/block/block-io.h
+++ b/include/block/block-io.h
@@ -94,14 +94,29 @@ bool bdrv_can_write_zeroes_with_unmap(BlockDriverState *bs);
int bdrv_block_status(BlockDriverState *bs, int64_t offset,
int64_t bytes, int64_t *pnum, int64_t *map,
BlockDriverState **file);
+
+int coroutine_fn bdrv_co_block_status_above(BlockDriverState *bs,
+ BlockDriverState *base,
+ int64_t offset, int64_t bytes,
+ int64_t *pnum, int64_t *map,
+ BlockDriverState **file);
int bdrv_block_status_above(BlockDriverState *bs, BlockDriverState *base,
int64_t offset, int64_t bytes, int64_t *pnum,
int64_t *map, BlockDriverState **file);
+
+int coroutine_fn bdrv_co_is_allocated(BlockDriverState *bs, int64_t offset,
+ int64_t bytes, int64_t *pnum);
int bdrv_is_allocated(BlockDriverState *bs, int64_t offset, int64_t bytes,
int64_t *pnum);
+
+int coroutine_fn bdrv_co_is_allocated_above(BlockDriverState *top,
+ BlockDriverState *base,
+ bool include_base, int64_t offset,
+ int64_t bytes, int64_t *pnum);
int bdrv_is_allocated_above(BlockDriverState *top, BlockDriverState *base,
bool include_base, int64_t offset, int64_t bytes,
int64_t *pnum);
+
int coroutine_fn bdrv_co_is_zero_fast(BlockDriverState *bs, int64_t offset,
int64_t bytes);
--
2.31.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v5 02/15] block-copy: add missing coroutine_fn annotations
2022-11-23 11:42 [PATCH v5 00/15] Still more coroutine and various fixes in block layer Emanuele Giuseppe Esposito
2022-11-23 11:42 ` [PATCH v5 01/15] block-io: introduce coroutine_fn duplicates for bdrv_common_block_status_above callers Emanuele Giuseppe Esposito
@ 2022-11-23 11:42 ` Emanuele Giuseppe Esposito
2022-11-23 16:32 ` Kevin Wolf
2022-11-23 11:42 ` [PATCH v5 03/15] nbd/server.c: " Emanuele Giuseppe Esposito
` (12 subsequent siblings)
14 siblings, 1 reply; 28+ messages in thread
From: Emanuele Giuseppe Esposito @ 2022-11-23 11:42 UTC (permalink / raw)
To: qemu-block
Cc: Kevin Wolf, Hanna Reitz, John Snow, Vladimir Sementsov-Ogievskiy,
Eric Blake, Fam Zheng, Stefan Hajnoczi, Denis V. Lunev,
Stefan Weil, Jeff Cody, Cleber Rosa, qemu-devel,
Emanuele Giuseppe Esposito, Paolo Bonzini
These functions end up calling bdrv_common_block_status_above(), a
generated_co_wrapper function.
In addition, they also happen to be always called in coroutine context,
meaning all callers are coroutine_fn.
This means that the g_c_w function will enter the qemu_in_coroutine()
case and eventually suspend (or in other words call qemu_coroutine_yield()).
Therefore we need to mark such functions coroutine_fn too.
Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
block/block-copy.c | 21 ++++++++++++---------
include/block/block-copy.h | 5 +++--
2 files changed, 15 insertions(+), 11 deletions(-)
diff --git a/block/block-copy.c b/block/block-copy.c
index bb947afdda..5e59d6262f 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;
@@ -590,8 +591,8 @@ static int block_copy_block_status(BlockCopyState *s, int64_t offset,
base = NULL;
}
- ret = bdrv_block_status_above(s->source->bs, base, offset, bytes, &num,
- NULL, NULL);
+ ret = bdrv_co_block_status_above(s->source->bs, base, offset, bytes, &num,
+ NULL, NULL);
if (ret < 0 || num < s->cluster_size) {
/*
* On error or if failed to obtain large enough chunk just fallback to
@@ -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;
@@ -624,7 +626,7 @@ static int block_copy_is_cluster_allocated(BlockCopyState *s, int64_t offset,
assert(QEMU_IS_ALIGNED(offset, s->cluster_size));
while (true) {
- ret = bdrv_is_allocated(bs, offset, bytes, &count);
+ ret = bdrv_co_is_allocated(bs, offset, bytes, &count);
if (ret < 0) {
return ret;
}
@@ -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;
diff --git a/include/block/block-copy.h b/include/block/block-copy.h
index ba0b425d78..8cea4f9b90 100644
--- a/include/block/block-copy.h
+++ b/include/block/block-copy.h
@@ -36,8 +36,9 @@ void block_copy_set_progress_meter(BlockCopyState *s, ProgressMeter *pm);
void block_copy_state_free(BlockCopyState *s);
void block_copy_reset(BlockCopyState *s, int64_t offset, int64_t bytes);
-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 coroutine_fn block_copy(BlockCopyState *s, int64_t offset, int64_t bytes,
bool ignore_ratelimit, uint64_t timeout_ns,
--
2.31.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v5 03/15] nbd/server.c: add missing coroutine_fn annotations
2022-11-23 11:42 [PATCH v5 00/15] Still more coroutine and various fixes in block layer Emanuele Giuseppe Esposito
2022-11-23 11:42 ` [PATCH v5 01/15] block-io: introduce coroutine_fn duplicates for bdrv_common_block_status_above callers Emanuele Giuseppe Esposito
2022-11-23 11:42 ` [PATCH v5 02/15] block-copy: add missing coroutine_fn annotations Emanuele Giuseppe Esposito
@ 2022-11-23 11:42 ` Emanuele Giuseppe Esposito
2022-11-23 11:42 ` [PATCH v5 04/15] block-backend: replace bdrv_*_above with blk_*_above Emanuele Giuseppe Esposito
` (11 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Emanuele Giuseppe Esposito @ 2022-11-23 11:42 UTC (permalink / raw)
To: qemu-block
Cc: Kevin Wolf, Hanna Reitz, John Snow, Vladimir Sementsov-Ogievskiy,
Eric Blake, Fam Zheng, Stefan Hajnoczi, Denis V. Lunev,
Stefan Weil, Jeff Cody, Cleber Rosa, qemu-devel,
Emanuele Giuseppe Esposito, Paolo Bonzini
These functions end up calling bdrv_*() implemented as generated_co_wrapper
functions.
In addition, they also happen to be always called in coroutine context,
meaning all callers are coroutine_fn.
This means that the g_c_w function will enter the qemu_in_coroutine()
case and eventually suspend (or in other words call qemu_coroutine_yield()).
Therefore we need to mark such functions coroutine_fn too.
Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
nbd/server.c | 29 ++++++++++++++++-------------
1 file changed, 16 insertions(+), 13 deletions(-)
diff --git a/nbd/server.c b/nbd/server.c
index ada16089f3..4af5c793a7 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -2141,14 +2141,15 @@ 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;
int64_t num;
- int ret = bdrv_block_status_above(bs, NULL, offset, bytes, &num,
- NULL, NULL);
+ int ret = bdrv_co_block_status_above(bs, NULL, offset, bytes, &num,
+ NULL, NULL);
if (ret < 0) {
return ret;
@@ -2168,13 +2169,14 @@ 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;
- int ret = bdrv_is_allocated_above(bs, NULL, false, offset, bytes,
- &num);
+ int ret = bdrv_co_is_allocated_above(bs, NULL, false, offset, bytes,
+ &num);
if (ret < 0) {
return ret;
@@ -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] 28+ messages in thread
* [PATCH v5 04/15] block-backend: replace bdrv_*_above with blk_*_above
2022-11-23 11:42 [PATCH v5 00/15] Still more coroutine and various fixes in block layer Emanuele Giuseppe Esposito
` (2 preceding siblings ...)
2022-11-23 11:42 ` [PATCH v5 03/15] nbd/server.c: " Emanuele Giuseppe Esposito
@ 2022-11-23 11:42 ` Emanuele Giuseppe Esposito
2022-11-23 16:42 ` Kevin Wolf
2022-11-23 11:42 ` [PATCH v5 05/15] block/vmdk: add missing coroutine_fn annotations Emanuele Giuseppe Esposito
` (10 subsequent siblings)
14 siblings, 1 reply; 28+ messages in thread
From: Emanuele Giuseppe Esposito @ 2022-11-23 11:42 UTC (permalink / raw)
To: qemu-block
Cc: Kevin Wolf, Hanna Reitz, John Snow, Vladimir Sementsov-Ogievskiy,
Eric Blake, Fam Zheng, Stefan Hajnoczi, Denis V. Lunev,
Stefan Weil, Jeff Cody, Cleber Rosa, qemu-devel,
Emanuele Giuseppe Esposito, Paolo Bonzini
Avoid mixing bdrv_* functions with blk_*, so create blk_* counterparts
for bdrv_block_status_above and bdrv_is_allocated_above.
Note that since bdrv_block_status_above only calls the g_c_w function
bdrv_common_block_status_above and is marked as coroutine_fn, call
directly bdrv_co_common_block_status_above() to avoid using a g_c_w.
Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
block/block-backend.c | 21 ++++++++++++++++++++
block/commit.c | 4 ++--
include/sysemu/block-backend-io.h | 9 +++++++++
nbd/server.c | 32 +++++++++++++++----------------
4 files changed, 48 insertions(+), 18 deletions(-)
diff --git a/block/block-backend.c b/block/block-backend.c
index 742efa7955..03bed68e4f 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_co_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_co_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_co_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..108be63628 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_co_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 4af5c793a7..2663ea282f 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_co_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_co_block_status_above(exp->common.blk, NULL,
+ offset + progress,
+ size - progress, &pnum, NULL,
+ NULL);
bool final;
if (status < 0) {
@@ -2141,15 +2141,15 @@ 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_co_block_status_above(bs, NULL, offset, bytes, &num,
- NULL, NULL);
+ int ret = blk_co_block_status_above(blk, NULL, offset, bytes, &num,
+ NULL, NULL);
if (ret < 0) {
return ret;
@@ -2169,14 +2169,14 @@ 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_co_is_allocated_above(bs, NULL, false, offset, bytes,
- &num);
+ int ret = blk_is_allocated_above(blk, NULL, false, offset, bytes,
+ &num);
if (ret < 0) {
return ret;
@@ -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,
--
2.31.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v5 05/15] block/vmdk: add missing coroutine_fn annotations
2022-11-23 11:42 [PATCH v5 00/15] Still more coroutine and various fixes in block layer Emanuele Giuseppe Esposito
` (3 preceding siblings ...)
2022-11-23 11:42 ` [PATCH v5 04/15] block-backend: replace bdrv_*_above with blk_*_above Emanuele Giuseppe Esposito
@ 2022-11-23 11:42 ` Emanuele Giuseppe Esposito
2022-11-23 11:42 ` [PATCH v5 06/15] block: avoid duplicating filename string in bdrv_create Emanuele Giuseppe Esposito
` (9 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Emanuele Giuseppe Esposito @ 2022-11-23 11:42 UTC (permalink / raw)
To: qemu-block
Cc: Kevin Wolf, Hanna Reitz, John Snow, Vladimir Sementsov-Ogievskiy,
Eric Blake, Fam Zheng, Stefan Hajnoczi, Denis V. Lunev,
Stefan Weil, Jeff Cody, Cleber Rosa, qemu-devel,
Emanuele Giuseppe Esposito, Paolo Bonzini
These functions end up calling bdrv_create() implemented as generated_co_wrapper
functions.
In addition, they also happen to be always called in coroutine context,
meaning all callers are coroutine_fn.
This means that the g_c_w function will enter the qemu_in_coroutine()
case and eventually suspend (or in other words call qemu_coroutine_yield()).
Therefore we need to mark such functions coroutine_fn too.
Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@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] 28+ messages in thread
* [PATCH v5 06/15] block: avoid duplicating filename string in bdrv_create
2022-11-23 11:42 [PATCH v5 00/15] Still more coroutine and various fixes in block layer Emanuele Giuseppe Esposito
` (4 preceding siblings ...)
2022-11-23 11:42 ` [PATCH v5 05/15] block/vmdk: add missing coroutine_fn annotations Emanuele Giuseppe Esposito
@ 2022-11-23 11:42 ` Emanuele Giuseppe Esposito
2022-11-23 16:45 ` Kevin Wolf
2022-11-23 11:42 ` [PATCH v5 07/15] block: introduce QEMU_IN_COROUTINE macro Emanuele Giuseppe Esposito
` (8 subsequent siblings)
14 siblings, 1 reply; 28+ messages in thread
From: Emanuele Giuseppe Esposito @ 2022-11-23 11:42 UTC (permalink / raw)
To: qemu-block
Cc: Kevin Wolf, Hanna Reitz, John Snow, Vladimir Sementsov-Ogievskiy,
Eric Blake, Fam Zheng, Stefan Hajnoczi, Denis V. Lunev,
Stefan Weil, Jeff Cody, Cleber Rosa, qemu-devel,
Emanuele Giuseppe Esposito
We know that the string will stay around until the function
returns, and the parameter of drv->bdrv_co_create_opts is const char*,
so it must not be modified either.
Suggested-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
block.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/block.c b/block.c
index 8c9f4ee37c..9d51e7b6e5 100644
--- a/block.c
+++ b/block.c
@@ -553,7 +553,7 @@ int bdrv_create(BlockDriver *drv, const char* filename,
Coroutine *co;
CreateCo cco = {
.drv = drv,
- .filename = g_strdup(filename),
+ .filename = filename,
.opts = opts,
.ret = NOT_DONE,
.err = NULL,
@@ -561,8 +561,7 @@ int bdrv_create(BlockDriver *drv, const char* filename,
if (!drv->bdrv_co_create_opts) {
error_setg(errp, "Driver '%s' does not support image creation", drv->format_name);
- ret = -ENOTSUP;
- goto out;
+ return -ENOTSUP;
}
if (qemu_in_coroutine()) {
@@ -585,8 +584,6 @@ int bdrv_create(BlockDriver *drv, const char* filename,
}
}
-out:
- g_free(cco.filename);
return ret;
}
--
2.31.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v5 07/15] block: introduce QEMU_IN_COROUTINE macro
2022-11-23 11:42 [PATCH v5 00/15] Still more coroutine and various fixes in block layer Emanuele Giuseppe Esposito
` (5 preceding siblings ...)
2022-11-23 11:42 ` [PATCH v5 06/15] block: avoid duplicating filename string in bdrv_create Emanuele Giuseppe Esposito
@ 2022-11-23 11:42 ` Emanuele Giuseppe Esposito
2022-11-23 16:49 ` Kevin Wolf
2022-11-23 11:42 ` [PATCH v5 08/15] block: distinguish between bdrv_create running in coroutine and not Emanuele Giuseppe Esposito
` (7 subsequent siblings)
14 siblings, 1 reply; 28+ messages in thread
From: Emanuele Giuseppe Esposito @ 2022-11-23 11:42 UTC (permalink / raw)
To: qemu-block
Cc: Kevin Wolf, Hanna Reitz, John Snow, Vladimir Sementsov-Ogievskiy,
Eric Blake, Fam Zheng, Stefan Hajnoczi, Denis V. Lunev,
Stefan Weil, Jeff Cody, Cleber Rosa, qemu-devel,
Emanuele Giuseppe Esposito
This macro will be used to mark all coroutine_fn functions.
Right now, it will be used for the newly introduced coroutine_fn, since
we know the callers.
As a TODO, in the future we might want to add this macro to all
corotuine_fn functions, to be sure that they are only called in
coroutines context.
Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
include/block/block-common.h | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/include/block/block-common.h b/include/block/block-common.h
index 297704c1e9..24de1d63fd 100644
--- a/include/block/block-common.h
+++ b/include/block/block-common.h
@@ -34,6 +34,17 @@
#include "qemu/hbitmap.h"
#include "qemu/transactions.h"
+/*
+ * QEMU_IN_COROUTINE
+ *
+ * To be used in all coroutine_fn functions, to make sure that the caller
+ * is always a coroutine.
+ */
+#define QEMU_IN_COROUTINE() \
+ do { \
+ assert(qemu_in_coroutine()); \
+ } while (0)
+
/*
* generated_co_wrapper
*
--
2.31.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v5 08/15] block: distinguish between bdrv_create running in coroutine and not
2022-11-23 11:42 [PATCH v5 00/15] Still more coroutine and various fixes in block layer Emanuele Giuseppe Esposito
` (6 preceding siblings ...)
2022-11-23 11:42 ` [PATCH v5 07/15] block: introduce QEMU_IN_COROUTINE macro Emanuele Giuseppe Esposito
@ 2022-11-23 11:42 ` Emanuele Giuseppe Esposito
2022-11-23 16:55 ` Kevin Wolf
2022-11-23 11:42 ` [PATCH v5 09/15] block: bdrv_create_file is a coroutine_fn Emanuele Giuseppe Esposito
` (6 subsequent siblings)
14 siblings, 1 reply; 28+ messages in thread
From: Emanuele Giuseppe Esposito @ 2022-11-23 11:42 UTC (permalink / raw)
To: qemu-block
Cc: Kevin Wolf, Hanna Reitz, John Snow, Vladimir Sementsov-Ogievskiy,
Eric Blake, Fam Zheng, Stefan Hajnoczi, Denis V. Lunev,
Stefan Weil, Jeff Cody, Cleber Rosa, 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().
Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
block.c | 72 +++++++++++++++++++++++++++++----------------------------
1 file changed, 37 insertions(+), 35 deletions(-)
diff --git a/block.c b/block.c
index 9d51e7b6e5..8a7f87f783 100644
--- a/block.c
+++ b/block.c
@@ -528,63 +528,65 @@ 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();
+ QEMU_IN_COROUTINE();
+ ERRP_GUARD();
+ assert(*errp == NULL);
+ assert(drv);
+
+ 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();
- 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 = 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);
- return -ENOTSUP;
- }
-
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 = 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;
}
-
- ret = cco.ret;
- if (ret < 0) {
- if (cco.err) {
- error_propagate(errp, cco.err);
- } else {
- error_setg_errno(errp, -ret, "Could not create image");
- }
- }
-
- return ret;
}
/**
--
2.31.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v5 09/15] block: bdrv_create_file is a coroutine_fn
2022-11-23 11:42 [PATCH v5 00/15] Still more coroutine and various fixes in block layer Emanuele Giuseppe Esposito
` (7 preceding siblings ...)
2022-11-23 11:42 ` [PATCH v5 08/15] block: distinguish between bdrv_create running in coroutine and not Emanuele Giuseppe Esposito
@ 2022-11-23 11:42 ` Emanuele Giuseppe Esposito
2022-11-23 16:57 ` Kevin Wolf
2022-11-23 11:42 ` [PATCH v5 10/15] block-coroutine-wrapper.py: introduce generated_co_wrapper_simple Emanuele Giuseppe Esposito
` (5 subsequent siblings)
14 siblings, 1 reply; 28+ messages in thread
From: Emanuele Giuseppe Esposito @ 2022-11-23 11:42 UTC (permalink / raw)
To: qemu-block
Cc: Kevin Wolf, Hanna Reitz, John Snow, Vladimir Sementsov-Ogievskiy,
Eric Blake, Fam Zheng, Stefan Hajnoczi, Denis V. Lunev,
Stefan Weil, Jeff Cody, Cleber Rosa, qemu-devel,
Emanuele Giuseppe Esposito
It is always called in coroutine_fn callbacks, therefore
it can directly call bdrv_co_create().
Rename it to bdrv_co_create_file too.
Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
block.c | 5 +++--
block/crypto.c | 2 +-
block/parallels.c | 2 +-
block/qcow.c | 2 +-
block/qcow2.c | 4 ++--
block/qed.c | 2 +-
block/raw-format.c | 2 +-
block/vdi.c | 2 +-
block/vhdx.c | 2 +-
block/vmdk.c | 2 +-
block/vpc.c | 2 +-
include/block/block-global-state.h | 3 ++-
12 files changed, 16 insertions(+), 14 deletions(-)
diff --git a/block.c b/block.c
index 8a7f87f783..8207f93353 100644
--- a/block.c
+++ b/block.c
@@ -724,7 +724,8 @@ out:
return ret;
}
-int bdrv_create_file(const char *filename, QemuOpts *opts, Error **errp)
+int coroutine_fn bdrv_co_create_file(const char *filename, QemuOpts *opts,
+ Error **errp)
{
QemuOpts *protocol_opts;
BlockDriver *drv;
@@ -765,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/block/crypto.c b/block/crypto.c
index 2fb8add458..bbeb9f437c 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -703,7 +703,7 @@ static int coroutine_fn block_crypto_co_create_opts_luks(BlockDriver *drv,
}
/* Create protocol layer */
- ret = bdrv_create_file(filename, opts, errp);
+ ret = bdrv_co_create_file(filename, opts, errp);
if (ret < 0) {
goto fail;
}
diff --git a/block/parallels.c b/block/parallels.c
index fa08c1104b..bbea2f2221 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -646,7 +646,7 @@ static int coroutine_fn parallels_co_create_opts(BlockDriver *drv,
}
/* Create and open the file (protocol layer) */
- ret = bdrv_create_file(filename, opts, errp);
+ ret = bdrv_co_create_file(filename, opts, errp);
if (ret < 0) {
goto done;
}
diff --git a/block/qcow.c b/block/qcow.c
index daa38839ab..18e17a5b12 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -973,7 +973,7 @@ static int coroutine_fn qcow_co_create_opts(BlockDriver *drv,
}
/* Create and open the file (protocol layer) */
- ret = bdrv_create_file(filename, opts, errp);
+ ret = bdrv_co_create_file(filename, opts, errp);
if (ret < 0) {
goto fail;
}
diff --git a/block/qcow2.c b/block/qcow2.c
index 4d6666d3ff..7cc49a3a6c 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -3871,7 +3871,7 @@ static int coroutine_fn qcow2_co_create_opts(BlockDriver *drv,
}
/* Create and open the file (protocol layer) */
- ret = bdrv_create_file(filename, opts, errp);
+ ret = bdrv_co_create_file(filename, opts, errp);
if (ret < 0) {
goto finish;
}
@@ -3886,7 +3886,7 @@ static int coroutine_fn qcow2_co_create_opts(BlockDriver *drv,
/* Create and open an external data file (protocol layer) */
val = qdict_get_try_str(qdict, BLOCK_OPT_DATA_FILE);
if (val) {
- ret = bdrv_create_file(val, opts, errp);
+ ret = bdrv_co_create_file(val, opts, errp);
if (ret < 0) {
goto finish;
}
diff --git a/block/qed.c b/block/qed.c
index c2691a85b1..9d54c8eec5 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -778,7 +778,7 @@ static int coroutine_fn bdrv_qed_co_create_opts(BlockDriver *drv,
}
/* Create and open the file (protocol layer) */
- ret = bdrv_create_file(filename, opts, errp);
+ ret = bdrv_co_create_file(filename, opts, errp);
if (ret < 0) {
goto fail;
}
diff --git a/block/raw-format.c b/block/raw-format.c
index a68014ef0b..28905b09ee 100644
--- a/block/raw-format.c
+++ b/block/raw-format.c
@@ -433,7 +433,7 @@ static int coroutine_fn raw_co_create_opts(BlockDriver *drv,
QemuOpts *opts,
Error **errp)
{
- return bdrv_create_file(filename, opts, errp);
+ return bdrv_co_create_file(filename, opts, errp);
}
static int raw_open(BlockDriverState *bs, QDict *options, int flags,
diff --git a/block/vdi.c b/block/vdi.c
index c0c111c4b9..479bcfe820 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -934,7 +934,7 @@ static int coroutine_fn vdi_co_create_opts(BlockDriver *drv,
qdict = qemu_opts_to_qdict_filtered(opts, NULL, &vdi_create_opts, true);
/* Create and open the file (protocol layer) */
- ret = bdrv_create_file(filename, opts, errp);
+ ret = bdrv_co_create_file(filename, opts, errp);
if (ret < 0) {
goto done;
}
diff --git a/block/vhdx.c b/block/vhdx.c
index bad9ca691b..4c929800fe 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -2084,7 +2084,7 @@ static int coroutine_fn vhdx_co_create_opts(BlockDriver *drv,
}
/* Create and open the file (protocol layer) */
- ret = bdrv_create_file(filename, opts, errp);
+ ret = bdrv_co_create_file(filename, opts, errp);
if (ret < 0) {
goto fail;
}
diff --git a/block/vmdk.c b/block/vmdk.c
index 0c32bf2e83..afd3471915 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -2294,7 +2294,7 @@ static int coroutine_fn vmdk_create_extent(const char *filename,
int ret;
BlockBackend *blk = NULL;
- ret = bdrv_create_file(filename, opts, errp);
+ ret = bdrv_co_create_file(filename, opts, errp);
if (ret < 0) {
goto exit;
}
diff --git a/block/vpc.c b/block/vpc.c
index 95841f259a..6ee95dcb96 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -1111,7 +1111,7 @@ static int coroutine_fn vpc_co_create_opts(BlockDriver *drv,
}
/* Create and open the file (protocol layer) */
- ret = bdrv_create_file(filename, opts, errp);
+ ret = bdrv_co_create_file(filename, opts, errp);
if (ret < 0) {
goto fail;
}
diff --git a/include/block/block-global-state.h b/include/block/block-global-state.h
index 00e0cf8aea..387a7cbb2e 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_co_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] 28+ messages in thread
* [PATCH v5 10/15] block-coroutine-wrapper.py: introduce generated_co_wrapper_simple
2022-11-23 11:42 [PATCH v5 00/15] Still more coroutine and various fixes in block layer Emanuele Giuseppe Esposito
` (8 preceding siblings ...)
2022-11-23 11:42 ` [PATCH v5 09/15] block: bdrv_create_file is a coroutine_fn Emanuele Giuseppe Esposito
@ 2022-11-23 11:42 ` Emanuele Giuseppe Esposito
2022-11-23 17:07 ` Kevin Wolf
2022-11-23 11:42 ` [PATCH v5 11/15] block-coroutine-wrapper.py: default to main loop aiocontext if function does not have a BlockDriverState parameter Emanuele Giuseppe Esposito
` (4 subsequent siblings)
14 siblings, 1 reply; 28+ messages in thread
From: Emanuele Giuseppe Esposito @ 2022-11-23 11:42 UTC (permalink / raw)
To: qemu-block
Cc: Kevin Wolf, Hanna Reitz, John Snow, Vladimir Sementsov-Ogievskiy,
Eric Blake, Fam Zheng, Stefan Hajnoczi, Denis V. Lunev,
Stefan Weil, Jeff Cody, Cleber Rosa, qemu-devel,
Emanuele Giuseppe Esposito
This new annotation creates just a function wrapper that creates
a new coroutine. It assumes the caller is not a coroutine.
This is much better as g_c_w, because it is clear if the caller
is a coroutine or not, and provides the advantage of automating
the code creation. In the future all g_c_w functions will be
substituted on g_c_w_simple.
Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
include/block/block-common.h | 1 +
scripts/block-coroutine-wrapper.py | 105 ++++++++++++++++++++---------
2 files changed, 75 insertions(+), 31 deletions(-)
diff --git a/include/block/block-common.h b/include/block/block-common.h
index 24de1d63fd..46b57501d8 100644
--- a/include/block/block-common.h
+++ b/include/block/block-common.h
@@ -54,6 +54,7 @@
* Read more in docs/devel/block-coroutine-wrapper.rst
*/
#define generated_co_wrapper
+#define generated_co_wrapper_simple
/* block.c */
typedef struct BlockDriver BlockDriver;
diff --git a/scripts/block-coroutine-wrapper.py b/scripts/block-coroutine-wrapper.py
index 08be813407..2fa3d01898 100644
--- a/scripts/block-coroutine-wrapper.py
+++ b/scripts/block-coroutine-wrapper.py
@@ -62,10 +62,28 @@ def __init__(self, param_decl: str) -> None:
class FuncDecl:
- def __init__(self, return_type: str, name: str, args: str) -> None:
+ def __init__(self, return_type: str, name: str, args: str,
+ variant: str) -> None:
self.return_type = return_type.strip()
self.name = name.strip()
+ self.struct_name = snake_to_camel(self.name)
self.args = [ParamDecl(arg.strip()) for arg in args.split(',')]
+ self.create_only_co = False
+
+ if variant == '_simple':
+ self.create_only_co = True
+
+ subsystem, subname = self.name.split('_', 1)
+ self.co_name = f'{subsystem}_co_{subname}'
+
+ t = self.args[0].type
+ if t == 'BlockDriverState *':
+ bs = 'bs'
+ elif t == 'BdrvChild *':
+ bs = 'child->bs'
+ else:
+ bs = 'blk_bs(blk)'
+ self.bs = bs
def gen_list(self, format: str) -> str:
return ', '.join(format.format_map(arg.__dict__) for arg in self.args)
@@ -75,7 +93,8 @@ def gen_block(self, format: str) -> str:
# Match wrappers declared with a generated_co_wrapper mark
-func_decl_re = re.compile(r'^int\s*generated_co_wrapper\s*'
+func_decl_re = re.compile(r'^int\s*generated_co_wrapper'
+ r'(?P<variant>(_[a-z][a-z0-9_]*)?)\s*'
r'(?P<wrapper_name>[a-z][a-z0-9_]*)'
r'\((?P<args>[^)]*)\);$', re.MULTILINE)
@@ -84,7 +103,8 @@ def func_decl_iter(text: str) -> Iterator:
for m in func_decl_re.finditer(text):
yield FuncDecl(return_type='int',
name=m.group('wrapper_name'),
- args=m.group('args'))
+ args=m.group('args'),
+ variant=m.group('variant'))
def snake_to_camel(func_name: str) -> str:
@@ -97,24 +117,63 @@ def snake_to_camel(func_name: str) -> str:
return ''.join(words)
+# Checks if we are already in coroutine
+def create_g_c_w(func: FuncDecl) -> str:
+ name = func.co_name
+ struct_name = func.struct_name
+ return f"""\
+int {func.name}({ func.gen_list('{decl}') })
+{{
+ if (qemu_in_coroutine()) {{
+ return {name}({ func.gen_list('{name}') });
+ }} else {{
+ {struct_name} s = {{
+ .poll_state.bs = {func.bs},
+ .poll_state.in_progress = true,
+
+{ func.gen_block(' .{name} = {name},') }
+ }};
+
+ s.poll_state.co = qemu_coroutine_create({name}_entry, &s);
+
+ return bdrv_poll_co(&s.poll_state);
+ }}
+}}"""
+
+
+# Assumes we are not in coroutine, and creates one
+def create_coroutine_only(func: FuncDecl) -> str:
+ name = func.co_name
+ struct_name = func.struct_name
+ return f"""\
+int {func.name}({ func.gen_list('{decl}') })
+{{
+ {struct_name} s = {{
+ .poll_state.bs = {func.bs},
+ .poll_state.in_progress = true,
+
+{ func.gen_block(' .{name} = {name},') }
+ }};
+ assert(!qemu_in_coroutine());
+
+ s.poll_state.co = qemu_coroutine_create({name}_entry, &s);
+
+ return bdrv_poll_co(&s.poll_state);
+}}"""
+
+
def gen_wrapper(func: FuncDecl) -> str:
assert not '_co_' in func.name
assert func.return_type == 'int'
assert func.args[0].type in ['BlockDriverState *', 'BdrvChild *',
'BlockBackend *']
- subsystem, subname = func.name.split('_', 1)
-
- name = f'{subsystem}_co_{subname}'
+ name = func.co_name
+ struct_name = func.struct_name
- t = func.args[0].type
- if t == 'BlockDriverState *':
- bs = 'bs'
- elif t == 'BdrvChild *':
- bs = 'child->bs'
- else:
- bs = 'blk_bs(blk)'
- struct_name = snake_to_camel(name)
+ creation_function = create_g_c_w
+ if func.create_only_co:
+ creation_function = create_coroutine_only
return f"""\
/*
@@ -136,23 +195,7 @@ def gen_wrapper(func: FuncDecl) -> str:
aio_wait_kick();
}}
-int {func.name}({ func.gen_list('{decl}') })
-{{
- if (qemu_in_coroutine()) {{
- return {name}({ func.gen_list('{name}') });
- }} else {{
- {struct_name} s = {{
- .poll_state.bs = {bs},
- .poll_state.in_progress = true,
-
-{ func.gen_block(' .{name} = {name},') }
- }};
-
- s.poll_state.co = qemu_coroutine_create({name}_entry, &s);
-
- return bdrv_poll_co(&s.poll_state);
- }}
-}}"""
+{creation_function(func)}"""
def gen_wrappers(input_code: str) -> str:
--
2.31.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v5 11/15] block-coroutine-wrapper.py: default to main loop aiocontext if function does not have a BlockDriverState parameter
2022-11-23 11:42 [PATCH v5 00/15] Still more coroutine and various fixes in block layer Emanuele Giuseppe Esposito
` (9 preceding siblings ...)
2022-11-23 11:42 ` [PATCH v5 10/15] block-coroutine-wrapper.py: introduce generated_co_wrapper_simple Emanuele Giuseppe Esposito
@ 2022-11-23 11:42 ` Emanuele Giuseppe Esposito
2022-11-23 11:42 ` [PATCH v5 12/15] " Emanuele Giuseppe Esposito
` (3 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Emanuele Giuseppe Esposito @ 2022-11-23 11:42 UTC (permalink / raw)
To: qemu-block
Cc: Kevin Wolf, Hanna Reitz, John Snow, Vladimir Sementsov-Ogievskiy,
Eric Blake, Fam Zheng, Stefan Hajnoczi, Denis V. Lunev,
Stefan Weil, Jeff Cody, Cleber Rosa, qemu-devel,
Emanuele Giuseppe Esposito
Basically BdrvPollCo->bs is only used by bdrv_poll_co(), and the
functions that it uses are both using bdrv_get_aio_context, that
defaults to qemu_get_aio_context() if bs is NULL.
Therefore pass NULL to BdrvPollCo to automatically generate a function
that create and runs a coroutine in the main loop.
Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
scripts/block-coroutine-wrapper.py | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/scripts/block-coroutine-wrapper.py b/scripts/block-coroutine-wrapper.py
index 2fa3d01898..7e8f2da84b 100644
--- a/scripts/block-coroutine-wrapper.py
+++ b/scripts/block-coroutine-wrapper.py
@@ -81,8 +81,10 @@ def __init__(self, return_type: str, name: str, args: str,
bs = 'bs'
elif t == 'BdrvChild *':
bs = 'child->bs'
- else:
+ elif t == 'BlockBackend *':
bs = 'blk_bs(blk)'
+ else:
+ bs = 'NULL'
self.bs = bs
def gen_list(self, format: str) -> str:
@@ -165,8 +167,6 @@ def create_coroutine_only(func: FuncDecl) -> str:
def gen_wrapper(func: FuncDecl) -> str:
assert not '_co_' in func.name
assert func.return_type == 'int'
- assert func.args[0].type in ['BlockDriverState *', 'BdrvChild *',
- 'BlockBackend *']
name = func.co_name
struct_name = func.struct_name
--
2.31.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v5 12/15] block-coroutine-wrapper.py: default to main loop aiocontext if function does not have a BlockDriverState parameter
2022-11-23 11:42 [PATCH v5 00/15] Still more coroutine and various fixes in block layer Emanuele Giuseppe Esposito
` (10 preceding siblings ...)
2022-11-23 11:42 ` [PATCH v5 11/15] block-coroutine-wrapper.py: default to main loop aiocontext if function does not have a BlockDriverState parameter Emanuele Giuseppe Esposito
@ 2022-11-23 11:42 ` Emanuele Giuseppe Esposito
2022-11-23 17:09 ` Kevin Wolf
2022-11-23 11:42 ` [PATCH v5 13/15] block-coroutine-wrapper.py: support also basic return types Emanuele Giuseppe Esposito
` (2 subsequent siblings)
14 siblings, 1 reply; 28+ messages in thread
From: Emanuele Giuseppe Esposito @ 2022-11-23 11:42 UTC (permalink / raw)
To: qemu-block
Cc: Kevin Wolf, Hanna Reitz, John Snow, Vladimir Sementsov-Ogievskiy,
Eric Blake, Fam Zheng, Stefan Hajnoczi, Denis V. Lunev,
Stefan Weil, Jeff Cody, Cleber Rosa, qemu-devel,
Emanuele Giuseppe Esposito
Right now, we take the first parameter of the function to get the
BlockDriverState to pass to bdrv_poll_co(), that internally calls
functions that figure in which aiocontext the coroutine should run.
However, it is useless to pass a bs just to get its own AioContext,
so instead pass it directly, and default to the main loop if no
BlockDriverState is passed as parameter.
Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
block/block-gen.h | 6 +++---
scripts/block-coroutine-wrapper.py | 14 +++++++-------
2 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/block/block-gen.h b/block/block-gen.h
index f80cf4897d..08d977f493 100644
--- a/block/block-gen.h
+++ b/block/block-gen.h
@@ -30,7 +30,7 @@
/* Base structure for argument packing structures */
typedef struct BdrvPollCo {
- BlockDriverState *bs;
+ AioContext *ctx;
bool in_progress;
int ret;
Coroutine *co; /* Keep pointer here for debugging */
@@ -40,8 +40,8 @@ static inline int bdrv_poll_co(BdrvPollCo *s)
{
assert(!qemu_in_coroutine());
- bdrv_coroutine_enter(s->bs, s->co);
- BDRV_POLL_WHILE(s->bs, s->in_progress);
+ aio_co_enter(s->ctx, s->co);
+ AIO_WAIT_WHILE(s->ctx, s->in_progress);
return s->ret;
}
diff --git a/scripts/block-coroutine-wrapper.py b/scripts/block-coroutine-wrapper.py
index 7e8f2da84b..1d552cb734 100644
--- a/scripts/block-coroutine-wrapper.py
+++ b/scripts/block-coroutine-wrapper.py
@@ -78,14 +78,14 @@ def __init__(self, return_type: str, name: str, args: str,
t = self.args[0].type
if t == 'BlockDriverState *':
- bs = 'bs'
+ ctx = 'bdrv_get_aio_context(bs)'
elif t == 'BdrvChild *':
- bs = 'child->bs'
+ ctx = 'bdrv_get_aio_context(child->bs)'
elif t == 'BlockBackend *':
- bs = 'blk_bs(blk)'
+ ctx = 'bdrv_get_aio_context(blk_bs(blk))'
else:
- bs = 'NULL'
- self.bs = bs
+ ctx = 'qemu_get_aio_context()'
+ self.ctx = ctx
def gen_list(self, format: str) -> str:
return ', '.join(format.format_map(arg.__dict__) for arg in self.args)
@@ -130,7 +130,7 @@ def create_g_c_w(func: FuncDecl) -> str:
return {name}({ func.gen_list('{name}') });
}} else {{
{struct_name} s = {{
- .poll_state.bs = {func.bs},
+ .poll_state.ctx = {func.ctx},
.poll_state.in_progress = true,
{ func.gen_block(' .{name} = {name},') }
@@ -151,7 +151,7 @@ def create_coroutine_only(func: FuncDecl) -> str:
int {func.name}({ func.gen_list('{decl}') })
{{
{struct_name} s = {{
- .poll_state.bs = {func.bs},
+ .poll_state.ctx = {func.ctx},
.poll_state.in_progress = true,
{ func.gen_block(' .{name} = {name},') }
--
2.31.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v5 13/15] block-coroutine-wrapper.py: support also basic return types
2022-11-23 11:42 [PATCH v5 00/15] Still more coroutine and various fixes in block layer Emanuele Giuseppe Esposito
` (11 preceding siblings ...)
2022-11-23 11:42 ` [PATCH v5 12/15] " Emanuele Giuseppe Esposito
@ 2022-11-23 11:42 ` Emanuele Giuseppe Esposito
2022-11-23 17:18 ` Kevin Wolf
2022-11-23 11:42 ` [PATCH v5 14/15] block: convert bdrv_create to generated_co_wrapper_simple Emanuele Giuseppe Esposito
2022-11-23 11:42 ` [PATCH v5 15/15] block/dirty-bitmap: convert coroutine-only functions " Emanuele Giuseppe Esposito
14 siblings, 1 reply; 28+ messages in thread
From: Emanuele Giuseppe Esposito @ 2022-11-23 11:42 UTC (permalink / raw)
To: qemu-block
Cc: Kevin Wolf, Hanna Reitz, John Snow, Vladimir Sementsov-Ogievskiy,
Eric Blake, Fam Zheng, Stefan Hajnoczi, Denis V. Lunev,
Stefan Weil, Jeff Cody, Cleber Rosa, qemu-devel,
Emanuele Giuseppe Esposito
Extend the regex to cover also return type, pointers included.
This implies that the value returned by the function cannot be
a simple "int" anymore, but the custom return type.
Therefore remove poll_state->ret and instead use a per-function
custom "ret" field.
Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
block/block-gen.h | 5 +----
scripts/block-coroutine-wrapper.py | 19 +++++++++++--------
2 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/block/block-gen.h b/block/block-gen.h
index 08d977f493..89b7daaa1f 100644
--- a/block/block-gen.h
+++ b/block/block-gen.h
@@ -32,18 +32,15 @@
typedef struct BdrvPollCo {
AioContext *ctx;
bool in_progress;
- int ret;
Coroutine *co; /* Keep pointer here for debugging */
} BdrvPollCo;
-static inline int bdrv_poll_co(BdrvPollCo *s)
+static inline void bdrv_poll_co(BdrvPollCo *s)
{
assert(!qemu_in_coroutine());
aio_co_enter(s->ctx, s->co);
AIO_WAIT_WHILE(s->ctx, s->in_progress);
-
- return s->ret;
}
#endif /* BLOCK_BLOCK_GEN_H */
diff --git a/scripts/block-coroutine-wrapper.py b/scripts/block-coroutine-wrapper.py
index 1d552cb734..ef19b71c2a 100644
--- a/scripts/block-coroutine-wrapper.py
+++ b/scripts/block-coroutine-wrapper.py
@@ -95,7 +95,8 @@ def gen_block(self, format: str) -> str:
# Match wrappers declared with a generated_co_wrapper mark
-func_decl_re = re.compile(r'^int\s*generated_co_wrapper'
+func_decl_re = re.compile(r'^(?P<return_type>[a-zA-Z][a-zA-Z0-9_]* [*]?)'
+ r'\s*generated_co_wrapper'
r'(?P<variant>(_[a-z][a-z0-9_]*)?)\s*'
r'(?P<wrapper_name>[a-z][a-z0-9_]*)'
r'\((?P<args>[^)]*)\);$', re.MULTILINE)
@@ -103,7 +104,7 @@ def gen_block(self, format: str) -> str:
def func_decl_iter(text: str) -> Iterator:
for m in func_decl_re.finditer(text):
- yield FuncDecl(return_type='int',
+ yield FuncDecl(return_type=m.group('return_type'),
name=m.group('wrapper_name'),
args=m.group('args'),
variant=m.group('variant'))
@@ -124,7 +125,7 @@ def create_g_c_w(func: FuncDecl) -> str:
name = func.co_name
struct_name = func.struct_name
return f"""\
-int {func.name}({ func.gen_list('{decl}') })
+{func.return_type} {func.name}({ func.gen_list('{decl}') })
{{
if (qemu_in_coroutine()) {{
return {name}({ func.gen_list('{name}') });
@@ -138,7 +139,8 @@ def create_g_c_w(func: FuncDecl) -> str:
s.poll_state.co = qemu_coroutine_create({name}_entry, &s);
- return bdrv_poll_co(&s.poll_state);
+ bdrv_poll_co(&s.poll_state);
+ return s.ret;
}}
}}"""
@@ -148,7 +150,7 @@ def create_coroutine_only(func: FuncDecl) -> str:
name = func.co_name
struct_name = func.struct_name
return f"""\
-int {func.name}({ func.gen_list('{decl}') })
+{func.return_type} {func.name}({ func.gen_list('{decl}') })
{{
{struct_name} s = {{
.poll_state.ctx = {func.ctx},
@@ -160,13 +162,13 @@ def create_coroutine_only(func: FuncDecl) -> str:
s.poll_state.co = qemu_coroutine_create({name}_entry, &s);
- return bdrv_poll_co(&s.poll_state);
+ bdrv_poll_co(&s.poll_state);
+ return s.ret;
}}"""
def gen_wrapper(func: FuncDecl) -> str:
assert not '_co_' in func.name
- assert func.return_type == 'int'
name = func.co_name
struct_name = func.struct_name
@@ -182,6 +184,7 @@ def gen_wrapper(func: FuncDecl) -> str:
typedef struct {struct_name} {{
BdrvPollCo poll_state;
+ {func.return_type} ret;
{ func.gen_block(' {decl};') }
}} {struct_name};
@@ -189,7 +192,7 @@ def gen_wrapper(func: FuncDecl) -> str:
{{
{struct_name} *s = opaque;
- s->poll_state.ret = {name}({ func.gen_list('s->{name}') });
+ s->ret = {name}({ func.gen_list('s->{name}') });
s->poll_state.in_progress = false;
aio_wait_kick();
--
2.31.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v5 14/15] block: convert bdrv_create to generated_co_wrapper_simple
2022-11-23 11:42 [PATCH v5 00/15] Still more coroutine and various fixes in block layer Emanuele Giuseppe Esposito
` (12 preceding siblings ...)
2022-11-23 11:42 ` [PATCH v5 13/15] block-coroutine-wrapper.py: support also basic return types Emanuele Giuseppe Esposito
@ 2022-11-23 11:42 ` Emanuele Giuseppe Esposito
2022-11-23 11:42 ` [PATCH v5 15/15] block/dirty-bitmap: convert coroutine-only functions " Emanuele Giuseppe Esposito
14 siblings, 0 replies; 28+ messages in thread
From: Emanuele Giuseppe Esposito @ 2022-11-23 11:42 UTC (permalink / raw)
To: qemu-block
Cc: Kevin Wolf, Hanna Reitz, John Snow, Vladimir Sementsov-Ogievskiy,
Eric Blake, Fam Zheng, Stefan Hajnoczi, Denis V. Lunev,
Stefan Weil, Jeff Cody, Cleber Rosa, qemu-devel,
Emanuele Giuseppe Esposito
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>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
---
block.c | 40 ++----------------------------
include/block/block-global-state.h | 10 ++++++--
2 files changed, 10 insertions(+), 40 deletions(-)
diff --git a/block.c b/block.c
index 8207f93353..5b4ba02d3a 100644
--- a/block.c
+++ b/block.c
@@ -528,8 +528,8 @@ typedef struct CreateCo {
Error *err;
} CreateCo;
-static int coroutine_fn bdrv_co_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 ret;
GLOBAL_STATE_CODE();
@@ -553,42 +553,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 387a7cbb2e..8f1c8ca910 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_co_create_file(const char *filename, QemuOpts *opts,
Error **errp);
--
2.31.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v5 15/15] block/dirty-bitmap: convert coroutine-only functions to generated_co_wrapper_simple
2022-11-23 11:42 [PATCH v5 00/15] Still more coroutine and various fixes in block layer Emanuele Giuseppe Esposito
` (13 preceding siblings ...)
2022-11-23 11:42 ` [PATCH v5 14/15] block: convert bdrv_create to generated_co_wrapper_simple Emanuele Giuseppe Esposito
@ 2022-11-23 11:42 ` Emanuele Giuseppe Esposito
14 siblings, 0 replies; 28+ messages in thread
From: Emanuele Giuseppe Esposito @ 2022-11-23 11:42 UTC (permalink / raw)
To: qemu-block
Cc: Kevin Wolf, Hanna Reitz, John Snow, Vladimir Sementsov-Ogievskiy,
Eric Blake, Fam Zheng, Stefan Hajnoczi, Denis V. Lunev,
Stefan Weil, Jeff Cody, Cleber Rosa, qemu-devel,
Emanuele Giuseppe Esposito
bdrv_can_store_new_dirty_bitmap and bdrv_remove_persistent_dirty_bitmap
check if they are running in a coroutine, directly calling the
coroutine callback if it's the case.
Except that no coroutine calls such functions, therefore that check
can be removed, and function creation can be offloaded to
g_c_w_simple.
Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
---
block/dirty-bitmap.c | 88 +-----------------------------------
block/meson.build | 1 +
include/block/block-common.h | 5 +-
include/block/block-io.h | 9 +++-
include/block/dirty-bitmap.h | 10 +++-
5 files changed, 21 insertions(+), 92 deletions(-)
diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index bf3dc0512a..21cf592889 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -388,7 +388,7 @@ void bdrv_release_named_dirty_bitmaps(BlockDriverState *bs)
* not fail.
* This function doesn't release corresponding BdrvDirtyBitmap.
*/
-static int coroutine_fn
+int coroutine_fn
bdrv_co_remove_persistent_dirty_bitmap(BlockDriverState *bs, const char *name,
Error **errp)
{
@@ -399,45 +399,6 @@ bdrv_co_remove_persistent_dirty_bitmap(BlockDriverState *bs, const char *name,
return 0;
}
-typedef struct BdrvRemovePersistentDirtyBitmapCo {
- BlockDriverState *bs;
- const char *name;
- Error **errp;
- int ret;
-} BdrvRemovePersistentDirtyBitmapCo;
-
-static void coroutine_fn
-bdrv_co_remove_persistent_dirty_bitmap_entry(void *opaque)
-{
- BdrvRemovePersistentDirtyBitmapCo *s = opaque;
-
- s->ret = bdrv_co_remove_persistent_dirty_bitmap(s->bs, s->name, s->errp);
- aio_wait_kick();
-}
-
-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;
- }
-}
-
bool
bdrv_supports_persistent_dirty_bitmap(BlockDriverState *bs)
{
@@ -447,7 +408,7 @@ bdrv_supports_persistent_dirty_bitmap(BlockDriverState *bs)
return false;
}
-static bool coroutine_fn
+bool coroutine_fn
bdrv_co_can_store_new_dirty_bitmap(BlockDriverState *bs, const char *name,
uint32_t granularity, Error **errp)
{
@@ -470,51 +431,6 @@ bdrv_co_can_store_new_dirty_bitmap(BlockDriverState *bs, const char *name,
return drv->bdrv_co_can_store_new_dirty_bitmap(bs, name, granularity, errp);
}
-typedef struct BdrvCanStoreNewDirtyBitmapCo {
- BlockDriverState *bs;
- const char *name;
- uint32_t granularity;
- Error **errp;
- bool ret;
-
- bool in_progress;
-} BdrvCanStoreNewDirtyBitmapCo;
-
-static void coroutine_fn bdrv_co_can_store_new_dirty_bitmap_entry(void *opaque)
-{
- BdrvCanStoreNewDirtyBitmapCo *s = opaque;
-
- s->ret = bdrv_co_can_store_new_dirty_bitmap(s->bs, s->name, s->granularity,
- s->errp);
- s->in_progress = false;
- aio_wait_kick();
-}
-
-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;
- }
-}
-
void bdrv_disable_dirty_bitmap(BdrvDirtyBitmap *bitmap)
{
bdrv_dirty_bitmaps_lock(bitmap->bs);
diff --git a/block/meson.build b/block/meson.build
index b7c68b83a3..c48a59571e 100644
--- a/block/meson.build
+++ b/block/meson.build
@@ -137,6 +137,7 @@ block_gen_c = custom_target('block-gen.c',
output: 'block-gen.c',
input: files(
'../include/block/block-io.h',
+ '../include/block/dirty-bitmap.h',
'../include/block/block-global-state.h',
'../include/sysemu/block-backend-io.h',
'coroutines.h'
diff --git a/include/block/block-common.h b/include/block/block-common.h
index 46b57501d8..ea5505e28f 100644
--- a/include/block/block-common.h
+++ b/include/block/block-common.h
@@ -29,8 +29,6 @@
#include "qemu/iov.h"
#include "qemu/coroutine.h"
#include "block/accounting.h"
-#include "block/dirty-bitmap.h"
-#include "block/blockjob.h"
#include "qemu/hbitmap.h"
#include "qemu/transactions.h"
@@ -56,6 +54,9 @@
#define generated_co_wrapper
#define generated_co_wrapper_simple
+#include "block/dirty-bitmap.h"
+#include "block/blockjob.h"
+
/* block.c */
typedef struct BlockDriver BlockDriver;
typedef struct BdrvChild BdrvChild;
diff --git a/include/block/block-io.h b/include/block/block-io.h
index 72919254cd..3c71329f01 100644
--- a/include/block/block-io.h
+++ b/include/block/block-io.h
@@ -215,8 +215,13 @@ AioContext *child_of_bds_get_parent_aio_context(BdrvChild *c);
void bdrv_io_plug(BlockDriverState *bs);
void bdrv_io_unplug(BlockDriverState *bs);
-bool bdrv_can_store_new_dirty_bitmap(BlockDriverState *bs, const char *name,
- uint32_t granularity, Error **errp);
+bool coroutine_fn bdrv_co_can_store_new_dirty_bitmap(BlockDriverState *bs,
+ const char *name,
+ uint32_t granularity,
+ Error **errp);
+bool generated_co_wrapper_simple
+bdrv_can_store_new_dirty_bitmap(BlockDriverState *bs, const char *name,
+ uint32_t granularity, Error **errp);
/**
*
diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
index 6528336c4c..2290e7fa28 100644
--- a/include/block/dirty-bitmap.h
+++ b/include/block/dirty-bitmap.h
@@ -34,8 +34,14 @@ int bdrv_dirty_bitmap_check(const BdrvDirtyBitmap *bitmap, uint32_t flags,
Error **errp);
void bdrv_release_dirty_bitmap(BdrvDirtyBitmap *bitmap);
void bdrv_release_named_dirty_bitmaps(BlockDriverState *bs);
-int bdrv_remove_persistent_dirty_bitmap(BlockDriverState *bs, const char *name,
- Error **errp);
+
+int coroutine_fn bdrv_co_remove_persistent_dirty_bitmap(BlockDriverState *bs,
+ const char *name,
+ Error **errp);
+int generated_co_wrapper_simple
+bdrv_remove_persistent_dirty_bitmap(BlockDriverState *bs, const char *name,
+ Error **errp);
+
void bdrv_disable_dirty_bitmap(BdrvDirtyBitmap *bitmap);
void bdrv_enable_dirty_bitmap(BdrvDirtyBitmap *bitmap);
void bdrv_enable_dirty_bitmap_locked(BdrvDirtyBitmap *bitmap);
--
2.31.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH v5 01/15] block-io: introduce coroutine_fn duplicates for bdrv_common_block_status_above callers
2022-11-23 11:42 ` [PATCH v5 01/15] block-io: introduce coroutine_fn duplicates for bdrv_common_block_status_above callers Emanuele Giuseppe Esposito
@ 2022-11-23 16:29 ` Kevin Wolf
2022-11-24 7:04 ` Paolo Bonzini
0 siblings, 1 reply; 28+ messages in thread
From: Kevin Wolf @ 2022-11-23 16:29 UTC (permalink / raw)
To: Emanuele Giuseppe Esposito
Cc: qemu-block, Hanna Reitz, John Snow, Vladimir Sementsov-Ogievskiy,
Eric Blake, Fam Zheng, Stefan Hajnoczi, Denis V. Lunev,
Stefan Weil, Jeff Cody, Cleber Rosa, qemu-devel, pbonzini
Am 23.11.2022 um 12:42 hat Emanuele Giuseppe Esposito geschrieben:
> bdrv_common_block_status_above() is a g_c_w, and it is being called by
> many "wrapper" functions like bdrv_is_allocated(),
> bdrv_is_allocated_above() and bdrv_block_status_above().
>
> Because we want to eventually split the coroutine from non-coroutine
> case in g_c_w, create duplicate wrappers that take care of directly
> calling the same coroutine functions called in the g_c_w.
>
> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
> ---
> block/io.c | 64 ++++++++++++++++++++++++++++++++++++++--
> include/block/block-io.h | 15 ++++++++++
> 2 files changed, 76 insertions(+), 3 deletions(-)
>
> diff --git a/block/io.c b/block/io.c
> index 38e57d1f67..1bc05c8282 100644
> --- a/block/io.c
> +++ b/block/io.c
> @@ -2533,6 +2533,19 @@ bdrv_co_common_block_status_above(BlockDriverState *bs,
> return ret;
> }
>
> +int coroutine_fn bdrv_co_block_status_above(BlockDriverState *bs,
> + BlockDriverState *base,
> + int64_t offset, int64_t bytes,
> + int64_t *pnum, int64_t *map,
> + BlockDriverState **file)
> +{
> + IO_CODE();
> + /* If QEMU_IN_COROUTINE() fails, use bdrv_block_status_above() */
> + QEMU_IN_COROUTINE();
This is an obvious patch order problem: The macro doesn't even exist
yet.
As I said, personally, I don't feel like putting QEMU_IN_COROUTINE()
assertions into every coroutine_fn is a useful thing to do. Static
analysis (maybe even with something vrc based in 'make check'? Paolo,
would this be realistic?) seems much preferable. But I'd like to hear
other opinions on this, too.
I feel the same way about the comment. Yes, of course, if you're not in
a coroutine, don't call the _co variant of a function, but the one
without it. But that goes without saying, doesn't it?
> + return bdrv_co_common_block_status_above(bs, base, false, true, offset,
> + bytes, pnum, map, file, NULL);
> +}
Apart from these considerations the patch looks right.
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v5 02/15] block-copy: add missing coroutine_fn annotations
2022-11-23 11:42 ` [PATCH v5 02/15] block-copy: add missing coroutine_fn annotations Emanuele Giuseppe Esposito
@ 2022-11-23 16:32 ` Kevin Wolf
0 siblings, 0 replies; 28+ messages in thread
From: Kevin Wolf @ 2022-11-23 16:32 UTC (permalink / raw)
To: Emanuele Giuseppe Esposito
Cc: qemu-block, Hanna Reitz, John Snow, Vladimir Sementsov-Ogievskiy,
Eric Blake, Fam Zheng, Stefan Hajnoczi, Denis V. Lunev,
Stefan Weil, Jeff Cody, Cleber Rosa, qemu-devel, Paolo Bonzini
Am 23.11.2022 um 12:42 hat Emanuele Giuseppe Esposito geschrieben:
> These functions end up calling bdrv_common_block_status_above(), a
> generated_co_wrapper function.
> In addition, they also happen to be always called in coroutine context,
> meaning all callers are coroutine_fn.
> This means that the g_c_w function will enter the qemu_in_coroutine()
> case and eventually suspend (or in other words call qemu_coroutine_yield()).
> Therefore we need to mark such functions coroutine_fn too.
>
> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v5 04/15] block-backend: replace bdrv_*_above with blk_*_above
2022-11-23 11:42 ` [PATCH v5 04/15] block-backend: replace bdrv_*_above with blk_*_above Emanuele Giuseppe Esposito
@ 2022-11-23 16:42 ` Kevin Wolf
0 siblings, 0 replies; 28+ messages in thread
From: Kevin Wolf @ 2022-11-23 16:42 UTC (permalink / raw)
To: Emanuele Giuseppe Esposito
Cc: qemu-block, Hanna Reitz, John Snow, Vladimir Sementsov-Ogievskiy,
Eric Blake, Fam Zheng, Stefan Hajnoczi, Denis V. Lunev,
Stefan Weil, Jeff Cody, Cleber Rosa, qemu-devel, Paolo Bonzini
Am 23.11.2022 um 12:42 hat Emanuele Giuseppe Esposito geschrieben:
> Avoid mixing bdrv_* functions with blk_*, so create blk_* counterparts
> for bdrv_block_status_above and bdrv_is_allocated_above.
>
> Note that since bdrv_block_status_above only calls the g_c_w function
> bdrv_common_block_status_above and is marked as coroutine_fn, call
> directly bdrv_co_common_block_status_above() to avoid using a g_c_w.
This sentence is a bit confusing, because it talks about the blk_* side
of things, but doesn't mention it at all.
The same argument is true for is_allocated.
> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> block/block-backend.c | 21 ++++++++++++++++++++
> block/commit.c | 4 ++--
> include/sysemu/block-backend-io.h | 9 +++++++++
> nbd/server.c | 32 +++++++++++++++----------------
> 4 files changed, 48 insertions(+), 18 deletions(-)
>
> diff --git a/block/block-backend.c b/block/block-backend.c
> index 742efa7955..03bed68e4f 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_co_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_co_block_status_above(blk_bs(blk), base, offset, bytes, pnum,
> + map, file);
> +}
> +
> +int coroutine_fn blk_is_allocated_above(BlockBackend *blk,
Any reason why you renamed only blk_co_block_status_above(), but not
this one into blk_co_is_allocated_above()?
> + BlockDriverState *base,
> + bool include_base, int64_t offset,
> + int64_t bytes, int64_t *pnum)
> +{
> + IO_CODE();
> + return bdrv_co_is_allocated_above(blk_bs(blk), base, include_base, offset,
> + bytes, pnum);
> +}
Kevin
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v5 06/15] block: avoid duplicating filename string in bdrv_create
2022-11-23 11:42 ` [PATCH v5 06/15] block: avoid duplicating filename string in bdrv_create Emanuele Giuseppe Esposito
@ 2022-11-23 16:45 ` Kevin Wolf
0 siblings, 0 replies; 28+ messages in thread
From: Kevin Wolf @ 2022-11-23 16:45 UTC (permalink / raw)
To: Emanuele Giuseppe Esposito
Cc: qemu-block, Hanna Reitz, John Snow, Vladimir Sementsov-Ogievskiy,
Eric Blake, Fam Zheng, Stefan Hajnoczi, Denis V. Lunev,
Stefan Weil, Jeff Cody, Cleber Rosa, qemu-devel
Am 23.11.2022 um 12:42 hat Emanuele Giuseppe Esposito geschrieben:
> We know that the string will stay around until the function
> returns, and the parameter of drv->bdrv_co_create_opts is const char*,
> so it must not be modified either.
>
> Suggested-by: Kevin Wolf <kwolf@redhat.com>
> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v5 07/15] block: introduce QEMU_IN_COROUTINE macro
2022-11-23 11:42 ` [PATCH v5 07/15] block: introduce QEMU_IN_COROUTINE macro Emanuele Giuseppe Esposito
@ 2022-11-23 16:49 ` Kevin Wolf
2022-11-24 7:01 ` Paolo Bonzini
0 siblings, 1 reply; 28+ messages in thread
From: Kevin Wolf @ 2022-11-23 16:49 UTC (permalink / raw)
To: Emanuele Giuseppe Esposito
Cc: qemu-block, Hanna Reitz, John Snow, Vladimir Sementsov-Ogievskiy,
Eric Blake, Fam Zheng, Stefan Hajnoczi, Denis V. Lunev,
Stefan Weil, Jeff Cody, Cleber Rosa, qemu-devel, pbonzini
Am 23.11.2022 um 12:42 hat Emanuele Giuseppe Esposito geschrieben:
> This macro will be used to mark all coroutine_fn functions.
> Right now, it will be used for the newly introduced coroutine_fn, since
> we know the callers.
>
> As a TODO, in the future we might want to add this macro to all
> corotuine_fn functions, to be sure that they are only called in
s/corotuine_fn/coroutine_fn/
> coroutines context.
>
> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
I already asked about other opinions on this in patch 1.
These assertions are runtime checks and I don't feel they are the right
tool to verify coroutine_fn consistency. Asserting in tricky places
makes sense to me, especially as long as we can't rely on static
analysis, but adding it everywhere feels over the top to me.
Kevin
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v5 08/15] block: distinguish between bdrv_create running in coroutine and not
2022-11-23 11:42 ` [PATCH v5 08/15] block: distinguish between bdrv_create running in coroutine and not Emanuele Giuseppe Esposito
@ 2022-11-23 16:55 ` Kevin Wolf
0 siblings, 0 replies; 28+ messages in thread
From: Kevin Wolf @ 2022-11-23 16:55 UTC (permalink / raw)
To: Emanuele Giuseppe Esposito
Cc: qemu-block, Hanna Reitz, John Snow, Vladimir Sementsov-Ogievskiy,
Eric Blake, Fam Zheng, Stefan Hajnoczi, Denis V. Lunev,
Stefan Weil, Jeff Cody, Cleber Rosa, qemu-devel
Am 23.11.2022 um 12:42 hat Emanuele Giuseppe Esposito geschrieben:
> 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().
>
> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v5 09/15] block: bdrv_create_file is a coroutine_fn
2022-11-23 11:42 ` [PATCH v5 09/15] block: bdrv_create_file is a coroutine_fn Emanuele Giuseppe Esposito
@ 2022-11-23 16:57 ` Kevin Wolf
0 siblings, 0 replies; 28+ messages in thread
From: Kevin Wolf @ 2022-11-23 16:57 UTC (permalink / raw)
To: Emanuele Giuseppe Esposito
Cc: qemu-block, Hanna Reitz, John Snow, Vladimir Sementsov-Ogievskiy,
Eric Blake, Fam Zheng, Stefan Hajnoczi, Denis V. Lunev,
Stefan Weil, Jeff Cody, Cleber Rosa, qemu-devel
Am 23.11.2022 um 12:42 hat Emanuele Giuseppe Esposito geschrieben:
> It is always called in coroutine_fn callbacks, therefore
> it can directly call bdrv_co_create().
>
> Rename it to bdrv_co_create_file too.
>
> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v5 10/15] block-coroutine-wrapper.py: introduce generated_co_wrapper_simple
2022-11-23 11:42 ` [PATCH v5 10/15] block-coroutine-wrapper.py: introduce generated_co_wrapper_simple Emanuele Giuseppe Esposito
@ 2022-11-23 17:07 ` Kevin Wolf
0 siblings, 0 replies; 28+ messages in thread
From: Kevin Wolf @ 2022-11-23 17:07 UTC (permalink / raw)
To: Emanuele Giuseppe Esposito
Cc: qemu-block, Hanna Reitz, John Snow, Vladimir Sementsov-Ogievskiy,
Eric Blake, Fam Zheng, Stefan Hajnoczi, Denis V. Lunev,
Stefan Weil, Jeff Cody, Cleber Rosa, qemu-devel
Am 23.11.2022 um 12:42 hat Emanuele Giuseppe Esposito geschrieben:
> This new annotation creates just a function wrapper that creates
> a new coroutine. It assumes the caller is not a coroutine.
>
> This is much better as g_c_w, because it is clear if the caller
> is a coroutine or not, and provides the advantage of automating
> the code creation. In the future all g_c_w functions will be
> substituted on g_c_w_simple.
>
> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v5 12/15] block-coroutine-wrapper.py: default to main loop aiocontext if function does not have a BlockDriverState parameter
2022-11-23 11:42 ` [PATCH v5 12/15] " Emanuele Giuseppe Esposito
@ 2022-11-23 17:09 ` Kevin Wolf
0 siblings, 0 replies; 28+ messages in thread
From: Kevin Wolf @ 2022-11-23 17:09 UTC (permalink / raw)
To: Emanuele Giuseppe Esposito
Cc: qemu-block, Hanna Reitz, John Snow, Vladimir Sementsov-Ogievskiy,
Eric Blake, Fam Zheng, Stefan Hajnoczi, Denis V. Lunev,
Stefan Weil, Jeff Cody, Cleber Rosa, qemu-devel
Am 23.11.2022 um 12:42 hat Emanuele Giuseppe Esposito geschrieben:
> Right now, we take the first parameter of the function to get the
> BlockDriverState to pass to bdrv_poll_co(), that internally calls
> functions that figure in which aiocontext the coroutine should run.
>
> However, it is useless to pass a bs just to get its own AioContext,
> so instead pass it directly, and default to the main loop if no
> BlockDriverState is passed as parameter.
>
> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
Patch 10 and 11 have the same subject line. Did you intend them to be
squashed together?
> diff --git a/scripts/block-coroutine-wrapper.py b/scripts/block-coroutine-wrapper.py
> index 7e8f2da84b..1d552cb734 100644
> --- a/scripts/block-coroutine-wrapper.py
> +++ b/scripts/block-coroutine-wrapper.py
> @@ -78,14 +78,14 @@ def __init__(self, return_type: str, name: str, args: str,
>
> t = self.args[0].type
> if t == 'BlockDriverState *':
> - bs = 'bs'
> + ctx = 'bdrv_get_aio_context(bs)'
> elif t == 'BdrvChild *':
> - bs = 'child->bs'
> + ctx = 'bdrv_get_aio_context(child->bs)'
> elif t == 'BlockBackend *':
> - bs = 'blk_bs(blk)'
> + ctx = 'bdrv_get_aio_context(blk_bs(blk))'
This should use blk_get_aio_context(). If a BlockBackend has no root
attached, i.e. blk_bs(blk) == NULL, it can still be in a non-mainloop
AioContext.
Kevin
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v5 13/15] block-coroutine-wrapper.py: support also basic return types
2022-11-23 11:42 ` [PATCH v5 13/15] block-coroutine-wrapper.py: support also basic return types Emanuele Giuseppe Esposito
@ 2022-11-23 17:18 ` Kevin Wolf
0 siblings, 0 replies; 28+ messages in thread
From: Kevin Wolf @ 2022-11-23 17:18 UTC (permalink / raw)
To: Emanuele Giuseppe Esposito
Cc: qemu-block, Hanna Reitz, John Snow, Vladimir Sementsov-Ogievskiy,
Eric Blake, Fam Zheng, Stefan Hajnoczi, Denis V. Lunev,
Stefan Weil, Jeff Cody, Cleber Rosa, qemu-devel
Am 23.11.2022 um 12:42 hat Emanuele Giuseppe Esposito geschrieben:
> Extend the regex to cover also return type, pointers included.
> This implies that the value returned by the function cannot be
> a simple "int" anymore, but the custom return type.
> Therefore remove poll_state->ret and instead use a per-function
> custom "ret" field.
>
> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
This looks unchanged compared to v4 where I already gave:
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v5 07/15] block: introduce QEMU_IN_COROUTINE macro
2022-11-23 16:49 ` Kevin Wolf
@ 2022-11-24 7:01 ` Paolo Bonzini
0 siblings, 0 replies; 28+ messages in thread
From: Paolo Bonzini @ 2022-11-24 7:01 UTC (permalink / raw)
To: Kevin Wolf
Cc: Emanuele Giuseppe Esposito, open list:Block layer core,
Hanna Reitz, John Snow, Vladimir Sementsov-Ogievskiy, Eric Blake,
Fam Zheng, Stefan Hajnoczi, Denis V. Lunev, Stefan Weil,
Jeff Cody, Cleber Rosa, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 516 bytes --]
Il mer 23 nov 2022, 17:49 Kevin Wolf <kwolf@redhat.com> ha scritto:
> I already asked about other opinions on this in patch 1.
>
> These assertions are runtime checks and I don't feel they are the right
> tool to verify coroutine_fn consistency. Asserting in tricky places
> makes sense to me, especially as long as we can't rely on static
> analysis, but adding it everywhere feels over the top to me.
>
I agree that they don't seem necessary, since static analysis is possible
and superior.
Paolo
> Kevin
>
>
[-- Attachment #2: Type: text/html, Size: 1115 bytes --]
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v5 01/15] block-io: introduce coroutine_fn duplicates for bdrv_common_block_status_above callers
2022-11-23 16:29 ` Kevin Wolf
@ 2022-11-24 7:04 ` Paolo Bonzini
0 siblings, 0 replies; 28+ messages in thread
From: Paolo Bonzini @ 2022-11-24 7:04 UTC (permalink / raw)
To: Kevin Wolf
Cc: Emanuele Giuseppe Esposito, open list:Block layer core,
Hanna Reitz, John Snow, Vladimir Sementsov-Ogievskiy, Eric Blake,
Fam Zheng, Stefan Hajnoczi, Denis V. Lunev, Stefan Weil,
Jeff Cody, Cleber Rosa, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 650 bytes --]
Il mer 23 nov 2022, 17:29 Kevin Wolf <kwolf@redhat.com> ha scritto:
> As I said, personally, I don't feel like putting QEMU_IN_COROUTINE()
> assertions into every coroutine_fn is a useful thing to do. Static
> analysis (maybe even with something vrc based in 'make check'? Paolo,
> would this be realistic?)
Yes, using pyclang just to build the call graph should be much much faster
than doing all the static analysis. So it should be feasible to integrate
it with "make check". We can decide whether to bundle vrc, which is just a
dozen files, or install it from pypi (sooner or later configure will have
to be taught about that, too).
Paolo
>
[-- Attachment #2: Type: text/html, Size: 1233 bytes --]
^ permalink raw reply [flat|nested] 28+ messages in thread
end of thread, other threads:[~2022-11-24 7:05 UTC | newest]
Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-23 11:42 [PATCH v5 00/15] Still more coroutine and various fixes in block layer Emanuele Giuseppe Esposito
2022-11-23 11:42 ` [PATCH v5 01/15] block-io: introduce coroutine_fn duplicates for bdrv_common_block_status_above callers Emanuele Giuseppe Esposito
2022-11-23 16:29 ` Kevin Wolf
2022-11-24 7:04 ` Paolo Bonzini
2022-11-23 11:42 ` [PATCH v5 02/15] block-copy: add missing coroutine_fn annotations Emanuele Giuseppe Esposito
2022-11-23 16:32 ` Kevin Wolf
2022-11-23 11:42 ` [PATCH v5 03/15] nbd/server.c: " Emanuele Giuseppe Esposito
2022-11-23 11:42 ` [PATCH v5 04/15] block-backend: replace bdrv_*_above with blk_*_above Emanuele Giuseppe Esposito
2022-11-23 16:42 ` Kevin Wolf
2022-11-23 11:42 ` [PATCH v5 05/15] block/vmdk: add missing coroutine_fn annotations Emanuele Giuseppe Esposito
2022-11-23 11:42 ` [PATCH v5 06/15] block: avoid duplicating filename string in bdrv_create Emanuele Giuseppe Esposito
2022-11-23 16:45 ` Kevin Wolf
2022-11-23 11:42 ` [PATCH v5 07/15] block: introduce QEMU_IN_COROUTINE macro Emanuele Giuseppe Esposito
2022-11-23 16:49 ` Kevin Wolf
2022-11-24 7:01 ` Paolo Bonzini
2022-11-23 11:42 ` [PATCH v5 08/15] block: distinguish between bdrv_create running in coroutine and not Emanuele Giuseppe Esposito
2022-11-23 16:55 ` Kevin Wolf
2022-11-23 11:42 ` [PATCH v5 09/15] block: bdrv_create_file is a coroutine_fn Emanuele Giuseppe Esposito
2022-11-23 16:57 ` Kevin Wolf
2022-11-23 11:42 ` [PATCH v5 10/15] block-coroutine-wrapper.py: introduce generated_co_wrapper_simple Emanuele Giuseppe Esposito
2022-11-23 17:07 ` Kevin Wolf
2022-11-23 11:42 ` [PATCH v5 11/15] block-coroutine-wrapper.py: default to main loop aiocontext if function does not have a BlockDriverState parameter Emanuele Giuseppe Esposito
2022-11-23 11:42 ` [PATCH v5 12/15] " Emanuele Giuseppe Esposito
2022-11-23 17:09 ` Kevin Wolf
2022-11-23 11:42 ` [PATCH v5 13/15] block-coroutine-wrapper.py: support also basic return types Emanuele Giuseppe Esposito
2022-11-23 17:18 ` Kevin Wolf
2022-11-23 11:42 ` [PATCH v5 14/15] block: convert bdrv_create to generated_co_wrapper_simple Emanuele Giuseppe Esposito
2022-11-23 11:42 ` [PATCH v5 15/15] block/dirty-bitmap: convert coroutine-only functions " Emanuele Giuseppe Esposito
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.