[Qemu-devel] Possible reference leak in device_set_realized(...)

[Qemu-devel] Possible reference leak in device_set_realized(...)

[Ilya Lesokhin]

[-- Attachment #1: Type: text/plain, Size: 1192 bytes --]

Hi,
I’m working on SRIOV support for VFIO and I’m suffering from a reference leak.
I’m using Knut Omang’s patches for SRIOV[1].

When the VF’s are enabled I call
pci_create(…) and then object_property_set_bool(OBJECT(&dev->qdev), true, "realized", &local_err);
and when the VF’s are disabled I call
object_property_set_bool(OBJECT(&dev->exp.sriov_pf.vf[i]->qdev), false, "realized", &local_err);
Following that sequence of events, the VFIO instance_finalize function is never called.

It seems that the leaked reference is created by object_property_add_child(…) which called by device_set_realized(…)
When the realized property is set.
Looking at the code of device_set_realized(…) I don’t see anything that might remove the reference taken by
object_property_add_child(…), when realized is set to false.

Does anyone know how this reference is supposed to be released?
Is object_finalize_child_property(…) supposed to be triggered somehow?

I was able to overcome this issue by calling object_unparent on my device but I’m not sure that the correct way of fixing it.

Thanks,
Ilya

[1] https://github.com/knuto/qemu/tree/sriov_patches_v6

[-- Attachment #2: Type: text/html, Size: 4436 bytes --]

Re: [Qemu-devel] Possible reference leak in device_set_realized(...)

[Paolo Bonzini]

On 31/12/2015 19:13, Ilya Lesokhin wrote:
> I was able to overcome this issue by calling object_unparent on my
> device but I’m not sure that the correct way of fixing it.

Yes, it's definitely the right way to fix it.

Paolo

Re: [Qemu-devel] Possible reference leak in device_set_realized(...)

[Knut Omang]

On Fri, 2016-01-01 at 23:37 +0100, Paolo Bonzini wrote:
> 
> On 31/12/2015 19:13, Ilya Lesokhin wrote:
> > I was able to overcome this issue by calling object_unparent on my
> > device but I’m not sure that the correct way of fixing it.
> 
> Yes, it's definitely the right way to fix it.

Sorry for the late follow-up on this one, but I had to find some more
time to spend with the code (and with valgrind too) to understand
better/verify what was going on in the qdev/qom layers.

In the SR/IOV patch the object is created by pci_create.
Since there is no corresponding pci_delete, I assume this means that
the correct way to clean up from pci_create is simply a call to
object_unparent() as you indicate, and this is what is missing from the
patch set.

So the full setup/teardown sequence per VF then becomes:

pci_create(...)
<realize>

<unrealize>
object_unparent(...)


Thanks,
Knut

Re: [Qemu-devel] Possible reference leak in device_set_realized(...)

[Paolo Bonzini]

On 12/01/2016 11:44, Knut Omang wrote:
> On Fri, 2016-01-01 at 23:37 +0100, Paolo Bonzini wrote:
>>
>> On 31/12/2015 19:13, Ilya Lesokhin wrote:
>>> I was able to overcome this issue by calling object_unparent on my
>>> device but I’m not sure that the correct way of fixing it.
>>
>> Yes, it's definitely the right way to fix it.
> 
> Sorry for the late follow-up on this one, but I had to find some more
> time to spend with the code (and with valgrind too) to understand
> better/verify what was going on in the qdev/qom layers.
> 
> In the SR/IOV patch the object is created by pci_create.
> Since there is no corresponding pci_delete, I assume this means that
> the correct way to clean up from pci_create is simply a call to
> object_unparent() as you indicate, and this is what is missing from the
> patch set.
> 
> So the full setup/teardown sequence per VF then becomes:
> 
> pci_create(...)
> <realize>
> 
> <unrealize>
> object_unparent(...)

Almost: object_unparent takes care of unrealizing.

Paolo