[libvirt] [PATCH 0/6] Add internal APIs for disabling error reporting

[Daniel P. Berrange]

On Mon, Nov 09, 2009 at 08:29:04AM -0500, Cole Robinson wrote:
> On 11/09/2009 07:57 AM, Daniel P. Berrange wrote:
> > On Thu, Nov 05, 2009 at 01:17:41PM -0500, Cole Robinson wrote:
> >> In testing migration, I was hitting an error in the Perform step, but this
> >> was being returned as 'Unknown Error' via virsh. The reason is that even
> >> a failed Perform will call MigrateFinish to do cleanup, but Finish will
> >> always return an NULL == error in that case, overwriting the original message.
> >>
> >> I've added internal APIs which allow temporarily disabling error reporting:
> >> any ReportError calls will log the error message, but will not overwrite the
> >> previous error and will not trigger the error callback. These APIs are then
> >> used in a few places where they are needed.
> > 
> > I don't particularly like this as a concept. Could we do this the other
> > way around, whereby we call  virSaveLastError() before the bit of code
> > which might clobber the error we want, and then add a new method to set
> > it back afterwards.
> > 
> > Daniel
> 
> Tried that originally. Problem is it doesn't easily work for virsh. If a
> libvirt call errors, virsh doesn't print the last error from GetLastError, it
> prints the last error that triggered the error callback. So for the end result
> to be reached, we would need to fire error callback twice for the same msg:
> once when the msg is raised, again when we reraise it after it was squashed.
> That would muddle up debug output at least.

That's actually a flaw in the way we trigger the error callback. In the
current code we fire the callback the momnent virRaiseError is called
internally. This is bad because internal code may reset the error if it
doesn't want it propagated. It is also bad because the callpath may be
holding active mutex locks which can then result in deadlock if the
error callback impl from the app calls back into libvirt. 

We need the triggering of the error callback to be the absolute last
thing done in the API call, which means it really needs to be triggered
explicitly from the entry points in src/libvirt.c.  I started work on 
this a while back, but then got side-tracked so never finished it

http://gitorious.org/~berrange/libvirt/staging/commits/error-callbacks


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