Re: [Qemu-devel] [PATCH v3 06/12] qdev: move deletion of children from finalize to unparent

[Andreas Färber]

Am 25.01.2013 12:46, schrieb Paolo Bonzini:
> A device will never be finalized as long as it has a reference from
> other devices that sit on its buses.  To ensure that the references
> go away, deassociate a bus from its children in the unparent callback
> for the bus.
> 
> Signed-off-by: Paolo Bonzini <address@hidden>

To help me and others understand this, the children often have
back-references to the bus blocking the bus' finalization? Is that what
you're tackling here?

Remind me, when exactly would the unparent hook being called in case of
a PCI host bridge? prep_pci create a PCI dev to represent itself, 1 ref,
and puts it on the bus. At some point, the bus is unparented (removed
from, e.g., /machine/unassigned, I interpret) and with this patch the
children are unparented and unref'ed. That would still leave the
original object_initialize() ref behind, so that I would still need to
unref it in a finalizefn, no?

Regards,
Andreas

> ---
>  hw/qdev.c | 37 +++++++++++++++++++++++++------------
>  1 file changed, 25 insertions(+), 12 deletions(-)
> 
> diff --git a/hw/qdev.c b/hw/qdev.c
> index 78eedf0..364386d 100644
> --- a/hw/qdev.c
> +++ b/hw/qdev.c
> @@ -433,6 +433,25 @@ static void qbus_realize(BusState *bus, DeviceState 
> *parent, const char *name)
>      }
>  }
>  
> +static void bus_unparent(Object *obj)
> +{
> +    BusState *bus = BUS(obj);
> +    BusChild *kid;
> +
> +    while ((kid = QTAILQ_FIRST(&bus->children)) != NULL) {
> +        DeviceState *dev = kid->child;
> +        qdev_free(dev);
> +    }
> +    if (bus->parent) {
> +        QLIST_REMOVE(bus, sibling);
> +        bus->parent->num_child_bus--;
> +        bus->parent = NULL;
> +    } else {
> +        assert(bus != sysbus_get_default()); /* main_system_bus is never 
> freed */
> +        qemu_unregister_reset(qbus_reset_all_fn, bus);
> +    }
> +}
> +
>  void qbus_create_inplace(BusState *bus, const char *typename,
>                           DeviceState *parent, const char *name)
>  {
> @@ -805,22 +824,15 @@ static void qbus_initfn(Object *obj)
>      QTAILQ_INIT(&bus->children);
>  }
>  
> +static void bus_class_init(ObjectClass *class, void *data)
> +{
> +    class->unparent = bus_unparent;
> +}
> +
>  static void qbus_finalize(Object *obj)
>  {
>      BusState *bus = BUS(obj);
> -    BusChild *kid;
>  
> -    while ((kid = QTAILQ_FIRST(&bus->children)) != NULL) {
> -        DeviceState *dev = kid->child;
> -        qdev_free(dev);
> -    }
> -    if (bus->parent) {
> -        QLIST_REMOVE(bus, sibling);
> -        bus->parent->num_child_bus--;
> -    } else {
> -        assert(bus != sysbus_get_default()); /* main_system_bus is never 
> freed */
> -        qemu_unregister_reset(qbus_reset_all_fn, bus);
> -    }
>      g_free((char *)bus->name);
>  }
>  
> @@ -832,6 +844,7 @@ static const TypeInfo bus_info = {
>      .class_size = sizeof(BusClass),
>      .instance_init = qbus_initfn,
>      .instance_finalize = qbus_finalize,
> +    .class_init = bus_class_init,
>  };
>  
>  static void qdev_register_types(void)
> 


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg