[Libvir] PATCH: Fix and cleanup ref counting/ domain/network object release

[Daniel P. Berrange]

On Mon, Jan 21, 2008 at 03:54:49AM -0500, Daniel Veillard wrote:
> On Sat, Jan 19, 2008 at 07:18:03PM +0000, Daniel P. Berrange wrote:
> > The referencing counting code for Connect/Domain/Network objects has many
> > repeated codepaths, not all of which are correct. eg, the virFreeDomain 
> > method forgets to release networks when garbage collecting a virConnectPtr,
> > and the virFreeNetwork method forgets to release domains. I've also found 
> > it very confusing having both virFreeDomain and virDomainFree.
> > 
> > So I've changed the impl of the cleanup functions in hash.c to use a 
> > slightly different structure. There are now 6 functions:
> > 
> >   - virUnrefConnect
> >   - virReleaseConnect
> >   - virUnrefDomain
> >   - virReleaseDomain
> >   - virUnrefNetwork
> > 
> > The 'virUnref*' APIs are intended for use by the drivers to decrement the
> > ref count on objects they no loner need. In the virUnref* method, if the
> > ref count hits zero, then it calls the corresponding virRelease* method
> > to actually free the object's memory.  The virUnref* methods are responsible
> > for acquiring the connection mutex. The virRelease* method mandate that5D
> > the mutex is already held, and will releae it as the very last step when
> > deleting the object.  The virReleaseDomain/virReleaeNetwork methods will
> > also derecement the ref count of the connection, and call virReleaseConnection
> > if appropriate.  The patch for all this looks horrific but its not as bad as
> > it looks, so I'd recommend reviewing the final code in hash.c, rather than 
> > the patch.
> 
>   Sounds okay,
> 
> > I have also switched from xmlMutex to the POSIX standard pthread_mutex_t 
> > object. In theory the former may give more portability, but the patches which
> > follow are also going to require actual pthread_t objects, for which libxml 
> > has no wrappers. Thus it is pointless using the xmlMutex abstraction. The 
> > added plus, is that pthread_mutex_t objects do not require any memory 
> > allocation, merely initialization of their pre-allocated contents so it 
> > simplifies a little code.
> 
>   That I found worrying. What is the added benefit for the loss of 
> portability? You now reference 2 relatively system specific kind of
> data structure, where we used to reference only the xmlMutex one which
> was portable. And i don't see why, you need to do this for the goal stated
> before.

The problem is that we need to use pthread_t / pthread_create APIs for
other parts of libvirt. libxml2 doesn't provide any portability layer
for threads - only for mutexes. Any platform which has pthread_t, already
has pthread_mutex_t, so although in theory the pthread_mutex_t is less
portable than xmlMutex, in practice the overall portability is the same
due to the constraint of needing pthread_t.

Regards,
Dan.
-- 
|=- Red Hat, Engineering, Emerging Technologies, Boston.  +1 978 392 2496 -=|
|=-           Perl modules: http://search.cpan.org/~danberr/              -=|
|=-               Projects: http://freshmeat.net/~danielpb/               -=|
|=-  GnuPG: 7D3B9505   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505  -=|