Re: [PATCH-for-5.2 v4] hw/core/qdev: Increase qdev_realize() kindness

[Markus Armbruster <armbru@redhat.com>]

Paolo Bonzini <pbonzini@redhat.com> writes:

> On 29/07/20 09:39, Markus Armbruster wrote:
>> Taking a step back, I disagree with the notion that assertions should be
>> avoided just because we have an Error **.  A programming error doesn't
>> become less wrong, and continuing when the program is in disorder
>> doesn't become any safer when you add an Error ** parameter to a
>> function.
>
> I don't think it is actually unsafe to continue after passing a bus-less
> device with a bus_type to qdev_realize.  It will fail, but orderly.
>
> So even though it's a programming error, it should not be a big deal to
> avoid the assertion here: either the caller will pass &error_abort, or
> it will print a nice error message and let the user go on with their
> business.

An error message the user can do nothing about other than report as a
bug is never nice.

We can try to phrase the message in a way that makes "this is a bug,
please report" clear, but doing that case-by-case can only result in
inconsistency and confusion.  Having a common way to do it gets us to:

>> If you're calling for recovering from programming errors where that can
>> be done safely, we can talk about creating the necessary infrastructure.
>> Handling them as if they were errors the user can do something about can
>> only lead to confusion.

Moreover, we want programming errors reported even when we recover, so
they get fixed.  If we treat them like any other error, that is not
assured: errors may be handled silently.

> I'm not particularly attached to the change, but it seemed inconsistent
> to use error_setg(&error_abort).

From error.h:

 * Please don't error_setg(&error_fatal, ...), use error_report() and
 * exit(), because that's more obvious.
 * Likewise, don't error_setg(&error_abort, ...), use assert().