All of lore.kernel.org
 help / color / mirror / Atom feed
From: Anthony Liguori <aliguori@linux.vnet.ibm.com>
To: Luiz Capitulino <lcapitulino@redhat.com>
Cc: Anthony Liguori <aliguori@us.ibm.com>,
	qemu-devel@nongnu.org, kraxel@redhat.com, pbonzini@redhat.com,
	hollisb@linux.vnet.ibm.com
Subject: Re: [Qemu-devel] [RFC 0/8]: QError v2
Date: Thu, 12 Nov 2009 08:44:03 -0600	[thread overview]
Message-ID: <4AFC1F33.4060405@linux.vnet.ibm.com> (raw)
In-Reply-To: <20091112114108.4cca3405@doriath>

Luiz Capitulino wrote:
>
>> #define QERR_DEVICE_ALREADY_OPEN "{'class': 'DeviceAlreadyOpen', 'data' 
>> : {'bus_num': %d, 'addr': %d}"
>>
>> qemu_error_new(QERR_DEVICE_ALREADY_OPEN, bus_num, addr);
>>     
>
>  What about DeviceAlreadyOpen errors with a different argument list?
>   

Why would you have this?  That would seem like a problem to me.  I think 
the errors need to be very well structured (just like everything else in 
QMP).

>> That gives us a nice simple interface with full error checking on the 
>> parameters.
>>     
>
>  I've said this is not so simple because people writing those macros
> would find out that the 'class' or 'data' _keys_ are missing or incorrect
> only at run-time, when the error is triggered.
>   

Sure but introducing new types of errors is not the common case.  Using 
existing errors is the common case.

>> For human readable strings, I'd suggest making a table somewhere else 
>> that looked like:
>>
>> QErrorStringTable qerror_descriptions[] = {
>> { QERR_DEVICE_ALREADY_OPEN, "This device at %(bus_num)d.%(addr)d is 
>> already open." },
>> ...
>> };
>>     
>
>  How do you suggest we lookup the table? Doing a strcmp() on
> QERR_DEVICE_ALREADY_OPEN?
>   

We can either change the index on the table to be just the class code or 
find something more clever.

>> There are a number of advantages to an approach like this.  The table 
>> can be reused by both in the server and by a client.
>>     
>
>  My suggestions on both problems makes me willing go back to my initial
> series, which had a table indexed by an error number.
>   

I don't understand why.

-- 
Regards,

Anthony Liguori

  reply	other threads:[~2009-11-12 14:44 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-11-04 20:03 [Qemu-devel] [RFC 0/8]: QError v2 Luiz Capitulino
2009-11-04 20:04 ` [Qemu-devel] [PATCH 1/8] QJSon: Introduce qobject_from_json_va() Luiz Capitulino
2009-11-04 20:04 ` [Qemu-devel] [PATCH 2/8] QString: Introduce qstring_append_chr() Luiz Capitulino
2009-11-04 20:04 ` [Qemu-devel] [PATCH 3/8] Add qstring_append_chr() unit-test Luiz Capitulino
2009-11-04 20:04 ` [Qemu-devel] [PATCH 4/8] QString: Introduce qstring_append_int() Luiz Capitulino
2009-11-04 20:04 ` [Qemu-devel] [PATCH 5/8] Introduce QError Luiz Capitulino
2009-11-05  9:34   ` [Qemu-devel] " Paolo Bonzini
2009-11-04 20:04 ` [Qemu-devel] [PATCH 6/8] monitor: QError support Luiz Capitulino
2009-11-04 20:04 ` [Qemu-devel] [PATCH 7/8] qdev: Use QError for not found error Luiz Capitulino
2009-11-04 20:04 ` [Qemu-devel] [PATCH 8/8] monitor: do_info_balloon(): use QError Luiz Capitulino
2009-11-11 21:20 ` [Qemu-devel] [RFC 0/8]: QError v2 Anthony Liguori
2009-11-11 23:35   ` Luiz Capitulino
2009-11-12 13:41   ` Luiz Capitulino
2009-11-12 14:44     ` Anthony Liguori [this message]
2009-11-12 15:10       ` Luiz Capitulino
2009-11-12 15:26         ` Anthony Liguori
2009-11-18 15:53       ` 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=4AFC1F33.4060405@linux.vnet.ibm.com \
    --to=aliguori@linux.vnet.ibm.com \
    --cc=aliguori@us.ibm.com \
    --cc=hollisb@linux.vnet.ibm.com \
    --cc=kraxel@redhat.com \
    --cc=lcapitulino@redhat.com \
    --cc=pbonzini@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.