Re: [Qemu-devel] [RFC] Fixing the error failure

[Anthony Liguori <anthony@codemonkey.ws>]

On 06/28/2012 02:50 AM, Markus Armbruster wrote:
> Anthony Liguori<anthony@codemonkey.ws>  writes:
>
>> On 06/26/2012 06:21 AM, Markus Armbruster wrote:
>>> "Daniel P. Berrange"<berrange@redhat.com>   writes:
>>>
>>>> On Tue, Jun 26, 2012 at 09:54:21AM +0200, Markus Armbruster wrote:
>>>>> Luiz Capitulino<lcapitulino@redhat.com>   writes:
>>>>>
>>>>>> On Thu, 21 Jun 2012 13:42:19 +0100
>>>>>> "Daniel P. Berrange"<berrange@redhat.com>   wrote:
>>> [...]
>>>>>>> In libvirt we have always reserved the right to change the error
>>>>>>> code reported for particular scenarios. So, for example, alot of
>>>>>>> our errors started out as "InternalError" (equiv to UndefinedError)
>>>>>>> but over time we have changed some to more specialized values
>>>>>>> eg "InvalidOperation", "ConfigNotSupported" and so on.
>>>>>
>>>>> Do you reserve the right to make changes other than from InternalError
>>>>> to a more specific one?
>>>>
>>>> If I'm perfectly honest, then yes. We have tried not to gratuitouslyy
>>>> change errors being reported, but they have definitely evolved over
>>>> time, so more distinct error scenarios are reported, where previously
>>>> many things would have been lumped under one code.
>>>
>>> Pragmatic co-evolution of errors with their actual use.  Makes sense to
>>> me, and I'd recommend we do the same in QEMU.
>>
>> Just to be clear, what we're discussing is:
>>
>>> diff --git a/qerror.c b/qerror.c
>>> index 92c4eff..ebe2eb0 100644
>>> --- a/qerror.c
>>> +++ b/qerror.c
>>> @@ -328,6 +328,10 @@ static const QErrorStringTable qerror_table[] = {
>>>           .error_fmt = QERR_SOCKET_CREATE_FAILED,
>>>           .desc      = "Failed to create socket",
>>>       },
>>> +    {
>>> +        .error_fmt = QERR_UNDEFINED_ERROR,
>>> +        .desc      = "Undefined Error: %(message)",
>>> +    },
>>>       {}
>>>   };
>>>
>>> diff --git a/qerror.h b/qerror.h
>>> index b4c8758..da8f086 100644
>>> --- a/qerror.h
>>> +++ b/qerror.h
>>> @@ -266,4 +266,7 @@ QError *qobject_to_qerror(const QObject *obj);
>>>   #define QERR_SOCKET_CREATE_FAILED \
>>>       "{ 'class': 'SockCreateFailed', 'data': {} }"
>>>
>>> +#define QERR_UNDEFINED_ERROR \
>>> +    "{ 'class': 'UndefinedError', 'data': { 'message': %s } }"
>>> +
>>>   #endif /* QERROR_H */
>>
>> Nothing more, nothing less.  No changes to wire protocol, no changes
>> to existing error uses, etc.
>
> What we're discussing is whatever people bring up, actually.
>
> So far, there haven't been any calls for a change of the wire protocol.

Changing all existing errors and repurposing 'desc' is effectively changing the 
wire protocol.

>
> There has been an agreement of sorts that the current "rich errors" have
> been "a failure" (your words).  Doesn't mean we all agree on the nature
> of the failure.
>
> Several people pointed out that:
>
> * our "rich" errors are so cumbersome to emit that adoption has been
>    slow,
>
> * our "rich" errors' human-readable descriptions lead to piss-poor
>    human-readable error messages[*] (pardon my french), and
>
> * the "richness" has no known uses after 2+ years.

Do you know of *any* QMP user beyond libvirt?  Let's face it, QMP is a protocol 
for libvirt.  What it looks like shouldn't be dictated by anything other than 
what libvirt wants/needs.

If libvirt isn't going to do more than look at an error type, then there's no 
point in providing more than that.

> Perhaps your idea of the rich errors failure stops at the first item
> (slow adoption).  Perhaps you even entertain hopes of solving the
> adoption problem by first asking for adoption of "simplified rich
> errors", and once that's done, push again for your original design.
>
> I disagree with both notions.
>
> Slow adoption is a *symptom*, not a cause: it's been slow, because the
> costs and drawbacks outweigh the benefits.  In other words, it's been
> slow, because people have had the good sense not to do something that's
> plainly not worth it.
>
> I agree converting existing uses of the failed rich errors API to
> whatever new API we come up with before anything else isn't useful.
>
> But I challenge the idea that we must not change existing uses of "rich"
> error reporting ever.

I think you're missing the forest for the trees.

The real problem we need to solve is not the quality of error messages. 
Honestly, we have all of the tools today to improve error messages.

The problem we need to solve is error propagation because without it, we cannot 
have asynchronous commands.  We keep ending up with extremely complex/cumbersome 
management interfaces that we need to support forever because we still have 
out-of-band errors that cannot be associated with a specific QMP command.

So I don't want to see us focus a bunch of effort on rewriting existing error 
users.  Is that a hard and fast rule?  Of course not.  If it makes sense to 
tweak an error here and there, that's fine.

But the problem to solve is killing off error_report.

Regards,

Anthony Liguori

   When such a use produces bad human-readable
> messages, that's a bug, and the most expedient fix could well be a
> switch to the new, saner API, even when that drops a bit of the richness
> (which after all has no known uses).
>
> For me, known suffering of human users weighs more heavily than
> hypothetical suffering of hypothetical tools.
>
>
> [*] This is why I refuse to work on error conversions.  Turning decent
> error messages into garbage is just too frustrating for me.  Yes, my
> life would be easier if I didn't care for users.