[PATCH 00/11] qemu-io-cmds: move to coroutine

[PATCH 00/11] qemu-io-cmds: move to coroutine

[Vladimir Sementsov-Ogievskiy]

Hi!

I've done this by accident. I decided that it is needed to make a nice
fix for a problem with aio context locking in hmp_qemu_io(). Still, the
problem is fixed without this series, so it's based on on the fix
instead of being preparation for it:

Based-on: <20210423134233.51495-1-vsementsov@virtuozzo.com>
  "[PATCH v2] monitor: hmp_qemu_io: acquire aio contex, fix crash"

When I understood this, the series was more than half-done, so I decided
to finish it. I think it's a good refactoring. And moving block-layer
things to coroutine is good in general: we reduce number of polling
loops here and there.

Vladimir Sementsov-Ogievskiy (11):
  block-coroutine-wrapper: allow non bdrv_ prefix
  block-coroutine-wrapper: support BlockBackend first argument
  block/block-gen.h: bind monitor
  block: introduce bdrv_debug_wait_break
  qemu-io-cmds: move qemu-io commands to coroutine
  block: drop unused bdrv_debug_is_suspended()
  block-backend: add _co_ versions of blk_save_vmstate /
    blk_load_vmstate
  qemu-io-cmds: refactor read_f(): drop extra helpers and variables
  qemu-io-cmds: refactor write_f(): drop extra helpers and variables
  qemu-io-cmds: drop do_co_readv() and do_co_writev() helpers
  block-backend: drop unused blk_save_vmstate() and blk_load_vmstate()

 block/block-gen.h                  |  22 ++-
 include/block/block.h              |   2 +-
 include/block/block_int.h          |   2 +-
 include/qemu-io.h                  |   9 +-
 include/sysemu/block-backend.h     |   6 +-
 block.c                            |  10 +-
 block/blkdebug.c                   |  17 ++-
 block/block-backend.c              |  21 ++-
 qemu-io-cmds.c                     | 237 ++++-------------------------
 block/meson.build                  |   3 +-
 scripts/block-coroutine-wrapper.py |  30 +++-
 11 files changed, 113 insertions(+), 246 deletions(-)

-- 
2.29.2

[PATCH 01/11] block-coroutine-wrapper: allow non bdrv_ prefix

[Vladimir Sementsov-Ogievskiy]

We are going to reuse the script to generate a qcow2_ function in
further commit. Prepare the script now.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 scripts/block-coroutine-wrapper.py | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/scripts/block-coroutine-wrapper.py b/scripts/block-coroutine-wrapper.py
index 0461fd1c45..85dbeb9ecf 100644
--- a/scripts/block-coroutine-wrapper.py
+++ b/scripts/block-coroutine-wrapper.py
@@ -98,12 +98,13 @@ def snake_to_camel(func_name: str) -> str:
 
 
 def gen_wrapper(func: FuncDecl) -> str:
-    assert func.name.startswith('bdrv_')
-    assert not func.name.startswith('bdrv_co_')
+    assert not '_co_' in func.name
     assert func.return_type == 'int'
     assert func.args[0].type in ['BlockDriverState *', 'BdrvChild *']
 
-    name = 'bdrv_co_' + func.name[5:]
+    subsystem, subname = func.name.split('_', 1)
+
+    name = f'{subsystem}_co_{subname}'
     bs = 'bs' if func.args[0].type == 'BlockDriverState *' else 'child->bs'
     struct_name = snake_to_camel(name)
 
-- 
2.29.2

[PATCH 02/11] block-coroutine-wrapper: support BlockBackend first argument

[Vladimir Sementsov-Ogievskiy]

We'll need to wrap functions with first argument of BlockBackend *
type. For this let's generalize core function and struct to work with
pure AioContext.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 block/block-gen.h                  | 12 ++++++------
 scripts/block-coroutine-wrapper.py | 23 ++++++++++++++++++-----
 2 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/block/block-gen.h b/block/block-gen.h
index f80cf4897d..c1fd3f40de 100644
--- a/block/block-gen.h
+++ b/block/block-gen.h
@@ -29,19 +29,19 @@
 #include "block/block_int.h"
 
 /* Base structure for argument packing structures */
-typedef struct BdrvPollCo {
-    BlockDriverState *bs;
+typedef struct AioPollCo {
+    AioContext *ctx;
     bool in_progress;
     int ret;
     Coroutine *co; /* Keep pointer here for debugging */
-} BdrvPollCo;
+} AioPollCo;
 
-static inline int bdrv_poll_co(BdrvPollCo *s)
+static inline int aio_poll_co(AioPollCo *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 85dbeb9ecf..114a54fcce 100644
--- a/scripts/block-coroutine-wrapper.py
+++ b/scripts/block-coroutine-wrapper.py
@@ -42,6 +42,8 @@ def gen_header():
 #include "qemu/osdep.h"
 #include "block/coroutines.h"
 #include "block/block-gen.h"
+#include "qemu-io.h"
+#include "sysemu/block-backend.h"
 #include "block/block_int.h"\
 """
 
@@ -100,12 +102,23 @@ def snake_to_camel(func_name: str) -> 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 *']
+    assert func.args[0].type in ['BlockDriverState *', 'BdrvChild *',
+                                 'BlockBackend *']
 
     subsystem, subname = func.name.split('_', 1)
 
     name = f'{subsystem}_co_{subname}'
-    bs = 'bs' if func.args[0].type == 'BlockDriverState *' else 'child->bs'
+
+    first_arg_type = func.args[0].type
+    if first_arg_type == 'BlockDriverState *':
+        ctx = 'bdrv_get_aio_context(bs)'
+    elif first_arg_type == 'BdrvChild *':
+        ctx = '(child ? bdrv_get_aio_context(child->bs) : ' \
+            'qemu_get_aio_context())'
+    else:
+        assert first_arg_type == 'BlockBackend *'
+        ctx = '(blk ? blk_get_aio_context(blk) : qemu_get_aio_context())'
+
     struct_name = snake_to_camel(name)
 
     return f"""\
@@ -114,7 +127,7 @@ def gen_wrapper(func: FuncDecl) -> str:
  */
 
 typedef struct {struct_name} {{
-    BdrvPollCo poll_state;
+    AioPollCo poll_state;
 { func.gen_block('    {decl};') }
 }} {struct_name};
 
@@ -134,7 +147,7 @@ def gen_wrapper(func: FuncDecl) -> str:
         return {name}({ func.gen_list('{name}') });
     }} else {{
         {struct_name} s = {{
-            .poll_state.bs = {bs},
+            .poll_state.ctx = {ctx},
             .poll_state.in_progress = true,
 
 { func.gen_block('            .{name} = {name},') }
@@ -142,7 +155,7 @@ def gen_wrapper(func: FuncDecl) -> str:
 
         s.poll_state.co = qemu_coroutine_create({name}_entry, &s);
 
-        return bdrv_poll_co(&s.poll_state);
+        return aio_poll_co(&s.poll_state);
     }}
 }}"""
 
-- 
2.29.2

[PATCH 03/11] block/block-gen.h: bind monitor

[Vladimir Sementsov-Ogievskiy]

If we have current monitor, let's bind it to wrapper coroutine too.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 block/block-gen.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/block/block-gen.h b/block/block-gen.h
index c1fd3f40de..61f055a8cc 100644
--- a/block/block-gen.h
+++ b/block/block-gen.h
@@ -27,6 +27,7 @@
 #define BLOCK_BLOCK_GEN_H
 
 #include "block/block_int.h"
+#include "monitor/monitor.h"
 
 /* Base structure for argument packing structures */
 typedef struct AioPollCo {
@@ -38,11 +39,20 @@ typedef struct AioPollCo {
 
 static inline int aio_poll_co(AioPollCo *s)
 {
+    Monitor *mon = monitor_cur();
     assert(!qemu_in_coroutine());
 
+    if (mon) {
+        monitor_set_cur(s->co, mon);
+    }
+
     aio_co_enter(s->ctx, s->co);
     AIO_WAIT_WHILE(s->ctx, s->in_progress);
 
+    if (mon) {
+        monitor_set_cur(s->co, NULL);
+    }
+
     return s->ret;
 }
 
-- 
2.29.2

[PATCH 04/11] block: introduce bdrv_debug_wait_break

[Vladimir Sementsov-Ogievskiy]

Add a handler to wait for the break to happen in coroutine context. It
will be used in further commit.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 include/block/block.h     |  1 +
 include/block/block_int.h |  1 +
 block.c                   | 11 +++++++++++
 block/blkdebug.c          | 16 ++++++++++++++++
 4 files changed, 29 insertions(+)

diff --git a/include/block/block.h b/include/block/block.h
index b3f6e509d4..e133adf54f 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -648,6 +648,7 @@ int bdrv_debug_breakpoint(BlockDriverState *bs, const char *event,
 int bdrv_debug_remove_breakpoint(BlockDriverState *bs, const char *tag);
 int bdrv_debug_resume(BlockDriverState *bs, const char *tag);
 bool bdrv_debug_is_suspended(BlockDriverState *bs, const char *tag);
+void coroutine_fn bdrv_debug_wait_break(BlockDriverState *bs, const char *tag);
 
 /**
  * bdrv_get_aio_context:
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 50af58af75..89e6904fc7 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -502,6 +502,7 @@ struct BlockDriver {
         const char *tag);
     int (*bdrv_debug_resume)(BlockDriverState *bs, const char *tag);
     bool (*bdrv_debug_is_suspended)(BlockDriverState *bs, const char *tag);
+    void (*bdrv_debug_wait_break)(BlockDriverState *bs, const char *tag);
 
     void (*bdrv_refresh_limits)(BlockDriverState *bs, Error **errp);
 
diff --git a/block.c b/block.c
index 001453105e..3ea088b9fb 100644
--- a/block.c
+++ b/block.c
@@ -5702,6 +5702,17 @@ bool bdrv_debug_is_suspended(BlockDriverState *bs, const char *tag)
     return false;
 }
 
+void coroutine_fn bdrv_debug_wait_break(BlockDriverState *bs, const char *tag)
+{
+    while (bs && bs->drv && !bs->drv->bdrv_debug_wait_break) {
+        bs = bdrv_primary_bs(bs);
+    }
+
+    if (bs && bs->drv && bs->drv->bdrv_debug_wait_break) {
+        bs->drv->bdrv_debug_wait_break(bs, tag);
+    }
+}
+
 /* backing_file can either be relative, or absolute, or a protocol.  If it is
  * relative, it must be relative to the chain.  So, passing in bs->filename
  * from a BDS as backing_file should not be done, as that may be relative to
diff --git a/block/blkdebug.c b/block/blkdebug.c
index 2c0b9b0ee8..10b7c38467 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -57,6 +57,7 @@ typedef struct BDRVBlkdebugState {
     QLIST_HEAD(, BlkdebugRule) rules[BLKDBG__MAX];
     QSIMPLEQ_HEAD(, BlkdebugRule) active_rules;
     QLIST_HEAD(, BlkdebugSuspendedReq) suspended_reqs;
+    CoQueue break_waiters;
 } BDRVBlkdebugState;
 
 typedef struct BlkdebugAIOCB {
@@ -467,6 +468,8 @@ static int blkdebug_open(BlockDriverState *bs, QDict *options, int flags,
     int ret;
     uint64_t align;
 
+    qemu_co_queue_init(&s->break_waiters);
+
     opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort);
     if (!qemu_opts_absorb_qdict(opts, options, errp)) {
         ret = -EINVAL;
@@ -785,6 +788,8 @@ static void suspend_request(BlockDriverState *bs, BlkdebugRule *rule)
     remove_rule(rule);
     QLIST_INSERT_HEAD(&s->suspended_reqs, &r, next);
 
+    qemu_co_queue_restart_all(&s->break_waiters);
+
     if (!qtest_enabled()) {
         printf("blkdebug: Suspended request '%s'\n", r.tag);
     }
@@ -922,6 +927,16 @@ static bool blkdebug_debug_is_suspended(BlockDriverState *bs, const char *tag)
     return false;
 }
 
+static void coroutine_fn
+blkdebug_debug_wait_break(BlockDriverState *bs, const char *tag)
+{
+    BDRVBlkdebugState *s = bs->opaque;
+
+    while (!blkdebug_debug_is_suspended(bs, tag)) {
+        qemu_co_queue_wait(&s->break_waiters, NULL);
+    }
+}
+
 static int64_t blkdebug_getlength(BlockDriverState *bs)
 {
     return bdrv_getlength(bs->file->bs);
@@ -1048,6 +1063,7 @@ static BlockDriver bdrv_blkdebug = {
                                 = blkdebug_debug_remove_breakpoint,
     .bdrv_debug_resume          = blkdebug_debug_resume,
     .bdrv_debug_is_suspended    = blkdebug_debug_is_suspended,
+    .bdrv_debug_wait_break      = blkdebug_debug_wait_break,
 
     .strong_runtime_opts        = blkdebug_strong_runtime_opts,
 };
-- 
2.29.2

[PATCH 05/11] qemu-io-cmds: move qemu-io commands to coroutine

[Vladimir Sementsov-Ogievskiy]

Move qemuio_command to coroutine with all qemu io commands to simplify
the code and avoid extra explicit polling loops.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 include/qemu-io.h |   9 +++-
 qemu-io-cmds.c    | 110 ++++++++++------------------------------------
 block/meson.build |   3 +-
 3 files changed, 34 insertions(+), 88 deletions(-)

diff --git a/include/qemu-io.h b/include/qemu-io.h
index 3af513004a..71cca117b9 100644
--- a/include/qemu-io.h
+++ b/include/qemu-io.h
@@ -18,6 +18,7 @@
 #ifndef QEMU_IO_H
 #define QEMU_IO_H
 
+#include "block/block.h"
 
 #define CMD_FLAG_GLOBAL ((int)0x80000000) /* don't iterate "args" */
 
@@ -45,7 +46,13 @@ typedef struct cmdinfo {
 
 extern bool qemuio_misalign;
 
-int qemuio_command(BlockBackend *blk, const char *cmd);
+int coroutine_fn qemuio_co_command(BlockBackend *blk, const char *cmd);
+
+/*
+ * Called with aio context of blk acquired. Or with qemu_get_aio_context()
+ * context acquired if no blk is NULL.
+ */
+int generated_co_wrapper qemuio_command(BlockBackend *blk, const char *cmd);
 
 void qemuio_add_command(const cmdinfo_t *ci);
 void qemuio_command_usage(const cmdinfo_t *ci);
diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
index 19149e014d..adc9e64c37 100644
--- a/qemu-io-cmds.c
+++ b/qemu-io-cmds.c
@@ -555,56 +555,16 @@ static int do_pwrite(BlockBackend *blk, char *buf, int64_t offset,
     return 1;
 }
 
-typedef struct {
-    BlockBackend *blk;
-    int64_t offset;
-    int64_t bytes;
-    int64_t *total;
-    int flags;
-    int ret;
-    bool done;
-} CoWriteZeroes;
-
-static void coroutine_fn co_pwrite_zeroes_entry(void *opaque)
+static int coroutine_fn
+do_co_pwrite_zeroes(BlockBackend *blk, int64_t offset,
+                    int64_t bytes, int flags, int64_t *total)
 {
-    CoWriteZeroes *data = opaque;
-
-    data->ret = blk_co_pwrite_zeroes(data->blk, data->offset, data->bytes,
-                                     data->flags);
-    data->done = true;
-    if (data->ret < 0) {
-        *data->total = data->ret;
-        return;
-    }
-
-    *data->total = data->bytes;
-}
-
-static int do_co_pwrite_zeroes(BlockBackend *blk, int64_t offset,
-                               int64_t bytes, int flags, int64_t *total)
-{
-    Coroutine *co;
-    CoWriteZeroes data = {
-        .blk    = blk,
-        .offset = offset,
-        .bytes  = bytes,
-        .total  = total,
-        .flags  = flags,
-        .done   = false,
-    };
-
-    if (bytes > INT_MAX) {
-        return -ERANGE;
-    }
-
-    co = qemu_coroutine_create(co_pwrite_zeroes_entry, &data);
-    bdrv_coroutine_enter(blk_bs(blk), co);
-    while (!data.done) {
-        aio_poll(blk_get_aio_context(blk), true);
-    }
-    if (data.ret < 0) {
-        return data.ret;
+    int ret = blk_co_pwrite_zeroes(blk, offset, bytes, flags);
+    if (ret < 0) {
+        *total = ret;
+        return ret;
     } else {
+        *total = bytes;
         return 1;
     }
 }
@@ -654,38 +614,22 @@ static int do_save_vmstate(BlockBackend *blk, char *buf, int64_t offset,
     return 1;
 }
 
-#define NOT_DONE 0x7fffffff
-static void aio_rw_done(void *opaque, int ret)
-{
-    *(int *)opaque = ret;
-}
-
-static int do_aio_readv(BlockBackend *blk, QEMUIOVector *qiov,
-                        int64_t offset, int *total)
+static int coroutine_fn do_co_readv(BlockBackend *blk, QEMUIOVector *qiov,
+                                    int64_t offset, int *total)
 {
-    int async_ret = NOT_DONE;
-
-    blk_aio_preadv(blk, offset, qiov, 0, aio_rw_done, &async_ret);
-    while (async_ret == NOT_DONE) {
-        main_loop_wait(false);
-    }
+    int ret = blk_co_preadv(blk, offset, qiov->size, qiov, 0);
 
     *total = qiov->size;
-    return async_ret < 0 ? async_ret : 1;
+    return ret < 0 ? ret : 1;
 }
 
-static int do_aio_writev(BlockBackend *blk, QEMUIOVector *qiov,
-                         int64_t offset, int flags, int *total)
+static int coroutine_fn do_co_writev(BlockBackend *blk, QEMUIOVector *qiov,
+                                     int64_t offset, int flags, int *total)
 {
-    int async_ret = NOT_DONE;
-
-    blk_aio_pwritev(blk, offset, qiov, flags, aio_rw_done, &async_ret);
-    while (async_ret == NOT_DONE) {
-        main_loop_wait(false);
-    }
+    int ret = blk_co_pwritev(blk, offset, qiov->size, qiov, flags);
 
     *total = qiov->size;
-    return async_ret < 0 ? async_ret : 1;
+    return ret < 0 ? ret : 1;
 }
 
 static void read_help(void)
@@ -910,7 +854,7 @@ static const cmdinfo_t readv_cmd = {
     .help       = readv_help,
 };
 
-static int readv_f(BlockBackend *blk, int argc, char **argv)
+static int coroutine_fn readv_f(BlockBackend *blk, int argc, char **argv)
 {
     struct timespec t1, t2;
     bool Cflag = false, qflag = false, vflag = false;
@@ -968,7 +912,7 @@ static int readv_f(BlockBackend *blk, int argc, char **argv)
     }
 
     clock_gettime(CLOCK_MONOTONIC, &t1);
-    ret = do_aio_readv(blk, &qiov, offset, &total);
+    ret = do_co_readv(blk, &qiov, offset, &total);
     clock_gettime(CLOCK_MONOTONIC, &t2);
 
     if (ret < 0) {
@@ -1047,7 +991,7 @@ static const cmdinfo_t write_cmd = {
     .help       = write_help,
 };
 
-static int write_f(BlockBackend *blk, int argc, char **argv)
+static int coroutine_fn write_f(BlockBackend *blk, int argc, char **argv)
 {
     struct timespec t1, t2;
     bool Cflag = false, qflag = false, bflag = false;
@@ -1235,7 +1179,7 @@ writev_help(void)
 "\n");
 }
 
-static int writev_f(BlockBackend *blk, int argc, char **argv);
+static int coroutine_fn writev_f(BlockBackend *blk, int argc, char **argv);
 
 static const cmdinfo_t writev_cmd = {
     .name       = "writev",
@@ -1248,7 +1192,7 @@ static const cmdinfo_t writev_cmd = {
     .help       = writev_help,
 };
 
-static int writev_f(BlockBackend *blk, int argc, char **argv)
+static int coroutine_fn writev_f(BlockBackend *blk, int argc, char **argv)
 {
     struct timespec t1, t2;
     bool Cflag = false, qflag = false;
@@ -1304,7 +1248,7 @@ static int writev_f(BlockBackend *blk, int argc, char **argv)
     }
 
     clock_gettime(CLOCK_MONOTONIC, &t1);
-    ret = do_aio_writev(blk, &qiov, offset, flags, &total);
+    ret = do_co_writev(blk, &qiov, offset, flags, &total);
     clock_gettime(CLOCK_MONOTONIC, &t2);
 
     if (ret < 0) {
@@ -2283,9 +2227,7 @@ static const cmdinfo_t resume_cmd = {
 
 static int wait_break_f(BlockBackend *blk, int argc, char **argv)
 {
-    while (!bdrv_debug_is_suspended(blk_bs(blk), argv[1])) {
-        aio_poll(blk_get_aio_context(blk), true);
-    }
+    bdrv_debug_wait_break(blk_bs(blk), argv[1]);
     return 0;
 }
 
@@ -2457,11 +2399,7 @@ static const cmdinfo_t help_cmd = {
     .oneline    = "help for one or all commands",
 };
 
-/*
- * Called with aio context of blk acquired. Or with qemu_get_aio_context()
- * context acquired if no blk is NULL.
- */
-int qemuio_command(BlockBackend *blk, const char *cmd)
+int coroutine_fn qemuio_co_command(BlockBackend *blk, const char *cmd)
 {
     char *input;
     const cmdinfo_t *ct;
diff --git a/block/meson.build b/block/meson.build
index d21990ec95..27e11aa199 100644
--- a/block/meson.build
+++ b/block/meson.build
@@ -114,7 +114,8 @@ wrapper_py = find_program('../scripts/block-coroutine-wrapper.py')
 block_gen_c = custom_target('block-gen.c',
                             output: 'block-gen.c',
                             input: files('../include/block/block.h',
-                                         'coroutines.h'),
+                                         'coroutines.h',
+                                         '../include/qemu-io.h'),
                             command: [wrapper_py, '@OUTPUT@', '@INPUT@'])
 block_ss.add(block_gen_c)
 
-- 
2.29.2

[PATCH 06/11] block: drop unused bdrv_debug_is_suspended()

[Vladimir Sementsov-Ogievskiy]

Now it's actually substituted by coroutine based
bdrv_debug_wait_break().

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 include/block/block.h     |  1 -
 include/block/block_int.h |  1 -
 block.c                   | 13 -------------
 block/blkdebug.c          |  1 -
 4 files changed, 16 deletions(-)

diff --git a/include/block/block.h b/include/block/block.h
index e133adf54f..fb1897c1e8 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -647,7 +647,6 @@ int bdrv_debug_breakpoint(BlockDriverState *bs, const char *event,
                            const char *tag);
 int bdrv_debug_remove_breakpoint(BlockDriverState *bs, const char *tag);
 int bdrv_debug_resume(BlockDriverState *bs, const char *tag);
-bool bdrv_debug_is_suspended(BlockDriverState *bs, const char *tag);
 void coroutine_fn bdrv_debug_wait_break(BlockDriverState *bs, const char *tag);
 
 /**
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 89e6904fc7..592acc960f 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -501,7 +501,6 @@ struct BlockDriver {
     int (*bdrv_debug_remove_breakpoint)(BlockDriverState *bs,
         const char *tag);
     int (*bdrv_debug_resume)(BlockDriverState *bs, const char *tag);
-    bool (*bdrv_debug_is_suspended)(BlockDriverState *bs, const char *tag);
     void (*bdrv_debug_wait_break)(BlockDriverState *bs, const char *tag);
 
     void (*bdrv_refresh_limits)(BlockDriverState *bs, Error **errp);
diff --git a/block.c b/block.c
index 3ea088b9fb..f026d710b7 100644
--- a/block.c
+++ b/block.c
@@ -5689,19 +5689,6 @@ int bdrv_debug_resume(BlockDriverState *bs, const char *tag)
     return -ENOTSUP;
 }
 
-bool bdrv_debug_is_suspended(BlockDriverState *bs, const char *tag)
-{
-    while (bs && bs->drv && !bs->drv->bdrv_debug_is_suspended) {
-        bs = bdrv_primary_bs(bs);
-    }
-
-    if (bs && bs->drv && bs->drv->bdrv_debug_is_suspended) {
-        return bs->drv->bdrv_debug_is_suspended(bs, tag);
-    }
-
-    return false;
-}
-
 void coroutine_fn bdrv_debug_wait_break(BlockDriverState *bs, const char *tag)
 {
     while (bs && bs->drv && !bs->drv->bdrv_debug_wait_break) {
diff --git a/block/blkdebug.c b/block/blkdebug.c
index 10b7c38467..608d1d5bd6 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -1062,7 +1062,6 @@ static BlockDriver bdrv_blkdebug = {
     .bdrv_debug_remove_breakpoint
                                 = blkdebug_debug_remove_breakpoint,
     .bdrv_debug_resume          = blkdebug_debug_resume,
-    .bdrv_debug_is_suspended    = blkdebug_debug_is_suspended,
     .bdrv_debug_wait_break      = blkdebug_debug_wait_break,
 
     .strong_runtime_opts        = blkdebug_strong_runtime_opts,
-- 
2.29.2

[PATCH 07/11] block-backend: add _co_ versions of blk_save_vmstate / blk_load_vmstate

[Vladimir Sementsov-Ogievskiy]

To be used in further commit. Don't worry about some duplication with
existing blk_save_vmstate() and blk_load_vmstate(): they will be
removed soon.

Note the difference: new functions returns 0 on success.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 include/sysemu/block-backend.h |  3 +++
 block/block-backend.c          | 37 ++++++++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+)

diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 880e903293..8676bbde5a 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -245,6 +245,9 @@ int blk_pdiscard(BlockBackend *blk, int64_t offset, int bytes);
 int blk_save_vmstate(BlockBackend *blk, const uint8_t *buf,
                      int64_t pos, int size);
 int blk_load_vmstate(BlockBackend *blk, uint8_t *buf, int64_t pos, int size);
+int blk_co_save_vmstate(BlockBackend *blk, const uint8_t *buf,
+                        int64_t pos, int size);
+int blk_co_load_vmstate(BlockBackend *blk, uint8_t *buf, int64_t pos, int size);
 int blk_probe_blocksizes(BlockBackend *blk, BlockSizes *bsz);
 int blk_probe_geometry(BlockBackend *blk, HDGeometry *geo);
 BlockAIOCB *blk_abort_aio_request(BlockBackend *blk,
diff --git a/block/block-backend.c b/block/block-backend.c
index 413af51f3b..d7f91ce7ad 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -14,6 +14,7 @@
 #include "sysemu/block-backend.h"
 #include "block/block_int.h"
 #include "block/blockjob.h"
+#include "block/coroutines.h"
 #include "block/throttle-groups.h"
 #include "hw/qdev-core.h"
 #include "sysemu/blockdev.h"
@@ -2227,6 +2228,42 @@ int blk_load_vmstate(BlockBackend *blk, uint8_t *buf, int64_t pos, int size)
     return bdrv_load_vmstate(blk_bs(blk), buf, pos, size);
 }
 
+int blk_co_save_vmstate(BlockBackend *blk, const uint8_t *buf,
+                        int64_t pos, int size)
+{
+    int ret;
+    QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF(qiov, buf, size);
+
+    if (!blk_is_available(blk)) {
+        return -ENOMEDIUM;
+    }
+
+    ret = bdrv_co_writev_vmstate(blk_bs(blk), &qiov, pos);
+    if (ret < 0) {
+        return ret;
+    }
+
+    if (!blk->enable_write_cache) {
+        ret = bdrv_flush(blk_bs(blk));
+    }
+
+    return ret < 0 ? ret : 0;
+}
+
+int blk_co_load_vmstate(BlockBackend *blk, uint8_t *buf, int64_t pos, int size)
+{
+    int ret;
+    QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF(qiov, buf, size);
+
+    if (!blk_is_available(blk)) {
+        return -ENOMEDIUM;
+    }
+
+    ret = bdrv_co_readv_vmstate(blk_bs(blk), &qiov, pos);
+
+    return ret < 0 ? ret : 0;
+}
+
 int blk_probe_blocksizes(BlockBackend *blk, BlockSizes *bsz)
 {
     if (!blk_is_available(blk)) {
-- 
2.29.2

[PATCH 08/11] qemu-io-cmds: refactor read_f(): drop extra helpers and variables

[Vladimir Sementsov-Ogievskiy]

Now all commands are in coroutines, so we want to move to _co_
functions. Let's just drop helpers and use blk_co_ functions directly.
Note that count is checked at start of read_f anyway.
Both blk_co_pread and blk_co_load_vmstate returns 0 on success, so we
should not care to set ret to 0 explicitly. Moreover, no caller is
care, is successful ret of qemuio_command positive or not.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 qemu-io-cmds.c | 44 ++++++--------------------------------------
 1 file changed, 6 insertions(+), 38 deletions(-)

diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
index adc9e64c37..bbebecba55 100644
--- a/qemu-io-cmds.c
+++ b/qemu-io-cmds.c
@@ -527,20 +527,6 @@ fail:
     return buf;
 }
 
-static int do_pread(BlockBackend *blk, char *buf, int64_t offset,
-                    int64_t bytes, int64_t *total)
-{
-    if (bytes > INT_MAX) {
-        return -ERANGE;
-    }
-
-    *total = blk_pread(blk, offset, (uint8_t *)buf, bytes);
-    if (*total < 0) {
-        return *total;
-    }
-    return 1;
-}
-
 static int do_pwrite(BlockBackend *blk, char *buf, int64_t offset,
                      int64_t bytes, int flags, int64_t *total)
 {
@@ -586,20 +572,6 @@ static int do_write_compressed(BlockBackend *blk, char *buf, int64_t offset,
     return 1;
 }
 
-static int do_load_vmstate(BlockBackend *blk, char *buf, int64_t offset,
-                           int64_t count, int64_t *total)
-{
-    if (count > INT_MAX) {
-        return -ERANGE;
-    }
-
-    *total = blk_load_vmstate(blk, (uint8_t *)buf, offset, count);
-    if (*total < 0) {
-        return *total;
-    }
-    return 1;
-}
-
 static int do_save_vmstate(BlockBackend *blk, char *buf, int64_t offset,
                            int64_t count, int64_t *total)
 {
@@ -667,17 +639,16 @@ static const cmdinfo_t read_cmd = {
     .help       = read_help,
 };
 
-static int read_f(BlockBackend *blk, int argc, char **argv)
+static int coroutine_fn read_f(BlockBackend *blk, int argc, char **argv)
 {
     struct timespec t1, t2;
     bool Cflag = false, qflag = false, vflag = false;
     bool Pflag = false, sflag = false, lflag = false, bflag = false;
-    int c, cnt, ret;
-    char *buf;
+    int c, ret;
+    uint8_t *buf;
     int64_t offset;
     int64_t count;
     /* Some compilers get confused and warn if this is not initialized.  */
-    int64_t total = 0;
     int pattern = 0;
     int64_t pattern_offset = 0, pattern_count = 0;
 
@@ -780,9 +751,9 @@ static int read_f(BlockBackend *blk, int argc, char **argv)
 
     clock_gettime(CLOCK_MONOTONIC, &t1);
     if (bflag) {
-        ret = do_load_vmstate(blk, buf, offset, count, &total);
+        ret = blk_co_load_vmstate(blk, buf, offset, count);
     } else {
-        ret = do_pread(blk, buf, offset, count, &total);
+        ret = blk_co_pread(blk, offset, count, buf, 0);
     }
     clock_gettime(CLOCK_MONOTONIC, &t2);
 
@@ -790,9 +761,6 @@ static int read_f(BlockBackend *blk, int argc, char **argv)
         printf("read failed: %s\n", strerror(-ret));
         goto out;
     }
-    cnt = ret;
-
-    ret = 0;
 
     if (Pflag) {
         void *cmp_buf = g_malloc(pattern_count);
@@ -816,7 +784,7 @@ static int read_f(BlockBackend *blk, int argc, char **argv)
 
     /* Finally, report back -- -C gives a parsable format */
     t2 = tsub(t2, t1);
-    print_report("read", &t2, offset, count, total, cnt, Cflag);
+    print_report("read", &t2, offset, count, count, 1, Cflag);
 
 out:
     qemu_io_free(buf);
-- 
2.29.2

[PATCH 09/11] qemu-io-cmds: refactor write_f(): drop extra helpers and variables

[Vladimir Sementsov-Ogievskiy]

We are in coroutine context. Let's call blk_co_ functions directly and
drop all these helpers.
Note that count is checked earlier in write_f, so we don't need the
check in helpers.
Also, both blk_co_save_vmstate() and blk_co_pwrite() return 0 on
success, so we should not care to set ret to 0 explicitly. Moreover, no
caller is interested in successful ret of qemuio_command being exactly
zero.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 qemu-io-cmds.c | 81 +++++---------------------------------------------
 1 file changed, 8 insertions(+), 73 deletions(-)

diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
index bbebecba55..2f0a27079d 100644
--- a/qemu-io-cmds.c
+++ b/qemu-io-cmds.c
@@ -527,65 +527,6 @@ fail:
     return buf;
 }
 
-static int do_pwrite(BlockBackend *blk, char *buf, int64_t offset,
-                     int64_t bytes, int flags, int64_t *total)
-{
-    if (bytes > INT_MAX) {
-        return -ERANGE;
-    }
-
-    *total = blk_pwrite(blk, offset, (uint8_t *)buf, bytes, flags);
-    if (*total < 0) {
-        return *total;
-    }
-    return 1;
-}
-
-static int coroutine_fn
-do_co_pwrite_zeroes(BlockBackend *blk, int64_t offset,
-                    int64_t bytes, int flags, int64_t *total)
-{
-    int ret = blk_co_pwrite_zeroes(blk, offset, bytes, flags);
-    if (ret < 0) {
-        *total = ret;
-        return ret;
-    } else {
-        *total = bytes;
-        return 1;
-    }
-}
-
-static int do_write_compressed(BlockBackend *blk, char *buf, int64_t offset,
-                               int64_t bytes, int64_t *total)
-{
-    int ret;
-
-    if (bytes > BDRV_REQUEST_MAX_BYTES) {
-        return -ERANGE;
-    }
-
-    ret = blk_pwrite_compressed(blk, offset, buf, bytes);
-    if (ret < 0) {
-        return ret;
-    }
-    *total = bytes;
-    return 1;
-}
-
-static int do_save_vmstate(BlockBackend *blk, char *buf, int64_t offset,
-                           int64_t count, int64_t *total)
-{
-    if (count > INT_MAX) {
-        return -ERANGE;
-    }
-
-    *total = blk_save_vmstate(blk, (uint8_t *)buf, offset, count);
-    if (*total < 0) {
-        return *total;
-    }
-    return 1;
-}
-
 static int coroutine_fn do_co_readv(BlockBackend *blk, QEMUIOVector *qiov,
                                     int64_t offset, int *total)
 {
@@ -945,7 +886,7 @@ static void write_help(void)
 "\n");
 }
 
-static int write_f(BlockBackend *blk, int argc, char **argv);
+static int coroutine_fn write_f(BlockBackend *blk, int argc, char **argv);
 
 static const cmdinfo_t write_cmd = {
     .name       = "write",
@@ -965,12 +906,11 @@ static int coroutine_fn write_f(BlockBackend *blk, int argc, char **argv)
     bool Cflag = false, qflag = false, bflag = false;
     bool Pflag = false, zflag = false, cflag = false, sflag = false;
     int flags = 0;
-    int c, cnt, ret;
-    char *buf = NULL;
+    int c, ret;
+    uint8_t *buf = NULL;
     int64_t offset;
     int64_t count;
     /* Some compilers get confused and warn if this is not initialized.  */
-    int64_t total = 0;
     int pattern = 0xcd;
     const char *file_name = NULL;
 
@@ -981,6 +921,7 @@ static int coroutine_fn write_f(BlockBackend *blk, int argc, char **argv)
             break;
         case 'c':
             cflag = true;
+            flags |= BDRV_REQ_WRITE_COMPRESSED;
             break;
         case 'C':
             Cflag = true;
@@ -1013,6 +954,7 @@ static int coroutine_fn write_f(BlockBackend *blk, int argc, char **argv)
             break;
         case 'z':
             zflag = true;
+            flags |= BDRV_REQ_ZERO_WRITE;
             break;
         default:
             qemuio_command_usage(&write_cmd);
@@ -1095,13 +1037,9 @@ static int coroutine_fn write_f(BlockBackend *blk, int argc, char **argv)
 
     clock_gettime(CLOCK_MONOTONIC, &t1);
     if (bflag) {
-        ret = do_save_vmstate(blk, buf, offset, count, &total);
-    } else if (zflag) {
-        ret = do_co_pwrite_zeroes(blk, offset, count, flags, &total);
-    } else if (cflag) {
-        ret = do_write_compressed(blk, buf, offset, count, &total);
+        ret = blk_co_save_vmstate(blk, buf, offset, count);
     } else {
-        ret = do_pwrite(blk, buf, offset, count, flags, &total);
+        ret = blk_co_pwrite(blk, offset, count, buf, flags);
     }
     clock_gettime(CLOCK_MONOTONIC, &t2);
 
@@ -1109,9 +1047,6 @@ static int coroutine_fn write_f(BlockBackend *blk, int argc, char **argv)
         printf("write failed: %s\n", strerror(-ret));
         goto out;
     }
-    cnt = ret;
-
-    ret = 0;
 
     if (qflag) {
         goto out;
@@ -1119,7 +1054,7 @@ static int coroutine_fn write_f(BlockBackend *blk, int argc, char **argv)
 
     /* Finally, report back -- -C gives a parsable format */
     t2 = tsub(t2, t1);
-    print_report("wrote", &t2, offset, count, total, cnt, Cflag);
+    print_report("wrote", &t2, offset, count, count, 1, Cflag);
 
 out:
     if (!zflag) {
-- 
2.29.2

[PATCH 10/11] qemu-io-cmds: drop do_co_readv() and do_co_writev() helpers

[Vladimir Sementsov-Ogievskiy]

They don't make much sense. Call blk_co_ functions directly and also
drop some redundant variables.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 qemu-io-cmds.c | 38 ++++++--------------------------------
 1 file changed, 6 insertions(+), 32 deletions(-)

diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
index 2f0a27079d..9a0e5322de 100644
--- a/qemu-io-cmds.c
+++ b/qemu-io-cmds.c
@@ -527,24 +527,6 @@ fail:
     return buf;
 }
 
-static int coroutine_fn do_co_readv(BlockBackend *blk, QEMUIOVector *qiov,
-                                    int64_t offset, int *total)
-{
-    int ret = blk_co_preadv(blk, offset, qiov->size, qiov, 0);
-
-    *total = qiov->size;
-    return ret < 0 ? ret : 1;
-}
-
-static int coroutine_fn do_co_writev(BlockBackend *blk, QEMUIOVector *qiov,
-                                     int64_t offset, int flags, int *total)
-{
-    int ret = blk_co_pwritev(blk, offset, qiov->size, qiov, flags);
-
-    *total = qiov->size;
-    return ret < 0 ? ret : 1;
-}
-
 static void read_help(void)
 {
     printf(
@@ -767,11 +749,10 @@ static int coroutine_fn readv_f(BlockBackend *blk, int argc, char **argv)
 {
     struct timespec t1, t2;
     bool Cflag = false, qflag = false, vflag = false;
-    int c, cnt, ret;
+    int c, ret;
     char *buf;
     int64_t offset;
     /* Some compilers get confused and warn if this is not initialized.  */
-    int total = 0;
     int nr_iov;
     QEMUIOVector qiov;
     int pattern = 0;
@@ -821,16 +802,13 @@ static int coroutine_fn readv_f(BlockBackend *blk, int argc, char **argv)
     }
 
     clock_gettime(CLOCK_MONOTONIC, &t1);
-    ret = do_co_readv(blk, &qiov, offset, &total);
+    ret = blk_co_preadv(blk, offset, qiov.size, &qiov, 0);
     clock_gettime(CLOCK_MONOTONIC, &t2);
 
     if (ret < 0) {
         printf("readv failed: %s\n", strerror(-ret));
         goto out;
     }
-    cnt = ret;
-
-    ret = 0;
 
     if (Pflag) {
         void *cmp_buf = g_malloc(qiov.size);
@@ -853,7 +831,7 @@ static int coroutine_fn readv_f(BlockBackend *blk, int argc, char **argv)
 
     /* Finally, report back -- -C gives a parsable format */
     t2 = tsub(t2, t1);
-    print_report("read", &t2, offset, qiov.size, total, cnt, Cflag);
+    print_report("read", &t2, offset, qiov.size, qiov.size, 1, Cflag);
 
 out:
     qemu_iovec_destroy(&qiov);
@@ -1100,11 +1078,10 @@ static int coroutine_fn writev_f(BlockBackend *blk, int argc, char **argv)
     struct timespec t1, t2;
     bool Cflag = false, qflag = false;
     int flags = 0;
-    int c, cnt, ret;
+    int c, ret;
     char *buf;
     int64_t offset;
     /* Some compilers get confused and warn if this is not initialized.  */
-    int total = 0;
     int nr_iov;
     int pattern = 0xcd;
     QEMUIOVector qiov;
@@ -1151,16 +1128,13 @@ static int coroutine_fn writev_f(BlockBackend *blk, int argc, char **argv)
     }
 
     clock_gettime(CLOCK_MONOTONIC, &t1);
-    ret = do_co_writev(blk, &qiov, offset, flags, &total);
+    ret = blk_co_pwritev(blk, offset, qiov.size,  &qiov, flags);
     clock_gettime(CLOCK_MONOTONIC, &t2);
 
     if (ret < 0) {
         printf("writev failed: %s\n", strerror(-ret));
         goto out;
     }
-    cnt = ret;
-
-    ret = 0;
 
     if (qflag) {
         goto out;
@@ -1168,7 +1142,7 @@ static int coroutine_fn writev_f(BlockBackend *blk, int argc, char **argv)
 
     /* Finally, report back -- -C gives a parsable format */
     t2 = tsub(t2, t1);
-    print_report("wrote", &t2, offset, qiov.size, total, cnt, Cflag);
+    print_report("wrote", &t2, offset, qiov.size, qiov.size, 1, Cflag);
 out:
     qemu_iovec_destroy(&qiov);
     qemu_io_free(buf);
-- 
2.29.2

[PATCH 11/11] block-backend: drop unused blk_save_vmstate() and blk_load_vmstate()

[Vladimir Sementsov-Ogievskiy]

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 include/sysemu/block-backend.h |  3 ---
 block/block-backend.c          | 30 ------------------------------
 2 files changed, 33 deletions(-)

diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 8676bbde5a..14cc410244 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -242,9 +242,6 @@ int blk_pwrite_compressed(BlockBackend *blk, int64_t offset, const void *buf,
 int blk_truncate(BlockBackend *blk, int64_t offset, bool exact,
                  PreallocMode prealloc, BdrvRequestFlags flags, Error **errp);
 int blk_pdiscard(BlockBackend *blk, int64_t offset, int bytes);
-int blk_save_vmstate(BlockBackend *blk, const uint8_t *buf,
-                     int64_t pos, int size);
-int blk_load_vmstate(BlockBackend *blk, uint8_t *buf, int64_t pos, int size);
 int blk_co_save_vmstate(BlockBackend *blk, const uint8_t *buf,
                         int64_t pos, int size);
 int blk_co_load_vmstate(BlockBackend *blk, uint8_t *buf, int64_t pos, int size);
diff --git a/block/block-backend.c b/block/block-backend.c
index d7f91ce7ad..83aafda791 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -2198,36 +2198,6 @@ int blk_truncate(BlockBackend *blk, int64_t offset, bool exact,
     return bdrv_truncate(blk->root, offset, exact, prealloc, flags, errp);
 }
 
-int blk_save_vmstate(BlockBackend *blk, const uint8_t *buf,
-                     int64_t pos, int size)
-{
-    int ret;
-
-    if (!blk_is_available(blk)) {
-        return -ENOMEDIUM;
-    }
-
-    ret = bdrv_save_vmstate(blk_bs(blk), buf, pos, size);
-    if (ret < 0) {
-        return ret;
-    }
-
-    if (ret == size && !blk->enable_write_cache) {
-        ret = bdrv_flush(blk_bs(blk));
-    }
-
-    return ret < 0 ? ret : size;
-}
-
-int blk_load_vmstate(BlockBackend *blk, uint8_t *buf, int64_t pos, int size)
-{
-    if (!blk_is_available(blk)) {
-        return -ENOMEDIUM;
-    }
-
-    return bdrv_load_vmstate(blk_bs(blk), buf, pos, size);
-}
-
 int blk_co_save_vmstate(BlockBackend *blk, const uint8_t *buf,
                         int64_t pos, int size)
 {
-- 
2.29.2

Re: [PATCH 03/11] block/block-gen.h: bind monitor

[Markus Armbruster]

Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> writes:

> If we have current monitor, let's bind it to wrapper coroutine too.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  block/block-gen.h | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/block/block-gen.h b/block/block-gen.h
> index c1fd3f40de..61f055a8cc 100644
> --- a/block/block-gen.h
> +++ b/block/block-gen.h
> @@ -27,6 +27,7 @@
>  #define BLOCK_BLOCK_GEN_H
>  
>  #include "block/block_int.h"
> +#include "monitor/monitor.h"
>  
>  /* Base structure for argument packing structures */
>  typedef struct AioPollCo {
> @@ -38,11 +39,20 @@ typedef struct AioPollCo {
>  
>  static inline int aio_poll_co(AioPollCo *s)
>  {
> +    Monitor *mon = monitor_cur();

This gets the currently executing coroutine's monitor from the hash
table.

>      assert(!qemu_in_coroutine());
>  
> +    if (mon) {
> +        monitor_set_cur(s->co, mon);

This writes it back.  No-op, since the coroutine hasn't changed.  Why?

> +    }
> +
>      aio_co_enter(s->ctx, s->co);
>      AIO_WAIT_WHILE(s->ctx, s->in_progress);
>  
> +    if (mon) {
> +        monitor_set_cur(s->co, NULL);

This removes s->co's monitor from the hash table.  Why?

> +    }
> +
>      return s->ret;
>  }

Re: [PATCH 03/11] block/block-gen.h: bind monitor

[Vladimir Sementsov-Ogievskiy]

24.04.2021 08:23, Markus Armbruster wrote:
> Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> writes:
> 
>> If we have current monitor, let's bind it to wrapper coroutine too.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>>   block/block-gen.h | 10 ++++++++++
>>   1 file changed, 10 insertions(+)
>>
>> diff --git a/block/block-gen.h b/block/block-gen.h
>> index c1fd3f40de..61f055a8cc 100644
>> --- a/block/block-gen.h
>> +++ b/block/block-gen.h
>> @@ -27,6 +27,7 @@
>>   #define BLOCK_BLOCK_GEN_H
>>   
>>   #include "block/block_int.h"
>> +#include "monitor/monitor.h"
>>   
>>   /* Base structure for argument packing structures */
>>   typedef struct AioPollCo {
>> @@ -38,11 +39,20 @@ typedef struct AioPollCo {
>>   
>>   static inline int aio_poll_co(AioPollCo *s)
>>   {
>> +    Monitor *mon = monitor_cur();
> 
> This gets the currently executing coroutine's monitor from the hash
> table.
> 
>>       assert(!qemu_in_coroutine());
>>   
>> +    if (mon) {
>> +        monitor_set_cur(s->co, mon);
> 
> This writes it back.  No-op, since the coroutine hasn't changed.  Why?

No. s->co != qemu_corotuine_current(), so it's not a write back, but creating a new entry in the hash map. s->co is a new coroutine which we are going to start.

> 
>> +    }
>> +
>>       aio_co_enter(s->ctx, s->co);
>>       AIO_WAIT_WHILE(s->ctx, s->in_progress);
>>   
>> +    if (mon) {
>> +        monitor_set_cur(s->co, NULL);
> 
> This removes s->co's monitor from the hash table.  Why?
> 
>> +    }
>> +
>>       return s->ret;
>>   }
> 

If I comment the new code of this patch (keeping the whole series applied), 249 fails, as error message goes simply to stderr, not to monitor:

249   fail       [11:56:54] [11:56:54]   0.3s   (last: 0.2s)  output mismatch (see 249.out.bad)
--- /work/src/qemu/up/hmp-qemu-io/tests/qemu-iotests/249.out
+++ 249.out.bad
@@ -9,7 +9,8 @@

  { 'execute': 'human-monitor-command',
         'arguments': {'command-line': 'qemu-io none0 "aio_write 0 2k"'}}
-{"return": "Block node is read-onlyrn"}
+QEMU_PROG: Block node is read-only
+{"return": ""}

  === Run block-commit on base using an invalid filter node name

@@ -24,7 +25,8 @@

  { 'execute': 'human-monitor-command',
         'arguments': {'command-line': 'qemu-io none0 "aio_write 0 2k"'}}
-{"return": "Block node is read-onlyrn"}
+QEMU_PROG: Block node is read-only
+{"return": ""}

  === Run block-commit on base using the default filter node name

@@ -43,5 +45,6 @@

  { 'execute': 'human-monitor-command',
         'arguments': {'command-line': 'qemu-io none0 "aio_write 0 2k"'}}
-{"return": "Block node is read-onlyrn"}
+QEMU_PROG: Block node is read-only
+{"return": ""}
  *** done
Failures: 249
Failed 1 of 1 iotests



-- 
Best regards,
Vladimir

Re: [PATCH 03/11] block/block-gen.h: bind monitor

[Markus Armbruster]

Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> writes:

> 24.04.2021 08:23, Markus Armbruster wrote:
>> Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> writes:
>> 
>>> If we have current monitor, let's bind it to wrapper coroutine too.
>>>
>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>>> ---
>>>   block/block-gen.h | 10 ++++++++++
>>>   1 file changed, 10 insertions(+)
>>>
>>> diff --git a/block/block-gen.h b/block/block-gen.h
>>> index c1fd3f40de..61f055a8cc 100644
>>> --- a/block/block-gen.h
>>> +++ b/block/block-gen.h
>>> @@ -27,6 +27,7 @@
>>>   #define BLOCK_BLOCK_GEN_H
>>>   
>>>   #include "block/block_int.h"
>>> +#include "monitor/monitor.h"
>>>   
>>>   /* Base structure for argument packing structures */
>>>   typedef struct AioPollCo {
>>> @@ -38,11 +39,20 @@ typedef struct AioPollCo {
>>>   
>>>   static inline int aio_poll_co(AioPollCo *s)
>>>   {
>>> +    Monitor *mon = monitor_cur();
>> 
>> This gets the currently executing coroutine's monitor from the hash
>> table.
>> 
>>>       assert(!qemu_in_coroutine());
>>>   
>>> +    if (mon) {
>>> +        monitor_set_cur(s->co, mon);
>> 
>> This writes it back.  No-op, since the coroutine hasn't changed.  Why?
>
> No. s->co != qemu_corotuine_current(), so it's not a write back, but creating a new entry in the hash map. s->co is a new coroutine which we are going to start.

Ah, that's what I missed.  Thanks!

[...]