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

[Markus Armbruster <armbru@redhat.com>]

zoudongjie <zoudongjie@huawei.com> writes:

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

Please do.

Should all these commands support time out?

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

I don't get what "disable I/O limits" is trying to tell the user.  Can
you explain?

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