[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 7/7] qom: allow properties to be registered a
From: |
Pavel Fedin |
Subject: |
Re: [Qemu-devel] [PATCH v4 7/7] qom: allow properties to be registered against classes |
Date: |
Tue, 13 Oct 2015 16:18:28 +0300 |
Hello!
> -----Original Message-----
> From: address@hidden [mailto:qemu-devel-
> address@hidden On Behalf Of Daniel P. Berrange
> Sent: Tuesday, October 13, 2015 3:38 PM
> To: address@hidden
> Cc: Pavel Fedin; Markus Armbruster; Paolo Bonzini; Andreas Färber
> Subject: [Qemu-devel] [PATCH v4 7/7] qom: allow properties to be registered
> against classes
>
> When there are many instances of a given class, registering
> properties against the instance is wasteful of resources. The
> majority of objects have a statically defined list of possible
> properties, so most of the properties are easily registerable
> against the class. Only those properties which are conditionally
> registered at runtime need be recorded against the klass.
>
> Registering properties against classes also makes it possible
> to provide static introspection of QOM - currently introspection
> is only possible after creating an instance of a class, which
> severely limits its usefulness.
>
> This impl only supports simple scalar properties. It does not
> attempt to allow child object / link object properties against
> the class. There are ways to support those too, but it would
> make this patch more complicated, so it is left as an exercise
> for the future.
>
> There is no equivalent to object_property_del provided, since
> classes must be immutable once they are defined.
>
> Signed-off-by: Daniel P. Berrange <address@hidden>
> ---
> include/qom/object.h | 50 ++++++++++
> qom/object.c | 237
> ++++++++++++++++++++++++++++++++++++++++++---
> tests/check-qom-proplist.c | 31 ++++--
> 3 files changed, 293 insertions(+), 25 deletions(-)
>
> diff --git a/include/qom/object.h b/include/qom/object.h
> index 2a54515..38f41d3 100644
> --- a/include/qom/object.h
> +++ b/include/qom/object.h
> @@ -381,6 +381,8 @@ struct ObjectClass
> const char *class_cast_cache[OBJECT_CLASS_CAST_CACHE];
>
> ObjectUnparent *unparent;
> +
> + GHashTable *properties;
> };
>
> /**
> @@ -947,6 +949,13 @@ ObjectProperty *object_property_add(Object *obj, const
> char *name,
>
> void object_property_del(Object *obj, const char *name, Error **errp);
>
> +ObjectProperty *object_class_property_add(ObjectClass *klass, const char
> *name,
> + const char *type,
> + ObjectPropertyAccessor *get,
> + ObjectPropertyAccessor *set,
> + ObjectPropertyRelease *release,
> + void *opaque, Error **errp);
> +
> /**
> * object_property_find:
> * @obj: the object
> @@ -957,6 +966,8 @@ void object_property_del(Object *obj, const char *name,
> Error **errp);
> */
> ObjectProperty *object_property_find(Object *obj, const char *name,
> Error **errp);
> +ObjectProperty *object_class_property_find(ObjectClass *klass, const char
> *name,
> + Error **errp);
>
> typedef struct ObjectPropertyIterator ObjectPropertyIterator;
>
> @@ -964,8 +975,14 @@ typedef struct ObjectPropertyIterator
> ObjectPropertyIterator;
> * object_property_iter_init:
> * @obj: the object
> *
> +<<<<<<< HEAD
> * Initializes an iterator for traversing all properties
> * registered against an object instance.
> +=======
> + * Iterates over all properties defined against the object
> + * instance, its class and all parent classes, calling
> + * @iter for each property.
> +>>>>>>> 0ca9307... qom: allow properties to be registered against classes
Conflict markers slipped in the comment
> *
> * It is forbidden to modify the property list while iterating,
> * whether removing or adding properties.
> @@ -1375,6 +1392,12 @@ void object_property_add_str(Object *obj, const char
> *name,
> void (*set)(Object *, const char *, Error **),
> Error **errp);
>
> +void object_class_property_add_str(ObjectClass *klass, const char *name,
> + char *(*get)(Object *, Error **),
> + void (*set)(Object *, const char *,
> + Error **),
> + Error **errp);
> +
> /**
> * object_property_add_bool:
> * @obj: the object to add a property to
> @@ -1391,6 +1414,11 @@ void object_property_add_bool(Object *obj, const char
> *name,
> void (*set)(Object *, bool, Error **),
> Error **errp);
>
> +void object_class_property_add_bool(ObjectClass *klass, const char *name,
> + bool (*get)(Object *, Error **),
> + void (*set)(Object *, bool, Error **),
> + Error **errp);
> +
> /**
> * object_property_add_enum:
> * @obj: the object to add a property to
> @@ -1410,6 +1438,13 @@ void object_property_add_enum(Object *obj, const char
> *name,
> void (*set)(Object *, int, Error **),
> Error **errp);
>
> +void object_class_property_add_enum(ObjectClass *klass, const char *name,
> + const char *typename,
> + const char * const *strings,
> + int (*get)(Object *, Error **),
> + void (*set)(Object *, int, Error **),
> + Error **errp);
> +
> /**
> * object_property_add_tm:
> * @obj: the object to add a property to
> @@ -1424,6 +1459,10 @@ void object_property_add_tm(Object *obj, const char
> *name,
> void (*get)(Object *, struct tm *, Error **),
> Error **errp);
>
> +void object_class_property_add_tm(ObjectClass *klass, const char *name,
> + void (*get)(Object *, struct tm *, Error
> **),
> + Error **errp);
> +
> /**
> * object_property_add_uint8_ptr:
> * @obj: the object to add a property to
> @@ -1436,6 +1475,8 @@ void object_property_add_tm(Object *obj, const char
> *name,
> */
> void object_property_add_uint8_ptr(Object *obj, const char *name,
> const uint8_t *v, Error **errp);
> +void object_class_property_add_uint8_ptr(ObjectClass *klass, const char
> *name,
> + const uint8_t *v, Error **errp);
>
> /**
> * object_property_add_uint16_ptr:
> @@ -1449,6 +1490,8 @@ void object_property_add_uint8_ptr(Object *obj, const
> char *name,
> */
> void object_property_add_uint16_ptr(Object *obj, const char *name,
> const uint16_t *v, Error **errp);
> +void object_class_property_add_uint16_ptr(ObjectClass *klass, const char
> *name,
> + const uint16_t *v, Error **errp);
>
> /**
> * object_property_add_uint32_ptr:
> @@ -1462,6 +1505,8 @@ void object_property_add_uint16_ptr(Object *obj, const
> char *name,
> */
> void object_property_add_uint32_ptr(Object *obj, const char *name,
> const uint32_t *v, Error **errp);
> +void object_class_property_add_uint32_ptr(ObjectClass *klass, const char
> *name,
> + const uint32_t *v, Error **errp);
>
> /**
> * object_property_add_uint64_ptr:
> @@ -1475,6 +1520,8 @@ void object_property_add_uint32_ptr(Object *obj, const
> char *name,
> */
> void object_property_add_uint64_ptr(Object *obj, const char *name,
> const uint64_t *v, Error **Errp);
> +void object_class_property_add_uint64_ptr(ObjectClass *klass, const char
> *name,
> + const uint64_t *v, Error **Errp);
>
> /**
> * object_property_add_alias:
> @@ -1526,6 +1573,9 @@ void object_property_add_const_link(Object *obj, const
> char *name,
> */
> void object_property_set_description(Object *obj, const char *name,
> const char *description, Error **errp);
> +void object_class_property_set_description(ObjectClass *klass, const char
> *name,
> + const char *description,
> + Error **errp);
>
> /**
> * object_child_foreach:
> diff --git a/qom/object.c b/qom/object.c
> index dd01652..f41edf4 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -68,6 +68,7 @@ struct TypeImpl
> };
>
> struct ObjectPropertyIterator {
> + ObjectClass *nextclass;
> GHashTableIter iter;
> };
>
> @@ -246,6 +247,16 @@ static void type_initialize_interface(TypeImpl *ti,
> TypeImpl
> *interface_type,
> iface_impl->class);
> }
>
> +static void property_free(gpointer data)
> +{
> + ObjectProperty *prop = data;
> +
> + g_free(prop->name);
> + g_free(prop->type);
> + g_free(prop->description);
> + g_free(prop);
> +}
> +
> static void type_initialize(TypeImpl *ti)
> {
> TypeImpl *parent;
> @@ -268,6 +279,8 @@ static void type_initialize(TypeImpl *ti)
> g_assert(parent->class_size <= ti->class_size);
> memcpy(ti->class, parent->class, parent->class_size);
> ti->class->interfaces = NULL;
> + ti->class->properties = g_hash_table_new_full(
> + g_str_hash, g_str_equal, g_free, property_free);
>
> for (e = parent->class->interfaces; e; e = e->next) {
> InterfaceClass *iface = e->data;
> @@ -292,6 +305,9 @@ static void type_initialize(TypeImpl *ti)
>
> type_initialize_interface(ti, t, t);
> }
> + } else {
> + ti->class->properties = g_hash_table_new_full(
> + g_str_hash, g_str_equal, g_free, property_free);
> }
>
> ti->class->type = ti;
> @@ -330,16 +346,6 @@ static void object_post_init_with_type(Object *obj,
> TypeImpl *ti)
> }
> }
>
> -static void 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;
> @@ -902,10 +908,11 @@ object_property_add(Object *obj, const char *name,
> const char *type,
> return ret;
> }
>
> - if (g_hash_table_contains(obj->properties, name)) {
> +
> + if (object_property_find(obj, name, NULL) != NULL) {
> error_setg(errp, "attempt to add duplicate property '%s'"
> - " to object (type '%s')", name,
> - object_get_typename(obj));
> + " to object (type '%s')", name,
> + object_get_typename(obj));
> return NULL;
> }
>
> @@ -923,10 +930,50 @@ object_property_add(Object *obj, const char *name,
> const char *type,
> return prop;
> }
>
> +ObjectProperty *
> +object_class_property_add(ObjectClass *klass,
> + const char *name,
> + const char *type,
> + ObjectPropertyAccessor *get,
> + ObjectPropertyAccessor *set,
> + ObjectPropertyRelease *release,
> + void *opaque,
> + Error **errp)
> +{
> + ObjectProperty *prop;
> +
> + if (object_class_property_find(klass, name, NULL) != NULL) {
> + error_setg(errp, "attempt to add duplicate property '%s'"
> + " to object (type '%s')", name,
> + object_class_get_name(klass));
> + return NULL;
> + }
> +
> + prop = g_malloc0(sizeof(*prop));
> +
> + prop->name = g_strdup(name);
> + prop->type = g_strdup(type);
> +
> + prop->get = get;
> + prop->set = set;
> + prop->release = release;
> + prop->opaque = opaque;
> +
> + g_hash_table_insert(klass->properties, g_strdup(name), prop);
> +
> + return prop;
> +}
> +
> ObjectProperty *object_property_find(Object *obj, const char *name,
> Error **errp)
> {
> ObjectProperty *prop;
> + ObjectClass *klass = object_get_class(obj);
> +
> + prop = object_class_property_find(klass, name, NULL);
> + if (prop) {
> + return prop;
> + }
>
> prop = g_hash_table_lookup(obj->properties, name);
> if (prop) {
> @@ -941,6 +988,7 @@ ObjectPropertyIterator *object_property_iter_init(Object
> *obj)
> {
> ObjectPropertyIterator *ret = g_new0(ObjectPropertyIterator, 1);
> g_hash_table_iter_init(&ret->iter, obj->properties);
> + ret->nextclass = object_get_class(obj);
> return ret;
> }
>
> @@ -957,12 +1005,37 @@ void object_property_iter_free(ObjectPropertyIterator
> *iter)
> ObjectProperty *object_property_iter_next(ObjectPropertyIterator *iter)
> {
> gpointer key, val;
> - if (!g_hash_table_iter_next(&iter->iter, &key, &val)) {
> - return NULL;
> + while (!g_hash_table_iter_next(&iter->iter, &key, &val)) {
> + if (!iter->nextclass) {
> + return NULL;
> + }
> + g_hash_table_iter_init(&iter->iter, iter->nextclass->properties);
> + iter->nextclass = object_class_get_parent(iter->nextclass);
> }
> return val;
> }
>
> +ObjectProperty *object_class_property_find(ObjectClass *klass, const char
> *name,
> + Error **errp)
> +{
> + ObjectProperty *prop;
> + ObjectClass *parent_klass;
> +
> + parent_klass = object_class_get_parent(klass);
> + if (parent_klass) {
> + prop = object_class_property_find(parent_klass, name, NULL);
> + if (prop) {
> + return prop;
> + }
> + }
> +
> + prop = g_hash_table_lookup(klass->properties, name);
> + if (!prop) {
> + error_setg(errp, "Property '.%s' not found", name);
> + }
> + return prop;
> +}
> +
>
> void object_property_del(Object *obj, const char *name, Error **errp)
> {
> @@ -1717,6 +1790,29 @@ void object_property_add_str(Object *obj, const char
> *name,
> }
> }
>
> +void object_class_property_add_str(ObjectClass *klass, const char *name,
> + char *(*get)(Object *, Error **),
> + void (*set)(Object *, const char *,
> + Error **),
> + Error **errp)
> +{
> + Error *local_err = NULL;
> + StringProperty *prop = g_malloc0(sizeof(*prop));
> +
> + prop->get = get;
> + prop->set = set;
> +
> + object_class_property_add(klass, name, "string",
> + get ? property_get_str : NULL,
> + set ? property_set_str : NULL,
> + property_release_str,
> + prop, &local_err);
> + if (local_err) {
> + error_propagate(errp, local_err);
> + g_free(prop);
> + }
> +}
> +
> typedef struct BoolProperty
> {
> bool (*get)(Object *, Error **);
> @@ -1784,6 +1880,28 @@ void object_property_add_bool(Object *obj, const char
> *name,
> }
> }
>
> +void object_class_property_add_bool(ObjectClass *klass, const char *name,
> + bool (*get)(Object *, Error **),
> + void (*set)(Object *, bool, Error **),
> + Error **errp)
> +{
> + Error *local_err = NULL;
> + BoolProperty *prop = g_malloc0(sizeof(*prop));
> +
> + prop->get = get;
> + prop->set = set;
> +
> + object_class_property_add(klass, name, "bool",
> + get ? property_get_bool : NULL,
> + set ? property_set_bool : NULL,
> + property_release_bool,
> + prop, &local_err);
> + if (local_err) {
> + error_propagate(errp, local_err);
> + g_free(prop);
> + }
> +}
> +
> static void property_get_enum(Object *obj, Visitor *v, void *opaque,
> const char *name, Error **errp)
> {
> @@ -1847,6 +1965,31 @@ void object_property_add_enum(Object *obj, const char
> *name,
> }
> }
>
> +void object_class_property_add_enum(ObjectClass *klass, const char *name,
> + const char *typename,
> + const char * const *strings,
> + int (*get)(Object *, Error **),
> + void (*set)(Object *, int, Error **),
> + Error **errp)
> +{
> + Error *local_err = NULL;
> + EnumProperty *prop = g_malloc(sizeof(*prop));
> +
> + prop->strings = strings;
> + prop->get = get;
> + prop->set = set;
> +
> + object_class_property_add(klass, name, typename,
> + get ? property_get_enum : NULL,
> + set ? property_set_enum : NULL,
> + property_release_enum,
> + prop, &local_err);
> + if (local_err) {
> + error_propagate(errp, local_err);
> + g_free(prop);
> + }
> +}
> +
> typedef struct TMProperty {
> void (*get)(Object *, struct tm *, Error **);
> } TMProperty;
> @@ -1926,6 +2069,25 @@ void object_property_add_tm(Object *obj, const char
> *name,
> }
> }
>
> +void object_class_property_add_tm(ObjectClass *klass, const char *name,
> + void (*get)(Object *, struct tm *, Error
> **),
> + Error **errp)
> +{
> + Error *local_err = NULL;
> + TMProperty *prop = g_malloc0(sizeof(*prop));
> +
> + prop->get = get;
> +
> + object_class_property_add(klass, name, "struct tm",
> + get ? property_get_tm : NULL, NULL,
> + property_release_tm,
> + prop, &local_err);
> + if (local_err) {
> + error_propagate(errp, local_err);
> + g_free(prop);
> + }
> +}
> +
> static char *qdev_get_type(Object *obj, Error **errp)
> {
> return g_strdup(object_get_typename(obj));
> @@ -1970,6 +2132,13 @@ void object_property_add_uint8_ptr(Object *obj, const
> char *name,
> NULL, NULL, (void *)v, errp);
> }
>
> +void object_class_property_add_uint8_ptr(ObjectClass *klass, const char
> *name,
> + const uint8_t *v, Error **errp)
> +{
> + object_class_property_add(klass, name, "uint8", property_get_uint8_ptr,
> + NULL, NULL, (void *)v, errp);
> +}
> +
> void object_property_add_uint16_ptr(Object *obj, const char *name,
> const uint16_t *v, Error **errp)
> {
> @@ -1977,6 +2146,13 @@ void object_property_add_uint16_ptr(Object *obj, const
> char *name,
> NULL, NULL, (void *)v, errp);
> }
>
> +void object_class_property_add_uint16_ptr(ObjectClass *klass, const char
> *name,
> + const uint16_t *v, Error **errp)
> +{
> + object_class_property_add(klass, name, "uint16", property_get_uint16_ptr,
> + NULL, NULL, (void *)v, errp);
> +}
> +
> void object_property_add_uint32_ptr(Object *obj, const char *name,
> const uint32_t *v, Error **errp)
> {
> @@ -1984,6 +2160,13 @@ void object_property_add_uint32_ptr(Object *obj, const
> char *name,
> NULL, NULL, (void *)v, errp);
> }
>
> +void object_class_property_add_uint32_ptr(ObjectClass *klass, const char
> *name,
> + const uint32_t *v, Error **errp)
> +{
> + object_class_property_add(klass, name, "uint32", property_get_uint32_ptr,
> + NULL, NULL, (void *)v, errp);
> +}
> +
> void object_property_add_uint64_ptr(Object *obj, const char *name,
> const uint64_t *v, Error **errp)
> {
> @@ -1991,6 +2174,13 @@ void object_property_add_uint64_ptr(Object *obj, const
> char *name,
> NULL, NULL, (void *)v, errp);
> }
>
> +void object_class_property_add_uint64_ptr(ObjectClass *klass, const char
> *name,
> + const uint64_t *v, Error **errp)
> +{
> + object_class_property_add(klass, name, "uint64", property_get_uint64_ptr,
> + NULL, NULL, (void *)v, errp);
> +}
> +
> typedef struct {
> Object *target_obj;
> char *target_name;
> @@ -2088,6 +2278,23 @@ void object_property_set_description(Object *obj,
> const char *name,
> op->description = g_strdup(description);
> }
>
> +void object_class_property_set_description(ObjectClass *klass,
> + const char *name,
> + const char *description,
> + Error **errp)
> +{
> + ObjectProperty *op;
> +
> + op = g_hash_table_lookup(klass->properties, name);
> + if (!op) {
> + error_setg(errp, "Property '.%s' not found", name);
> + return;
> + }
> +
> + g_free(op->description);
> + op->description = g_strdup(description);
> +}
> +
> static void object_instance_init(Object *obj)
> {
> object_property_add_str(obj, "type", qdev_get_type, NULL, NULL);
> diff --git a/tests/check-qom-proplist.c b/tests/check-qom-proplist.c
> index 1be8b9e..87de80b 100644
> --- a/tests/check-qom-proplist.c
> +++ b/tests/check-qom-proplist.c
> @@ -123,18 +123,28 @@ static void dummy_init(Object *obj)
> dummy_get_bv,
> dummy_set_bv,
> NULL);
> - object_property_add_str(obj, "sv",
> - dummy_get_sv,
> - dummy_set_sv,
> - NULL);
> - object_property_add_enum(obj, "av",
> - "DummyAnimal",
> - dummy_animal_map,
> - dummy_get_av,
> - dummy_set_av,
> - NULL);
> }
>
> +
> +static void dummy_class_init(ObjectClass *cls, void *data)
> +{
> + object_class_property_add_bool(cls, "bv",
> + dummy_get_bv,
> + dummy_set_bv,
> + NULL);
> + object_class_property_add_str(cls, "sv",
> + dummy_get_sv,
> + dummy_set_sv,
> + NULL);
> + object_class_property_add_enum(cls, "av",
> + "DummyAnimal",
> + dummy_animal_map,
> + dummy_get_av,
> + dummy_set_av,
> + NULL);
> +}
> +
> +
> static void dummy_finalize(Object *obj)
> {
> DummyObject *dobj = DUMMY_OBJECT(obj);
> @@ -150,6 +160,7 @@ static const TypeInfo dummy_info = {
> .instance_init = dummy_init,
> .instance_finalize = dummy_finalize,
> .class_size = sizeof(DummyObjectClass),
> + .class_init = dummy_class_init,
> };
>
> static void test_dummy_createv(void)
> --
> 2.4.3
Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia
- [Qemu-devel] [PATCH v4 0/7] qom: more efficient object property handling, Daniel P. Berrange, 2015/10/13
- [Qemu-devel] [PATCH v4 2/7] qmp: convert QMP code to use object property iterators, Daniel P. Berrange, 2015/10/13
- [Qemu-devel] [PATCH v4 5/7] net: convert net filter code to use object property iterators, Daniel P. Berrange, 2015/10/13
- [Qemu-devel] [PATCH v4 3/7] vl: convert machine help code to use object property iterators, Daniel P. Berrange, 2015/10/13
- [Qemu-devel] [PATCH v4 7/7] qom: allow properties to be registered against classes, Daniel P. Berrange, 2015/10/13
- Re: [Qemu-devel] [PATCH v4 7/7] qom: allow properties to be registered against classes,
Pavel Fedin <=
- [Qemu-devel] [PATCH v4 6/7] qom: replace object property list with GHashTable, Daniel P. Berrange, 2015/10/13
- Re: [Qemu-devel] [PATCH v4 0/7] qom: more efficient object property handling, Andreas Färber, 2015/10/13
- Re: [Qemu-devel] [PATCH v4 0/7] qom: more efficient object property handling, Pavel Fedin, 2015/10/14
- [Qemu-devel] [PATCH v4 1/7] qom: introduce ObjectPropertyIterator struct for iteration, Daniel P. Berrange, 2015/10/15
- [Qemu-devel] [PATCH v4 4/7] ppc: convert spapr code to use object property iterators, Daniel P. Berrange, 2015/10/15