qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Daniel P. Berrangé" <berrange@redhat.com>
To: Markus Armbruster <armbru@redhat.com>
Cc: qemu-devel@nongnu.org
Subject: Re: Abuse of warnings for unhandled errors and programming errors
Date: Tue, 19 Aug 2025 13:30:22 +0100	[thread overview]
Message-ID: <aKRuXq0_5ULf5yp-@redhat.com> (raw)
In-Reply-To: <87h5yijh3b.fsf@pond.sub.org>

On Fri, Aug 08, 2025 at 11:30:32AM +0200, Markus Armbruster wrote:
> In "[PATCH 00/12] Error reporting cleanup, a fix, and &error_warn
> removal", I challenged the use of warnings in a few places.  I think the
> topic deserves a wider audience than the one a rather pedestrian cleanup
> series draws.
> 
> 
> To make my case, I need to start with errors.  We distinguish between
> ordinary errors (for lack of a better word) and programming errors.
> 
> Ordinary errors are things like nonsensical user requests, unavailable
> resources, and so forth.  A correct program is prepared for such
> failures, detects them, and reports them to the user.  The user can then
> fix their request, try again when resources are available, and so forth.
> 
> Tools for reporting ordinary errors are error_report(),
> error_report_err(), &error_fatal, and friends.

The thing about nonsense user rquests / unavailable resources , etc
is that almost none of them should imply exiting QEMU, except if they
occur in the context of system startup before the VM starts executing.
Once running we should do everything in our power to not let the users
workload die.

From that POV, I tend to wish that error_fatal did not exist and that
we instead propagated all fatal errors up until reaching main(), so
we were not at risk of using error_fatal in runtime scenarios. We're
largely stuck with what we've got though, due to our need to retrofit
error reporting in to our existing codebase design.

I do try to push back in review any time we introduce new code that
doesn't propagate errors as high up the stack as possible/practcal.

> Programming errors are bugs.  A developer needs to fix the program.
> Unlike ordinary errors, programming errors are *unexpected*.
> 
> Programming errors are commonly not recoverable.  The proper tool for
> unrecoverable ones is assertions.  &error_abort can be a convenient way
> to assert "this can't fail".

We could have called it &error_assert but that's bike shed colouring :-)

> On to warnings.
> 
> When some failure doesn't prevent satisfying some request, an ordinary
> error can be misleading.  We make it a warning instead then.
> 
> What if it's a programming error we recover from?
> 
> Aside: trying to recover in a buggy program is risky, but that's not the
> debate I want to have here.
> 
> How do we want such recoverable programming errors reported?
> 
> Warning?  We seem to be abusing warnings this way, and I hate it.  What
> we have to report is a *bug*, and we should make that crystal clear.
> "warning: FunctionYouNeverHeardAbout() failed" does not.  It could be
> anything, and you likely need to look at the source to find out.
> 
> Ordinary error reporting with "internal error: " prefix, so the user
> understands this is a bug, and all they can do about it is report it?
> 
> Log the bug somehow?
> 
> Thoughts?

I don't see 'warnings' as something directly actionable for a user.
Rather they are messages that I would want to see included in a log
file that a user attaches to a bug report if they find some behavioural
problem. If the user understands the warning great, but that isn't a
requirement.

IOW, while informative warnings is of course better than not, as long
as the warning message contains sufficient info for the maintainer to
understand what happened the minimum quality bar is satisfied IMHO.

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



  reply	other threads:[~2025-08-19 12:31 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-08  9:30 Abuse of warnings for unhandled errors and programming errors Markus Armbruster
2025-08-19 12:30 ` Daniel P. Berrangé [this message]
2025-09-10 11:05   ` Markus Armbruster
2025-09-15 18:02     ` Daniel P. Berrangé
2025-09-16 13:25       ` Markus Armbruster

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aKRuXq0_5ULf5yp-@redhat.com \
    --to=berrange@redhat.com \
    --cc=armbru@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).