All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH RFC 0/7] Making QOM introspectable
@ 2015-08-26 12:03 Daniel P. Berrange
  2015-08-26 12:03 ` [Qemu-devel] [PATCH RFC 1/7] qom: allow properties to be registered against classes Daniel P. Berrange
                   ` (7 more replies)
  0 siblings, 8 replies; 26+ messages in thread
From: Daniel P. Berrange @ 2015-08-26 12:03 UTC (permalink / raw)
  To: qemu-devel; +Cc: Markus Armbruster, Andreas Färber

There are many problems in QEMU related to introspection
of features, which Markus has been attacking/slaying for
a while. One of the remaining elephants in the corner of
the room which I've not seen work on is QOM.

QOM has a nice class/object framework, kind of like GLib's
GObject, but with the difference that properties are
registered against objects rather than classes. IIRC the
rationale for this is that in some cases the properties
registered are going to be dynamically decided at runtime,
so it isn't possible to statically define them against
classes. Now this may well be true of some properties,
but there are equally plenty of cases where the properties
/are/ invariant and could be registered against classes.

There are two core problems with registering properties
against object instances:

 - It is wasteful of memory to duplicate the allocation
   of ObjectProperty structs against each object
   instance. When you have N object instances, you
   have O(N) memory usage, instead of O(1). This is not
   a problem for objects which are mostly singletons,
   but there are cases in QEMU where you instantiate
   many instances of the same class and/or have many
   properties.

 - It prevents static introspection. Since the property
   is only registered in the object initializer, there
   is no way to programmatically query what properties
   an object supports without first instantiating it.
   Taking machine types as an example, if you want to
   introspect every machine type supported by a QEMU
   binary you thus have to launch QEMU many times,
   passing a different -M argument each time. As the
   number of different machine types & objects
   increases this quickly becomes impractical.

This series thus extends QOM to make it possible to register
properties against the classes, in addition to against object
instances. When looking up a property, a search will be done
starting at the base class, then each subclass in turn, and
finally against the object. Names are enforced to be unique
across the parent classes for sanity.

This only currently supports simple scalar properties where
the actual property value storage is managed by the object
instance. The object child and object link properties use
implicit storage in the ObjectProperty struct's 'opaque'
field, so we can't allow those against the class. Solving
this is doable, but more work, so is left as an exercise
for the future.

The first patch adds the neccessary QOM functionality. The
following 6 patches then illustrate the fairly trivial
conversions of a bunch of objects.

The eventual goal ought to be that everything is registered
against the class, leaving only the (hopefully few) cases
where per-instance properties are truely needed unconverted.

This series doesn't attempt to implement introspection
either - this would require a new QMP command such as
'qom-list-type-props' to query the properties against
classes.

I'm not really planning to spend much more time on this
myself. I'm just using this series to illustrate how I
believe the introspection problem in QOM can be fairly
easily addressed. Hopefully this will stimulate some
discussion & interest in doing the full job....

Daniel P. Berrange (7):
  qom: allow properties to be registered against classes
  hostmem: register properties against the class instead of object
  rng: register properties against the class instead of object
  tpm: register properties against the class instead of object
  cpu: avoid using object instance state in property getter
  x86-cpu: register properties against the class instead of object
  machine: register properties against the class instead of object

 backends/hostmem-file.c |  26 +++---
 backends/hostmem.c      |  41 ++++-----
 backends/rng-egd.c      |  12 +--
 backends/rng-random.c   |  10 +--
 backends/rng.c          |  14 ++-
 backends/tpm.c          |  12 +--
 hw/core/machine.c       | 193 +++++++++++++++++++--------------------
 include/qom/object.h    |  44 +++++++++
 qom/object.c            | 233 ++++++++++++++++++++++++++++++++++++++++++++++--
 target-i386/cpu.c       |  88 +++++++++++-------
 10 files changed, 476 insertions(+), 197 deletions(-)

-- 
2.4.3

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PATCH RFC 1/7] qom: allow properties to be registered against classes
  2015-08-26 12:03 [Qemu-devel] [PATCH RFC 0/7] Making QOM introspectable Daniel P. Berrange
@ 2015-08-26 12:03 ` Daniel P. Berrange
  2015-09-02 16:18   ` Andreas Färber
  2015-09-04 21:38   ` Marc-André Lureau
  2015-08-26 12:03 ` [Qemu-devel] [PATCH RFC 2/7] hostmem: register properties against the class instead of object Daniel P. Berrange
                   ` (6 subsequent siblings)
  7 siblings, 2 replies; 26+ messages in thread
From: Daniel P. Berrange @ 2015-08-26 12:03 UTC (permalink / raw)
  To: qemu-devel; +Cc: Markus Armbruster, Andreas Färber

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.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 include/qom/object.h |  44 ++++++++++
 qom/object.c         | 233 +++++++++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 270 insertions(+), 7 deletions(-)

diff --git a/include/qom/object.h b/include/qom/object.h
index 807978e..068162e 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -383,6 +383,8 @@ struct ObjectClass
     const char *class_cast_cache[OBJECT_CLASS_CAST_CACHE];
 
     ObjectUnparent *unparent;
+
+    QTAILQ_HEAD(, ObjectProperty) properties;
 };
 
 /**
@@ -949,6 +951,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
@@ -959,6 +968,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);
 
 void object_unparent(Object *obj);
 
@@ -1327,6 +1338,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
@@ -1343,6 +1360,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
@@ -1362,6 +1384,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
@@ -1376,6 +1405,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
@@ -1388,6 +1421,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:
@@ -1401,6 +1436,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:
@@ -1414,6 +1451,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:
@@ -1427,6 +1466,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:
@@ -1478,6 +1519,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 eea8edf..6977b43 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -264,6 +264,7 @@ 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;
+        QTAILQ_INIT(&ti->class->properties);
 
         for (e = parent->class->interfaces; e; e = e->next) {
             InterfaceClass *iface = e->data;
@@ -288,6 +289,8 @@ static void type_initialize(TypeImpl *ti)
 
             type_initialize_interface(ti, t, t);
         }
+    } else {
+        QTAILQ_INIT(&ti->class->properties);
     }
 
     ti->class->type = ti;
@@ -860,13 +863,12 @@ 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'"
-                       " to object (type '%s')", name,
-                       object_get_typename(obj));
-            return NULL;
-        }
+
+    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));
+        return NULL;
     }
 
     prop = g_malloc0(sizeof(*prop));
@@ -883,10 +885,70 @@ 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;
+    size_t name_len = strlen(name);
+
+    if (name_len >= 3 && !memcmp(name + name_len - 3, "[*]", 4)) {
+        int i;
+        ObjectProperty *ret;
+        char *name_no_array = g_strdup(name);
+
+        name_no_array[name_len - 3] = '\0';
+        for (i = 0; ; ++i) {
+            char *full_name = g_strdup_printf("%s[%d]", name_no_array, i);
+
+            ret = object_class_property_add(klass, full_name, type, get, set,
+                                            release, opaque, NULL);
+            g_free(full_name);
+            if (ret) {
+                break;
+            }
+        }
+        g_free(name_no_array);
+        return ret;
+    }
+
+    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;
+
+    QTAILQ_INSERT_TAIL(&klass->properties, prop, node);
+    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;
+    }
 
     QTAILQ_FOREACH(prop, &obj->properties, node) {
         if (strcmp(prop->name, name) == 0) {
@@ -898,6 +960,30 @@ ObjectProperty *object_property_find(Object *obj, const char *name,
     return NULL;
 }
 
+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;
+        }
+    }
+
+    QTAILQ_FOREACH(prop, &klass->properties, node) {
+        if (strcmp(prop->name, name) == 0) {
+            return prop;
+        }
+    }
+
+    error_setg(errp, "Property '.%s' not found", name);
+    return NULL;
+}
+
 void object_property_del(Object *obj, const char *name, Error **errp)
 {
     ObjectProperty *prop = object_property_find(obj, name, errp);
@@ -1635,6 +1721,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 **);
@@ -1696,6 +1805,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)
 {
@@ -1748,6 +1879,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;
@@ -1827,6 +1983,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));
@@ -1871,6 +2046,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)
 {
@@ -1878,6 +2060,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)
 {
@@ -1885,6 +2074,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)
 {
@@ -1892,6 +2088,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;
@@ -1989,6 +2192,22 @@ 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 = object_class_property_find(klass, name, errp);
+    if (!op) {
+        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);
-- 
2.4.3

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PATCH RFC 2/7] hostmem: register properties against the class instead of object
  2015-08-26 12:03 [Qemu-devel] [PATCH RFC 0/7] Making QOM introspectable Daniel P. Berrange
  2015-08-26 12:03 ` [Qemu-devel] [PATCH RFC 1/7] qom: allow properties to be registered against classes Daniel P. Berrange
@ 2015-08-26 12:03 ` Daniel P. Berrange
  2015-08-26 12:03 ` [Qemu-devel] [PATCH RFC 3/7] rng: " Daniel P. Berrange
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 26+ messages in thread
From: Daniel P. Berrange @ 2015-08-26 12:03 UTC (permalink / raw)
  To: qemu-devel; +Cc: Markus Armbruster, Andreas Färber

This converts the hostmem and hostmem-file objects to register
their properties against the class rather than object.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 backends/hostmem-file.c | 26 +++++++++++---------------
 backends/hostmem.c      | 41 +++++++++++++++++++++--------------------
 2 files changed, 32 insertions(+), 35 deletions(-)

diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
index 4b55361..4e528d9 100644
--- a/backends/hostmem-file.c
+++ b/backends/hostmem-file.c
@@ -59,14 +59,6 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
 #endif
 }
 
-static void
-file_backend_class_init(ObjectClass *oc, void *data)
-{
-    HostMemoryBackendClass *bc = MEMORY_BACKEND_CLASS(oc);
-
-    bc->alloc = file_backend_memory_alloc;
-}
-
 static char *get_mem_path(Object *o, Error **errp)
 {
     HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(o);
@@ -108,21 +100,25 @@ static void file_memory_backend_set_share(Object *o, bool value, Error **errp)
     fb->share = value;
 }
 
+
 static void
-file_backend_instance_init(Object *o)
+file_backend_class_init(ObjectClass *oc, void *data)
 {
-    object_property_add_bool(o, "share",
-                        file_memory_backend_get_share,
-                        file_memory_backend_set_share, NULL);
-    object_property_add_str(o, "mem-path", get_mem_path,
-                            set_mem_path, NULL);
+    HostMemoryBackendClass *bc = MEMORY_BACKEND_CLASS(oc);
+
+    bc->alloc = file_backend_memory_alloc;
+
+    object_class_property_add_bool(oc, "share",
+                                   file_memory_backend_get_share,
+                                   file_memory_backend_set_share, NULL);
+    object_class_property_add_str(oc, "mem-path", get_mem_path,
+                                  set_mem_path, NULL);
 }
 
 static const TypeInfo file_backend_info = {
     .name = TYPE_MEMORY_BACKEND_FILE,
     .parent = TYPE_MEMORY_BACKEND,
     .class_init = file_backend_class_init,
-    .instance_init = file_backend_instance_init,
     .instance_size = sizeof(HostMemoryBackendFile),
 };
 
diff --git a/backends/hostmem.c b/backends/hostmem.c
index 41ba2af..ca5448b 100644
--- a/backends/hostmem.c
+++ b/backends/hostmem.c
@@ -228,26 +228,6 @@ static void host_memory_backend_init(Object *obj)
     backend->merge = machine_mem_merge(machine);
     backend->dump = machine_dump_guest_core(machine);
     backend->prealloc = mem_prealloc;
-
-    object_property_add_bool(obj, "merge",
-                        host_memory_backend_get_merge,
-                        host_memory_backend_set_merge, NULL);
-    object_property_add_bool(obj, "dump",
-                        host_memory_backend_get_dump,
-                        host_memory_backend_set_dump, NULL);
-    object_property_add_bool(obj, "prealloc",
-                        host_memory_backend_get_prealloc,
-                        host_memory_backend_set_prealloc, NULL);
-    object_property_add(obj, "size", "int",
-                        host_memory_backend_get_size,
-                        host_memory_backend_set_size, NULL, NULL, NULL);
-    object_property_add(obj, "host-nodes", "int",
-                        host_memory_backend_get_host_nodes,
-                        host_memory_backend_set_host_nodes, NULL, NULL, NULL);
-    object_property_add_enum(obj, "policy", "HostMemPolicy",
-                             HostMemPolicy_lookup,
-                             host_memory_backend_get_policy,
-                             host_memory_backend_set_policy, NULL);
 }
 
 MemoryRegion *
@@ -348,6 +328,27 @@ host_memory_backend_class_init(ObjectClass *oc, void *data)
 
     ucc->complete = host_memory_backend_memory_complete;
     ucc->can_be_deleted = host_memory_backend_can_be_deleted;
+
+    object_class_property_add_bool(oc, "merge",
+                                   host_memory_backend_get_merge,
+                                   host_memory_backend_set_merge, NULL);
+    object_class_property_add_bool(oc, "dump",
+                                   host_memory_backend_get_dump,
+                                   host_memory_backend_set_dump, NULL);
+    object_class_property_add_bool(oc, "prealloc",
+                                   host_memory_backend_get_prealloc,
+                                   host_memory_backend_set_prealloc, NULL);
+    object_class_property_add(oc, "size", "int",
+                              host_memory_backend_get_size,
+                              host_memory_backend_set_size, NULL, NULL, NULL);
+    object_class_property_add(oc, "host-nodes", "int",
+                              host_memory_backend_get_host_nodes,
+                              host_memory_backend_set_host_nodes,
+                              NULL, NULL, NULL);
+    object_class_property_add_enum(oc, "policy", "HostMemPolicy",
+                                   HostMemPolicy_lookup,
+                                   host_memory_backend_get_policy,
+                                   host_memory_backend_set_policy, NULL);
 }
 
 static const TypeInfo host_memory_backend_info = {
-- 
2.4.3

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PATCH RFC 3/7] rng: register properties against the class instead of object
  2015-08-26 12:03 [Qemu-devel] [PATCH RFC 0/7] Making QOM introspectable Daniel P. Berrange
  2015-08-26 12:03 ` [Qemu-devel] [PATCH RFC 1/7] qom: allow properties to be registered against classes Daniel P. Berrange
  2015-08-26 12:03 ` [Qemu-devel] [PATCH RFC 2/7] hostmem: register properties against the class instead of object Daniel P. Berrange
@ 2015-08-26 12:03 ` Daniel P. Berrange
  2015-08-26 12:03 ` [Qemu-devel] [PATCH RFC 4/7] tpm: " Daniel P. Berrange
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 26+ messages in thread
From: Daniel P. Berrange @ 2015-08-26 12:03 UTC (permalink / raw)
  To: qemu-devel; +Cc: Markus Armbruster, Andreas Färber

This converts the rng, rng-egd & rng-random objects to register
their properties against the class rather than object.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 backends/rng-egd.c    | 12 ++++--------
 backends/rng-random.c | 10 +++++-----
 backends/rng.c        | 14 +++++---------
 3 files changed, 14 insertions(+), 22 deletions(-)

diff --git a/backends/rng-egd.c b/backends/rng-egd.c
index 6c13409..8b2a9dc 100644
--- a/backends/rng-egd.c
+++ b/backends/rng-egd.c
@@ -186,13 +186,6 @@ static char *rng_egd_get_chardev(Object *obj, Error **errp)
     return NULL;
 }
 
-static void rng_egd_init(Object *obj)
-{
-    object_property_add_str(obj, "chardev",
-                            rng_egd_get_chardev, rng_egd_set_chardev,
-                            NULL);
-}
-
 static void rng_egd_finalize(Object *obj)
 {
     RngEgd *s = RNG_EGD(obj);
@@ -214,6 +207,10 @@ static void rng_egd_class_init(ObjectClass *klass, void *data)
     rbc->request_entropy = rng_egd_request_entropy;
     rbc->cancel_requests = rng_egd_cancel_requests;
     rbc->opened = rng_egd_opened;
+
+    object_class_property_add_str(klass, "chardev",
+                                  rng_egd_get_chardev, rng_egd_set_chardev,
+                                  NULL);
 }
 
 static const TypeInfo rng_egd_info = {
@@ -221,7 +218,6 @@ static const TypeInfo rng_egd_info = {
     .parent = TYPE_RNG_BACKEND,
     .instance_size = sizeof(RngEgd),
     .class_init = rng_egd_class_init,
-    .instance_init = rng_egd_init,
     .instance_finalize = rng_egd_finalize,
 };
 
diff --git a/backends/rng-random.c b/backends/rng-random.c
index 4e51f46..175c61b 100644
--- a/backends/rng-random.c
+++ b/backends/rng-random.c
@@ -110,11 +110,6 @@ static void rng_random_init(Object *obj)
 {
     RndRandom *s = RNG_RANDOM(obj);
 
-    object_property_add_str(obj, "filename",
-                            rng_random_get_filename,
-                            rng_random_set_filename,
-                            NULL);
-
     s->filename = g_strdup("/dev/random");
     s->fd = -1;
 }
@@ -137,6 +132,11 @@ static void rng_random_class_init(ObjectClass *klass, void *data)
 
     rbc->request_entropy = rng_random_request_entropy;
     rbc->opened = rng_random_opened;
+
+    object_class_property_add_str(klass, "filename",
+                                  rng_random_get_filename,
+                                  rng_random_set_filename,
+                                  NULL);
 }
 
 static const TypeInfo rng_random_info = {
diff --git a/backends/rng.c b/backends/rng.c
index 5065fdc..c9c0632 100644
--- a/backends/rng.c
+++ b/backends/rng.c
@@ -72,26 +72,22 @@ static void rng_backend_prop_set_opened(Object *obj, bool value, Error **errp)
     s->opened = true;
 }
 
-static void rng_backend_init(Object *obj)
-{
-    object_property_add_bool(obj, "opened",
-                             rng_backend_prop_get_opened,
-                             rng_backend_prop_set_opened,
-                             NULL);
-}
-
 static void rng_backend_class_init(ObjectClass *oc, void *data)
 {
     UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc);
 
     ucc->complete = rng_backend_complete;
+
+    object_class_property_add_bool(oc, "opened",
+                                   rng_backend_prop_get_opened,
+                                   rng_backend_prop_set_opened,
+                                   NULL);
 }
 
 static const TypeInfo rng_backend_info = {
     .name = TYPE_RNG_BACKEND,
     .parent = TYPE_OBJECT,
     .instance_size = sizeof(RngBackend),
-    .instance_init = rng_backend_init,
     .class_size = sizeof(RngBackendClass),
     .class_init = rng_backend_class_init,
     .abstract = true,
-- 
2.4.3

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PATCH RFC 4/7] tpm: register properties against the class instead of object
  2015-08-26 12:03 [Qemu-devel] [PATCH RFC 0/7] Making QOM introspectable Daniel P. Berrange
                   ` (2 preceding siblings ...)
  2015-08-26 12:03 ` [Qemu-devel] [PATCH RFC 3/7] rng: " Daniel P. Berrange
@ 2015-08-26 12:03 ` Daniel P. Berrange
  2015-08-26 12:03 ` [Qemu-devel] [PATCH RFC 5/7] cpu: avoid using object instance state in property getter Daniel P. Berrange
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 26+ messages in thread
From: Daniel P. Berrange @ 2015-08-26 12:03 UTC (permalink / raw)
  To: qemu-devel; +Cc: Markus Armbruster, Andreas Färber

This converts the tpm object to register its properties against
the class rather than object.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 backends/tpm.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/backends/tpm.c b/backends/tpm.c
index a512693..cb540b9 100644
--- a/backends/tpm.c
+++ b/backends/tpm.c
@@ -148,12 +148,12 @@ static void tpm_backend_prop_set_opened(Object *obj, bool value, Error **errp)
     s->opened = true;
 }
 
-static void tpm_backend_instance_init(Object *obj)
+static void tpm_backend_class_init(ObjectClass *klass, void *data)
 {
-    object_property_add_bool(obj, "opened",
-                             tpm_backend_prop_get_opened,
-                             tpm_backend_prop_set_opened,
-                             NULL);
+    object_class_property_add_bool(klass, "opened",
+                                   tpm_backend_prop_get_opened,
+                                   tpm_backend_prop_set_opened,
+                                   NULL);
 }
 
 void tpm_backend_thread_deliver_request(TPMBackendThread *tbt)
@@ -183,7 +183,7 @@ static const TypeInfo tpm_backend_info = {
     .name = TYPE_TPM_BACKEND,
     .parent = TYPE_OBJECT,
     .instance_size = sizeof(TPMBackend),
-    .instance_init = tpm_backend_instance_init,
+    .class_init = tpm_backend_class_init,
     .class_size = sizeof(TPMBackendClass),
     .abstract = true,
 };
-- 
2.4.3

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PATCH RFC 5/7] cpu: avoid using object instance state in property getter
  2015-08-26 12:03 [Qemu-devel] [PATCH RFC 0/7] Making QOM introspectable Daniel P. Berrange
                   ` (3 preceding siblings ...)
  2015-08-26 12:03 ` [Qemu-devel] [PATCH RFC 4/7] tpm: " Daniel P. Berrange
@ 2015-08-26 12:03 ` Daniel P. Berrange
  2015-08-26 12:03 ` [Qemu-devel] [PATCH RFC 6/7] x86-cpu: register properties against the class instead of object Daniel P. Berrange
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 26+ messages in thread
From: Daniel P. Berrange @ 2015-08-26 12:03 UTC (permalink / raw)
  To: qemu-devel; +Cc: Markus Armbruster, Andreas Färber

When registering the properties 'feature-words' and 'filtered-features'
object instance data is being passed in. This can easily be accessed
directly via the 'Object *obj' parameter passed to the getter, so
the object instance data does not need to be supplied at property
registration time.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 target-i386/cpu.c | 39 ++++++++++++++++++++++++++++++++-------
 1 file changed, 32 insertions(+), 7 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index cfb8aa7..780a5bc 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1766,12 +1766,10 @@ static void x86_cpuid_set_apic_id(Object *obj, Visitor *v, void *opaque,
 }
 
 /* Generic getter for "feature-words" and "filtered-features" properties */
-static void x86_cpu_get_feature_words(Object *obj, Visitor *v, void *opaque,
-                                      const char *name, Error **errp)
+static X86CPUFeatureWordInfoList *
+x86_cpu_get_feature_words_helper(Object *obj, uint32_t *array)
 {
-    uint32_t *array = (uint32_t *)opaque;
     FeatureWord w;
-    Error *err = NULL;
     X86CPUFeatureWordInfo word_infos[FEATURE_WORDS] = { };
     X86CPUFeatureWordInfoList list_entries[FEATURE_WORDS] = { };
     X86CPUFeatureWordInfoList *list = NULL;
@@ -1791,10 +1789,37 @@ static void x86_cpu_get_feature_words(Object *obj, Visitor *v, void *opaque,
         list = &list_entries[w];
     }
 
+    return list;
+}
+
+
+static void x86_cpu_get_feature_words(Object *obj, Visitor *v, void *opaque,
+                                      const char *name, Error **errp)
+{
+    X86CPU *cpu = X86_CPU(obj);
+    uint32_t *array = cpu->env.features;
+    X86CPUFeatureWordInfoList *list =
+        x86_cpu_get_feature_words_helper(obj, array);
+    Error *err = NULL;
+
     visit_type_X86CPUFeatureWordInfoList(v, &list, "feature-words", &err);
     error_propagate(errp, err);
 }
 
+
+static void x86_cpu_get_filtered_features(Object *obj, Visitor *v, void *opaque,
+                                          const char *name, Error **errp)
+{
+    X86CPU *cpu = X86_CPU(obj);
+    uint32_t *array = cpu->filtered_features;
+    X86CPUFeatureWordInfoList *list =
+        x86_cpu_get_feature_words_helper(obj, array);
+    Error *err = NULL;
+
+    visit_type_X86CPUFeatureWordInfoList(v, &list, "filtered-features", &err);
+    error_propagate(errp, err);
+}
+
 static void x86_get_hv_spinlocks(Object *obj, Visitor *v, void *opaque,
                                  const char *name, Error **errp)
 {
@@ -3036,10 +3061,10 @@ static void x86_cpu_initfn(Object *obj)
                         x86_cpuid_set_apic_id, NULL, NULL, NULL);
     object_property_add(obj, "feature-words", "X86CPUFeatureWordInfo",
                         x86_cpu_get_feature_words,
-                        NULL, NULL, (void *)env->features, NULL);
+                        NULL, NULL, NULL, NULL);
     object_property_add(obj, "filtered-features", "X86CPUFeatureWordInfo",
-                        x86_cpu_get_feature_words,
-                        NULL, NULL, (void *)cpu->filtered_features, NULL);
+                        x86_cpu_get_filtered_features,
+                        NULL, NULL, NULL, NULL);
 
     cpu->hyperv_spinlock_attempts = HYPERV_SPINLOCK_NEVER_RETRY;
 
-- 
2.4.3

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PATCH RFC 6/7] x86-cpu: register properties against the class instead of object
  2015-08-26 12:03 [Qemu-devel] [PATCH RFC 0/7] Making QOM introspectable Daniel P. Berrange
                   ` (4 preceding siblings ...)
  2015-08-26 12:03 ` [Qemu-devel] [PATCH RFC 5/7] cpu: avoid using object instance state in property getter Daniel P. Berrange
@ 2015-08-26 12:03 ` Daniel P. Berrange
  2015-08-26 12:03 ` [Qemu-devel] [PATCH RFC 7/7] machine: " Daniel P. Berrange
  2015-09-02  9:05 ` [Qemu-devel] [PATCH RFC 0/7] Making QOM introspectable Daniel P. Berrange
  7 siblings, 0 replies; 26+ messages in thread
From: Daniel P. Berrange @ 2015-08-26 12:03 UTC (permalink / raw)
  To: qemu-devel; +Cc: Markus Armbruster, Andreas Färber

This converts the x86 CPU object to register its properties against
the class rather than object.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 target-i386/cpu.c | 55 ++++++++++++++++++++++++++++---------------------------
 1 file changed, 28 insertions(+), 27 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 780a5bc..e183d0b 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -3038,33 +3038,6 @@ static void x86_cpu_initfn(Object *obj)
     cs->env_ptr = env;
     cpu_exec_init(cs, &error_abort);
 
-    object_property_add(obj, "family", "int",
-                        x86_cpuid_version_get_family,
-                        x86_cpuid_version_set_family, NULL, NULL, NULL);
-    object_property_add(obj, "model", "int",
-                        x86_cpuid_version_get_model,
-                        x86_cpuid_version_set_model, NULL, NULL, NULL);
-    object_property_add(obj, "stepping", "int",
-                        x86_cpuid_version_get_stepping,
-                        x86_cpuid_version_set_stepping, NULL, NULL, NULL);
-    object_property_add_str(obj, "vendor",
-                            x86_cpuid_get_vendor,
-                            x86_cpuid_set_vendor, NULL);
-    object_property_add_str(obj, "model-id",
-                            x86_cpuid_get_model_id,
-                            x86_cpuid_set_model_id, NULL);
-    object_property_add(obj, "tsc-frequency", "int",
-                        x86_cpuid_get_tsc_freq,
-                        x86_cpuid_set_tsc_freq, NULL, NULL, NULL);
-    object_property_add(obj, "apic-id", "int",
-                        x86_cpuid_get_apic_id,
-                        x86_cpuid_set_apic_id, NULL, NULL, NULL);
-    object_property_add(obj, "feature-words", "X86CPUFeatureWordInfo",
-                        x86_cpu_get_feature_words,
-                        NULL, NULL, NULL, NULL);
-    object_property_add(obj, "filtered-features", "X86CPUFeatureWordInfo",
-                        x86_cpu_get_filtered_features,
-                        NULL, NULL, NULL, NULL);
 
     cpu->hyperv_spinlock_attempts = HYPERV_SPINLOCK_NEVER_RETRY;
 
@@ -3199,6 +3172,34 @@ static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
 #endif
     cc->cpu_exec_enter = x86_cpu_exec_enter;
     cc->cpu_exec_exit = x86_cpu_exec_exit;
+
+    object_class_property_add(oc, "family", "int",
+                              x86_cpuid_version_get_family,
+                              x86_cpuid_version_set_family, NULL, NULL, NULL);
+    object_class_property_add(oc, "model", "int",
+                              x86_cpuid_version_get_model,
+                              x86_cpuid_version_set_model, NULL, NULL, NULL);
+    object_class_property_add(oc, "stepping", "int",
+                              x86_cpuid_version_get_stepping,
+                              x86_cpuid_version_set_stepping, NULL, NULL, NULL);
+    object_class_property_add_str(oc, "vendor",
+                                  x86_cpuid_get_vendor,
+                                  x86_cpuid_set_vendor, NULL);
+    object_class_property_add_str(oc, "model-id",
+                                  x86_cpuid_get_model_id,
+                                  x86_cpuid_set_model_id, NULL);
+    object_class_property_add(oc, "tsc-frequency", "int",
+                              x86_cpuid_get_tsc_freq,
+                              x86_cpuid_set_tsc_freq, NULL, NULL, NULL);
+    object_class_property_add(oc, "apic-id", "int",
+                              x86_cpuid_get_apic_id,
+                              x86_cpuid_set_apic_id, NULL, NULL, NULL);
+    object_class_property_add(oc, "feature-words", "X86CPUFeatureWordInfo",
+                              x86_cpu_get_feature_words,
+                              NULL, NULL, NULL, NULL);
+    object_class_property_add(oc, "filtered-features", "X86CPUFeatureWordInfo",
+                              x86_cpu_get_filtered_features,
+                              NULL, NULL, NULL, NULL);
 }
 
 static const TypeInfo x86_cpu_type_info = {
-- 
2.4.3

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PATCH RFC 7/7] machine: register properties against the class instead of object
  2015-08-26 12:03 [Qemu-devel] [PATCH RFC 0/7] Making QOM introspectable Daniel P. Berrange
                   ` (5 preceding siblings ...)
  2015-08-26 12:03 ` [Qemu-devel] [PATCH RFC 6/7] x86-cpu: register properties against the class instead of object Daniel P. Berrange
@ 2015-08-26 12:03 ` Daniel P. Berrange
  2015-09-02  9:05 ` [Qemu-devel] [PATCH RFC 0/7] Making QOM introspectable Daniel P. Berrange
  7 siblings, 0 replies; 26+ messages in thread
From: Daniel P. Berrange @ 2015-08-26 12:03 UTC (permalink / raw)
  To: qemu-devel; +Cc: Markus Armbruster, Andreas Färber

This converts the machine base object to register its properties
against the class rather than object.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 hw/core/machine.c | 193 +++++++++++++++++++++++++++---------------------------
 1 file changed, 97 insertions(+), 96 deletions(-)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index ac4654e..0fd80d3 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -300,6 +300,103 @@ static void machine_class_init(ObjectClass *oc, void *data)
 
     /* Default 128 MB as guest ram size */
     mc->default_ram_size = 128 * M_BYTE;
+
+    object_class_property_add_str(oc, "accel",
+                                  machine_get_accel, machine_set_accel, NULL);
+    object_class_property_set_description(oc, "accel",
+                                          "Accelerator list",
+                                          NULL);
+    object_class_property_add_bool(oc, "kernel-irqchip",
+                                   NULL,
+                                   machine_set_kernel_irqchip,
+                                   NULL);
+    object_class_property_set_description(oc, "kernel-irqchip",
+                                          "Use KVM in-kernel irqchip",
+                                          NULL);
+    object_class_property_add(oc, "kvm-shadow-mem", "int",
+                              machine_get_kvm_shadow_mem,
+                              machine_set_kvm_shadow_mem,
+                              NULL, NULL, NULL);
+    object_class_property_set_description(oc, "kvm-shadow-mem",
+                                          "KVM shadow MMU size",
+                                          NULL);
+    object_class_property_add_str(oc, "kernel",
+                                  machine_get_kernel, machine_set_kernel, NULL);
+    object_class_property_set_description(oc, "kernel",
+                                          "Linux kernel image file",
+                                          NULL);
+    object_class_property_add_str(oc, "initrd",
+                                  machine_get_initrd, machine_set_initrd, NULL);
+    object_class_property_set_description(oc, "initrd",
+                                          "Linux initial ramdisk file",
+                                          NULL);
+    object_class_property_add_str(oc, "append",
+                                  machine_get_append, machine_set_append, NULL);
+    object_class_property_set_description(oc, "append",
+                                          "Linux kernel command line",
+                                          NULL);
+    object_class_property_add_str(oc, "dtb",
+                                  machine_get_dtb, machine_set_dtb, NULL);
+    object_class_property_set_description(oc, "dtb",
+                                          "Linux kernel device tree file",
+                                          NULL);
+    object_class_property_add_str(oc, "dumpdtb",
+                                  machine_get_dumpdtb,
+                                  machine_set_dumpdtb, NULL);
+    object_class_property_set_description(oc, "dumpdtb",
+                                          "Dump current dtb to a file and quit",
+                                          NULL);
+    object_class_property_add(oc, "phandle-start", "int",
+                              machine_get_phandle_start,
+                              machine_set_phandle_start,
+                              NULL, NULL, NULL);
+    object_class_property_set_description(oc, "phandle-start",
+                                          "The first phandle ID we may generate dynamically",
+                                          NULL);
+    object_class_property_add_str(oc, "dt-compatible",
+                                  machine_get_dt_compatible,
+                                  machine_set_dt_compatible,
+                                  NULL);
+    object_class_property_set_description(oc, "dt-compatible",
+                                          "Overrides the \"compatible\" property of the dt root node",
+                                          NULL);
+    object_class_property_add_bool(oc, "dump-guest-core",
+                                   machine_get_dump_guest_core,
+                                   machine_set_dump_guest_core,
+                                   NULL);
+    object_class_property_set_description(oc, "dump-guest-core",
+                                          "Include guest memory in  a core dump",
+                                          NULL);
+    object_class_property_add_bool(oc, "mem-merge",
+                                   machine_get_mem_merge,
+                                   machine_set_mem_merge, NULL);
+    object_class_property_set_description(oc, "mem-merge",
+                                          "Enable/disable memory merge support",
+                                          NULL);
+    object_class_property_add_bool(oc, "usb",
+                                   machine_get_usb,
+                                   machine_set_usb, NULL);
+    object_class_property_set_description(oc, "usb",
+                                          "Set on/off to enable/disable usb",
+                                          NULL);
+    object_class_property_add_str(oc, "firmware",
+                                  machine_get_firmware,
+                                  machine_set_firmware, NULL);
+    object_class_property_set_description(oc, "firmware",
+                                          "Firmware image",
+                                          NULL);
+    object_class_property_add_bool(oc, "iommu",
+                                   machine_get_iommu,
+                                   machine_set_iommu, NULL);
+    object_class_property_set_description(oc, "iommu",
+                                          "Set on/off to enable/disable Intel IOMMU (VT-d)",
+                                          NULL);
+    object_class_property_add_bool(oc, "suppress-vmdesc",
+                                   machine_get_suppress_vmdesc,
+                                   machine_set_suppress_vmdesc, NULL);
+    object_class_property_set_description(oc, "suppress-vmdesc",
+                                          "Set on to disable self-describing migration",
+                                          NULL);
 }
 
 static void machine_initfn(Object *obj)
@@ -311,102 +408,6 @@ static void machine_initfn(Object *obj)
     ms->dump_guest_core = true;
     ms->mem_merge = true;
 
-    object_property_add_str(obj, "accel",
-                            machine_get_accel, machine_set_accel, NULL);
-    object_property_set_description(obj, "accel",
-                                    "Accelerator list",
-                                    NULL);
-    object_property_add_bool(obj, "kernel-irqchip",
-                             NULL,
-                             machine_set_kernel_irqchip,
-                             NULL);
-    object_property_set_description(obj, "kernel-irqchip",
-                                    "Use KVM in-kernel irqchip",
-                                    NULL);
-    object_property_add(obj, "kvm-shadow-mem", "int",
-                        machine_get_kvm_shadow_mem,
-                        machine_set_kvm_shadow_mem,
-                        NULL, NULL, NULL);
-    object_property_set_description(obj, "kvm-shadow-mem",
-                                    "KVM shadow MMU size",
-                                    NULL);
-    object_property_add_str(obj, "kernel",
-                            machine_get_kernel, machine_set_kernel, NULL);
-    object_property_set_description(obj, "kernel",
-                                    "Linux kernel image file",
-                                    NULL);
-    object_property_add_str(obj, "initrd",
-                            machine_get_initrd, machine_set_initrd, NULL);
-    object_property_set_description(obj, "initrd",
-                                    "Linux initial ramdisk file",
-                                    NULL);
-    object_property_add_str(obj, "append",
-                            machine_get_append, machine_set_append, NULL);
-    object_property_set_description(obj, "append",
-                                    "Linux kernel command line",
-                                    NULL);
-    object_property_add_str(obj, "dtb",
-                            machine_get_dtb, machine_set_dtb, NULL);
-    object_property_set_description(obj, "dtb",
-                                    "Linux kernel device tree file",
-                                    NULL);
-    object_property_add_str(obj, "dumpdtb",
-                            machine_get_dumpdtb, machine_set_dumpdtb, NULL);
-    object_property_set_description(obj, "dumpdtb",
-                                    "Dump current dtb to a file and quit",
-                                    NULL);
-    object_property_add(obj, "phandle-start", "int",
-                        machine_get_phandle_start,
-                        machine_set_phandle_start,
-                        NULL, NULL, NULL);
-    object_property_set_description(obj, "phandle-start",
-                                    "The first phandle ID we may generate dynamically",
-                                    NULL);
-    object_property_add_str(obj, "dt-compatible",
-                            machine_get_dt_compatible,
-                            machine_set_dt_compatible,
-                            NULL);
-    object_property_set_description(obj, "dt-compatible",
-                                    "Overrides the \"compatible\" property of the dt root node",
-                                    NULL);
-    object_property_add_bool(obj, "dump-guest-core",
-                             machine_get_dump_guest_core,
-                             machine_set_dump_guest_core,
-                             NULL);
-    object_property_set_description(obj, "dump-guest-core",
-                                    "Include guest memory in  a core dump",
-                                    NULL);
-    object_property_add_bool(obj, "mem-merge",
-                             machine_get_mem_merge,
-                             machine_set_mem_merge, NULL);
-    object_property_set_description(obj, "mem-merge",
-                                    "Enable/disable memory merge support",
-                                    NULL);
-    object_property_add_bool(obj, "usb",
-                             machine_get_usb,
-                             machine_set_usb, NULL);
-    object_property_set_description(obj, "usb",
-                                    "Set on/off to enable/disable usb",
-                                    NULL);
-    object_property_add_str(obj, "firmware",
-                            machine_get_firmware,
-                            machine_set_firmware, NULL);
-    object_property_set_description(obj, "firmware",
-                                    "Firmware image",
-                                    NULL);
-    object_property_add_bool(obj, "iommu",
-                             machine_get_iommu,
-                             machine_set_iommu, NULL);
-    object_property_set_description(obj, "iommu",
-                                    "Set on/off to enable/disable Intel IOMMU (VT-d)",
-                                    NULL);
-    object_property_add_bool(obj, "suppress-vmdesc",
-                             machine_get_suppress_vmdesc,
-                             machine_set_suppress_vmdesc, NULL);
-    object_property_set_description(obj, "suppress-vmdesc",
-                                    "Set on to disable self-describing migration",
-                                    NULL);
-
     /* Register notifier when init is done for sysbus sanity checks */
     ms->sysbus_notifier.notify = machine_init_notify;
     qemu_add_machine_init_done_notifier(&ms->sysbus_notifier);
-- 
2.4.3

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 0/7] Making QOM introspectable
  2015-08-26 12:03 [Qemu-devel] [PATCH RFC 0/7] Making QOM introspectable Daniel P. Berrange
                   ` (6 preceding siblings ...)
  2015-08-26 12:03 ` [Qemu-devel] [PATCH RFC 7/7] machine: " Daniel P. Berrange
@ 2015-09-02  9:05 ` Daniel P. Berrange
  2015-09-02 11:14   ` Markus Armbruster
  2015-09-02 16:16   ` Andreas Färber
  7 siblings, 2 replies; 26+ messages in thread
From: Daniel P. Berrange @ 2015-09-02  9:05 UTC (permalink / raw)
  To: qemu-devel; +Cc: Markus Armbruster, Andreas Färber

Ping, any thoughts on this....

On Wed, Aug 26, 2015 at 01:03:07PM +0100, Daniel P. Berrange wrote:
> There are many problems in QEMU related to introspection
> of features, which Markus has been attacking/slaying for
> a while. One of the remaining elephants in the corner of
> the room which I've not seen work on is QOM.
> 
> QOM has a nice class/object framework, kind of like GLib's
> GObject, but with the difference that properties are
> registered against objects rather than classes. IIRC the
> rationale for this is that in some cases the properties
> registered are going to be dynamically decided at runtime,
> so it isn't possible to statically define them against
> classes. Now this may well be true of some properties,
> but there are equally plenty of cases where the properties
> /are/ invariant and could be registered against classes.
> 
> There are two core problems with registering properties
> against object instances:
> 
>  - It is wasteful of memory to duplicate the allocation
>    of ObjectProperty structs against each object
>    instance. When you have N object instances, you
>    have O(N) memory usage, instead of O(1). This is not
>    a problem for objects which are mostly singletons,
>    but there are cases in QEMU where you instantiate
>    many instances of the same class and/or have many
>    properties.
> 
>  - It prevents static introspection. Since the property
>    is only registered in the object initializer, there
>    is no way to programmatically query what properties
>    an object supports without first instantiating it.
>    Taking machine types as an example, if you want to
>    introspect every machine type supported by a QEMU
>    binary you thus have to launch QEMU many times,
>    passing a different -M argument each time. As the
>    number of different machine types & objects
>    increases this quickly becomes impractical.
> 
> This series thus extends QOM to make it possible to register
> properties against the classes, in addition to against object
> instances. When looking up a property, a search will be done
> starting at the base class, then each subclass in turn, and
> finally against the object. Names are enforced to be unique
> across the parent classes for sanity.
> 
> This only currently supports simple scalar properties where
> the actual property value storage is managed by the object
> instance. The object child and object link properties use
> implicit storage in the ObjectProperty struct's 'opaque'
> field, so we can't allow those against the class. Solving
> this is doable, but more work, so is left as an exercise
> for the future.
> 
> The first patch adds the neccessary QOM functionality. The
> following 6 patches then illustrate the fairly trivial
> conversions of a bunch of objects.
> 
> The eventual goal ought to be that everything is registered
> against the class, leaving only the (hopefully few) cases
> where per-instance properties are truely needed unconverted.
> 
> This series doesn't attempt to implement introspection
> either - this would require a new QMP command such as
> 'qom-list-type-props' to query the properties against
> classes.
> 
> I'm not really planning to spend much more time on this
> myself. I'm just using this series to illustrate how I
> believe the introspection problem in QOM can be fairly
> easily addressed. Hopefully this will stimulate some
> discussion & interest in doing the full job....
> 
> Daniel P. Berrange (7):
>   qom: allow properties to be registered against classes
>   hostmem: register properties against the class instead of object
>   rng: register properties against the class instead of object
>   tpm: register properties against the class instead of object
>   cpu: avoid using object instance state in property getter
>   x86-cpu: register properties against the class instead of object
>   machine: register properties against the class instead of object
> 
>  backends/hostmem-file.c |  26 +++---
>  backends/hostmem.c      |  41 ++++-----
>  backends/rng-egd.c      |  12 +--
>  backends/rng-random.c   |  10 +--
>  backends/rng.c          |  14 ++-
>  backends/tpm.c          |  12 +--
>  hw/core/machine.c       | 193 +++++++++++++++++++--------------------
>  include/qom/object.h    |  44 +++++++++
>  qom/object.c            | 233 ++++++++++++++++++++++++++++++++++++++++++++++--
>  target-i386/cpu.c       |  88 +++++++++++-------
>  10 files changed, 476 insertions(+), 197 deletions(-)
> 
> -- 
> 2.4.3
> 

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 0/7] Making QOM introspectable
  2015-09-02  9:05 ` [Qemu-devel] [PATCH RFC 0/7] Making QOM introspectable Daniel P. Berrange
@ 2015-09-02 11:14   ` Markus Armbruster
  2015-09-02 16:16   ` Andreas Färber
  1 sibling, 0 replies; 26+ messages in thread
From: Markus Armbruster @ 2015-09-02 11:14 UTC (permalink / raw)
  To: Daniel P. Berrange; +Cc: qemu-devel, Andreas Färber

"Daniel P. Berrange" <berrange@redhat.com> writes:

> Ping, any thoughts on this....

I'll probably have thoughts once I got "[PATCH v4] qapi: QMP
introspection" out of the door.  Apologies for the delay...

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 0/7] Making QOM introspectable
  2015-09-02  9:05 ` [Qemu-devel] [PATCH RFC 0/7] Making QOM introspectable Daniel P. Berrange
  2015-09-02 11:14   ` Markus Armbruster
@ 2015-09-02 16:16   ` Andreas Färber
  1 sibling, 0 replies; 26+ messages in thread
From: Andreas Färber @ 2015-09-02 16:16 UTC (permalink / raw)
  To: Daniel P. Berrange, qemu-devel; +Cc: Markus Armbruster

Am 02.09.2015 um 11:05 schrieb Daniel P. Berrange:
> Ping, any thoughts on this....

The subject is a little offensive as QOM *is* our introspection
framework for devices and objects. What this really seems to be about is
static vs. per-instance (dynamic) introspection.

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton; HRB 21284 (AG Nürnberg)

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 1/7] qom: allow properties to be registered against classes
  2015-08-26 12:03 ` [Qemu-devel] [PATCH RFC 1/7] qom: allow properties to be registered against classes Daniel P. Berrange
@ 2015-09-02 16:18   ` Andreas Färber
  2015-09-03 15:49     ` Daniel P. Berrange
  2015-09-11 16:09     ` Daniel P. Berrange
  2015-09-04 21:38   ` Marc-André Lureau
  1 sibling, 2 replies; 26+ messages in thread
From: Andreas Färber @ 2015-09-02 16:18 UTC (permalink / raw)
  To: Daniel P. Berrange, qemu-devel; +Cc: Markus Armbruster

Am 26.08.2015 um 14:03 schrieb Daniel P. Berrange:
> 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.
> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  include/qom/object.h |  44 ++++++++++
>  qom/object.c         | 233 +++++++++++++++++++++++++++++++++++++++++++++++++--
>  2 files changed, 270 insertions(+), 7 deletions(-)
> 
> diff --git a/include/qom/object.h b/include/qom/object.h
> index 807978e..068162e 100644
> --- a/include/qom/object.h
> +++ b/include/qom/object.h
> @@ -383,6 +383,8 @@ struct ObjectClass
>      const char *class_cast_cache[OBJECT_CLASS_CAST_CACHE];
>  
>      ObjectUnparent *unparent;
> +
> +    QTAILQ_HEAD(, ObjectProperty) properties;
>  };
>  
>  /**
[snip]

I had suggested exactly this looong time ago, but Anthony opposed it. I
don't quite remember why...

Did you do any benchmarks on performance impact?

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton; HRB 21284 (AG Nürnberg)

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 1/7] qom: allow properties to be registered against classes
  2015-09-02 16:18   ` Andreas Färber
@ 2015-09-03 15:49     ` Daniel P. Berrange
  2015-09-03 16:37       ` Markus Armbruster
  2015-09-11 16:09     ` Daniel P. Berrange
  1 sibling, 1 reply; 26+ messages in thread
From: Daniel P. Berrange @ 2015-09-03 15:49 UTC (permalink / raw)
  To: Andreas Färber; +Cc: qemu-devel, Markus Armbruster

On Wed, Sep 02, 2015 at 06:18:28PM +0200, Andreas Färber wrote:
> Am 26.08.2015 um 14:03 schrieb Daniel P. Berrange:
> > 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.
> > 
> > Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> > ---
> >  include/qom/object.h |  44 ++++++++++
> >  qom/object.c         | 233 +++++++++++++++++++++++++++++++++++++++++++++++++--
> >  2 files changed, 270 insertions(+), 7 deletions(-)
> > 
> > diff --git a/include/qom/object.h b/include/qom/object.h
> > index 807978e..068162e 100644
> > --- a/include/qom/object.h
> > +++ b/include/qom/object.h
> > @@ -383,6 +383,8 @@ struct ObjectClass
> >      const char *class_cast_cache[OBJECT_CLASS_CAST_CACHE];
> >  
> >      ObjectUnparent *unparent;
> > +
> > +    QTAILQ_HEAD(, ObjectProperty) properties;
> >  };
> >  
> >  /**
> [snip]
> 
> I had suggested exactly this looong time ago, but Anthony opposed it. I
> don't quite remember why...

It is a while back now so I don't remember all aspects of the discussion
either. The main thing I remember is that we could not simply use the
existing GObject model from glib, since that exclusively records properties
against the object class. In some cases, particularly the relationships
between objects, QEMU needed to be able to define properties on the fly
against object instances.

So that was a definite reason Anthony wanted to have the ability to have
properties against object instances. I don't remember reading anything
about *not* also having the option to define properties against the
object classes. So hopefully this proposal gets us the best of both
worlds - the flexibility of allowing per-instance properties when
needed, along with the reduced memory usage & static introspection
benefits of per-class properties where possible.

> Did you do any benchmarks on performance impact?

I've not done any measurements of impact on CPU time or memory usage.
As mentioned in the intro, I'd expect memory usage to decline significantly
in the case where there are many instances of the same class. Attribute
getter/setters probably have a small CPU hit, due to the need to consult
the linked lists in both the class and object structs. I think that will
be small overall though, and if it is a problem we would probably better
off switching in a hashtable in place of the linked list, so we have
O(1) lookup instead of O(n) lookups.

That all said, I'll try to create a test program to measure these
effects to get some clear numbers to consider.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 1/7] qom: allow properties to be registered against classes
  2015-09-03 15:49     ` Daniel P. Berrange
@ 2015-09-03 16:37       ` Markus Armbruster
  2015-09-03 16:41         ` Andreas Färber
  0 siblings, 1 reply; 26+ messages in thread
From: Markus Armbruster @ 2015-09-03 16:37 UTC (permalink / raw)
  To: Daniel P. Berrange; +Cc: Paolo Bonzini, Andreas Färber, qemu-devel

Adding Paolo for additional QOM smarts.

"Daniel P. Berrange" <berrange@redhat.com> writes:

> On Wed, Sep 02, 2015 at 06:18:28PM +0200, Andreas Färber wrote:
>> Am 26.08.2015 um 14:03 schrieb Daniel P. Berrange:
>> > 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.
>> > 
>> > Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
>> > ---
>> >  include/qom/object.h |  44 ++++++++++
>> >  qom/object.c         | 233 +++++++++++++++++++++++++++++++++++++++++++++++++--
>> >  2 files changed, 270 insertions(+), 7 deletions(-)
>> > 
>> > diff --git a/include/qom/object.h b/include/qom/object.h
>> > index 807978e..068162e 100644
>> > --- a/include/qom/object.h
>> > +++ b/include/qom/object.h
>> > @@ -383,6 +383,8 @@ struct ObjectClass
>> >      const char *class_cast_cache[OBJECT_CLASS_CAST_CACHE];
>> >  
>> >      ObjectUnparent *unparent;
>> > +
>> > +    QTAILQ_HEAD(, ObjectProperty) properties;
>> >  };
>> >  
>> >  /**
>> [snip]
>> 
>> I had suggested exactly this looong time ago, but Anthony opposed it. I
>> don't quite remember why...
>
> It is a while back now so I don't remember all aspects of the discussion
> either. The main thing I remember is that we could not simply use the
> existing GObject model from glib, since that exclusively records properties
> against the object class. In some cases, particularly the relationships
> between objects, QEMU needed to be able to define properties on the fly
> against object instances.

I remember Anthony's assertion that this is the case, but I don't
remember the actual problems where this is actually the case.

What properties do we currently define that could not be defined against
the class?

> So that was a definite reason Anthony wanted to have the ability to have
> properties against object instances. I don't remember reading anything
> about *not* also having the option to define properties against the
> object classes. So hopefully this proposal gets us the best of both
> worlds - the flexibility of allowing per-instance properties when
> needed, along with the reduced memory usage & static introspection
> benefits of per-class properties where possible.

If I can't have "properties are always declared in data, not created by
code", then I'd still prefer "almost all properties are declared in
data" over "properties are always created by code", because reasoning
over static data is way easier than reasoning over code.

>> Did you do any benchmarks on performance impact?
>
> I've not done any measurements of impact on CPU time or memory usage.
> As mentioned in the intro, I'd expect memory usage to decline significantly
> in the case where there are many instances of the same class. Attribute
> getter/setters probably have a small CPU hit, due to the need to consult
> the linked lists in both the class and object structs. I think that will
> be small overall though, and if it is a problem we would probably better
> off switching in a hashtable in place of the linked list, so we have
> O(1) lookup instead of O(n) lookups.
>
> That all said, I'll try to create a test program to measure these
> effects to get some clear numbers to consider.

Yes, please.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 1/7] qom: allow properties to be registered against classes
  2015-09-03 16:37       ` Markus Armbruster
@ 2015-09-03 16:41         ` Andreas Färber
  2015-09-03 17:02           ` Markus Armbruster
  0 siblings, 1 reply; 26+ messages in thread
From: Andreas Färber @ 2015-09-03 16:41 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: Paolo Bonzini, qemu-devel

Am 03.09.2015 um 18:37 schrieb Markus Armbruster:
> "Daniel P. Berrange" <berrange@redhat.com> writes:
>> On Wed, Sep 02, 2015 at 06:18:28PM +0200, Andreas Färber wrote:
>>> I had suggested exactly this looong time ago, but Anthony opposed it. I
>>> don't quite remember why...
>>
>> It is a while back now so I don't remember all aspects of the discussion
>> either. The main thing I remember is that we could not simply use the
>> existing GObject model from glib, since that exclusively records properties
>> against the object class. In some cases, particularly the relationships
>> between objects, QEMU needed to be able to define properties on the fly
>> against object instances.
> 
> I remember Anthony's assertion that this is the case, but I don't
> remember the actual problems where this is actually the case.
> 
> What properties do we currently define that could not be defined against
> the class?

All child<> properties and everything on Container objects.

Cheers,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton; HRB 21284 (AG Nürnberg)

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 1/7] qom: allow properties to be registered against classes
  2015-09-03 16:41         ` Andreas Färber
@ 2015-09-03 17:02           ` Markus Armbruster
  2015-09-03 17:09             ` Daniel P. Berrange
  0 siblings, 1 reply; 26+ messages in thread
From: Markus Armbruster @ 2015-09-03 17:02 UTC (permalink / raw)
  To: Andreas Färber; +Cc: Paolo Bonzini, qemu-devel

Andreas Färber <afaerber@suse.de> writes:

> Am 03.09.2015 um 18:37 schrieb Markus Armbruster:
>> "Daniel P. Berrange" <berrange@redhat.com> writes:
>>> On Wed, Sep 02, 2015 at 06:18:28PM +0200, Andreas Färber wrote:
>>>> I had suggested exactly this looong time ago, but Anthony opposed it. I
>>>> don't quite remember why...
>>>
>>> It is a while back now so I don't remember all aspects of the discussion
>>> either. The main thing I remember is that we could not simply use the
>>> existing GObject model from glib, since that exclusively records properties
>>> against the object class. In some cases, particularly the relationships
>>> between objects, QEMU needed to be able to define properties on the fly
>>> against object instances.
>> 
>> I remember Anthony's assertion that this is the case, but I don't
>> remember the actual problems where this is actually the case.
>> 
>> What properties do we currently define that could not be defined against
>> the class?
>
> All child<> properties and everything on Container objects.

Apologies if you had to explain this a dozen times already, but here
goes my ignorant question anyway: why can't these properties be defined
against the class?

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 1/7] qom: allow properties to be registered against classes
  2015-09-03 17:02           ` Markus Armbruster
@ 2015-09-03 17:09             ` Daniel P. Berrange
  2015-09-03 17:21               ` Andreas Färber
  0 siblings, 1 reply; 26+ messages in thread
From: Daniel P. Berrange @ 2015-09-03 17:09 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: Paolo Bonzini, Andreas Färber, qemu-devel

On Thu, Sep 03, 2015 at 07:02:25PM +0200, Markus Armbruster wrote:
> Andreas Färber <afaerber@suse.de> writes:
> 
> > Am 03.09.2015 um 18:37 schrieb Markus Armbruster:
> >> "Daniel P. Berrange" <berrange@redhat.com> writes:
> >>> On Wed, Sep 02, 2015 at 06:18:28PM +0200, Andreas Färber wrote:
> >>>> I had suggested exactly this looong time ago, but Anthony opposed it. I
> >>>> don't quite remember why...
> >>>
> >>> It is a while back now so I don't remember all aspects of the discussion
> >>> either. The main thing I remember is that we could not simply use the
> >>> existing GObject model from glib, since that exclusively records properties
> >>> against the object class. In some cases, particularly the relationships
> >>> between objects, QEMU needed to be able to define properties on the fly
> >>> against object instances.
> >> 
> >> I remember Anthony's assertion that this is the case, but I don't
> >> remember the actual problems where this is actually the case.
> >> 
> >> What properties do we currently define that could not be defined against
> >> the class?
> >
> > All child<> properties and everything on Container objects.
> 
> Apologies if you had to explain this a dozen times already, but here
> goes my ignorant question anyway: why can't these properties be defined
> against the class?

IIUC, you can have zero or more "child<N>" properties registered.
You only know how many such properties to register at runtime,
and the count can vary per object instance. So you have to
register "child<1>", "child<2>", etc properties on objects.

If we wanted to register these against the class, we could
introduce an "array of objects" property type. So we would
just register a "children" array property against the class,
which can be populated with arbitrary number of objects at
runtime.  If we did this though, we'd probably need to setup
some backwards compat support so that any code (or external
user of QEMU) that tries to use "child<1>" would get transparently
forwarded to "children" property, element 1.

I think it could be worth exploring this idea, but since it is
a significantly more complex thing I didn't try in this series,
and just ignored the child/container problem.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 1/7] qom: allow properties to be registered against classes
  2015-09-03 17:09             ` Daniel P. Berrange
@ 2015-09-03 17:21               ` Andreas Färber
  2015-09-03 17:25                 ` Daniel P. Berrange
  0 siblings, 1 reply; 26+ messages in thread
From: Andreas Färber @ 2015-09-03 17:21 UTC (permalink / raw)
  To: Daniel P. Berrange, Markus Armbruster; +Cc: Paolo Bonzini, qemu-devel

Am 03.09.2015 um 19:09 schrieb Daniel P. Berrange:
> On Thu, Sep 03, 2015 at 07:02:25PM +0200, Markus Armbruster wrote:
>> Andreas Färber <afaerber@suse.de> writes:
>>
>>> Am 03.09.2015 um 18:37 schrieb Markus Armbruster:
>>>> "Daniel P. Berrange" <berrange@redhat.com> writes:
>>>>> On Wed, Sep 02, 2015 at 06:18:28PM +0200, Andreas Färber wrote:
>>>>>> I had suggested exactly this looong time ago, but Anthony opposed it. I
>>>>>> don't quite remember why...
>>>>>
>>>>> It is a while back now so I don't remember all aspects of the discussion
>>>>> either. The main thing I remember is that we could not simply use the
>>>>> existing GObject model from glib, since that exclusively records properties
>>>>> against the object class. In some cases, particularly the relationships
>>>>> between objects, QEMU needed to be able to define properties on the fly
>>>>> against object instances.
>>>>
>>>> I remember Anthony's assertion that this is the case, but I don't
>>>> remember the actual problems where this is actually the case.
>>>>
>>>> What properties do we currently define that could not be defined against
>>>> the class?
>>>
>>> All child<> properties and everything on Container objects.
>>
>> Apologies if you had to explain this a dozen times already, but here
>> goes my ignorant question anyway: why can't these properties be defined
>> against the class?
> 
> IIUC, you can have zero or more "child<N>" properties registered.
> You only know how many such properties to register at runtime,
> and the count can vary per object instance. So you have to
> register "child<1>", "child<2>", etc properties on objects.
> 
> If we wanted to register these against the class, we could
> introduce an "array of objects" property type. So we would
> just register a "children" array property against the class,
> which can be populated with arbitrary number of objects at
> runtime.  If we did this though, we'd probably need to setup
> some backwards compat support so that any code (or external
> user of QEMU) that tries to use "child<1>" would get transparently
> forwarded to "children" property, element 1.

For that array concept I reserved literal "[*]" recently (patches welcome!),

> I think it could be worth exploring this idea,

but here child<...> is the type. Properties can have arbitrary names, in
some cases (containers) varying from instance to instance, thus are
dynamic. E.g., -device => /machine/peripheral-anon/device[n].

The peculiarity of child<> properties is that the property itself
contains the value pointer, rather than its parent object instance.
Therefore we'll need both class and object level properties, as I
thought you had done in your patch.

Markus, if we need an in-depth discussion, please put it on the agenda
for Tuesday. :)

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton; HRB 21284 (AG Nürnberg)

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 1/7] qom: allow properties to be registered against classes
  2015-09-03 17:21               ` Andreas Färber
@ 2015-09-03 17:25                 ` Daniel P. Berrange
  2015-09-04  6:56                   ` Markus Armbruster
  0 siblings, 1 reply; 26+ messages in thread
From: Daniel P. Berrange @ 2015-09-03 17:25 UTC (permalink / raw)
  To: Andreas Färber; +Cc: Paolo Bonzini, Markus Armbruster, qemu-devel

On Thu, Sep 03, 2015 at 07:21:07PM +0200, Andreas Färber wrote:
> Am 03.09.2015 um 19:09 schrieb Daniel P. Berrange:
> > On Thu, Sep 03, 2015 at 07:02:25PM +0200, Markus Armbruster wrote:
> >> Andreas Färber <afaerber@suse.de> writes:
> >>
> >>> Am 03.09.2015 um 18:37 schrieb Markus Armbruster:
> >>>> "Daniel P. Berrange" <berrange@redhat.com> writes:
> >>>>> On Wed, Sep 02, 2015 at 06:18:28PM +0200, Andreas Färber wrote:
> >>>>>> I had suggested exactly this looong time ago, but Anthony opposed it. I
> >>>>>> don't quite remember why...
> >>>>>
> >>>>> It is a while back now so I don't remember all aspects of the discussion
> >>>>> either. The main thing I remember is that we could not simply use the
> >>>>> existing GObject model from glib, since that exclusively records properties
> >>>>> against the object class. In some cases, particularly the relationships
> >>>>> between objects, QEMU needed to be able to define properties on the fly
> >>>>> against object instances.
> >>>>
> >>>> I remember Anthony's assertion that this is the case, but I don't
> >>>> remember the actual problems where this is actually the case.
> >>>>
> >>>> What properties do we currently define that could not be defined against
> >>>> the class?
> >>>
> >>> All child<> properties and everything on Container objects.
> >>
> >> Apologies if you had to explain this a dozen times already, but here
> >> goes my ignorant question anyway: why can't these properties be defined
> >> against the class?
> > 
> > IIUC, you can have zero or more "child<N>" properties registered.
> > You only know how many such properties to register at runtime,
> > and the count can vary per object instance. So you have to
> > register "child<1>", "child<2>", etc properties on objects.
> > 
> > If we wanted to register these against the class, we could
> > introduce an "array of objects" property type. So we would
> > just register a "children" array property against the class,
> > which can be populated with arbitrary number of objects at
> > runtime.  If we did this though, we'd probably need to setup
> > some backwards compat support so that any code (or external
> > user of QEMU) that tries to use "child<1>" would get transparently
> > forwarded to "children" property, element 1.
> 
> For that array concept I reserved literal "[*]" recently (patches welcome!),
> 
> > I think it could be worth exploring this idea,
> 
> but here child<...> is the type. Properties can have arbitrary names, in
> some cases (containers) varying from instance to instance, thus are
> dynamic. E.g., -device => /machine/peripheral-anon/device[n].

Good point, I had forgotten that.

> The peculiarity of child<> properties is that the property itself
> contains the value pointer, rather than its parent object instance.

Yep, that confused me for a while !

> Therefore we'll need both class and object level properties, as I
> thought you had done in your patch.

Yes, that's what this proposed patch series does.

> Markus, if we need an in-depth discussion, please put it on the agenda
> for Tuesday. :)

Personally I'd prefer to see us focus on just solving the simple
cases first, so we don't end up stuck arguing over the hard
cases and holding up potential quick wins !

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 1/7] qom: allow properties to be registered against classes
  2015-09-03 17:25                 ` Daniel P. Berrange
@ 2015-09-04  6:56                   ` Markus Armbruster
  2015-09-07 12:54                     ` Paolo Bonzini
  0 siblings, 1 reply; 26+ messages in thread
From: Markus Armbruster @ 2015-09-04  6:56 UTC (permalink / raw)
  To: Daniel P. Berrange; +Cc: Paolo Bonzini, Andreas Färber, qemu-devel

"Daniel P. Berrange" <berrange@redhat.com> writes:

> On Thu, Sep 03, 2015 at 07:21:07PM +0200, Andreas Färber wrote:
>> Am 03.09.2015 um 19:09 schrieb Daniel P. Berrange:
>> > On Thu, Sep 03, 2015 at 07:02:25PM +0200, Markus Armbruster wrote:
>> >> Andreas Färber <afaerber@suse.de> writes:
>> >>
>> >>> Am 03.09.2015 um 18:37 schrieb Markus Armbruster:
>> >>>> "Daniel P. Berrange" <berrange@redhat.com> writes:
>> >>>>> On Wed, Sep 02, 2015 at 06:18:28PM +0200, Andreas Färber wrote:
>> >>>>>> I had suggested exactly this looong time ago, but Anthony opposed it. I
>> >>>>>> don't quite remember why...
>> >>>>>
>> >>>>> It is a while back now so I don't remember all aspects of the discussion
>> >>>>> either. The main thing I remember is that we could not simply use the
>> >>>>> existing GObject model from glib, since that exclusively
>> >>>>> records properties
>> >>>>> against the object class. In some cases, particularly the relationships
>> >>>>> between objects, QEMU needed to be able to define properties on the fly
>> >>>>> against object instances.
>> >>>>
>> >>>> I remember Anthony's assertion that this is the case, but I don't
>> >>>> remember the actual problems where this is actually the case.
>> >>>>
>> >>>> What properties do we currently define that could not be defined against
>> >>>> the class?
>> >>>
>> >>> All child<> properties and everything on Container objects.
>> >>
>> >> Apologies if you had to explain this a dozen times already, but here
>> >> goes my ignorant question anyway: why can't these properties be defined
>> >> against the class?
>> > 
>> > IIUC, you can have zero or more "child<N>" properties registered.
>> > You only know how many such properties to register at runtime,
>> > and the count can vary per object instance. So you have to
>> > register "child<1>", "child<2>", etc properties on objects.
>> > 
>> > If we wanted to register these against the class, we could
>> > introduce an "array of objects" property type. So we would
>> > just register a "children" array property against the class,
>> > which can be populated with arbitrary number of objects at
>> > runtime.  If we did this though, we'd probably need to setup
>> > some backwards compat support so that any code (or external
>> > user of QEMU) that tries to use "child<1>" would get transparently
>> > forwarded to "children" property, element 1.
>> 
>> For that array concept I reserved literal "[*]" recently (patches welcome!),
>> 
>> > I think it could be worth exploring this idea,
>> 
>> but here child<...> is the type. Properties can have arbitrary names, in
>> some cases (containers) varying from instance to instance, thus are
>> dynamic. E.g., -device => /machine/peripheral-anon/device[n].
>
> Good point, I had forgotten that.
>
>> The peculiarity of child<> properties is that the property itself
>> contains the value pointer, rather than its parent object instance.
>
> Yep, that confused me for a while !
>
>> Therefore we'll need both class and object level properties, as I
>> thought you had done in your patch.
>
> Yes, that's what this proposed patch series does.
>
>> Markus, if we need an in-depth discussion, please put it on the agenda
>> for Tuesday. :)
>
> Personally I'd prefer to see us focus on just solving the simple
> cases first, so we don't end up stuck arguing over the hard
> cases and holding up potential quick wins !

That's a sensible approach when the solutions to the simple cases are
likely to stand regardless of whether and how we later crack the hard
cases.  I guess it's sensible here.  Let me explain.

I prefer defining properties in static data rather than code, because
reasoning over static data is so much easier.  If we can find a way to
do that even for the hard cases, lovely.  If we can't, then I'd still
prefer the softer cases done in data.  Two mechanisms instead of one
(bad), but the vast majority of cases becomes simpler (good).

So, if there's rough consensus Dan's RFC is worth pursuing, we can and
should defer the in-depth discussion of hard cases until we get around
to them.

Makes sense?

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 1/7] qom: allow properties to be registered against classes
  2015-08-26 12:03 ` [Qemu-devel] [PATCH RFC 1/7] qom: allow properties to be registered against classes Daniel P. Berrange
  2015-09-02 16:18   ` Andreas Färber
@ 2015-09-04 21:38   ` Marc-André Lureau
  2015-09-07  8:46     ` Daniel P. Berrange
  1 sibling, 1 reply; 26+ messages in thread
From: Marc-André Lureau @ 2015-09-04 21:38 UTC (permalink / raw)
  To: Daniel P. Berrange; +Cc: QEMU, Andreas Färber, Markus Armbruster

Hi

On Wed, Aug 26, 2015 at 2:03 PM, Daniel P. Berrange <berrange@redhat.com> wrote:
> +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;
> +    size_t name_len = strlen(name);
> +
> +    if (name_len >= 3 && !memcmp(name + name_len - 3, "[*]", 4)) {
> +        int i;
> +        ObjectProperty *ret;
> +        char *name_no_array = g_strdup(name);
> +

I question the need for dynamic/array property name registered in
classes. What would be more useful is an array property instead. It
would help to introspect classes for dynamic "children[*]" case.
object_property_add_child() could verify/check against the class
declaration, and grow the instance properties list (like it does now,
but it would be only for instances of children[] items). On
introspection of classes, the class "children[*]" property would be
visible, but would be hidden when introspecting the instance, and you
wouldn't be able to lookup that "array" property.

It seems relatively straightforward to deal with the link<> case, by
storing the offset of the "child" pointer. This seems fine if limited
to a single link<> (it should probably check the prop is not of the
name[*] style already), ex:
https://gist.github.com/elmarco/905241b683fb9c5f2a08

Your patches looks good  to me in general but object_property_del()
should be fixed, since the prop find may belong to the class.

cheers

-- 
Marc-André Lureau

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 1/7] qom: allow properties to be registered against classes
  2015-09-04 21:38   ` Marc-André Lureau
@ 2015-09-07  8:46     ` Daniel P. Berrange
  2015-09-07 13:11       ` Andreas Färber
  0 siblings, 1 reply; 26+ messages in thread
From: Daniel P. Berrange @ 2015-09-07  8:46 UTC (permalink / raw)
  To: Marc-André Lureau; +Cc: QEMU, Andreas Färber, Markus Armbruster

On Fri, Sep 04, 2015 at 11:38:06PM +0200, Marc-André Lureau wrote:
> Hi
> 
> On Wed, Aug 26, 2015 at 2:03 PM, Daniel P. Berrange <berrange@redhat.com> wrote:
> > +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;
> > +    size_t name_len = strlen(name);
> > +
> > +    if (name_len >= 3 && !memcmp(name + name_len - 3, "[*]", 4)) {
> > +        int i;
> > +        ObjectProperty *ret;
> > +        char *name_no_array = g_strdup(name);
> > +
> 
> I question the need for dynamic/array property name registered in
> classes. What would be more useful is an array property instead. It
> would help to introspect classes for dynamic "children[*]" case.
> object_property_add_child() could verify/check against the class
> declaration, and grow the instance properties list (like it does now,
> but it would be only for instances of children[] items). On
> introspection of classes, the class "children[*]" property would be
> visible, but would be hidden when introspecting the instance, and you
> wouldn't be able to lookup that "array" property.
> 
> It seems relatively straightforward to deal with the link<> case, by
> storing the offset of the "child" pointer. This seems fine if limited
> to a single link<> (it should probably check the prop is not of the
> name[*] style already), ex:
> https://gist.github.com/elmarco/905241b683fb9c5f2a08
> 
> Your patches looks good  to me in general but object_property_del()
> should be fixed, since the prop find may belong to the class.

Actually I skipped object_property_del() intentionally. Classes should
be immutable once defined, so deleting a property from a class would
not be appropriate.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 1/7] qom: allow properties to be registered against classes
  2015-09-04  6:56                   ` Markus Armbruster
@ 2015-09-07 12:54                     ` Paolo Bonzini
  0 siblings, 0 replies; 26+ messages in thread
From: Paolo Bonzini @ 2015-09-07 12:54 UTC (permalink / raw)
  To: Markus Armbruster, Daniel P. Berrange; +Cc: Andreas Färber, qemu-devel



On 04/09/2015 08:56, Markus Armbruster wrote:
>> >
>> > Personally I'd prefer to see us focus on just solving the simple
>> > cases first, so we don't end up stuck arguing over the hard
>> > cases and holding up potential quick wins !
> That's a sensible approach when the solutions to the simple cases are
> likely to stand regardless of whether and how we later crack the hard
> cases.  I guess it's sensible here.  Let me explain.
> 
> I prefer defining properties in static data rather than code, because
> reasoning over static data is so much easier.  If we can find a way to
> do that even for the hard cases, lovely.  If we can't, then I'd still
> prefer the softer cases done in data.  Two mechanisms instead of one
> (bad), but the vast majority of cases becomes simpler (good).

I agree.  Plus, we already have two mechanisms anyway "thanks" to qdev
static properties, so it's a net improvement.

paolo

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 1/7] qom: allow properties to be registered against classes
  2015-09-07  8:46     ` Daniel P. Berrange
@ 2015-09-07 13:11       ` Andreas Färber
  2015-09-07 13:17         ` Daniel P. Berrange
  0 siblings, 1 reply; 26+ messages in thread
From: Andreas Färber @ 2015-09-07 13:11 UTC (permalink / raw)
  To: Daniel P. Berrange; +Cc: Marc-André Lureau, QEMU, Markus Armbruster

Am 07.09.2015 um 10:46 schrieb Daniel P. Berrange:
> On Fri, Sep 04, 2015 at 11:38:06PM +0200, Marc-André Lureau wrote:
>> On Wed, Aug 26, 2015 at 2:03 PM, Daniel P. Berrange <berrange@redhat.com> wrote:
>>> +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;
>>> +    size_t name_len = strlen(name);
>>> +
>>> +    if (name_len >= 3 && !memcmp(name + name_len - 3, "[*]", 4)) {
>>> +        int i;
>>> +        ObjectProperty *ret;
>>> +        char *name_no_array = g_strdup(name);
>>> +
>>
>> I question the need for dynamic/array property name registered in
>> classes. What would be more useful is an array property instead. It
>> would help to introspect classes for dynamic "children[*]" case.
>> object_property_add_child() could verify/check against the class
>> declaration, and grow the instance properties list (like it does now,
>> but it would be only for instances of children[] items). On
>> introspection of classes, the class "children[*]" property would be
>> visible, but would be hidden when introspecting the instance, and you
>> wouldn't be able to lookup that "array" property.
>>
>> It seems relatively straightforward to deal with the link<> case, by
>> storing the offset of the "child" pointer. This seems fine if limited
>> to a single link<> (it should probably check the prop is not of the
>> name[*] style already), ex:
>> https://gist.github.com/elmarco/905241b683fb9c5f2a08
>>
>> Your patches looks good  to me in general but object_property_del()
>> should be fixed, since the prop find may belong to the class.
> 
> Actually I skipped object_property_del() intentionally. Classes should
> be immutable once defined, so deleting a property from a class would
> not be appropriate.

Agreed, I don't see a use case either.

Can you propose a sentence to amend the commit message with? Then I
would apply this patch to my upcoming qom-next pull, then the unreviewed
rest could go through their respective maintainers.

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton; HRB 21284 (AG Nürnberg)

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 1/7] qom: allow properties to be registered against classes
  2015-09-07 13:11       ` Andreas Färber
@ 2015-09-07 13:17         ` Daniel P. Berrange
  0 siblings, 0 replies; 26+ messages in thread
From: Daniel P. Berrange @ 2015-09-07 13:17 UTC (permalink / raw)
  To: Andreas Färber; +Cc: Marc-André Lureau, QEMU, Markus Armbruster

On Mon, Sep 07, 2015 at 03:11:56PM +0200, Andreas Färber wrote:
> Am 07.09.2015 um 10:46 schrieb Daniel P. Berrange:
> > On Fri, Sep 04, 2015 at 11:38:06PM +0200, Marc-André Lureau wrote:
> >> On Wed, Aug 26, 2015 at 2:03 PM, Daniel P. Berrange <berrange@redhat.com> wrote:
> >>> +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;
> >>> +    size_t name_len = strlen(name);
> >>> +
> >>> +    if (name_len >= 3 && !memcmp(name + name_len - 3, "[*]", 4)) {
> >>> +        int i;
> >>> +        ObjectProperty *ret;
> >>> +        char *name_no_array = g_strdup(name);
> >>> +
> >>
> >> I question the need for dynamic/array property name registered in
> >> classes. What would be more useful is an array property instead. It
> >> would help to introspect classes for dynamic "children[*]" case.
> >> object_property_add_child() could verify/check against the class
> >> declaration, and grow the instance properties list (like it does now,
> >> but it would be only for instances of children[] items). On
> >> introspection of classes, the class "children[*]" property would be
> >> visible, but would be hidden when introspecting the instance, and you
> >> wouldn't be able to lookup that "array" property.
> >>
> >> It seems relatively straightforward to deal with the link<> case, by
> >> storing the offset of the "child" pointer. This seems fine if limited
> >> to a single link<> (it should probably check the prop is not of the
> >> name[*] style already), ex:
> >> https://gist.github.com/elmarco/905241b683fb9c5f2a08
> >>
> >> Your patches looks good  to me in general but object_property_del()
> >> should be fixed, since the prop find may belong to the class.
> > 
> > Actually I skipped object_property_del() intentionally. Classes should
> > be immutable once defined, so deleting a property from a class would
> > not be appropriate.
> 
> Agreed, I don't see a use case either.
> 
> Can you propose a sentence to amend the commit message with? Then I
> would apply this patch to my upcoming qom-next pull, then the unreviewed
> rest could go through their respective maintainers.

"Supporting for deletion of properties registered on classes is
 omitted, since class definitions must be immutable once created"

Before you queue it though, I was going to repost with the support
for magic "[*]" property expansion removed, unless you think that
is really needed. It doesn't do anything you can't do explicitly
so I figure its cleaner to not add this magic to the class imp
too, as its known to suffer poor scalability.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 1/7] qom: allow properties to be registered against classes
  2015-09-02 16:18   ` Andreas Färber
  2015-09-03 15:49     ` Daniel P. Berrange
@ 2015-09-11 16:09     ` Daniel P. Berrange
  1 sibling, 0 replies; 26+ messages in thread
From: Daniel P. Berrange @ 2015-09-11 16:09 UTC (permalink / raw)
  To: Andreas Färber; +Cc: qemu-devel, Markus Armbruster

On Wed, Sep 02, 2015 at 06:18:28PM +0200, Andreas Färber wrote:
> Am 26.08.2015 um 14:03 schrieb Daniel P. Berrange:
> > 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.
> > 
> > Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> > ---
> >  include/qom/object.h |  44 ++++++++++
> >  qom/object.c         | 233 +++++++++++++++++++++++++++++++++++++++++++++++++--
> >  2 files changed, 270 insertions(+), 7 deletions(-)
> > 

[snip]

> Did you do any benchmarks on performance impact?

I created a crude test benchmark, consisting of an object which
has 20 string properties registered against it.

I then timed how long it took to create and free 1,000,000 instances
of this object.

 - Props registered against object: 8.5 seconds
 - Props registered against class:  1.1 seconds

I created a second test that also timed object accesses but there
was no measurable difference.

As expected, there is a considerable memory benefit - with 20 properties,
each object consumed 2600 bytes less, when registered to the class, or
approx 130 bytes less per property.

AFAICT, the performance benefit is essentially due to not having the
overhead of mallocing the ObjectProperty struct and its contents for
every new object instance.


BTW in testing this I found the g_mem_set_vtable() call has a
quite considerable performance penalty, even when tracing is not
compiled in. It added 17% to my test running time!  We might want
to investigate that performance impact in QEMU as a whole and
check that it is actually acceptable for real deployment.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2015-09-11 16:10 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-08-26 12:03 [Qemu-devel] [PATCH RFC 0/7] Making QOM introspectable Daniel P. Berrange
2015-08-26 12:03 ` [Qemu-devel] [PATCH RFC 1/7] qom: allow properties to be registered against classes Daniel P. Berrange
2015-09-02 16:18   ` Andreas Färber
2015-09-03 15:49     ` Daniel P. Berrange
2015-09-03 16:37       ` Markus Armbruster
2015-09-03 16:41         ` Andreas Färber
2015-09-03 17:02           ` Markus Armbruster
2015-09-03 17:09             ` Daniel P. Berrange
2015-09-03 17:21               ` Andreas Färber
2015-09-03 17:25                 ` Daniel P. Berrange
2015-09-04  6:56                   ` Markus Armbruster
2015-09-07 12:54                     ` Paolo Bonzini
2015-09-11 16:09     ` Daniel P. Berrange
2015-09-04 21:38   ` Marc-André Lureau
2015-09-07  8:46     ` Daniel P. Berrange
2015-09-07 13:11       ` Andreas Färber
2015-09-07 13:17         ` Daniel P. Berrange
2015-08-26 12:03 ` [Qemu-devel] [PATCH RFC 2/7] hostmem: register properties against the class instead of object Daniel P. Berrange
2015-08-26 12:03 ` [Qemu-devel] [PATCH RFC 3/7] rng: " Daniel P. Berrange
2015-08-26 12:03 ` [Qemu-devel] [PATCH RFC 4/7] tpm: " Daniel P. Berrange
2015-08-26 12:03 ` [Qemu-devel] [PATCH RFC 5/7] cpu: avoid using object instance state in property getter Daniel P. Berrange
2015-08-26 12:03 ` [Qemu-devel] [PATCH RFC 6/7] x86-cpu: register properties against the class instead of object Daniel P. Berrange
2015-08-26 12:03 ` [Qemu-devel] [PATCH RFC 7/7] machine: " Daniel P. Berrange
2015-09-02  9:05 ` [Qemu-devel] [PATCH RFC 0/7] Making QOM introspectable Daniel P. Berrange
2015-09-02 11:14   ` Markus Armbruster
2015-09-02 16:16   ` Andreas Färber

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.