[libvirt] [PATCH] Improve error reporting in virsh

[Daniel P. Berrange]

On Thu, Feb 05, 2009 at 08:37:29PM +0000, Daniel P. Berrange wrote:
> On Thu, Feb 05, 2009 at 06:09:10PM +0000, John Levon wrote:
> > 
> > In contrast, "only set an error if one isn't set already" is simple,
> > immediately understandable, and covers almost all the common cases.
> 
> I'm not totally against this idea, but with a few caveats.
> 
> First, we'd need to make some changes to the libvirtd  because it calls
> into a number of APIs, both public entry points & some of the internal
> helpers directly, but does not reset the error in betweeen these calls.
> Of course it also doesn't actually do anything with errors that these
> calls may raise - they just happen to be printed on stderr. If we only
> set when not already set, then most won't even appear on stderr in
> the libvirtd context. Not sure of the best / least error prone way to 
> fix this really. 
> 
> I'd still like to try and find a way to make the error raising process
> more robust. The idea of having calls to virRaiseError, which may or
> may not raise an error, depending on whether the thing we just called
> remembered to raise an error is not a great situation to be in. 
> 
> As you point out it is pretty hard to get this right and/or identify
> places where it is wrong. As a starting point, though adding code to
> virRaiseError which prints a stack trace to stderr() whenever it is
> called with an error already set, would help our awareness. Obviously
> this would only want to be turned on in dev-builds.
> 
> Given my experiance with the OCAML/CIL stuff on threading, I think it
> might also be practical to perform static analysis of the callpaths
> and identify a fairly large proportion of flaws this way. I think it
> would be possible to just track exit paths, and keep note of whether
> there are any virRaiseError calls in each method, and match against
> call locations. Ok, not easy, but I'll have a stab at this next time
> I have a spare moment.

Opps, missed the final one I should have  mentioned. The driver open
process is actually quite annoying wrt this particular problem, because
we explicitly expect a number of calls to fail during probing of drivers.
If any of those raise errors, we do explicitly want to override them
in many cases. This could possibly be addressed by just adding an explicit
call to virResetError at the point we wish to override the early error.

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 :|