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@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list