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

[Michael Tokarev <mjt@tls.msk.ru>]

[-- Attachment #1: Type: text/plain, Size: 432 bytes --]

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!

/mjt

[-- Attachment #2: qom:-object_delete-should-unparent-the-object-first.diff --]
[-- Type: text/x-patch, Size: 2688 bytes --]

Author: Paolo Bonzini <pbonzini@redhat.com>
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 <pbonzini@redhat.com>
    Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>

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);
 }