Patchwork [09/11] qdev: drop extra references at creation time

login
register
mail settings
Submitter Paolo Bonzini
Date Dec. 5, 2012, 8:44 p.m.
Message ID <1354740282-20679-10-git-send-email-pbonzini@redhat.com>
Download mbox | patch
Permalink /patch/203942/
State New
Headers show

Comments

Paolo Bonzini - Dec. 5, 2012, 8:44 p.m.
qdev_free and qbus_free have to do unparent+unref, because nobody else
drops the initial reference (the one included by object_initialize)
before them.

For device_init_func and do_device_add, this is trivially correct,
since the DeviceState goes out of scope.

For qdev_create, qdev_try_create and qbus_init, it is a bit more tricky.
What we are doing here is just assuming that the caller knows what it's
doing, and won't call qdev_free/qbus_free while the device is still there.
This is a pretty reasonable assumption and (behind the scenes) is also
what GObject/GTK does.  GTK actually has a "floating reference" that
goes away as soon as the caller does gtk_container_add or something
like that, but in the end qbus_init and qdev_try_create are already
adding the new object to its qdev parent!  So in the end the two solutions
are the same.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/qdev-monitor.c | 5 ++++-
 hw/qdev.c         | 5 ++---
 vl.c              | 1 +
 3 files changed, 7 insertions(+), 4 deletions(-)

Patch

diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c
index a1b4d6a..ed62fd3 100644
--- a/hw/qdev-monitor.c
+++ b/hw/qdev-monitor.c
@@ -578,6 +578,7 @@  int do_device_add(Monitor *mon, const QDict *qdict, QObject **ret_data)
 {
     Error *local_err = NULL;
     QemuOpts *opts;
+    DeviceState *dev;
 
     opts = qemu_opts_from_qdict(qemu_find_opts("device"), qdict, &local_err);
     if (error_is_set(&local_err)) {
@@ -589,10 +590,12 @@  int do_device_add(Monitor *mon, const QDict *qdict, QObject **ret_data)
         qemu_opts_del(opts);
         return 0;
     }
-    if (!qdev_device_add(opts)) {
+    dev = qdev_device_add(opts);
+    if (!dev) {
         qemu_opts_del(opts);
         return -1;
     }
+    object_unref(OBJECT(dev));
     return 0;
 }
 
diff --git a/hw/qdev.c b/hw/qdev.c
index dcd39b3..9f4fd92 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -142,7 +142,7 @@  DeviceState *qdev_try_create(BusState *bus, const char *type)
     }
 
     qdev_set_parent_bus(dev, bus);
-
+    object_unref(OBJECT(dev));
     return dev;
 }
 
@@ -269,7 +269,6 @@  void qdev_init_nofail(DeviceState *dev)
 void qdev_free(DeviceState *dev)
 {
     object_unparent(OBJECT(dev));
-    object_unref(OBJECT(dev));
 }
 
 void qdev_machine_creation_done(void)
@@ -441,6 +440,7 @@  void qbus_init(BusState *bus, DeviceState *parent, const char *name)
         QLIST_INSERT_HEAD(&bus->parent->child_bus, bus, sibling);
         bus->parent->num_child_bus++;
         object_property_add_child(OBJECT(bus->parent), bus->name, OBJECT(bus), NULL);
+        object_unref(OBJECT(bus));
     } else if (bus != sysbus_get_default()) {
         /* TODO: once all bus devices are qdevified,
            only reset handler for main_system_bus should be registered here. */
@@ -487,7 +487,6 @@  BusState *qbus_create(const char *typename, DeviceState *parent, const char *nam
 void qbus_free(BusState *bus)
 {
     object_unparent(OBJECT(bus));
-    object_unref(OBJECT(bus));
 }
 
 static char *bus_get_fw_dev_path(BusState *bus, DeviceState *dev)
diff --git a/vl.c b/vl.c
index a3ab384..71c339c 100644
--- a/vl.c
+++ b/vl.c
@@ -2052,6 +2052,7 @@  static int device_init_func(QemuOpts *opts, void *opaque)
     dev = qdev_device_add(opts);
     if (!dev)
         return -1;
+    object_unref(OBJECT(dev));
     return 0;
 }