Re: [Libguestfs] [PATCH] generator: c.ml - wrap non deamon function with recursive mutex
On Fri, May 16, 2014 at 04:59:54PM +0100, Richard W.M. Jones wrote:
On Fri, May 16, 2014 at 11:47:17AM -0400, Daniel P. Berrange wrote:
> On Fri, May 16, 2014 at 04:44:41PM +0100, Richard W.M. Jones wrote:
> > On Thu, May 15, 2014 at 05:39:08PM +0200, mzatko(a)redhat.com wrote:
> > > +
> > > + pr "gl_recursive_lock_define_initialized(static,
global_lock)\n";
> >
> > static?
> >
> > I suspect this only allows one libguestfs handle per process into the
> > critical section at once. I think the lock needs to be per-handle
> > unless I'm misunderstanding what this is for.
>
> Agreed, any locking should be per-handle, and I wouldn't expect to need
> to use recursive mutexes either. Internal libguestfs code shouldn't be
> calling back out to the public libguestfs API surely, so shouldn't need
> to have re-entrancy
Actually libguestfs does call itself quite a lot. Whether it should
call itself through the public API or not is another matter -- we
could change that fairly easily. Is there a problem with recursive
mutexes?
Not a technical issue, rather its more of a philosophical issue. IME, if
you permit recursive mutexes, it allows people to create a tangled mess
of interdepedant/recursive API calls which can become quite hard to
understand over time. Non-recursive mutexes help to force cleaner code
separation.
Of course if libguestfs already calls itself reentrantly via its public
API entry points, the issue is moot, unless you decide you want to spend
the time to refactor it.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|