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