[v3,05/11] block: Run bdrv_do_query_node_info in a coroutine

Commit Message

Fabiano Rosas April 9, 2024, 2:59 p.m. UTC

Move this function into a coroutine so we can convert the whole
qmp_query_block command into a coroutine in the next patches.

Placing the entire command in a coroutine allow us to yield all the
way back to the main loop, releasing the BQL and unblocking the main
loop.

When the whole conversion is completed, we'll be able to avoid a
priority inversion that happens when a QMP command calls a slow
(buggy) system call and blocks the vcpu thread from doing mmio due to
contention on the BQL.

About coroutine safety:

Most callees have coroutine versions themselves and thus are safe to
call in a coroutine. The remaining ones:

- bdrv_refresh_filename, bdrv_get_full_backing_filename: String
  manipulation, nothing that would be unsafe for use in coroutines;

- bdrv_get_format_name: Just accesses a field;

- bdrv_get_specific_info, bdrv_query_snapshot_info_list: No locks or
  anything that would poll or block.

(using a mixed wrapper for now, but after all callers are converted,
this can become a coroutine exclusively)

Signed-off-by: Fabiano Rosas <[e-mail]>
---
- used the coroutine version of the called functions when available
---
 block/qapi.c         | 11 ++++++-----
 include/block/qapi.h |  8 ++++++++
 2 files changed, 14 insertions(+), 5 deletions(-)

Patch

diff --git a/block/qapi.c b/block/qapi.c
index 2b5793f1d9..26a9510345 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -225,8 +225,9 @@  int bdrv_query_snapshot_info_list(BlockDriverState *bs,
  * Helper function for other query info functions.  Store information about @bs
  * in @info, setting @errp on error.
  */
-static void GRAPH_RDLOCK
-bdrv_do_query_node_info(BlockDriverState *bs, BlockNodeInfo *info, Error **errp)
+void coroutine_fn
+bdrv_co_do_query_node_info(BlockDriverState *bs, BlockNodeInfo *info,
+                           Error **errp)
 {
     int64_t size;
     const char *backing_filename;
@@ -234,7 +235,7 @@  bdrv_do_query_node_info(BlockDriverState *bs, BlockNodeInfo *info, Error **errp)
     int ret;
     Error *err = NULL;
 
-    size = bdrv_getlength(bs);
+    size = bdrv_co_getlength(bs);
     if (size < 0) {
         error_setg_errno(errp, -size, "Can't get image size '%s'",
                          bs->exact_filename);
@@ -246,13 +247,13 @@  bdrv_do_query_node_info(BlockDriverState *bs, BlockNodeInfo *info, Error **errp)
     info->filename        = g_strdup(bs->filename);
     info->format          = g_strdup(bdrv_get_format_name(bs));
     info->virtual_size    = size;
-    info->actual_size     = bdrv_get_allocated_file_size(bs);
+    info->actual_size     = bdrv_co_get_allocated_file_size(bs);
     info->has_actual_size = info->actual_size >= 0;
     if (bs->encrypted) {
         info->encrypted = true;
         info->has_encrypted = true;
     }
-    if (bdrv_get_info(bs, &bdi) >= 0) {
+    if (bdrv_co_get_info(bs, &bdi) >= 0) {
         if (bdi.cluster_size != 0) {
             info->cluster_size = bdi.cluster_size;
             info->has_cluster_size = true;
diff --git a/include/block/qapi.h b/include/block/qapi.h
index 54c48de26a..234730b3de 100644
--- a/include/block/qapi.h
+++ b/include/block/qapi.h
@@ -25,6 +25,7 @@ 
 #ifndef BLOCK_QAPI_H
 #define BLOCK_QAPI_H
 
+#include "block/block-common.h"
 #include "block/graph-lock.h"
 #include "block/snapshot.h"
 #include "qapi/qapi-types-block-core.h"
@@ -49,4 +50,11 @@  void bdrv_image_info_specific_dump(ImageInfoSpecific *info_spec,
                                    const char *prefix,
                                    int indentation);
 void bdrv_node_info_dump(BlockNodeInfo *info, int indentation, bool protocol);
+
+void coroutine_fn GRAPH_RDLOCK
+bdrv_co_do_query_node_info(BlockDriverState *bs, BlockNodeInfo *info,
+                           Error **errp);
+void co_wrapper_bdrv_rdlock
+bdrv_do_query_node_info(BlockDriverState *bs, BlockNodeInfo *info,
+                        Error **errp);
 #endif