[libvirt] [PATCH] Free object after *Destroy in python bindings

Cole Robinson crobinso at redhat.com
Tue May 20 18:43:42 UTC 2008


Daniel P. Berrange wrote:
> On Mon, May 19, 2008 at 05:17:07PM -0400, Cole Robinson wrote:
>> Daniel P. Berrange wrote:
>>> On Mon, May 19, 2008 at 04:21:55PM -0400, Cole Robinson wrote:
>>>> Cole Robinson wrote:
>>>>> The patch below fixes an issue in the python bindings with the
>>>>> vir*Destroy methods. After the object is successfully destroyed,
>>>>> the payload is cleared, using 'self._o = None'. This unfortunately
>>>>> screws up virt object reference counts, as the payload should be 
>>>>> free'd using the appropriate vir*Free function.
>>>>>
>>>> Hmm, I might be wrong about this. Reading the virDomainDestroy
>>>> description in libvirt.c, destroy is supposed to free the
>>>> domain object if the operation is successful. The qemu driver
>>>> currently does not do this. In fact, from what I gather, none
>>>> of the drivers do this.
>>> The docs are wrong. Destory merely hard-kills the object being managed.
>>> It does not free memory associated with the object.
>>>
>>>> If the virDomainDestroy comment is correct, then the original
>>>> python statement is sufficient, but the drivers need to have
>>>> a few virDomainFree's added.
>>> I believe your patch is correct - python should not be dropping its 
>>> reference to the underlying C object, afte invoking the destroy method.
>>> It should be only be dropped after free is called
>> I guess it should also be asked if the python bindings should be doing
>> anything to the domain/network/... object after a destroy _at_all_.
> 
> I think destroy should work in same way as any other operation. The 
> only method call which is special is the virDomainFree().
> 
>> Shutdown and reboot don't alter the object, even undefine doesn't. The
>> original statement just seems to be compensating for the original idea
>> that Destroy was freeing the underlying object.
>>
>> Taking the statement out entirely would be a change in existing behavior,
>> but wouldn't break any existing use of the bindings and would most likely
>> prevent more memory leaks, since the automatic garbage collection would
>> now actually be able to free the object appropriately.
> 
> It would actually resolve bugs if we fixed the python objects behaviour
> here. As you say the garbage collector will then 'do the right thing'
> so it should not have any negative impact on apps to fix this issue.
> 
> Dan

Here is the correct patch then: remove any special case cleanup for the
destroy functions.

Thanks,
Cole
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: libvirt-python-destroy-cleanup-2-patch
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20080520/f5b9b283/attachment-0001.ksh>


More information about the libvir-list mailing list