On 10.10.25 10:52, Michael Tokarev wrote:
On 10/9/25 17:17, Markus Armbruster wrote:
Vladimir Sementsov-Ogievskiy <[email protected]> writes:

They were deprecated in 9.2, now we can remove them.
New options to use are reconnect-ms.

Speaking of the option itself.. I'd not remove it, instead,
I'd de-deprecate it, and allow units to be specified for it,
like reconnect=10ms (defaults to s).  Or reconnect=0.1 (in
fractions of second).  But it's just me, it looks like :)

QAPI is not for humans) So simple milliseconds integer is better,
then parse the variety of suffixes. And it better fit into json
(number is number, not a string)


Also, `has_reconnect_ms` becomes redundant after applying this
patch, - it should be enough to use just reconnect_ms, which
defaults to 0.  But this can be done in a subsequent cleanup.


You mean just use sock->reconnect_ms instead of explicit

   int64_t reconnect_ms = sock->has_reconnect_ms ? sock->reconnect_ms : 0;

?

I believe that QMP will zero-initialize everything. But I'm not sure
that we do zero initialize this structure on all paths.. Keeping also in mind
handling other fields here like

    bool is_telnet      = sock->has_telnet  ? sock->telnet  : false;
    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;

To drop this, we should check for all paths, that incoming structure is
zero-initialized. And no guarantee that it does not break in future.

Probably, we can implement a new QAPI feature "value with default to zero",
so that we can avoid existence of .has_foo field at all for such fields.
No field - no problem.. But not in this series)

--
Best regards,
Vladimir

Reply via email to