qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Gerd Hoffmann <kraxel@redhat.com>
To: Luiz Capitulino <lcapitulino@redhat.com>
Cc: aliguori@us.ibm.com, qemu-devel@nongnu.org, avi@redhat.com
Subject: Re: [Qemu-devel] [PATCH v1 00/14]: Initial QObject conversion
Date: Fri, 02 Oct 2009 17:36:13 +0200	[thread overview]
Message-ID: <4AC61DED.5030702@redhat.com> (raw)
In-Reply-To: <20091002115509.0085b7e3@doriath>

   Hi,
>
>> So maybe they shouldn't be named MonitorError in the first place?
>
>   What do you suggest?
>
>   Hm.. This could be QError, meaning that it's also a QObject,
> so that we can put it in dicts, lists, etc.

Makes sense to me.

Attaching the pretty-print callback to the error struct not to the 
monitor command is a good move too.  Makes it easier to reuse these 
functions.

Hmm, maybe it would be even better to link the pretty-print functions to 
error codes?

>> Why user_error is needed in the first place btw?  To maintain
>> backward-compatible error message formating?
>
>   Yes and to do pretty printing too.
>
>   For example:
>
> qemu_error("Device \"%s\" not found.  Try -device '?' for a list.\n",
>             driver);
>
>   Makes sense for humans, but for QMP it would probably look like:
>
> { "error": { "code": 1234,
>               "desc": "device not found",
>               "data": { "name": "foobar-device" } } }

You could have a generic function which makes a message like this:

   'device not found: name=foobar-device'

out of the QError.  Not too bad.  You'll loose the '-device ?' hint though.

I think it is reasonable to make the pretty-print callback optional and 
use the generic function in case it isn't present.  So you have 
something working by default and can override it of the message isn't 
userfriendly or backward-compatible enougth.

>   So, when QMP is disabled that function will be called to print the error
> for humans. When QMP is enabled this is never called and the protocol
> emission code will use QError to emit something like the { "error" }
> dict above.

Thanks for the explanation.

cheers,
   Gerd

  reply	other threads:[~2009-10-02 15:36 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-10-01 15:50 [Qemu-devel] [PATCH v1 00/14]: Initial QObject conversion Luiz Capitulino
2009-10-01 15:50 ` [Qemu-devel] [PATCH 01/14] QObject: Accept NULL Luiz Capitulino
2009-10-01 15:50 ` [Qemu-devel] [PATCH 02/14] Introduce monitor-error module Luiz Capitulino
2009-10-01 15:50 ` [Qemu-devel] [PATCH 03/14] monitor: Add new members to mon_cmd_t Luiz Capitulino
2009-10-01 15:50 ` [Qemu-devel] [PATCH 04/14] monitor: Handle new and old style handlers Luiz Capitulino
2009-10-01 15:50 ` [Qemu-devel] [PATCH 05/14] monitor: Initial MonitorError usage Luiz Capitulino
2009-10-01 15:50 ` [Qemu-devel] [PATCH 06/14] monitor: do_info(): handle new and old info handlers Luiz Capitulino
2009-10-01 15:50 ` [Qemu-devel] [PATCH 07/14] monitor: Convert do_quit() do QObject Luiz Capitulino
2009-10-01 15:50 ` [Qemu-devel] [PATCH 08/14] monitor: Convert do_stop() to QObject Luiz Capitulino
2009-10-01 15:50 ` [Qemu-devel] [PATCH 09/14] monitor: Convert do_system_reset() " Luiz Capitulino
2009-10-01 15:50 ` [Qemu-devel] [PATCH 10/14] monitor: Convert do_system_powerdown() " Luiz Capitulino
2009-10-01 15:50 ` [Qemu-devel] [PATCH 11/14] monitor: Convert do_balloon() " Luiz Capitulino
2009-10-01 15:50 ` [Qemu-devel] [PATCH 12/14] monitor: Convert do_info_version() " Luiz Capitulino
2009-10-01 15:50 ` [Qemu-devel] [PATCH 13/14] monitor-error: Add do_info_balloon() errors Luiz Capitulino
2009-10-01 15:50 ` [Qemu-devel] [PATCH 14/14] monitor: Convert do_info_balloon() to QObject Luiz Capitulino
2009-10-01 16:01 ` [Qemu-devel] Re: [PATCH v1 00/14]: Initial QObject conversion Avi Kivity
2009-10-01 21:21   ` Luiz Capitulino
2009-10-03  7:59     ` Avi Kivity
2009-10-05 13:16       ` Luiz Capitulino
2009-10-02 12:47 ` [Qemu-devel] " Gerd Hoffmann
2009-10-02 13:47   ` Luiz Capitulino
2009-10-02 14:17     ` Gerd Hoffmann
2009-10-02 14:55       ` Luiz Capitulino
2009-10-02 15:36         ` Gerd Hoffmann [this message]
2009-10-02 18:32           ` Luiz Capitulino

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=4AC61DED.5030702@redhat.com \
    --to=kraxel@redhat.com \
    --cc=aliguori@us.ibm.com \
    --cc=avi@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).