Re: [PATCH v3 0/7] Do not unparent in instance_finalize()

[Daniel P. Berrangé <berrange@xxxxxxxxxx>]

On Wed, Sep 17, 2025 at 07:13:25PM +0900, Akihiko Odaki wrote:
> Based-on: <cover.1751493467.git.balaton@xxxxxxxxxx>
> ("[PATCH v2 00/14] hw/pci-host/raven clean ups")
> 
> Supersedes: <20240829-memory-v1-1-ac07af2f4fa5@xxxxxxxxxx>
> ("[PATCH] docs/devel: Prohibit calling object_unparent() for memory region")
> 
> Children are automatically unparented so manually unparenting is
> unnecessary.

Where is automatic unparenting you're referring to being done ?

> Worse, automatic unparenting happens before the instance_finalize()
> callback of the parent gets called, so object_unparent() calls in
> the callback will refer to objects that are already unparented, which
> is semantically incorrect.

IIUC, object_property_add_child will acquire a reference on
the child, and object_property_del_child (and thus
object_unparent) will release that reference.

The 'object_finalize' method, and thus 'instance_finalize'
callback, won't be invoked until the last reference is
dropped on the object in question.

IOW, it should be impossible for 'object_finalize' to ever
run, as long as the child has a parent set.

So if we're in the 'finalize' then 'object_unparent' must
be a no-op as the child must already have no references
held and thus no parent.

IOW, the reason to remove 'object_unparent' calls from
finalize is surely because they do nothing at all,
rather than this talk about callbacks being run at the
wrong time ?

> 
> Signed-off-by: Akihiko Odaki <odaki@xxxxxxxxxxxxxxxxxxxxxx>
> ---
> Changes in v3:
> - Added patches to remove other object_unparent() calls in
>   instance_finalize().
> - Dropped patch "qdev: Automatically delete memory subregions" and the
>   succeeding patches to avoid Ccing many.
> - Link to v2: 
> https://lore.kernel.org/qemu-devel/20250915-use-v2-0-f4c7ff13bfe9@xxxxxxxxxxxxxxxxxxxxxx
> 
> Changes in v2:
> - Added a reference to "[PATCH] docs/devel: Prohibit calling
>   object_unparent() for memory region", which does something similar to
>   patch "docs/devel: Do not unparent in instance_finalize()" but I
>   forgot I sent it in the past.
> - Fixed a typo in patch
>   "docs/devel: Do not unparent in instance_finalize()" and
>   "[PATCH 02/22] vfio/pci: Do not unparent in instance_finalize()".
> - Dropped patches to move address_space_init() calls; I intend to
>   QOM-ify to fix memory leaks automatically as discussed in the
>   following thread:
>   
> https://lore.kernel.org/qemu-devel/cd21698f-db77-eb75-6966-d559fdcab835@xxxxxxxxxx/
>   But the QOM-ification will be big so I'll send it as a separate
>   series.
> - Rebased on top of "[PATCH v2 00/14] hw/pci-host/raven clean ups".
>   https://lore.kernel.org/qemu-devel/cover.1751493467.git.balaton@xxxxxxxxxx/
> - Link to v1: 
> https://lore.kernel.org/qemu-devel/20250906-use-v1-0-c51caafd1eb7@xxxxxxxxxxxxxxxxxxxxxx
> 
> ---
> Akihiko Odaki (7):
>       docs/devel: Do not unparent in instance_finalize()
>       vfio/pci: Do not unparent in instance_finalize()
>       hw/core/register: Do not unparent in instance_finalize()
>       hv-balloon: hw/core/register: Do not unparent in instance_finalize()
>       hw/sd/sdhci: Do not unparent in instance_finalize()
>       vfio: Do not unparent in instance_finalize()
>       hw/xen: Do not unparent in instance_finalize()
> 
>  docs/devel/memory.rst  | 19 ++++++-------------
>  hw/core/register.c     |  1 -
>  hw/hyperv/hv-balloon.c | 12 +-----------
>  hw/sd/sdhci.c          |  4 ----
>  hw/vfio/pci-quirks.c   |  9 +--------
>  hw/vfio/pci.c          |  4 ----
>  hw/vfio/region.c       |  3 ---
>  hw/xen/xen_pt_msi.c    | 11 +----------
>  8 files changed, 9 insertions(+), 54 deletions(-)
> ---
> base-commit: e101d33792530093fa0b0a6e5f43e4d8cfe4581e
> change-id: 20250906-use-37ecc903a9e0
> 
> Best regards,
> --  
> Akihiko Odaki <odaki@xxxxxxxxxxxxxxxxxxxxxx>
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|