[Qemu-devel] [RFC 0/4] QOM class properties - do we need them?

[Qemu-devel] [RFC 0/4] QOM class properties - do we need them?

[David Gibson]

QOM has the concept of both "object class" properties and "object
instance" properties.

The accessor functions installed for the rarely-used class properties
still take an Object *, so the *value* of such properties is still
per-instance; it's just the *existence* (and type) of the property
that is per-class.

Of course, that's also true in practice for the great majority of
"instance" properties, because they're created identically and
unconditionally for every instance from the per-class instance_init
hook.

This also means that the (unused) object_class_property_add_*_ptr()
functions don't make a lot of sense, since they require a fixed
pointer which means the value of such a property would only be
per-class.

Given that, is there really any value to supporting the "class"
properties in addition to the "instance" properties?  This series is
an RFC which removes all support for class properties, changing the
few existing users to instance properties instead.

Alternatively, if we *don't* want to remove class properties, should
we instead be trying to convert the many, many "instance" properties
whose existence is actually per-class to be class properties?

David Gibson (4):
  qcrypto: Remove usage of class properties
  s390: Don't use class properties
  tests: Remove tests for class properties
  qom: Abolish class properties

 crypto/secret.c            |  58 +++++++-------
 crypto/tlscreds.c          |  44 +++++-----
 crypto/tlscredsanon.c      |  16 ++--
 crypto/tlscredsx509.c      |  26 +++---
 include/qom/object.h       |  42 ----------
 qom/object.c               | 195 ---------------------------------------------
 target-s390x/cpu.c         |   1 -
 target-s390x/cpu_models.c  |  47 +++++------
 tests/check-qom-proplist.c |  30 +++----
 9 files changed, 106 insertions(+), 353 deletions(-)

-- 
2.7.4

[Qemu-devel] [RFC 1/4] qcrypto: Remove usage of class properties

[David Gibson]

qcrypto is one of the few users of QOM class properties, as opposed to
instance properties.  In anticipation of removing class properties,
convert qcrypto to use instance properties instead.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 crypto/secret.c       | 58 +++++++++++++++++++++++++++------------------------
 crypto/tlscreds.c     | 44 +++++++++++++++++---------------------
 crypto/tlscredsanon.c | 16 +++++++++-----
 crypto/tlscredsx509.c | 26 +++++++++++------------
 4 files changed, 74 insertions(+), 70 deletions(-)

diff --git a/crypto/secret.c b/crypto/secret.c
index 285ab7a..92905d9 100644
--- a/crypto/secret.c
+++ b/crypto/secret.c
@@ -352,6 +352,36 @@ qcrypto_secret_complete(UserCreatable *uc, Error **errp)
     object_property_set_bool(OBJECT(uc), true, "loaded", errp);
 }
 
+static void
+qcrypto_secret_instance_init(Object *obj)
+{
+    object_property_add_bool(obj, "loaded",
+                             qcrypto_secret_prop_get_loaded,
+                             qcrypto_secret_prop_set_loaded,
+                             NULL);
+    object_property_add_enum(obj, "format",
+                             "QCryptoSecretFormat",
+                             QCryptoSecretFormat_lookup,
+                             qcrypto_secret_prop_get_format,
+                             qcrypto_secret_prop_set_format,
+                             NULL);
+    object_property_add_str(obj, "data",
+                            qcrypto_secret_prop_get_data,
+                            qcrypto_secret_prop_set_data,
+                            NULL);
+    object_property_add_str(obj, "file",
+                            qcrypto_secret_prop_get_file,
+                            qcrypto_secret_prop_set_file,
+                            NULL);
+    object_property_add_str(obj, "keyid",
+                            qcrypto_secret_prop_get_keyid,
+                            qcrypto_secret_prop_set_keyid,
+                            NULL);
+    object_property_add_str(obj, "iv",
+                            qcrypto_secret_prop_get_iv,
+                            qcrypto_secret_prop_set_iv,
+                            NULL);
+}
 
 static void
 qcrypto_secret_finalize(Object *obj)
@@ -371,33 +401,6 @@ qcrypto_secret_class_init(ObjectClass *oc, void *data)
     UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc);
 
     ucc->complete = qcrypto_secret_complete;
-
-    object_class_property_add_bool(oc, "loaded",
-                                   qcrypto_secret_prop_get_loaded,
-                                   qcrypto_secret_prop_set_loaded,
-                                   NULL);
-    object_class_property_add_enum(oc, "format",
-                                   "QCryptoSecretFormat",
-                                   QCryptoSecretFormat_lookup,
-                                   qcrypto_secret_prop_get_format,
-                                   qcrypto_secret_prop_set_format,
-                                   NULL);
-    object_class_property_add_str(oc, "data",
-                                  qcrypto_secret_prop_get_data,
-                                  qcrypto_secret_prop_set_data,
-                                  NULL);
-    object_class_property_add_str(oc, "file",
-                                  qcrypto_secret_prop_get_file,
-                                  qcrypto_secret_prop_set_file,
-                                  NULL);
-    object_class_property_add_str(oc, "keyid",
-                                  qcrypto_secret_prop_get_keyid,
-                                  qcrypto_secret_prop_set_keyid,
-                                  NULL);
-    object_class_property_add_str(oc, "iv",
-                                  qcrypto_secret_prop_get_iv,
-                                  qcrypto_secret_prop_set_iv,
-                                  NULL);
 }
 
 
@@ -489,6 +492,7 @@ static const TypeInfo qcrypto_secret_info = {
     .parent = TYPE_OBJECT,
     .name = TYPE_QCRYPTO_SECRET,
     .instance_size = sizeof(QCryptoSecret),
+    .instance_init = qcrypto_secret_instance_init,
     .instance_finalize = qcrypto_secret_finalize,
     .class_size = sizeof(QCryptoSecretClass),
     .class_init = qcrypto_secret_class_init,
diff --git a/crypto/tlscreds.c b/crypto/tlscreds.c
index a896553..4aafdd5 100644
--- a/crypto/tlscreds.c
+++ b/crypto/tlscreds.c
@@ -221,35 +221,30 @@ qcrypto_tls_creds_prop_get_endpoint(Object *obj,
 
 
 static void
-qcrypto_tls_creds_class_init(ObjectClass *oc, void *data)
-{
-    object_class_property_add_bool(oc, "verify-peer",
-                                   qcrypto_tls_creds_prop_get_verify,
-                                   qcrypto_tls_creds_prop_set_verify,
-                                   NULL);
-    object_class_property_add_str(oc, "dir",
-                                  qcrypto_tls_creds_prop_get_dir,
-                                  qcrypto_tls_creds_prop_set_dir,
-                                  NULL);
-    object_class_property_add_enum(oc, "endpoint",
-                                   "QCryptoTLSCredsEndpoint",
-                                   QCryptoTLSCredsEndpoint_lookup,
-                                   qcrypto_tls_creds_prop_get_endpoint,
-                                   qcrypto_tls_creds_prop_set_endpoint,
-                                   NULL);
-    object_class_property_add_str(oc, "priority",
-                                  qcrypto_tls_creds_prop_get_priority,
-                                  qcrypto_tls_creds_prop_set_priority,
-                                  NULL);
-}
-
-
-static void
 qcrypto_tls_creds_init(Object *obj)
 {
     QCryptoTLSCreds *creds = QCRYPTO_TLS_CREDS(obj);
 
     creds->verifyPeer = true;
+
+    object_property_add_bool(obj, "verify-peer",
+                             qcrypto_tls_creds_prop_get_verify,
+                             qcrypto_tls_creds_prop_set_verify,
+                             NULL);
+    object_property_add_str(obj, "dir",
+                            qcrypto_tls_creds_prop_get_dir,
+                            qcrypto_tls_creds_prop_set_dir,
+                            NULL);
+    object_property_add_enum(obj, "endpoint",
+                             "QCryptoTLSCredsEndpoint",
+                             QCryptoTLSCredsEndpoint_lookup,
+                             qcrypto_tls_creds_prop_get_endpoint,
+                             qcrypto_tls_creds_prop_set_endpoint,
+                             NULL);
+    object_property_add_str(obj, "priority",
+                            qcrypto_tls_creds_prop_get_priority,
+                            qcrypto_tls_creds_prop_set_priority,
+                            NULL);
 }
 
 
@@ -269,7 +264,6 @@ static const TypeInfo qcrypto_tls_creds_info = {
     .instance_size = sizeof(QCryptoTLSCreds),
     .instance_init = qcrypto_tls_creds_init,
     .instance_finalize = qcrypto_tls_creds_finalize,
-    .class_init = qcrypto_tls_creds_class_init,
     .class_size = sizeof(QCryptoTLSCredsClass),
     .abstract = true,
 };
diff --git a/crypto/tlscredsanon.c b/crypto/tlscredsanon.c
index 1464220..7e86760 100644
--- a/crypto/tlscredsanon.c
+++ b/crypto/tlscredsanon.c
@@ -173,6 +173,16 @@ qcrypto_tls_creds_anon_complete(UserCreatable *uc, Error **errp)
 
 
 static void
+qcrypto_tls_creds_anon_instance_init(Object *obj)
+{
+    object_property_add_bool(obj, "loaded",
+                             qcrypto_tls_creds_anon_prop_get_loaded,
+                             qcrypto_tls_creds_anon_prop_set_loaded,
+                             NULL);
+}
+
+
+static void
 qcrypto_tls_creds_anon_finalize(Object *obj)
 {
     QCryptoTLSCredsAnon *creds = QCRYPTO_TLS_CREDS_ANON(obj);
@@ -187,11 +197,6 @@ qcrypto_tls_creds_anon_class_init(ObjectClass *oc, void *data)
     UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc);
 
     ucc->complete = qcrypto_tls_creds_anon_complete;
-
-    object_class_property_add_bool(oc, "loaded",
-                                   qcrypto_tls_creds_anon_prop_get_loaded,
-                                   qcrypto_tls_creds_anon_prop_set_loaded,
-                                   NULL);
 }
 
 
@@ -199,6 +204,7 @@ static const TypeInfo qcrypto_tls_creds_anon_info = {
     .parent = TYPE_QCRYPTO_TLS_CREDS,
     .name = TYPE_QCRYPTO_TLS_CREDS_ANON,
     .instance_size = sizeof(QCryptoTLSCredsAnon),
+    .instance_init = qcrypto_tls_creds_anon_instance_init,
     .instance_finalize = qcrypto_tls_creds_anon_finalize,
     .class_size = sizeof(QCryptoTLSCredsAnonClass),
     .class_init = qcrypto_tls_creds_anon_class_init,
diff --git a/crypto/tlscredsx509.c b/crypto/tlscredsx509.c
index 50eb54f..7213849 100644
--- a/crypto/tlscredsx509.c
+++ b/crypto/tlscredsx509.c
@@ -810,6 +810,19 @@ qcrypto_tls_creds_x509_init(Object *obj)
     QCryptoTLSCredsX509 *creds = QCRYPTO_TLS_CREDS_X509(obj);
 
     creds->sanityCheck = true;
+
+    object_property_add_bool(obj, "loaded",
+                             qcrypto_tls_creds_x509_prop_get_loaded,
+                             qcrypto_tls_creds_x509_prop_set_loaded,
+                             NULL);
+    object_property_add_bool(obj, "sanity-check",
+                             qcrypto_tls_creds_x509_prop_get_sanity,
+                             qcrypto_tls_creds_x509_prop_set_sanity,
+                             NULL);
+    object_property_add_str(obj, "passwordid",
+                            qcrypto_tls_creds_x509_prop_get_passwordid,
+                            qcrypto_tls_creds_x509_prop_set_passwordid,
+                            NULL);
 }
 
 
@@ -829,19 +842,6 @@ qcrypto_tls_creds_x509_class_init(ObjectClass *oc, void *data)
     UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc);
 
     ucc->complete = qcrypto_tls_creds_x509_complete;
-
-    object_class_property_add_bool(oc, "loaded",
-                                   qcrypto_tls_creds_x509_prop_get_loaded,
-                                   qcrypto_tls_creds_x509_prop_set_loaded,
-                                   NULL);
-    object_class_property_add_bool(oc, "sanity-check",
-                                   qcrypto_tls_creds_x509_prop_get_sanity,
-                                   qcrypto_tls_creds_x509_prop_set_sanity,
-                                   NULL);
-    object_class_property_add_str(oc, "passwordid",
-                                  qcrypto_tls_creds_x509_prop_get_passwordid,
-                                  qcrypto_tls_creds_x509_prop_set_passwordid,
-                                  NULL);
 }
 
 
-- 
2.7.4

[Qemu-devel] [RFC 2/4] s390: Don't use class properties

[David Gibson]

s390 is one of the very few users of QOM class properties (as opposed
to instance properties).  In anticipation of removing support for
class properties, convert s390 to use instance properties instead.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 target-s390x/cpu.c        |  1 -
 target-s390x/cpu_models.c | 47 ++++++++++++++++++++++-------------------------
 2 files changed, 22 insertions(+), 26 deletions(-)

diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
index 2f3c8e2..cadeef4 100644
--- a/target-s390x/cpu.c
+++ b/target-s390x/cpu.c
@@ -446,7 +446,6 @@ static void s390_cpu_class_init(ObjectClass *oc, void *data)
      * object_unref().
      */
     dc->cannot_destroy_with_object_finalize_yet = true;
-    s390_cpu_model_class_register_props(oc);
 }
 
 static const TypeInfo s390_cpu_type_info = {
diff --git a/target-s390x/cpu_models.c b/target-s390x/cpu_models.c
index 3ff6a70..4e9afa3 100644
--- a/target-s390x/cpu_models.c
+++ b/target-s390x/cpu_models.c
@@ -865,11 +865,33 @@ static void set_feature_group(Object *obj, Visitor *v, const char *name,
     }
 }
 
+static bool get_is_migration_safe(Object *obj, Error **errp)
+{
+    return S390_CPU_GET_CLASS(obj)->is_migration_safe;
+}
+
+static bool get_is_static(Object *obj, Error **errp)
+{
+    return S390_CPU_GET_CLASS(obj)->is_static;
+}
+
+static char *get_description(Object *obj, Error **errp)
+{
+    return g_strdup(S390_CPU_GET_CLASS(obj)->desc);
+}
+
 void s390_cpu_model_register_props(Object *obj)
 {
     S390FeatGroup group;
     S390Feat feat;
 
+    object_property_add_bool(obj, "migration-safe", get_is_migration_safe,
+                             NULL, NULL);
+    object_property_add_bool(obj, "static", get_is_static,
+                             NULL, NULL);
+    object_property_add_str(obj, "description", get_description, NULL,
+                            NULL);
+
     for (feat = 0; feat < S390_FEAT_MAX; feat++) {
         const S390FeatDef *def = s390_feat_def(feat);
         object_property_add(obj, def->name, "bool", get_feature,
@@ -943,31 +965,6 @@ static void s390_cpu_model_finalize(Object *obj)
     cpu->model = NULL;
 }
 
-static bool get_is_migration_safe(Object *obj, Error **errp)
-{
-    return S390_CPU_GET_CLASS(obj)->is_migration_safe;
-}
-
-static bool get_is_static(Object *obj, Error **errp)
-{
-    return S390_CPU_GET_CLASS(obj)->is_static;
-}
-
-static char *get_description(Object *obj, Error **errp)
-{
-    return g_strdup(S390_CPU_GET_CLASS(obj)->desc);
-}
-
-void s390_cpu_model_class_register_props(ObjectClass *oc)
-{
-    object_class_property_add_bool(oc, "migration-safe", get_is_migration_safe,
-                                   NULL, NULL);
-    object_class_property_add_bool(oc, "static", get_is_static,
-                                   NULL, NULL);
-    object_class_property_add_str(oc, "description", get_description, NULL,
-                                  NULL);
-}
-
 #ifdef CONFIG_KVM
 static void s390_host_cpu_model_class_init(ObjectClass *oc, void *data)
 {
-- 
2.7.4

[Qemu-devel] [RFC 3/4] tests: Remove tests for class properties

[David Gibson]

There are no-longer any users of QOM class properties (as opposed to
instance properties).  In anticipation of removing support for them,
remove tests for them from the testsuite.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 tests/check-qom-proplist.c | 30 ++++++++++--------------------
 1 file changed, 10 insertions(+), 20 deletions(-)

diff --git a/tests/check-qom-proplist.c b/tests/check-qom-proplist.c
index a16cefc..e684949 100644
--- a/tests/check-qom-proplist.c
+++ b/tests/check-qom-proplist.c
@@ -124,25 +124,16 @@ static void dummy_init(Object *obj)
                              dummy_get_bv,
                              dummy_set_bv,
                              NULL);
-}
-
-
-static void dummy_class_init(ObjectClass *cls, void *data)
-{
-    object_class_property_add_bool(cls, "bv",
-                                   dummy_get_bv,
-                                   dummy_set_bv,
-                                   NULL);
-    object_class_property_add_str(cls, "sv",
-                                  dummy_get_sv,
-                                  dummy_set_sv,
-                                  NULL);
-    object_class_property_add_enum(cls, "av",
-                                   "DummyAnimal",
-                                   dummy_animal_map,
-                                   dummy_get_av,
-                                   dummy_set_av,
-                                   NULL);
+    object_property_add_str(obj, "sv",
+                            dummy_get_sv,
+                            dummy_set_sv,
+                            NULL);
+    object_property_add_enum(obj, "av",
+                             "DummyAnimal",
+                             dummy_animal_map,
+                             dummy_get_av,
+                             dummy_set_av,
+                             NULL);
 }
 
 
@@ -161,7 +152,6 @@ static const TypeInfo dummy_info = {
     .instance_init = dummy_init,
     .instance_finalize = dummy_finalize,
     .class_size = sizeof(DummyObjectClass),
-    .class_init = dummy_class_init,
 };
 
 
-- 
2.7.4

[Qemu-devel] [RFC 4/4] qom: Abolish class properties

[David Gibson]

QOM allows both object class properties as well as object instance
properties.  object class properties were never much used, and provide
no real benefit.  Note that it's only the *existence* of such
properties that's established on a per-class basis - the way the
accessors work, the property's *value* is still per-instance.

Generally it's just as easy to create an instance property for every
instance from the (per-class) instance_init hook as it is to create a
class property.

Therefore, remove all support for object class properties, simplifying
the QOM model a bit.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 include/qom/object.h |  42 -----------
 qom/object.c         | 195 ---------------------------------------------------
 2 files changed, 237 deletions(-)

diff --git a/include/qom/object.h b/include/qom/object.h
index 5ecc2d1..8b4e96c 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -941,13 +941,6 @@ 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
@@ -958,8 +951,6 @@ ObjectProperty *object_class_property_add(ObjectClass *klass, const char *name,
  */
 ObjectProperty *object_property_find(Object *obj, const char *name,
                                      Error **errp);
-ObjectProperty *object_class_property_find(ObjectClass *klass, const char *name,
-                                           Error **errp);
 
 typedef struct ObjectPropertyIterator {
     ObjectClass *nextclass;
@@ -1375,12 +1366,6 @@ 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
@@ -1397,11 +1382,6 @@ 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
@@ -1421,13 +1401,6 @@ 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
@@ -1442,10 +1415,6 @@ 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
@@ -1458,8 +1427,6 @@ void object_class_property_add_tm(ObjectClass *klass, 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:
@@ -1473,8 +1440,6 @@ void object_class_property_add_uint8_ptr(ObjectClass *klass, 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:
@@ -1488,8 +1453,6 @@ void object_class_property_add_uint16_ptr(ObjectClass *klass, 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:
@@ -1503,8 +1466,6 @@ void object_class_property_add_uint32_ptr(ObjectClass *klass, 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:
@@ -1556,9 +1517,6 @@ 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 8166b7d..5a00cf5 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -948,50 +948,10 @@ object_property_add(Object *obj, const char *name, const char *type,
     return prop;
 }
 
-ObjectProperty *
-object_class_property_add(ObjectClass *klass,
-                          const char *name,
-                          const char *type,
-                          ObjectPropertyAccessor *get,
-                          ObjectPropertyAccessor *set,
-                          ObjectPropertyRelease *release,
-                          void *opaque,
-                          Error **errp)
-{
-    ObjectProperty *prop;
-
-    if (object_class_property_find(klass, name, NULL) != NULL) {
-        error_setg(errp, "attempt to add duplicate property '%s'"
-                   " to object (type '%s')", name,
-                   object_class_get_name(klass));
-        return NULL;
-    }
-
-    prop = g_malloc0(sizeof(*prop));
-
-    prop->name = g_strdup(name);
-    prop->type = g_strdup(type);
-
-    prop->get = get;
-    prop->set = set;
-    prop->release = release;
-    prop->opaque = opaque;
-
-    g_hash_table_insert(klass->properties, g_strdup(name), prop);
-
-    return prop;
-}
-
 ObjectProperty *object_property_find(Object *obj, const char *name,
                                      Error **errp)
 {
     ObjectProperty *prop;
-    ObjectClass *klass = object_get_class(obj);
-
-    prop = object_class_property_find(klass, name, NULL);
-    if (prop) {
-        return prop;
-    }
 
     prop = g_hash_table_lookup(obj->properties, name);
     if (prop) {
@@ -1022,27 +982,6 @@ ObjectProperty *object_property_iter_next(ObjectPropertyIterator *iter)
     return val;
 }
 
-ObjectProperty *object_class_property_find(ObjectClass *klass, const char *name,
-                                           Error **errp)
-{
-    ObjectProperty *prop;
-    ObjectClass *parent_klass;
-
-    parent_klass = object_class_get_parent(klass);
-    if (parent_klass) {
-        prop = object_class_property_find(parent_klass, name, NULL);
-        if (prop) {
-            return prop;
-        }
-    }
-
-    prop = g_hash_table_lookup(klass->properties, name);
-    if (!prop) {
-        error_setg(errp, "Property '.%s' not found", name);
-    }
-    return prop;
-}
-
 void object_property_del(Object *obj, const char *name, Error **errp)
 {
     ObjectProperty *prop = g_hash_table_lookup(obj->properties, name);
@@ -1792,29 +1731,6 @@ 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 **);
@@ -1882,28 +1798,6 @@ 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, const char *name,
                               void *opaque, Error **errp)
 {
@@ -1967,31 +1861,6 @@ 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;
@@ -2070,25 +1939,6 @@ 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));
@@ -2129,13 +1979,6 @@ 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)
 {
@@ -2143,13 +1986,6 @@ 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)
 {
@@ -2157,13 +1993,6 @@ 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)
 {
@@ -2171,13 +2000,6 @@ 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;
@@ -2275,23 +2097,6 @@ void object_property_set_description(Object *obj, const char *name,
     op->description = g_strdup(description);
 }
 
-void object_class_property_set_description(ObjectClass *klass,
-                                           const char *name,
-                                           const char *description,
-                                           Error **errp)
-{
-    ObjectProperty *op;
-
-    op = g_hash_table_lookup(klass->properties, name);
-    if (!op) {
-        error_setg(errp, "Property '.%s' not found", name);
-        return;
-    }
-
-    g_free(op->description);
-    op->description = g_strdup(description);
-}
-
 static void object_instance_init(Object *obj)
 {
     object_property_add_str(obj, "type", qdev_get_type, NULL, NULL);
-- 
2.7.4

Re: [Qemu-devel] [RFC 0/4] QOM class properties - do we need them?

[Paolo Bonzini]

On 29/09/2016 02:16, David Gibson wrote:
> Alternatively, if we *don't* want to remove class properties, should
> we instead be trying to convert the many, many "instance" properties
> whose existence is actually per-class to be class properties?

Yes, this was the point of introducing them. :)

Paolo

> David Gibson (4):
>   qcrypto: Remove usage of class properties
>   s390: Don't use class properties
>   tests: Remove tests for class properties
>   qom: Abolish class properties
> 
>  crypto/secret.c            |  58 +++++++-------
>  crypto/tlscreds.c          |  44 +++++-----
>  crypto/tlscredsanon.c      |  16 ++--
>  crypto/tlscredsx509.c      |  26 +++---
>  include/qom/object.h       |  42 ----------
>  qom/object.c               | 195 ---------------------------------------------
>  target-s390x/cpu.c         |   1 -
>  target-s390x/cpu_models.c  |  47 +++++------
>  tests/check-qom-proplist.c |  30 +++----
>  9 files changed, 106 insertions(+), 353 deletions(-)
> 

Re: [Qemu-devel] [RFC 0/4] QOM class properties - do we need them?

[Daniel P. Berrange]

On Thu, Sep 29, 2016 at 10:16:41AM +1000, David Gibson wrote:
> QOM has the concept of both "object class" properties and "object
> instance" properties.
> 
> The accessor functions installed for the rarely-used class properties
> still take an Object *, so the *value* of such properties is still
> per-instance; it's just the *existence* (and type) of the property
> that is per-class.

Yes, of course. This is the whole point of class properties. It avoids
allocating the same ObjectProperty struct against every object instance
which wastes massive amounts of memory in scenarios where there are lots
of instances created.

> Of course, that's also true in practice for the great majority of
> "instance" properties, because they're created identically and
> unconditionally for every instance from the per-class instance_init
> hook.
> 
> This also means that the (unused) object_class_property_add_*_ptr()
> functions don't make a lot of sense, since they require a fixed
> pointer which means the value of such a property would only be
> per-class.
> 
> Given that, is there really any value to supporting the "class"
> properties in addition to the "instance" properties?  This series is
> an RFC which removes all support for class properties, changing the
> few existing users to instance properties instead.
> 
> Alternatively, if we *don't* want to remove class properties, should
> we instead be trying to convert the many, many "instance" properties
> whose existence is actually per-class to be class properties?

Practically all instances properties should become class properties
as its going to save wasting memory once most are converted.

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

Re: [Qemu-devel] [RFC 0/4] QOM class properties - do we need them?

[Andreas Färber]

Am 29.09.2016 um 10:14 schrieb Daniel P. Berrange:
> On Thu, Sep 29, 2016 at 10:16:41AM +1000, David Gibson wrote:
>> QOM has the concept of both "object class" properties and "object
>> instance" properties.
>>
>> The accessor functions installed for the rarely-used class properties
>> still take an Object *, so the *value* of such properties is still
>> per-instance; it's just the *existence* (and type) of the property
>> that is per-class.
> 
> Yes, of course. This is the whole point of class properties. It avoids
> allocating the same ObjectProperty struct against every object instance
> which wastes massive amounts of memory in scenarios where there are lots
> of instances created.

+1

>> Of course, that's also true in practice for the great majority of
>> "instance" properties, because they're created identically and
>> unconditionally for every instance from the per-class instance_init
>> hook.
>>
>> This also means that the (unused) object_class_property_add_*_ptr()
>> functions don't make a lot of sense, since they require a fixed
>> pointer which means the value of such a property would only be
>> per-class.
>>
>> Given that, is there really any value to supporting the "class"
>> properties in addition to the "instance" properties?  This series is
>> an RFC which removes all support for class properties, changing the
>> few existing users to instance properties instead.
>>
>> Alternatively, if we *don't* want to remove class properties, should
>> we instead be trying to convert the many, many "instance" properties
>> whose existence is actually per-class to be class properties?
> 
> Practically all instances properties should become class properties
> as its going to save wasting memory once most are converted.

Not all, but most. child<> properties were the reason to have properties
on the instance.

Regards,
Andreas

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

Re: [Qemu-devel] [RFC 0/4] QOM class properties - do we need them?

[Andreas Färber]

Am 29.09.2016 um 02:16 schrieb David Gibson:
> is there really any value to supporting the "class"
> properties in addition to the "instance" properties?

Yes, it makes enumerating available properties easier by not requiring
to instantiate a new instance for printing, e.g., ",help" information.

Regards,
Andreas

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

Re: [Qemu-devel] [RFC 0/4] QOM class properties - do we need them?

[Daniel P. Berrange]

On Thu, Sep 29, 2016 at 12:12:32PM +0200, Andreas Färber wrote:
> Am 29.09.2016 um 10:14 schrieb Daniel P. Berrange:
> > On Thu, Sep 29, 2016 at 10:16:41AM +1000, David Gibson wrote:
> >> QOM has the concept of both "object class" properties and "object
> >> instance" properties.
> >>
> >> The accessor functions installed for the rarely-used class properties
> >> still take an Object *, so the *value* of such properties is still
> >> per-instance; it's just the *existence* (and type) of the property
> >> that is per-class.
> > 
> > Yes, of course. This is the whole point of class properties. It avoids
> > allocating the same ObjectProperty struct against every object instance
> > which wastes massive amounts of memory in scenarios where there are lots
> > of instances created.
> 
> +1
> 
> >> Of course, that's also true in practice for the great majority of
> >> "instance" properties, because they're created identically and
> >> unconditionally for every instance from the per-class instance_init
> >> hook.
> >>
> >> This also means that the (unused) object_class_property_add_*_ptr()
> >> functions don't make a lot of sense, since they require a fixed
> >> pointer which means the value of such a property would only be
> >> per-class.
> >>
> >> Given that, is there really any value to supporting the "class"
> >> properties in addition to the "instance" properties?  This series is
> >> an RFC which removes all support for class properties, changing the
> >> few existing users to instance properties instead.
> >>
> >> Alternatively, if we *don't* want to remove class properties, should
> >> we instead be trying to convert the many, many "instance" properties
> >> whose existence is actually per-class to be class properties?
> > 
> > Practically all instances properties should become class properties
> > as its going to save wasting memory once most are converted.
> 
> Not all, but most. child<> properties were the reason to have properties
> on the instance.

That's why I said "Practically all", instead of just "all" :-)

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

Re: [Qemu-devel] [RFC 0/4] QOM class properties - do we need them?

[Andreas Färber]

Am 29.09.2016 um 12:21 schrieb Daniel P. Berrange:
> On Thu, Sep 29, 2016 at 12:12:32PM +0200, Andreas Färber wrote:
>> Am 29.09.2016 um 10:14 schrieb Daniel P. Berrange:
>>> Practically all instances properties should become class properties
>>> as its going to save wasting memory once most are converted.
>>
>> Not all, but most. child<> properties were the reason to have properties
>> on the instance.
> 
> That's why I said "Practically all", instead of just "all" :-)

To me as non-native speaker "practically" is the opposite of
"theoretically". :)

Cheers,
Andreas

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

Re: [Qemu-devel] [RFC 0/4] QOM class properties - do we need them?

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 2550 bytes --]

On Thu, Sep 29, 2016 at 09:14:16AM +0100, Daniel P. Berrange wrote:
> On Thu, Sep 29, 2016 at 10:16:41AM +1000, David Gibson wrote:
> > QOM has the concept of both "object class" properties and "object
> > instance" properties.
> > 
> > The accessor functions installed for the rarely-used class properties
> > still take an Object *, so the *value* of such properties is still
> > per-instance; it's just the *existence* (and type) of the property
> > that is per-class.
> 
> Yes, of course. This is the whole point of class properties. It avoids
> allocating the same ObjectProperty struct against every object instance
> which wastes massive amounts of memory in scenarios where there are lots
> of instances created.

Ah, that makes sense.

> > Of course, that's also true in practice for the great majority of
> > "instance" properties, because they're created identically and
> > unconditionally for every instance from the per-class instance_init
> > hook.
> > 
> > This also means that the (unused) object_class_property_add_*_ptr()
> > functions don't make a lot of sense, since they require a fixed
> > pointer which means the value of such a property would only be
> > per-class.
> > 
> > Given that, is there really any value to supporting the "class"
> > properties in addition to the "instance" properties?  This series is
> > an RFC which removes all support for class properties, changing the
> > few existing users to instance properties instead.
> > 
> > Alternatively, if we *don't* want to remove class properties, should
> > we instead be trying to convert the many, many "instance" properties
> > whose existence is actually per-class to be class properties?
> 
> Practically all instances properties should become class properties
> as its going to save wasting memory once most are converted.

Heh, ok.  Well, I'll keep that in mind when I'm adding properties in
future.  I wonder if there's a way we can better get the word out that
this is how properties should usually be done.

That said.. I'm still thinking we should remove
object_class_property_add_*_ptr().  Those take an actual pointer to
the value, meaning that it can't have different values per-instance.
These only create read-only properties, so they're not actually
dangerous, but they really don't seem very useful.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] [RFC 0/4] QOM class properties - do we need them?

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1235 bytes --]

On Thu, Sep 29, 2016 at 12:23:41PM +0200, Andreas Färber wrote:
> Am 29.09.2016 um 12:21 schrieb Daniel P. Berrange:
> > On Thu, Sep 29, 2016 at 12:12:32PM +0200, Andreas Färber wrote:
> >> Am 29.09.2016 um 10:14 schrieb Daniel P. Berrange:
> >>> Practically all instances properties should become class properties
> >>> as its going to save wasting memory once most are converted.
> >>
> >> Not all, but most. child<> properties were the reason to have properties
> >> on the instance.
> > 
> > That's why I said "Practically all", instead of just "all" :-)
> 
> To me as non-native speaker "practically" is the opposite of
> "theoretically". :)

Heh, more English corner cases.  "Theoretically" and "in theory" mean
the same thing, but "practically" and "in practice" don't.  Or.. they
do in theory, but not in practice :p.  For bonus confusion, saying
"practically speaking" would, in this context, mean what you expected,
but "practically" on its own doesn't - it means basically the same as
"almost" or "nearly".

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] [RFC 0/4] QOM class properties - do we need them?

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 699 bytes --]

On Thu, Sep 29, 2016 at 12:16:48PM +0200, Andreas Färber wrote:
> Am 29.09.2016 um 02:16 schrieb David Gibson:
> > is there really any value to supporting the "class"
> > properties in addition to the "instance" properties?
> 
> Yes, it makes enumerating available properties easier by not requiring
> to instantiate a new instance for printing, e.g., ",help"
> information.

A good point.  I shall try to be on the lookup for opportunities to
change existing properties to class properties.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] [RFC 0/4] QOM class properties - do we need them?

[Daniel P. Berrange]

On Fri, Sep 30, 2016 at 09:33:20AM +1000, David Gibson wrote:
> On Thu, Sep 29, 2016 at 09:14:16AM +0100, Daniel P. Berrange wrote:
> > On Thu, Sep 29, 2016 at 10:16:41AM +1000, David Gibson wrote:
> > > QOM has the concept of both "object class" properties and "object
> > > instance" properties.
> > > 
> > > The accessor functions installed for the rarely-used class properties
> > > still take an Object *, so the *value* of such properties is still
> > > per-instance; it's just the *existence* (and type) of the property
> > > that is per-class.
> > 
> > Yes, of course. This is the whole point of class properties. It avoids
> > allocating the same ObjectProperty struct against every object instance
> > which wastes massive amounts of memory in scenarios where there are lots
> > of instances created.
> 
> Ah, that makes sense.
> 
> > > Of course, that's also true in practice for the great majority of
> > > "instance" properties, because they're created identically and
> > > unconditionally for every instance from the per-class instance_init
> > > hook.
> > > 
> > > This also means that the (unused) object_class_property_add_*_ptr()
> > > functions don't make a lot of sense, since they require a fixed
> > > pointer which means the value of such a property would only be
> > > per-class.
> > > 
> > > Given that, is there really any value to supporting the "class"
> > > properties in addition to the "instance" properties?  This series is
> > > an RFC which removes all support for class properties, changing the
> > > few existing users to instance properties instead.
> > > 
> > > Alternatively, if we *don't* want to remove class properties, should
> > > we instead be trying to convert the many, many "instance" properties
> > > whose existence is actually per-class to be class properties?
> > 
> > Practically all instances properties should become class properties
> > as its going to save wasting memory once most are converted.
> 
> Heh, ok.  Well, I'll keep that in mind when I'm adding properties in
> future.  I wonder if there's a way we can better get the word out that
> this is how properties should usually be done.
> 
> That said.. I'm still thinking we should remove
> object_class_property_add_*_ptr().  Those take an actual pointer to
> the value, meaning that it can't have different values per-instance.
> These only create read-only properties, so they're not actually
> dangerous, but they really don't seem very useful.

Yeah, that method seems dubious - my mistake in just copying all
the existing property methods.


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

Re: [Qemu-devel] [RFC 0/4] QOM class properties - do we need them?

[Kevin Wolf]

Am 29.09.2016 um 12:23 hat Andreas Färber geschrieben:
> Am 29.09.2016 um 12:21 schrieb Daniel P. Berrange:
> > On Thu, Sep 29, 2016 at 12:12:32PM +0200, Andreas Färber wrote:
> >> Am 29.09.2016 um 10:14 schrieb Daniel P. Berrange:
> >>> Practically all instances properties should become class properties
> >>> as its going to save wasting memory once most are converted.
> >>
> >> Not all, but most. child<> properties were the reason to have properties
> >> on the instance.
> > 
> > That's why I said "Practically all", instead of just "all" :-)
> 
> To me as non-native speaker "practically" is the opposite of
> "theoretically". :)

To me it made perfect sense that "pratically all" means "praktisch
alle". ;-)

Kevin