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

[Akihiko Odaki]

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/