On Fri, Aug 08, 2025 at 10:08:18AM +0200, Markus Armbruster wrote:
> watch_add() reports _open_osfhandle() failure with
> error_setg(&error_warn, ...).
>
> I'm not familiar with Spice, so I can't say whether it will work after
> such a failure. If it doesn't, then this should be an error. If it
> does, then why bother the user with a warning that isn't actionable,
> and likely confusing?
If watch_add fails, spice is dead. Eventually the NULL returned from
watch_add will bubble up to the spice_server_init function which will
return non-zero and QEMU will do
error_report("failed to initialize spice server");
exit(1);
The warning in watch_add gives a better chance of understanding
what failed than this generic error_report() does.
>
> Regardless of that, error_setg_win32(&error_warn, ...) is undesirable
> just like error_setg(&error_fatal, ...) and error_setg(&error_abort,
> ...) are. Replace by warn_report().
>
> Cc: Marc-André Lureau <marcandre.lur...@redhat.com>
> Signed-off-by: Markus Armbruster <arm...@redhat.com>
> ---
> ui/spice-core.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
Reviewed-by: Daniel P. Berrangé <berra...@redhat.com>
>
> diff --git a/ui/spice-core.c b/ui/spice-core.c
> index 5992f9daec..97bdd171cd 100644
> --- a/ui/spice-core.c
> +++ b/ui/spice-core.c
> @@ -132,7 +132,8 @@ static SpiceWatch *watch_add(int fd, int event_mask,
> SpiceWatchFunc func, void *
> #ifdef WIN32
> fd = _open_osfhandle(fd, _O_BINARY);
> if (fd < 0) {
> - error_setg_win32(&error_warn, WSAGetLastError(), "Couldn't associate
> a FD with the SOCKET");
> + warn_report("Couldn't associate a FD with the SOCKET: %s"
> + g_win32_error_message(WSAGetLastError()));
> return NULL;
> }
> #endif
> --
> 2.49.0
>
>
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 :|
