[Qemu-devel] [PATCH 2/3] qdev: Refactor device_set_realized to avoid resource leak

[arei.gonglei]

From: Gonglei <address@hidden>

At present, the local variable local_err is reused at multi-places,
Which will cause resource leak in some scenarios.

Example:

1. Assuming that "dc->realize(dev, &local_err)" execute successful
   and local_err == NULL;
2. Executing device hotplug in hotplug_handler_plug(), but failed
  (It is prone to occur). Then local_err != NULL;
3. error_propagate(errp, local_err) and return. But the resources
 which been allocated in dc->realize() will be leaked.
 Simple backtrace:
  dc->realize()
   |->device_realize
            |->pci_qdev_init()
                |->do_pci_register_device()
                |->etc.

To avoid the resource leak, using some different local error variables.
For different error conditions, release the corresponding resources.

Signed-off-by: Gonglei <address@hidden>
---
 hw/core/qdev.c | 75 +++++++++++++++++++++++++++++++++++-----------------------
 1 file changed, 46 insertions(+), 29 deletions(-)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 3e7085e..b3a463b 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -839,41 +839,58 @@ static void device_set_realized(Object *obj, bool value, 
Error **errp)
             dc->realize(dev, &local_err);
         }
 
-        if (dev->parent_bus && dev->parent_bus->hotplug_handler &&
-            local_err == NULL) {
-            hotplug_handler_plug(dev->parent_bus->hotplug_handler,
-                                 dev, &local_err);
-        } else if (local_err == NULL &&
-                   object_dynamic_cast(qdev_get_machine(), TYPE_MACHINE)) {
-            HotplugHandler *hotplug_ctrl;
-            MachineState *machine = MACHINE(qdev_get_machine());
-            MachineClass *mc = MACHINE_GET_CLASS(machine);
-
-            if (mc->get_hotplug_handler) {
-                hotplug_ctrl = mc->get_hotplug_handler(machine, dev);
-                if (hotplug_ctrl) {
-                    hotplug_handler_plug(hotplug_ctrl, dev, &local_err);
+        if (local_err == NULL) {
+            Error *hotplug_err = NULL;
+
+            if (dev->parent_bus && dev->parent_bus->hotplug_handler) {
+                hotplug_handler_plug(dev->parent_bus->hotplug_handler,
+                                     dev, &hotplug_err);
+            } else if (object_dynamic_cast(qdev_get_machine(), TYPE_MACHINE)) {
+                HotplugHandler *hotplug_ctrl;
+                MachineState *machine = MACHINE(qdev_get_machine());
+                MachineClass *mc = MACHINE_GET_CLASS(machine);
+
+                if (mc->get_hotplug_handler) {
+                    hotplug_ctrl = mc->get_hotplug_handler(machine, dev);
+                    if (hotplug_ctrl) {
+                        hotplug_handler_plug(hotplug_ctrl, dev, &hotplug_err);
+                    }
                 }
             }
-        }
 
-        if (qdev_get_vmsd(dev) && local_err == NULL) {
-            vmstate_register_with_alias_id(dev, -1, qdev_get_vmsd(dev), dev,
-                                           dev->instance_id_alias,
-                                           dev->alias_required_for_version);
-        }
-        if (local_err == NULL) {
-            QLIST_FOREACH(bus, &dev->child_bus, sibling) {
-                object_property_set_bool(OBJECT(bus), true, "realized",
-                                         &local_err);
-                if (local_err != NULL) {
-                    break;
+            if (hotplug_err == NULL) {
+                Error *err = NULL;
+                if (qdev_get_vmsd(dev)) {
+                    vmstate_register_with_alias_id(dev, -1, qdev_get_vmsd(dev),
+                                                   dev, dev->instance_id_alias,
+                                                   
dev->alias_required_for_version);
                 }
+
+                QLIST_FOREACH(bus, &dev->child_bus, sibling) {
+                    object_property_set_bool(OBJECT(bus), true, "realized",
+                                             &err);
+                    if (err != NULL) {
+                        if (qdev_get_vmsd(dev)) {
+                            vmstate_unregister(dev, qdev_get_vmsd(dev), dev);
+                        }
+
+                        break;
+                    }
+                }
+
+                if (dev->hotplugged && err == NULL) {
+                    device_reset(dev);
+                }
+                error_free(err);
+            } else {
+                if (dc->unrealize) {
+                    dc->unrealize(dev, NULL);
+                }
+
+                error_propagate(errp, hotplug_err);
+                return;
             }
         }
-        if (dev->hotplugged && local_err == NULL) {
-            device_reset(dev);
-        }
         dev->pending_deleted_event = false;
     } else if (!value && dev->realized) {
         QLIST_FOREACH(bus, &dev->child_bus, sibling) {
-- 
1.7.12.4