Re: [PATCH v3 2/2] qapi/throttle: add timeout parameter for qmp_block_set_io_throttle()

[zoudongjie]

On Thu, 27 Mar 2025 09:04:51 +0100, Markus wrote:
> zoudongjie <zoudongjie@huawei.com> writes:
> 
> > On Wed, Mar 26, 2025 at 10:53:20 +0100, Markus wrote:
> >> zoudongjie via <qemu-devel@nongnu.org> writes:
> >> 
> >> > 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.
> 
> What other commands could similarly block?

In theory, any command may be blocked if it calls bdrv_drained_begin().
I did a quick check, qmp_block_resize() could similarly block, since it called
bdrv_drained_begin(), I'm going to verify it later.

> 
> >> > Therefore, we add a timeout parameter for qmp_block_set_io_throttle to 
> >> > control
> >> > its execution duration.
> >> 
> >> What's the QMP reply when a block_set_io_throttle command times out?
> >
> > The reply is:
> > {"id":"libvirt-484","error":{"class":"GenericError","desc":"Blk io limits 
> > disable timeout"}}
> 
> I find the error message confusing.  Suggest "command timed out".

This message doesn't provide more details about the error, especially for 
developers.
How about using "command timed out: disable I/O limits"

> 
> >> >
> >> > The default value of timeout is 0, that is infinite wait, consistent with
> >> > previous behavior.
> >> >
> >> > Signed-off-by: Zhu Yangyang <zhuyangyang14@huawei.com>
> 
> [...]