diff mbox series

[v2,2/2] qapi/throttle: add timeout parameter for qmp_block_set_io_throttle()

Message ID 20250321070917.2889854-3-zoudongjie@huawei.com (mailing list archive)
State New
Headers show
Series qapi/throttle: Fix qmp_block_set_io_throttle blocked for too long | expand

Commit Message

zoudongjie March 21, 2025, 7:09 a.m. UTC 
From: Zhu Yangyang <zhuyangyang14@huawei.com>

Calling qmp_block_set_io_throttle() will be blocked for a long time
when a network disk is configured and the network failure is just about
to occur.

Therefore, we add a timeout parameter for qmp_block_set_io_throttle to control
its execution duration.

The default value of timeout is 0, that is infinite wait, consistent with
previous behavior.

Signed-off-by: Zhu Yangyang <zhuyangyang14@huawei.com>
---
 block/block-backend.c                       | 14 +++++++++++++-
 block/qapi-system.c                         | 10 +++++++++-
 include/system/block-backend-global-state.h |  1 +
 qapi/block-core.json                        |  5 ++++-
 4 files changed, 27 insertions(+), 3 deletions(-)

Comments

Stefan Hajnoczi March 24, 2025, 6:12 p.m. UTC | #1 
On Fri, Mar 21, 2025 at 03:09:17PM +0800, zoudongjie wrote:
> From: Zhu Yangyang <zhuyangyang14@huawei.com>
> 
> Calling qmp_block_set_io_throttle() will be blocked for a long time
> when a network disk is configured and the network failure is just about
> to occur.
> 
> Therefore, we add a timeout parameter for qmp_block_set_io_throttle to control
> its execution duration.
> 
> The default value of timeout is 0, that is infinite wait, consistent with
> previous behavior.
> 
> Signed-off-by: Zhu Yangyang <zhuyangyang14@huawei.com>
> ---
>  block/block-backend.c                       | 14 +++++++++++++-
>  block/qapi-system.c                         | 10 +++++++++-
>  include/system/block-backend-global-state.h |  1 +
>  qapi/block-core.json                        |  5 ++++-
>  4 files changed, 27 insertions(+), 3 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
diff mbox series

Patch

diff --git a/block/block-backend.c b/block/block-backend.c
index a402db13f2..fc1554ea55 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -2679,20 +2679,32 @@  void blk_set_io_limits(BlockBackend *blk, ThrottleConfig *cfg)
 }
 
 void blk_io_limits_disable(BlockBackend *blk)
+{
+    blk_io_limits_disable_timeout(blk, 0);
+}
+
+int blk_io_limits_disable_timeout(BlockBackend *blk, uint64_t timeout_ns)
 {
     BlockDriverState *bs = blk_bs(blk);
     ThrottleGroupMember *tgm = &blk->public.throttle_group_member;
+    int ret = 0;
+
     assert(tgm->throttle_state);
     GLOBAL_STATE_CODE();
     if (bs) {
         bdrv_ref(bs);
-        bdrv_drained_begin(bs);
+        ret = bdrv_drained_begin_timeout(bs, timeout_ns);
+        if (ret < 0) {
+            goto out;
+        }
     }
     throttle_group_unregister_tgm(tgm);
+out:
     if (bs) {
         bdrv_drained_end(bs);
         bdrv_unref(bs);
     }
+    return ret;
 }
 
 /* should be called before blk_set_io_limits if a limit is set */
diff --git a/block/qapi-system.c b/block/qapi-system.c
index 54b7409b2b..3b7ba0959c 100644
--- a/block/qapi-system.c
+++ b/block/qapi-system.c
@@ -423,6 +423,7 @@  void qmp_block_set_io_throttle(BlockIOThrottle *arg, Error **errp)
     ThrottleConfig cfg;
     BlockDriverState *bs;
     BlockBackend *blk;
+    uint64_t timeout_ns;
 
     blk = qmp_get_blk(arg->device, arg->id, errp);
     if (!blk) {
@@ -444,6 +445,10 @@  void qmp_block_set_io_throttle(BlockIOThrottle *arg, Error **errp)
     cfg.buckets[THROTTLE_OPS_READ].avg  = arg->iops_rd;
     cfg.buckets[THROTTLE_OPS_WRITE].avg = arg->iops_wr;
 
+    timeout_ns = 0; /* 0 means infinite */
+    if (arg->has_timeout) {
+        timeout_ns = arg->timeout * NANOSECONDS_PER_SECOND;
+    }
     if (arg->has_bps_max) {
         cfg.buckets[THROTTLE_BPS_TOTAL].max = arg->bps_max;
     }
@@ -502,7 +507,10 @@  void qmp_block_set_io_throttle(BlockIOThrottle *arg, Error **errp)
         blk_set_io_limits(blk, &cfg);
     } else if (blk_get_public(blk)->throttle_group_member.throttle_state) {
         /* If all throttling settings are set to 0, disable I/O limits */
-        blk_io_limits_disable(blk);
+        if (blk_io_limits_disable_timeout(blk, timeout_ns) < 0) {
+            error_setg(errp, "Blk io limits disable timeout");
+            return;
+        }
     }
 }
 
diff --git a/include/system/block-backend-global-state.h b/include/system/block-backend-global-state.h
index 35b5e8380b..8a2a6cbda4 100644
--- a/include/system/block-backend-global-state.h
+++ b/include/system/block-backend-global-state.h
@@ -110,6 +110,7 @@  int blk_probe_geometry(BlockBackend *blk, HDGeometry *geo);
 
 void blk_set_io_limits(BlockBackend *blk, ThrottleConfig *cfg);
 void blk_io_limits_disable(BlockBackend *blk);
+int blk_io_limits_disable_timeout(BlockBackend *blk, uint64_t timeout_ns);
 void blk_io_limits_enable(BlockBackend *blk, const char *group);
 void blk_io_limits_update_group(BlockBackend *blk, const char *group);
 void blk_set_force_allow_inactivate(BlockBackend *blk);
diff --git a/qapi/block-core.json b/qapi/block-core.json
index b1937780e1..88ef593efd 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2626,6 +2626,9 @@ 
 #
 # @group: throttle group name (Since 2.4)
 #
+# @timeout: In seconds. Timeout for block_set_io_throttle,
+#     0 means no limit. Defaults to 0. (Since 10.0)
+#
 # Features:
 #
 # @deprecated: Member @device is deprecated.  Use @id instead.
@@ -2642,7 +2645,7 @@ 
             '*bps_max_length': 'int', '*bps_rd_max_length': 'int',
             '*bps_wr_max_length': 'int', '*iops_max_length': 'int',
             '*iops_rd_max_length': 'int', '*iops_wr_max_length': 'int',
-            '*iops_size': 'int', '*group': 'str' } }
+            '*iops_size': 'int', '*group': 'str', '*timeout': 'uint32'} }
 
 ##
 # @ThrottleLimits: