Re: [PATCH v3 6/7] pcie_sriov: Reuse SR-IOV VF device instances

[Akihiko Odaki <akihiko.odaki@daynix.com>]

On 2024/02/13 17:51, Minwoo Im wrote:
>> -----Original Message-----
>> From: qemu-block-bounces+minwoo.im.dev=gmail.com@nongnu.org <qemu-block-
>> bounces+minwoo.im.dev=gmail.com@nongnu.org> On Behalf Of Akihiko Odaki
>> Sent: Monday, February 12, 2024 7:21 PM
>> To: Philippe Mathieu-Daudé <philmd@linaro.org>; Michael S. Tsirkin
>> <mst@redhat.com>; Marcel Apfelbaum <marcel.apfelbaum@gmail.com>; Alex
>> Williamson <alex.williamson@redhat.com>; Cédric Le Goater <clg@redhat.com>;
>> Paolo Bonzini <pbonzini@redhat.com>; Daniel P. Berrangé <berrange@redhat.com>;
>> Eduardo Habkost <eduardo@habkost.net>; Sriram Yagnaraman
>> <sriram.yagnaraman@est.tech>; Jason Wang <jasowang@redhat.com>; Keith Busch
>> <kbusch@kernel.org>; Klaus Jensen <its@irrelevant.dk>
>> Cc: qemu-devel@nongnu.org; qemu-block@nongnu.org; Akihiko Odaki
>> <akihiko.odaki@daynix.com>
>> Subject: [PATCH v3 6/7] pcie_sriov: Reuse SR-IOV VF device instances
>>
>> Disable SR-IOV VF devices by reusing code to power down PCI devices
>> instead of removing them when the guest requests to disable VFs. This
>> allows to realize devices and report VF realization errors at PF
>> realization time.
>>
>> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> 
> Hello Akihiko,
> 
> I think this patch fixes the issue reported in [1].  The latest master branch
> also causes an object-related assertion error when we enable VF(s) and disable
> through sysfs over and over again (at least twice).  But this issue is also
> fixed with your patch.
> 
> **
> ERROR:../qom/object.c:753:object_finalize: assertion failed: (obj->parent == NULL)
> Bail out! ERROR:../qom/object.c:753:object_finalize: assertion failed: (obj->parent == NULL)

I looked into this and found it's not fixed with my patch though the 
symptom may be gone.

The problem is that object_ref() is not called when copying subsys. An 
obvious fix is to add an object_ref() call, but I think it's too hacky 
and error-prone. It's better to enumerate nvme_props and call 
object_property_get_qobject() and object_property_set_qobject() for each 
property.

Regards,
Akihiko Odaki

> 
> Klaus,
> 
> If this patchset is applied, I think [1] can be dropped.  What do you think?
> 
> Thanks,
> 
> [1] https://lore.kernel.org/qemu-devel/20240109022953epcms2p54550dcfc9f831a515206513ae98e7511@epcms2p5/
>