qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Anthony Liguori <anthony@codemonkey.ws>
To: Markus Armbruster <armbru@redhat.com>
Cc: Luiz Capitulino <lcapitulino@redhat.com>,
	qemu-devel@nongnu.org, kraxel@redhat.com
Subject: Re: [Qemu-devel] [PATCH 00/10]: QError v4
Date: Sun, 22 Nov 2009 10:08:16 -0600	[thread overview]
Message-ID: <4B0961F0.3070004@codemonkey.ws> (raw)
In-Reply-To: <m3iqd48abi.fsf@crossbow.pond.sub.org>

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.

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. 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.

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.

Regards,

Anthony Liguori

  reply	other threads:[~2009-11-22 16:08 UTC|newest]

Thread overview: 55+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-11-17 19:43 [Qemu-devel] [PATCH 00/10]: QError v4 Luiz Capitulino
2009-11-17 19:43 ` [Qemu-devel] [PATCH 01/10] QJSON: Introduce qobject_from_jsonv() Luiz Capitulino
2009-11-17 19:43 ` [Qemu-devel] [PATCH 02/10] QString: Introduce qstring_append_chr() Luiz Capitulino
2009-11-17 19:43 ` [Qemu-devel] [PATCH 03/10] QString: Introduce qstring_append_int() Luiz Capitulino
2009-11-17 19:43 ` [Qemu-devel] [PATCH 04/10] QString: Introduce qstring_from_substr() Luiz Capitulino
2009-11-17 19:43 ` [Qemu-devel] [PATCH 05/10] utests: Add qstring_append_chr() unit-test Luiz Capitulino
2009-11-17 19:43 ` [Qemu-devel] [PATCH 06/10] utests: Add qstring_from_substr() unit-test Luiz Capitulino
2009-11-17 19:43 ` [Qemu-devel] [PATCH 07/10] Introduce QError Luiz Capitulino
2009-11-18 15:16   ` Markus Armbruster
2009-11-18 17:23     ` Luiz Capitulino
2009-11-19  8:42       ` Markus Armbruster
2009-11-19 12:59         ` [Qemu-devel] " Paolo Bonzini
2009-11-18 18:14   ` [Qemu-devel] " Daniel P. Berrange
2009-11-18 19:58     ` Anthony Liguori
2009-11-18 20:13       ` Luiz Capitulino
2009-11-17 19:43 ` [Qemu-devel] [PATCH 08/10] monitor: QError support Luiz Capitulino
2009-11-18 15:16   ` Markus Armbruster
2009-11-18 17:29     ` Luiz Capitulino
2009-11-18 18:16   ` Daniel P. Berrange
2009-11-17 19:43 ` [Qemu-devel] [PATCH 09/10] qdev: Use QError for 'device not found' error Luiz Capitulino
2009-11-18 15:17   ` Markus Armbruster
2009-11-18 17:32     ` Luiz Capitulino
2009-11-20  7:23     ` Amit Shah
2009-11-17 19:43 ` [Qemu-devel] [PATCH 10/10] monitor: do_info_balloon(): use QError Luiz Capitulino
2009-11-18 15:17   ` Markus Armbruster
2009-11-18 15:58     ` Anthony Liguori
2009-11-18 18:10       ` Luiz Capitulino
2009-11-18 16:06 ` [Qemu-devel] [PATCH 00/10]: QError v4 Markus Armbruster
2009-11-18 18:08   ` Anthony Liguori
2009-11-19  2:36     ` Jamie Lokier
2009-11-20 15:56       ` Anthony Liguori
2009-11-20 16:20         ` Luiz Capitulino
2009-11-20 16:27           ` Anthony Liguori
2009-11-20 17:57             ` Markus Armbruster
2009-11-20 17:29         ` Markus Armbruster
2009-11-20 17:37           ` Anthony Liguori
2009-11-19 10:11     ` Markus Armbruster
2009-11-20 16:13       ` Anthony Liguori
2009-11-20 18:47         ` Markus Armbruster
2009-11-20 19:04           ` Anthony Liguori
2009-11-21 10:02             ` Markus Armbruster
2009-11-22 16:08               ` Anthony Liguori [this message]
2009-11-23 13:06                 ` Luiz Capitulino
2009-11-23 13:11                   ` Anthony Liguori
2009-11-23 13:34                     ` Luiz Capitulino
2009-11-23 13:50                       ` Alexander Graf
2009-11-24 11:55                         ` Luiz Capitulino
2009-11-24 12:13                           ` Alexander Graf
2009-11-23 16:08                 ` Markus Armbruster
2009-11-23 12:42               ` Luiz Capitulino
2009-11-23 16:15                 ` Markus Armbruster
2009-11-18 18:13   ` [Qemu-devel] " Paolo Bonzini
2009-11-19 10:25     ` Markus Armbruster
2009-11-19 13:01       ` Paolo Bonzini
2009-11-19 14:11         ` Markus Armbruster

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4B0961F0.3070004@codemonkey.ws \
    --to=anthony@codemonkey.ws \
    --cc=armbru@redhat.com \
    --cc=kraxel@redhat.com \
    --cc=lcapitulino@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).