[Qemu-devel] [PATCH 10/12] block: Drain throttling queue with BdrvChild callback

[Kevin Wolf <kwolf@redhat.com>]

This removes the last part of I/O throttling from block/io.c and moves
it to the BlockBackend.

When draining the queue of a BlockDriverState, we must make sure that no
new requests can come in for it. Request sources from outside the block
layer are disabled with aio_disable_external(), but the throttling queue
must be handled separately.

The two obvious options we have are either to implement a similar
mechanism in BlockBackend that queues requests and avoids to pass them
to the BDS, or to flush the whole queue. The first option seems nicer
and could prevent bypassing the I/O limit, but the second is closer to
what we're already doing on the BDS level, so this patch keeps it this
way for now.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/block-backend.c     | 29 +++++++++++++++++++++++++++--
 block/io.c                | 35 ++++++++---------------------------
 include/block/block_int.h |  4 ++--
 3 files changed, 37 insertions(+), 31 deletions(-)

diff --git a/block/block-backend.c b/block/block-backend.c
index d2bd268..c71ce4d 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -90,9 +90,12 @@ static void blk_root_inherit_options(int *child_flags, QDict *child_options,
     /* We're not supposed to call this function for root nodes */
     abort();
 }
+static bool blk_drain_throttling_queue(BdrvChild *child);
 
 static const BdrvChildRole child_root = {
-    .inherit_options = blk_root_inherit_options,
+    .inherit_options    = blk_root_inherit_options,
+
+    .drain_queue        = blk_drain_throttling_queue,
 };
 
 /*
@@ -1677,7 +1680,7 @@ void blk_set_io_limits(BlockBackend *blk, ThrottleConfig *cfg)
 void blk_io_limits_disable(BlockBackend *blk)
 {
     blk->public.io_limits_enabled = false;
-    bdrv_start_throttled_reqs(blk_bs(blk));
+    blk_drain_throttling_queue(blk->root);
     throttle_group_unregister_blk(blk);
 }
 
@@ -1705,3 +1708,25 @@ void blk_io_limits_update_group(BlockBackend *blk, const char *group)
     blk_io_limits_disable(blk);
     blk_io_limits_enable(blk, group);
 }
+
+/* this function drain all the throttled IOs */
+static bool blk_drain_throttling_queue(BdrvChild *child)
+{
+    BlockBackend *blk = child->opaque;
+    BlockBackendPublic *blkp = &blk->public;
+    bool drained = false;
+    bool enabled = blkp->io_limits_enabled;
+    int i;
+
+    blkp->io_limits_enabled = false;
+
+    for (i = 0; i < 2; i++) {
+        while (qemu_co_enter_next(&blkp->throttled_reqs[i])) {
+            drained = true;
+        }
+    }
+
+    blkp->io_limits_enabled = enabled;
+
+    return drained;
+}
diff --git a/block/io.c b/block/io.c
index 22dbb43..f6edab8 100644
--- a/block/io.c
+++ b/block/io.c
@@ -27,7 +27,6 @@
 #include "sysemu/block-backend.h"
 #include "block/blockjob.h"
 #include "block/block_int.h"
-#include "block/throttle-groups.h"
 #include "qemu/error-report.h"
 
 #define NOT_DONE 0x7fffffff /* used while emulated sync operation in progress */
@@ -56,31 +55,6 @@ static void coroutine_fn bdrv_co_do_rw(void *opaque);
 static int coroutine_fn bdrv_co_do_write_zeroes(BlockDriverState *bs,
     int64_t sector_num, int nb_sectors, BdrvRequestFlags flags);
 
-/* this function drain all the throttled IOs */
-bool bdrv_start_throttled_reqs(BlockDriverState *bs)
-{
-    if (!bs->blk) {
-        return false;
-    }
-
-    BlockBackendPublic *blkp = blk_get_public(bs->blk);
-    bool drained = false;
-    bool enabled = blk_get_public(bs->blk)->io_limits_enabled;
-    int i;
-
-    blkp->io_limits_enabled = false;
-
-    for (i = 0; i < 2; i++) {
-        while (qemu_co_enter_next(&blkp->throttled_reqs[i])) {
-            drained = true;
-        }
-    }
-
-    blkp->io_limits_enabled = enabled;
-
-    return drained;
-}
-
 void bdrv_setup_io_funcs(BlockDriver *bdrv)
 {
     /* Block drivers without coroutine functions need emulation */
@@ -2668,12 +2642,19 @@ void bdrv_io_unplug(BlockDriverState *bs)
 void bdrv_flush_io_queue(BlockDriverState *bs)
 {
     BlockDriver *drv = bs->drv;
+    BdrvChild *c;
+
     if (drv && drv->bdrv_flush_io_queue) {
         drv->bdrv_flush_io_queue(bs);
     } else if (bs->file) {
         bdrv_flush_io_queue(bs->file->bs);
     }
-    bdrv_start_throttled_reqs(bs);
+
+    QLIST_FOREACH(c, &bs->parents, next_parent) {
+        if (c->role->drain_queue) {
+            c->role->drain_queue(c);
+        }
+    }
 }
 
 void bdrv_drained_begin(BlockDriverState *bs)
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 1b3c310..e064d9d 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -355,6 +355,8 @@ typedef struct BdrvAioNotifier {
 struct BdrvChildRole {
     void (*inherit_options)(int *child_flags, QDict *child_options,
                             int parent_flags, QDict *parent_options);
+
+    bool (*drain_queue)(BdrvChild *child);
 };
 
 extern const BdrvChildRole child_file;
@@ -508,8 +510,6 @@ int get_tmp_filename(char *filename, int size);
 BlockDriver *bdrv_probe_all(const uint8_t *buf, int buf_size,
                             const char *filename);
 
-bool bdrv_start_throttled_reqs(BlockDriverState *bs);
-
 
 /**
  * bdrv_add_before_write_notifier:
-- 
1.8.3.1