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

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

 



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@xxxxxxxxxx    +1-919-301-3266
Libvirt virtualization library http://libvirt.org



Attachment: signature.asc
Description: OpenPGP digital 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]