Ping :) I think this one should be good to go now! On 9/5/24 11:57 AM, 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. > > Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov(a)yandex-team.ru&gt; > Acked-by: Peter Krempa <pkrempa(a)redhat.com&gt; > Signed-off-by: Daniil Tatianin <d-tatianin(a)yandex-team.ru&gt; > --- > > Changes since v0: > - Mention the deprecation in docs (Paolo) > > --- > chardev/char-socket.c | 33 ++++++++++++++++++++++++--------- > chardev/char.c | 3 +++ > docs/about/deprecated.rst | 6 ++++++ > include/chardev/char-socket.h | 2 +- > qapi/char.json | 17 +++++++++++++++-- > 5 files changed, 49 insertions(+), 12 deletions(-) > > diff --git a/chardev/char-socket.c b/chardev/char-socket.c > index 1ca9441b1b..c24331ac23 100644 > --- a/chardev/char-socket.c > +++ b/chardev/char-socket.c > @@ -74,7 +74,7 @@ static void qemu_chr_socket_restart_timer(Chardev *chr) > assert(!s->reconnect_timer); > name = g_strdup_printf("chardev-socket-reconnect-%s", chr->label); > s->reconnect_timer = qemu_chr_timeout_add_ms(chr, > - s->reconnect_time * 1000, > + s->reconnect_time_ms, > socket_reconnect_timeout, > chr); > g_source_set_name(s->reconnect_timer, name); > @@ -481,7 +481,7 @@ static void tcp_chr_disconnect_locked(Chardev *chr) > if (emit_close) { > qemu_chr_be_event(chr, CHR_EVENT_CLOSED); > } > - if (s->reconnect_time && !s->reconnect_timer) { > + if (s->reconnect_time_ms && !s->reconnect_timer) { > qemu_chr_socket_restart_timer(chr); > } > } > @@ -1080,9 +1080,9 @@ static int tcp_chr_wait_connected(Chardev *chr, Error **errp) > } else { > Error *err = NULL; > if (tcp_chr_connect_client_sync(chr, &err) < 0) { > - if (s->reconnect_time) { > + if (s->reconnect_time_ms) { > error_free(err); > - g_usleep(s->reconnect_time * 1000ULL * 1000ULL); > + g_usleep(s->reconnect_time_ms * 1000ULL); > } else { > error_propagate(errp, err); > return -1; > @@ -1267,13 +1267,13 @@ skip_listen: > > > static int qmp_chardev_open_socket_client(Chardev *chr, > - int64_t reconnect, > + int64_t reconnect_ms, > Error **errp) > { > SocketChardev *s = SOCKET_CHARDEV(chr); > > - if (reconnect > 0) { > - s->reconnect_time = reconnect; > + if (reconnect_ms > 0) { > + s->reconnect_time_ms = reconnect_ms; > tcp_chr_connect_client_async(chr); > return 0; > } else { > @@ -1371,7 +1371,7 @@ static void qmp_chardev_open_socket(Chardev *chr, > bool is_tn3270 = sock->has_tn3270 ? sock->tn3270 : false; > bool is_waitconnect = sock->has_wait ? sock->wait : false; > bool is_websock = sock->has_websocket ? sock->websocket : false; > - int64_t reconnect = sock->has_reconnect ? sock->reconnect : 0; > + int64_t reconnect_ms = 0; > SocketAddress *addr; > > s->is_listen = is_listen; > @@ -1443,7 +1443,13 @@ static void qmp_chardev_open_socket(Chardev *chr, > return; > } > } else { > - if (qmp_chardev_open_socket_client(chr, reconnect, errp) < 0) { > + if (sock->has_reconnect) { > + reconnect_ms = sock->reconnect * 1000ULL; > + } else if (sock->has_reconnect_ms) { > + reconnect_ms = sock->reconnect_ms; > + } > + > + if (qmp_chardev_open_socket_client(chr, reconnect_ms, errp) < 0) { > return; > } > } > @@ -1509,6 +1515,15 @@ static void qemu_chr_parse_socket(QemuOpts *opts, ChardevBackend *backend, > sock->wait = qemu_opt_get_bool(opts, "wait", true); > sock->has_reconnect = qemu_opt_find(opts, "reconnect"); > sock->reconnect = qemu_opt_get_number(opts, "reconnect", 0); > + sock->has_reconnect_ms = qemu_opt_find(opts, "reconnect-ms"); > + sock->reconnect_ms = qemu_opt_get_number(opts, "reconnect-ms", 0); > + > + if (sock->has_reconnect_ms && sock->has_reconnect) { > + error_setg(errp, > + "'reconnect' and 'reconnect-ms' are mutually exclusive"); > + return; > + } > + > sock->tls_creds = g_strdup(qemu_opt_get(opts, "tls-creds")); > sock->tls_authz = g_strdup(qemu_opt_get(opts, "tls-authz")); > > diff --git a/chardev/char.c b/chardev/char.c > index ba847b6e9e..35623c78a3 100644 > --- a/chardev/char.c > +++ b/chardev/char.c > @@ -888,6 +888,9 @@ QemuOptsList qemu_chardev_opts = { > },{ > .name = "reconnect", > .type = QEMU_OPT_NUMBER, > + },{ > + .name = "reconnect-ms", > + .type = QEMU_OPT_NUMBER, > },{ > .name = "telnet", > .type = QEMU_OPT_BOOL, > diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst > index 88f0f03786..e5db9bc6e9 100644 > --- a/docs/about/deprecated.rst > +++ b/docs/about/deprecated.rst > @@ -430,6 +430,12 @@ Backend ``memory`` (since 9.0) > > ``memory`` is a deprecated synonym for ``ringbuf``. > > +``reconnect`` (since 9.2) > +^^^^^^^^^^^^^^^^^^^^^^^^^ > + > +The ``reconnect`` option only allows specifiying second granularity timeouts, > +which is not enough for all types of use cases, use ``reconnect-ms`` instead. > + > CPU device properties > ''''''''''''''''''''' > > diff --git a/include/chardev/char-socket.h b/include/chardev/char-socket.h > index 0708ca6fa9..d6d13ad37f 100644 > --- a/include/chardev/char-socket.h > +++ b/include/chardev/char-socket.h > @@ -74,7 +74,7 @@ struct SocketChardev { > bool is_websock; > > GSource *reconnect_timer; > - int64_t reconnect_time; > + int64_t reconnect_time_ms; > bool connect_err_reported; > > QIOTask *connect_task; > 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' } > > ##