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

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.