Re: [libvirt] [PATCH] Improve error reporting in virsh

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.