diff mbox

[v4,7/7] qom: allow properties to be registered against classes

Message ID 1444739866-14798-8-git-send-email-berrange@redhat.com
State New
Headers show

Commit Message

Daniel P. Berrangé Oct. 13, 2015, 12:37 p.m. UTC
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 <berrange@redhat.com>
---
 include/qom/object.h       |  50 ++++++++++
 qom/object.c               | 237 ++++++++++++++++++++++++++++++++++++++++++---
 tests/check-qom-proplist.c |  31 ++++--
 3 files changed, 293 insertions(+), 25 deletions(-)

Comments

Pavel Fedin Oct. 13, 2015, 1:18 p.m. UTC | #1
Hello!

> -----Original Message-----
> From: qemu-devel-bounces+p.fedin=samsung.com@nongnu.org [mailto:qemu-devel-
> bounces+p.fedin=samsung.com@nongnu.org] On Behalf Of Daniel P. Berrange
> Sent: Tuesday, October 13, 2015 3:38 PM
> To: qemu-devel@nongnu.org
> 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 <berrange@redhat.com>
> ---
>  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
Andreas Färber Nov. 5, 2015, 6:12 p.m. UTC | #2
Am 13.10.2015 um 15:18 schrieb Pavel Fedin:
>> 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
[...]
>> @@ -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

There is no v5 fixing this yet? If agreement has been reached, I would
at least start applying from the front, to avoid another lengthy review
cycle; I could try to resolve the conflict myself if it's the only nit.

Regards,
Andreas

P.S. Pavel, please delete unneeded quotation from your reply, I only
found that one line of inline comment.
Daniel P. Berrangé Nov. 6, 2015, 9:32 a.m. UTC | #3
On Thu, Nov 05, 2015 at 07:12:39PM +0100, Andreas Färber wrote:
> Am 13.10.2015 um 15:18 schrieb Pavel Fedin:
> >> 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
> [...]
> >> @@ -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
> 
> There is no v5 fixing this yet? If agreement has been reached, I would
> at least start applying from the front, to avoid another lengthy review
> cycle; I could try to resolve the conflict myself if it's the only nit.

Opps, I missed this comment. No, I've not sent a v5, but it looks like
it is just this comment that's the problem which is why I didn't catch
it during compilation.


Regards,
Daniel
Andreas Färber Nov. 18, 2015, 11:35 p.m. UTC | #4
Am 06.11.2015 um 10:32 schrieb Daniel P. Berrange:
> On Thu, Nov 05, 2015 at 07:12:39PM +0100, Andreas Färber wrote:
>> Am 13.10.2015 um 15:18 schrieb Pavel Fedin:
>>>> 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
>> [...]
>>>> @@ -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
>>
>> There is no v5 fixing this yet? If agreement has been reached, I would
>> at least start applying from the front, to avoid another lengthy review
>> cycle; I could try to resolve the conflict myself if it's the only nit.
> 
> Opps, I missed this comment. No, I've not sent a v5, but it looks like
> it is just this comment that's the problem which is why I didn't catch
> it during compilation.

Fixed up and queued on qom-next for next merge window:
https://github.com/afaerber/qemu-cpu/commits/qom-next

Thanks,
Andreas
diff mbox

Patch

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
  *
  * 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)