From: "Daniel P. Berrangé" <berrange@redhat.com>
To: qemu-devel@nongnu.org
Cc: "Daniel P. Berrangé" <berrange@redhat.com>,
"Philippe Mathieu-Daudé" <philmd@linaro.org>,
"Markus Armbruster" <armbru@redhat.com>,
"Paolo Bonzini" <pbonzini@redhat.com>,
"Eric Blake" <eblake@redhat.com>,
"Marc-André Lureau" <marcandre.lureau@redhat.com>,
"Juraj Marcin" <jmarcin@redhat.com>
Subject: [PULL 19/23] util/qemu-sockets: Refactor success and failure paths in inet_listen_saddr()
Date: Thu, 22 May 2025 11:29:19 +0100 [thread overview]
Message-ID: <20250522102923.309452-20-berrange@redhat.com> (raw)
In-Reply-To: <20250522102923.309452-1-berrange@redhat.com>
From: Juraj Marcin <jmarcin@redhat.com>
To get a listening socket, we need to first create a socket, try binding
it to a certain port, and lastly starting listening to it. Each of these
operations can fail due to various reasons, one of them being that the
requested address/port is already in use. In such case, the function
tries the same process with a new port number.
This patch refactors the port number loop, so the success path is no
longer buried inside the 'if' statements in the middle of the loop. Now,
the success path is not nested and ends at the end of the iteration
after successful socket creation, binding, and listening. In case any of
the operations fails, it either continues to the next iteration (and the
next port) or jumps out of the loop to handle the error and exits the
function.
Signed-off-by: Juraj Marcin <jmarcin@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
util/qemu-sockets.c | 51 ++++++++++++++++++++++++---------------------
1 file changed, 27 insertions(+), 24 deletions(-)
diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index 4a878e0527..329fdbfd97 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -303,11 +303,20 @@ static int inet_listen_saddr(InetSocketAddress *saddr,
port_min = inet_getport(e);
port_max = saddr->has_to ? saddr->to + port_offset : port_min;
for (p = port_min; p <= port_max; p++) {
+ if (slisten >= 0) {
+ /*
+ * We have a socket we tried with the previous port. It cannot
+ * be rebound, we need to close it and create a new one.
+ */
+ close(slisten);
+ slisten = -1;
+ }
inet_setport(e, p);
slisten = create_fast_reuse_socket(e);
if (slisten < 0) {
- /* First time we expect we might fail to create the socket
+ /*
+ * First time we expect we might fail to create the socket
* eg if 'e' has AF_INET6 but ipv6 kmod is not loaded.
* Later iterations should always succeed if first iteration
* worked though, so treat that as fatal.
@@ -317,40 +326,38 @@ static int inet_listen_saddr(InetSocketAddress *saddr,
} else {
error_setg_errno(errp, errno,
"Failed to recreate failed listening socket");
- goto listen_failed;
+ goto fail;
}
}
socket_created = true;
rc = try_bind(slisten, saddr, e);
if (rc < 0) {
- if (errno != EADDRINUSE) {
- error_setg_errno(errp, errno, "Failed to bind socket");
- goto listen_failed;
- }
- } else {
- if (!listen(slisten, num)) {
- goto listen_ok;
+ if (errno == EADDRINUSE) {
+ /* This port is already used, try the next one */
+ continue;
}
- if (errno != EADDRINUSE) {
- error_setg_errno(errp, errno, "Failed to listen on socket");
- goto listen_failed;
+ error_setg_errno(errp, errno, "Failed to bind socket");
+ goto fail;
+ }
+ if (listen(slisten, num)) {
+ if (errno == EADDRINUSE) {
+ /* This port is already used, try the next one */
+ continue;
}
+ error_setg_errno(errp, errno, "Failed to listen on socket");
+ goto fail;
}
- /* Someone else managed to bind to the same port and beat us
- * to listen on it! Socket semantics does not allow us to
- * recover from this situation, so we need to recreate the
- * socket to allow bind attempts for subsequent ports:
- */
- close(slisten);
- slisten = -1;
+ /* We have a listening socket */
+ freeaddrinfo(res);
+ return slisten;
}
}
error_setg_errno(errp, errno,
socket_created ?
"Failed to find an available port" :
"Failed to create a socket");
-listen_failed:
+fail:
saved_errno = errno;
if (slisten >= 0) {
close(slisten);
@@ -358,10 +365,6 @@ listen_failed:
freeaddrinfo(res);
errno = saved_errno;
return -1;
-
-listen_ok:
- freeaddrinfo(res);
- return slisten;
}
#ifdef _WIN32
--
2.49.0
next prev parent reply other threads:[~2025-05-22 10:33 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-05-22 10:29 [PULL 00/23] Misc VNC, I/O, Crypto & checkpatch fixes Daniel P. Berrangé
2025-05-22 10:29 ` [PULL 01/23] ui/vnc.c: replace big endian flag with byte order value Daniel P. Berrangé
2025-05-22 10:29 ` [PULL 02/23] ui/vnc: take account of client byte order in pixman format Daniel P. Berrangé
2025-06-03 11:18 ` Thomas Huth
2025-06-03 11:30 ` Daniel P. Berrangé
2025-06-03 13:49 ` Daniel P. Berrangé
2025-06-03 14:43 ` Philippe Mathieu-Daudé
2025-05-22 10:29 ` [PULL 03/23] ui/vnc: fix tight palette pixel encoding for 8/16-bpp formats Daniel P. Berrangé
2025-05-22 10:29 ` [PULL 04/23] tests: skip encrypted secret tests if AES is not available Daniel P. Berrangé
2025-05-22 10:29 ` [PULL 05/23] tests: skip legacy qcow2 encryption test " Daniel P. Berrangé
2025-05-22 10:29 ` [PULL 06/23] tests: fix skipping cipher tests when " Daniel P. Berrangé
2025-05-22 10:29 ` [PULL 07/23] crypto: fully drop built-in cipher provider Daniel P. Berrangé
2025-05-22 10:29 ` [PULL 08/23] Revert "scripts: mandate that new files have SPDX-License-Identifier" Daniel P. Berrangé
2025-05-22 10:29 ` [PULL 09/23] scripts/checkpatch.pl: fix various indentation mistakes Daniel P. Berrangé
2025-05-22 10:29 ` [PULL 10/23] scripts/checkpatch: introduce tracking of file start/end Daniel P. Berrangé
2025-05-22 10:29 ` [PULL 11/23] scripts/checkpatch: use new hook for ACPI test data check Daniel P. Berrangé
2025-05-22 10:29 ` [PULL 12/23] scripts/checkpatch: use new hook for file permissions check Daniel P. Berrangé
2025-05-22 10:29 ` [PULL 13/23] scripts/checkpatch: expand pattern for matching makefiles Daniel P. Berrangé
2025-05-22 10:29 ` [PULL 14/23] scripts/checkpatch: use new hook for MAINTAINERS update check Daniel P. Berrangé
2025-05-22 10:29 ` [PULL 15/23] scripts/checkpatch: reimplement mandate for SPDX-License-Identifier Daniel P. Berrangé
2025-05-22 10:29 ` [PULL 16/23] scripts/checkpatch: reject license boilerplate on new files Daniel P. Berrangé
2025-05-22 10:29 ` [PULL 17/23] io: Fix partial struct copy in qio_dns_resolver_lookup_sync_inet() Daniel P. Berrangé
2025-05-22 10:29 ` [PULL 18/23] util/qemu-sockets: Refactor setting client sockopts into a separate function Daniel P. Berrangé
2025-05-22 10:29 ` Daniel P. Berrangé [this message]
2025-07-10 12:17 ` [PULL 19/23] util/qemu-sockets: Refactor success and failure paths in inet_listen_saddr() Peter Maydell
2025-07-10 12:50 ` Juraj Marcin
2025-07-10 12:55 ` Daniel P. Berrangé
2025-07-10 13:00 ` Peter Maydell
2025-05-22 10:29 ` [PULL 20/23] util/qemu-sockets: Add support for keep-alive flag to passive sockets Daniel P. Berrangé
2025-05-22 10:29 ` [PULL 21/23] util/qemu-sockets: Refactor inet_parse() to use QemuOpts Daniel P. Berrangé
2025-05-22 10:29 ` [PULL 22/23] util/qemu-sockets: Introduce inet socket options controlling TCP keep-alive Daniel P. Berrangé
2025-05-22 10:29 ` [PULL 23/23] scripts/checkpatch.pl: mandate SPDX tag for Rust src files Daniel P. Berrangé
2025-05-23 13:21 ` [PULL 00/23] Misc VNC, I/O, Crypto & checkpatch fixes Stefan Hajnoczi
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=20250522102923.309452-20-berrange@redhat.com \
--to=berrange@redhat.com \
--cc=armbru@redhat.com \
--cc=eblake@redhat.com \
--cc=jmarcin@redhat.com \
--cc=marcandre.lureau@redhat.com \
--cc=pbonzini@redhat.com \
--cc=philmd@linaro.org \
--cc=qemu-devel@nongnu.org \
/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).