Re: [Qemu-devel] [RFC PATCH 6/6] pc: Support firmware configuration with -blockdev

[Markus Armbruster]

The problem at hand is how to destroy a device created with
qdev_create() without ever realizing it.

This hack passes tests:

diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c
index ed608a53d3..1bd538796b 100644
--- a/hw/i386/pc_sysfw.c
+++ b/hw/i386/pc_sysfw.c
@@ -116,14 +116,9 @@ static void pc_system_flash_cleanup_unused(PCMachineState 
*pcms)
             prop_name = g_strdup_printf("pflash%d", i);
             object_property_del(OBJECT(pcms), prop_name, &error_abort);
             g_free(prop_name);
-            /*
-             * FIXME delete @dev_obj.  Straight object_unref() is
-             * wrong, since it leaves dangling references in the
-             * parent bus behind.  object_unparent() doesn't work,
-             * either: since @dev_obj hasn't been realized,
-             * dev_obj->parent is null, and object_unparent() does
-             * nothing.
-             */
+            object_ref(dev_obj);
+            object_get_class(dev_obj)->unparent(dev_obj);
+            object_unref(dev_obj);
             pcms->flash[i] = NULL;
         }
     }

If you have a better idea, please let me know.

Here's how I arrived at my hack.  We need to undo qdev_create().
qdev_create() is qdev_try_create() plus error handling.  The latter is
uninteresting here, so let's have a look at just the former:

    DeviceState *qdev_try_create(BusState *bus, const char *type)
    {
        DeviceState *dev;

        if (object_class_by_name(type) == NULL) {
            return NULL;
        }
        dev = DEVICE(object_new(type));
        if (!dev) {
            return NULL;
        }

        if (!bus) {
            /* Assert that the device really is a SysBusDevice before
             * we put it onto the sysbus. Non-sysbus devices which aren't
             * being put onto a bus should be created with object_new(TYPE_FOO),
             * not qdev_create(NULL, TYPE_FOO).
             */
            g_assert(object_dynamic_cast(OBJECT(dev), TYPE_SYS_BUS_DEVICE));
            bus = sysbus_get_default();
        }

--->    qdev_set_parent_bus(dev, bus);
        object_unref(OBJECT(dev));
        return dev;
    }

We need to undo qdev_set_parent_bus().

    void qdev_set_parent_bus(DeviceState *dev, BusState *bus)
    {
        bool replugging = dev->parent_bus != NULL;

        if (replugging) {
            /* Keep a reference to the device while it's not plugged into
             * any bus, to avoid it potentially evaporating when it is
             * dereffed in bus_remove_child().
             */
            object_ref(OBJECT(dev));
            bus_remove_child(dev->parent_bus, dev);
            object_unref(OBJECT(dev->parent_bus));
        }
        dev->parent_bus = bus;
        object_ref(OBJECT(bus));
--->    bus_add_child(bus, dev);
        if (replugging) {
            object_unref(OBJECT(dev));
        }
    }

dev->parent_bus is null, since we didn't realize the device.  All we
need to undo is bus_add_child(), with bus_remove_child().  Destruction
of realized devices does that here:

    static void device_unparent(Object *obj)
    {
        DeviceState *dev = DEVICE(obj);
        BusState *bus;

        if (dev->realized) {
            object_property_set_bool(obj, false, "realized", NULL);
        }
        while (dev->num_child_bus) {
            bus = QLIST_FIRST(&dev->child_bus);
            object_unparent(OBJECT(bus));
        }
        if (dev->parent_bus) {
            bus_remove_child(dev->parent_bus, dev);
            object_unref(OBJECT(dev->parent_bus));
            dev->parent_bus = NULL;
        }
    }

dev->realized is false, dev->num_child_bus is zero, dev->parent_bus is
main_system_bus.  Okay, this does what we need.  Now how do we call it?
It's static...  but it's a method:

    static void device_class_init(ObjectClass *class, void *data)
    {
        DeviceClass *dc = DEVICE_CLASS(class);

        class->unparent = device_unparent;
        [...]
    }

Alright, we can call object_get_class(dev_obj)->unparent(dev_obj).

Final complication: if I call just that, the device's reference counter
goes down to zero in the middle of device_unparent(), and we use after
free.  So I bracket he call with object_ref() and object_unref().