[PATCH 0/2] finalize 'reconnect' deprecation
We've deprecated the 'reconnect' property in c8e2b6b4d7e, but all the tests, as well as the stream netdev were still left using it. This series finalizes the deprecation and replaces all local usage and docs metions of 'reconnect' with 'reconnect-ms'. Daniil Tatianin (2): net/stream: deprecate 'reconnect' in favor of 'reconnect-ms' chardev: finalize 'reconnect' deprecation docs/COLO-FT.txt | 4 +-- docs/about/deprecated.rst | 10 ++++++++ docs/system/ppc/powernv.rst | 2 +- net/stream.c | 34 +++++++++++++++++--------- qapi/net.json | 13 +++++++++- qemu-options.hx | 46 +++++++++++++++++------------------ tests/qtest/ipmi-bt-test.c | 2 +- tests/qtest/netdev-socket.c | 2 +- tests/qtest/vhost-user-test.c | 2 +- tests/unit/test-char.c | 8 +++--- 10 files changed, 77 insertions(+), 46 deletions(-) -- 2.34.1
Do the same thing we already did for chardev in c8e2b6b4d7e, and introduce a new 'reconnect-ms' option to make it possible to specify sub-second timeouts. This also changes the related documentaion and tests to use reconnect-ms as well. Signed-off-by: Daniil Tatianin <d-tatianin@yandex-team.ru> --- docs/about/deprecated.rst | 10 ++++++++++ net/stream.c | 34 ++++++++++++++++++++++------------ qapi/net.json | 13 ++++++++++++- qemu-options.hx | 24 ++++++++++++------------ tests/qtest/netdev-socket.c | 2 +- 5 files changed, 57 insertions(+), 26 deletions(-) diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst index ce38a3d0cf..1e1e9f5f18 100644 --- a/docs/about/deprecated.rst +++ b/docs/about/deprecated.rst @@ -400,6 +400,16 @@ Backend ``memory`` (since 9.0) The ``reconnect`` option only allows specifiying second granularity timeouts, which is not enough for all types of use cases, use ``reconnect-ms`` instead. + +Net device options +'''''''''''''''''' + +Stream ``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/net/stream.c b/net/stream.c index 97e6ec6679..4de5613844 100644 --- a/net/stream.c +++ b/net/stream.c @@ -51,7 +51,7 @@ typedef struct NetStreamState { guint ioc_write_tag; SocketReadState rs; unsigned int send_index; /* number of bytes sent*/ - uint32_t reconnect; + uint32_t reconnect_ms; guint timer_tag; SocketAddress *addr; } NetStreamState; @@ -387,10 +387,9 @@ static gboolean net_stream_reconnect(gpointer data) static void net_stream_arm_reconnect(NetStreamState *s) { - if (s->reconnect && s->timer_tag == 0) { + if (s->reconnect_ms && s->timer_tag == 0) { qemu_set_info_str(&s->nc, "connecting"); - s->timer_tag = g_timeout_add_seconds(s->reconnect, - net_stream_reconnect, s); + s->timer_tag = g_timeout_add(s->reconnect_ms, net_stream_reconnect, s); } } @@ -398,7 +397,7 @@ static int net_stream_client_init(NetClientState *peer, const char *model, const char *name, SocketAddress *addr, - uint32_t reconnect, + uint32_t reconnect_ms, Error **errp) { NetStreamState *s; @@ -412,8 +411,8 @@ static int net_stream_client_init(NetClientState *peer, s->ioc = QIO_CHANNEL(sioc); s->nc.link_down = true; - s->reconnect = reconnect; - if (reconnect) { + s->reconnect_ms = reconnect_ms; + if (reconnect_ms) { s->addr = QAPI_CLONE(SocketAddress, addr); } qio_channel_socket_connect_async(sioc, addr, @@ -432,13 +431,24 @@ 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, - sock->has_reconnect ? sock->reconnect : 0, - errp); + reconnect_ms, errp); } - if (sock->has_reconnect) { - error_setg(errp, "'reconnect' option is incompatible with " - "socket in server mode"); + if (sock->has_reconnect || sock->has_reconnect_ms) { + error_setg(errp, "'reconnect' and 'reconnect-ms' options are " + "incompatible with socket in server mode"); return -1; } return net_stream_server_init(peer, "stream", name, sock->addr, errp); diff --git a/qapi/net.json b/qapi/net.json index 87fc0d0b28..2739a2f423 100644 --- a/qapi/net.json +++ b/qapi/net.json @@ -650,15 +650,26 @@ # 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) +# # 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': 'uint32' } } + '*reconnect': { 'type': 'int', 'features': [ 'deprecated' ] }, + '*reconnect-ms': 'int' } } ## # @NetdevDgramOptions: diff --git a/qemu-options.hx b/qemu-options.hx index daae494147..bb228f6200 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -2833,9 +2833,9 @@ DEF("netdev", HAS_ARG, QEMU_OPTION_netdev, "-netdev socket,id=str[,fd=h][,udp=host:port][,localaddr=host:port]\n" " configure a network backend to connect to another network\n" " using an UDP tunnel\n" - "-netdev stream,id=str[,server=on|off],addr.type=inet,addr.host=host,addr.port=port[,to=maxport][,numeric=on|off][,keep-alive=on|off][,mptcp=on|off][,addr.ipv4=on|off][,addr.ipv6=on|off][,reconnect=seconds]\n" - "-netdev stream,id=str[,server=on|off],addr.type=unix,addr.path=path[,abstract=on|off][,tight=on|off][,reconnect=seconds]\n" - "-netdev stream,id=str[,server=on|off],addr.type=fd,addr.str=file-descriptor[,reconnect=seconds]\n" + "-netdev stream,id=str[,server=on|off],addr.type=inet,addr.host=host,addr.port=port[,to=maxport][,numeric=on|off][,keep-alive=on|off][,mptcp=on|off][,addr.ipv4=on|off][,addr.ipv6=on|off][,reconnect-ms=milliseconds]\n" + "-netdev stream,id=str[,server=on|off],addr.type=unix,addr.path=path[,abstract=on|off][,tight=on|off][,reconnect-ms=milliseconds]\n" + "-netdev stream,id=str[,server=on|off],addr.type=fd,addr.str=file-descriptor[,reconnect-ms=milliseconds]\n" " configure a network backend to connect to another network\n" " using a socket connection in stream mode.\n" "-netdev dgram,id=str,remote.type=inet,remote.host=maddr,remote.port=port[,local.type=inet,local.host=addr]\n" @@ -3291,7 +3291,7 @@ SRST -device e1000,netdev=n1,mac=52:54:00:12:34:56 \\ -netdev socket,id=n1,mcast=239.192.168.1:1102,localaddr=1.2.3.4 -``-netdev stream,id=str[,server=on|off],addr.type=inet,addr.host=host,addr.port=port[,to=maxport][,numeric=on|off][,keep-alive=on|off][,mptcp=on|off][,addr.ipv4=on|off][,addr.ipv6=on|off][,reconnect=seconds]`` +``-netdev stream,id=str[,server=on|off],addr.type=inet,addr.host=host,addr.port=port[,to=maxport][,numeric=on|off][,keep-alive=on|off][,mptcp=on|off][,addr.ipv4=on|off][,addr.ipv6=on|off][,reconnect-ms=milliseconds]`` Configure a network backend to connect to another QEMU virtual machine or a proxy using a TCP/IP socket. ``server=on|off`` @@ -3333,9 +3333,9 @@ SRST # second VM |qemu_system| linux.img \\ -device virtio-net,netdev=net0,mac=52:54:00:12:34:57 \\ - -netdev stream,id=net0,server=off,addr.type=inet,addr.host=localhost,addr.port=1234,reconnect=5 + -netdev stream,id=net0,server=off,addr.type=inet,addr.host=localhost,addr.port=1234,reconnect-ms=5000 -``-netdev stream,id=str[,server=on|off],addr.type=unix,addr.path=path[,abstract=on|off][,tight=on|off][,reconnect=seconds]`` +``-netdev stream,id=str[,server=on|off],addr.type=unix,addr.path=path[,abstract=on|off][,tight=on|off][,reconnect-ms=milliseconds]`` Configure a network backend to connect to another QEMU virtual machine or a proxy using a stream oriented unix domain socket. ``server=on|off`` @@ -3350,8 +3350,8 @@ SRST ``tight=on|off`` if false, pad an abstract socket address with enough null bytes to make it fill struct sockaddr_un member sun_path. - ``reconnect=seconds`` - for a client socket, if a socket is disconnected, then attempt a reconnect after the given number of seconds. + ``reconnect-ms=milliseconds`` + 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. (default: 0) Example (using passt as a replacement of -netdev user): @@ -3377,9 +3377,9 @@ SRST # second VM |qemu_system| linux.img \\ -device virtio-net,netdev=net0,mac=52:54:00:12:34:57 \\ - -netdev stream,id=net0,server=off,addr.type=unix,addr.path=/tmp/qemu0,reconnect=5 + -netdev stream,id=net0,server=off,addr.type=unix,addr.path=/tmp/qemu0,reconnect-ms=5000 -``-netdev stream,id=str[,server=on|off],addr.type=fd,addr.str=file-descriptor[,reconnect=seconds]`` +``-netdev stream,id=str[,server=on|off],addr.type=fd,addr.str=file-descriptor[,reconnect-ms=milliseconds]`` Configure a network backend to connect to another QEMU virtual machine or a proxy using a stream oriented socket file descriptor. ``server=on|off`` @@ -4390,14 +4390,14 @@ SRST ``telnet options:`` localhost 5555 - ``tcp:[host]:port[,server=on|off][,wait=on|off][,nodelay=on|off][,reconnect=seconds]`` + ``tcp:[host]:port[,server=on|off][,wait=on|off][,nodelay=on|off][,reconnect-ms=milliseconds]`` The TCP Net Console has two modes of operation. It can send the serial I/O to a location or wait for a connection from a location. By default the TCP Net Console is sent to host at the port. If you use the ``server=on`` option QEMU will wait for a client socket application to connect to the port before continuing, unless the ``wait=on|off`` option was specified. The ``nodelay=on|off`` - option disables the Nagle buffering algorithm. The ``reconnect=on`` + option disables the Nagle buffering algorithm. The ``reconnect-ms`` option only applies if ``server=no`` is set, if the connection goes down it will attempt to reconnect at the given interval. If host is omitted, 0.0.0.0 is assumed. Only one TCP connection at a diff --git a/tests/qtest/netdev-socket.c b/tests/qtest/netdev-socket.c index fc7d11961e..317af03817 100644 --- a/tests/qtest/netdev-socket.c +++ b/tests/qtest/netdev-socket.c @@ -204,7 +204,7 @@ static void test_stream_unix_reconnect(void) qts1 = qtest_initf("-nodefaults -M none " "-netdev stream,server=false,id=st0,addr.type=unix," - "addr.path=%s,reconnect=1", path); + "addr.path=%s,reconnect-ms=1000", path); wait_stream_connected(qts0, "st0", &addr); g_assert_cmpint(addr->type, ==, SOCKET_ADDRESS_TYPE_UNIX); -- 2.34.1
Change all related docs and tests to use the new 'reconnect-ms' option instead of the now deprecated 'reconnect'. Signed-off-by: Daniil Tatianin <d-tatianin@yandex-team.ru> --- docs/COLO-FT.txt | 4 ++-- docs/system/ppc/powernv.rst | 2 +- qemu-options.hx | 22 +++++++++++----------- tests/qtest/ipmi-bt-test.c | 2 +- tests/qtest/vhost-user-test.c | 2 +- tests/unit/test-char.c | 8 ++++---- 6 files changed, 20 insertions(+), 20 deletions(-) diff --git a/docs/COLO-FT.txt b/docs/COLO-FT.txt index 2e760a4aee..2283a09c08 100644 --- a/docs/COLO-FT.txt +++ b/docs/COLO-FT.txt @@ -193,8 +193,8 @@ any IP's here, except for the $primary_ip variable. -device piix3-usb-uhci -device usb-tablet -name secondary \ -netdev tap,id=hn0,vhost=off,helper=/usr/lib/qemu/qemu-bridge-helper \ -device rtl8139,id=e0,netdev=hn0 \ - -chardev socket,id=red0,host=$primary_ip,port=9003,reconnect=1 \ - -chardev socket,id=red1,host=$primary_ip,port=9004,reconnect=1 \ + -chardev socket,id=red0,host=$primary_ip,port=9003,reconnect-ms=1000 \ + -chardev socket,id=red1,host=$primary_ip,port=9004,reconnect-ms=1000 \ -object filter-redirector,id=f1,netdev=hn0,queue=tx,indev=red0 \ -object filter-redirector,id=f2,netdev=hn0,queue=rx,outdev=red1 \ -object filter-rewriter,id=rew0,netdev=hn0,queue=all \ diff --git a/docs/system/ppc/powernv.rst b/docs/system/ppc/powernv.rst index 09f3965858..de7a807ac7 100644 --- a/docs/system/ppc/powernv.rst +++ b/docs/system/ppc/powernv.rst @@ -181,7 +181,7 @@ connected to a remote QEMU machine acting as BMC, using these options .. code-block:: bash - -chardev socket,id=ipmi0,host=localhost,port=9002,reconnect=10 \ + -chardev socket,id=ipmi0,host=localhost,port=9002,reconnect-ms=10000 \ -device ipmi-bmc-extern,id=bmc0,chardev=ipmi0 \ -device isa-ipmi-bt,bmc=bmc0,irq=10 \ -nodefaults diff --git a/qemu-options.hx b/qemu-options.hx index bb228f6200..dacc9790a4 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -1102,7 +1102,7 @@ SRST external entity that provides the IPMI services. A connection is made to an external BMC simulator. If you do this, - it is strongly recommended that you use the "reconnect=" chardev + it is strongly recommended that you use the "reconnect-ms=" chardev option to reconnect to the simulator if the connection is lost. Note that if this is not used carefully, it can be a security issue, as the interface has the ability to send resets, NMIs, and power off @@ -3318,8 +3318,8 @@ SRST ``ipv6=on|off`` whether to accept IPv6 addresses, default to try both IPv4 and IPv6 - ``reconnect=seconds`` - for a client socket, if a socket is disconnected, then attempt a reconnect after the given number of seconds. + ``reconnect-ms=milliseconds`` + 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. (default: 0) Example (two guests connected using a TCP/IP socket): @@ -3388,8 +3388,8 @@ SRST ``addr.str=file-descriptor`` file descriptor number to use as a socket - ``reconnect=seconds`` - for a client socket, if a socket is disconnected, then attempt a reconnect after the given number of seconds. + ``reconnect-ms=milliseconds`` + 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. (default: 0) ``-netdev dgram,id=str,remote.type=inet,remote.host=maddr,remote.port=port[,local.type=inet,local.host=addr]`` @@ -3679,9 +3679,9 @@ DEF("chardev", HAS_ARG, QEMU_OPTION_chardev, "-chardev help\n" "-chardev null,id=id[,mux=on|off][,logfile=PATH][,logappend=on|off]\n" "-chardev socket,id=id[,host=host],port=port[,to=to][,ipv4=on|off][,ipv6=on|off][,nodelay=on|off]\n" - " [,server=on|off][,wait=on|off][,telnet=on|off][,websocket=on|off][,reconnect=seconds][,mux=on|off]\n" + " [,server=on|off][,wait=on|off][,telnet=on|off][,websocket=on|off][,reconnect-ms=milliseconds][,mux=on|off]\n" " [,logfile=PATH][,logappend=on|off][,tls-creds=ID][,tls-authz=ID] (tcp)\n" - "-chardev socket,id=id,path=path[,server=on|off][,wait=on|off][,telnet=on|off][,websocket=on|off][,reconnect=seconds]\n" + "-chardev socket,id=id,path=path[,server=on|off][,wait=on|off][,telnet=on|off][,websocket=on|off][,reconnect-ms=milliseconds]\n" " [,mux=on|off][,logfile=PATH][,logappend=on|off][,abstract=on|off][,tight=on|off] (unix)\n" "-chardev udp,id=id[,host=host],port=port[,localaddr=localaddr]\n" " [,localport=localport][,ipv4=on|off][,ipv6=on|off][,mux=on|off]\n" @@ -3792,7 +3792,7 @@ The available backends are: A void device. This device will not emit any data, and will drop any data it receives. The null backend does not take any options. -``-chardev socket,id=id[,TCP options or unix options][,server=on|off][,wait=on|off][,telnet=on|off][,websocket=on|off][,reconnect=seconds][,tls-creds=id][,tls-authz=id]`` +``-chardev socket,id=id[,TCP options or unix options][,server=on|off][,wait=on|off][,telnet=on|off][,websocket=on|off][,reconnect-ms=milliseconds][,tls-creds=id][,tls-authz=id]`` Create a two-way stream socket, which can be either a TCP or a unix socket. A unix socket will be created if ``path`` is specified. Behaviour is undefined if TCP options are specified for a unix @@ -3809,9 +3809,9 @@ The available backends are: ``websocket=on|off`` specifies that the socket uses WebSocket protocol for communication. - ``reconnect`` sets the timeout for reconnecting on non-server + ``reconnect-ms`` sets the timeout for reconnecting on non-server sockets when the remote end goes away. qemu will delay this many - seconds and then attempt to reconnect. Zero disables reconnecting, + milliseconds and then attempt to reconnect. Zero disables reconnecting, and is the default. ``tls-creds`` requests enablement of the TLS protocol for @@ -4427,7 +4427,7 @@ SRST The WebSocket protocol is used instead of raw tcp socket. The port acts as a WebSocket server. Client mode is not supported. - ``unix:path[,server=on|off][,wait=on|off][,reconnect=seconds]`` + ``unix:path[,server=on|off][,wait=on|off][,reconnect-ms=milliseconds]`` A unix domain socket is used instead of a tcp socket. The option works the same as if you had specified ``-serial tcp`` except the unix domain socket path is used for connections. diff --git a/tests/qtest/ipmi-bt-test.c b/tests/qtest/ipmi-bt-test.c index 13f7c841f5..637732fd5a 100644 --- a/tests/qtest/ipmi-bt-test.c +++ b/tests/qtest/ipmi-bt-test.c @@ -411,7 +411,7 @@ int main(int argc, char **argv) g_test_init(&argc, &argv, NULL); global_qtest = qtest_initf( - " -chardev socket,id=ipmi0,host=127.0.0.1,port=%d,reconnect=10" + " -chardev socket,id=ipmi0,host=127.0.0.1,port=%d,reconnect-ms=10000" " -device ipmi-bmc-extern,chardev=ipmi0,id=bmc0" " -device isa-ipmi-bt,bmc=bmc0", emu_port); qtest_irq_intercept_in(global_qtest, "ioapic"); diff --git a/tests/qtest/vhost-user-test.c b/tests/qtest/vhost-user-test.c index d6075001e7..8948fb81ef 100644 --- a/tests/qtest/vhost-user-test.c +++ b/tests/qtest/vhost-user-test.c @@ -920,7 +920,7 @@ static void wait_for_rings_started(TestServer *s, size_t count) static inline void test_server_connect(TestServer *server) { - test_server_create_chr(server, ",reconnect=1"); + test_server_create_chr(server, ",reconnect-ms=1000"); } static gboolean diff --git a/tests/unit/test-char.c b/tests/unit/test-char.c index a1c6bb874c..a6e8753e1c 100644 --- a/tests/unit/test-char.c +++ b/tests/unit/test-char.c @@ -1545,18 +1545,18 @@ int main(int argc, char **argv) static CharSocketClientTestConfig client2 ## name = \ { addr, NULL, true, false, char_socket_event }; \ static CharSocketClientTestConfig client3 ## name = \ - { addr, ",reconnect=1", false, false, char_socket_event }; \ + { addr, ",reconnect-ms=1000", false, false, char_socket_event }; \ static CharSocketClientTestConfig client4 ## name = \ - { addr, ",reconnect=1", true, false, char_socket_event }; \ + { addr, ",reconnect-ms=1000", true, false, char_socket_event }; \ static CharSocketClientTestConfig client5 ## name = \ { addr, NULL, false, true, char_socket_event }; \ static CharSocketClientTestConfig client6 ## name = \ { addr, NULL, true, true, char_socket_event }; \ static CharSocketClientTestConfig client7 ## name = \ - { addr, ",reconnect=1", true, false, \ + { addr, ",reconnect-ms=1000", true, false, \ char_socket_event_with_error }; \ static CharSocketClientTestConfig client8 ## name = \ - { addr, ",reconnect=1", false, false, char_socket_event }; \ + { addr, ",reconnect-ms=1000", false, false, char_socket_event };\ g_test_add_data_func("/char/socket/client/mainloop/" # name, \ &client1 ##name, char_socket_client_test); \ g_test_add_data_func("/char/socket/client/wait-conn/" # name, \ -- 2.34.1
On Fri, Oct 25, 2024 at 11:36 AM Daniil Tatianin <d-tatianin@yandex-team.ru> wrote:
We've deprecated the 'reconnect' property in c8e2b6b4d7e, but all the tests, as well as the stream netdev were still left using it. This series finalizes the deprecation and replaces all local usage and docs metions of 'reconnect' with 'reconnect-ms'.
Daniil Tatianin (2): net/stream: deprecate 'reconnect' in favor of 'reconnect-ms' chardev: finalize 'reconnect' deprecation
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
docs/COLO-FT.txt | 4 +-- docs/about/deprecated.rst | 10 ++++++++ docs/system/ppc/powernv.rst | 2 +- net/stream.c | 34 +++++++++++++++++--------- qapi/net.json | 13 +++++++++- qemu-options.hx | 46 +++++++++++++++++------------------ tests/qtest/ipmi-bt-test.c | 2 +- tests/qtest/netdev-socket.c | 2 +- tests/qtest/vhost-user-test.c | 2 +- tests/unit/test-char.c | 8 +++--- 10 files changed, 77 insertions(+), 46 deletions(-)
-- 2.34.1
participants (2)
-
Daniil Tatianin -
Marc-André Lureau