On Thu, 13 Mar, 2025 at 12:25:08 +0800, Stefan Hajnoczi wrote:
> On Sat, Mar 08, 2025 at 06:16:18PM +0800, zoudongjie wrote:
> > From: Zhu Yangyang <zhuyangyan...@huawei.com>
> >
> > bdrv_drained_begin() is blocked for a long time when network storage is used
> > and the network link has just failed.
> > Therefore, the timeout period is set here.
> >
> > Signed-off-by: Zhu Yangyang <zhuyangyan...@huawei.com>
> > ---
> > block/block-backend.c | 14 +++++++++++++-
> > block/qapi-system.c | 7 ++++++-
> > include/system/block-backend-global-state.h | 1 +
> > 3 files changed, 20 insertions(+), 2 deletions(-)
> >
> > diff --git a/block/block-backend.c b/block/block-backend.c
> > index 9288f7e1c6..409d4559c3 100644
> > --- a/block/block-backend.c
> > +++ b/block/block-backend.c
> > @@ -2691,20 +2691,32 @@ void blk_set_io_limits(BlockBackend *blk,
> > ThrottleConfig *cfg)
> > }
> >
> > void blk_io_limits_disable(BlockBackend *blk)
> > +{
> > + blk_io_limits_disable_timeout(blk, -1);
> > +}
> > +
> > +int blk_io_limits_disable_timeout(BlockBackend *blk, int64_t timeout_ms)
> > {
> > 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_ms);
> > + 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..96fa8e5e51 100644
> > --- a/block/qapi-system.c
> > +++ b/block/qapi-system.c
> > @@ -39,6 +39,8 @@
> > #include "system/block-backend.h"
> > #include "system/blockdev.h"
> >
> > +#define QMP_BLOCKING_TIMEOUT 5000 /* ms */
> > +
> > static BlockBackend *qmp_get_blk(const char *blk_name, const char *qdev_id,
> > Error **errp)
> > {
> > @@ -502,7 +504,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, QMP_BLOCKING_TIMEOUT) < 0) {
>
> Please add a timeout parameter to the QMP command instead of hardcoding
> the timeout. It should also support an infinite timeout and that should
> be the default (so that existing users aren't surprised with a new error
> they don't handle).
Okay, I'm going to modify it in patch v2.
>
> > + 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 9cc9b008ec..e55ea9dc64 100644
> > --- a/include/system/block-backend-global-state.h
> > +++ b/include/system/block-backend-global-state.h
> > @@ -111,6 +111,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, int64_t timeout_ms);
> > 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);
> > --
> > 2.33.0
> >
