Re: [Qemu-devel] commit da57febfed "qdev: give all devices a canonical path" broke usb_del

[Anthony Liguori]

Michael Tokarev <address@hidden> writes:

> On 08.08.2012 17:09, Michael Tokarev wrote:
> []
>> Something similar should be applied to 1.1-stable.  FWIW, some
>> changes are not needed there.
>
> Cherry-pick to stable-1.1 removes the two unneeded hunks.
> This is what I plan to include into debian package.  It
> fixes the original usb_del issue, and I didn't find new
> regressions so far - tried a few device_del and similar.
>
> Should it go to qemu/stable-1.1 as well?
>
> Thank you!
>
> /mjtAuthor: Paolo Bonzini <address@hidden>
> Date:   Wed Aug 8 14:39:11 2012 +0200
> Bug-Debian: http://bugs.debian.org/684282
> Comment: cherry-picked from qemu/master to stable-1.1 (mjt)
>
>     qom: object_delete should unparent the object first
>     
>     object_deinit is only called when the reference count goes to zero,
>     and yet tries to do an object_unparent.  Now, object_unparent
>     either does nothing or it will decrease the reference count.
>     Because we know the reference count is zero, the object_unparent
>     call in object_deinit is useless.
>     
>     Instead, we need to disconnect the object from its parent just
>     before we remove the last reference apart from the parent's.  This
>     happens in object_delete.  Once we do this, all calls to
>     object_unparent peppered through QEMU can go away.
>     
>     Signed-off-by: Paolo Bonzini <address@hidden>
>     Signed-off-by: Michael Tokarev <address@hidden>
>
> diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
> index 0345490..585da4e 100644
> --- a/hw/acpi_piix4.c
> +++ b/hw/acpi_piix4.c
> @@ -299,7 +299,6 @@ static void acpi_piix_eject_slot(PIIX4PMState *s, 
> unsigned slots)
>              if (pc->no_hotplug) {
>                  slot_free = false;
>              } else {
> -                object_unparent(OBJECT(dev));
>                  qdev_free(qdev);
>              }
>          }
> diff --git a/hw/qdev.c b/hw/qdev.c
> index 6a8f6bd..9bb1c6b 100644
> --- a/hw/qdev.c
> +++ b/hw/qdev.c
> @@ -240,7 +240,6 @@ void qbus_reset_all_fn(void *opaque)
>  int qdev_simple_unplug_cb(DeviceState *dev)
>  {
>      /* just zap it */
> -    object_unparent(OBJECT(dev));
>      qdev_free(dev);
>      return 0;
>  }
> diff --git a/hw/xen_platform.c b/hw/xen_platform.c
> index 0214f37..84221df 100644
> --- a/hw/xen_platform.c
> +++ b/hw/xen_platform.c
> @@ -87,9 +87,6 @@ static void unplug_nic(PCIBus *b, PCIDevice *d)
>  {
>      if (pci_get_word(d->config + PCI_CLASS_DEVICE) ==
>              PCI_CLASS_NETWORK_ETHERNET) {
> -        /* Until qdev_free includes a call to object_unparent, we call it 
> here
> -         */
> -        object_unparent(&d->qdev.parent_obj);
>          qdev_free(&d->qdev);
>      }
>  }
> diff --git a/qom/object.c b/qom/object.c
> index 6f839ad..58dd886 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -347,8 +347,6 @@ static void object_deinit(Object *obj, TypeImpl *type)
>      if (type_has_parent(type)) {
>          object_deinit(obj, type_get_parent(type));
>      }
> -
> -    object_unparent(obj);
>  }
>  
>  void object_finalize(void *data)
> @@ -385,8 +383,9 @@ Object *object_new(const char *typename)
>  
>  void object_delete(Object *obj)
>  {
> +    object_unparent(obj);
> +    g_assert(obj->ref == 1);
>      object_unref(obj);
> -    g_assert(obj->ref == 0);
>      g_free(obj);
>  }

This won't work with composition.  object_delete() is never called for
child<> objects.

Regards,

Anthony Liguori


>