All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] remove deprecated 'reconnect' options
@ 2025-09-24 12:44 Vladimir Sementsov-Ogievskiy
  0 siblings, 0 replies; 15+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-09-24 12:44 UTC (permalink / raw)
  To: armbru
  Cc: qemu-devel, eblake, jasowang, devel, pbonzini, marcandre.lureau,
	yc-core, vsementsov, d-tatianin

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

Vladimir Sementsov-Ogievskiy (2):
  chardev: remove deprecated 'reconnect' option
  net/stream: remove deprecated 'reconnect' option

 chardev/char-socket.c           | 20 +++-----------------
 chardev/char.c                  |  3 ---
 docs/about/deprecated.rst       | 15 ---------------
 docs/about/removed-features.rst | 22 ++++++++++++++++++++++
 net/stream.c                    | 20 +++++---------------
 qapi/char.json                  | 11 -----------
 qapi/net.json                   | 10 ----------
 7 files changed, 30 insertions(+), 71 deletions(-)

-- 
2.48.1



^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 0/2] remove deprecated 'reconnect' options
@ 2025-09-24 13:33 Vladimir Sementsov-Ogievskiy
  2025-09-24 13:33 ` [PATCH v2 1/2] chardev: remove deprecated 'reconnect' option Vladimir Sementsov-Ogievskiy
                   ` (5 more replies)
  0 siblings, 6 replies; 15+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-09-24 13:33 UTC (permalink / raw)
  To: armbru
  Cc: qemu-devel, eblake, jasowang, devel, pbonzini, marcandre.lureau,
	yc-core, vsementsov, d-tatianin

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



^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v2 1/2] chardev: remove deprecated 'reconnect' option
  2025-09-24 13:33 [PATCH 0/2] remove deprecated 'reconnect' options Vladimir Sementsov-Ogievskiy
@ 2025-09-24 13:33 ` Vladimir Sementsov-Ogievskiy
  2025-09-24 13:38   ` Daniil Tatianin
  2025-09-24 13:33 ` [PATCH v2 2/2] net/stream: " Vladimir Sementsov-Ogievskiy
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 15+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-09-24 13:33 UTC (permalink / raw)
  To: armbru
  Cc: qemu-devel, eblake, jasowang, devel, pbonzini, marcandre.lureau,
	yc-core, vsementsov, d-tatianin

It was deprecated in 9.2, time to remove.

Note, that (which become obvious with this commit) we forget to do some
checks for reconnect-ms options, for example, it was silently ignored
for listening server, instead of error-out. The commit fixes this, as
now we use reconnect_ms everywhere.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 chardev/char-socket.c           | 24 +++++-------------------
 chardev/char.c                  |  3 ---
 docs/about/deprecated.rst       |  6 ------
 docs/about/removed-features.rst | 12 ++++++++++++
 qapi/char.json                  | 14 +-------------
 5 files changed, 18 insertions(+), 41 deletions(-)

diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index cb4ec78ebe..62852e3caf 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -1296,9 +1296,9 @@ static bool qmp_chardev_validate_socket(ChardevSocket *sock,
     /* Validate any options which have a dependency on address type */
     switch (addr->type) {
     case SOCKET_ADDRESS_TYPE_FD:
-        if (sock->has_reconnect) {
+        if (sock->has_reconnect_ms) {
             error_setg(errp,
-                       "'reconnect' option is incompatible with "
+                       "'reconnect-ms' option is incompatible with "
                        "'fd' address type");
             return false;
         }
@@ -1342,9 +1342,9 @@ static bool qmp_chardev_validate_socket(ChardevSocket *sock,
 
     /* Validate any options which have a dependency on client vs server */
     if (!sock->has_server || sock->server) {
-        if (sock->has_reconnect) {
+        if (sock->has_reconnect_ms) {
             error_setg(errp,
-                       "'reconnect' option is incompatible with "
+                       "'reconnect-ms' option is incompatible with "
                        "socket in server listen mode");
             return false;
         }
@@ -1361,12 +1361,6 @@ static bool qmp_chardev_validate_socket(ChardevSocket *sock,
         }
     }
 
-    if (sock->has_reconnect_ms && sock->has_reconnect) {
-        error_setg(errp,
-            "'reconnect' and 'reconnect-ms' are mutually exclusive");
-        return false;
-    }
-
     return true;
 }
 
@@ -1384,7 +1378,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_ms = 0;
+    int64_t reconnect_ms = sock->has_reconnect_ms ? sock->reconnect_ms : 0;
     SocketAddress *addr;
 
     s->is_listen = is_listen;
@@ -1456,12 +1450,6 @@ static void qmp_chardev_open_socket(Chardev *chr,
             return;
         }
     } else {
-        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;
         }
@@ -1526,8 +1514,6 @@ static void qemu_chr_parse_socket(QemuOpts *opts, ChardevBackend *backend,
      */
     sock->has_wait = qemu_opt_find(opts, "wait") || sock->server;
     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);
 
diff --git a/chardev/char.c b/chardev/char.c
index bbebd246c3..a43b7e5481 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -892,9 +892,6 @@ QemuOptsList qemu_chardev_opts = {
         },{
             .name = "nodelay",
             .type = QEMU_OPT_BOOL,
-        },{
-            .name = "reconnect",
-            .type = QEMU_OPT_NUMBER,
         },{
             .name = "reconnect-ms",
             .type = QEMU_OPT_NUMBER,
diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index aa300bbd50..ba0be97513 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -438,12 +438,6 @@ Backend ``memory`` (since 9.0)
 
 ``memory`` is a deprecated synonym for ``ringbuf``.
 
-``reconnect`` (since 9.2)
-^^^^^^^^^^^^^^^^^^^^^^^^^
-
-The ``reconnect`` option only allows specifying second granularity timeouts,
-which is not enough for all types of use cases, use ``reconnect-ms`` instead.
-
 
 Net device options
 ''''''''''''''''''
diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
index a5338e44c2..d67928956a 100644
--- a/docs/about/removed-features.rst
+++ b/docs/about/removed-features.rst
@@ -1361,4 +1361,16 @@ The ``blacklist`` config file option has been renamed to ``block-rpcs``
 (to be in sync with the renaming of the corresponding command line
 option).
 
+Device options
+--------------
+
+Character device options
+''''''''''''''''''''''''
+
+``reconnect`` (removed in 10.2)
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+The ``reconnect`` has been replaced by ``reconnect-ms``, which provides
+better precision.
+
 .. _Intel discontinuance notification: https://www.intel.com/content/www/us/en/content-details/781327/intel-is-discontinuing-ip-ordering-codes-listed-in-pdn2312-for-nios-ii-ip.html
diff --git a/qapi/char.json b/qapi/char.json
index f0a53f742c..b07e3bb827 100644
--- a/qapi/char.json
+++ b/qapi/char.json
@@ -269,22 +269,11 @@
 # @websocket: enable websocket protocol on server sockets
 #     (default: false) (Since: 3.1)
 #
-# @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.  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.
+#     Setting this to zero disables this function.
 #     (default: 0) (Since: 9.2)
 #
-# Features:
-#
-# @deprecated: Member @reconnect is deprecated.  Use @reconnect-ms
-#     instead.
-#
 # Since: 1.4
 ##
 { 'struct': 'ChardevSocket',
@@ -297,7 +286,6 @@
             '*telnet': 'bool',
             '*tn3270': 'bool',
             '*websocket': 'bool',
-            '*reconnect': { 'type': 'int', 'features': [ 'deprecated' ] },
             '*reconnect-ms': 'int' },
   'base': 'ChardevCommon' }
 
-- 
2.48.1



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 2/2] net/stream: remove deprecated 'reconnect' option
  2025-09-24 13:33 [PATCH 0/2] remove deprecated 'reconnect' options Vladimir Sementsov-Ogievskiy
  2025-09-24 13:33 ` [PATCH v2 1/2] chardev: remove deprecated 'reconnect' option Vladimir Sementsov-Ogievskiy
@ 2025-09-24 13:33 ` Vladimir Sementsov-Ogievskiy
  2025-09-24 13:38   ` Daniil Tatianin
  2025-09-24 13:50 ` [PATCH 0/2] remove deprecated 'reconnect' options Ján Tomko
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 15+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-09-24 13:33 UTC (permalink / raw)
  To: armbru
  Cc: qemu-devel, eblake, jasowang, devel, pbonzini, marcandre.lureau,
	yc-core, vsementsov, d-tatianin

It was deprecated in 9.2, time to remove.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 docs/about/deprecated.rst       |  9 ---------
 docs/about/removed-features.rst | 10 ++++++++++
 net/stream.c                    | 20 +++++---------------
 qapi/net.json                   | 13 +------------
 4 files changed, 16 insertions(+), 36 deletions(-)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index ba0be97513..4452c08bf5 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -439,15 +439,6 @@ Backend ``memory`` (since 9.0)
 ``memory`` is a deprecated synonym for ``ringbuf``.
 
 
-Net device options
-''''''''''''''''''
-
-Stream ``reconnect`` (since 9.2)
-^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-
-The ``reconnect`` option only allows specifying second granularity timeouts,
-which is not enough for all types of use cases, use ``reconnect-ms`` instead.
-
 CPU device properties
 '''''''''''''''''''''
 
diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
index d67928956a..ae7d7287fc 100644
--- a/docs/about/removed-features.rst
+++ b/docs/about/removed-features.rst
@@ -1373,4 +1373,14 @@ Character device options
 The ``reconnect`` has been replaced by ``reconnect-ms``, which provides
 better precision.
 
+Net device options
+''''''''''''''''''
+
+Stream ``reconnect`` (removed in 10.2)
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+The ``reconnect`` has been replaced by ``reconnect-ms``, which provides
+better precision.
+
+
 .. _Intel discontinuance notification: https://www.intel.com/content/www/us/en/content-details/781327/intel-is-discontinuing-ip-ordering-codes-listed-in-pdn2312-for-nios-ii-ip.html
diff --git a/net/stream.c b/net/stream.c
index 94f823a2a7..ea83f4a763 100644
--- a/net/stream.c
+++ b/net/stream.c
@@ -274,23 +274,13 @@ int net_init_stream(const Netdev *netdev, const char *name,
     sock = &netdev->u.stream;
 
     if (!sock->has_server || !sock->server) {
-        uint32_t reconnect_ms = 0;
-
-        if (sock->has_reconnect && sock->has_reconnect_ms) {
-            error_setg(errp, "'reconnect' and 'reconnect-ms' are mutually "
-                             "exclusive");
-            return -1;
-        } else if (sock->has_reconnect_ms) {
-            reconnect_ms = sock->reconnect_ms;
-        } else if (sock->has_reconnect) {
-            reconnect_ms = sock->reconnect * 1000u;
-        }
-
         return net_stream_client_init(peer, "stream", name, sock->addr,
-                                      reconnect_ms, errp);
+                                      sock->has_reconnect_ms ?
+                                          sock->reconnect_ms : 0,
+                                      errp);
     }
-    if (sock->has_reconnect || sock->has_reconnect_ms) {
-        error_setg(errp, "'reconnect' and 'reconnect-ms' options are "
+    if (sock->has_reconnect_ms) {
+        error_setg(errp, "'reconnect-ms' option is "
                          "incompatible with socket in server mode");
         return -1;
     }
diff --git a/qapi/net.json b/qapi/net.json
index 78bcc9871e..642d4d03cd 100644
--- a/qapi/net.json
+++ b/qapi/net.json
@@ -770,29 +770,18 @@
 #
 # @server: create server socket (default: false)
 #
-# @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 8.0)
-#
 # @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)
+#     this to zero disables this function.  (default: 0) (Since: 9.2)
 #
 # Only `SocketAddress` types 'unix', 'inet' and 'fd' are supported.
 #
-# Features:
-#
-# @deprecated: Member @reconnect is deprecated.  Use @reconnect-ms
-#     instead.
-#
 # Since: 7.2
 ##
 { 'struct': 'NetdevStreamOptions',
   'data': {
     'addr':   'SocketAddress',
     '*server': 'bool',
-    '*reconnect': { 'type': 'int', 'features': [ 'deprecated' ] },
     '*reconnect-ms': 'int' } }
 
 ##
-- 
2.48.1



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 1/2] chardev: remove deprecated 'reconnect' option
  2025-09-24 13:33 ` [PATCH v2 1/2] chardev: remove deprecated 'reconnect' option Vladimir Sementsov-Ogievskiy
@ 2025-09-24 13:38   ` Daniil Tatianin
  0 siblings, 0 replies; 15+ messages in thread
From: Daniil Tatianin @ 2025-09-24 13:38 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy, armbru
  Cc: qemu-devel, eblake, jasowang, devel, pbonzini, marcandre.lureau,
	yc-core

On 9/24/25 4:33 PM, Vladimir Sementsov-Ogievskiy wrote:

> It was deprecated in 9.2, time to remove.
>
> Note, that (which become obvious with this commit) we forget to do some
> checks for reconnect-ms options, for example, it was silently ignored
> for listening server, instead of error-out. The commit fixes this, as
> now we use reconnect_ms everywhere.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>

Reviewed-by: Daniil Tatianin <d-tatianin@yandex-team.ru>

> ---
>   chardev/char-socket.c           | 24 +++++-------------------
>   chardev/char.c                  |  3 ---
>   docs/about/deprecated.rst       |  6 ------
>   docs/about/removed-features.rst | 12 ++++++++++++
>   qapi/char.json                  | 14 +-------------
>   5 files changed, 18 insertions(+), 41 deletions(-)
>
> diff --git a/chardev/char-socket.c b/chardev/char-socket.c
> index cb4ec78ebe..62852e3caf 100644
> --- a/chardev/char-socket.c
> +++ b/chardev/char-socket.c
> @@ -1296,9 +1296,9 @@ static bool qmp_chardev_validate_socket(ChardevSocket *sock,
>       /* Validate any options which have a dependency on address type */
>       switch (addr->type) {
>       case SOCKET_ADDRESS_TYPE_FD:
> -        if (sock->has_reconnect) {
> +        if (sock->has_reconnect_ms) {
>               error_setg(errp,
> -                       "'reconnect' option is incompatible with "
> +                       "'reconnect-ms' option is incompatible with "
>                          "'fd' address type");
>               return false;
>           }
> @@ -1342,9 +1342,9 @@ static bool qmp_chardev_validate_socket(ChardevSocket *sock,
>   
>       /* Validate any options which have a dependency on client vs server */
>       if (!sock->has_server || sock->server) {
> -        if (sock->has_reconnect) {
> +        if (sock->has_reconnect_ms) {
>               error_setg(errp,
> -                       "'reconnect' option is incompatible with "
> +                       "'reconnect-ms' option is incompatible with "
>                          "socket in server listen mode");
>               return false;
>           }
> @@ -1361,12 +1361,6 @@ static bool qmp_chardev_validate_socket(ChardevSocket *sock,
>           }
>       }
>   
> -    if (sock->has_reconnect_ms && sock->has_reconnect) {
> -        error_setg(errp,
> -            "'reconnect' and 'reconnect-ms' are mutually exclusive");
> -        return false;
> -    }
> -
>       return true;
>   }
>   
> @@ -1384,7 +1378,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_ms = 0;
> +    int64_t reconnect_ms = sock->has_reconnect_ms ? sock->reconnect_ms : 0;
>       SocketAddress *addr;
>   
>       s->is_listen = is_listen;
> @@ -1456,12 +1450,6 @@ static void qmp_chardev_open_socket(Chardev *chr,
>               return;
>           }
>       } else {
> -        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;
>           }
> @@ -1526,8 +1514,6 @@ static void qemu_chr_parse_socket(QemuOpts *opts, ChardevBackend *backend,
>        */
>       sock->has_wait = qemu_opt_find(opts, "wait") || sock->server;
>       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);
>   
> diff --git a/chardev/char.c b/chardev/char.c
> index bbebd246c3..a43b7e5481 100644
> --- a/chardev/char.c
> +++ b/chardev/char.c
> @@ -892,9 +892,6 @@ QemuOptsList qemu_chardev_opts = {
>           },{
>               .name = "nodelay",
>               .type = QEMU_OPT_BOOL,
> -        },{
> -            .name = "reconnect",
> -            .type = QEMU_OPT_NUMBER,
>           },{
>               .name = "reconnect-ms",
>               .type = QEMU_OPT_NUMBER,
> diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
> index aa300bbd50..ba0be97513 100644
> --- a/docs/about/deprecated.rst
> +++ b/docs/about/deprecated.rst
> @@ -438,12 +438,6 @@ Backend ``memory`` (since 9.0)
>   
>   ``memory`` is a deprecated synonym for ``ringbuf``.
>   
> -``reconnect`` (since 9.2)
> -^^^^^^^^^^^^^^^^^^^^^^^^^
> -
> -The ``reconnect`` option only allows specifying second granularity timeouts,
> -which is not enough for all types of use cases, use ``reconnect-ms`` instead.
> -
>   
>   Net device options
>   ''''''''''''''''''
> diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
> index a5338e44c2..d67928956a 100644
> --- a/docs/about/removed-features.rst
> +++ b/docs/about/removed-features.rst
> @@ -1361,4 +1361,16 @@ The ``blacklist`` config file option has been renamed to ``block-rpcs``
>   (to be in sync with the renaming of the corresponding command line
>   option).
>   
> +Device options
> +--------------
> +
> +Character device options
> +''''''''''''''''''''''''
> +
> +``reconnect`` (removed in 10.2)
> +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> +
> +The ``reconnect`` has been replaced by ``reconnect-ms``, which provides
> +better precision.
> +
>   .. _Intel discontinuance notification: https://www.intel.com/content/www/us/en/content-details/781327/intel-is-discontinuing-ip-ordering-codes-listed-in-pdn2312-for-nios-ii-ip.html
> diff --git a/qapi/char.json b/qapi/char.json
> index f0a53f742c..b07e3bb827 100644
> --- a/qapi/char.json
> +++ b/qapi/char.json
> @@ -269,22 +269,11 @@
>   # @websocket: enable websocket protocol on server sockets
>   #     (default: false) (Since: 3.1)
>   #
> -# @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.  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.
> +#     Setting this to zero disables this function.
>   #     (default: 0) (Since: 9.2)
>   #
> -# Features:
> -#
> -# @deprecated: Member @reconnect is deprecated.  Use @reconnect-ms
> -#     instead.
> -#
>   # Since: 1.4
>   ##
>   { 'struct': 'ChardevSocket',
> @@ -297,7 +286,6 @@
>               '*telnet': 'bool',
>               '*tn3270': 'bool',
>               '*websocket': 'bool',
> -            '*reconnect': { 'type': 'int', 'features': [ 'deprecated' ] },
>               '*reconnect-ms': 'int' },
>     'base': 'ChardevCommon' }
>   


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 2/2] net/stream: remove deprecated 'reconnect' option
  2025-09-24 13:33 ` [PATCH v2 2/2] net/stream: " Vladimir Sementsov-Ogievskiy
@ 2025-09-24 13:38   ` Daniil Tatianin
  0 siblings, 0 replies; 15+ messages in thread
From: Daniil Tatianin @ 2025-09-24 13:38 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy, armbru
  Cc: qemu-devel, eblake, jasowang, devel, pbonzini, marcandre.lureau,
	yc-core

On 9/24/25 4:33 PM, Vladimir Sementsov-Ogievskiy wrote:

> It was deprecated in 9.2, time to remove.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>

Reviewed-by: Daniil Tatianin <d-tatianin@yandex-team.ru>

> ---
>   docs/about/deprecated.rst       |  9 ---------
>   docs/about/removed-features.rst | 10 ++++++++++
>   net/stream.c                    | 20 +++++---------------
>   qapi/net.json                   | 13 +------------
>   4 files changed, 16 insertions(+), 36 deletions(-)
>
> diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
> index ba0be97513..4452c08bf5 100644
> --- a/docs/about/deprecated.rst
> +++ b/docs/about/deprecated.rst
> @@ -439,15 +439,6 @@ Backend ``memory`` (since 9.0)
>   ``memory`` is a deprecated synonym for ``ringbuf``.
>   
>   
> -Net device options
> -''''''''''''''''''
> -
> -Stream ``reconnect`` (since 9.2)
> -^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> -
> -The ``reconnect`` option only allows specifying second granularity timeouts,
> -which is not enough for all types of use cases, use ``reconnect-ms`` instead.
> -
>   CPU device properties
>   '''''''''''''''''''''
>   
> diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
> index d67928956a..ae7d7287fc 100644
> --- a/docs/about/removed-features.rst
> +++ b/docs/about/removed-features.rst
> @@ -1373,4 +1373,14 @@ Character device options
>   The ``reconnect`` has been replaced by ``reconnect-ms``, which provides
>   better precision.
>   
> +Net device options
> +''''''''''''''''''
> +
> +Stream ``reconnect`` (removed in 10.2)
> +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> +
> +The ``reconnect`` has been replaced by ``reconnect-ms``, which provides
> +better precision.
> +
> +
>   .. _Intel discontinuance notification: https://www.intel.com/content/www/us/en/content-details/781327/intel-is-discontinuing-ip-ordering-codes-listed-in-pdn2312-for-nios-ii-ip.html
> diff --git a/net/stream.c b/net/stream.c
> index 94f823a2a7..ea83f4a763 100644
> --- a/net/stream.c
> +++ b/net/stream.c
> @@ -274,23 +274,13 @@ int net_init_stream(const Netdev *netdev, const char *name,
>       sock = &netdev->u.stream;
>   
>       if (!sock->has_server || !sock->server) {
> -        uint32_t reconnect_ms = 0;
> -
> -        if (sock->has_reconnect && sock->has_reconnect_ms) {
> -            error_setg(errp, "'reconnect' and 'reconnect-ms' are mutually "
> -                             "exclusive");
> -            return -1;
> -        } else if (sock->has_reconnect_ms) {
> -            reconnect_ms = sock->reconnect_ms;
> -        } else if (sock->has_reconnect) {
> -            reconnect_ms = sock->reconnect * 1000u;
> -        }
> -
>           return net_stream_client_init(peer, "stream", name, sock->addr,
> -                                      reconnect_ms, errp);
> +                                      sock->has_reconnect_ms ?
> +                                          sock->reconnect_ms : 0,
> +                                      errp);
>       }
> -    if (sock->has_reconnect || sock->has_reconnect_ms) {
> -        error_setg(errp, "'reconnect' and 'reconnect-ms' options are "
> +    if (sock->has_reconnect_ms) {
> +        error_setg(errp, "'reconnect-ms' option is "
>                            "incompatible with socket in server mode");
>           return -1;
>       }
> diff --git a/qapi/net.json b/qapi/net.json
> index 78bcc9871e..642d4d03cd 100644
> --- a/qapi/net.json
> +++ b/qapi/net.json
> @@ -770,29 +770,18 @@
>   #
>   # @server: create server socket (default: false)
>   #
> -# @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 8.0)
> -#
>   # @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)
> +#     this to zero disables this function.  (default: 0) (Since: 9.2)
>   #
>   # Only `SocketAddress` types 'unix', 'inet' and 'fd' are supported.
>   #
> -# Features:
> -#
> -# @deprecated: Member @reconnect is deprecated.  Use @reconnect-ms
> -#     instead.
> -#
>   # Since: 7.2
>   ##
>   { 'struct': 'NetdevStreamOptions',
>     'data': {
>       'addr':   'SocketAddress',
>       '*server': 'bool',
> -    '*reconnect': { 'type': 'int', 'features': [ 'deprecated' ] },
>       '*reconnect-ms': 'int' } }
>   
>   ##


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/2] remove deprecated 'reconnect' options
  2025-09-24 13:33 [PATCH 0/2] remove deprecated 'reconnect' options Vladimir Sementsov-Ogievskiy
  2025-09-24 13:33 ` [PATCH v2 1/2] chardev: remove deprecated 'reconnect' option Vladimir Sementsov-Ogievskiy
  2025-09-24 13:33 ` [PATCH v2 2/2] net/stream: " Vladimir Sementsov-Ogievskiy
@ 2025-09-24 13:50 ` Ján Tomko
  2025-09-24 13:52 ` Marc-André Lureau
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 15+ messages in thread
From: Ján Tomko @ 2025-09-24 13:50 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy
  Cc: armbru, qemu-devel, eblake, jasowang, devel, pbonzini,
	marcandre.lureau, yc-core, d-tatianin

[-- Attachment #1: Type: text/plain, Size: 848 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/2] remove deprecated 'reconnect' options
  2025-09-24 13:33 [PATCH 0/2] remove deprecated 'reconnect' options Vladimir Sementsov-Ogievskiy
                   ` (2 preceding siblings ...)
  2025-09-24 13:50 ` [PATCH 0/2] remove deprecated 'reconnect' options Ján Tomko
@ 2025-09-24 13:52 ` Marc-André Lureau
  2025-10-09  8:17 ` Vladimir Sementsov-Ogievskiy
  2025-10-09 14:17 ` Markus Armbruster
  5 siblings, 0 replies; 15+ messages in thread
From: Marc-André Lureau @ 2025-09-24 13:52 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy
  Cc: armbru, qemu-devel, eblake, jasowang, devel, pbonzini, yc-core,
	d-tatianin

[-- Attachment #1: Type: text/plain, Size: 948 bytes --]

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

[-- Attachment #2: Type: text/html, Size: 1599 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/2] remove deprecated 'reconnect' options
  2025-09-24 13:33 [PATCH 0/2] remove deprecated 'reconnect' options Vladimir Sementsov-Ogievskiy
                   ` (3 preceding siblings ...)
  2025-09-24 13:52 ` Marc-André Lureau
@ 2025-10-09  8:17 ` Vladimir Sementsov-Ogievskiy
  2025-10-09 14:17 ` Markus Armbruster
  5 siblings, 0 replies; 15+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-10-09  8:17 UTC (permalink / raw)
  To: armbru
  Cc: qemu-devel, eblake, jasowang, devel, pbonzini, marcandre.lureau,
	yc-core, d-tatianin

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


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/2] remove deprecated 'reconnect' options
  2025-09-24 13:33 [PATCH 0/2] remove deprecated 'reconnect' options Vladimir Sementsov-Ogievskiy
                   ` (4 preceding siblings ...)
  2025-10-09  8:17 ` Vladimir Sementsov-Ogievskiy
@ 2025-10-09 14:17 ` Markus Armbruster
  2025-10-10  7:45   ` Michael Tokarev
  2025-10-10  7:52   ` Michael Tokarev
  5 siblings, 2 replies; 15+ messages in thread
From: Markus Armbruster @ 2025-10-09 14:17 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy
  Cc: qemu-devel, eblake, jasowang, devel, pbonzini, marcandre.lureau,
	yc-core, d-tatianin, qemu-trivial

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



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/2] remove deprecated 'reconnect' options
  2025-10-09 14:17 ` Markus Armbruster
@ 2025-10-10  7:45   ` Michael Tokarev
  2025-10-10  7:52   ` Michael Tokarev
  1 sibling, 0 replies; 15+ messages in thread
From: Michael Tokarev @ 2025-10-10  7:45 UTC (permalink / raw)
  To: Markus Armbruster, Vladimir Sementsov-Ogievskiy
  Cc: qemu-devel, eblake, jasowang, devel, pbonzini, marcandre.lureau,
	yc-core, d-tatianin, 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.

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



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/2] remove deprecated 'reconnect' options
  2025-10-09 14:17 ` Markus Armbruster
  2025-10-10  7:45   ` Michael Tokarev
@ 2025-10-10  7:52   ` Michael Tokarev
  2025-10-10  9:02     ` Vladimir Sementsov-Ogievskiy
  1 sibling, 1 reply; 15+ messages in thread
From: Michael Tokarev @ 2025-10-10  7:52 UTC (permalink / raw)
  To: Markus Armbruster, Vladimir Sementsov-Ogievskiy
  Cc: qemu-devel, eblake, jasowang, devel, pbonzini, marcandre.lureau,
	yc-core, d-tatianin, 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


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/2] remove deprecated 'reconnect' options
  2025-10-10  7:52   ` Michael Tokarev
@ 2025-10-10  9:02     ` Vladimir Sementsov-Ogievskiy
  2025-10-10 11:24       ` Markus Armbruster
  0 siblings, 1 reply; 15+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-10-10  9:02 UTC (permalink / raw)
  To: Michael Tokarev, Markus Armbruster
  Cc: qemu-devel, eblake, jasowang, devel, pbonzini, marcandre.lureau,
	yc-core, d-tatianin, qemu-trivial

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


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/2] remove deprecated 'reconnect' options
  2025-10-10  9:02     ` Vladimir Sementsov-Ogievskiy
@ 2025-10-10 11:24       ` Markus Armbruster
  2025-10-10 12:06         ` Vladimir Sementsov-Ogievskiy
  0 siblings, 1 reply; 15+ messages in thread
From: Markus Armbruster @ 2025-10-10 11:24 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy
  Cc: Michael Tokarev, qemu-devel, eblake, jasowang, devel, pbonzini,
	marcandre.lureau, yc-core, d-tatianin, qemu-trivial

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.



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/2] remove deprecated 'reconnect' options
  2025-10-10 11:24       ` Markus Armbruster
@ 2025-10-10 12:06         ` Vladimir Sementsov-Ogievskiy
  0 siblings, 0 replies; 15+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-10-10 12:06 UTC (permalink / raw)
  To: Markus Armbruster
  Cc: Michael Tokarev, qemu-devel, eblake, jasowang, devel, pbonzini,
	marcandre.lureau, yc-core, d-tatianin, qemu-trivial

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


^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2025-10-10 12:08 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-24 13:33 [PATCH 0/2] remove deprecated 'reconnect' options Vladimir Sementsov-Ogievskiy
2025-09-24 13:33 ` [PATCH v2 1/2] chardev: remove deprecated 'reconnect' option Vladimir Sementsov-Ogievskiy
2025-09-24 13:38   ` Daniil Tatianin
2025-09-24 13:33 ` [PATCH v2 2/2] net/stream: " Vladimir Sementsov-Ogievskiy
2025-09-24 13:38   ` Daniil Tatianin
2025-09-24 13:50 ` [PATCH 0/2] remove deprecated 'reconnect' options Ján Tomko
2025-09-24 13:52 ` Marc-André Lureau
2025-10-09  8:17 ` Vladimir Sementsov-Ogievskiy
2025-10-09 14:17 ` Markus Armbruster
2025-10-10  7:45   ` Michael Tokarev
2025-10-10  7:52   ` Michael Tokarev
2025-10-10  9:02     ` Vladimir Sementsov-Ogievskiy
2025-10-10 11:24       ` Markus Armbruster
2025-10-10 12:06         ` Vladimir Sementsov-Ogievskiy
  -- strict thread matches above, loose matches on Subject: below --
2025-09-24 12:44 Vladimir Sementsov-Ogievskiy

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.