Re: [Qemu-devel] [PATCH 18/21] qdev: convert busses to QEMU Object Model

[Andreas Färber]

Am 02.05.2012 13:31, schrieb Paolo Bonzini:
> From: Anthony Liguori <address@hidden>
> 
> This is far less interesting than it sounds.  We simply add an Object to each
> BusState and then register the types appropriately.  Most of the interesting
> refactoring will follow in the next patches.
> 
> Since we're changing fundamental type names (BusInfo -> BusClass), it all 
> needs
> to convert at once.  Fortunately, not a lot of code is affected.
> 
> Signed-off-by: Anthony Liguori <address@hidden>
> Signed-off-by: Paolo Bonzini <address@hidden>

Thanks, I applied this to qom-next with two changes:
http://repo.or.cz/w/qemu/afaerber.git/shortlog/refs/heads/qom-next

First, I enforced all new TypeInfos to be static const rather than just
static. If someone has time to fix up all the static ones in the code
base to not set bad examples that would be appreciated - they needed to
be writable originally while transitioning from qdev to QOM.

Second, I applied the following patch on top, backporting
object_delete() / object_finalize() from "qbus: initialize in standard
way", since we introduce object_new() usage here:

diff --git a/hw/pci.c b/hw/pci.c
index c5d3100..b2afcf6 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -295,7 +295,7 @@ PCIBus *pci_bus_new(DeviceState *parent, const char
*name,
     PCIBus *bus;

     bus = g_malloc0(sizeof(*bus));
-    bus->qbus.qdev_allocated = 1;
+    bus->qbus.glib_allocated = true;
     pci_bus_new_inplace(bus, parent, name, address_space_mem,
                         address_space_io, devfn_min);
     return bus;
diff --git a/hw/qdev.c b/hw/qdev.c
index 63baa0a..63012b5 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -423,7 +423,7 @@ BusState *qbus_create(const char *typename,
DeviceState *parent, const char *nam
     BusState *bus;

     bus = BUS(object_new(typename));
-    bus->qdev_allocated = 1;
+    bus->qom_allocated = true;
     do_qbus_create_inplace(bus, typename, parent, name);
     return bus;
 }
@@ -443,8 +443,13 @@ void qbus_free(BusState *bus)
         qemu_unregister_reset(qbus_reset_all_fn, bus);
     }
     g_free((void*)bus->name);
-    if (bus->qdev_allocated) {
-        g_free(bus);
+    if (bus->qom_allocated) {
+        object_delete(OBJECT(bus));
+    } else {
+        object_finalize(OBJECT(bus));
+        if (bus->glib_allocated) {
+            g_free(bus);
+        }
     }
 }

diff --git a/hw/qdev.h b/hw/qdev.h
index 0da4b74..f3dd004 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -100,12 +100,19 @@ struct BusClass {
     int (*reset)(BusState *bus);
 };

+/**
+ * BusState:
+ * @qom_allocated: Indicates whether the object was allocated by QOM.
+ * @glib_allocated: Indicates whether the object was initialized in-place
+ * yet is expected to be freed with g_free().
+ */
 struct BusState {
     Object obj;
     DeviceState *parent;
     const char *name;
     int allow_hotplug;
-    int qdev_allocated;
+    bool qom_allocated;
+    bool glib_allocated;
     QTAILQ_HEAD(ChildrenHead, DeviceState) children;
     QLIST_ENTRY(BusState) sibling;
 };
diff --git a/hw/sysbus.c b/hw/sysbus.c
index 369ee44..2347f51 100644
--- a/hw/sysbus.c
+++ b/hw/sysbus.c
@@ -274,7 +274,7 @@ static void main_system_bus_create(void)
     main_system_bus = g_malloc0(system_bus_info.instance_size);
     qbus_create_inplace(main_system_bus, TYPE_SYSTEM_BUS, NULL,
                         "main-system-bus");
-    main_system_bus->qdev_allocated = 1;
+    main_system_bus->glib_allocated = true;
 }

 BusState *sysbus_get_default(void)


I admit that qom_allocated vs. glib_allocated is slightly ugly but it is
more correct. If memory serves me correctly there was talk of
hotplugging PCI host controllers on pseries for passthrough or
something, so properly cleaning up seems worth the additional ~four bytes.
Fix-ups welcome - maybe use an enum or, better, fix the PCIBus
allocation scheme so that we can drop glib_allocated (SysBus is asserted
not to be freed).

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg