Re: [Qemu-devel] [PATCH 00/10]: QError v4

[Markus Armbruster <armbru@redhat.com>]

Anthony Liguori <anthony@codemonkey.ws> writes:

> Markus Armbruster wrote:
>> I'm thinking and talking primarily about the protocol, and that probably
>> makes me too terse on implementation.
>>
>> I didn't mean to suggest that for adding the data part we should add new
>> arguments providing the data.  That would be dumb indeed.
>>
>> Instead, I'd like to start with an extremely simple error reporting
>> mechanism, which requires an equally simple conversion, namely from
>> monitor_printf("human-readable error") to something of the form
>> qmp_error(error_code, "human-readable error").
>>   
>
> This is what I want to focus on because I think everything just boils
> down to this. Namely:
>
> qmp_error(error_code, "human-readable error");
>
> In my mind, the purpose of QMP is to provide a machine readable form
> of the monitor protocol. We have a monitor protocol today but because
> it's written as free-form text (often English sentences), it's
> difficult/impossible to parse which makes managing qemu extremely
> difficult. We have problems with escaping characters, use of
> non-deterministic grammars, etc.

Agreed.

> What I'm fundamentally opposed to is the "human-readable error" part
> of your proposal because it's free-form and unstructured. It has all
> the same problems as the current monitor interface. I'm opposed to
> this for the same reason I would be opposed to an "info balloon"
> command output of:
>
> {"data" : {"target" : "The guest has 32MB of memory"}}
>
> If all you were doing is outputting "info balloon" to a user in a
> command line tool, sure, this would be easier for the client to deal
> with. However, the whole purpose of QMP is to allow tools to make
> sense out of the monitor commands.
>
> The contents of an error are just as important as the output of a
> command.

Agreed.

>          The error code helps a bit but the problem with a qmp_error()
> that takes a string parameter is that there is additional information
> in that string that is inaccessible to a client.
>
> I'm certainly willing to consider alternative ways to do qmp_error()
> but taking a free form string is not an option in my mind. It goes
> against the fundamentals of what we're trying to build with QMP.
>
> So if you're opposed to structured error data, just having
> qmp_error(error_code) is a reasonable alternative. I don't think it's
> the right thing to do, but I think it's still within the spirit of the
> goals of QMP.

The error code is for machines, the free-form text is for humans.
There's ample precedence for this usage in existing protocols, e.g. RFC
2616 - Hypertext Transfer Protocol -- HTTP/1.1, section 6.1.1:

    6.1.1 Status Code and Reason Phrase

    The Status-Code element is a 3-digit integer result code of the
    attempt to understand and satisfy the request.  [...]  The
    Reason-Phrase is intended to give a short textual description of the
    Status-Code.  The Status-Code is intended for use by automata and
    the Reason-Phrase is intended for the human user.  The client is not
    required to examine or display the Reason- Phrase.

and RFC 959 - File Transfer Protocol:

    4.2.  FTP REPLIES

    [...]

    An FTP reply consists of a three digit number (transmitted as three
    alphanumeric characters) followed by some text.  The number is
    intended for use by automata to determine what state to enter next;
    the text is intended for the human user.  It is intended that the
    three digits contain enough encoded information that the
    user-process (the User-PI) will not need to examine the text and may
    either discard it or pass it on to the user, as appropriate.  In
    particular, the text may be server-dependent, so there are likely to
    be varying texts for each reply code.

> I think this is the fundamental thing to come to an agreement on in
> this discussion before we can even delve into the merits or lack
> thereof of structured error data.

First and foremost, I'd like to have reasonably specific error codes.
Give each distinct failure its own error code (it's not like there's a
shortage of codes).  Then, the error code should be enough information
for the client to handle the error sensibly.

I'd like to have a simple classification scheme for error codes, to help
clients handle errors they don't know.  Discussed elsewhere in this
thread.

In addition to the machine-readable error code, I'd like to include a
human-readable error message.  It is strictly for humans, not for
machines.  It helps humans interpret what goes on on the wire (useful
for debugging).  Some clients may find it useful to pass it on to their
human users, or log it.

I'm not fundamentally opposed to adding structured error data to that.
I just think doing it in the first iteration is unwise.  Of course, the
people writing and merging the patch are certainly free to do it anyway.