Re: Repository for work-in-progress storage patches

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

 



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?

Rich.

--
Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/
Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod
Street, Windsor, Berkshire, SL4 1TE, United Kingdom.  Registered in
England and Wales under Company Registration No. 03798903

Attachment: smime.p7s
Description: S/MIME Cryptographic Signature

--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]