On 12.09.25 19:59, Daniel P. Berrangé wrote:
On Thu, Sep 11, 2025 at 12:20:02PM +0300, Vladimir Sementsov-Ogievskiy wrote:
Use common qemu_set_blocking() instead.
Note that pre-patch the behavior of Win32 and Linux realizations
are inconsistent: we ignore failure for Win32, and assert success
for Linux.
How do we convert the callers?
3. io/channel-socket.c: here we convert both old calls to
qemu_socket_set_nonblock() and qemu_socket_set_block() to
one new call. Pre-patch we assert success for Linux in
qemu_socket_set_nonblock(), and ignore all other errors here.
Still, all callers pass errp=NULL to qio_channel_set_blocking(),
so after patch we ignore all errors. Switching from assertion
to ignoring may be not very good, but still acceptable, keeping
in mind that all callers of qio_channel_set_blocking() do
explicitly ignore the error.
This is a bit questionable. IMHO the reason why nearly all
callers pass errp=NULL is laziness based on the assumption
that the code actually asserts, and no one thinking to check
the win impl which didn't asset.
IOW, I think we need a prior patch that sets the 'errp'
in all these callers, either to &error_abort, or to
propage the error if practical.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@yandex-team.ru>
Reviewed-by: Peter Xu <pet...@redhat.com>
---
contrib/ivshmem-server/ivshmem-server.c | 6 +++++-
hw/hyperv/syndbg.c | 4 +++-
hw/virtio/vhost-user.c | 5 ++++-
include/qemu/sockets.h | 1 -
io/channel-socket.c | 7 +++----
net/dgram.c | 16 +++++++++++++---
net/l2tpv3.c | 5 +++--
net/socket.c | 20 ++++++++++++++++----
qga/channel-posix.c | 7 ++++++-
tests/unit/socket-helpers.c | 5 ++++-
tests/unit/test-crypto-tlssession.c | 8 ++++----
util/oslib-posix.c | 7 -------
util/oslib-win32.c | 5 -----
util/vhost-user-server.c | 4 ++--
14 files changed, 63 insertions(+), 37 deletions(-)
diff --git a/contrib/ivshmem-server/ivshmem-server.c
b/contrib/ivshmem-server/ivshmem-server.c
index 2f3c7320a6..a65943d0b8 100644
--- a/contrib/ivshmem-server/ivshmem-server.c
+++ b/contrib/ivshmem-server/ivshmem-server.c
@@ -146,9 +146,13 @@ ivshmem_server_handle_new_conn(IvshmemServer *server)
return -1;
}
- qemu_socket_set_nonblock(newfd);
IVSHMEM_SERVER_DEBUG(server, "accept()=%d\n", newfd);
+ if (!qemu_set_blocking(newfd, false, NULL)) {
&error_warn here so we diagnose the reason we're returning -1
to the caller ?
O, I missed we have error_warn already. It was always my dream to implement it,
but never got around to it.
Ok
+ close(newfd);
+ return -1;
+ }
+
/* allocate new structure for this peer */
peer = g_malloc0(sizeof(*peer));
peer->sock_fd = newfd;
diff --git a/tests/unit/socket-helpers.c b/tests/unit/socket-helpers.c
index 37db24f72a..1b7e283f24 100644
--- a/tests/unit/socket-helpers.c
+++ b/tests/unit/socket-helpers.c
@@ -88,7 +88,10 @@ static int socket_can_bind_connect(const char *hostname, int
family)
goto cleanup;
}
- qemu_socket_set_nonblock(cfd);
+ if (!qemu_set_blocking(cfd, false, NULL)) {
&error_abort here.
Ok
+ goto cleanup;
+ }
+
if (connect(cfd, (struct sockaddr *)&ss, sslen) < 0) {
if (errno == EINPROGRESS) {
check_soerr = true;
With regards,
Daniel
--
Best regards,
Vladimir
