Re: [PATCH] chardev: introduce 'reconnect-ms' and deprecate 'reconnect'

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Sep 04, 2024 at 08:19:13 +0300, Daniil Tatianin wrote:
> The 'reconnect' option only allows to specify the time in seconds,
> which is way too long for certain workflows.
> 
> We have a lightweight disk backend server, which takes about 20ms to
> live update, but due to this limitation in QEMU, previously the guest
> disk controller would hang for one second because it would take this
> long for QEMU to reinitialize the socket connection.
> 
> Introduce a new option called 'reconnect-ms', which is the same as
> 'reconnect', except the value is treated as milliseconds. These are
> mutually exclusive and specifying both results in an error.
> 
> 'reconnect' is also deprecated by this commit to make it possible to
> remove it in the future as to not keep two options that control the
> same thing.
> 
> Signed-off-by: Daniil Tatianin <d-tatianin@xxxxxxxxxxxxxx>
> ---

[...]

> diff --git a/qapi/char.json b/qapi/char.json
> index ef58445cee..7f117438c6 100644
> --- a/qapi/char.json
> +++ b/qapi/char.json
> @@ -273,7 +273,19 @@
>  #
>  # @reconnect: For a client socket, if a socket is disconnected, then
>  #     attempt a reconnect after the given number of seconds.  Setting
> -#     this to zero disables this function.  (default: 0) (Since: 2.2)
> +#     this to zero disables this function.  The use of this member is
> +#     deprecated, use @reconnect-ms instead. (default: 0) (Since: 2.2)
> +#
> +# @reconnect-ms: For a client socket, if a socket is disconnected,
> +#     then attempt a reconnect after the given number of milliseconds.
> +#     Setting this to zero disables this function.  This member is
> +#     mutually exclusive with @reconnect.
> +#     (default: 0) (Since: 9.2)
> +#
> +# Features:
> +#
> +# @deprecated: Member @reconnect is deprecated.  Use @reconnect-ms
> +#     instead.
>  #
>  # Since: 1.4
>  ##
> @@ -287,7 +299,8 @@
>              '*telnet': 'bool',
>              '*tn3270': 'bool',
>              '*websocket': 'bool',
> -            '*reconnect': 'int' },
> +            '*reconnect': { 'type': 'int', 'features': [ 'deprecated' ] },
> +            '*reconnect-ms': 'int' },
>    'base': 'ChardevCommon' }

This is a little off-topic:

So I wanted to make libvirt use the new parameter to stay ahead
deprecation. I've applied this patch to qemu, dumped capabilities and
pretty much expected a bunch of test cases in libvirt fail as they'd be
using a deprecated field as libvirt is supposed to validate everything.

And the test suite passed unexpectedly. I've dug further and noticed
that for some reason libvirt doesn't still use JSON parameters for
-chardev (which is the pre-requisite for validation).

I've also noticed that at some point I attempted to convert it over
witnessed by having an (unused) capability named QEMU_CAPS_CHARDEV_JSON
that I've introduced.

My questions are:
1) Does '-chardev' accept JSON identical to 'chardev-add' QMP command?

If yes:
2) Since when can that be used? (What can I use as a witness)
3) Are there any gotchas?

I wonder this as I'd love to finish that out, but I really don't fancy
digging into qemu to find a gotcha 3/4 of the way there.

Anyways, as I've already stated, this patch is okay for libvirt, but I
didn't review the implementation, thus, on behalf of libvirt:

ACKed-by: Peter Krempa <pkrempa@xxxxxxxxxx>



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux