qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [RFC] Fixing the error failure


From: Daniel P. Berrange
Subject: Re: [Qemu-devel] [RFC] Fixing the error failure
Date: Tue, 26 Jun 2012 10:35:12 +0100
User-agent: Mutt/1.5.21 (2010-09-15)

On Tue, Jun 26, 2012 at 09:54:21AM +0200, Markus Armbruster wrote:
> Luiz Capitulino <address@hidden> writes:
> 
> > On Thu, 21 Jun 2012 13:42:19 +0100
> > "Daniel P. Berrange" <address@hidden> wrote:
> >
> > [...]
> >
> >> > However, we'd change how we use 'desc' and our error classes. 'desc' 
> >> > would
> >> > become a string which is filled by a printf-like function (see section 
> >> > 2) and
> >> 
> >> Good, using a printf-like string for desc is the big change
> >> I wanted to see in QMP errors.
> >> 
> >> > we'd replace all error classes we have today by the following ones:
> >> 
> >> Nooo, that's going a bit to far in simplification....
> >
> > [...]
> >
> >> There is a balance to be struck here - previously we were tending
> >> to invent a new error class for every conceivable error condition.
> >> This proposal meanwhile is swinging too far to the other extreme
> >> having only 4/5 classes. There is a balance to be had here.
> >> 
> >> It is perfectly reasonable, and indeed useful, to have distinct
> >> errors like CommandNotFound, CommandDisabled, and so on.  What
> 
> "Useful" means "users put it to use".  Since we have users, we can work
> with *data* on users' needs rather than hypothesises.
> 
> >> we shouldn't do, however, is do things like invent a new error
> >> for every possible errno value. The examples of  PropertyValueNotFound,
> >> PropertyValueNotPowerOf2, PropertyValueOutOfRange are cases where
> >> we invented too many codes, and in the new world we would do
> >> something like
> >> 
> >>    PropertyValueInvalid
> >>      msg = "Value must be a power of 2"
> >>      msg = "Value must be in range 1-30"
> >>      msg = "Value not found"
> >> 
> >> We have to just use prudent judgement to decide whether to use
> >> an existing error, or create a new one.
> 
> If a user wants to handle a failure mode (or a set of failure modes)
> differently than others, then it needs its own error.
> 
> To get sane errors rather than the byzantine mess we have now, co-evolve
> them with users.
> 
> > This seems a good middle ground too me, but let's elaborate it further.
> >
> > I think what you're proposing boils down to three points:
> >
> >  1. Allow printf-like strings for desc
> 
> I want this badly, because it gives me a fighting chance at emitting
> descriptions fit for human consumption.
> 
> >  2. Avoid creating a new error class for each errno
> 
> Silliness avoidance.
> 
> >  3. Don't create similar error classes
> >
> > Item 3 seems quite obvious, but we've failed doing that and the main reason
> > for that is probably because desc is too strict today, so people end up
> > creating new errors just to have a different desc.
> 
> Yes, that's one reason.  The other reason is following bad examples and
> try to emit the most "complete" error object conceivable, whether users
> need it or not.  Generally results in completely baroque error objects.
> 
> > This means that having a printf-like string will help. I propose having
> > the following function for this:
> >
> >  error_set_fmt(Error **errp, ErrorClass err_class, const char *fmt, ...);
> >
> > Or, we could rename the current error_set() function to 
> > error_set_deprecated()
> > and use the above function only (renamed to error_set()).
> 
> I'm very much in favour of giving the preferred error reporting function
> the most obvious and concise name: error_set().  That means renaming the
> squatter.
> 
> > Also, I believe this fits well with some internal improvements I'm planning
> > for some time now, like having qapi-errors.json (I already have a PoC for 
> > this,
> > but haven't posted it yet).
> >
> > Lastly, regarding item 2, I think we already have consensus that returning:
> >
> >  { "error": "WriteFailed", "data": { "os-error": "eio" },
> >    "desc": "input/output error while writing to '/tmp/forty-two'" }
> >
> > Is better than returning:
> >
> >  { "error": "IOError" }
> 
> It's better only if users actually care for the difference.
> 
> In your example: after EIO, you're generally hosed.  What differences in
> handling read vs. write error do you envisage?
> 
> [Snipped part restored:]
> >> In libvirt we have always reserved the right to change the error
> >> code reported for particular scenarios. So, for example, alot of
> >> our errors started out as "InternalError" (equiv to UndefinedError)
> >> but over time we have changed some to more specialized values
> >> eg "InvalidOperation", "ConfigNotSupported" and so on.
> 
> Do you reserve the right to make changes other than from InternalError
> to a more specific one?

If I'm perfectly honest, then yes. We have tried not to gratuitouslyy
change errors being reported, but they have definitely evolved over
time, so more distinct error scenarios are reported, where previously
many things would have been lumped under one code.

> >> We should probably explicitly note that any use of "UndefinedError"
> >> is liable to be changed in a future QEMU release.
> 
> Makes sense to me.

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|



reply via email to

[Prev in Thread] Current Thread [Next in Thread]