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.
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
