Re: [Qemu-devel] [RFC PATCH 02/15] qdev: store DeviceState's canonical path to use when unparenting

[Michael Roth]

Quoting Paolo Bonzini (2015-05-01 15:43:45)
> 
> 
> On 01/05/2015 01:03, Michael Roth wrote:
> > 
> > I played around with the idea of temporarilly moving unparented, unfinalized
> > objects to an "orphan" container. It seemed like a fun way of tracking 
> > leaked
> > objects, and avoids the assert, but that got wierd pretty quickly... and
> > having DEVICE_DELETED randomly change up the device path didn't seem like
> > the intended behavior, so this hack ended up seeming pretty reasonable.
> > 
> > The other approach, which I hadn't looked into too closely, was to defer
> > unparenting an object until it's ref count goes to 0. Could maybe look into
> > that instead if it seems less hacky.
> 
> What about unparenting children devices in the device's unrealize
> callback?  It sucks that you have to do it manually, but using stale
> canonical paths isn't the nicest thing either.

That does seems to do the trick. It felt wrong when I first looked at
it because in some cases the children attach themselves to the parent
without making making the parent aware, but using the child property
list ends up seeming pretty reasonable:

static int spapr_phb_unparent_child_devices(Object *child, void *opaque)
{
    DeviceState *dev = (DeviceState *) object_dynamic_cast(child, TYPE_DEVICE);
    if (dev) {
        object_unparent(child);
    }                                                                           
                           
    return 0;
}

static void spapr_phb_unrealize(DeviceState *dev, Error **errp)
{
    ...
    /* clean up child devices (spapr_drc, spapr_iommu, etc.).
     * non-DeviceState's will be handled explicitly
     */
    object_child_foreach(OBJECT(dev), spapr_phb_unparent_child_devices, NULL);
    ...
}

Maybe if we have some more examples pop up it might make sense to
move that to qdev:device_unparent(), since we sort of expect that
for Devices.

Either way this does seem nicer. Thanks!

> 
> Paolo
>