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

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

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

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.


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