qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Anthony Liguori
Subject: Re: [Qemu-devel] [RFC] Fixing the error failure
Date: Mon, 02 Jul 2012 07:47:56 -0500
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:11.0) Gecko/20120329 Thunderbird/11.0.1

On 06/28/2012 02:50 AM, Markus Armbruster wrote:
Anthony Liguori<address@hidden>  writes:

On 06/26/2012 06:21 AM, Markus Armbruster wrote:
"Daniel P. Berrange"<address@hidden>   writes:

On Tue, Jun 26, 2012 at 09:54:21AM +0200, Markus Armbruster wrote:
Luiz Capitulino<address@hidden>   writes:

On Thu, 21 Jun 2012 13:42:19 +0100
"Daniel P. Berrange"<address@hidden>   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.




reply via email to

[Prev in Thread] Current Thread [Next in Thread]