Re: [PATCH v2 3/8] util: add qemu_set_blocking() function

["Daniel P. Berrangé" <berrange@redhat.com>]

On Thu, Sep 11, 2025 at 12:20:01PM +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 Unix-specific g_unix_set_fd_nonblocking()
> (of course, all calls are out of Windows-compatible code), which
> is glib specific with GError, which we can't use, and have to
> handle error-reporting by hand after the call.
> 
> and several platform-agnostic qemu_* helpers:
> 
> - qemu_socket_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(), providing and error, but not errp,
> so we have to handle it after the call
> 
> - qemu_socket_set_block(), which simply ignores an error
> 
> Note, that *_socket_* word in original API, which we are going
> to substitute was intended, because Windows support these operations
> only for sockets. What leads to solution of dropping it again?
> 
> 1. Having a QEMU-native wrapper with errp parameter
> for g_unix_set_fd_nonblocking() for non-socket fds worth doing,
> at least to unify error handling.
> 
> 2. So, if try to keep _socket_ vs _file_ words, we'll have two
> actually duplicated functions for Linux, which actually will
> be executed successfully on any (good enough) fds, and nothing
> prevent using them improperly except for the name. That doesn't
> look good.
> 
> 3. Naming helped us in the world where we crash on errors or
> ignore them. Now, with errp parameter, callers are intended to
> proper error checking. And for places where we really OK with
> crash-on-error semantics (like tests), we have an explicit
> &error_abort.
> 
> So, this commit starts a series, which will effectively revert
> commit ff5927baa7ffb9 "util: rename qemu_*block() socket functions"
> (which in turn was reverting f9e8cacc5557e43
> "oslib-posix: rename socket_set_nonblock() to qemu_set_nonblock()",
> so that's a long story).
> Now we don't simply rename, instead we provide the new API and
> update all the callers.
> 
> This commit only introduces a new fd-layer wrapper. Next commits
> will replace old API calls with it, and finally remove old API.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> ---
>  include/qemu/osdep.h |  1 +
>  util/oslib-posix.c   | 15 +++++++++++++++
>  util/oslib-win32.c   | 18 ++++++++++++++++++
>  3 files changed, 34 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 :|