[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 7/9] qdev: Use QError for not found error
From: |
Daniel P. Berrange |
Subject: |
Re: [Qemu-devel] [PATCH 7/9] qdev: Use QError for not found error |
Date: |
Mon, 19 Oct 2009 11:12:41 +0100 |
User-agent: |
Mutt/1.4.1i |
On Wed, Oct 14, 2009 at 12:34:21AM +0200, 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) {
> Now let's look at errors from the client's perspective.
>
> Clients need to classify errors to figure out how to respond to them.
> Something like client error (i.e. your command was stupid, and trying
> again will be just as stupid), permanent server error (it wasn't stupid,
> but I can't do it for you), transient server error (I couldn't do it for
> you, but trying again later could help).
>
> Some classical protocols (HTTP, FTP) provide error class (they cleverly
> encode it into the error code). This gives clients a chance to sanely
> handle errors they don't know.
>
> Even with error class figured out, clients may still be interested in
> the exact error code, at least in some cases.
>
> They may also be interested in a human readable description of the
> error, to present to their human users. Some classical protocols
> provide that in addition to an error code. This gives clients a chance
> to sanely report errors they don't know to human users.
>
> I suspect that the additional error data is the error's least
> interesting part most of the time. When we get QERR_QDEV_NFOUND, I
> figure it's usually clear what device we were trying to find.
>
> But these are just my educated guesses. I'd love to hear what folks
> involved with actual clients have to say. Anyone from libvirt?
I think just returning error codes to the client is far too little
information. I don't think we need the fully normalized structure
that Luiz originally proposed with bus/dev addresses split out, but
we certainly need to include a string description giving as much
detail as possible. If attaching a host USB device failed, I don't
want a single error code QERR_OPEN_FAILED, nor a generic message
like 'could not open device', i want the exact details, eg
'could not open device: permission denied'
'could not open device: no such file or directory'
'could not open device: device or resource busy'
The localization issue is somewhat of a red herring though. This string
description is not something that clients would ultimately expose to the
user. The client apps would likely present a more generic error message
to the user, based off the error code. The error description received
from QEMU will instead be written out to the logs as a record for later
troubleshooting. eg if the user files a bug report, it will include the
full error details, so libvirt/qemu maintainers have a better chance of
figuring out what went wrong.
So ultimately these error messages are for developers, not users benefit.
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
[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 <=
- 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
[Qemu-devel] [PATCH 9/9] monitor: do_info_balloon(): use QError, Luiz Capitulino, 2009/10/13
Re: [Qemu-devel] [PATCH v0 0/9] QError, Anthony Liguori, 2009/10/15