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 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 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; diff --git a/include/qom/object.h b/include/qom/object.h index 0bb89d481e..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; }; @@ -960,6 +958,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); /** @@ -1488,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), @@ -1503,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/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) 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); diff --git a/qom/object.c b/qom/object.c index c0decb6e96..d7515697a3 100644 --- a/qom/object.c +++ b/qom/object.c @@ -67,6 +67,10 @@ struct TypeImpl InterfaceImpl interfaces[MAX_INTERFACES]; }; +struct ObjectPropertyIterator { + GHashTableIter iter; +}; + static Type type_interface; static GHashTable *type_table_get(void) @@ -261,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; @@ -326,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; @@ -333,14 +347,15 @@ 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; 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); } @@ -359,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; } } @@ -413,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); } @@ -779,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; @@ -833,7 +872,7 @@ void object_ref(Object *obj) if (!obj) { return; } - atomic_inc(&obj->ref); + atomic_inc(&obj->ref); } void object_unref(Object *obj) @@ -841,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) { @@ -879,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)); @@ -898,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; } @@ -907,33 +944,52 @@ 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); return NULL; } +ObjectPropertyIterator *object_property_iter_init(Object *obj) +{ + ObjectPropertyIterator *ret = g_new0(ObjectPropertyIterator, 1); + g_hash_table_iter_init(&ret->iter, obj->properties); + return ret; +} + +void object_property_iter_free(ObjectPropertyIterator *iter) +{ + if (!iter) { + return; + } + g_free(iter); +} + +ObjectProperty *object_property_iter_next(ObjectPropertyIterator *iter) +{ + gpointer key, val; + if (!g_hash_table_iter_next(&iter->iter, &key, &val)) { + return NULL; + } + 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, @@ -1453,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; } @@ -1541,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)) { diff --git a/tests/check-qom-proplist.c b/tests/check-qom-proplist.c index 7400b1fce9..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; @@ -283,20 +425,83 @@ 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)); +} + + +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(); } diff --git a/vl.c b/vl.c index f9c661a4d0..525929bc4b 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; }