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 -=|