On Mon, Sep 15, 2025 at 10:30:58PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> Use common qemu_set_blocking() instead.
>
> Note that pre-patch the behavior of Win32 and Linux realizations
> are inconsistent: we ignore failure for Win32, and assert success
> for Linux.
>
> How do we convert the callers?
>
> 1. Most of callers call qemu_socket_set_nonblock() on a
> freshly created socket fd, in conditions when we may simply
> report an error. Seems correct switching to error handling
> both for Windows (pre-patch error is ignored) and Linux
> (pre-patch we assert success). Anyway, we normally don't
> expect errors in these cases.
>
> Still in tests let's use &error_abort for simplicity.
>
> What are exclusions?
>
> 2. hw/virtio/vhost-user.c - we are inside #ifdef CONFIG_LINUX,
> so no damage in switching to error handling from assertion.
>
> 3. io/channel-socket.c: here we convert both old calls to
> qemu_socket_set_nonblock() and qemu_socket_set_block() to
> one new call. Pre-patch we assert success for Linux in
> qemu_socket_set_nonblock(), and ignore all other errors here.
> So, for Windows switch is a bit dangerous: we may get
> new errors or crashes(when error_abort is passed) in
> cases where we have silently ignored the error before
> (was it correct in all such cases, if they were?) Still,
> there is no other way to stricter API than take
> this risk.
>
> 4. util/vhost-user-server - compiled only for Linux (see
> util/meson.build), so we are safe, switching from assertion to
> &error_abort.
>
> Note: In qga/channel-posix.c we use g_warning(), where g_printerr()
> would actually be a better choice. Still let's for now follow
> common style of qga, where g_warning() is commonly used to print
> such messages, and no call to g_printerr(). Converting everything
> to use g_printerr() should better be another series.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@yandex-team.ru>
> ---
> contrib/ivshmem-server/ivshmem-server.c | 9 ++++++++-
> 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 | 4 +++-
> 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, 65 insertions(+), 37 deletions(-)
> diff --git a/util/vhost-user-server.c b/util/vhost-user-server.c
> index d805a92394..b3416ab956 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);
> }
> }
The caller of this method is able to handle errors more gracefully
than 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;
Can we put a TODO here that error_abort should be fixed to be more
graceful - either by moving the set_blocking call out of this
callback entirely, or allowing this method to return errors.
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 :|
