From 8af734ca318369c4eb76f355ed4cc7d4e6abc2a6 Mon Sep 17 00:00:00 2001 From: Alexey Kardashevskiy Date: Mon, 14 Jul 2014 00:41:08 +1000 Subject: qom: Make object_child_foreach() safe for objects removal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Current object_child_foreach() uses QTAILQ_FOREACH() to walk through children and that makes children removal from the callback impossible. This makes object_child_foreach() use QTAILQ_FOREACH_SAFE(). Signed-off-by: Alexey Kardashevskiy Reviewed-by: Hu Tao Signed-off-by: Andreas Färber --- qom/object.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/qom/object.c b/qom/object.c index 79bd0cc474..a298b32f8b 100644 --- a/qom/object.c +++ b/qom/object.c @@ -668,10 +668,10 @@ void object_class_foreach(void (*fn)(ObjectClass *klass, void *opaque), int object_child_foreach(Object *obj, int (*fn)(Object *child, void *opaque), void *opaque) { - ObjectProperty *prop; + ObjectProperty *prop, *next; int ret = 0; - QTAILQ_FOREACH(prop, &obj->properties, node) { + QTAILQ_FOREACH_SAFE(prop, &obj->properties, node, next) { if (object_property_is_child(prop)) { ret = fn(prop->opaque, opaque); if (ret != 0) { -- cgit v1.2.3 From d2659e27e1ec0b5126faa0f4fef78755950b39e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andreas=20F=C3=A4rber?= Date: Wed, 23 Jul 2014 16:16:26 +0200 Subject: machine: Clean up -machine handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since commit c4090f8, -object options are no longer handled through object_set_property(), so clean up -object leftovers by renaming the function and dropping special-casing of qom-type and id properties. Cc: Paolo Bonzini Reviewed-by: Marcel Apfelbaum Signed-off-by: Andreas Färber --- vl.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/vl.c b/vl.c index a9fd2070e7..9c9acf5e65 100644 --- a/vl.c +++ b/vl.c @@ -2841,15 +2841,15 @@ static void free_and_trace(gpointer mem) free(mem); } -static int object_set_property(const char *name, const char *value, void *opaque) +static int machine_set_property(const char *name, const char *value, + void *opaque) { Object *obj = OBJECT(opaque); StringInputVisitor *siv; Error *local_err = NULL; char *c, *qom_name; - if (strcmp(name, "qom-type") == 0 || strcmp(name, "id") == 0 || - strcmp(name, "type") == 0) { + if (strcmp(name, "type") == 0) { return 0; } @@ -4254,7 +4254,7 @@ int main(int argc, char **argv, char **envp) } machine_opts = qemu_get_machine_opts(); - if (qemu_opt_foreach(machine_opts, object_set_property, current_machine, + if (qemu_opt_foreach(machine_opts, machine_set_property, current_machine, 1) < 0) { object_unref(OBJECT(current_machine)); exit(1); -- cgit v1.2.3 From 339659041f87a76f8b71ad3d12cadfc5f89b4bb3 Mon Sep 17 00:00:00 2001 From: Peter Crosthwaite Date: Tue, 19 Aug 2014 23:55:52 -0700 Subject: qom: Add automatic arrayification to object_property_add() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If "[*]" is given as the last part of a QOM property name, treat that as an array property. The added property is given the first available name, replacing the * with a decimal number counting from 0. First add with name "foo[*]" will be "foo[0]". Second "foo[1]" and so on. Callers may inspect the ObjectProperty * return value to see what number the added property was given. Signed-off-by: Peter Crosthwaite Signed-off-by: Andreas Färber --- qom/object.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/qom/object.c b/qom/object.c index a298b32f8b..da0919a3dd 100644 --- a/qom/object.c +++ b/qom/object.c @@ -728,6 +728,27 @@ object_property_add(Object *obj, const char *name, const char *type, void *opaque, Error **errp) { ObjectProperty *prop; + size_t name_len = strlen(name); + + if (name_len >= 3 && !memcmp(name + name_len - 3, "[*]", 4)) { + int i; + ObjectProperty *ret; + char *name_no_array = g_strdup(name); + + name_no_array[name_len - 3] = '\0'; + for (i = 0; ; ++i) { + char *full_name = g_strdup_printf("%s[%d]", name_no_array, i); + + ret = object_property_add(obj, full_name, type, get, set, + release, opaque, NULL); + g_free(full_name); + if (ret) { + break; + } + } + g_free(name_no_array); + return ret; + } QTAILQ_FOREACH(prop, &obj->properties, node) { if (strcmp(prop->name, name) == 0) { -- cgit v1.2.3 From 843ef73a690aabbea6a897e86a426ee7554d7aff Mon Sep 17 00:00:00 2001 From: Peter Crosthwaite Date: Tue, 19 Aug 2014 23:56:26 -0700 Subject: memory: Remove object_property_add_child_array() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Obsoleted by automatic object_property_add() arrayification. Reviewed-by: Paolo Bonzini Signed-off-by: Peter Crosthwaite Signed-off-by: Andreas Färber --- memory.c | 30 +++++------------------------- 1 file changed, 5 insertions(+), 25 deletions(-) diff --git a/memory.c b/memory.c index ef0be1c3aa..1bae951df7 100644 --- a/memory.c +++ b/memory.c @@ -876,30 +876,6 @@ static char *memory_region_escape_name(const char *name) return escaped; } -static void object_property_add_child_array(Object *owner, - const char *name, - Object *child) -{ - int i; - char *base_name = memory_region_escape_name(name); - - for (i = 0; ; i++) { - char *full_name = g_strdup_printf("%s[%d]", base_name, i); - Error *local_err = NULL; - - object_property_add_child(owner, full_name, child, &local_err); - g_free(full_name); - if (!local_err) { - break; - } - - error_free(local_err); - } - - g_free(base_name); -} - - void memory_region_init(MemoryRegion *mr, Object *owner, const char *name, @@ -917,8 +893,12 @@ void memory_region_init(MemoryRegion *mr, mr->name = g_strdup(name); if (name) { - object_property_add_child_array(owner, name, OBJECT(mr)); + char *escaped_name = memory_region_escape_name(name); + char *name_array = g_strdup_printf("%s[*]", escaped_name); + object_property_add_child(owner, name_array, OBJECT(mr), &error_abort); object_unref(OBJECT(mr)); + g_free(name_array); + g_free(escaped_name); } } -- cgit v1.2.3 From d578029e71311de1b1476229d88d4aca02b783a3 Mon Sep 17 00:00:00 2001 From: Gonglei Date: Tue, 2 Sep 2014 20:03:05 +0800 Subject: qdev: Use error_abort instead of using local_err MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This error can not happen normally. If it happens, it indicates something very wrong, we should abort QEMU. Moreover, the user can only refer to /machine/peripheral or /objects, not /machine/unattached. While at it, remove superfluous check about local_err. Signed-off-by: Gonglei Reviewed-by: Peter Crosthwaite Signed-off-by: Andreas Färber --- hw/core/qdev.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/core/qdev.c b/hw/core/qdev.c index da1ba48c99..4a1ac5b3f9 100644 --- a/hw/core/qdev.c +++ b/hw/core/qdev.c @@ -820,13 +820,13 @@ static void device_set_realized(Object *obj, bool value, Error **errp) } if (value && !dev->realized) { - if (!obj->parent && local_err == NULL) { + if (!obj->parent) { static int unattached_count; gchar *name = g_strdup_printf("device[%d]", unattached_count++); object_property_add_child(container_get(qdev_get_machine(), "/unattached"), - name, obj, &local_err); + name, obj, &error_abort); g_free(name); } -- cgit v1.2.3 From cd4520adcab70dbac8db3fe4d41836dca63715a4 Mon Sep 17 00:00:00 2001 From: Gonglei Date: Thu, 4 Sep 2014 10:18:25 +0800 Subject: qdev: Use NULL instead of local_err for qbus_child unrealize MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Forcefully unrealize all children regardless of errors in earlier iterations (if any). We should keep going with cleanup operation rather than report an error immediately. Therefore store the first child unrealization failure and propagate it at the end. We also forcefully unregister vmsd and unrealize actual object, too. Signed-off-by: Gonglei Reviewed-by: Peter Crosthwaite Cc: qemu-stable@nongnu.org Signed-off-by: Andreas Färber --- hw/core/qdev.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/hw/core/qdev.c b/hw/core/qdev.c index 4a1ac5b3f9..6f37cd324a 100644 --- a/hw/core/qdev.c +++ b/hw/core/qdev.c @@ -871,18 +871,18 @@ static void device_set_realized(Object *obj, bool value, Error **errp) } dev->pending_deleted_event = false; } else if (!value && dev->realized) { + Error **local_errp = NULL; QLIST_FOREACH(bus, &dev->child_bus, sibling) { + local_errp = local_err ? NULL : &local_err; object_property_set_bool(OBJECT(bus), false, "realized", - &local_err); - if (local_err != NULL) { - break; - } + local_errp); } - if (qdev_get_vmsd(dev) && local_err == NULL) { + if (qdev_get_vmsd(dev)) { vmstate_unregister(dev, qdev_get_vmsd(dev), dev); } - if (dc->unrealize && local_err == NULL) { - dc->unrealize(dev, &local_err); + if (dc->unrealize) { + local_errp = local_err ? NULL : &local_err; + dc->unrealize(dev, local_errp); } dev->pending_deleted_event = true; } -- cgit v1.2.3 From 1d45a705fc007a13f20d18473290082eae6d1725 Mon Sep 17 00:00:00 2001 From: Gonglei Date: Thu, 4 Sep 2014 10:18:26 +0800 Subject: qdev: Add cleanup logic in device_set_realized() to avoid resource leak MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit At present, this function doesn't have partial cleanup implemented, which will cause resource leaks in some scenarios. Example: 1. Assume that "dc->realize(dev, &local_err)" executes successful and local_err == NULL; 2. device hotplug in hotplug_handler_plug() executes but fails (it is prone to occur). Then local_err != NULL; 3. error_propagate(errp, local_err) and return. But the resources which have been allocated in dc->realize() will be leaked. Simple backtrace: dc->realize() |->device_realize |->pci_qdev_init() |->do_pci_register_device() |->etc. Add fuller cleanup logic which assures that function can goto appropriate error label as local_err population is detected at each relevant point. Signed-off-by: Gonglei Reviewed-by: Peter Crosthwaite Cc: qemu-stable@nongnu.org Signed-off-by: Andreas Färber --- hw/core/qdev.c | 52 ++++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 38 insertions(+), 14 deletions(-) diff --git a/hw/core/qdev.c b/hw/core/qdev.c index 6f37cd324a..fcb16383a1 100644 --- a/hw/core/qdev.c +++ b/hw/core/qdev.c @@ -834,12 +834,14 @@ 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) { + if (local_err != NULL) { + goto fail; + } + + if (dev->parent_bus && dev->parent_bus->hotplug_handler) { hotplug_handler_plug(dev->parent_bus->hotplug_handler, dev, &local_err); - } else if (local_err == NULL && - object_dynamic_cast(qdev_get_machine(), TYPE_MACHINE)) { + } 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); @@ -852,21 +854,24 @@ static void device_set_realized(Object *obj, bool value, Error **errp) } } - if (qdev_get_vmsd(dev) && local_err == NULL) { + if (local_err != NULL) { + goto post_realize_fail; + } + + 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); } - if (local_err == NULL) { - QLIST_FOREACH(bus, &dev->child_bus, sibling) { - object_property_set_bool(OBJECT(bus), true, "realized", + + QLIST_FOREACH(bus, &dev->child_bus, sibling) { + object_property_set_bool(OBJECT(bus), true, "realized", &local_err); - if (local_err != NULL) { - break; - } + if (local_err != NULL) { + goto child_realize_fail; } } - if (dev->hotplugged && local_err == NULL) { + if (dev->hotplugged) { device_reset(dev); } dev->pending_deleted_event = false; @@ -888,11 +893,30 @@ static void device_set_realized(Object *obj, bool value, Error **errp) } if (local_err != NULL) { - error_propagate(errp, local_err); - return; + goto fail; } dev->realized = value; + return; + +child_realize_fail: + QLIST_FOREACH(bus, &dev->child_bus, sibling) { + object_property_set_bool(OBJECT(bus), false, "realized", + NULL); + } + + if (qdev_get_vmsd(dev)) { + vmstate_unregister(dev, qdev_get_vmsd(dev), dev); + } + +post_realize_fail: + if (dc->unrealize) { + dc->unrealize(dev, NULL); + } + +fail: + error_propagate(errp, local_err); + return; } static bool device_get_hotpluggable(Object *obj, Error **errp) -- cgit v1.2.3