[libvirt] PATCH: Thread-safe error reporting public API

[Daniel Veillard]

On Fri, Dec 12, 2008 at 10:51:33AM +0000, Daniel P. Berrange wrote:
> On Fri, Dec 12, 2008 at 09:46:14AM +0100, Daniel Veillard wrote:
> > On Thu, Dec 11, 2008 at 08:16:36PM +0000, Daniel P. Berrange wrote:
> > > The current public API for error reporting consists of 
> > > 
> > >  - A global error object
> > >  - A per-connection error object
> > > 
> > > Some functions always set errors on the global object. Other functions
> > > always set errors on the per-connection object, except when they set 
> > > errors on the global object. Both have built-in race conditions if they
> > > are accessed from multiple threads because of the time between the API
> > > call raising the error, and the caller querying it.
> > > 
> > > The solution to this is to do away with all of the existing error objects
> > > and replace them with a per-thread error object. Well, except we can't
> > > do away with existing objects because of ABI compatability. This turns
> > > out to not be such a bad problem after all....
> > > 
> > > So with this patch...
> > > 
> > > 
> > > virterror.c gets ability to track a per-thread virErrorPtr instance. This
> > > object is stored in a thread local variable via pthread_{get,set}specific.
> > > It is allocated when first required, and deleted when the thread exits
> > > 
> > > Every single API will *always* set the  virErrorPtr object associated with
> > > its current thread.
> > > 
> > > In the public virterror.h we add
> > > 
> > >   virErrorPtr             virThreadGetLastError (void);
> > >   int                     virThreadCopyLastError (virErrorPtr to);
> > >   void                    virThreadResetLastError (void);
> > > 
> > > This provides a guarenteed thread-safe public API for fetching error
> > > information. All the other existing APIs have docs updated to recommend
> > > against their use.
> > 
> >   I understand the problem but I don't understand the way you expect to
> > fix it. Historically this copies the libxml2 error APIs, but in libxml2
> > the error data are stored in thread safe local storage. Until now
> > basically libvirt could not be used in a threaded way or at least not
> > without lot of constraints. Now we are getting thread safe, I suggest to
> > just retrofit thread safety into the exiting API, since they have the
> > exact same signature I see no reason to annoy the application writer
> > with extra API and deprecated ones.
> >   If the application is single threaded nothing changes for them in
> >   practice.
> >   If the application is multi-threaded the old behaviour wasn't making
> >   sense, could not be trusted and the new code does basically "the
> >   right thing" now.
> 
> 
> Yes, I guess that does atually make sense.
> 
> So I'll make the global virGetLastError() thread-safe, and then there's
> no need for the new APIs, and also no need for anyone to call the
> virConnGetLastError(), though I'll make sure that still syncs to 
> whatever error is stored in the global location anyway.

  okay, cool !

> >   I guess saying "from 0.6.0 the API becomes thread-safe and as a result
> > the error data become thread specific" sounds just fine to me, and that
> > probably works better for most applications (and bindings).
> 
> Sounds like a good plan.

  I guess there won't be any 0.5.2 and we will jump to 0.6.0 directly,
I would expect a release toward the end of the month, I see the
ChangeLog growing dangerously fast ...

> With this error stuff done and the other patches I've posted this week
> all the major thread-safety problems are addressed. Its now a case of
> tracking down all the loose-ends / minor faults. The Xen driver will
> be easy enough todo - its mostly stateless apart from the async events
> which is easy . Then there's the strerror_r() stuff and other related
> _r() functions. And whatever other edge-cases i discover.

  As long as we can keep the existing APIs and clean everything up
behind the scene, I guess we can consider ourselves lucky !

   thanks !

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel at veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/