chardev/char-socket.c | 24 +++++------------------- chardev/char.c | 3 --- docs/about/deprecated.rst | 15 --------------- docs/about/removed-features.rst | 22 ++++++++++++++++++++++ net/stream.c | 20 +++++--------------- qapi/char.json | 14 +------------- qapi/net.json | 13 +------------ 7 files changed, 34 insertions(+), 77 deletions(-)
They were deprecated in 9.2, now we can remove them. New options to use are reconnect-ms. v2: fixup docs and error messages Vladimir Sementsov-Ogievskiy (2): chardev: remove deprecated 'reconnect' option net/stream: remove deprecated 'reconnect' option chardev/char-socket.c | 24 +++++------------------- chardev/char.c | 3 --- docs/about/deprecated.rst | 15 --------------- docs/about/removed-features.rst | 22 ++++++++++++++++++++++ net/stream.c | 20 +++++--------------- qapi/char.json | 14 +------------- qapi/net.json | 13 +------------ 7 files changed, 34 insertions(+), 77 deletions(-) -- 2.48.1
Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes: > They were deprecated in 9.2, now we can remove them. > New options to use are reconnect-ms. Acked-by: Markus Armbruster <armbru@redhat.com> Who would like to merge this? * Marc-André, since PATCH 1 is chardev * Jason, since PATCH 2 is net * Myself, since both touch qapi/ * qemu-trivial
On 10/9/25 17:17, Markus Armbruster wrote: > Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> 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 :) 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. Thanks, /mjt
On 10.10.25 10:52, Michael Tokarev wrote:
> On 10/9/25 17:17, Markus Armbruster wrote:
>> Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> 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
Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:
> On 10.10.25 10:52, Michael Tokarev wrote:
>> On 10/9/25 17:17, Markus Armbruster wrote:
>>> Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> 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;
>
> ?
We routinely exploit that QAPI initializes absent members to zero.
> 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)
The simple way to do "optional" is to have the machinery supply a
default value.
The less simple way is to add a distinct extra value that means
"absent". This permits "absent" to means something else than any value.
QAPI does the latter. Not my choice; I inherited it :)
For pointers, generated C uses null as distinct extra value. Works,
because "present" implies non-null.
For other types, generated C uses a pair of variables (has_FOO, FOO),
where
(true, V) means present with value V
(false, zero) means absent
(false, non-zero) is invalid
This results in slightly more complicated code.
Most of the time, code maps "absent" to a default value. This default
value is not visible in the schema, it's buried in the C code. When a
type gets used in multiple places, each place can pick its own default.
Bothersome to document, and the system cannot ensure the code matches
its documentation. Strong dislike.
Special case: when the default value is zero, we can ignore has_FOO and
just use FOO. See "routinely exploit" above.
We could extend the QAPI schema language to let us specify the default
value. The generator could then elide has_FOO. Code would become
simpler.
On 10.10.25 14:24, Markus Armbruster wrote:
> Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:
>
>> On 10.10.25 10:52, Michael Tokarev wrote:
>>> On 10/9/25 17:17, Markus Armbruster wrote:
>>>> Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> 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;
>>
>> ?
>
> We routinely exploit that QAPI initializes absent members to zero.
What I'm afraid of: generated code is not the only source of QAPI structures,
sometimes they are initialized by hand. And looking at code like
bool is_telnet = sock->has_telnet ? sock->telnet : false;
I can't say, does the structure comes from generated code and this check
is redundant, or the structure may come from other place, and we chose
be explicitly safe here..
>
>> 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)
>
> The simple way to do "optional" is to have the machinery supply a
> default value.
>
> The less simple way is to add a distinct extra value that means
> "absent". This permits "absent" to means something else than any value.
>
> QAPI does the latter. Not my choice; I inherited it :)
>
> For pointers, generated C uses null as distinct extra value. Works,
> because "present" implies non-null.
>
> For other types, generated C uses a pair of variables (has_FOO, FOO),
> where
>
> (true, V) means present with value V
> (false, zero) means absent
> (false, non-zero) is invalid
Existing of invalid combinations is always a headache
>
> This results in slightly more complicated code.
>
> Most of the time, code maps "absent" to a default value. This default
> value is not visible in the schema, it's buried in the C code. When a
> type gets used in multiple places, each place can pick its own default.
> Bothersome to document, and the system cannot ensure the code matches
> its documentation. Strong dislike.
>
> Special case: when the default value is zero, we can ignore has_FOO and
> just use FOO. See "routinely exploit" above.
>
> We could extend the QAPI schema language to let us specify the default
> value. The generator could then elide has_FOO. Code would become
> simpler.
>
Yes, I meant something like this. May be in some future day... Some AI agent
will come with patches, after reading our discussion)
--
Best regards,
Vladimir
On 10/9/25 17:17, Markus Armbruster wrote: > Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes: > >> They were deprecated in 9.2, now we can remove them. >> New options to use are reconnect-ms. Reviewed-by: Michael Tokarev <mjt@tls.msk.ru> > Who would like to merge this? > > * Marc-André, since PATCH 1 is chardev > * Jason, since PATCH 2 is net > * Myself, since both touch qapi/ > * qemu-trivial I picked it up for qemu-trivial, but feel free to merge it through other means (I'll just rebase before sending pullreq). Mind you, qemu-trivial might be slow at times since I tend to collect more changes before sending a pullreq. Thanks, /mjt
ping) On 24.09.25 16:33, Vladimir Sementsov-Ogievskiy wrote: > They were deprecated in 9.2, now we can remove them. > New options to use are reconnect-ms. > > v2: fixup docs and error messages > > Vladimir Sementsov-Ogievskiy (2): > chardev: remove deprecated 'reconnect' option > net/stream: remove deprecated 'reconnect' option > > chardev/char-socket.c | 24 +++++------------------- > chardev/char.c | 3 --- > docs/about/deprecated.rst | 15 --------------- > docs/about/removed-features.rst | 22 ++++++++++++++++++++++ > net/stream.c | 20 +++++--------------- > qapi/char.json | 14 +------------- > qapi/net.json | 13 +------------ > 7 files changed, 34 insertions(+), 77 deletions(-) > -- Best regards, Vladimir
Hi On Wed, Sep 24, 2025 at 5:33 PM Vladimir Sementsov-Ogievskiy < vsementsov@yandex-team.ru> wrote: > They were deprecated in 9.2, now we can remove them. > New options to use are reconnect-ms. > > v2: fixup docs and error messages > > Vladimir Sementsov-Ogievskiy (2): > chardev: remove deprecated 'reconnect' option > net/stream: remove deprecated 'reconnect' option > > chardev/char-socket.c | 24 +++++------------------- > chardev/char.c | 3 --- > docs/about/deprecated.rst | 15 --------------- > docs/about/removed-features.rst | 22 ++++++++++++++++++++++ > net/stream.c | 20 +++++--------------- > qapi/char.json | 14 +------------- > qapi/net.json | 13 +------------ > 7 files changed, 34 insertions(+), 77 deletions(-) > > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> > -- > 2.48.1 > >
On a Wednesday in 2025, Vladimir Sementsov-Ogievskiy wrote: >They were deprecated in 9.2, now we can remove them. >New options to use are reconnect-ms. > >v2: fixup docs and error messages > >Vladimir Sementsov-Ogievskiy (2): > chardev: remove deprecated 'reconnect' option > net/stream: remove deprecated 'reconnect' option > > chardev/char-socket.c | 24 +++++------------------- > chardev/char.c | 3 --- > docs/about/deprecated.rst | 15 --------------- > docs/about/removed-features.rst | 22 ++++++++++++++++++++++ > net/stream.c | 20 +++++--------------- > qapi/char.json | 14 +------------- > qapi/net.json | 13 +------------ > 7 files changed, 34 insertions(+), 77 deletions(-) > Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
© 2016 - 2026 Red Hat, Inc.