[libvirt] [PATCH] Unify most error reporting (ver 2)

[Daniel P. Berrange]

On Fri, Oct 10, 2008 at 11:47:49AM +0200, Jim Meyering wrote:
> Daniel Veillard <veillard at redhat.com> wrote:
> 
> > On Thu, Oct 09, 2008 at 11:44:02AM -0400, Cole Robinson wrote:
> >> Daniel Veillard wrote:
> >> > On Wed, Oct 08, 2008 at 08:35:38PM +0100, Daniel P. Berrange wrote:
> >
> >   Hum, this breaks "make check"
> >
> > PASS: read-non-seekable
> > --- out 2008-10-10 10:23:57.000000000 +0200
> > +++ exp 2008-10-10 10:23:57.000000000 +0200
> > @@ -1,2 +1,2 @@
> > -libvir: Test error : internal error Domain is still running
> > +libvir: Test error test: internal error Domain is still running
> >  error: Failed to undefine domain test
> >  FAIL: undefine
> >
> > Seems we loose the domain name in the error message when trying to
> > undefine a running domain:
> >
> > paphio:~/libvirt/tests -> /usr/bin/virsh -q -c test:///default undefine
> > test
> > libvir: Test error test: internal error Domain is still running
> > error: Failed to undefine domain test
> > paphio:~/libvirt/tests -> ../src/virsh -q -c test:///default undefine
> > test
> > libvir: Test error : internal error Domain is still running
> > error: Failed to undefine domain test
> > paphio:~/libvirt/tests ->
> >
> > I should be possible to restore the old behaviour of reporting the domain
> > name there
> 
> The documentation suggests that the new interface was initially
> supposed to have dom and net parameters:

We removed them because they're deprecated and 90% of the driver code 
never provides them - the test driver was the exception to the rule here

> 
> /**
>  * __virReportErrorHelper
>  *
>  * @conn: the connection to the hypervisor if available
>  * @dom: the domain if available
>  * @net: the network if available
>  * @domcode: the virErrorDomain indicating where it's coming from
> ...
> void __virReportErrorHelper(virConnectPtr conn, int domcode, int errcode,
>                             const char *filename ATTRIBUTE_UNUSED,
>                             const char *funcname ATTRIBUTE_UNUSED,
>                             long long linenr ATTRIBUTE_UNUSED,
>                             const char *fmt, ...)
> {
> ...
>     virerr = __virErrorMsg(errcode, (errorMessage[0] ? errorMessage : NULL));
>     __virRaiseError(conn, NULL, NULL, domcode, errcode, VIR_ERR_ERROR,
>                     virerr, errorMessage, NULL, -1, -1, virerr, errorMessage);
> }
> 
> Shouldn't be hard to make the code agree with the documentation.
> There are fewer than 20 uses of __virReportErrorHelper so far.

That's not really solving the problem because very little code passes
in a non-NULL dom/net parameter.

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