On 09.06.23 22:19, Fabiano Rosas wrote:
From: Lin Ma <l...@suse.com>

We're converting callers of bdrv_get_allocated_file_size() to run in
coroutines because that function will be made asynchronous when called
(indirectly) from the QMP dispatcher.

This QMP command is a candidate because it indirectly calls
bdrv_get_allocated_file_size() through bdrv_block_device_info() ->
bdrv_query_image_info() -> bdrv_query_image_info().

The previous patches have determined that bdrv_query_image_info() and
bdrv_do_query_node_info() are coroutine-safe so we can just make the
QMP command run in a coroutine.

Signed-off-by: Lin Ma <l...@suse.com>
Signed-off-by: Fabiano Rosas <faro...@suse.de>
---
  block.c              | 2 +-
  blockdev.c           | 6 +++---
  qapi/block-core.json | 3 ++-
  3 files changed, 6 insertions(+), 5 deletions(-)

(I see patch 9 does something with HMP code, but) hmp_info_block() calls qmp_query_named_block_nodes(), and I don’t think it may call such a coroutine_fn directly.

diff --git a/block.c b/block.c
index f94cee8930..abed744b60 100644
--- a/block.c
+++ b/block.c
@@ -6148,7 +6148,7 @@ BlockDeviceInfoList *bdrv_named_nodes_list(bool flat,
list = NULL;
      QTAILQ_FOREACH(bs, &graph_bdrv_states, node_list) {
-        BlockDeviceInfo *info = bdrv_block_device_info(NULL, bs, flat, errp);
+        BlockDeviceInfo *info = bdrv_co_block_device_info(NULL, bs, flat, 
errp);

As far as I understand, only functions marked as coroutine_fn may call other coroutine_fn.  Regardless of whether bdrv_named_nodes_list() is only called by another coroutine_fn, we still have to mark it as coroutine_fn, too (and probably rename it to bdrv_co_named_nodes_list()).

Also, this function (bdrv_named_nodes_list()) uses GRAPH_RDLOCK_GUARD_MAINLOOP().  Is that the correct thing to use in a coroutine context?

Hanna

          if (!info) {
              qapi_free_BlockDeviceInfoList(list);
              return NULL;
diff --git a/blockdev.c b/blockdev.c
index e6eba61484..8b5f7d06c8 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2818,9 +2818,9 @@ void qmp_drive_backup(DriveBackup *backup, Error **errp)
      blockdev_do_action(&action, errp);
  }
-BlockDeviceInfoList *qmp_query_named_block_nodes(bool has_flat,
-                                                 bool flat,
-                                                 Error **errp)
+BlockDeviceInfoList *coroutine_fn qmp_query_named_block_nodes(bool has_flat,
+                                                              bool flat,
+                                                              Error **errp)
  {
      bool return_flat = has_flat && flat;
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 5dd5f7e4b0..9d4c92f2c9 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1972,7 +1972,8 @@
  { 'command': 'query-named-block-nodes',
    'returns': [ 'BlockDeviceInfo' ],
    'data': { '*flat': 'bool' },
-  'allow-preconfig': true }
+  'allow-preconfig': true,
+  'coroutine': true}
##
  # @XDbgBlockGraphNodeType:


Reply via email to