On 09.09.25 12:15, Daniel P. Berrangé wrote:
On Tue, Sep 09, 2025 at 12:11:19PM +0300, Vladimir Sementsov-Ogievskiy wrote:
On 09.09.25 12:00, Daniel P. Berrangé wrote:
On Wed, Sep 03, 2025 at 12:44:07PM +0300, Vladimir Sementsov-Ogievskiy wrote:
Instead of open-coded g_unix_set_fd_nonblocking() calls, use
QEMU wrapper qemu_socket_set_nonblock().
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@yandex-team.ru>
---
chardev/char-fd.c | 4 ++--
chardev/char-pty.c | 3 +--
chardev/char-serial.c | 3 +--
chardev/char-stdio.c | 3 +--
hw/input/virtio-input-host.c | 3 +--
hw/misc/ivshmem-flat.c | 4 +++-
hw/misc/ivshmem-pci.c | 8 +++++++-
hw/virtio/vhost-vsock.c | 8 ++------
io/channel-command.c | 9 ++++++---
io/channel-file.c | 3 +--
net/tap-bsd.c | 12 ++++++++++--
net/tap-linux.c | 8 +++++++-
net/tap-solaris.c | 7 ++++++-
net/tap.c | 21 ++++++---------------
qga/commands-posix.c | 3 +--
tests/qtest/fuzz/virtio_net_fuzz.c | 2 +-
tests/qtest/vhost-user-test.c | 3 +--
tests/unit/test-iov.c | 5 +++--
ui/input-linux.c | 3 +--
util/event_notifier-posix.c | 5 +++--
util/main-loop.c | 6 +++++-
21 files changed, 69 insertions(+), 54 deletions(-)
diff --git a/io/channel-command.c b/io/channel-command.c
index 8966dd3a2b..8ae9a026b3 100644
--- a/io/channel-command.c
+++ b/io/channel-command.c
@@ -277,9 +277,12 @@ static int qio_channel_command_set_blocking(QIOChannel
*ioc,
cioc->blocking = enabled;
#else
- if ((cioc->writefd >= 0 && !g_unix_set_fd_nonblocking(cioc->writefd,
!enabled, NULL)) ||
- (cioc->readfd >= 0 && !g_unix_set_fd_nonblocking(cioc->readfd,
!enabled, NULL))) {
- error_setg_errno(errp, errno, "Failed to set FD nonblocking");
+ if (cioc->writefd >= 0 &&
+ !qemu_set_blocking(cioc->writefd, enabled, errp)) {
+ return -1;
+ }
+ if (cioc->readfd >= 0 &&
+ !qemu_set_blocking(cioc->readfd, enabled, errp)) {
return -1;
}
#endif
diff --git a/io/channel-file.c b/io/channel-file.c
index ca3f180cc2..5cef75a67c 100644
--- a/io/channel-file.c
+++ b/io/channel-file.c
@@ -223,8 +223,7 @@ static int qio_channel_file_set_blocking(QIOChannel *ioc,
#else
QIOChannelFile *fioc = QIO_CHANNEL_FILE(ioc);
- if (!g_unix_set_fd_nonblocking(fioc->fd, !enabled, NULL)) {
- error_setg_errno(errp, errno, "Failed to set FD nonblocking");
+ if (!qemu_set_blocking(fioc->fd, enabled, errp)) {
return -1;
}
return 0;
This is wrong for Windows. fioc->fd is not a socket, but this is passing
it to an API whose impl assume it is receiving a socket.
But what is changed with the patch? g_unix_set_fd_nonblocking(fioc->fd, ..) is
wrong for Windows as well.
Actually I missed the #ifdef. This code is in an #else branch that excludes
compilation on Wnidows - the Windows brach just raise an error.
And making separate qemu_set_blocking() and qemu_socket_set_blocking(), which
do the same
thing, doesn't make sense..
Hmm. But we can define qemu_set_blocking() only for Linux, keeping
qemu_socket_set_blocking() the generic
function. Still, nothing prevents using qemu_socket_set_blocking() on
non-sockets..
We just relying on the name of the function alerting the developer / reviewer.
If you're dealing with a FIFO pipe FD you are (hopefully) going to realize
that qemu_socket_XXX is not a function to be used.
Understand. But having both qemu_XXX() and qemu_socket_XXX(), I'd be confused,
why not just use qemu_XXX() everywhere.
So, for final API, either we have one function qemu_set_blocking() (as the
series propose), and lose this alert,
or, we should have something like:
qemu_socket_set_blocking() - generic, for calling on sockets, Windows or Linux
qemu_unix_set_blocking() - defined only for Linux
What I dislike with the latter:
1. For Linux functions are duplicates actually. So, we have a defined only for
Linux duplicate of generic function
2. Nothing prevents using qemu_socket_set_blocking() on any fds in Linux except
name (and it will succeed!!)
--
Best regards,
Vladimir
