On Wed, Oct 08, 2008 at 02:17:45PM -0400, Cole Robinson wrote:
The attached patch is a more complete cut at unifying
error reporting among the different source files.
Most files have their own custom Error function, each
with varying degrees of functionality, and lots of
code duplication.
The attached patch adds a helper function to virterror.c
that centralizes all this logic, and redefines each
local error function as a macro that calls out to
the helper. The fixed files now offer printf style
reporting, and the macros pass off function name,
file name, and line number of the reported error
(currently not used, but handy to have).
This change centralizes probably 90% of the error calls.
Some files have pretty custom error functions that
don't map easily to the helper, or call __virRaiseError
directly in a number of places, so I skipped these for
now. Eventually we should move all these edge cases over
so any src/ wide error changes will be pretty easy to
make.
Ok.
The one thing we lose here is that some some places
were logging info in the err{1,2,3} and int{1,2} fields
of raised error. I don't consider this a loss: even
where it was used, it was used inconsistently and
rarely for anything that would be useful to a user.
In the places where the data had some value, I
included it in the error string.
Agreed - it had no documented use either.
One side question: is src/xmlrpc.{c,h} even used? And
if not, can it be removed?
It is not used at this time. It was for the hypothetical
alternative Xen driver which talks XML-RPC which we never
got around to writing. Perhaps it really is time to kill
it -we can get it back from CVS if ever required.
ACK to this patch
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 :|