Richard W.M. Jones
2014-May-16 15:44 UTC
Re: [Libguestfs] [PATCH] generator: c.ml - wrap non deamon function with recursive mutex
On Thu, May 15, 2014 at 05:39:08PM +0200, mzatko@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. Rich.> + pr "\n"; > + > pr "#endif /* GUESTFS_INTERNAL_ACTIONS_H_ */\n" > > (* Generate guestfs-internal-frontend-cleanups.h file. *) > @@ -1567,6 +1574,8 @@ and generate_client_actions hash () > c_name style; > pr "{\n"; > > + pr " gl_recursive_lock_lock (global_lock);\n"; > + > handle_null_optargs optargs c_name; > > pr " int trace_flag = g->trace;\n"; > @@ -1617,6 +1626,9 @@ and generate_client_actions hash () > trace_return name style "r"; > ); > pr "\n"; > + > + pr " gl_recursive_lock_unlock (global_lock);\n"; > + > pr " return r;\n"; > pr "}\n"; > pr "\n" > -- > 1.8.5.3 > > _______________________________________________ > Libguestfs mailing list > Libguestfs@redhat.com > https://www.redhat.com/mailman/listinfo/libguestfs-- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-builder quickly builds VMs from scratch http://libguestfs.org/virt-builder.1.html
Daniel P. Berrange
2014-May-16 15:47 UTC
Re: [Libguestfs] [PATCH] generator: c.ml - wrap non deamon function with recursive mutex
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@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 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 :|
Richard W.M. Jones
2014-May-16 15:59 UTC
Re: [Libguestfs] [PATCH] generator: c.ml - wrap non deamon function with recursive mutex
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@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-entrancyActually 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? Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into KVM guests. http://libguestfs.org/virt-v2v