[PATCH 00/10] io: deal with blocking/non-blocking fds

[PATCH 00/10] io: deal with blocking/non-blocking fds

[Vladimir Sementsov-Ogievskiy]

Hi all!

The series brings two things:

1. unify code which sets fds blocking/non-blocking through the whole
source

2. for fds, which comes from qio_channel_readv_full(), stop making
them blocking in generic code, and move this logic to the callers,
all except coming from migration qemu-file (see last patch)

Vladimir Sementsov-Ogievskiy (10):
  io/channel: document how qio_channel_readv_full() handles fds
  char-socket: rework tcp_chr_recv()
  util: add qemu_set_blocking() function
  util: drop qemu_socket_set_nonblock()
  util: drop qemu_socket_try_set_nonblock()
  util: drop qemu_socket_set_block()
  use qemu_set_blocking instead of g_unix_set_fd_nonblocking
  oslib-posix: add qemu_fds_set_blocking() helper
  qio_channel_readv_full(): move setting fd blocking to callers
  migration/qemu-file: don't make incoming fds blocking again

 chardev/char-fd.c                       |  4 +--
 chardev/char-pty.c                      |  3 +-
 chardev/char-serial.c                   |  3 +-
 chardev/char-socket.c                   | 45 ++++++++++++-------------
 chardev/char-stdio.c                    |  3 +-
 contrib/ivshmem-server/ivshmem-server.c |  5 ++-
 hw/hyperv/syndbg.c                      |  4 ++-
 hw/input/virtio-input-host.c            |  3 +-
 hw/misc/ivshmem-flat.c                  |  4 ++-
 hw/misc/ivshmem-pci.c                   |  8 ++++-
 hw/remote/mpqemu-link.c                 |  3 ++
 hw/vfio-user/proxy.c                    |  4 +++
 hw/virtio/vhost-user.c                  | 10 +++++-
 hw/virtio/vhost-vsock.c                 |  8 ++---
 include/io/channel.h                    | 12 +++++++
 include/qemu/osdep.h                    |  8 +++++
 include/qemu/sockets.h                  |  3 --
 io/channel-command.c                    |  9 +++--
 io/channel-file.c                       |  3 +-
 io/channel-socket.c                     | 26 +++++++-------
 net/dgram.c                             | 28 ++++++++-------
 net/l2tpv3.c                            |  5 +--
 net/socket.c                            | 27 ++++++++++-----
 net/stream.c                            |  9 ++---
 net/stream_data.c                       | 10 +++---
 net/tap-bsd.c                           | 12 +++++--
 net/tap-linux.c                         |  8 ++++-
 net/tap-solaris.c                       |  7 +++-
 net/tap.c                               | 21 ++++--------
 qga/channel-posix.c                     |  7 +++-
 qga/commands-posix.c                    |  3 +-
 scsi/qemu-pr-helper.c                   |  4 +++
 tests/qtest/fuzz/virtio_net_fuzz.c      |  2 +-
 tests/qtest/tpm-emu.c                   |  1 +
 tests/qtest/vhost-user-test.c           |  3 +-
 tests/unit/socket-helpers.c             |  5 ++-
 tests/unit/test-crypto-tlssession.c     |  8 ++---
 tests/unit/test-io-channel-socket.c     |  1 +
 tests/unit/test-iov.c                   |  5 +--
 ui/input-linux.c                        |  3 +-
 util/event_notifier-posix.c             |  5 +--
 util/main-loop.c                        |  6 +++-
 util/oslib-posix.c                      | 27 +++++++++------
 util/oslib-win32.c                      | 25 ++++++--------
 util/vhost-user-server.c                |  9 +++--
 45 files changed, 244 insertions(+), 165 deletions(-)

-- 
2.48.1

[PATCH 01/10] io/channel: document how qio_channel_readv_full() handles fds

[Vladimir Sementsov-Ogievskiy]

The only realization, which may have incoming fds is
qio_channel_socket_readv() (in io/channel-socket.c).
qio_channel_socket_readv() do call (through
qio_channel_socket_copy_fds()) qemu_socket_set_block() and
qemu_set_cloexec() for each fd.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 include/io/channel.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/include/io/channel.h b/include/io/channel.h
index 234e5db70d..b848d50b99 100644
--- a/include/io/channel.h
+++ b/include/io/channel.h
@@ -117,6 +117,12 @@ struct QIOChannelClass {
                          size_t nfds,
                          int flags,
                          Error **errp);
+
+    /*
+     * The io_readv handler must guarantee that all
+     * incoming fds are set BLOCKING and CLOEXEC (if
+     * available).
+     */
     ssize_t (*io_readv)(QIOChannel *ioc,
                         const struct iovec *iov,
                         size_t niov,
@@ -124,6 +130,7 @@ struct QIOChannelClass {
                         size_t *nfds,
                         int flags,
                         Error **errp);
+
     int (*io_close)(QIOChannel *ioc,
                     Error **errp);
     GSource * (*io_create_watch)(QIOChannel *ioc,
@@ -234,6 +241,9 @@ void qio_channel_set_name(QIOChannel *ioc,
  * was allocated. It is the callers responsibility
  * to call close() on each file descriptor and to
  * call g_free() on the array pointer in @fds.
+ * qio_channel_readv_full() guarantees that all
+ * incoming fds are set BLOCKING and CLOEXEC (if
+ * available).
  *
  * It is an error to pass a non-NULL @fds parameter
  * unless qio_channel_has_feature() returns a true
-- 
2.48.1

Re: [PATCH 01/10] io/channel: document how qio_channel_readv_full() handles fds

[Daniel P. Berrangé]

On Wed, Sep 03, 2025 at 12:44:01PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> The only realization, which may have incoming fds is
> qio_channel_socket_readv() (in io/channel-socket.c).
> qio_channel_socket_readv() do call (through
> qio_channel_socket_copy_fds()) qemu_socket_set_block() and
> qemu_set_cloexec() for each fd.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> ---
>  include/io/channel.h | 10 ++++++++++
>  1 file changed, 10 insertions(+)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


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 :|

[PATCH 02/10] char-socket: rework tcp_chr_recv()

[Vladimir Sementsov-Ogievskiy]

First, qio_channel_readv_full() already guarantees BLOCKING and
CLOEXEC states for incoming descriptors, no reason call extra
ioctls.

Second, current implementation calls _set_block() and _set_cloexec()
again on old descriptors on failure path - we fix this too.

Finally, handling errors exactly after qio_channel_readv_full() call
looks more readable.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 chardev/char-socket.c | 37 +++++++++++++------------------------
 1 file changed, 13 insertions(+), 24 deletions(-)

diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index 1e8313915b..5b9b19ba8b 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -293,6 +293,18 @@ static ssize_t tcp_chr_recv(Chardev *chr, char *buf, size_t len)
                                      0, &err);
     }
 
+    if (ret == QIO_CHANNEL_ERR_BLOCK) {
+        errno = EAGAIN;
+        return -1;
+    } else if (ret == -1) {
+        trace_chr_socket_recv_err(chr, chr->label, error_get_pretty(err));
+        error_free(err);
+        errno = EIO;
+        return -1;
+    }
+
+    assert(ret >= 0);
+
     if (msgfds_num) {
         /* close and clean read_msgfds */
         for (i = 0; i < s->read_msgfds_num; i++) {
@@ -307,30 +319,7 @@ static ssize_t tcp_chr_recv(Chardev *chr, char *buf, size_t len)
         s->read_msgfds_num = msgfds_num;
     }
 
-    for (i = 0; i < s->read_msgfds_num; i++) {
-        int fd = s->read_msgfds[i];
-        if (fd < 0) {
-            continue;
-        }
-
-        /* O_NONBLOCK is preserved across SCM_RIGHTS so reset it */
-        qemu_socket_set_block(fd);
-
-#ifndef MSG_CMSG_CLOEXEC
-        qemu_set_cloexec(fd);
-#endif
-    }
-
-    if (ret == QIO_CHANNEL_ERR_BLOCK) {
-        errno = EAGAIN;
-        ret = -1;
-    } else if (ret == -1) {
-        trace_chr_socket_recv_err(chr, chr->label, error_get_pretty(err));
-        error_free(err);
-        errno = EIO;
-    } else if (ret == 0) {
-        trace_chr_socket_recv_eof(chr, chr->label);
-    }
+    trace_chr_socket_recv_eof(chr, chr->label);
 
     return ret;
 }
-- 
2.48.1

Re: [PATCH 02/10] char-socket: rework tcp_chr_recv()

[Peter Xu]

On Wed, Sep 03, 2025 at 12:44:02PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> First, qio_channel_readv_full() already guarantees BLOCKING and
> CLOEXEC states for incoming descriptors, no reason call extra
> ioctls.
> 
> Second, current implementation calls _set_block() and _set_cloexec()
> again on old descriptors on failure path - we fix this too.
> 
> Finally, handling errors exactly after qio_channel_readv_full() call
> looks more readable.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> ---
>  chardev/char-socket.c | 37 +++++++++++++------------------------
>  1 file changed, 13 insertions(+), 24 deletions(-)
> 
> diff --git a/chardev/char-socket.c b/chardev/char-socket.c
> index 1e8313915b..5b9b19ba8b 100644
> --- a/chardev/char-socket.c
> +++ b/chardev/char-socket.c
> @@ -293,6 +293,18 @@ static ssize_t tcp_chr_recv(Chardev *chr, char *buf, size_t len)
>                                       0, &err);
>      }
>  
> +    if (ret == QIO_CHANNEL_ERR_BLOCK) {
> +        errno = EAGAIN;
> +        return -1;
> +    } else if (ret == -1) {
> +        trace_chr_socket_recv_err(chr, chr->label, error_get_pretty(err));
> +        error_free(err);
> +        errno = EIO;
> +        return -1;
> +    }
> +
> +    assert(ret >= 0);
> +
>      if (msgfds_num) {
>          /* close and clean read_msgfds */
>          for (i = 0; i < s->read_msgfds_num; i++) {
> @@ -307,30 +319,7 @@ static ssize_t tcp_chr_recv(Chardev *chr, char *buf, size_t len)
>          s->read_msgfds_num = msgfds_num;
>      }
>  
> -    for (i = 0; i < s->read_msgfds_num; i++) {
> -        int fd = s->read_msgfds[i];
> -        if (fd < 0) {
> -            continue;
> -        }
> -
> -        /* O_NONBLOCK is preserved across SCM_RIGHTS so reset it */
> -        qemu_socket_set_block(fd);
> -
> -#ifndef MSG_CMSG_CLOEXEC
> -        qemu_set_cloexec(fd);
> -#endif
> -    }
> -
> -    if (ret == QIO_CHANNEL_ERR_BLOCK) {
> -        errno = EAGAIN;
> -        ret = -1;
> -    } else if (ret == -1) {
> -        trace_chr_socket_recv_err(chr, chr->label, error_get_pretty(err));
> -        error_free(err);
> -        errno = EIO;
> -    } else if (ret == 0) {
> -        trace_chr_socket_recv_eof(chr, chr->label);
> -    }
> +    trace_chr_socket_recv_eof(chr, chr->label);

This tracepoint may still need to be put into a ret==0 check.

Looks reasonable other than that..

>  
>      return ret;
>  }
> -- 
> 2.48.1
> 

-- 
Peter Xu

Re: [PATCH 02/10] char-socket: rework tcp_chr_recv()

[Vladimir Sementsov-Ogievskiy]

On 09.09.25 00:53, Peter Xu wrote:
> On Wed, Sep 03, 2025 at 12:44:02PM +0300, Vladimir Sementsov-Ogievskiy wrote:
>> First, qio_channel_readv_full() already guarantees BLOCKING and
>> CLOEXEC states for incoming descriptors, no reason call extra
>> ioctls.
>>
>> Second, current implementation calls _set_block() and _set_cloexec()
>> again on old descriptors on failure path - we fix this too.
>>
>> Finally, handling errors exactly after qio_channel_readv_full() call
>> looks more readable.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
>> ---
>>   chardev/char-socket.c | 37 +++++++++++++------------------------
>>   1 file changed, 13 insertions(+), 24 deletions(-)
>>
>> diff --git a/chardev/char-socket.c b/chardev/char-socket.c
>> index 1e8313915b..5b9b19ba8b 100644
>> --- a/chardev/char-socket.c
>> +++ b/chardev/char-socket.c
>> @@ -293,6 +293,18 @@ static ssize_t tcp_chr_recv(Chardev *chr, char *buf, size_t len)
>>                                        0, &err);
>>       }
>>   
>> +    if (ret == QIO_CHANNEL_ERR_BLOCK) {
>> +        errno = EAGAIN;
>> +        return -1;
>> +    } else if (ret == -1) {
>> +        trace_chr_socket_recv_err(chr, chr->label, error_get_pretty(err));
>> +        error_free(err);
>> +        errno = EIO;
>> +        return -1;
>> +    }
>> +
>> +    assert(ret >= 0);
>> +
>>       if (msgfds_num) {
>>           /* close and clean read_msgfds */
>>           for (i = 0; i < s->read_msgfds_num; i++) {
>> @@ -307,30 +319,7 @@ static ssize_t tcp_chr_recv(Chardev *chr, char *buf, size_t len)
>>           s->read_msgfds_num = msgfds_num;
>>       }
>>   
>> -    for (i = 0; i < s->read_msgfds_num; i++) {
>> -        int fd = s->read_msgfds[i];
>> -        if (fd < 0) {
>> -            continue;
>> -        }
>> -
>> -        /* O_NONBLOCK is preserved across SCM_RIGHTS so reset it */
>> -        qemu_socket_set_block(fd);
>> -
>> -#ifndef MSG_CMSG_CLOEXEC
>> -        qemu_set_cloexec(fd);
>> -#endif
>> -    }
>> -
>> -    if (ret == QIO_CHANNEL_ERR_BLOCK) {
>> -        errno = EAGAIN;
>> -        ret = -1;
>> -    } else if (ret == -1) {
>> -        trace_chr_socket_recv_err(chr, chr->label, error_get_pretty(err));
>> -        error_free(err);
>> -        errno = EIO;
>> -    } else if (ret == 0) {
>> -        trace_chr_socket_recv_eof(chr, chr->label);
>> -    }
>> +    trace_chr_socket_recv_eof(chr, chr->label);
> 
> This tracepoint may still need to be put into a ret==0 check.
> 
> Looks reasonable other than that..
> 

Oh, right. Will fix.

>>   
>>       return ret;
>>   }
>> -- 
>> 2.48.1
>>
> 


-- 
Best regards,
Vladimir

Re: [PATCH 02/10] char-socket: rework tcp_chr_recv()

[Daniel P. Berrangé]

On Wed, Sep 03, 2025 at 12:44:02PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> First, qio_channel_readv_full() already guarantees BLOCKING and
> CLOEXEC states for incoming descriptors, no reason call extra
> ioctls.
> 
> Second, current implementation calls _set_block() and _set_cloexec()
> again on old descriptors on failure path - we fix this too.
> 
> Finally, handling errors exactly after qio_channel_readv_full() call
> looks more readable.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> ---
>  chardev/char-socket.c | 37 +++++++++++++------------------------
>  1 file changed, 13 insertions(+), 24 deletions(-)
> 
> diff --git a/chardev/char-socket.c b/chardev/char-socket.c
> index 1e8313915b..5b9b19ba8b 100644
> --- a/chardev/char-socket.c
> +++ b/chardev/char-socket.c
> @@ -293,6 +293,18 @@ static ssize_t tcp_chr_recv(Chardev *chr, char *buf, size_t len)
>                                       0, &err);
>      }
>  
> +    if (ret == QIO_CHANNEL_ERR_BLOCK) {
> +        errno = EAGAIN;
> +        return -1;
> +    } else if (ret == -1) {
> +        trace_chr_socket_recv_err(chr, chr->label, error_get_pretty(err));
> +        error_free(err);
> +        errno = EIO;
> +        return -1;
> +    }
> +
> +    assert(ret >= 0);

Moving this logic to here means that in the blocking I/O, and/or error
paths, we are not clearing out the previously read s->read_msgfds_num
These should be considered obsolete at the point we start a new read
call, regardless of its success, hence why we had the code ordered in
this way.


> +
>      if (msgfds_num) {
>          /* close and clean read_msgfds */
>          for (i = 0; i < s->read_msgfds_num; i++) {
> @@ -307,30 +319,7 @@ static ssize_t tcp_chr_recv(Chardev *chr, char *buf, size_t len)
>          s->read_msgfds_num = msgfds_num;
>      }
>  
> -    for (i = 0; i < s->read_msgfds_num; i++) {
> -        int fd = s->read_msgfds[i];
> -        if (fd < 0) {
> -            continue;
> -        }
> -
> -        /* O_NONBLOCK is preserved across SCM_RIGHTS so reset it */
> -        qemu_socket_set_block(fd);
> -
> -#ifndef MSG_CMSG_CLOEXEC
> -        qemu_set_cloexec(fd);
> -#endif
> -    }

This for(){} removal is fnie.

> -
> -    if (ret == QIO_CHANNEL_ERR_BLOCK) {
> -        errno = EAGAIN;
> -        ret = -1;
> -    } else if (ret == -1) {
> -        trace_chr_socket_recv_err(chr, chr->label, error_get_pretty(err));
> -        error_free(err);
> -        errno = EIO;
> -    } else if (ret == 0) {
> -        trace_chr_socket_recv_eof(chr, chr->label);
> -    }
> +    trace_chr_socket_recv_eof(chr, chr->label);
>  
>      return ret;
>  }
> -- 
> 2.48.1
> 

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 :|

Re: [PATCH 02/10] char-socket: rework tcp_chr_recv()

[Vladimir Sementsov-Ogievskiy]

On 09.09.25 11:38, Daniel P. Berrangé wrote:
> On Wed, Sep 03, 2025 at 12:44:02PM +0300, Vladimir Sementsov-Ogievskiy wrote:
>> First, qio_channel_readv_full() already guarantees BLOCKING and
>> CLOEXEC states for incoming descriptors, no reason call extra
>> ioctls.
>>
>> Second, current implementation calls _set_block() and _set_cloexec()
>> again on old descriptors on failure path - we fix this too.
>>
>> Finally, handling errors exactly after qio_channel_readv_full() call
>> looks more readable.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
>> ---
>>   chardev/char-socket.c | 37 +++++++++++++------------------------
>>   1 file changed, 13 insertions(+), 24 deletions(-)
>>
>> diff --git a/chardev/char-socket.c b/chardev/char-socket.c
>> index 1e8313915b..5b9b19ba8b 100644
>> --- a/chardev/char-socket.c
>> +++ b/chardev/char-socket.c
>> @@ -293,6 +293,18 @@ static ssize_t tcp_chr_recv(Chardev *chr, char *buf, size_t len)
>>                                        0, &err);
>>       }
>>   
>> +    if (ret == QIO_CHANNEL_ERR_BLOCK) {
>> +        errno = EAGAIN;
>> +        return -1;
>> +    } else if (ret == -1) {
>> +        trace_chr_socket_recv_err(chr, chr->label, error_get_pretty(err));
>> +        error_free(err);
>> +        errno = EIO;
>> +        return -1;
>> +    }
>> +
>> +    assert(ret >= 0);
> 
> Moving this logic to here means that in the blocking I/O, and/or error
> paths, we are not clearing out the previously read s->read_msgfds_num
> These should be considered obsolete at the point we start a new read
> call, regardless of its success, hence why we had the code ordered in
> this way.

Oops, thanks! That's not obvious, I'll add a comment.

> 
> 
>> +
>>       if (msgfds_num) {
>>           /* close and clean read_msgfds */
>>           for (i = 0; i < s->read_msgfds_num; i++) {
>> @@ -307,30 +319,7 @@ static ssize_t tcp_chr_recv(Chardev *chr, char *buf, size_t len)
>>           s->read_msgfds_num = msgfds_num;
>>       }
>>   
>> -    for (i = 0; i < s->read_msgfds_num; i++) {
>> -        int fd = s->read_msgfds[i];
>> -        if (fd < 0) {
>> -            continue;
>> -        }
>> -
>> -        /* O_NONBLOCK is preserved across SCM_RIGHTS so reset it */
>> -        qemu_socket_set_block(fd);
>> -
>> -#ifndef MSG_CMSG_CLOEXEC
>> -        qemu_set_cloexec(fd);
>> -#endif
>> -    }
> 
> This for(){} removal is fnie.
> 
>> -
>> -    if (ret == QIO_CHANNEL_ERR_BLOCK) {
>> -        errno = EAGAIN;
>> -        ret = -1;
>> -    } else if (ret == -1) {
>> -        trace_chr_socket_recv_err(chr, chr->label, error_get_pretty(err));
>> -        error_free(err);
>> -        errno = EIO;
>> -    } else if (ret == 0) {
>> -        trace_chr_socket_recv_eof(chr, chr->label);
>> -    }
>> +    trace_chr_socket_recv_eof(chr, chr->label);
>>   
>>       return ret;
>>   }
>> -- 
>> 2.48.1
>>
> 
> With regards,
> Daniel


-- 
Best regards,
Vladimir

[PATCH 03/10] util: add qemu_set_blocking() function

[Vladimir Sementsov-Ogievskiy]

In generic code we have qio_channel_set_blocking(), which takes
bool parameter, and qemu_file_set_blocking(), which as well takes
bool parameter.

At lower fd-layer we have a mess of functions:

- enough direct calls to g_unix_set_fd_nonblocking()
and several wrappers without bool parameter:

- qemu_scoket_set_nonblock(), which asserts success for posix (still,
  in most cases we can handle the error in better way) and ignores
  error for win32 realization

- qemu_socket_try_set_nonblock(), the best one

- qemu_socket_set_block(), which simply ignores an error, the worst
  case

And all three lack errp argument, so we have to handle it after the
call.

So let's introduce a new socket-layer wrapper, and use it consistently
in following commits.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 include/qemu/osdep.h |  1 +
 util/oslib-posix.c   | 12 ++++++++++++
 util/oslib-win32.c   | 18 ++++++++++++++++++
 3 files changed, 31 insertions(+)

diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index be3460b32f..1b38cb7e45 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -687,6 +687,7 @@ ssize_t qemu_write_full(int fd, const void *buf, size_t count)
     G_GNUC_WARN_UNUSED_RESULT;
 
 void qemu_set_cloexec(int fd);
+bool qemu_set_blocking(int fd, bool block, Error **errp);
 
 /* Return a dynamically allocated directory path that is appropriate for storing
  * local state.
diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index 4ff577e5de..e473938195 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -250,6 +250,18 @@ void qemu_anon_ram_free(void *ptr, size_t size)
 #endif
 }
 
+bool qemu_set_blocking(int fd, bool block, Error **errp)
+{
+    if (!g_unix_set_fd_nonblocking(fd, !block, NULL)) {
+        error_setg_errno(errp, errno,
+                         "Can't set file descriptor %d %s", fd,
+                         block ? "blocking" : "non-blocking");
+        return false;
+    }
+
+    return true;
+}
+
 void qemu_socket_set_block(int fd)
 {
     g_unix_set_fd_nonblocking(fd, false, NULL);
diff --git a/util/oslib-win32.c b/util/oslib-win32.c
index b7351634ec..03044f5b59 100644
--- a/util/oslib-win32.c
+++ b/util/oslib-win32.c
@@ -177,6 +177,24 @@ static int socket_error(void)
     }
 }
 
+bool qemu_set_blocking(int fd, bool block, Error **errp)
+{
+    unsigned long opt = block ? 0 : 1;
+
+    if (block) {
+        qemu_socket_unselect(fd, NULL);
+    }
+
+    if (ioctlsocket(fd, FIONBIO, &opt) != NO_ERROR) {
+        error_setg_errno(errp, socket_error(),
+                         "Can't set file descriptor %d %s", fd,
+                         block ? "blocking" : "non-blocking");
+        return false;
+    }
+
+    return true;
+}
+
 void qemu_socket_set_block(int fd)
 {
     unsigned long opt = 0;
-- 
2.48.1

Re: [PATCH 03/10] util: add qemu_set_blocking() function

[Peter Xu]

On Wed, Sep 03, 2025 at 12:44:03PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> In generic code we have qio_channel_set_blocking(), which takes
> bool parameter, and qemu_file_set_blocking(), which as well takes
> bool parameter.
> 
> At lower fd-layer we have a mess of functions:
> 
> - enough direct calls to g_unix_set_fd_nonblocking()
> and several wrappers without bool parameter:
> 
> - qemu_scoket_set_nonblock(), which asserts success for posix (still,
>   in most cases we can handle the error in better way) and ignores
>   error for win32 realization
> 
> - qemu_socket_try_set_nonblock(), the best one
> 
> - qemu_socket_set_block(), which simply ignores an error, the worst
>   case
> 
> And all three lack errp argument, so we have to handle it after the
> call.
> 
> So let's introduce a new socket-layer wrapper, and use it consistently
> in following commits.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> ---
>  include/qemu/osdep.h |  1 +
>  util/oslib-posix.c   | 12 ++++++++++++
>  util/oslib-win32.c   | 18 ++++++++++++++++++
>  3 files changed, 31 insertions(+)
> 
> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> index be3460b32f..1b38cb7e45 100644
> --- a/include/qemu/osdep.h
> +++ b/include/qemu/osdep.h
> @@ -687,6 +687,7 @@ ssize_t qemu_write_full(int fd, const void *buf, size_t count)
>      G_GNUC_WARN_UNUSED_RESULT;
>  
>  void qemu_set_cloexec(int fd);
> +bool qemu_set_blocking(int fd, bool block, Error **errp);
>  
>  /* Return a dynamically allocated directory path that is appropriate for storing
>   * local state.
> diff --git a/util/oslib-posix.c b/util/oslib-posix.c
> index 4ff577e5de..e473938195 100644
> --- a/util/oslib-posix.c
> +++ b/util/oslib-posix.c
> @@ -250,6 +250,18 @@ void qemu_anon_ram_free(void *ptr, size_t size)
>  #endif
>  }
>  
> +bool qemu_set_blocking(int fd, bool block, Error **errp)
> +{
> +    if (!g_unix_set_fd_nonblocking(fd, !block, NULL)) {

If we want to do the best, we could also pass in GError** here, and convert
GError(.message) to errp?

> +        error_setg_errno(errp, errno,
> +                         "Can't set file descriptor %d %s", fd,
> +                         block ? "blocking" : "non-blocking");
> +        return false;
> +    }
> +
> +    return true;
> +}
> +
>  void qemu_socket_set_block(int fd)
>  {
>      g_unix_set_fd_nonblocking(fd, false, NULL);
> diff --git a/util/oslib-win32.c b/util/oslib-win32.c
> index b7351634ec..03044f5b59 100644
> --- a/util/oslib-win32.c
> +++ b/util/oslib-win32.c
> @@ -177,6 +177,24 @@ static int socket_error(void)
>      }
>  }
>  
> +bool qemu_set_blocking(int fd, bool block, Error **errp)
> +{
> +    unsigned long opt = block ? 0 : 1;
> +
> +    if (block) {
> +        qemu_socket_unselect(fd, NULL);
> +    }
> +
> +    if (ioctlsocket(fd, FIONBIO, &opt) != NO_ERROR) {
> +        error_setg_errno(errp, socket_error(),
> +                         "Can't set file descriptor %d %s", fd,
> +                         block ? "blocking" : "non-blocking");
> +        return false;
> +    }
> +
> +    return true;
> +}
> +
>  void qemu_socket_set_block(int fd)
>  {
>      unsigned long opt = 0;
> -- 
> 2.48.1
> 

-- 
Peter Xu

Re: [PATCH 03/10] util: add qemu_set_blocking() function

[Vladimir Sementsov-Ogievskiy]

On 09.09.25 01:02, Peter Xu wrote:
> On Wed, Sep 03, 2025 at 12:44:03PM +0300, Vladimir Sementsov-Ogievskiy wrote:
>> In generic code we have qio_channel_set_blocking(), which takes
>> bool parameter, and qemu_file_set_blocking(), which as well takes
>> bool parameter.
>>
>> At lower fd-layer we have a mess of functions:
>>
>> - enough direct calls to g_unix_set_fd_nonblocking()
>> and several wrappers without bool parameter:
>>
>> - qemu_scoket_set_nonblock(), which asserts success for posix (still,
>>    in most cases we can handle the error in better way) and ignores
>>    error for win32 realization
>>
>> - qemu_socket_try_set_nonblock(), the best one
>>
>> - qemu_socket_set_block(), which simply ignores an error, the worst
>>    case
>>
>> And all three lack errp argument, so we have to handle it after the
>> call.
>>
>> So let's introduce a new socket-layer wrapper, and use it consistently
>> in following commits.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
>> ---
>>   include/qemu/osdep.h |  1 +
>>   util/oslib-posix.c   | 12 ++++++++++++
>>   util/oslib-win32.c   | 18 ++++++++++++++++++
>>   3 files changed, 31 insertions(+)
>>
>> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
>> index be3460b32f..1b38cb7e45 100644
>> --- a/include/qemu/osdep.h
>> +++ b/include/qemu/osdep.h
>> @@ -687,6 +687,7 @@ ssize_t qemu_write_full(int fd, const void *buf, size_t count)
>>       G_GNUC_WARN_UNUSED_RESULT;
>>   
>>   void qemu_set_cloexec(int fd);
>> +bool qemu_set_blocking(int fd, bool block, Error **errp);
>>   
>>   /* Return a dynamically allocated directory path that is appropriate for storing
>>    * local state.
>> diff --git a/util/oslib-posix.c b/util/oslib-posix.c
>> index 4ff577e5de..e473938195 100644
>> --- a/util/oslib-posix.c
>> +++ b/util/oslib-posix.c
>> @@ -250,6 +250,18 @@ void qemu_anon_ram_free(void *ptr, size_t size)
>>   #endif
>>   }
>>   
>> +bool qemu_set_blocking(int fd, bool block, Error **errp)
>> +{
>> +    if (!g_unix_set_fd_nonblocking(fd, !block, NULL)) {
> 
> If we want to do the best, we could also pass in GError** here, and convert
> GError(.message) to errp?

Hmm, agree, will try.

Interesting, why we just don't sit on GError..

> 
>> +        error_setg_errno(errp, errno,
>> +                         "Can't set file descriptor %d %s", fd,
>> +                         block ? "blocking" : "non-blocking");
>> +        return false;
>> +    }
>> +
>> +    return true;
>> +}
>> +
>>   void qemu_socket_set_block(int fd)
>>   {
>>       g_unix_set_fd_nonblocking(fd, false, NULL);
>> diff --git a/util/oslib-win32.c b/util/oslib-win32.c
>> index b7351634ec..03044f5b59 100644
>> --- a/util/oslib-win32.c
>> +++ b/util/oslib-win32.c
>> @@ -177,6 +177,24 @@ static int socket_error(void)
>>       }
>>   }
>>   
>> +bool qemu_set_blocking(int fd, bool block, Error **errp)
>> +{
>> +    unsigned long opt = block ? 0 : 1;
>> +
>> +    if (block) {
>> +        qemu_socket_unselect(fd, NULL);
>> +    }
>> +
>> +    if (ioctlsocket(fd, FIONBIO, &opt) != NO_ERROR) {
>> +        error_setg_errno(errp, socket_error(),
>> +                         "Can't set file descriptor %d %s", fd,
>> +                         block ? "blocking" : "non-blocking");
>> +        return false;
>> +    }
>> +
>> +    return true;
>> +}
>> +
>>   void qemu_socket_set_block(int fd)
>>   {
>>       unsigned long opt = 0;
>> -- 
>> 2.48.1
>>
> 


-- 
Best regards,
Vladimir

Re: [PATCH 03/10] util: add qemu_set_blocking() function

[Daniel P. Berrangé]

On Wed, Sep 03, 2025 at 12:44:03PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> In generic code we have qio_channel_set_blocking(), which takes
> bool parameter, and qemu_file_set_blocking(), which as well takes
> bool parameter.
> 
> At lower fd-layer we have a mess of functions:
> 
> - enough direct calls to g_unix_set_fd_nonblocking()
> and several wrappers without bool parameter:
> 
> - qemu_scoket_set_nonblock(), which asserts success for posix (still,
>   in most cases we can handle the error in better way) and ignores
>   error for win32 realization
> 
> - qemu_socket_try_set_nonblock(), the best one

IMHO this method should not actually exist. The only reason it was
used in the net/ dir was to validate an FD received from the monitor
was indeed an open socket FD. net/socket.c later added a explicit
method 'net_socket_fd_check' to solve this, so we could have switched
away from try_set_nonblock to set_nonblock.

> 
> - qemu_socket_set_block(), which simply ignores an error, the worst
>   case
> 
> And all three lack errp argument, so we have to handle it after the
> call.

They lacked an errp on the expectation that this call should never
fail, if the FD is valid. While I understand the motivation of that
decision, IMHO, it is an undesirable optimization in the API design.

Not all callers can have such confidence. e.g. received the FD from
the monitor, where we really really really must never allow invalid
user supplied FD to result in an assert().

I'm inclined to agree that we should have an errp here, and any
caller that is *certain* their FD is valid could easily pass in
&error_abort.

> 
> So let's introduce a new socket-layer wrapper, and use it consistently
> in following commits.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> ---
>  include/qemu/osdep.h |  1 +
>  util/oslib-posix.c   | 12 ++++++++++++
>  util/oslib-win32.c   | 18 ++++++++++++++++++
>  3 files changed, 31 insertions(+)
> 
> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> index be3460b32f..1b38cb7e45 100644
> --- a/include/qemu/osdep.h
> +++ b/include/qemu/osdep.h
> @@ -687,6 +687,7 @@ ssize_t qemu_write_full(int fd, const void *buf, size_t count)
>      G_GNUC_WARN_UNUSED_RESULT;
>  
>  void qemu_set_cloexec(int fd);
> +bool qemu_set_blocking(int fd, bool block, Error **errp);
>  
>  /* Return a dynamically allocated directory path that is appropriate for storing
>   * local state.
> diff --git a/util/oslib-posix.c b/util/oslib-posix.c
> index 4ff577e5de..e473938195 100644
> --- a/util/oslib-posix.c
> +++ b/util/oslib-posix.c
> @@ -250,6 +250,18 @@ void qemu_anon_ram_free(void *ptr, size_t size)
>  #endif
>  }
>  
> +bool qemu_set_blocking(int fd, bool block, Error **errp)
> +{
> +    if (!g_unix_set_fd_nonblocking(fd, !block, NULL)) {
> +        error_setg_errno(errp, errno,
> +                         "Can't set file descriptor %d %s", fd,
> +                         block ? "blocking" : "non-blocking");
> +        return false;
> +    }
> +
> +    return true;
> +}
> +
>  void qemu_socket_set_block(int fd)
>  {
>      g_unix_set_fd_nonblocking(fd, false, NULL);
> diff --git a/util/oslib-win32.c b/util/oslib-win32.c
> index b7351634ec..03044f5b59 100644
> --- a/util/oslib-win32.c
> +++ b/util/oslib-win32.c
> @@ -177,6 +177,24 @@ static int socket_error(void)
>      }
>  }
>  
> +bool qemu_set_blocking(int fd, bool block, Error **errp)
> +{
> +    unsigned long opt = block ? 0 : 1;
> +
> +    if (block) {
> +        qemu_socket_unselect(fd, NULL);
> +    }
> +
> +    if (ioctlsocket(fd, FIONBIO, &opt) != NO_ERROR) {
> +        error_setg_errno(errp, socket_error(),
> +                         "Can't set file descriptor %d %s", fd,
> +                         block ? "blocking" : "non-blocking");
> +        return false;
> +    }
> +
> +    return true;
> +}

I don't think this is a good idea - it is giving the impression that
it works on any FD, but it only supports sockets.

With our current APIs it is clear that setting non-blocking is portable
for sockets, but via use of g_unix_set_fd_nonblocking, non-portable
for non-sockets.

The API design of this method is fine, but I think we should keep 
separate 'qemu_socket_set_blocking' and 'qemu_file_set_blocking'
methods, so that we maintain a clear awareness of the difference
for Windows.

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 :|

[PATCH 04/10] util: drop qemu_socket_set_nonblock()

[Vladimir Sementsov-Ogievskiy]

Use common qemu_set_blocking() instead.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 contrib/ivshmem-server/ivshmem-server.c |  5 ++++-
 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, 62 insertions(+), 37 deletions(-)

diff --git a/contrib/ivshmem-server/ivshmem-server.c b/contrib/ivshmem-server/ivshmem-server.c
index 2f3c7320a6..9ccd436ee4 100644
--- a/contrib/ivshmem-server/ivshmem-server.c
+++ b/contrib/ivshmem-server/ivshmem-server.c
@@ -146,7 +146,10 @@ ivshmem_server_handle_new_conn(IvshmemServer *server)
         return -1;
     }
 
-    qemu_socket_set_nonblock(newfd);
+    if (!qemu_set_blocking(newfd, false, NULL)) {
+        close(newfd);
+        return -1;
+    }
     IVSHMEM_SERVER_DEBUG(server, "accept()=%d\n", newfd);
 
     /* allocate new structure for this peer */
diff --git a/hw/hyperv/syndbg.c b/hw/hyperv/syndbg.c
index ac7e15f6f1..bcdfdf6af7 100644
--- a/hw/hyperv/syndbg.c
+++ b/hw/hyperv/syndbg.c
@@ -338,7 +338,9 @@ static void hv_syndbg_realize(DeviceState *dev, Error **errp)
         return;
     }
 
-    qemu_socket_set_nonblock(syndbg->socket);
+    if (!qemu_set_blocking(syndbg->socket, false, errp)) {
+        return;
+    }
 
     syndbg->servaddr.sin_port = htons(syndbg->host_port);
     syndbg->servaddr.sin_family = AF_INET;
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 1e1d6b0d6e..36c9c2e04d 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -2039,7 +2039,10 @@ static int vhost_user_postcopy_advise(struct vhost_dev *dev, Error **errp)
         error_setg(errp, "%s: Failed to get ufd", __func__);
         return -EIO;
     }
-    qemu_socket_set_nonblock(ufd);
+    if (!qemu_set_blocking(ufd, false, errp)) {
+        close(ufd);
+        return -EINVAL;
+    }
 
     /* register ufd with userfault thread */
     u->postcopy_fd.fd = ufd;
diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h
index c562690d89..6477f90b9e 100644
--- a/include/qemu/sockets.h
+++ b/include/qemu/sockets.h
@@ -48,7 +48,6 @@ int socket_set_cork(int fd, int v);
 int socket_set_nodelay(int fd);
 void qemu_socket_set_block(int fd);
 int qemu_socket_try_set_nonblock(int fd);
-void qemu_socket_set_nonblock(int fd);
 int socket_set_fast_reuse(int fd);
 
 #ifdef WIN32
diff --git a/io/channel-socket.c b/io/channel-socket.c
index 3b7ca924ff..e77602a22e 100644
--- a/io/channel-socket.c
+++ b/io/channel-socket.c
@@ -820,11 +820,10 @@ qio_channel_socket_set_blocking(QIOChannel *ioc,
 {
     QIOChannelSocket *sioc = QIO_CHANNEL_SOCKET(ioc);
 
-    if (enabled) {
-        qemu_socket_set_block(sioc->fd);
-    } else {
-        qemu_socket_set_nonblock(sioc->fd);
+    if (!qemu_set_blocking(sioc->fd, enabled, errp)) {
+        return -1;
     }
+
     return 0;
 }
 
diff --git a/net/dgram.c b/net/dgram.c
index 48f653bceb..fb9ded30df 100644
--- a/net/dgram.c
+++ b/net/dgram.c
@@ -226,7 +226,10 @@ static int net_dgram_mcast_create(struct sockaddr_in *mcastaddr,
         }
     }
 
-    qemu_socket_set_nonblock(fd);
+    if (!qemu_set_blocking(fd, false, errp)) {
+        goto fail;
+    }
+
     return fd;
 fail:
     if (fd >= 0) {
@@ -504,7 +507,11 @@ int net_init_dgram(const Netdev *netdev, const char *name,
             close(fd);
             return -1;
         }
-        qemu_socket_set_nonblock(fd);
+
+        if (!qemu_set_blocking(fd, false, errp)) {
+            close(fd);
+            return -1;
+        }
 
         dest_len = sizeof(raddr_in);
         dest_addr = g_malloc(dest_len);
@@ -551,7 +558,10 @@ int net_init_dgram(const Netdev *netdev, const char *name,
             close(fd);
             return -1;
         }
-        qemu_socket_set_nonblock(fd);
+        if (!qemu_set_blocking(fd, false, errp)) {
+            close(fd);
+            return -1;
+        }
 
         dest_len = sizeof(raddr_un);
         dest_addr = g_malloc(dest_len);
diff --git a/net/l2tpv3.c b/net/l2tpv3.c
index b5547cb917..cdfc641aa6 100644
--- a/net/l2tpv3.c
+++ b/net/l2tpv3.c
@@ -648,6 +648,9 @@ int net_init_l2tpv3(const Netdev *netdev,
         error_setg(errp, "could not bind socket err=%i", errno);
         goto outerr;
     }
+    if (!qemu_set_blocking(fd, false, errp)) {
+        goto outerr;
+    }
 
     freeaddrinfo(result);
 
@@ -709,8 +712,6 @@ int net_init_l2tpv3(const Netdev *netdev,
     s->vec = g_new(struct iovec, MAX_L2TPV3_IOVCNT);
     s->header_buf = g_malloc(s->header_size);
 
-    qemu_socket_set_nonblock(fd);
-
     s->fd = fd;
     s->counter = 0;
 
diff --git a/net/socket.c b/net/socket.c
index 784dda686f..db25e3d9ae 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -295,7 +295,10 @@ static int net_socket_mcast_create(struct sockaddr_in *mcastaddr,
         }
     }
 
-    qemu_socket_set_nonblock(fd);
+    if (!qemu_set_blocking(fd, false, errp)) {
+        goto fail;
+    }
+
     return fd;
 fail:
     if (fd >= 0)
@@ -508,7 +511,10 @@ static int net_socket_listen_init(NetClientState *peer,
         error_setg_errno(errp, errno, "can't create stream socket");
         return -1;
     }
-    qemu_socket_set_nonblock(fd);
+    if (!qemu_set_blocking(fd, false, errp)) {
+        close(fd);
+        return -1;
+    }
 
     socket_set_fast_reuse(fd);
 
@@ -556,7 +562,10 @@ static int net_socket_connect_init(NetClientState *peer,
         error_setg_errno(errp, errno, "can't create stream socket");
         return -1;
     }
-    qemu_socket_set_nonblock(fd);
+    if (!qemu_set_blocking(fd, false, errp)) {
+        close(fd);
+        return -1;
+    }
 
     connected = 0;
     for(;;) {
@@ -671,7 +680,10 @@ static int net_socket_udp_init(NetClientState *peer,
         close(fd);
         return -1;
     }
-    qemu_socket_set_nonblock(fd);
+    if (!qemu_set_blocking(fd, false, errp)) {
+        close(fd);
+        return -1;
+    }
 
     s = net_socket_fd_init_dgram(peer, model, name, fd, 0, NULL, errp);
     if (!s) {
diff --git a/qga/channel-posix.c b/qga/channel-posix.c
index 465d688ecb..ddf8ebdc5e 100644
--- a/qga/channel-posix.c
+++ b/qga/channel-posix.c
@@ -28,6 +28,7 @@ static gboolean ga_channel_listen_accept(GIOChannel *channel,
     GAChannel *c = data;
     int ret, client_fd;
     bool accepted = false;
+    Error *err = NULL;
 
     g_assert(channel != NULL);
 
@@ -36,7 +37,11 @@ static gboolean ga_channel_listen_accept(GIOChannel *channel,
         g_warning("error converting fd to gsocket: %s", strerror(errno));
         goto out;
     }
-    qemu_socket_set_nonblock(client_fd);
+    if (!qemu_set_blocking(client_fd, false, &err)) {
+        g_warning("errer: %s", error_get_pretty(err));
+        error_free(err);
+        goto out;
+    }
     ret = ga_channel_client_add(c, client_fd);
     if (ret) {
         g_warning("error setting up connection");
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)) {
+        goto cleanup;
+    }
+
     if (connect(cfd, (struct sockaddr *)&ss, sslen) < 0) {
         if (errno == EINPROGRESS) {
             check_soerr = true;
diff --git a/tests/unit/test-crypto-tlssession.c b/tests/unit/test-crypto-tlssession.c
index 554054e934..61311cbe6e 100644
--- a/tests/unit/test-crypto-tlssession.c
+++ b/tests/unit/test-crypto-tlssession.c
@@ -112,8 +112,8 @@ static void test_crypto_tls_session_psk(void)
      * thread, so we need these non-blocking to avoid deadlock
      * of ourselves
      */
-    qemu_socket_set_nonblock(channel[0]);
-    qemu_socket_set_nonblock(channel[1]);
+    qemu_set_blocking(channel[0], false, &error_abort);
+    qemu_set_blocking(channel[1], false, &error_abort);
 
     clientCreds = test_tls_creds_psk_create(
         QCRYPTO_TLS_CREDS_ENDPOINT_CLIENT,
@@ -264,8 +264,8 @@ static void test_crypto_tls_session_x509(const void *opaque)
      * thread, so we need these non-blocking to avoid deadlock
      * of ourselves
      */
-    qemu_socket_set_nonblock(channel[0]);
-    qemu_socket_set_nonblock(channel[1]);
+    qemu_set_blocking(channel[0], false, &error_abort);
+    qemu_set_blocking(channel[1], false, &error_abort);
 
 #define CLIENT_CERT_DIR "tests/test-crypto-tlssession-client/"
 #define SERVER_CERT_DIR "tests/test-crypto-tlssession-server/"
diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index e473938195..dc23b33210 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -272,13 +272,6 @@ int qemu_socket_try_set_nonblock(int fd)
     return g_unix_set_fd_nonblocking(fd, true, NULL) ? 0 : -errno;
 }
 
-void qemu_socket_set_nonblock(int fd)
-{
-    int f;
-    f = qemu_socket_try_set_nonblock(fd);
-    assert(f == 0);
-}
-
 int socket_set_fast_reuse(int fd)
 {
     int val = 1, ret;
diff --git a/util/oslib-win32.c b/util/oslib-win32.c
index 03044f5b59..1566eb57e7 100644
--- a/util/oslib-win32.c
+++ b/util/oslib-win32.c
@@ -211,11 +211,6 @@ int qemu_socket_try_set_nonblock(int fd)
     return 0;
 }
 
-void qemu_socket_set_nonblock(int fd)
-{
-    (void)qemu_socket_try_set_nonblock(fd);
-}
-
 int socket_set_fast_reuse(int fd)
 {
     /* Enabling the reuse of an endpoint that was used by a socket still in
diff --git a/util/vhost-user-server.c b/util/vhost-user-server.c
index b19229074a..8dcd32dc65 100644
--- a/util/vhost-user-server.c
+++ b/util/vhost-user-server.c
@@ -78,7 +78,7 @@ static void vmsg_unblock_fds(VhostUserMsg *vmsg)
     }
 
     for (i = 0; i < vmsg->fd_num; i++) {
-        qemu_socket_set_nonblock(vmsg->fds[i]);
+        qemu_set_blocking(vmsg->fds[i], false, &error_abort);
     }
 }
 
@@ -303,7 +303,7 @@ set_watch(VuDev *vu_dev, int fd, int vu_evt,
 
         vu_fd_watch->fd = fd;
         vu_fd_watch->cb = cb;
-        qemu_socket_set_nonblock(fd);
+        qemu_set_blocking(fd, false, &error_abort);
         aio_set_fd_handler(server->ctx, fd, kick_handler,
                            NULL, NULL, NULL, vu_fd_watch);
         vu_fd_watch->vu_dev = vu_dev;
-- 
2.48.1

Re: [PATCH 04/10] util: drop qemu_socket_set_nonblock()

[Peter Xu]

On Wed, Sep 03, 2025 at 12:44:04PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> Use common qemu_set_blocking() instead.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>

Posix's qemu_socket_set_nonblock() asserts the retval.. While Windows's one
doesn't.  IIUC that's the only reason you provided the generic error
path in all callers, just in case some of them might fail on Windows?

I wished Windows's one has an assertion from the start too.  Maybe none of
the failure path would really trigger.. Not a big deal:

Reviewed-by: Peter Xu <peterx@redhat.com>

One nitpick below:

> ---
>  contrib/ivshmem-server/ivshmem-server.c |  5 ++++-
>  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, 62 insertions(+), 37 deletions(-)
> 
> diff --git a/contrib/ivshmem-server/ivshmem-server.c b/contrib/ivshmem-server/ivshmem-server.c
> index 2f3c7320a6..9ccd436ee4 100644
> --- a/contrib/ivshmem-server/ivshmem-server.c
> +++ b/contrib/ivshmem-server/ivshmem-server.c
> @@ -146,7 +146,10 @@ ivshmem_server_handle_new_conn(IvshmemServer *server)
>          return -1;
>      }
>  
> -    qemu_socket_set_nonblock(newfd);
> +    if (!qemu_set_blocking(newfd, false, NULL)) {

Better if with a IVSHMEM_SERVER_DEBUG(), which follows the original lines.

> +        close(newfd);
> +        return -1;
> +    }
>      IVSHMEM_SERVER_DEBUG(server, "accept()=%d\n", newfd);
>  
>      /* allocate new structure for this peer */

-- 
Peter Xu

Re: [PATCH 04/10] util: drop qemu_socket_set_nonblock()

[Vladimir Sementsov-Ogievskiy]

On 09.09.25 01:16, Peter Xu wrote:
> On Wed, Sep 03, 2025 at 12:44:04PM +0300, Vladimir Sementsov-Ogievskiy wrote:
>> Use common qemu_set_blocking() instead.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> 
> Posix's qemu_socket_set_nonblock() asserts the retval.. While Windows's one
> doesn't.  IIUC that's the only reason you provided the generic error
> path in all callers, just in case some of them might fail on Windows?

Honestly, I thought that checking error on Linux is good too.. It may fail,
why not to check, where possible?

> 
> I wished Windows's one has an assertion from the start too.  Maybe none of
> the failure path would really trigger.. Not a big deal:
> 
> Reviewed-by: Peter Xu <peterx@redhat.com>
> 
> One nitpick below:
> 
>> ---
>>   contrib/ivshmem-server/ivshmem-server.c |  5 ++++-
>>   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, 62 insertions(+), 37 deletions(-)
>>
>> diff --git a/contrib/ivshmem-server/ivshmem-server.c b/contrib/ivshmem-server/ivshmem-server.c
>> index 2f3c7320a6..9ccd436ee4 100644
>> --- a/contrib/ivshmem-server/ivshmem-server.c
>> +++ b/contrib/ivshmem-server/ivshmem-server.c
>> @@ -146,7 +146,10 @@ ivshmem_server_handle_new_conn(IvshmemServer *server)
>>           return -1;
>>       }
>>   
>> -    qemu_socket_set_nonblock(newfd);
>> +    if (!qemu_set_blocking(newfd, false, NULL)) {
> 
> Better if with a IVSHMEM_SERVER_DEBUG(), which follows the original lines.

Will add

> 
>> +        close(newfd);
>> +        return -1;
>> +    }
>>       IVSHMEM_SERVER_DEBUG(server, "accept()=%d\n", newfd);
>>   
>>       /* allocate new structure for this peer */
> 

Thanks for reviewing my patches!


-- 
Best regards,
Vladimir

Re: [PATCH 04/10] util: drop qemu_socket_set_nonblock()

[Daniel P. Berrangé]

On Tue, Sep 09, 2025 at 11:19:39AM +0300, Vladimir Sementsov-Ogievskiy wrote:
> On 09.09.25 01:16, Peter Xu wrote:
> > On Wed, Sep 03, 2025 at 12:44:04PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> > > Use common qemu_set_blocking() instead.
> > > 
> > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> > 
> > Posix's qemu_socket_set_nonblock() asserts the retval.. While Windows's one
> > doesn't.  IIUC that's the only reason you provided the generic error
> > path in all callers, just in case some of them might fail on Windows?
> 
> Honestly, I thought that checking error on Linux is good too.. It may fail,
> why not to check, where possible?

Yep, it diagnoses the case where the FD might be invalid, or where
QEMU might not have access to it. This could potentially avoid killing
the VM if a FD was passed to QEMU over monitor that had access limited
by SELinux.


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 :|

Re: [PATCH 04/10] util: drop qemu_socket_set_nonblock()

[Vladimir Sementsov-Ogievskiy]

On 10.09.25 12:41, Daniel P. Berrangé wrote:
> On Tue, Sep 09, 2025 at 11:19:39AM +0300, Vladimir Sementsov-Ogievskiy wrote:
>> On 09.09.25 01:16, Peter Xu wrote:
>>> On Wed, Sep 03, 2025 at 12:44:04PM +0300, Vladimir Sementsov-Ogievskiy wrote:
>>>> Use common qemu_set_blocking() instead.
>>>>
>>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
>>>
>>> Posix's qemu_socket_set_nonblock() asserts the retval.. While Windows's one
>>> doesn't.  IIUC that's the only reason you provided the generic error
>>> path in all callers, just in case some of them might fail on Windows?
>>
>> Honestly, I thought that checking error on Linux is good too.. It may fail,
>> why not to check, where possible?
> 
> Yep, it diagnoses the case where the FD might be invalid, or where
> QEMU might not have access to it. This could potentially avoid killing
> the VM if a FD was passed to QEMU over monitor that had access limited
> by SELinux.
> 

Good reason! I'll add it commit msg.

-- 
Best regards,
Vladimir

Re: [PATCH 04/10] util: drop qemu_socket_set_nonblock()

[Daniel P. Berrangé]

On Wed, Sep 03, 2025 at 12:44:04PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> Use common qemu_set_blocking() instead.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> ---
>  contrib/ivshmem-server/ivshmem-server.c |  5 ++++-
>  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, 62 insertions(+), 37 deletions(-)
> 

> diff --git a/qga/channel-posix.c b/qga/channel-posix.c
> index 465d688ecb..ddf8ebdc5e 100644
> --- a/qga/channel-posix.c
> +++ b/qga/channel-posix.c
> @@ -28,6 +28,7 @@ static gboolean ga_channel_listen_accept(GIOChannel *channel,
>      GAChannel *c = data;
>      int ret, client_fd;
>      bool accepted = false;
> +    Error *err = NULL;
>  
>      g_assert(channel != NULL);
>  
> @@ -36,7 +37,11 @@ static gboolean ga_channel_listen_accept(GIOChannel *channel,
>          g_warning("error converting fd to gsocket: %s", strerror(errno));
>          goto out;
>      }
> -    qemu_socket_set_nonblock(client_fd);
> +    if (!qemu_set_blocking(client_fd, false, &err)) {
> +        g_warning("errer: %s", error_get_pretty(err));

s/errer/error/


This is a pre-existing problem, but none of this code should be using
g_warning. g_printerr() should have been used for printing error
messages. I'm not expecting you to fix that, just an observation.

> +        error_free(err);
> +        goto out;
> +    }
>      ret = ga_channel_client_add(c, client_fd);
>      if (ret) {
>          g_warning("error setting up connection");


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 :|

Re: [PATCH 04/10] util: drop qemu_socket_set_nonblock()

[Vladimir Sementsov-Ogievskiy]

On 10.09.25 12:44, Daniel P. Berrangé wrote:
> On Wed, Sep 03, 2025 at 12:44:04PM +0300, Vladimir Sementsov-Ogievskiy wrote:
>> Use common qemu_set_blocking() instead.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
>> ---
>>   contrib/ivshmem-server/ivshmem-server.c |  5 ++++-
>>   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, 62 insertions(+), 37 deletions(-)
>>
> 
>> diff --git a/qga/channel-posix.c b/qga/channel-posix.c
>> index 465d688ecb..ddf8ebdc5e 100644
>> --- a/qga/channel-posix.c
>> +++ b/qga/channel-posix.c
>> @@ -28,6 +28,7 @@ static gboolean ga_channel_listen_accept(GIOChannel *channel,
>>       GAChannel *c = data;
>>       int ret, client_fd;
>>       bool accepted = false;
>> +    Error *err = NULL;
>>   
>>       g_assert(channel != NULL);
>>   
>> @@ -36,7 +37,11 @@ static gboolean ga_channel_listen_accept(GIOChannel *channel,
>>           g_warning("error converting fd to gsocket: %s", strerror(errno));
>>           goto out;
>>       }
>> -    qemu_socket_set_nonblock(client_fd);
>> +    if (!qemu_set_blocking(client_fd, false, &err)) {
>> +        g_warning("errer: %s", error_get_pretty(err));
> 
> s/errer/error/
> 
> 
> This is a pre-existing problem, but none of this code should be using
> g_warning. g_printerr() should have been used for printing error
> messages. I'm not expecting you to fix that, just an observation.

Sure.

> 
>> +        error_free(err);
>> +        goto out;
>> +    }
>>       ret = ga_channel_client_add(c, client_fd);
>>       if (ret) {
>>           g_warning("error setting up connection");
> 
> 
> With regards,
> Daniel


-- 
Best regards,
Vladimir

Re: [PATCH 04/10] util: drop qemu_socket_set_nonblock()

[Vladimir Sementsov-Ogievskiy]

On 10.09.25 13:33, Vladimir Sementsov-Ogievskiy wrote:
> On 10.09.25 12:44, Daniel P. Berrangé wrote:
>> On Wed, Sep 03, 2025 at 12:44:04PM +0300, Vladimir Sementsov-Ogievskiy wrote:
>>> Use common qemu_set_blocking() instead.
>>>
>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
>>> ---
>>>   contrib/ivshmem-server/ivshmem-server.c |  5 ++++-
>>>   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, 62 insertions(+), 37 deletions(-)
>>>
>>
>>> diff --git a/qga/channel-posix.c b/qga/channel-posix.c
>>> index 465d688ecb..ddf8ebdc5e 100644
>>> --- a/qga/channel-posix.c
>>> +++ b/qga/channel-posix.c
>>> @@ -28,6 +28,7 @@ static gboolean ga_channel_listen_accept(GIOChannel *channel,
>>>       GAChannel *c = data;
>>>       int ret, client_fd;
>>>       bool accepted = false;
>>> +    Error *err = NULL;
>>>       g_assert(channel != NULL);
>>> @@ -36,7 +37,11 @@ static gboolean ga_channel_listen_accept(GIOChannel *channel,
>>>           g_warning("error converting fd to gsocket: %s", strerror(errno));
>>>           goto out;
>>>       }
>>> -    qemu_socket_set_nonblock(client_fd);
>>> +    if (!qemu_set_blocking(client_fd, false, &err)) {
>>> +        g_warning("errer: %s", error_get_pretty(err));
>>
>> s/errer/error/
>>
>>
>> This is a pre-existing problem, but none of this code should be using
>> g_warning. g_printerr() should have been used for printing error
>> messages. I'm not expecting you to fix that, just an observation.
> 
> Sure.

Ha, some how I've read "not expecting" with opposite meaning) So by "sure", I meant "will fix".

Still now looking at code and your comment more carefully, this means one more patch, as
adding g_prinerr among two g_warnings is mess.

One more patch won't hurt I think.

> 
>>
>>> +        error_free(err);
>>> +        goto out;
>>> +    }
>>>       ret = ga_channel_client_add(c, client_fd);
>>>       if (ret) {
>>>           g_warning("error setting up connection");
>>
>>
>> With regards,
>> Daniel
> 
> 


-- 
Best regards,
Vladimir

Re: [PATCH 04/10] util: drop qemu_socket_set_nonblock()

[Vladimir Sementsov-Ogievskiy]

On 10.09.25 12:44, Daniel P. Berrangé wrote:
>> @@ -36,7 +37,11 @@ static gboolean ga_channel_listen_accept(GIOChannel *channel,
>>           g_warning("error converting fd to gsocket: %s", strerror(errno));
>>           goto out;
>>       }
>> -    qemu_socket_set_nonblock(client_fd);
>> +    if (!qemu_set_blocking(client_fd, false, &err)) {
>> +        g_warning("errer: %s", error_get_pretty(err));
> s/errer/error/
> 
> 
> This is a pre-existing problem, but none of this code should be using
> g_warning. g_printerr() should have been used for printing error
> messages. I'm not expecting you to fix that, just an observation.


Why not g_error()? I see some g_warnings in qga code a correct "warnings", not "errors".. And if we use sometimes g_warning, the g_error is more correct pair for it.

Or we don't want any of g_error / g_warning in QEMU code?

-- 
Best regards,
Vladimir

Re: [PATCH 04/10] util: drop qemu_socket_set_nonblock()

[Daniel P. Berrangé]

On Wed, Sep 10, 2025 at 08:55:57PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> On 10.09.25 12:44, Daniel P. Berrangé wrote:
> > > @@ -36,7 +37,11 @@ static gboolean ga_channel_listen_accept(GIOChannel *channel,
> > >           g_warning("error converting fd to gsocket: %s", strerror(errno));
> > >           goto out;
> > >       }
> > > -    qemu_socket_set_nonblock(client_fd);
> > > +    if (!qemu_set_blocking(client_fd, false, &err)) {
> > > +        g_warning("errer: %s", error_get_pretty(err));
> > s/errer/error/
> > 
> > 
> > This is a pre-existing problem, but none of this code should be using
> > g_warning. g_printerr() should have been used for printing error
> > messages. I'm not expecting you to fix that, just an observation.
> 
> 
> Why not g_error()? I see some g_warnings in qga code a correct "warnings", not "errors".. And if we use sometimes g_warning, the g_error is more correct pair for it.
> 
> Or we don't want any of g_error / g_warning in QEMU code?

g_error will call abort() after printing the message, which will
prevent graceful cleanup and result in a core file, so is not
very desirable to use.

g_warning will also turn into g_error if G_DEBUG=fatal-warnings
is set.

We really just want a plain message printed on the console with
no side effects, and g_printerr gives us that.


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 :|

Re: [PATCH 04/10] util: drop qemu_socket_set_nonblock()

[Vladimir Sementsov-Ogievskiy]

On 10.09.25 21:15, Daniel P. Berrangé wrote:
> On Wed, Sep 10, 2025 at 08:55:57PM +0300, Vladimir Sementsov-Ogievskiy wrote:
>> On 10.09.25 12:44, Daniel P. Berrangé wrote:
>>>> @@ -36,7 +37,11 @@ static gboolean ga_channel_listen_accept(GIOChannel *channel,
>>>>            g_warning("error converting fd to gsocket: %s", strerror(errno));
>>>>            goto out;
>>>>        }
>>>> -    qemu_socket_set_nonblock(client_fd);
>>>> +    if (!qemu_set_blocking(client_fd, false, &err)) {
>>>> +        g_warning("errer: %s", error_get_pretty(err));
>>> s/errer/error/
>>>
>>>
>>> This is a pre-existing problem, but none of this code should be using
>>> g_warning. g_printerr() should have been used for printing error
>>> messages. I'm not expecting you to fix that, just an observation.
>>
>>
>> Why not g_error()? I see some g_warnings in qga code a correct "warnings", not "errors".. And if we use sometimes g_warning, the g_error is more correct pair for it.
>>
>> Or we don't want any of g_error / g_warning in QEMU code?
> 
> g_error will call abort() after printing the message, which will
> prevent graceful cleanup and result in a core file, so is not
> very desirable to use.

Oh, right, I was stupid)

> 
> g_warning will also turn into g_error if G_DEBUG=fatal-warnings
> is set.
> 
> We really just want a plain message printed on the console with
> no side effects, and g_printerr gives us that.
> 
> 

Still, it's strange for me, that we just not use error_reprot()/warn_report() everywhere.

I see we have underlying error_vprintf() realization for code without monitor in stubs/error-printf.c..


-- 
Best regards,
Vladimir

Re: [PATCH 04/10] util: drop qemu_socket_set_nonblock()

[Daniel P. Berrangé]

On Wed, Sep 10, 2025 at 09:52:40PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> On 10.09.25 21:15, Daniel P. Berrangé wrote:
> > On Wed, Sep 10, 2025 at 08:55:57PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> > > On 10.09.25 12:44, Daniel P. Berrangé wrote:
> > > > > @@ -36,7 +37,11 @@ static gboolean ga_channel_listen_accept(GIOChannel *channel,
> > > > >            g_warning("error converting fd to gsocket: %s", strerror(errno));
> > > > >            goto out;
> > > > >        }
> > > > > -    qemu_socket_set_nonblock(client_fd);
> > > > > +    if (!qemu_set_blocking(client_fd, false, &err)) {
> > > > > +        g_warning("errer: %s", error_get_pretty(err));
> > > > s/errer/error/
> > > > 
> > > > 
> > > > This is a pre-existing problem, but none of this code should be using
> > > > g_warning. g_printerr() should have been used for printing error
> > > > messages. I'm not expecting you to fix that, just an observation.
> > > 
> > > 
> > > Why not g_error()? I see some g_warnings in qga code a correct "warnings", not "errors".. And if we use sometimes g_warning, the g_error is more correct pair for it.
> > > 
> > > Or we don't want any of g_error / g_warning in QEMU code?
> > 
> > g_error will call abort() after printing the message, which will
> > prevent graceful cleanup and result in a core file, so is not
> > very desirable to use.
> 
> Oh, right, I was stupid)
> 
> > 
> > g_warning will also turn into g_error if G_DEBUG=fatal-warnings
> > is set.
> > 
> > We really just want a plain message printed on the console with
> > no side effects, and g_printerr gives us that.
> > 
> > 
> 
> Still, it's strange for me, that we just not use error_reprot()/warn_report() everywhere.

Those would be viable alternatives too. Basically anything is better
than 'g_warning' for this scenario :-)

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 :|

[PATCH 05/10] util: drop qemu_socket_try_set_nonblock()

[Vladimir Sementsov-Ogievskiy]

Now we can use qemu_set_blocking() in these cases.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 include/qemu/sockets.h |  1 -
 net/dgram.c            | 12 +++---------
 net/socket.c           |  7 ++-----
 net/stream.c           |  9 +++------
 net/stream_data.c      | 10 ++++------
 util/oslib-posix.c     |  4 ----
 util/oslib-win32.c     |  9 ---------
 7 files changed, 12 insertions(+), 40 deletions(-)

diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h
index 6477f90b9e..9512fec514 100644
--- a/include/qemu/sockets.h
+++ b/include/qemu/sockets.h
@@ -47,7 +47,6 @@ ssize_t qemu_send_full(int s, const void *buf, size_t count)
 int socket_set_cork(int fd, int v);
 int socket_set_nodelay(int fd);
 void qemu_socket_set_block(int fd);
-int qemu_socket_try_set_nonblock(int fd);
 int socket_set_fast_reuse(int fd);
 
 #ifdef WIN32
diff --git a/net/dgram.c b/net/dgram.c
index fb9ded30df..baa126d514 100644
--- a/net/dgram.c
+++ b/net/dgram.c
@@ -287,7 +287,7 @@ static int net_dgram_mcast_init(NetClientState *peer,
                                 Error **errp)
 {
     NetDgramState *s;
-    int fd, ret;
+    int fd;
     struct sockaddr_in *saddr;
 
     if (remote->type != SOCKET_ADDRESS_TYPE_INET) {
@@ -335,11 +335,8 @@ static int net_dgram_mcast_init(NetClientState *peer,
                 g_free(saddr);
                 return -1;
             }
-            ret = qemu_socket_try_set_nonblock(fd);
-            if (ret < 0) {
+            if (!qemu_set_blocking(fd, false, errp)) {
                 g_free(saddr);
-                error_setg_errno(errp, -ret, "%s: Can't use file descriptor %d",
-                                 name, fd);
                 return -1;
             }
 
@@ -572,10 +569,7 @@ int net_init_dgram(const Netdev *netdev, const char *name,
         if (fd == -1) {
             return -1;
         }
-        ret = qemu_socket_try_set_nonblock(fd);
-        if (ret < 0) {
-            error_setg_errno(errp, -ret, "%s: Can't use file descriptor %d",
-                             name, fd);
+        if (!qemu_set_blocking(fd, false, errp)) {
             return -1;
         }
         dest_addr = NULL;
diff --git a/net/socket.c b/net/socket.c
index db25e3d9ae..1ad03fc9d4 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -718,7 +718,7 @@ int net_init_socket(const Netdev *netdev, const char *name,
     }
 
     if (sock->fd) {
-        int fd, ret, so_type;
+        int fd, so_type;
 
         fd = monitor_fd_param(monitor_cur(), sock->fd, errp);
         if (fd == -1) {
@@ -728,10 +728,7 @@ int net_init_socket(const Netdev *netdev, const char *name,
         if (so_type < 0) {
             return -1;
         }
-        ret = qemu_socket_try_set_nonblock(fd);
-        if (ret < 0) {
-            error_setg_errno(errp, -ret, "%s: Can't use file descriptor %d",
-                             name, fd);
+        if (!qemu_set_blocking(fd, false, errp)) {
             return -1;
         }
         switch (so_type) {
diff --git a/net/stream.c b/net/stream.c
index d893f02cab..94f823a2a7 100644
--- a/net/stream.c
+++ b/net/stream.c
@@ -138,7 +138,6 @@ static void net_stream_server_listening(QIOTask *task, gpointer opaque)
     NetStreamData *d = opaque;
     QIOChannelSocket *listen_sioc = QIO_CHANNEL_SOCKET(d->listen_ioc);
     SocketAddress *addr;
-    int ret;
     Error *err = NULL;
 
     if (qio_task_propagate_error(task, &err)) {
@@ -149,13 +148,11 @@ static void net_stream_server_listening(QIOTask *task, gpointer opaque)
 
     addr = qio_channel_socket_get_local_address(listen_sioc, NULL);
     g_assert(addr != NULL);
-    ret = qemu_socket_try_set_nonblock(listen_sioc->fd);
-    if (addr->type == SOCKET_ADDRESS_TYPE_FD && ret < 0) {
-        qemu_set_info_str(&d->nc, "can't use file descriptor %s (errno %d)",
-                          addr->u.fd.str, -ret);
+    if (!qemu_set_blocking(listen_sioc->fd, false, &err)) {
+        qemu_set_info_str(&d->nc, "error: %s", error_get_pretty(err));
+        error_free(err);
         return;
     }
-    g_assert(ret == 0);
     qapi_free_SocketAddress(addr);
 
     d->nc.link_down = true;
diff --git a/net/stream_data.c b/net/stream_data.c
index 5af27e0d1d..03740e9f73 100644
--- a/net/stream_data.c
+++ b/net/stream_data.c
@@ -12,6 +12,7 @@
 #include "net/net.h"
 #include "io/channel.h"
 #include "io/net-listener.h"
+#include "qemu/sockets.h"
 
 #include "stream_data.h"
 
@@ -154,7 +155,6 @@ int net_stream_data_client_connected(QIOTask *task, NetStreamData *d)
 {
     QIOChannelSocket *sioc = QIO_CHANNEL_SOCKET(d->ioc);
     SocketAddress *addr;
-    int ret;
     Error *err = NULL;
 
     if (qio_task_propagate_error(task, &err)) {
@@ -166,14 +166,12 @@ int net_stream_data_client_connected(QIOTask *task, NetStreamData *d)
     addr = qio_channel_socket_get_remote_address(sioc, NULL);
     g_assert(addr != NULL);
 
-    ret = qemu_socket_try_set_nonblock(sioc->fd);
-    if (addr->type == SOCKET_ADDRESS_TYPE_FD && ret < 0) {
-        qemu_set_info_str(&d->nc, "can't use file descriptor %s (errno %d)",
-                          addr->u.fd.str, -ret);
+    if (!qemu_set_blocking(sioc->fd, false, &err)) {
+        qemu_set_info_str(&d->nc, "error: %s", error_get_pretty(err));
+        error_free(err);
         qapi_free_SocketAddress(addr);
         goto error;
     }
-    g_assert(ret == 0);
     qapi_free_SocketAddress(addr);
 
     net_socket_rs_init(&d->rs, net_stream_data_rs_finalize, false);
diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index dc23b33210..51f16d57ff 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -267,10 +267,6 @@ void qemu_socket_set_block(int fd)
     g_unix_set_fd_nonblocking(fd, false, NULL);
 }
 
-int qemu_socket_try_set_nonblock(int fd)
-{
-    return g_unix_set_fd_nonblocking(fd, true, NULL) ? 0 : -errno;
-}
 
 int socket_set_fast_reuse(int fd)
 {
diff --git a/util/oslib-win32.c b/util/oslib-win32.c
index 1566eb57e7..bf5d478c5c 100644
--- a/util/oslib-win32.c
+++ b/util/oslib-win32.c
@@ -202,15 +202,6 @@ void qemu_socket_set_block(int fd)
     ioctlsocket(fd, FIONBIO, &opt);
 }
 
-int qemu_socket_try_set_nonblock(int fd)
-{
-    unsigned long opt = 1;
-    if (ioctlsocket(fd, FIONBIO, &opt) != NO_ERROR) {
-        return -socket_error();
-    }
-    return 0;
-}
-
 int socket_set_fast_reuse(int fd)
 {
     /* Enabling the reuse of an endpoint that was used by a socket still in
-- 
2.48.1

Re: [PATCH 05/10] util: drop qemu_socket_try_set_nonblock()

[Peter Xu]

On Wed, Sep 03, 2025 at 12:44:05PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> Now we can use qemu_set_blocking() in these cases.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>

Reviewed-by: Peter Xu <peterx@redhat.com>

-- 
Peter Xu

[PATCH 06/10] util: drop qemu_socket_set_block()

[Vladimir Sementsov-Ogievskiy]

Now we can use qemu_set_blocking() and stop ignore errors in
these case.

Just crash on error for now, as we'll rework it in following commits.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 include/qemu/sockets.h | 1 -
 io/channel-socket.c    | 3 ++-
 util/oslib-posix.c     | 6 ------
 util/oslib-win32.c     | 7 -------
 4 files changed, 2 insertions(+), 15 deletions(-)

diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h
index 9512fec514..be351d85f7 100644
--- a/include/qemu/sockets.h
+++ b/include/qemu/sockets.h
@@ -46,7 +46,6 @@ ssize_t qemu_send_full(int s, const void *buf, size_t count)
     G_GNUC_WARN_UNUSED_RESULT;
 int socket_set_cork(int fd, int v);
 int socket_set_nodelay(int fd);
-void qemu_socket_set_block(int fd);
 int socket_set_fast_reuse(int fd);
 
 #ifdef WIN32
diff --git a/io/channel-socket.c b/io/channel-socket.c
index e77602a22e..4f7e86f72f 100644
--- a/io/channel-socket.c
+++ b/io/channel-socket.c
@@ -498,7 +498,8 @@ static void qio_channel_socket_copy_fds(struct msghdr *msg,
             }
 
             /* O_NONBLOCK is preserved across SCM_RIGHTS so reset it */
-            qemu_socket_set_block(fd);
+            /* TODO: don't crash on error, just handle it! */
+            qemu_set_blocking(fd, true, &error_abort);
 
 #ifndef MSG_CMSG_CLOEXEC
             qemu_set_cloexec(fd);
diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index 51f16d57ff..8891d82db0 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -262,12 +262,6 @@ bool qemu_set_blocking(int fd, bool block, Error **errp)
     return true;
 }
 
-void qemu_socket_set_block(int fd)
-{
-    g_unix_set_fd_nonblocking(fd, false, NULL);
-}
-
-
 int socket_set_fast_reuse(int fd)
 {
     int val = 1, ret;
diff --git a/util/oslib-win32.c b/util/oslib-win32.c
index bf5d478c5c..b9ce2f96ee 100644
--- a/util/oslib-win32.c
+++ b/util/oslib-win32.c
@@ -195,13 +195,6 @@ bool qemu_set_blocking(int fd, bool block, Error **errp)
     return true;
 }
 
-void qemu_socket_set_block(int fd)
-{
-    unsigned long opt = 0;
-    qemu_socket_unselect(fd, NULL);
-    ioctlsocket(fd, FIONBIO, &opt);
-}
-
 int socket_set_fast_reuse(int fd)
 {
     /* Enabling the reuse of an endpoint that was used by a socket still in
-- 
2.48.1

[PATCH 07/10] use qemu_set_blocking instead of g_unix_set_fd_nonblocking

[Vladimir Sementsov-Ogievskiy]

Instead of open-coded g_unix_set_fd_nonblocking() calls, use
QEMU wrapper qemu_socket_set_nonblock().

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@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/chardev/char-fd.c b/chardev/char-fd.c
index 6f03adf872..739dc68c36 100644
--- a/chardev/char-fd.c
+++ b/chardev/char-fd.c
@@ -212,8 +212,8 @@ void qemu_chr_open_fd(Chardev *chr,
     FDChardev *s = FD_CHARDEV(chr);
     g_autofree char *name = NULL;
 
-    if (fd_out >= 0 && !g_unix_set_fd_nonblocking(fd_out, true, NULL)) {
-        assert(!"Failed to set FD nonblocking");
+    if (fd_out >= 0) {
+        qemu_set_blocking(fd_out, false, &error_abort);
     }
 
     if (fd_out == fd_in && fd_in >= 0) {
diff --git a/chardev/char-pty.c b/chardev/char-pty.c
index 674e9b3f14..fe6bfb043d 100644
--- a/chardev/char-pty.c
+++ b/chardev/char-pty.c
@@ -349,8 +349,7 @@ static void char_pty_open(Chardev *chr,
     }
 
     close(slave_fd);
-    if (!g_unix_set_fd_nonblocking(master_fd, true, NULL)) {
-        error_setg_errno(errp, errno, "Failed to set FD nonblocking");
+    if (!qemu_set_blocking(master_fd, false, errp)) {
         return;
     }
 
diff --git a/chardev/char-serial.c b/chardev/char-serial.c
index 0a68b4b4e0..1ff31dcde3 100644
--- a/chardev/char-serial.c
+++ b/chardev/char-serial.c
@@ -271,8 +271,7 @@ static void qmp_chardev_open_serial(Chardev *chr,
     if (fd < 0) {
         return;
     }
-    if (!g_unix_set_fd_nonblocking(fd, true, NULL)) {
-        error_setg_errno(errp, errno, "Failed to set FD nonblocking");
+    if (!qemu_set_blocking(fd, false, errp)) {
         return;
     }
     tty_serial_init(fd, 115200, 'N', 8, 1);
diff --git a/chardev/char-stdio.c b/chardev/char-stdio.c
index 48db8d2f30..193727e807 100644
--- a/chardev/char-stdio.c
+++ b/chardev/char-stdio.c
@@ -107,8 +107,7 @@ static void qemu_chr_open_stdio(Chardev *chr,
     old_fd0_flags = fcntl(0, F_GETFL);
     old_fd1_flags = fcntl(1, F_GETFL);
     tcgetattr(0, &oldtty);
-    if (!g_unix_set_fd_nonblocking(0, true, NULL)) {
-        error_setg_errno(errp, errno, "Failed to set FD nonblocking");
+    if (!qemu_set_blocking(0, false, errp)) {
         return;
     }
     atexit(term_exit);
diff --git a/hw/input/virtio-input-host.c b/hw/input/virtio-input-host.c
index bbfee9d3b9..9f62532559 100644
--- a/hw/input/virtio-input-host.c
+++ b/hw/input/virtio-input-host.c
@@ -114,8 +114,7 @@ static void virtio_input_host_realize(DeviceState *dev, Error **errp)
         error_setg_file_open(errp, errno, vih->evdev);
         return;
     }
-    if (!g_unix_set_fd_nonblocking(vih->fd, true, NULL)) {
-        error_setg_errno(errp, errno, "Failed to set FD nonblocking");
+    if (!qemu_set_blocking(vih->fd, false, errp)) {
         goto err_close;
     }
 
diff --git a/hw/misc/ivshmem-flat.c b/hw/misc/ivshmem-flat.c
index fe4be6be17..29f298bbcb 100644
--- a/hw/misc/ivshmem-flat.c
+++ b/hw/misc/ivshmem-flat.c
@@ -12,6 +12,7 @@
 #include "qemu/units.h"
 #include "qemu/error-report.h"
 #include "qemu/module.h"
+#include "qemu/sockets.h"
 #include "qapi/error.h"
 #include "hw/irq.h"
 #include "hw/qdev-properties-system.h"
@@ -154,7 +155,8 @@ static void ivshmem_flat_add_vector(IvshmemFTState *s, IvshmemPeer *peer,
      * peer.
      */
     peer->vector[peer->vector_counter].id = peer->vector_counter;
-    g_unix_set_fd_nonblocking(vector_fd, true, NULL);
+    /* WARNING: qemu_socket_set_nonblock() return code ignored */
+    qemu_set_blocking(vector_fd, false, NULL);
     event_notifier_init_fd(&peer->vector[peer->vector_counter].event_notifier,
                            vector_fd);
 
diff --git a/hw/misc/ivshmem-pci.c b/hw/misc/ivshmem-pci.c
index d47ae739d6..b7da89fe3a 100644
--- a/hw/misc/ivshmem-pci.c
+++ b/hw/misc/ivshmem-pci.c
@@ -21,6 +21,7 @@
 #include "qemu/units.h"
 #include "qapi/error.h"
 #include "qemu/cutils.h"
+#include "qemu/sockets.h"
 #include "hw/pci/pci.h"
 #include "hw/qdev-properties.h"
 #include "hw/qdev-properties-system.h"
@@ -540,7 +541,12 @@ static void process_msg_connect(IVShmemState *s, uint16_t posn, int fd,
 
     IVSHMEM_DPRINTF("eventfds[%d][%d] = %d\n", posn, vector, fd);
     event_notifier_init_fd(&peer->eventfds[vector], fd);
-    g_unix_set_fd_nonblocking(fd, true, NULL); /* msix/irqfd poll non block */
+
+    /* msix/irqfd poll non block */
+    if (!qemu_set_blocking(fd, false, errp)) {
+        close(fd);
+        return;
+    }
 
     if (posn == s->vm_id) {
         setup_interrupt(s, vector, errp);
diff --git a/hw/virtio/vhost-vsock.c b/hw/virtio/vhost-vsock.c
index 6e4088831f..107d88babe 100644
--- a/hw/virtio/vhost-vsock.c
+++ b/hw/virtio/vhost-vsock.c
@@ -147,9 +147,7 @@ static void vhost_vsock_device_realize(DeviceState *dev, Error **errp)
             return;
         }
 
-        if (!g_unix_set_fd_nonblocking(vhostfd, true, NULL)) {
-            error_setg_errno(errp, errno,
-                             "vhost-vsock: unable to set non-blocking mode");
+        if (!qemu_set_blocking(vhostfd, false, errp)) {
             return;
         }
     } else {
@@ -160,9 +158,7 @@ static void vhost_vsock_device_realize(DeviceState *dev, Error **errp)
             return;
         }
 
-        if (!g_unix_set_fd_nonblocking(vhostfd, true, NULL)) {
-            error_setg_errno(errp, errno,
-                             "Failed to set FD nonblocking");
+        if (!qemu_set_blocking(vhostfd, false, errp)) {
             return;
         }
     }
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;
diff --git a/net/tap-bsd.c b/net/tap-bsd.c
index b4c84441ba..2e444e59b5 100644
--- a/net/tap-bsd.c
+++ b/net/tap-bsd.c
@@ -98,7 +98,12 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
             return -1;
         }
     }
-    g_unix_set_fd_nonblocking(fd, true, NULL);
+
+    if (!qemu_set_blocking(fd, false, errp) {
+        close(fd);
+        return -1;
+    }
+
     return fd;
 }
 
@@ -189,7 +194,10 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
         goto error;
     }
 
-    g_unix_set_fd_nonblocking(fd, true, NULL);
+    if (!qemu_set_blocking(fd, false, errp) {
+        goto error;
+    }
+
     return fd;
 
 error:
diff --git a/net/tap-linux.c b/net/tap-linux.c
index 22ec2f45d2..786f339c13 100644
--- a/net/tap-linux.c
+++ b/net/tap-linux.c
@@ -34,6 +34,7 @@
 #include "qapi/error.h"
 #include "qemu/error-report.h"
 #include "qemu/cutils.h"
+#include "qemu/sockets.h"
 
 #define PATH_NET_TUN "/dev/net/tun"
 
@@ -124,7 +125,12 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
         return -1;
     }
     pstrcpy(ifname, ifname_size, ifr.ifr_name);
-    g_unix_set_fd_nonblocking(fd, true, NULL);
+
+    if (!qemu_set_blocking(fd, false, NULL)) {
+        close(fd);
+        return -1;
+    }
+
     return fd;
 }
 
diff --git a/net/tap-solaris.c b/net/tap-solaris.c
index 51b7830bef..02709590c1 100644
--- a/net/tap-solaris.c
+++ b/net/tap-solaris.c
@@ -198,7 +198,12 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
             return -1;
         }
     }
-    g_unix_set_fd_nonblocking(fd, true, NULL);
+
+    if (!qemu_set_blocking(fd, false NULL)) {
+        close(fd);
+        return -1;
+    }
+
     return fd;
 }
 
diff --git a/net/tap.c b/net/tap.c
index f7df702f97..f37133e301 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -627,8 +627,7 @@ int net_init_bridge(const Netdev *netdev, const char *name,
         return -1;
     }
 
-    if (!g_unix_set_fd_nonblocking(fd, true, NULL)) {
-        error_setg_errno(errp, errno, "Failed to set FD nonblocking");
+    if (!qemu_set_blocking(fd, false, errp)) {
         return -1;
     }
     vnet_hdr = tap_probe_vnet_hdr(fd, errp);
@@ -729,9 +728,7 @@ static void net_init_tap_one(const NetdevTapOptions *tap, NetClientState *peer,
                 error_propagate(errp, err);
                 goto failed;
             }
-            if (!g_unix_set_fd_nonblocking(vhostfd, true, NULL)) {
-                error_setg_errno(errp, errno, "%s: Can't use file descriptor %d",
-                                 name, fd);
+            if (!qemu_set_blocking(vhostfd, false, errp)) {
                 goto failed;
             }
         } else {
@@ -741,8 +738,7 @@ static void net_init_tap_one(const NetdevTapOptions *tap, NetClientState *peer,
                                  "tap: open vhost char device failed");
                 goto failed;
             }
-            if (!g_unix_set_fd_nonblocking(vhostfd, true, NULL)) {
-                error_setg_errno(errp, errno, "Failed to set FD nonblocking");
+            if (!qemu_set_blocking(vhostfd, false, errp)) {
                 goto failed;
             }
         }
@@ -839,9 +835,7 @@ int net_init_tap(const Netdev *netdev, const char *name,
             return -1;
         }
 
-        if (!g_unix_set_fd_nonblocking(fd, true, NULL)) {
-            error_setg_errno(errp, errno, "%s: Can't use file descriptor %d",
-                             name, fd);
+        if (!qemu_set_blocking(fd, false, errp)) {
             close(fd);
             return -1;
         }
@@ -895,10 +889,8 @@ int net_init_tap(const Netdev *netdev, const char *name,
                 goto free_fail;
             }
 
-            if (!g_unix_set_fd_nonblocking(fd, true, NULL)) {
+            if (!qemu_set_blocking(fd, false, errp)) {
                 ret = -1;
-                error_setg_errno(errp, errno, "%s: Can't use file descriptor %d",
-                                 name, fd);
                 goto free_fail;
             }
 
@@ -951,8 +943,7 @@ free_fail:
             return -1;
         }
 
-        if (!g_unix_set_fd_nonblocking(fd, true, NULL)) {
-            error_setg_errno(errp, errno, "Failed to set FD nonblocking");
+        if (!qemu_set_blocking(fd, false, errp)) {
             return -1;
         }
         vnet_hdr = tap_probe_vnet_hdr(fd, errp);
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 12bc086d79..5070f27d75 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -503,9 +503,8 @@ int64_t qmp_guest_file_open(const char *path, const char *mode,
     /* set fd non-blocking to avoid common use cases (like reading from a
      * named pipe) from hanging the agent
      */
-    if (!g_unix_set_fd_nonblocking(fileno(fh), true, NULL)) {
+    if (!qemu_set_blocking(fileno(fh), false, errp)) {
         fclose(fh);
-        error_setg_errno(errp, errno, "Failed to set FD nonblocking");
         return -1;
     }
 
diff --git a/tests/qtest/fuzz/virtio_net_fuzz.c b/tests/qtest/fuzz/virtio_net_fuzz.c
index e239875e3b..e9b13d3e4f 100644
--- a/tests/qtest/fuzz/virtio_net_fuzz.c
+++ b/tests/qtest/fuzz/virtio_net_fuzz.c
@@ -132,7 +132,7 @@ static void *virtio_net_test_setup_socket(GString *cmd_line, void *arg)
 {
     int ret = socketpair(PF_UNIX, SOCK_STREAM, 0, sockfds);
     g_assert_cmpint(ret, !=, -1);
-    g_unix_set_fd_nonblocking(sockfds[0], true, NULL);
+    qemu_set_blocking(sockfds[0], false, &error_abort);
     sockfds_initialized = true;
     g_string_append_printf(cmd_line, " -netdev socket,fd=%d,id=hs0 ",
                            sockfds[1]);
diff --git a/tests/qtest/vhost-user-test.c b/tests/qtest/vhost-user-test.c
index 75cb3e44b2..ba70bf997c 100644
--- a/tests/qtest/vhost-user-test.c
+++ b/tests/qtest/vhost-user-test.c
@@ -472,8 +472,7 @@ static void chr_read(void *opaque, const uint8_t *buf, int size)
          * The receive function forces it to be blocking,
          * so revert it back to non-blocking.
          */
-        g_unix_set_fd_nonblocking(fd, true, &err);
-        g_assert_no_error(err);
+        qemu_set_blocking(fd, false, &error_abort);
         break;
 
     case VHOST_USER_SET_LOG_BASE:
diff --git a/tests/unit/test-iov.c b/tests/unit/test-iov.c
index 75bc3be005..63e2b1583c 100644
--- a/tests/unit/test-iov.c
+++ b/tests/unit/test-iov.c
@@ -1,4 +1,5 @@
 #include "qemu/osdep.h"
+#include "qapi/error.h"
 #include "qemu/iov.h"
 #include "qemu/sockets.h"
 
@@ -186,7 +187,7 @@ static void test_io(void)
 
        close(sv[0]);
        FD_SET(sv[1], &fds);
-       g_unix_set_fd_nonblocking(sv[1], true, NULL);
+       qemu_set_blocking(sv[1], false, &error_abort);
        r = g_test_rand_int_range(sz / 2, sz);
        setsockopt(sv[1], SOL_SOCKET, SO_SNDBUF, &r, sizeof(r));
 
@@ -222,7 +223,7 @@ static void test_io(void)
 
        close(sv[1]);
        FD_SET(sv[0], &fds);
-       g_unix_set_fd_nonblocking(sv[0], true, NULL);
+       qemu_set_blocking(sv[0], false, &error_abort);
        r = g_test_rand_int_range(sz / 2, sz);
        setsockopt(sv[0], SOL_SOCKET, SO_RCVBUF, &r, sizeof(r));
        usleep(500000);
diff --git a/ui/input-linux.c b/ui/input-linux.c
index 92e1a1aa64..44d0c15a9b 100644
--- a/ui/input-linux.c
+++ b/ui/input-linux.c
@@ -316,8 +316,7 @@ static void input_linux_complete(UserCreatable *uc, Error **errp)
         error_setg_file_open(errp, errno, il->evdev);
         return;
     }
-    if (!g_unix_set_fd_nonblocking(il->fd, true, NULL)) {
-        error_setg_errno(errp, errno, "Failed to set FD nonblocking");
+    if (!qemu_set_blocking(il->fd, false, errp)) {
         return;
     }
 
diff --git a/util/event_notifier-posix.c b/util/event_notifier-posix.c
index 76420c5b56..8c05e0d528 100644
--- a/util/event_notifier-posix.c
+++ b/util/event_notifier-posix.c
@@ -14,6 +14,7 @@
 #include "qemu/cutils.h"
 #include "qemu/event_notifier.h"
 #include "qemu/main-loop.h"
+#include "qemu/sockets.h"
 
 #ifdef CONFIG_EVENTFD
 #include <sys/eventfd.h>
@@ -52,11 +53,11 @@ int event_notifier_init(EventNotifier *e, int active)
         if (!g_unix_open_pipe(fds, FD_CLOEXEC, NULL)) {
             return -errno;
         }
-        if (!g_unix_set_fd_nonblocking(fds[0], true, NULL)) {
+        if (!qemu_set_blocking(fds[0], false, NULL)) {
             ret = -errno;
             goto fail;
         }
-        if (!g_unix_set_fd_nonblocking(fds[1], true, NULL)) {
+        if (!qemu_set_blocking(fds[1], false, NULL)) {
             ret = -errno;
             goto fail;
         }
diff --git a/util/main-loop.c b/util/main-loop.c
index 51aeb2432e..6885be688c 100644
--- a/util/main-loop.c
+++ b/util/main-loop.c
@@ -34,6 +34,7 @@
 #include "block/thread-pool.h"
 #include "qemu/error-report.h"
 #include "qemu/queue.h"
+#include "qemu/sockets.h"
 #include "qom/object.h"
 
 #ifndef _WIN32
@@ -114,7 +115,10 @@ static int qemu_signal_init(Error **errp)
         return -errno;
     }
 
-    g_unix_set_fd_nonblocking(sigfd, true, NULL);
+    if (!qemu_set_blocking(sigfd, false, errp)) {
+        close(sigfd);
+        return -EINVAL;
+    }
 
     qemu_set_fd_handler(sigfd, sigfd_handler, NULL, (void *)(intptr_t)sigfd);
 
-- 
2.48.1

Re: [PATCH 07/10] use qemu_set_blocking instead of g_unix_set_fd_nonblocking

[Daniel P. Berrangé]

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 <vsementsov@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.


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 :|

Re: [PATCH 07/10] use qemu_set_blocking instead of g_unix_set_fd_nonblocking

[Vladimir Sementsov-Ogievskiy]

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 <vsementsov@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.

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..

I don't know. For me it seems better to have one qemu_set_blocking, and rely on Windows realization to
correctly error-out for non-socket fds.

Do you have and idea, how final API should look, covering sockets and non-socket fds?

I also found ff5927baa7ffb9c978 commit about it, see my last answer to the cover-letter.

-- 
Best regards,
Vladimir

Re: [PATCH 07/10] use qemu_set_blocking instead of g_unix_set_fd_nonblocking

[Daniel P. Berrangé]

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 <vsementsov@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 :|

Re: [PATCH 07/10] use qemu_set_blocking instead of g_unix_set_fd_nonblocking

[Vladimir Sementsov-Ogievskiy]

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 <vsementsov@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

Re: [PATCH 07/10] use qemu_set_blocking instead of g_unix_set_fd_nonblocking

[Daniel P. Berrangé]

On Tue, Sep 09, 2025 at 12:50:00PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> 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 <vsementsov@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!!)

I looked at what we do in libvirt, and there we just have a single
function virSetBlocking() that simply does ioctrlsocket() on Windows.
I can't say we've had any significant trouble with that, though our
Windows scope & usage is much less than QEMU's.

A key difference though is that callers are expected to check the
return value for failure.


With the current QEMU design where we do NOT expect callers to check
for failure, I think it is important that the function naming make
clear the intended use case, as we won't get runtime diagnosis of
incorrect calls.

So with this in mind, I'm actually OK with your proposed design for
a general  qemu_set_blocking, as that has an Error **errp parameter
encouraging failure checking by callers.

The ioctlsocket method will also return a clear error if passed
something that is not a SOCKET, but a generic file backed HANDLE.

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 :|

Re: [PATCH 07/10] use qemu_set_blocking instead of g_unix_set_fd_nonblocking

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 266 bytes --]

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().

The commit description is outdated: the patch actually uses
qemu_set_blocking().

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH 08/10] oslib-posix: add qemu_fds_set_blocking() helper

[Vladimir Sementsov-Ogievskiy]

And use it in io/channel-socket.c. This simplifies the following
commit, which will move this functionality from io/channel-socket.c
to the callers.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 include/qemu/osdep.h |  7 +++++++
 io/channel-socket.c  | 24 +++++++++++++-----------
 util/oslib-posix.c   | 12 ++++++++++++
 3 files changed, 32 insertions(+), 11 deletions(-)

diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index 1b38cb7e45..dde98d588c 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -689,6 +689,13 @@ ssize_t qemu_write_full(int fd, const void *buf, size_t count)
 void qemu_set_cloexec(int fd);
 bool qemu_set_blocking(int fd, bool block, Error **errp);
 
+/*
+ * qemu_fds_set_blockinging:
+ * Call qemu_socket_set_block() on several fds.
+ * When @nfds = 0, does nothing, @fds is not touched.
+ */
+bool qemu_fds_set_blockinging(int *fds, int nfds, bool block, Error **errp);
+
 /* Return a dynamically allocated directory path that is appropriate for storing
  * local state.
  *
diff --git a/io/channel-socket.c b/io/channel-socket.c
index 4f7e86f72f..96098b5bcc 100644
--- a/io/channel-socket.c
+++ b/io/channel-socket.c
@@ -472,8 +472,11 @@ static void qio_channel_socket_copy_fds(struct msghdr *msg,
     *fds = NULL;
 
     for (cmsg = CMSG_FIRSTHDR(msg); cmsg; cmsg = CMSG_NXTHDR(msg, cmsg)) {
-        int fd_size, i;
+        int fd_size;
         int gotfds;
+#ifndef MSG_CMSG_CLOEXEC
+        int i;
+#endif
 
         if (cmsg->cmsg_len < CMSG_LEN(sizeof(int)) ||
             cmsg->cmsg_level != SOL_SOCKET ||
@@ -491,20 +494,19 @@ static void qio_channel_socket_copy_fds(struct msghdr *msg,
         *fds = g_renew(int, *fds, *nfds + gotfds);
         memcpy(*fds + *nfds, CMSG_DATA(cmsg), fd_size);
 
+        /* O_NONBLOCK is preserved across SCM_RIGHTS so reset it */
+        /* TODO: don't crash on error, just handle it! */
+        qemu_fds_set_blockinging(*fds + *nfds, gotfds, true, &error_abort);
+
+#ifndef MSG_CMSG_CLOEXEC
         for (i = 0; i < gotfds; i++) {
             int fd = (*fds)[*nfds + i];
-            if (fd < 0) {
-                continue;
+            if (fd >= 0) {
+                qemu_set_cloexec(fd);
             }
-
-            /* O_NONBLOCK is preserved across SCM_RIGHTS so reset it */
-            /* TODO: don't crash on error, just handle it! */
-            qemu_set_blocking(fd, true, &error_abort);
-
-#ifndef MSG_CMSG_CLOEXEC
-            qemu_set_cloexec(fd);
-#endif
         }
+#endif
+
         *nfds += gotfds;
     }
 }
diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index 8891d82db0..8589ff21ec 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -262,6 +262,18 @@ bool qemu_set_blocking(int fd, bool block, Error **errp)
     return true;
 }
 
+bool qemu_fds_set_blockinging(int *fds, int nfds, bool block, Error **errp)
+{
+    int i;
+    for (i = 0; i < nfds; i++) {
+        if (fds[i] >= 0 && !qemu_set_blocking(fds[i], block, errp)) {
+            return false;
+        }
+    }
+
+    return true;
+}
+
 int socket_set_fast_reuse(int fd)
 {
     int val = 1, ret;
-- 
2.48.1

Re: [PATCH 08/10] oslib-posix: add qemu_fds_set_blocking() helper

[Vladimir Sementsov-Ogievskiy]

On 03.09.25 12:44, Vladimir Sementsov-Ogievskiy wrote:
> And use it in io/channel-socket.c. This simplifies the following
> commit, which will move this functionality from io/channel-socket.c
> to the callers.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> ---
>   include/qemu/osdep.h |  7 +++++++
>   io/channel-socket.c  | 24 +++++++++++++-----------
>   util/oslib-posix.c   | 12 ++++++++++++
>   3 files changed, 32 insertions(+), 11 deletions(-)
> 
> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> index 1b38cb7e45..dde98d588c 100644
> --- a/include/qemu/osdep.h
> +++ b/include/qemu/osdep.h
> @@ -689,6 +689,13 @@ ssize_t qemu_write_full(int fd, const void *buf, size_t count)
>   void qemu_set_cloexec(int fd);
>   bool qemu_set_blocking(int fd, bool block, Error **errp);
>   
> +/*
> + * qemu_fds_set_blockinging:
> + * Call qemu_socket_set_block() on several fds.
> + * When @nfds = 0, does nothing, @fds is not touched.
> + */
> +bool qemu_fds_set_blockinging(int *fds, int nfds, bool block, Error **errp);

Oops. s/inging/ing/ is needed.



-- 
Best regards,
Vladimir

Re: [PATCH 08/10] oslib-posix: add qemu_fds_set_blocking() helper

[Daniel P. Berrangé]

On Wed, Sep 03, 2025 at 12:44:08PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> And use it in io/channel-socket.c. This simplifies the following
> commit, which will move this functionality from io/channel-socket.c
> to the callers.

Looking at how many callers will still want the current
functionality, I'm inclined to add a flag for QIOChannel

  QIO_CHANNEL_READ_FLAG_PRESERVE_BLOCKING

which migration can pass to qio_channel_readv_full and
thus avoid adding this method and duplicated calls to
it in (almost) every caller.

> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> ---
>  include/qemu/osdep.h |  7 +++++++
>  io/channel-socket.c  | 24 +++++++++++++-----------
>  util/oslib-posix.c   | 12 ++++++++++++
>  3 files changed, 32 insertions(+), 11 deletions(-)
> 
> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> index 1b38cb7e45..dde98d588c 100644
> --- a/include/qemu/osdep.h
> +++ b/include/qemu/osdep.h
> @@ -689,6 +689,13 @@ ssize_t qemu_write_full(int fd, const void *buf, size_t count)
>  void qemu_set_cloexec(int fd);
>  bool qemu_set_blocking(int fd, bool block, Error **errp);
>  
> +/*
> + * qemu_fds_set_blockinging:
> + * Call qemu_socket_set_block() on several fds.
> + * When @nfds = 0, does nothing, @fds is not touched.
> + */
> +bool qemu_fds_set_blockinging(int *fds, int nfds, bool block, Error **errp);
> +
>  /* Return a dynamically allocated directory path that is appropriate for storing
>   * local state.
>   *
> diff --git a/io/channel-socket.c b/io/channel-socket.c
> index 4f7e86f72f..96098b5bcc 100644
> --- a/io/channel-socket.c
> +++ b/io/channel-socket.c
> @@ -472,8 +472,11 @@ static void qio_channel_socket_copy_fds(struct msghdr *msg,
>      *fds = NULL;
>  
>      for (cmsg = CMSG_FIRSTHDR(msg); cmsg; cmsg = CMSG_NXTHDR(msg, cmsg)) {
> -        int fd_size, i;
> +        int fd_size;
>          int gotfds;
> +#ifndef MSG_CMSG_CLOEXEC
> +        int i;
> +#endif
>  
>          if (cmsg->cmsg_len < CMSG_LEN(sizeof(int)) ||
>              cmsg->cmsg_level != SOL_SOCKET ||
> @@ -491,20 +494,19 @@ static void qio_channel_socket_copy_fds(struct msghdr *msg,
>          *fds = g_renew(int, *fds, *nfds + gotfds);
>          memcpy(*fds + *nfds, CMSG_DATA(cmsg), fd_size);
>  
> +        /* O_NONBLOCK is preserved across SCM_RIGHTS so reset it */
> +        /* TODO: don't crash on error, just handle it! */
> +        qemu_fds_set_blockinging(*fds + *nfds, gotfds, true, &error_abort);
> +
> +#ifndef MSG_CMSG_CLOEXEC
>          for (i = 0; i < gotfds; i++) {
>              int fd = (*fds)[*nfds + i];
> -            if (fd < 0) {
> -                continue;
> +            if (fd >= 0) {
> +                qemu_set_cloexec(fd);
>              }
> -
> -            /* O_NONBLOCK is preserved across SCM_RIGHTS so reset it */
> -            /* TODO: don't crash on error, just handle it! */
> -            qemu_set_blocking(fd, true, &error_abort);
> -
> -#ifndef MSG_CMSG_CLOEXEC
> -            qemu_set_cloexec(fd);
> -#endif
>          }
> +#endif
> +
>          *nfds += gotfds;
>      }
>  }
> diff --git a/util/oslib-posix.c b/util/oslib-posix.c
> index 8891d82db0..8589ff21ec 100644
> --- a/util/oslib-posix.c
> +++ b/util/oslib-posix.c
> @@ -262,6 +262,18 @@ bool qemu_set_blocking(int fd, bool block, Error **errp)
>      return true;
>  }
>  
> +bool qemu_fds_set_blockinging(int *fds, int nfds, bool block, Error **errp)
> +{
> +    int i;
> +    for (i = 0; i < nfds; i++) {
> +        if (fds[i] >= 0 && !qemu_set_blocking(fds[i], block, errp)) {
> +            return false;
> +        }
> +    }
> +
> +    return true;
> +}
> +
>  int socket_set_fast_reuse(int fd)
>  {
>      int val = 1, ret;
> -- 
> 2.48.1
> 

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 :|

Re: [PATCH 08/10] oslib-posix: add qemu_fds_set_blocking() helper

[Vladimir Sementsov-Ogievskiy]

On 10.09.25 12:52, Daniel P. Berrangé wrote:
> On Wed, Sep 03, 2025 at 12:44:08PM +0300, Vladimir Sementsov-Ogievskiy wrote:
>> And use it in io/channel-socket.c. This simplifies the following
>> commit, which will move this functionality from io/channel-socket.c
>> to the callers.
> 
> Looking at how many callers will still want the current
> functionality, I'm inclined to add a flag for QIOChannel
> 
>    QIO_CHANNEL_READ_FLAG_PRESERVE_BLOCKING
> 
> which migration can pass to qio_channel_readv_full and
> thus avoid adding this method and duplicated calls to
> it in (almost) every caller.
> 

Yes, I thought about this too, when preparing the series.

I'll make a patch, and make the fds-set-blocking refactoring series to be the separate one.

>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
>> ---
>>   include/qemu/osdep.h |  7 +++++++
>>   io/channel-socket.c  | 24 +++++++++++++-----------
>>   util/oslib-posix.c   | 12 ++++++++++++
>>   3 files changed, 32 insertions(+), 11 deletions(-)
>>
>> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
>> index 1b38cb7e45..dde98d588c 100644
>> --- a/include/qemu/osdep.h
>> +++ b/include/qemu/osdep.h
>> @@ -689,6 +689,13 @@ ssize_t qemu_write_full(int fd, const void *buf, size_t count)
>>   void qemu_set_cloexec(int fd);
>>   bool qemu_set_blocking(int fd, bool block, Error **errp);
>>   
>> +/*
>> + * qemu_fds_set_blockinging:
>> + * Call qemu_socket_set_block() on several fds.
>> + * When @nfds = 0, does nothing, @fds is not touched.
>> + */
>> +bool qemu_fds_set_blockinging(int *fds, int nfds, bool block, Error **errp);
>> +
>>   /* Return a dynamically allocated directory path that is appropriate for storing
>>    * local state.
>>    *
>> diff --git a/io/channel-socket.c b/io/channel-socket.c
>> index 4f7e86f72f..96098b5bcc 100644
>> --- a/io/channel-socket.c
>> +++ b/io/channel-socket.c
>> @@ -472,8 +472,11 @@ static void qio_channel_socket_copy_fds(struct msghdr *msg,
>>       *fds = NULL;
>>   
>>       for (cmsg = CMSG_FIRSTHDR(msg); cmsg; cmsg = CMSG_NXTHDR(msg, cmsg)) {
>> -        int fd_size, i;
>> +        int fd_size;
>>           int gotfds;
>> +#ifndef MSG_CMSG_CLOEXEC
>> +        int i;
>> +#endif
>>   
>>           if (cmsg->cmsg_len < CMSG_LEN(sizeof(int)) ||
>>               cmsg->cmsg_level != SOL_SOCKET ||
>> @@ -491,20 +494,19 @@ static void qio_channel_socket_copy_fds(struct msghdr *msg,
>>           *fds = g_renew(int, *fds, *nfds + gotfds);
>>           memcpy(*fds + *nfds, CMSG_DATA(cmsg), fd_size);
>>   
>> +        /* O_NONBLOCK is preserved across SCM_RIGHTS so reset it */
>> +        /* TODO: don't crash on error, just handle it! */
>> +        qemu_fds_set_blockinging(*fds + *nfds, gotfds, true, &error_abort);
>> +
>> +#ifndef MSG_CMSG_CLOEXEC
>>           for (i = 0; i < gotfds; i++) {
>>               int fd = (*fds)[*nfds + i];
>> -            if (fd < 0) {
>> -                continue;
>> +            if (fd >= 0) {
>> +                qemu_set_cloexec(fd);
>>               }
>> -
>> -            /* O_NONBLOCK is preserved across SCM_RIGHTS so reset it */
>> -            /* TODO: don't crash on error, just handle it! */
>> -            qemu_set_blocking(fd, true, &error_abort);
>> -
>> -#ifndef MSG_CMSG_CLOEXEC
>> -            qemu_set_cloexec(fd);
>> -#endif
>>           }
>> +#endif
>> +
>>           *nfds += gotfds;
>>       }
>>   }
>> diff --git a/util/oslib-posix.c b/util/oslib-posix.c
>> index 8891d82db0..8589ff21ec 100644
>> --- a/util/oslib-posix.c
>> +++ b/util/oslib-posix.c
>> @@ -262,6 +262,18 @@ bool qemu_set_blocking(int fd, bool block, Error **errp)
>>       return true;
>>   }
>>   
>> +bool qemu_fds_set_blockinging(int *fds, int nfds, bool block, Error **errp)
>> +{
>> +    int i;
>> +    for (i = 0; i < nfds; i++) {
>> +        if (fds[i] >= 0 && !qemu_set_blocking(fds[i], block, errp)) {
>> +            return false;
>> +        }
>> +    }
>> +
>> +    return true;
>> +}
>> +
>>   int socket_set_fast_reuse(int fd)
>>   {
>>       int val = 1, ret;
>> -- 
>> 2.48.1
>>
> 
> With regards,
> Daniel


-- 
Best regards,
Vladimir

[PATCH 09/10] qio_channel_readv_full(): move setting fd blocking to callers

[Vladimir Sementsov-Ogievskiy]

In future we'll need a way to pass non-blocking sockets
through migration channel (backed by unix-socket for local
migration). In this case setting fd blocking in
qio_channel_readv_full() and than setting it non-blocking
again in migration state loading code is not quit good:

1. For CPR scenario it's just wrong, because we pass fds when
source is still running. Making fd blocking when it is still
being used on source will break things. [Still, most probably
we don't no have CPR-supporting states which operates with
non-blocking sockets]

2. It's just ineffective to call the ioctl twice for nothing.

So, we'll need a way to avoid call to qemu_socket_set_block()
in qio_channel_readv_full().

Still let's go further, and simply keep in qio_channel_readv_full()
blocking status of fd as is, and let's the caller to care about it.
It's good at least for symmetry of the API: qemu_channel_writev_full()
doesn't modify the fd, let's qio_channel_readv_full() not doing so
too.

So, this commit moves qemu_socket_set_block() calls from
qio_channel_readv_full() to callers, mostly with help of
qemu_fds_set_blocking() function.

Let's look through all the users of qio_channel_readv_full():

1. Some callers just pass NULLs as fds / nfds, nothing to do here:

- qio_channel_readv()
- qio_channel_read()
- migration_channel_read_peek()

2. Some final users of the API, to be updated:

Add call to qemu_fds_set_blocking():
- qemu_fill_buffer() in migration/qemu-file.c
- test_io_channel_unix_fd_pass() in test-io-channel-socket.c
- vu_message_read() in vhost-user-server.c
- tcp_chr_recv() in chardev/char-socket.c
- vfio_user_recv_one() in vfio-user/proxy.c

Uses only one fd, so add call to qemu_socket_set_block():
- prh_read() in qemu-pr-helper.c
- tpm_emu_ctrl_thread() in qtest/tpm-emu.c

3. An API wrapper, which has own users:

qio_channel_readv_full_all_eof(), called from:
  - multifd_recv_thread() (migration/multifd.c)- with NULLs
  - qio_channel_readv_full_all() - wrapper, called from:
    - qio_channel_readv_all() - wrapper, with NULLs
    - backend_read() (virtio/vhost-user.c): use only one fd,
	  add call to qemu_socket_set_block()
  - qio_channel_readv_all_eof() - wrapper, with NULLs
  - mpqemu_read(), static (in hw/remote/mpqemu-link.c), called from:
    - mpqemu_msg_recv(): add call to qemu_fds_set_blocking()

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 chardev/char-socket.c               |  8 ++++++++
 hw/remote/mpqemu-link.c             |  3 +++
 hw/vfio-user/proxy.c                |  4 ++++
 hw/virtio/vhost-user.c              |  5 +++++
 include/io/channel.h                | 12 +++++++-----
 io/channel-socket.c                 |  4 ----
 migration/qemu-file.c               |  6 ++++++
 scsi/qemu-pr-helper.c               |  4 ++++
 tests/qtest/tpm-emu.c               |  1 +
 tests/unit/test-io-channel-socket.c |  1 +
 util/vhost-user-server.c            |  5 +++++
 11 files changed, 44 insertions(+), 9 deletions(-)

diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index 5b9b19ba8b..89d199b426 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -305,6 +305,14 @@ static ssize_t tcp_chr_recv(Chardev *chr, char *buf, size_t len)
 
     assert(ret >= 0);
 
+    if (!qemu_fds_set_blockinging(msgfds, msgfds_num, true, NULL)) {
+        for (i = 0; i < msgfds_num; i++) {
+            close(msgfds[i]);
+        }
+        errno = EINVAL;
+        return -1;
+    }
+
     if (msgfds_num) {
         /* close and clean read_msgfds */
         for (i = 0; i < s->read_msgfds_num; i++) {
diff --git a/hw/remote/mpqemu-link.c b/hw/remote/mpqemu-link.c
index 49885a1db6..d5716ff92f 100644
--- a/hw/remote/mpqemu-link.c
+++ b/hw/remote/mpqemu-link.c
@@ -162,6 +162,9 @@ copy_fds:
         goto fail;
     }
     if (nfds) {
+        if (!qemu_fds_set_blockinging(fds, nfds, true, errp)) {
+            goto fail;
+        }
         memcpy(msg->fds, fds, nfds * sizeof(int));
     }
 
diff --git a/hw/vfio-user/proxy.c b/hw/vfio-user/proxy.c
index 2275d3fe39..55efa7fd77 100644
--- a/hw/vfio-user/proxy.c
+++ b/hw/vfio-user/proxy.c
@@ -300,6 +300,10 @@ static int vfio_user_recv_one(VFIOUserProxy *proxy, Error **errp)
     trace_vfio_user_recv_hdr(proxy->sockname, hdr.id, hdr.command, hdr.size,
                              hdr.flags);
 
+    if (!qemu_fds_set_blockinging(fdp, numfds, true, errp)) {
+        goto err;
+    }
+
     /*
      * For replies, find the matching pending request.
      * For requests, reap incoming FDs.
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 36c9c2e04d..badd9d7851 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -1801,6 +1801,11 @@ static gboolean backend_read(QIOChannel *ioc, GIOCondition condition,
         goto err;
     }
 
+    if (fd && !qemu_set_blocking(fd[0], true, &local_err)) {
+        error_report_err(local_err);
+        goto err;
+    }
+
     if (hdr.size > VHOST_USER_PAYLOAD_SIZE) {
         error_report("Failed to read msg header."
                 " Size %d exceeds the maximum %zu.", hdr.size,
diff --git a/include/io/channel.h b/include/io/channel.h
index b848d50b99..40823c1728 100644
--- a/include/io/channel.h
+++ b/include/io/channel.h
@@ -119,9 +119,10 @@ struct QIOChannelClass {
                          Error **errp);
 
     /*
-     * The io_readv handler must guarantee that all
-     * incoming fds are set BLOCKING and CLOEXEC (if
-     * available).
+     * The io_readv handler must set all incoming fds
+     * CLOEXEC, and must NOT modify fds in any other
+     * way (for example, must not change its BLOCKING
+     * status)
      */
     ssize_t (*io_readv)(QIOChannel *ioc,
                         const struct iovec *iov,
@@ -242,8 +243,9 @@ void qio_channel_set_name(QIOChannel *ioc,
  * to call close() on each file descriptor and to
  * call g_free() on the array pointer in @fds.
  * qio_channel_readv_full() guarantees that all
- * incoming fds are set BLOCKING and CLOEXEC (if
- * available).
+ * incoming fds are set CLOEXEC (if available).
+ * qio_channel_readv_full() doesn't modify BLOCKING
+ * state of fds.
  *
  * It is an error to pass a non-NULL @fds parameter
  * unless qio_channel_has_feature() returns a true
diff --git a/io/channel-socket.c b/io/channel-socket.c
index 96098b5bcc..b87a1f3e38 100644
--- a/io/channel-socket.c
+++ b/io/channel-socket.c
@@ -494,10 +494,6 @@ static void qio_channel_socket_copy_fds(struct msghdr *msg,
         *fds = g_renew(int, *fds, *nfds + gotfds);
         memcpy(*fds + *nfds, CMSG_DATA(cmsg), fd_size);
 
-        /* O_NONBLOCK is preserved across SCM_RIGHTS so reset it */
-        /* TODO: don't crash on error, just handle it! */
-        qemu_fds_set_blockinging(*fds + *nfds, gotfds, true, &error_abort);
-
 #ifndef MSG_CMSG_CLOEXEC
         for (i = 0; i < gotfds; i++) {
             int fd = (*fds)[*nfds + i];
diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index b6ac190034..b1d042e298 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -361,6 +361,12 @@ static ssize_t coroutine_mixed_fn qemu_fill_buffer(QEMUFile *f)
         qemu_file_set_error_obj(f, len, local_error);
     }
 
+    /*
+     * NOTE: don't worry about error_abort, it will be removed
+     * in the next commit
+     */
+    qemu_fds_set_blockinging(fds, nfd, true, &error_abort);
+
     for (int i = 0; i < nfd; i++) {
         FdEntry *fde = g_new0(FdEntry, 1);
         fde->fd = fds[i];
diff --git a/scsi/qemu-pr-helper.c b/scsi/qemu-pr-helper.c
index b69dd982d6..8ca817f187 100644
--- a/scsi/qemu-pr-helper.c
+++ b/scsi/qemu-pr-helper.c
@@ -620,6 +620,10 @@ static int coroutine_fn prh_read(PRHelperClient *client, void *buf, int sz,
             goto err;
         }
 
+        if (!qemu_fds_set_blockinging(fds, nfds, true, errp)) {
+            goto err;
+        }
+
         /* Stash one file descriptor per request.  */
         if (nfds) {
             bool too_many = false;
diff --git a/tests/qtest/tpm-emu.c b/tests/qtest/tpm-emu.c
index 9e4c2005d0..6ef30ac6b8 100644
--- a/tests/qtest/tpm-emu.c
+++ b/tests/qtest/tpm-emu.c
@@ -119,6 +119,7 @@ void *tpm_emu_ctrl_thread(void *data)
         cmd = be32_to_cpu(cmd);
         g_assert_cmpint(cmd, ==, CMD_SET_DATAFD);
         g_assert_cmpint(nfd, ==, 1);
+        qemu_set_blocking(*pfd, true, &error_abort);
         s->tpm_ioc = QIO_CHANNEL(qio_channel_socket_new_fd(*pfd, &error_abort));
         g_free(pfd);
 
diff --git a/tests/unit/test-io-channel-socket.c b/tests/unit/test-io-channel-socket.c
index dc7be96e9c..815ee1d812 100644
--- a/tests/unit/test-io-channel-socket.c
+++ b/tests/unit/test-io-channel-socket.c
@@ -464,6 +464,7 @@ static void test_io_channel_unix_fd_pass(void)
                            &error_abort);
 
     g_assert(nfdrecv == G_N_ELEMENTS(fdsend));
+    qemu_fds_set_blockinging(fdrecv, nfdrecv, true, &error_abort);
     /* Each recvd FD should be different from sent FD */
     for (i = 0; i < nfdrecv; i++) {
         g_assert_cmpint(fdrecv[i], !=, testfd);
diff --git a/util/vhost-user-server.c b/util/vhost-user-server.c
index 8dcd32dc65..a2ae318ea3 100644
--- a/util/vhost-user-server.c
+++ b/util/vhost-user-server.c
@@ -161,6 +161,11 @@ vu_message_read(VuDev *vu_dev, int conn_fd, VhostUserMsg *vmsg)
                 g_free(fds);
                 goto fail;
             }
+            if (!qemu_fds_set_blockinging(fds, nfds, true, &local_err)) {
+                error_report_err(local_err);
+                g_free(fds);
+                goto fail;
+            }
             memcpy(vmsg->fds + vmsg->fd_num, fds, nfds * sizeof(vmsg->fds[0]));
             vmsg->fd_num += nfds;
             g_free(fds);
-- 
2.48.1

[PATCH 10/10] migration/qemu-file: don't make incoming fds blocking again

[Vladimir Sementsov-Ogievskiy]

In migration we want to pass fd "as is", not changing its
blocking status.

The only current user of these fds is CPR state (through VMSTATE_FD),
which of-course doesn't want to modify fds on target when source is
still running and use these fds.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 migration/qemu-file.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index b1d042e298..b6ac190034 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -361,12 +361,6 @@ static ssize_t coroutine_mixed_fn qemu_fill_buffer(QEMUFile *f)
         qemu_file_set_error_obj(f, len, local_error);
     }
 
-    /*
-     * NOTE: don't worry about error_abort, it will be removed
-     * in the next commit
-     */
-    qemu_fds_set_blockinging(fds, nfd, true, &error_abort);
-
     for (int i = 0; i < nfd; i++) {
         FdEntry *fde = g_new0(FdEntry, 1);
         fde->fd = fds[i];
-- 
2.48.1

Re: [PATCH 10/10] migration/qemu-file: don't make incoming fds blocking again

[Vladimir Sementsov-Ogievskiy]

CC Steve

On 03.09.25 12:44, Vladimir Sementsov-Ogievskiy wrote:
> In migration we want to pass fd "as is", not changing its
> blocking status.
> 
> The only current user of these fds is CPR state (through VMSTATE_FD),
> which of-course doesn't want to modify fds on target when source is
> still running and use these fds.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> ---
>   migration/qemu-file.c | 6 ------
>   1 file changed, 6 deletions(-)
> 
> diff --git a/migration/qemu-file.c b/migration/qemu-file.c
> index b1d042e298..b6ac190034 100644
> --- a/migration/qemu-file.c
> +++ b/migration/qemu-file.c
> @@ -361,12 +361,6 @@ static ssize_t coroutine_mixed_fn qemu_fill_buffer(QEMUFile *f)
>           qemu_file_set_error_obj(f, len, local_error);
>       }
>   
> -    /*
> -     * NOTE: don't worry about error_abort, it will be removed
> -     * in the next commit
> -     */
> -    qemu_fds_set_blockinging(fds, nfd, true, &error_abort);
> -
>       for (int i = 0; i < nfd; i++) {
>           FdEntry *fde = g_new0(FdEntry, 1);
>           fde->fd = fds[i];


-- 
Best regards,
Vladimir

Re: [PATCH 00/10] io: deal with blocking/non-blocking fds

[Lei Yang]

Tested the current series of patches, mixed with patches from series
[1] and [2], and the virtio-net regression tests passed. I also tested
local VM migration under multiple NIC queues enabled and disabled, it
also passed.

[1] https://patchwork.ozlabs.org/project/qemu-devel/cover/20250903124934.1169899-1-vsementsov@yandex-team.ru/
[2] https://patchwork.ozlabs.org/project/qemu-devel/cover/20250903133706.1177633-1-vsementsov@yandex-team.ru/

Tested-by: Lei Yang <leiyang@redhat.com>

On Wed, Sep 3, 2025 at 5:46 PM Vladimir Sementsov-Ogievskiy
<vsementsov@yandex-team.ru> wrote:
>
> Hi all!
>
> The series brings two things:
>
> 1. unify code which sets fds blocking/non-blocking through the whole
> source
>
> 2. for fds, which comes from qio_channel_readv_full(), stop making
> them blocking in generic code, and move this logic to the callers,
> all except coming from migration qemu-file (see last patch)
>
> Vladimir Sementsov-Ogievskiy (10):
>   io/channel: document how qio_channel_readv_full() handles fds
>   char-socket: rework tcp_chr_recv()
>   util: add qemu_set_blocking() function
>   util: drop qemu_socket_set_nonblock()
>   util: drop qemu_socket_try_set_nonblock()
>   util: drop qemu_socket_set_block()
>   use qemu_set_blocking instead of g_unix_set_fd_nonblocking
>   oslib-posix: add qemu_fds_set_blocking() helper
>   qio_channel_readv_full(): move setting fd blocking to callers
>   migration/qemu-file: don't make incoming fds blocking again
>
>  chardev/char-fd.c                       |  4 +--
>  chardev/char-pty.c                      |  3 +-
>  chardev/char-serial.c                   |  3 +-
>  chardev/char-socket.c                   | 45 ++++++++++++-------------
>  chardev/char-stdio.c                    |  3 +-
>  contrib/ivshmem-server/ivshmem-server.c |  5 ++-
>  hw/hyperv/syndbg.c                      |  4 ++-
>  hw/input/virtio-input-host.c            |  3 +-
>  hw/misc/ivshmem-flat.c                  |  4 ++-
>  hw/misc/ivshmem-pci.c                   |  8 ++++-
>  hw/remote/mpqemu-link.c                 |  3 ++
>  hw/vfio-user/proxy.c                    |  4 +++
>  hw/virtio/vhost-user.c                  | 10 +++++-
>  hw/virtio/vhost-vsock.c                 |  8 ++---
>  include/io/channel.h                    | 12 +++++++
>  include/qemu/osdep.h                    |  8 +++++
>  include/qemu/sockets.h                  |  3 --
>  io/channel-command.c                    |  9 +++--
>  io/channel-file.c                       |  3 +-
>  io/channel-socket.c                     | 26 +++++++-------
>  net/dgram.c                             | 28 ++++++++-------
>  net/l2tpv3.c                            |  5 +--
>  net/socket.c                            | 27 ++++++++++-----
>  net/stream.c                            |  9 ++---
>  net/stream_data.c                       | 10 +++---
>  net/tap-bsd.c                           | 12 +++++--
>  net/tap-linux.c                         |  8 ++++-
>  net/tap-solaris.c                       |  7 +++-
>  net/tap.c                               | 21 ++++--------
>  qga/channel-posix.c                     |  7 +++-
>  qga/commands-posix.c                    |  3 +-
>  scsi/qemu-pr-helper.c                   |  4 +++
>  tests/qtest/fuzz/virtio_net_fuzz.c      |  2 +-
>  tests/qtest/tpm-emu.c                   |  1 +
>  tests/qtest/vhost-user-test.c           |  3 +-
>  tests/unit/socket-helpers.c             |  5 ++-
>  tests/unit/test-crypto-tlssession.c     |  8 ++---
>  tests/unit/test-io-channel-socket.c     |  1 +
>  tests/unit/test-iov.c                   |  5 +--
>  ui/input-linux.c                        |  3 +-
>  util/event_notifier-posix.c             |  5 +--
>  util/main-loop.c                        |  6 +++-
>  util/oslib-posix.c                      | 27 +++++++++------
>  util/oslib-win32.c                      | 25 ++++++--------
>  util/vhost-user-server.c                |  9 +++--
>  45 files changed, 244 insertions(+), 165 deletions(-)
>
> --
> 2.48.1
>
>

Re: [PATCH 00/10] io: deal with blocking/non-blocking fds

[Vladimir Sementsov-Ogievskiy]

On 03.09.25 12:44, Vladimir Sementsov-Ogievskiy wrote:
> Hi all!
> 
> The series brings two things:
> 
> 1. unify code which sets fds blocking/non-blocking through the whole
> source
> 
> 2. for fds, which comes from qio_channel_readv_full(), stop making
> them blocking in generic code, and move this logic to the callers,
> all except coming from migration qemu-file (see last patch)

Oops, I found commit

commit ff5927baa7ffb9c97873a071f6a8d85a3584182b

     util: rename qemu_*block() socket functions

     The qemu_*block() functions are meant to be be used with sockets (the
     win32 implementation expects SOCKET)

     Over time, those functions where used with Win32 SOCKET or
     file-descriptors interchangeably. But for portability, they must only be
     used with socket-like file-descriptors. FDs can use
     g_unix_set_fd_nonblocking() instead.

     Rename the functions with "socket" in the name to prevent bad usages.

     This is effectively reverting commit f9e8cacc5557e43 ("oslib-posix:
     rename socket_set_nonblock() to qemu_set_nonblock()").


, which I effectively revert with these series..

Marc-André, Stefan could you take a look at my series?

I'm sure, that it's good to have one good interface, with errp and error
checking, instead of bucket of different functions, and making Error by
hand in many cases.

Except for the logic with files vs sockets, mentioned in the old commit.
Still, does it really make sense? It seems good to have a wrapper for
g_unix_set_fd_nonblocking() callded on FDs too..

So, formally, on top of my series I may add

bool qemu_socket_set_blocking(int fd, bool block, Error **errp)
{
    return qemu_set_blocking(fd, block errp);
}

and use it where _socket_ functions were used.

But that will not prevent someone in future to use qemu_set_blocking(), where
qemu_socket_set_blocking() should be used. No sense in it..

I think, in Windows, if we try to make non-socket to be non-blocking, we should get
a meaningful error, do we?

-- 
Best regards,
Vladimir

Re: [PATCH 00/10] io: deal with blocking/non-blocking fds

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 268 bytes --]

On Tue, Sep 09, 2025 at 11:12:25AM +0300, Vladimir Sementsov-Ogievskiy wrote:
> On 03.09.25 12:44, Vladimir Sementsov-Ogievskiy wrote:
> Marc-André, Stefan could you take a look at my series?

Please discuss with Daniel Berrangé and Marc-André. Thanks!

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]