Re: [PATCH v4 6/7] vfio: Do not unparent in instance_finalize()

[Cédric Le Goater <clg@xxxxxxxxxx>]

On 9/24/25 06:37, Akihiko Odaki wrote:
> Children are automatically unparented so manually unparenting is
> unnecessary.
>
> 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.
>
> Signed-off-by: Akihiko Odaki <odaki@xxxxxxxxxxxxxxxxxxxxxx>
> Reviewed-by: Daniel P. Berrangé <berrange@xxxxxxxxxx>
> ---
>   hw/vfio/pci-quirks.c | 9 +--------
>   hw/vfio/region.c     | 3 ---
>   2 files changed, 1 insertion(+), 11 deletions(-)
>
> diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c
> index c97606dbf194..b5da6afbf5b0 100644
> --- a/hw/vfio/pci-quirks.c
> +++ b/hw/vfio/pci-quirks.c
> @@ -1159,15 +1159,12 @@ void vfio_vga_quirk_exit(VFIOPCIDevice *vdev)
>   
>   void vfio_vga_quirk_finalize(VFIOPCIDevice *vdev)
>   {
> -    int i, j;
> +    int i;
>   
>       for (i = 0; i < ARRAY_SIZE(vdev->vga->region); i++) {
>           while (!QLIST_EMPTY(&vdev->vga->region[i].quirks)) {
>               VFIOQuirk *quirk = QLIST_FIRST(&vdev->vga->region[i].quirks);
>               QLIST_REMOVE(quirk, next);
> -            for (j = 0; j < quirk->nr_mem; j++) {
> -                object_unparent(OBJECT(&quirk->mem[j]));
> -            }
>               g_free(quirk->mem);
>               g_free(quirk->data);
>               g_free(quirk);
> @@ -1207,14 +1204,10 @@ void vfio_bar_quirk_exit(VFIOPCIDevice *vdev, int nr)
>   void vfio_bar_quirk_finalize(VFIOPCIDevice *vdev, int nr)
>   {
>       VFIOBAR *bar = &vdev->bars[nr];
> -    int i;
>   
>       while (!QLIST_EMPTY(&bar->quirks)) {
>           VFIOQuirk *quirk = QLIST_FIRST(&bar->quirks);
>           QLIST_REMOVE(quirk, next);
> -        for (i = 0; i < quirk->nr_mem; i++) {
> -            object_unparent(OBJECT(&quirk->mem[i]));
> -        }
>           g_free(quirk->mem);
>           g_free(quirk->data);
>           g_free(quirk);
> diff --git a/hw/vfio/region.c b/hw/vfio/region.c
> index d04c57db630f..b165ab0b9378 100644
> --- a/hw/vfio/region.c
> +++ b/hw/vfio/region.c
> @@ -365,12 +365,9 @@ void vfio_region_finalize(VFIORegion *region)
>       for (i = 0; i < region->nr_mmaps; i++) {
>           if (region->mmaps[i].mmap) {
>               munmap(region->mmaps[i].mmap, region->mmaps[i].size);
> -            object_unparent(OBJECT(&region->mmaps[i].mem));
>           }
>       }
>   
> -    object_unparent(OBJECT(region->mem));
> -
>       g_free(region->mem);
>       g_free(region->mmaps);
>   
What about vfio_subregion_unmap() calling object_unparent() too ?

C.