Re: [libvirt] PATCH: Ensure errors are guarenteed reported in virConnectOpen

On Wed, Aug 20, 2008 at 07:24:59PM +0100, Daniel P. Berrange wrote: > On Wed, Aug 20, 2008 at 02:22:58PM +0100, Richard W.M. Jones wrote: > > On Tue, Aug 19, 2008 at 11:35:18AM +0100, Daniel P. Berrange wrote: > > > The guarenteed correct solution is actually rather simple > > > > > > - Always report errors against the virConnect object, even in the driver > > > open method > > > > > > - In the cleanup patch of do_open() in libvirt.c, if no global error is > > > set, copy the error from the virConnect object's virError, into the > > > global virError. > > > > +1 , although unfortunately this isn't thread-safe. Nothing can be > > thread-safe unless we change the API. > > Well we're not making it any less thread-safe. You alrady have to use the > virGetLastError() global func - we're simply making sure it actually > records all errors. > > To make it thread-safe we'll need to add a real virGetThreadLastError() > API, which is something on my todo list - with that new apps can just > call thevirGetThreadLastError() exclusively and never need to know the > distinction between global/connection errors which causes so much > trouble. I'm fairly sure I can preseve existing semantics at same > time with some suitable internal cleverness. I really wished we could avoid thread local storage mess, and in general anything having to do with API exported global variables. In general (I mean for the vast majority of the userland code dealing with libvirt) there is always a domain or connection object where we can plug the error, and provide it in-context. The only exception is the connection creation, maybe this means we need to provide a better entry point for this, but I would really prefer to not expose the notion of thread at the API level.