On 5/12/20 9:43 AM, Kevin Wolf wrote:
We have a few bdrv_*() functions that can either spawn a new coroutine
and wait for it with BDRV_POLL_WHILE() or use a fastpath if they are
alreeady running in a coroutine. All of them duplicate basically the
already
same code.
Factor the common code into a new function bdrv_run_co().
Signed-off-by: Kevin Wolf <address@hidden>
---
block/io.c | 104 +++++++++++++++--------------------------------------
1 file changed, 28 insertions(+), 76 deletions(-)
diff --git a/block/io.c b/block/io.c
index 7d30e61edc..c1badaadc9 100644
--- a/block/io.c
+++ b/block/io.c
@@ -891,6 +891,22 @@ static int
bdrv_check_byte_request(BlockDriverState *bs, int64_t offset,
return 0;
}
+static int bdrv_run_co(BlockDriverState *bs, CoroutineEntry *entry,
+ void *opaque, int *ret)
+{
+ if (qemu_in_coroutine()) {
+ /* Fast-path if already in coroutine context */
+ entry(opaque);
+ } else {
+ Coroutine *co = qemu_coroutine_create(entry, opaque);
+ *ret = NOT_DONE;
+ bdrv_coroutine_enter(bs, co);
+ BDRV_POLL_WHILE(bs, *ret == NOT_DONE);
For my reference, NOT_DONE is defined as INT_MAX, which does not seem to
be used as a return value in other situations.
@@ -923,25 +939,15 @@ static int bdrv_prwv_co(BdrvChild *child,
int64_t offset,
QEMUIOVector *qiov, bool is_write,
BdrvRequestFlags flags)
{
- Coroutine *co;
RwCo rwco = {
.child = child,
.offset = offset,
.qiov = qiov,
.is_write = is_write,
- .ret = NOT_DONE,
.flags = flags,
};
- if (qemu_in_coroutine()) {
- /* Fast-path if already in coroutine context */
- bdrv_rw_co_entry(&rwco);
- } else {
- co = qemu_coroutine_create(bdrv_rw_co_entry, &rwco);
- bdrv_coroutine_enter(child->bs, co);
- BDRV_POLL_WHILE(child->bs, rwco.ret == NOT_DONE);
- }
- return rwco.ret;
+ return bdrv_run_co(child->bs, bdrv_rw_co_entry, &rwco, &rwco.ret);
So code that previously looped on NOT_DONE is obviously safe, while...
}
int bdrv_pwrite_zeroes(BdrvChild *child, int64_t offset,
@@ -2230,7 +2236,6 @@ typedef struct BdrvCoBlockStatusData {
int64_t *map;
BlockDriverState **file;
int ret;
- bool done;
} BdrvCoBlockStatusData;
int coroutine_fn bdrv_co_block_status_from_file(BlockDriverState *bs,
@@ -2492,7 +2497,6 @@ static void coroutine_fn
bdrv_block_status_above_co_entry(void *opaque)
data->want_zero,
data->offset, data->bytes,
data->pnum, data->map,
data->file);
- data->done = true;
aio_wait_kick();
...code that looped on something else now has to be checked that
data->ret is still being set to something useful. Fortunately that is
true here.
@@ -2669,22 +2663,13 @@ static inline int
bdrv_rw_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos,
bool is_read)
{
- if (qemu_in_coroutine()) {
- return bdrv_co_rw_vmstate(bs, qiov, pos, is_read);
- } else {
- BdrvVmstateCo data = {
- .bs = bs,
- .qiov = qiov,
- .pos = pos,
- .is_read = is_read,
- .ret = -EINPROGRESS,
- };
- Coroutine *co =
qemu_coroutine_create(bdrv_co_rw_vmstate_entry, &data);
-
- bdrv_coroutine_enter(bs, co);
- BDRV_POLL_WHILE(bs, data.ret == -EINPROGRESS);
- return data.ret;
It's a little harder to see whether -EINPROGRESS might ever be returned
by a driver, but again this looks safe.