Re: [Qemu-devel] [PATCH v0 0/9] QError

[Anthony Liguori <aliguori@us.ibm.com>]

Markus Armbruster wrote:
> I just caught up with this topic, and since there's no obvious one
> message to reply to, I just reply here.
>
> I agree with Anthony we better get the error reporting protocol
> approximately right the first time, and rushing it could lead to tears
> later on.  Mind, I said "approximately right", not "perfect".
>
> However, there are more than one way to screw this up.  One is certainly
> to fall short of client requirements in a way that is costly to fix
> (think incompatible protocol revision).  Another is to overshoot them in
> a way that is costly to maintain.  A third one is to spend too much time
> on figuring out the perfect solution.
>
> I believe our true problem is that we're still confused and/or
> disagreeing on client requirements, and this has led to a design that
> tries to cover all the conceivable bases, and feels overengineered to
> me.
>
> There are only so many complexity credits you can spend in a program,
> both globally and individually.  I'm very, very wary of making error
> reporting more complex than absolutely, desperately necessary.
> Reporting errors should remain as easy as we can make it, for reasons
> that have already been mentioned by me and others:
>
> * The more cumbersome it is to report an error, the less it is done, and
>   the more vaguely it is done.  If you have to edit more than the error
>   site to report an error accurately, then chances skyrocket that it
>   won't be reported, or it'll be reported inaccurately.  And not because
>   coders are stupid or lazy, but because they make sensible use of their
>   very limited complexity credits: if you can either get the job done
>   with lousy error messages, or not get it done at all, guess what the
>   smart choice is.
>
> * It's highly desirable for errors to do double duty as documentation.
>   Code like this (random pick) doesn't need a comment:
>
>         if (qemu_opt_get(opts, "vlan")) {
>             qemu_error("The 'vlan' parameter is not valid with -netdev\n");
>             return -1;
>         }
>   

Or:

if (qemu_opt_get(opts, "vlan")) {
   throw InvalidParameter("The 'vlan' parameter is not valid with -netdev");
}

Which would become:

#define INVALID_PARAMETER "InvalidParameter"

if (qemu_opt_get(opts, "vlan")) {
    qemu_error(INVALID_PARAMETER, "The 'vlan' parameter is not valid 
with -netdev");
    return -1;
}

And if we find that this is a common occurrance, we probably should do:

invalid_parameter("vlan", "-netdev");

Which would expand to:

asprintf(&buf, "The '%s' parameter is not valid with %s", param, option);
qemu_error_full("InvalidParameter", "{'param': %s, 'option': %s, 'desc': 
%s}", param, option, desc);

The real fundamental requirements are:

1) In order to support high level languages, an error class is needed to 
generate proper exceptions.
2) To integrate well into high level languages exception mechanisms, we 
make errors first class objects.
3) Exceptions should have a string representation.  desc is a standard 
string representation.

There's really nothing complex here.

-- 
Regards,

Anthony Liguori