[Qemu-devel] [PATCH 2/8] qdev: device free fixups.

[Gerd Hoffmann <kraxel@redhat.com>]

Two bug fixes:
 * When freeing a device we unregister stuff unconditionally,
   even in case we didn't register in the first place because
   the ->init() callback failed.
 * When freeing a device with child busses attached we don't
   zap the child bus (and the devices attached to it).

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/qdev.c |   35 +++++++++++++++++++++++++++++++----
 hw/qdev.h |    7 +++++++
 2 files changed, 38 insertions(+), 4 deletions(-)

diff --git a/hw/qdev.c b/hw/qdev.c
index 43372c1..4931da1 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -102,6 +102,7 @@ DeviceState *qdev_create(BusState *bus, const char *name)
     qdev_prop_set_defaults(dev, dev->parent_bus->info->props);
     qdev_prop_set_compat(dev);
     QLIST_INSERT_HEAD(&bus->children, dev, sibling);
+    dev->state = DEV_STATE_CREATED;
     return dev;
 }
 
@@ -216,6 +217,7 @@ int qdev_init(DeviceState *dev)
 {
     int rc;
 
+    assert(dev->state == DEV_STATE_CREATED);
     rc = dev->info->init(dev, dev->info);
     if (rc < 0)
         return rc;
@@ -223,18 +225,27 @@ int qdev_init(DeviceState *dev)
         qemu_register_reset(dev->info->reset, dev);
     if (dev->info->vmsd)
         vmstate_register(-1, dev->info->vmsd, dev);
+    dev->state = DEV_STATE_INITIALIZED;
     return 0;
 }
 
 /* Unlink device from bus and free the structure.  */
 void qdev_free(DeviceState *dev)
 {
+    BusState *bus;
+
+    if (dev->state == DEV_STATE_INITIALIZED) {
+        while (dev->num_child_bus) {
+            bus = QLIST_FIRST(&dev->child_bus);
+            qbus_free(bus);
+        }
 #if 0 /* FIXME: need sane vmstate_unregister function */
-    if (dev->info->vmsd)
-        vmstate_unregister(dev->info->vmsd, dev);
+        if (dev->info->vmsd)
+            vmstate_unregister(dev->info->vmsd, dev);
 #endif
-    if (dev->info->reset)
-        qemu_unregister_reset(dev->info->reset, dev);
+        if (dev->info->reset)
+            qemu_unregister_reset(dev->info->reset, dev);
+    }
     QLIST_REMOVE(dev, sibling);
     qemu_free(dev);
 }
@@ -549,6 +560,22 @@ BusState *qbus_create(BusInfo *info, DeviceState *parent, const char *name)
     return bus;
 }
 
+void qbus_free(BusState *bus)
+{
+    DeviceState *dev;
+
+    while ((dev = QLIST_FIRST(&bus->children)) != NULL) {
+        qdev_free(dev);
+    }
+    if (bus->parent) {
+        QLIST_REMOVE(bus, sibling);
+        bus->parent->num_child_bus--;
+    }
+    if (bus->qdev_allocated) {
+        qemu_free(bus);
+    }
+}
+
 #define qdev_printf(fmt, ...) monitor_printf(mon, "%*s" fmt, indent, "", ## __VA_ARGS__)
 static void qbus_print(Monitor *mon, BusState *bus, int indent);
 
diff --git a/hw/qdev.h b/hw/qdev.h
index ccc45b9..c036aff 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -19,10 +19,16 @@ typedef struct BusState BusState;
 
 typedef struct BusInfo BusInfo;
 
+enum DevState {
+    DEV_STATE_CREATED = 1,
+    DEV_STATE_INITIALIZED,
+};
+
 /* This structure should not be accessed directly.  We declare it here
    so that it can be embedded in individual device state structures.  */
 struct DeviceState {
     const char *id;
+    enum DevState state;
     DeviceInfo *info;
     BusState *parent_bus;
     int num_gpio_out;
@@ -148,6 +154,7 @@ BusState *qdev_get_parent_bus(DeviceState *dev);
 void qbus_create_inplace(BusState *bus, BusInfo *info,
                          DeviceState *parent, const char *name);
 BusState *qbus_create(BusInfo *info, DeviceState *parent, const char *name);
+void qbus_free(BusState *bus);
 
 #define FROM_QBUS(type, dev) DO_UPCAST(type, qbus, dev)
 
-- 
1.6.2.5