Re: [PATCH 07/12] ui/spice-core: Clean up error reporting

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

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.lureau@redhat.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  ui/spice-core.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Reviewed-by: Daniel P. Berrangé <berrange@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 :|