[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] Re: [PATCH 7/9] qdev: Use QError for not found error
From: |
Luiz Capitulino |
Subject: |
[Qemu-devel] Re: [PATCH 7/9] qdev: Use QError for not found error |
Date: |
Wed, 14 Oct 2009 13:42:35 -0300 |
On Wed, 14 Oct 2009 15:29:41 +0200
Paolo Bonzini <address@hidden> wrote:
> On 10/14/2009 12:34 AM, Markus Armbruster wrote:
> > Luiz Capitulino<address@hidden> writes:
> >
> >> Signed-off-by: Luiz Capitulino<address@hidden>
> >> ---
> >> hw/qdev.c | 4 ++--
> >> 1 files changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/hw/qdev.c b/hw/qdev.c
> >> index 906e897..3ce48f7 100644
> >> --- a/hw/qdev.c
> >> +++ b/hw/qdev.c
> >> @@ -28,6 +28,7 @@
> >> #include "net.h"
> >> #include "qdev.h"
> >> #include "sysemu.h"
> >> +#include "qerror.h"
> >> #include "monitor.h"
> >>
> >> static int qdev_hotplug = 0;
> >> @@ -176,8 +177,7 @@ DeviceState *qdev_device_add(QemuOpts *opts)
> >> /* find driver */
> >> info = qdev_find_info(NULL, driver);
> >> if (!info) {
> >> - qemu_error("Device \"%s\" not found. Try -device '?' for a
> >> list.\n",
> >> - driver);
> >> + qemu_error_structed(QERR_QDEV_NFOUND, "{ s: s }", "name", driver);
> >> return NULL;
> >> }
> >> if (info->no_user) {
> >
> > And here we pay the price for structured error objects: reporting an
> > error becomes more difficult. The harder you make reporting errors, the
> > fewer errors get caught and reported. Also, the result is harder to
> > read.
>
> If you count also the new code in 6/9, reporting an error definitely
> becomes harder. Finding a middle ground would be great, and I think
> that the right solution is to put as many things as possible in a single
> place. For example:
>
> 1) there is a lot of code that is duplicated in every occurrence of an
> error. One possibility to avoid this, would be to use a more
> printf-like syntax for qemu_object_from_va, for example one of these:
>
> .... "{ name: %s }", driver);
> .... "{ 'name': %s }", driver);
>
> Then you can move the first argument to the error table:
>
> + .code = QERR_QDEV_NFOUND,
> + .desc = "device not found",
> + .keys = "{ name: %s }"
>
> so that the qemu_error_structed call looks like
>
> qemu_error_structed (QERR_QDEV_NFOUND, driver);
>
> 2) do we need full flexibility of a function for printing errors? What
> about adding a print-from-qobject function, so that you could replace
> qerror_table[].user_print with a string like
>
> .formatted_print = "Device \"%{name}\" not found. Try -device"
> " '?' for a list."
I like both, but I see them as future improvements.
> 3) I don't think this technique is used in qemu, but what about putting
> all errors in a header file like
We have qemu-monitor.hx, which is something like it.
> QEMU_ERROR(QERR_DEV_NFOUND,
> "device not found",
> whatever);
>
> Then you can use the header file to generate both the enum and the error
> table:
>
> typedef enum QErrorCode {
> #define QEMU_ERROR(code_, ...) code_,
> #include "qemu.def"
> #undef QEMU_ERROR
> } QErrorCode;
>
> ...
>
> static QErrorTable qerror_table[] = {
> #define QEMU_ERROR(code_, desc_, data_) \
> { .code = (code_), .desc = (desc_), .data = (data_),
> #include "qemu-error.def"
> #undef QEMU_ERROR
> };
>
> This has the disadvantage of not allowing usage of designated initializers.
Not sure if it really helps.
[Qemu-devel] [PATCH 7/9] qdev: Use QError for not found error, Luiz Capitulino, 2009/10/13
- Re: [Qemu-devel] [PATCH 7/9] qdev: Use QError for not found error, Markus Armbruster, 2009/10/13
- Re: [Qemu-devel] [PATCH 7/9] qdev: Use QError for not found error, Luiz Capitulino, 2009/10/14
- Re: [Qemu-devel] [PATCH 7/9] qdev: Use QError for not found error, Daniel P. Berrange, 2009/10/19
- Re: [Qemu-devel] [PATCH 7/9] qdev: Use QError for not found error, Gerd Hoffmann, 2009/10/19
- Re: [Qemu-devel] [PATCH 7/9] qdev: Use QError for not found error, Daniel P. Berrange, 2009/10/19
- [Qemu-devel] Re: [PATCH 7/9] qdev: Use QError for not found error, Paolo Bonzini, 2009/10/19
Re: [Qemu-devel] [PATCH 7/9] qdev: Use QError for not found error, Anthony Liguori, 2009/10/19
Re: [Qemu-devel] [PATCH 7/9] qdev: Use QError for not found error, Daniel P. Berrange, 2009/10/19
Re: [Qemu-devel] [PATCH 7/9] qdev: Use QError for not found error, Anthony Liguori, 2009/10/19
Re: [Qemu-devel] [PATCH 7/9] qdev: Use QError for not found error, Daniel P. Berrange, 2009/10/19
[Qemu-devel] [PATCH 8/9] QError: Add do_info_balloon() errors, Luiz Capitulino, 2009/10/13