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

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

On Tue, Sep 09, 2025 at 01:41:51PM +0200, Markus Armbruster wrote:
> Daniel P. Berrangé <berrange@redhat.com> writes:
> 
> > 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.
> 
> Would you like me to work this into the commit message?

Yes, if you're re-spinning might as well add the context info.

> 
> >> 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>
> 
> Thanks!
> 
> [...]
> 

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