[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [Libvir] Repository for work-in-progress storage patches



On Sat, Jan 19, 2008 at 02:37:51PM +0000, Richard W.M. Jones wrote:
> Daniel P. Berrange wrote:
> >On Fri, Jan 18, 2008 at 07:55:35PM +0000, Daniel P. Berrange wrote:
> >>On Fri, Jan 18, 2008 at 07:44:41PM +0000, Richard W.M. Jones wrote:
> >>>A few comments:
> >>>
> >>>You should put #include <config.h> at the very top of all your C files 
> >>>(before any other headers/defines), to avoid warnings.
> >>>
> >>>There was one place where you'd used virFreeStoragePool, which should be 
> >>>virStoragePoolFree.
> >>>
> >>>Functions such as 'virDomainDestroy' have changed their semantics in 
> >>>that they now free the virDomainPtr object.  I understand why you do 
> >>>this (the domain no longer exists, so the object cannot be used), but it 
> >>>is a big change from the p.o.v. of those of us using proper garbage 
> >>>collection, and moreover it's an ABI change.  Existing correct code 
> >>>which did 'virDomainDestroy (dom); virDomainFree (dom);' could now 
> >>>segfault.
> >>No it isn't an ABI change. The virDomainDestroy method has always
> >>behaved this way. I simply moved the calls to free out of the per
> >>driver destroy method, and into the top level libvirt.c destroy
> >>method to remove the need for drivers to re-implement this every
> >>time.
> >
> >Actually, no I take that back. The qemu driver always did this. The Xen
> >driver did not. So the QEMU driver is actually buggy. I'll remove this
> >free'ing of virDomainPtr from virDomainDestroy and fix the QEMU driver.
> 
> Actually I think I'm getting confused now.
> 
> The current (in CVS) documentation for virDomainDestroy & 
> virNetworkDestroy documents that they free the object.  So bug in the 
> Xen driver instead?

Well, personally I'll take actual application visible behaviour over
docs. The behaviour of the QEMU driver in free'ing the object is not 
exposed to any apps, since QEMU driver is only acessed by the daemon,
and apps get to it indirectly via the remote driver. So if neither
the remote driver, nor Xen driver  free the objects that is a defacto
API contract regardless of docs.

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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]