[libvirt] [PATCHv2 10/13] list: provide python bindings for snapshots

[Eric Blake]

On 06/18/2012 08:22 AM, Peter Krempa wrote:
> On 06/15/12 06:18, Eric Blake wrote:
>> This adds support for the new virDomainListAllSnapshots (a domain
>> function) and virDomainSnapshotListAllChildren (a snapshot function)
>> to the libvirt-python bindings.  The implementation is done manually
>> as the generator does not support wrapping lists of C pointers into
>> python objects.
>>

>> +    for (i = 0; i < c_retval; i++) {
>> +        if ((pyobj_snap = libvirt_virDomainSnapshotPtrWrap(snaps[i]))
>> == NULL ||
>> +            PyList_SetItem(py_retval, i, pyobj_snap) < 0) {
> 
> Sadly, this code pattern doesn't save us from leaking memory: The
> PyCapsule objects, that are used to wrap libvirt pointers lack a
> destructor, so if this whole function fails, calling Py_DECREF() on the
> resulting output list doesn't save us from leaking all the processed
> references that were stored in the List. Later on, when the list of
> PyCapsules is converted into actual python-libvirt objects, those
> objects contain destructors that dispose the pointers properly.
> 
> A workaround would be not to NULL members of the snap array and on
> successful end of the loop just set c_retval to 0 so that the cleanup
> loop is not called. But this still is not perfect.
> 
> To fix all possibilities of leaking these pointers, we will need to
> provide destructor callbacks for PyCapsules that wrap libvirt pointers
> and increase the reference count, when the Capsule object is created.

> This does not apply to libvirt_virDomainSnapshotListNames and others as
> strings are wrapped by python wrappers that (I assume) have destructors,
> but does apply on my domain listing bindings.

Sounds like a global cleanup worth doing over multiple affected
functions at once...

> 
> ACK, as the code follows a pattern, but memory leaks are possible yet
> very unlikely.

so I pushed it as-is, leaving the PyCapsule fix for later.

-- 
Eric Blake   eblake at redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org



-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 620 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20120619/3ff75296/attachment-0001.sig>