From 3b6ca4022d150ad273d4cd9556c2f4873389f965 Mon Sep 17 00:00:00 2001 From: Ildar Isaev Date: Wed, 4 Mar 2015 17:09:46 +0300 Subject: [PATCH 01/10] qdev: Change Property::offset field to ptrdiff_t type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Property::offset field is calculated as a diff between two pointers: arrayprop->prop.offset = eltptr - (void *)dev; If offset is declared as int, this subtraction can cause type overflow, thus leading to failure of the subsequent assertion: assert(qdev_get_prop_ptr(dev, &arrayprop->prop) == eltptr); So ptrdiff_t should be used instead. Signed-off-by: Ildar Isaev Reviewed-by: Peter Maydell Signed-off-by: Andreas Färber --- include/hw/qdev-core.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h index e6dbde42c4..c537969f4e 100644 --- a/include/hw/qdev-core.h +++ b/include/hw/qdev-core.h @@ -237,7 +237,7 @@ struct BusState { struct Property { const char *name; PropertyInfo *info; - int offset; + ptrdiff_t offset; uint8_t bitnr; qtype_code qtype; int64_t defval; From a00c94824126901168bca5b89147f9e334a49e87 Mon Sep 17 00:00:00 2001 From: "Daniel P. Berrange" Date: Tue, 13 Oct 2015 13:37:40 +0100 Subject: [PATCH 02/10] qom: Introduce ObjectPropertyIterator struct for iteration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Some users of QOM need to be able to iterate over properties defined against an object instance. Currently they are just directly using the QTAIL macros against the object properties data structure. This is bad because it exposes them to changes in the data structure used to store properties, as well as changes in functionality such as ability to register properties against the class. This provides an ObjectPropertyIterator struct which will insulate the callers from the particular data structure used to store properties. It can be used thus ObjectProperty *prop; ObjectPropertyIterator *iter; iter = object_property_iter_init(obj); while ((prop = object_property_iter_next(iter))) { ... do something with prop ... } object_property_iter_free(iter); Signed-off-by: Daniel P. Berrange Tested-by: Pavel Fedin [AF: Fixed examples, style cleanups] Signed-off-by: Andreas Färber --- include/qom/object.h | 49 ++++++++++++++++++++++++++++++++++++++ qom/object.c | 28 ++++++++++++++++++++++ tests/check-qom-proplist.c | 46 +++++++++++++++++++++++++++++++++++ 3 files changed, 123 insertions(+) diff --git a/include/qom/object.h b/include/qom/object.h index 0bb89d481e..9f703140bd 100644 --- a/include/qom/object.h +++ b/include/qom/object.h @@ -960,6 +960,55 @@ void object_property_del(Object *obj, const char *name, Error **errp); ObjectProperty *object_property_find(Object *obj, const char *name, Error **errp); +typedef struct ObjectPropertyIterator ObjectPropertyIterator; + +/** + * object_property_iter_init: + * @obj: the object + * + * Initializes an iterator for traversing all properties + * registered against an object instance. + * + * It is forbidden to modify the property list while iterating, + * whether removing or adding properties. + * + * Typical usage pattern would be + * + * + * Using object property iterators + * + * ObjectProperty *prop; + * ObjectPropertyIterator *iter; + * + * iter = object_property_iter_init(obj); + * while ((prop = object_property_iter_next(iter))) { + * ... do something with prop ... + * } + * object_property_iter_free(iter); + * + * + * + * Returns: the new iterator + */ +ObjectPropertyIterator *object_property_iter_init(Object *obj); + +/** + * object_property_iter_free: + * @iter: the iterator instance + * + * Releases any resources associated with the iterator. + */ +void object_property_iter_free(ObjectPropertyIterator *iter); + +/** + * object_property_iter_next: + * @iter: the iterator instance + * + * Returns: the next property, or %NULL when all properties + * have been traversed. + */ +ObjectProperty *object_property_iter_next(ObjectPropertyIterator *iter); + void object_unparent(Object *obj); /** diff --git a/qom/object.c b/qom/object.c index c0decb6e96..1c926ceea1 100644 --- a/qom/object.c +++ b/qom/object.c @@ -67,6 +67,10 @@ struct TypeImpl InterfaceImpl interfaces[MAX_INTERFACES]; }; +struct ObjectPropertyIterator { + ObjectProperty *next; +}; + static Type type_interface; static GHashTable *type_table_get(void) @@ -917,6 +921,30 @@ ObjectProperty *object_property_find(Object *obj, const char *name, return NULL; } +ObjectPropertyIterator *object_property_iter_init(Object *obj) +{ + ObjectPropertyIterator *ret = g_new0(ObjectPropertyIterator, 1); + ret->next = QTAILQ_FIRST(&obj->properties); + return ret; +} + +void object_property_iter_free(ObjectPropertyIterator *iter) +{ + if (!iter) { + return; + } + g_free(iter); +} + +ObjectProperty *object_property_iter_next(ObjectPropertyIterator *iter) +{ + ObjectProperty *ret = iter->next; + if (ret) { + iter->next = QTAILQ_NEXT(iter->next, node); + } + return ret; +} + void object_property_del(Object *obj, const char *name, Error **errp) { ObjectProperty *prop = object_property_find(obj, name, errp); diff --git a/tests/check-qom-proplist.c b/tests/check-qom-proplist.c index 7400b1fce9..1be8b9e6f2 100644 --- a/tests/check-qom-proplist.c +++ b/tests/check-qom-proplist.c @@ -283,6 +283,51 @@ static void test_dummy_getenum(void) &err); g_assert(err != NULL); error_free(err); + + object_unparent(OBJECT(dobj)); +} + + +static void test_dummy_iterator(void) +{ + Object *parent = object_get_objects_root(); + DummyObject *dobj = DUMMY_OBJECT( + object_new_with_props(TYPE_DUMMY, + parent, + "dummy0", + &error_abort, + "bv", "yes", + "sv", "Hiss hiss hiss", + "av", "platypus", + NULL)); + + ObjectProperty *prop; + ObjectPropertyIterator *iter; + bool seenbv = false, seensv = false, seenav = false, seentype; + + iter = object_property_iter_init(OBJECT(dobj)); + while ((prop = object_property_iter_next(iter))) { + if (g_str_equal(prop->name, "bv")) { + seenbv = true; + } else if (g_str_equal(prop->name, "sv")) { + seensv = true; + } else if (g_str_equal(prop->name, "av")) { + seenav = true; + } else if (g_str_equal(prop->name, "type")) { + /* This prop comes from the base Object class */ + seentype = true; + } else { + g_printerr("Found prop '%s'\n", prop->name); + g_assert_not_reached(); + } + } + object_property_iter_free(iter); + g_assert(seenbv); + g_assert(seenav); + g_assert(seensv); + g_assert(seentype); + + object_unparent(OBJECT(dobj)); } @@ -297,6 +342,7 @@ int main(int argc, char **argv) g_test_add_func("/qom/proplist/createv", test_dummy_createv); g_test_add_func("/qom/proplist/badenum", test_dummy_badenum); g_test_add_func("/qom/proplist/getenum", test_dummy_getenum); + g_test_add_func("/qom/proplist/iterator", test_dummy_iterator); return g_test_run(); } From 1b30c094dcb69273b7661897c067906f81e5b967 Mon Sep 17 00:00:00 2001 From: "Daniel P. Berrange" Date: Tue, 13 Oct 2015 13:37:41 +0100 Subject: [PATCH 03/10] qmp: Convert QMP code to use object property iterators MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stop directly accessing the Object::properties field data structure and instead use the formal object property iterator APIs. This insulates the code from future data structure changes in the Object struct. Signed-off-by: Daniel P. Berrange Tested-by: Pavel Fedin Signed-off-by: Andreas Färber --- qmp.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/qmp.c b/qmp.c index ddc63ea9f6..0a1fa19925 100644 --- a/qmp.c +++ b/qmp.c @@ -210,6 +210,7 @@ ObjectPropertyInfoList *qmp_qom_list(const char *path, Error **errp) bool ambiguous = false; ObjectPropertyInfoList *props = NULL; ObjectProperty *prop; + ObjectPropertyIterator *iter; obj = object_resolve_path(path, &ambiguous); if (obj == NULL) { @@ -222,7 +223,8 @@ ObjectPropertyInfoList *qmp_qom_list(const char *path, Error **errp) return NULL; } - QTAILQ_FOREACH(prop, &obj->properties, node) { + iter = object_property_iter_init(obj); + while ((prop = object_property_iter_next(iter))) { ObjectPropertyInfoList *entry = g_malloc0(sizeof(*entry)); entry->value = g_malloc0(sizeof(ObjectPropertyInfo)); @@ -232,6 +234,7 @@ ObjectPropertyInfoList *qmp_qom_list(const char *path, Error **errp) entry->value->name = g_strdup(prop->name); entry->value->type = g_strdup(prop->type); } + object_property_iter_free(iter); return props; } @@ -503,6 +506,7 @@ DevicePropertyInfoList *qmp_device_list_properties(const char *typename, ObjectClass *klass; Object *obj; ObjectProperty *prop; + ObjectPropertyIterator *iter; DevicePropertyInfoList *prop_list = NULL; klass = object_class_by_name(typename); @@ -531,7 +535,8 @@ DevicePropertyInfoList *qmp_device_list_properties(const char *typename, obj = object_new(typename); - QTAILQ_FOREACH(prop, &obj->properties, node) { + iter = object_property_iter_init(obj); + while ((prop = object_property_iter_next(iter))) { DevicePropertyInfo *info; DevicePropertyInfoList *entry; @@ -562,6 +567,7 @@ DevicePropertyInfoList *qmp_device_list_properties(const char *typename, entry->next = prop_list; prop_list = entry; } + object_property_iter_free(iter); object_unref(obj); From 2465bc564d39d9b62fa21b3e84313be3b32dbc16 Mon Sep 17 00:00:00 2001 From: "Daniel P. Berrange" Date: Tue, 13 Oct 2015 13:37:42 +0100 Subject: [PATCH 04/10] vl: Convert machine help code to use object property iterators MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stop directly accessing the Object::properties field data structure and instead use the formal object property iterator APIs. This insulates the code from future data structure changes in the Object struct. Signed-off-by: Daniel P. Berrange Tested-by: Pavel Fedin Signed-off-by: Andreas Färber --- vl.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/vl.c b/vl.c index 7d993a5243..4211ff1ffd 100644 --- a/vl.c +++ b/vl.c @@ -1536,12 +1536,14 @@ MachineInfoList *qmp_query_machines(Error **errp) static int machine_help_func(QemuOpts *opts, MachineState *machine) { ObjectProperty *prop; + ObjectPropertyIterator *iter; if (!qemu_opt_has_help_opt(opts)) { return 0; } - QTAILQ_FOREACH(prop, &OBJECT(machine)->properties, node) { + iter = object_property_iter_init(OBJECT(machine)); + while ((prop = object_property_iter_next(iter))) { if (!prop->set) { continue; } @@ -1554,6 +1556,7 @@ static int machine_help_func(QemuOpts *opts, MachineState *machine) error_printf("\n"); } } + object_property_iter_free(iter); return 1; } From 9a842f7d3ce421b39c7edbfe2c47efeac5db6c28 Mon Sep 17 00:00:00 2001 From: "Daniel P. Berrange" Date: Tue, 13 Oct 2015 13:37:43 +0100 Subject: [PATCH 05/10] ppc: Convert spapr code to use object property iterators MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stop directly accessing the Object::properties field data structure and instead use the formal object property iterator APIs. This insulates the code from future data structure changes in the Object struct. Signed-off-by: Daniel P. Berrange Tested-by: Pavel Fedin Signed-off-by: Andreas Färber --- hw/ppc/spapr_drc.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c index 5d6ea7ce41..f34bc04c48 100644 --- a/hw/ppc/spapr_drc.c +++ b/hw/ppc/spapr_drc.c @@ -657,6 +657,7 @@ int spapr_drc_populate_dt(void *fdt, int fdt_offset, Object *owner, { Object *root_container; ObjectProperty *prop; + ObjectPropertyIterator *iter; uint32_t drc_count = 0; GArray *drc_indexes, *drc_power_domains; GString *drc_names, *drc_types; @@ -680,7 +681,8 @@ int spapr_drc_populate_dt(void *fdt, int fdt_offset, Object *owner, */ root_container = container_get(object_get_root(), DRC_CONTAINER_PATH); - QTAILQ_FOREACH(prop, &root_container->properties, node) { + iter = object_property_iter_init(root_container); + while ((prop = object_property_iter_next(iter))) { Object *obj; sPAPRDRConnector *drc; sPAPRDRConnectorClass *drck; @@ -721,6 +723,7 @@ int spapr_drc_populate_dt(void *fdt, int fdt_offset, Object *owner, spapr_drc_get_type_str(drc->type)); drc_types = g_string_insert_len(drc_types, -1, "\0", 1); } + object_property_iter_free(iter); /* now write the drc count into the space we reserved at the * beginning of the arrays previously From 456fb0bfe0b27c54d316be7fe3b362247f732656 Mon Sep 17 00:00:00 2001 From: "Daniel P. Berrange" Date: Tue, 13 Oct 2015 13:37:44 +0100 Subject: [PATCH 06/10] net: Convert net filter code to use object property iterators MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stop directly accessing the Object::properties field data structure and instead use the formal object property iterator APIs. This insulates the code from future data structure changes in the Object struct. Signed-off-by: Daniel P. Berrange Tested-by: Pavel Fedin Signed-off-by: Andreas Färber --- net/filter.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/net/filter.c b/net/filter.c index 326f2b5ac8..1365bad026 100644 --- a/net/filter.c +++ b/net/filter.c @@ -137,6 +137,7 @@ static void netfilter_complete(UserCreatable *uc, Error **errp) Error *local_err = NULL; char *str, *info; ObjectProperty *prop; + ObjectPropertyIterator *iter; StringOutputVisitor *ov; if (!nf->netdev_id) { @@ -173,7 +174,8 @@ static void netfilter_complete(UserCreatable *uc, Error **errp) QTAILQ_INSERT_TAIL(&nf->netdev->filters, nf, next); /* generate info str */ - QTAILQ_FOREACH(prop, &OBJECT(nf)->properties, node) { + iter = object_property_iter_init(OBJECT(nf)); + while ((prop = object_property_iter_next(iter))) { if (!strcmp(prop->name, "type")) { continue; } @@ -187,6 +189,7 @@ static void netfilter_complete(UserCreatable *uc, Error **errp) g_free(str); g_free(info); } + object_property_iter_free(iter); } static void netfilter_finalize(Object *obj) From 8c4d156c187c84b574d287bd4b9ddf9a6975de7c Mon Sep 17 00:00:00 2001 From: "Daniel P. Berrange" Date: Mon, 16 Nov 2015 15:37:34 +0000 Subject: [PATCH 07/10] qom: Add a test case for complex property finalization MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Devices have some quite complex object child/link relationships which place some requirements on the object_property_del_all() function to consider that properties can be modified while being iterated over. This extends the QOM property test case to replicate the device like structure and expose any potential bugs in the object_property_del_all() function. Signed-off-by: Daniel P. Berrange Signed-off-by: Andreas Färber --- tests/check-qom-proplist.c | 159 +++++++++++++++++++++++++++++++++++++ 1 file changed, 159 insertions(+) diff --git a/tests/check-qom-proplist.c b/tests/check-qom-proplist.c index 1be8b9e6f2..e674c0fa89 100644 --- a/tests/check-qom-proplist.c +++ b/tests/check-qom-proplist.c @@ -152,6 +152,148 @@ static const TypeInfo dummy_info = { .class_size = sizeof(DummyObjectClass), }; + +/* + * The following 3 object classes are used to + * simulate the kind of relationships seen in + * qdev, which result in complex object + * property destruction ordering. + * + * DummyDev has a 'bus' child to a DummyBus + * DummyBus has a 'backend' child to a DummyBackend + * DummyDev has a 'backend' link to DummyBackend + * + * When DummyDev is finalized, it unparents the + * DummyBackend, which unparents the DummyDev + * which deletes the 'backend' link from DummyDev + * to DummyBackend. This illustrates that the + * object_property_del_all() method needs to + * cope with the list of properties being changed + * while it iterates over them. + */ +typedef struct DummyDev DummyDev; +typedef struct DummyDevClass DummyDevClass; +typedef struct DummyBus DummyBus; +typedef struct DummyBusClass DummyBusClass; +typedef struct DummyBackend DummyBackend; +typedef struct DummyBackendClass DummyBackendClass; + +#define TYPE_DUMMY_DEV "qemu-dummy-dev" +#define TYPE_DUMMY_BUS "qemu-dummy-bus" +#define TYPE_DUMMY_BACKEND "qemu-dummy-backend" + +#define DUMMY_DEV(obj) \ + OBJECT_CHECK(DummyDev, (obj), TYPE_DUMMY_DEV) +#define DUMMY_BUS(obj) \ + OBJECT_CHECK(DummyBus, (obj), TYPE_DUMMY_BUS) +#define DUMMY_BACKEND(obj) \ + OBJECT_CHECK(DummyBackend, (obj), TYPE_DUMMY_BACKEND) + +struct DummyDev { + Object parent_obj; + + DummyBus *bus; +}; + +struct DummyDevClass { + ObjectClass parent_class; +}; + +struct DummyBus { + Object parent_obj; + + DummyBackend *backend; +}; + +struct DummyBusClass { + ObjectClass parent_class; +}; + +struct DummyBackend { + Object parent_obj; +}; + +struct DummyBackendClass { + ObjectClass parent_class; +}; + + +static void dummy_dev_init(Object *obj) +{ + DummyDev *dev = DUMMY_DEV(obj); + DummyBus *bus = DUMMY_BUS(object_new(TYPE_DUMMY_BUS)); + DummyBackend *backend = DUMMY_BACKEND(object_new(TYPE_DUMMY_BACKEND)); + + object_property_add_child(obj, "bus", OBJECT(bus), NULL); + dev->bus = bus; + object_property_add_child(OBJECT(bus), "backend", OBJECT(backend), NULL); + bus->backend = backend; + + object_property_add_link(obj, "backend", TYPE_DUMMY_BACKEND, + (Object **)&bus->backend, NULL, 0, NULL); +} + +static void dummy_dev_unparent(Object *obj) +{ + DummyDev *dev = DUMMY_DEV(obj); + object_unparent(OBJECT(dev->bus)); +} + +static void dummy_dev_class_init(ObjectClass *klass, void *opaque) +{ + klass->unparent = dummy_dev_unparent; +} + + +static void dummy_bus_init(Object *obj) +{ +} + +static void dummy_bus_unparent(Object *obj) +{ + DummyBus *bus = DUMMY_BUS(obj); + object_property_del(obj->parent, "backend", NULL); + object_unparent(OBJECT(bus->backend)); +} + +static void dummy_bus_class_init(ObjectClass *klass, void *opaque) +{ + klass->unparent = dummy_bus_unparent; +} + +static void dummy_backend_init(Object *obj) +{ +} + + +static const TypeInfo dummy_dev_info = { + .name = TYPE_DUMMY_DEV, + .parent = TYPE_OBJECT, + .instance_size = sizeof(DummyDev), + .instance_init = dummy_dev_init, + .class_size = sizeof(DummyDevClass), + .class_init = dummy_dev_class_init, +}; + +static const TypeInfo dummy_bus_info = { + .name = TYPE_DUMMY_BUS, + .parent = TYPE_OBJECT, + .instance_size = sizeof(DummyBus), + .instance_init = dummy_bus_init, + .class_size = sizeof(DummyBusClass), + .class_init = dummy_bus_class_init, +}; + +static const TypeInfo dummy_backend_info = { + .name = TYPE_DUMMY_BACKEND, + .parent = TYPE_OBJECT, + .instance_size = sizeof(DummyBackend), + .instance_init = dummy_backend_init, + .class_size = sizeof(DummyBackendClass), +}; + + + static void test_dummy_createv(void) { Error *err = NULL; @@ -331,18 +473,35 @@ static void test_dummy_iterator(void) } +static void test_dummy_delchild(void) +{ + Object *parent = object_get_objects_root(); + DummyDev *dev = DUMMY_DEV( + object_new_with_props(TYPE_DUMMY_DEV, + parent, + "dev0", + &error_abort, + NULL)); + + object_unparent(OBJECT(dev)); +} + int main(int argc, char **argv) { g_test_init(&argc, &argv, NULL); module_call_init(MODULE_INIT_QOM); type_register_static(&dummy_info); + type_register_static(&dummy_dev_info); + type_register_static(&dummy_bus_info); + type_register_static(&dummy_backend_info); g_test_add_func("/qom/proplist/createlist", test_dummy_createlist); g_test_add_func("/qom/proplist/createv", test_dummy_createv); g_test_add_func("/qom/proplist/badenum", test_dummy_badenum); g_test_add_func("/qom/proplist/getenum", test_dummy_getenum); g_test_add_func("/qom/proplist/iterator", test_dummy_iterator); + g_test_add_func("/qom/proplist/delchild", test_dummy_delchild); return g_test_run(); } From b604a854e843505007c59d68112c654556102a20 Mon Sep 17 00:00:00 2001 From: Pavel Fedin Date: Tue, 13 Oct 2015 13:37:45 +0100 Subject: [PATCH 08/10] qom: Replace object property list with GHashTable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ARM GICv3 systems with large number of CPUs create lots of IRQ pins. Since every pin is represented as a property, number of these properties becomes very large. Every property add first makes sure there's no duplicates. Traversing the list becomes very slow, therefore QEMU initialization takes significant time (several seconds for e. g. 16 CPUs). This patch replaces list with GHashTable, making lookup very fast. The only drawback is that object_child_foreach() and object_child_foreach_recursive() cannot add or remove properties during traversal, since GHashTableIter does not have modify-safe version. However, the code seems not to modify objects via these functions. Signed-off-by: Pavel Fedin Signed-off-by: Daniel P. Berrange Tested-by: Pavel Fedin [AF: Fixed object_property_del_{all,child}() issues; g_hash_table_contains() -> g_hash_table_lookup(), suggested by Daniel] Reviewed-by: Daniel P. Berrange Signed-off-by: Andreas Färber --- include/qom/object.h | 10 ++-- qom/object.c | 118 +++++++++++++++++++++++++++---------------- 2 files changed, 82 insertions(+), 46 deletions(-) diff --git a/include/qom/object.h b/include/qom/object.h index 9f703140bd..f172fea0b6 100644 --- a/include/qom/object.h +++ b/include/qom/object.h @@ -344,8 +344,6 @@ typedef struct ObjectProperty ObjectPropertyResolve *resolve; ObjectPropertyRelease *release; void *opaque; - - QTAILQ_ENTRY(ObjectProperty) node; } ObjectProperty; /** @@ -405,7 +403,7 @@ struct Object /*< private >*/ ObjectClass *class; ObjectFree *free; - QTAILQ_HEAD(, ObjectProperty) properties; + GHashTable *properties; uint32_t ref; Object *parent; }; @@ -1537,6 +1535,9 @@ void object_property_set_description(Object *obj, const char *name, * Call @fn passing each child of @obj and @opaque to it, until @fn returns * non-zero. * + * It is forbidden to add or remove children from @obj from the @fn + * callback. + * * Returns: The last value returned by @fn, or 0 if there is no child. */ int object_child_foreach(Object *obj, int (*fn)(Object *child, void *opaque), @@ -1552,6 +1553,9 @@ int object_child_foreach(Object *obj, int (*fn)(Object *child, void *opaque), * non-zero. Calls recursively, all child nodes of @obj will also be passed * all the way down to the leaf nodes of the tree. Depth first ordering. * + * It is forbidden to add or remove children from @obj (or its + * child nodes) from the @fn callback. + * * Returns: The last value returned by @fn, or 0 if there is no child. */ int object_child_foreach_recursive(Object *obj, diff --git a/qom/object.c b/qom/object.c index 1c926ceea1..ff34c86d5b 100644 --- a/qom/object.c +++ b/qom/object.c @@ -68,7 +68,7 @@ struct TypeImpl }; struct ObjectPropertyIterator { - ObjectProperty *next; + GHashTableIter iter; }; static Type type_interface; @@ -330,6 +330,16 @@ static void object_post_init_with_type(Object *obj, TypeImpl *ti) } } +static void object_property_free(gpointer data) +{ + ObjectProperty *prop = data; + + g_free(prop->name); + g_free(prop->type); + g_free(prop->description); + g_free(prop); +} + void object_initialize_with_type(void *data, size_t size, TypeImpl *type) { Object *obj = data; @@ -344,7 +354,8 @@ void object_initialize_with_type(void *data, size_t size, TypeImpl *type) memset(obj, 0, type->instance_size); obj->class = type->class; object_ref(obj); - QTAILQ_INIT(&obj->properties); + obj->properties = g_hash_table_new_full(g_str_hash, g_str_equal, + NULL, object_property_free); object_init_with_type(obj, type); object_post_init_with_type(obj, type); } @@ -363,29 +374,51 @@ static inline bool object_property_is_child(ObjectProperty *prop) static void object_property_del_all(Object *obj) { - while (!QTAILQ_EMPTY(&obj->properties)) { - ObjectProperty *prop = QTAILQ_FIRST(&obj->properties); + ObjectProperty *prop; + GHashTableIter iter; + gpointer key, value; + bool released; - QTAILQ_REMOVE(&obj->properties, prop, node); - - if (prop->release) { - prop->release(obj, prop->name, prop->opaque); + do { + released = false; + g_hash_table_iter_init(&iter, obj->properties); + while (g_hash_table_iter_next(&iter, &key, &value)) { + prop = value; + if (prop->release) { + prop->release(obj, prop->name, prop->opaque); + prop->release = NULL; + released = true; + break; + } + g_hash_table_iter_remove(&iter); } + } while (released); - g_free(prop->name); - g_free(prop->type); - g_free(prop->description); - g_free(prop); - } + g_hash_table_unref(obj->properties); } static void object_property_del_child(Object *obj, Object *child, Error **errp) { ObjectProperty *prop; + GHashTableIter iter; + gpointer key, value; - QTAILQ_FOREACH(prop, &obj->properties, node) { + g_hash_table_iter_init(&iter, obj->properties); + while (g_hash_table_iter_next(&iter, &key, &value)) { + prop = value; if (object_property_is_child(prop) && prop->opaque == child) { - object_property_del(obj, prop->name, errp); + if (prop->release) { + prop->release(obj, prop->name, prop->opaque); + prop->release = NULL; + } + break; + } + } + g_hash_table_iter_init(&iter, obj->properties); + while (g_hash_table_iter_next(&iter, &key, &value)) { + prop = value; + if (object_property_is_child(prop) && prop->opaque == child) { + g_hash_table_iter_remove(&iter); break; } } @@ -783,10 +816,12 @@ static int do_object_child_foreach(Object *obj, int (*fn)(Object *child, void *opaque), void *opaque, bool recurse) { - ObjectProperty *prop, *next; + GHashTableIter iter; + ObjectProperty *prop; int ret = 0; - QTAILQ_FOREACH_SAFE(prop, &obj->properties, node, next) { + g_hash_table_iter_init(&iter, obj->properties); + while (g_hash_table_iter_next(&iter, NULL, (gpointer *)&prop)) { if (object_property_is_child(prop)) { Object *child = prop->opaque; @@ -883,13 +918,11 @@ object_property_add(Object *obj, const char *name, const char *type, return ret; } - QTAILQ_FOREACH(prop, &obj->properties, node) { - if (strcmp(prop->name, name) == 0) { - error_setg(errp, "attempt to add duplicate property '%s'" + if (g_hash_table_lookup(obj->properties, name) != NULL) { + error_setg(errp, "attempt to add duplicate property '%s'" " to object (type '%s')", name, object_get_typename(obj)); - return NULL; - } + return NULL; } prop = g_malloc0(sizeof(*prop)); @@ -902,7 +935,7 @@ object_property_add(Object *obj, const char *name, const char *type, prop->release = release; prop->opaque = opaque; - QTAILQ_INSERT_TAIL(&obj->properties, prop, node); + g_hash_table_insert(obj->properties, prop->name, prop); return prop; } @@ -911,10 +944,9 @@ ObjectProperty *object_property_find(Object *obj, const char *name, { ObjectProperty *prop; - QTAILQ_FOREACH(prop, &obj->properties, node) { - if (strcmp(prop->name, name) == 0) { - return prop; - } + prop = g_hash_table_lookup(obj->properties, name); + if (prop) { + return prop; } error_setg(errp, "Property '.%s' not found", name); @@ -924,7 +956,7 @@ ObjectProperty *object_property_find(Object *obj, const char *name, ObjectPropertyIterator *object_property_iter_init(Object *obj) { ObjectPropertyIterator *ret = g_new0(ObjectPropertyIterator, 1); - ret->next = QTAILQ_FIRST(&obj->properties); + g_hash_table_iter_init(&ret->iter, obj->properties); return ret; } @@ -938,30 +970,26 @@ void object_property_iter_free(ObjectPropertyIterator *iter) ObjectProperty *object_property_iter_next(ObjectPropertyIterator *iter) { - ObjectProperty *ret = iter->next; - if (ret) { - iter->next = QTAILQ_NEXT(iter->next, node); + gpointer key, val; + if (!g_hash_table_iter_next(&iter->iter, &key, &val)) { + return NULL; } - return ret; + return val; } void object_property_del(Object *obj, const char *name, Error **errp) { - ObjectProperty *prop = object_property_find(obj, name, errp); - if (prop == NULL) { + ObjectProperty *prop = g_hash_table_lookup(obj->properties, name); + + if (!prop) { + error_setg(errp, "Property '.%s' not found", name); return; } if (prop->release) { prop->release(obj, name, prop->opaque); } - - QTAILQ_REMOVE(&obj->properties, prop, node); - - g_free(prop->name); - g_free(prop->type); - g_free(prop->description); - g_free(prop); + g_hash_table_remove(obj->properties, name); } void object_property_get(Object *obj, Visitor *v, const char *name, @@ -1481,11 +1509,13 @@ void object_property_add_const_link(Object *obj, const char *name, gchar *object_get_canonical_path_component(Object *obj) { ObjectProperty *prop = NULL; + GHashTableIter iter; g_assert(obj); g_assert(obj->parent != NULL); - QTAILQ_FOREACH(prop, &obj->parent->properties, node) { + g_hash_table_iter_init(&iter, obj->parent->properties); + while (g_hash_table_iter_next(&iter, NULL, (gpointer *)&prop)) { if (!object_property_is_child(prop)) { continue; } @@ -1569,11 +1599,13 @@ static Object *object_resolve_partial_path(Object *parent, bool *ambiguous) { Object *obj; + GHashTableIter iter; ObjectProperty *prop; obj = object_resolve_abs_path(parent, parts, typename, 0); - QTAILQ_FOREACH(prop, &parent->properties, node) { + g_hash_table_iter_init(&iter, parent->properties); + while (g_hash_table_iter_next(&iter, NULL, (gpointer *)&prop)) { Object *found; if (!object_property_is_child(prop)) { From 8438a13543a281e3e61542cc0763bf1b05169016 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andreas=20F=C3=A4rber?= Date: Mon, 16 Nov 2015 17:49:20 +0100 Subject: [PATCH 09/10] qom: Clean up assertions to display values on failure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Instead of using g_assert() for integer comparisons, use g_assert_cmpint() so that we can see the respective values. While at it, fix one stray indentation. Reviewed-by: Daniel P. Berrange Signed-off-by: Andreas Färber --- qom/object.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/qom/object.c b/qom/object.c index ff34c86d5b..d7515697a3 100644 --- a/qom/object.c +++ b/qom/object.c @@ -265,7 +265,7 @@ static void type_initialize(TypeImpl *ti) GSList *e; int i; - g_assert(parent->class_size <= ti->class_size); + g_assert_cmpint(parent->class_size, <=, ti->class_size); memcpy(ti->class, parent->class, parent->class_size); ti->class->interfaces = NULL; @@ -347,9 +347,9 @@ void object_initialize_with_type(void *data, size_t size, TypeImpl *type) g_assert(type != NULL); type_initialize(type); - g_assert(type->instance_size >= sizeof(Object)); + g_assert_cmpint(type->instance_size, >=, sizeof(Object)); g_assert(type->abstract == false); - g_assert(size >= type->instance_size); + g_assert_cmpint(size, >=, type->instance_size); memset(obj, 0, type->instance_size); obj->class = type->class; @@ -450,7 +450,7 @@ static void object_finalize(void *data) object_property_del_all(obj); object_deinit(obj, ti); - g_assert(obj->ref == 0); + g_assert_cmpint(obj->ref, ==, 0); if (obj->free) { obj->free(obj); } @@ -872,7 +872,7 @@ void object_ref(Object *obj) if (!obj) { return; } - atomic_inc(&obj->ref); + atomic_inc(&obj->ref); } void object_unref(Object *obj) @@ -880,7 +880,7 @@ void object_unref(Object *obj) if (!obj) { return; } - g_assert(obj->ref > 0); + g_assert_cmpint(obj->ref, >, 0); /* parent always holds a reference to its children */ if (atomic_fetch_dec(&obj->ref) == 1) { From 9f4aa7cef2214137db192b252f1d4fc1799d05c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andreas=20F=C3=A4rber?= Date: Wed, 18 Nov 2015 19:03:29 +0100 Subject: [PATCH 10/10] MAINTAINERS: Add check-qom-{interface,proplist} to QOM MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add the QOM unit tests to the QOM maintenance area so that maintainers get CC'ed on changes and to document QOM test coverage. Reviewed-by: Daniel P. Berrange Reviewed-by: Igor Mammedov Signed-off-by: Andreas Färber --- MAINTAINERS | 2 ++ 1 file changed, 2 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 9e1fa7236a..28f0139138 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1164,6 +1164,8 @@ F: include/qom/ X: include/qom/cpu.h F: qom/ X: qom/cpu.c +F: tests/check-qom-interface.c +F: tests/check-qom-proplist.c F: tests/qom-test.c QMP