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
> > 

Reply via email to