qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Daniel P. Berrangé" <berrange@redhat.com>
To: qemu-devel@nongnu.org
Cc: "Stefan Hajnoczi" <stefanha@redhat.com>,
	"Philippe Mathieu-Daudé" <philmd@linaro.org>,
	"Daniel P. Berrangé" <berrange@redhat.com>,
	"Michael S . Tsirkin" <mst@redhat.com>,
	"Jason Wang" <jasowang@redhat.com>,
	"Laurent Vivier" <lvivier@redhat.com>,
	"Thomas Huth" <thuth@redhat.com>,
	"Marc-André Lureau" <marcandre.lureau@redhat.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>
Subject: [PATCH 4/6] net: add explicit info about connecting/listening state
Date: Thu,  4 Jan 2024 16:29:40 +0000	[thread overview]
Message-ID: <20240104162942.211458-5-berrange@redhat.com> (raw)
In-Reply-To: <20240104162942.211458-1-berrange@redhat.com>

When running 'info network', if the stream backend is still in
the process of connecting, or waiting for an incoming connection,
no information is displayed.

There is also no way to distinguish whether the server is still
in the process of setting up the listener socket, or whether it
is ready to accept incoming client connections.

This leads to a race condition in the netdev-socket qtest which
launches a server process followed by a client process. Under
high load conditions it is possible for the client to attempt
to connect before the server is accepting clients. For the
scenarios which do not set the 'reconnect' option, this opens
up a race which can lead to the test scenario failing to reach
the expected state.

Now that 'info network' can distinguish between initialization
phase and the listening phase, the netdev-socket qtest will
correctly synchronize, such that the client QEMU is not spawned
until the server is ready.

This should solve the non-deterministic failures seen with the
netdev-socket qtest.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 net/stream.c                |  5 ++++-
 tests/qtest/netdev-socket.c | 10 +++++-----
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/net/stream.c b/net/stream.c
index 9204b4c96e..0defb21d45 100644
--- a/net/stream.c
+++ b/net/stream.c
@@ -173,7 +173,7 @@ static gboolean net_stream_send(QIOChannel *ioc,
 
         net_socket_rs_init(&s->rs, net_stream_rs_finalize, false);
         s->nc.link_down = true;
-        qemu_set_info_str(&s->nc, "%s", "");
+        qemu_set_info_str(&s->nc, "listening");
 
         qapi_event_send_netdev_stream_disconnected(s->nc.name);
         net_stream_arm_reconnect(s);
@@ -292,6 +292,7 @@ static void net_stream_server_listening(QIOTask *task, gpointer opaque)
     s->nc.link_down = true;
     s->listener = qio_net_listener_new();
 
+    qemu_set_info_str(&s->nc, "listening");
     net_socket_rs_init(&s->rs, net_stream_rs_finalize, false);
     qio_net_listener_set_client_func(s->listener, net_stream_listen, s, NULL);
     qio_net_listener_add(s->listener, listen_sioc);
@@ -309,6 +310,7 @@ static int net_stream_server_init(NetClientState *peer,
 
     nc = qemu_new_net_client(&net_stream_info, peer, model, name);
     s = DO_UPCAST(NetStreamState, nc, nc);
+    qemu_set_info_str(&s->nc, "initializing");
 
     s->listen_ioc = QIO_CHANNEL(listen_sioc);
     qio_channel_socket_listen_async(listen_sioc, addr, 0,
@@ -400,6 +402,7 @@ static int net_stream_client_init(NetClientState *peer,
 
     nc = qemu_new_net_client(&net_stream_info, peer, model, name);
     s = DO_UPCAST(NetStreamState, nc, nc);
+    qemu_set_info_str(&s->nc, "connecting");
 
     s->ioc = QIO_CHANNEL(sioc);
     s->nc.link_down = true;
diff --git a/tests/qtest/netdev-socket.c b/tests/qtest/netdev-socket.c
index 3fc2ac26d0..91441f7922 100644
--- a/tests/qtest/netdev-socket.c
+++ b/tests/qtest/netdev-socket.c
@@ -127,7 +127,7 @@ static void test_stream_inet_ipv4(void)
                        "addr.ipv4=on,addr.ipv6=off,"
                        "addr.host=127.0.0.1,addr.port=%d", port);
 
-    EXPECT_STATE(qts0, "st0: index=0,type=stream,\r\n", 0);
+    EXPECT_STATE(qts0, "st0: index=0,type=stream,listening\r\n", 0);
 
     qts1 = qtest_initf("-nodefaults -M none "
                        "-netdev stream,server=false,id=st0,addr.type=inet,"
@@ -200,7 +200,7 @@ static void test_stream_unix_reconnect(void)
                        "-netdev stream,id=st0,server=true,addr.type=unix,"
                        "addr.path=%s", path);
 
-    EXPECT_STATE(qts0, "st0: index=0,type=stream,\r\n", 0);
+    EXPECT_STATE(qts0, "st0: index=0,type=stream,listening\r\n", 0);
 
     qts1 = qtest_initf("-nodefaults -M none "
                        "-netdev stream,server=false,id=st0,addr.type=unix,"
@@ -250,7 +250,7 @@ static void test_stream_inet_ipv6(void)
                        "addr.ipv4=off,addr.ipv6=on,"
                        "addr.host=::1,addr.port=%d", port);
 
-    EXPECT_STATE(qts0, "st0: index=0,type=stream,\r\n", 0);
+    EXPECT_STATE(qts0, "st0: index=0,type=stream,listening\r\n", 0);
 
     qts1 = qtest_initf("-nodefaults -M none "
                        "-netdev stream,server=false,id=st0,addr.type=inet,"
@@ -282,7 +282,7 @@ static void test_stream_unix(void)
                        "addr.type=unix,addr.path=%s,",
                        path);
 
-    EXPECT_STATE(qts0, "st0: index=0,type=stream,\r\n", 0);
+    EXPECT_STATE(qts0, "st0: index=0,type=stream,listening\r\n", 0);
 
     qts1 = qtest_initf("-nodefaults -M none "
                        "-netdev stream,id=st0,server=false,"
@@ -314,7 +314,7 @@ static void test_stream_unix_abstract(void)
                        "addr.abstract=on",
                        path);
 
-    EXPECT_STATE(qts0, "st0: index=0,type=stream,\r\n", 0);
+    EXPECT_STATE(qts0, "st0: index=0,type=stream,listening\r\n", 0);
 
     qts1 = qtest_initf("-nodefaults -M none "
                        "-netdev stream,id=st0,server=false,"
-- 
2.43.0



  parent reply	other threads:[~2024-01-04 16:31 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-01-04 16:29 [PATCH 0/6] net: fix non-deterministic failures of the 'netdev-socket' qtest Daniel P. Berrangé
2024-01-04 16:29 ` [PATCH 1/6] Revert "netdev: set timeout depending on loadavg" Daniel P. Berrangé
2024-01-04 16:29 ` [PATCH 2/6] Revert "osdep: add getloadavg" Daniel P. Berrangé
2024-01-04 16:29 ` [PATCH 3/6] Revert "tests/qtest/netdev-socket: Raise connection timeout to 120 seconds" Daniel P. Berrangé
2024-01-04 16:29 ` Daniel P. Berrangé [this message]
2024-01-04 16:29 ` [PATCH 5/6] net: handle QIOTask completion to report useful error message Daniel P. Berrangé
2024-01-04 16:29 ` [PATCH 6/6] qtest: ensure netdev-socket tests have non-overlapping names Daniel P. Berrangé
2024-01-04 17:47   ` Philippe Mathieu-Daudé
2024-01-04 16:45 ` [PATCH 0/6] net: fix non-deterministic failures of the 'netdev-socket' qtest Stefan Hajnoczi
2024-01-15  2:36   ` Jason Wang
2024-01-15 10:19     ` Peter Maydell
2024-01-16  1:05       ` Jason Wang
2024-01-09 13:49 ` Daniel P. Berrangé

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240104162942.211458-5-berrange@redhat.com \
    --to=berrange@redhat.com \
    --cc=jasowang@redhat.com \
    --cc=lvivier@redhat.com \
    --cc=marcandre.lureau@redhat.com \
    --cc=mst@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=philmd@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    --cc=thuth@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).