From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43)
	id 1MtkBD-0008RK-Io
	for qemu-devel@nongnu.org; Fri, 02 Oct 2009 11:36:23 -0400
Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43)
	id 1MtkB8-0008R1-5f
	for qemu-devel@nongnu.org; Fri, 02 Oct 2009 11:36:22 -0400
Received: from [199.232.76.173] (port=47693 helo=monty-python.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1MtkB7-0008Qy-Vz
	for qemu-devel@nongnu.org; Fri, 02 Oct 2009 11:36:18 -0400
Received: from mx1.redhat.com ([209.132.183.28]:41388)
	by monty-python.gnu.org with esmtp (Exim 4.60)
	(envelope-from <kraxel@redhat.com>) id 1MtkB7-0002IK-E4
	for qemu-devel@nongnu.org; Fri, 02 Oct 2009 11:36:17 -0400
Message-ID: <4AC61DED.5030702@redhat.com>
Date: Fri, 02 Oct 2009 17:36:13 +0200
From: Gerd Hoffmann <kraxel@redhat.com>
MIME-Version: 1.0
Subject: Re: [Qemu-devel] [PATCH v1 00/14]: Initial QObject conversion
References: <1254412245-10452-1-git-send-email-lcapitulino@redhat.com>	<4AC5F646.9040800@redhat.com>	<20091002104729.510d67de@doriath>	<4AC60B7F.5040809@redhat.com>
	<20091002115509.0085b7e3@doriath>
In-Reply-To: <20091002115509.0085b7e3@doriath>
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.gnu.org/pipermail/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Luiz Capitulino <lcapitulino@redhat.com>
Cc: aliguori@us.ibm.com, qemu-devel@nongnu.org, avi@redhat.com

   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