* [PATCH 00/12] qom: Make all -object types use only class properties
@ 2020-10-09 16:01 Eduardo Habkost
2020-10-09 16:01 ` [PATCH 01/12] qom: Helpers for pointer properties Eduardo Habkost
` (11 more replies)
0 siblings, 12 replies; 28+ messages in thread
From: Eduardo Habkost @ 2020-10-09 16:01 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Marc-André Lureau, John Snow, Markus Armbruster
To make introspection possible, make all TYPE_USER_CREATABLE
classes use only class properties, and add a mechanism to lock
QOM properties so writable instance properties won't be allowed
anymore.
Eduardo Habkost (12):
qom: Helpers for pointer properties
qom: Introduce PointerProperty struct
qom: Make object_class_property_add_uint*_ptr() get offset
sev: Use class properties
rng: Use class properties
can_host: Use class properties
colo: Use class properties
netfilter: Reorder functions
netfilter: Use class properties
input: Use class properties
[RFC] qom: Property lock mechanism
[RFC] qom: Lock properties of all TYPE_USER_CREATABLE types
include/qom/object.h | 25 +++-
backends/rng-egd.c | 10 +-
backends/rng-random.c | 8 +-
backends/rng.c | 6 +-
net/can/can_host.c | 16 +--
net/colo-compare.c | 57 ++++----
net/dump.c | 10 +-
net/filter-buffer.c | 26 ++--
net/filter-mirror.c | 64 ++++-----
net/filter-rewriter.c | 7 +-
net/filter.c | 24 ++--
qom/object.c | 256 ++++++++++++++++++---------------
qom/object_interfaces.c | 11 ++
target/i386/sev.c | 22 +--
tests/check-qom-proplist.c | 10 +-
tests/test-qdev-global-props.c | 70 +++++++++
ui/input-barrier.c | 44 +++---
ui/input-linux.c | 27 ++--
18 files changed, 407 insertions(+), 286 deletions(-)
--
2.26.2
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 01/12] qom: Helpers for pointer properties
2020-10-09 16:01 [PATCH 00/12] qom: Make all -object types use only class properties Eduardo Habkost
@ 2020-10-09 16:01 ` Eduardo Habkost
2020-10-09 16:01 ` [PATCH 02/12] qom: Introduce PointerProperty struct Eduardo Habkost
` (10 subsequent siblings)
11 siblings, 0 replies; 28+ messages in thread
From: Eduardo Habkost @ 2020-10-09 16:01 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Daniel P. Berrangé,
Markus Armbruster, Paolo Bonzini, Marc-André Lureau,
John Snow
Reduce some code duplication and make future refactoring easier,
by adding object_property_add_uint_ptr() and
object_class_property_add_uint_ptr() helpers.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: "Daniel P. Berrangé" <berrange@redhat.com>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: qemu-devel@nongnu.org
---
qom/object.c | 168 ++++++++++++++++++++-------------------------------
1 file changed, 64 insertions(+), 104 deletions(-)
diff --git a/qom/object.c b/qom/object.c
index 1065355233..313d2f9834 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -2529,24 +2529,44 @@ static void property_set_uint64_ptr(Object *obj, Visitor *v, const char *name,
*field = value;
}
+static ObjectProperty *
+object_property_add_uint_ptr(Object *obj, const char *name,
+ const char *type,
+ ObjectPropertyAccessor getter,
+ ObjectPropertyAccessor setter,
+ ObjectPropertyFlags flags,
+ void *ptr)
+{
+ return object_property_add(obj, name, type,
+ (flags & OBJ_PROP_FLAG_READ) ? getter : NULL,
+ (flags & OBJ_PROP_FLAG_WRITE) ? setter : NULL,
+ NULL, ptr);
+}
+
+static ObjectProperty *
+object_class_property_add_uint_ptr(ObjectClass *oc, const char *name,
+ const char *type,
+ ObjectPropertyAccessor getter,
+ ObjectPropertyAccessor setter,
+ ObjectPropertyFlags flags,
+ void *ptr)
+{
+ return object_class_property_add(oc, name, type,
+ (flags & OBJ_PROP_FLAG_READ) ? getter : NULL,
+ (flags & OBJ_PROP_FLAG_WRITE) ? setter : NULL,
+ NULL, ptr);
+}
+
ObjectProperty *
object_property_add_uint8_ptr(Object *obj, const char *name,
const uint8_t *v,
ObjectPropertyFlags flags)
{
- ObjectPropertyAccessor *getter = NULL;
- ObjectPropertyAccessor *setter = NULL;
-
- if ((flags & OBJ_PROP_FLAG_READ) == OBJ_PROP_FLAG_READ) {
- getter = property_get_uint8_ptr;
- }
-
- if ((flags & OBJ_PROP_FLAG_WRITE) == OBJ_PROP_FLAG_WRITE) {
- setter = property_set_uint8_ptr;
- }
-
- return object_property_add(obj, name, "uint8",
- getter, setter, NULL, (void *)v);
+ return object_property_add_uint_ptr(obj, name, "uint8",
+ property_get_uint8_ptr,
+ property_set_uint8_ptr,
+ flags,
+ (void *)v);
}
ObjectProperty *
@@ -2554,19 +2574,10 @@ object_class_property_add_uint8_ptr(ObjectClass *klass, const char *name,
const uint8_t *v,
ObjectPropertyFlags flags)
{
- ObjectPropertyAccessor *getter = NULL;
- ObjectPropertyAccessor *setter = NULL;
-
- if ((flags & OBJ_PROP_FLAG_READ) == OBJ_PROP_FLAG_READ) {
- getter = property_get_uint8_ptr;
- }
-
- if ((flags & OBJ_PROP_FLAG_WRITE) == OBJ_PROP_FLAG_WRITE) {
- setter = property_set_uint8_ptr;
- }
-
- return object_class_property_add(klass, name, "uint8",
- getter, setter, NULL, (void *)v);
+ return object_class_property_add_uint_ptr(klass, name, "uint8",
+ property_get_uint8_ptr,
+ property_set_uint8_ptr,
+ flags, (void *)v);
}
ObjectProperty *
@@ -2574,19 +2585,11 @@ object_property_add_uint16_ptr(Object *obj, const char *name,
const uint16_t *v,
ObjectPropertyFlags flags)
{
- ObjectPropertyAccessor *getter = NULL;
- ObjectPropertyAccessor *setter = NULL;
-
- if ((flags & OBJ_PROP_FLAG_READ) == OBJ_PROP_FLAG_READ) {
- getter = property_get_uint16_ptr;
- }
-
- if ((flags & OBJ_PROP_FLAG_WRITE) == OBJ_PROP_FLAG_WRITE) {
- setter = property_set_uint16_ptr;
- }
-
- return object_property_add(obj, name, "uint16",
- getter, setter, NULL, (void *)v);
+ return object_property_add_uint_ptr(obj, name, "uint16",
+ property_get_uint16_ptr,
+ property_set_uint16_ptr,
+ flags,
+ (void *)v);
}
ObjectProperty *
@@ -2594,19 +2597,10 @@ object_class_property_add_uint16_ptr(ObjectClass *klass, const char *name,
const uint16_t *v,
ObjectPropertyFlags flags)
{
- ObjectPropertyAccessor *getter = NULL;
- ObjectPropertyAccessor *setter = NULL;
-
- if ((flags & OBJ_PROP_FLAG_READ) == OBJ_PROP_FLAG_READ) {
- getter = property_get_uint16_ptr;
- }
-
- if ((flags & OBJ_PROP_FLAG_WRITE) == OBJ_PROP_FLAG_WRITE) {
- setter = property_set_uint16_ptr;
- }
-
- return object_class_property_add(klass, name, "uint16",
- getter, setter, NULL, (void *)v);
+ return object_class_property_add_uint_ptr(klass, name, "uint16",
+ property_get_uint16_ptr,
+ property_set_uint16_ptr,
+ flags, (void *)v);
}
ObjectProperty *
@@ -2614,19 +2608,11 @@ object_property_add_uint32_ptr(Object *obj, const char *name,
const uint32_t *v,
ObjectPropertyFlags flags)
{
- ObjectPropertyAccessor *getter = NULL;
- ObjectPropertyAccessor *setter = NULL;
-
- if ((flags & OBJ_PROP_FLAG_READ) == OBJ_PROP_FLAG_READ) {
- getter = property_get_uint32_ptr;
- }
-
- if ((flags & OBJ_PROP_FLAG_WRITE) == OBJ_PROP_FLAG_WRITE) {
- setter = property_set_uint32_ptr;
- }
-
- return object_property_add(obj, name, "uint32",
- getter, setter, NULL, (void *)v);
+ return object_property_add_uint_ptr(obj, name, "uint32",
+ property_get_uint32_ptr,
+ property_set_uint32_ptr,
+ flags,
+ (void *)v);
}
ObjectProperty *
@@ -2634,19 +2620,10 @@ object_class_property_add_uint32_ptr(ObjectClass *klass, const char *name,
const uint32_t *v,
ObjectPropertyFlags flags)
{
- ObjectPropertyAccessor *getter = NULL;
- ObjectPropertyAccessor *setter = NULL;
-
- if ((flags & OBJ_PROP_FLAG_READ) == OBJ_PROP_FLAG_READ) {
- getter = property_get_uint32_ptr;
- }
-
- if ((flags & OBJ_PROP_FLAG_WRITE) == OBJ_PROP_FLAG_WRITE) {
- setter = property_set_uint32_ptr;
- }
-
- return object_class_property_add(klass, name, "uint32",
- getter, setter, NULL, (void *)v);
+ return object_class_property_add_uint_ptr(klass, name, "uint32",
+ property_get_uint32_ptr,
+ property_set_uint32_ptr,
+ flags, (void *)v);
}
ObjectProperty *
@@ -2654,19 +2631,11 @@ object_property_add_uint64_ptr(Object *obj, const char *name,
const uint64_t *v,
ObjectPropertyFlags flags)
{
- ObjectPropertyAccessor *getter = NULL;
- ObjectPropertyAccessor *setter = NULL;
-
- if ((flags & OBJ_PROP_FLAG_READ) == OBJ_PROP_FLAG_READ) {
- getter = property_get_uint64_ptr;
- }
-
- if ((flags & OBJ_PROP_FLAG_WRITE) == OBJ_PROP_FLAG_WRITE) {
- setter = property_set_uint64_ptr;
- }
-
- return object_property_add(obj, name, "uint64",
- getter, setter, NULL, (void *)v);
+ return object_property_add_uint_ptr(obj, name, "uint64",
+ property_get_uint64_ptr,
+ property_set_uint64_ptr,
+ flags,
+ (void *)v);
}
ObjectProperty *
@@ -2674,19 +2643,10 @@ object_class_property_add_uint64_ptr(ObjectClass *klass, const char *name,
const uint64_t *v,
ObjectPropertyFlags flags)
{
- ObjectPropertyAccessor *getter = NULL;
- ObjectPropertyAccessor *setter = NULL;
-
- if ((flags & OBJ_PROP_FLAG_READ) == OBJ_PROP_FLAG_READ) {
- getter = property_get_uint64_ptr;
- }
-
- if ((flags & OBJ_PROP_FLAG_WRITE) == OBJ_PROP_FLAG_WRITE) {
- setter = property_set_uint64_ptr;
- }
-
- return object_class_property_add(klass, name, "uint64",
- getter, setter, NULL, (void *)v);
+ return object_class_property_add_uint_ptr(klass, name, "uint64",
+ property_get_uint64_ptr,
+ property_set_uint64_ptr,
+ flags, (void *)v);
}
typedef struct {
--
2.26.2
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 02/12] qom: Introduce PointerProperty struct
2020-10-09 16:01 [PATCH 00/12] qom: Make all -object types use only class properties Eduardo Habkost
2020-10-09 16:01 ` [PATCH 01/12] qom: Helpers for pointer properties Eduardo Habkost
@ 2020-10-09 16:01 ` Eduardo Habkost
2020-10-09 16:01 ` [PATCH 03/12] qom: Make object_class_property_add_uint*_ptr() get offset Eduardo Habkost
` (9 subsequent siblings)
11 siblings, 0 replies; 28+ messages in thread
From: Eduardo Habkost @ 2020-10-09 16:01 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Daniel P. Berrangé,
Markus Armbruster, Paolo Bonzini, Marc-André Lureau,
John Snow
PointerProperty will hold additional info about pointer
properties. It will allow us to implement more complex logic in
pointer property getters and setters.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: "Daniel P. Berrangé" <berrange@redhat.com>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: qemu-devel@nongnu.org
---
qom/object.c | 46 ++++++++++++++++++++++++++++++++++++----------
1 file changed, 36 insertions(+), 10 deletions(-)
diff --git a/qom/object.c b/qom/object.c
index 313d2f9834..17692ed5c3 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -2449,17 +2449,30 @@ static char *object_get_type(Object *obj, Error **errp)
return g_strdup(object_get_typename(obj));
}
+typedef struct {
+ /* Pointer to property value */
+ void *ptr;
+} PointerProperty;
+
+static void *pointer_property_get_ptr(Object *obj, PointerProperty *prop)
+{
+ return prop->ptr;
+}
+
static void property_get_uint8_ptr(Object *obj, Visitor *v, const char *name,
void *opaque, Error **errp)
{
- uint8_t value = *(uint8_t *)opaque;
+ PointerProperty *prop = opaque;
+ uint8_t *field = pointer_property_get_ptr(obj, prop);
+ uint8_t value = *field;
visit_type_uint8(v, name, &value, errp);
}
static void property_set_uint8_ptr(Object *obj, Visitor *v, const char *name,
void *opaque, Error **errp)
{
- uint8_t *field = opaque;
+ PointerProperty *prop = opaque;
+ uint8_t *field = pointer_property_get_ptr(obj, prop);
uint8_t value;
if (!visit_type_uint8(v, name, &value, errp)) {
@@ -2472,14 +2485,17 @@ static void property_set_uint8_ptr(Object *obj, Visitor *v, const char *name,
static void property_get_uint16_ptr(Object *obj, Visitor *v, const char *name,
void *opaque, Error **errp)
{
- uint16_t value = *(uint16_t *)opaque;
+ PointerProperty *prop = opaque;
+ uint16_t *field = pointer_property_get_ptr(obj, prop);
+ uint16_t value = *field;
visit_type_uint16(v, name, &value, errp);
}
static void property_set_uint16_ptr(Object *obj, Visitor *v, const char *name,
void *opaque, Error **errp)
{
- uint16_t *field = opaque;
+ PointerProperty *prop = opaque;
+ uint16_t *field = pointer_property_get_ptr(obj, prop);
uint16_t value;
if (!visit_type_uint16(v, name, &value, errp)) {
@@ -2492,14 +2508,17 @@ static void property_set_uint16_ptr(Object *obj, Visitor *v, const char *name,
static void property_get_uint32_ptr(Object *obj, Visitor *v, const char *name,
void *opaque, Error **errp)
{
- uint32_t value = *(uint32_t *)opaque;
+ PointerProperty *prop = opaque;
+ uint32_t *field = pointer_property_get_ptr(obj, prop);
+ uint32_t value = *field;
visit_type_uint32(v, name, &value, errp);
}
static void property_set_uint32_ptr(Object *obj, Visitor *v, const char *name,
void *opaque, Error **errp)
{
- uint32_t *field = opaque;
+ PointerProperty *prop = opaque;
+ uint32_t *field = pointer_property_get_ptr(obj, prop);
uint32_t value;
if (!visit_type_uint32(v, name, &value, errp)) {
@@ -2512,14 +2531,17 @@ static void property_set_uint32_ptr(Object *obj, Visitor *v, const char *name,
static void property_get_uint64_ptr(Object *obj, Visitor *v, const char *name,
void *opaque, Error **errp)
{
- uint64_t value = *(uint64_t *)opaque;
+ PointerProperty *prop = opaque;
+ uint64_t *field = pointer_property_get_ptr(obj, prop);
+ uint64_t value = *field;
visit_type_uint64(v, name, &value, errp);
}
static void property_set_uint64_ptr(Object *obj, Visitor *v, const char *name,
void *opaque, Error **errp)
{
- uint64_t *field = opaque;
+ PointerProperty *prop = opaque;
+ uint64_t *field = pointer_property_get_ptr(obj, prop);
uint64_t value;
if (!visit_type_uint64(v, name, &value, errp)) {
@@ -2537,10 +2559,12 @@ object_property_add_uint_ptr(Object *obj, const char *name,
ObjectPropertyFlags flags,
void *ptr)
{
+ PointerProperty *prop = g_new0(PointerProperty, 1);
+ prop->ptr = ptr;
return object_property_add(obj, name, type,
(flags & OBJ_PROP_FLAG_READ) ? getter : NULL,
(flags & OBJ_PROP_FLAG_WRITE) ? setter : NULL,
- NULL, ptr);
+ NULL, (void *)prop);
}
static ObjectProperty *
@@ -2551,10 +2575,12 @@ object_class_property_add_uint_ptr(ObjectClass *oc, const char *name,
ObjectPropertyFlags flags,
void *ptr)
{
+ PointerProperty *prop = g_new0(PointerProperty, 1);
+ prop->ptr = ptr;
return object_class_property_add(oc, name, type,
(flags & OBJ_PROP_FLAG_READ) ? getter : NULL,
(flags & OBJ_PROP_FLAG_WRITE) ? setter : NULL,
- NULL, ptr);
+ NULL, (void *)prop);
}
ObjectProperty *
--
2.26.2
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 03/12] qom: Make object_class_property_add_uint*_ptr() get offset
2020-10-09 16:01 [PATCH 00/12] qom: Make all -object types use only class properties Eduardo Habkost
2020-10-09 16:01 ` [PATCH 01/12] qom: Helpers for pointer properties Eduardo Habkost
2020-10-09 16:01 ` [PATCH 02/12] qom: Introduce PointerProperty struct Eduardo Habkost
@ 2020-10-09 16:01 ` Eduardo Habkost
2020-10-09 17:24 ` Eric Blake
2020-10-21 12:24 ` Igor Mammedov
2020-10-09 16:01 ` [PATCH 04/12] sev: Use class properties Eduardo Habkost
` (8 subsequent siblings)
11 siblings, 2 replies; 28+ messages in thread
From: Eduardo Habkost @ 2020-10-09 16:01 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Daniel P. Berrangé,
Markus Armbruster, Paolo Bonzini, Marc-André Lureau,
John Snow
The existing object_class_property_add_uint*_ptr() functions are
not very useful, because they need a pointer to the property
value, which can't really be provided before the object is
created.
Replace the pointer parameter in those functions with a
`ptrdiff_t offset` parameter.
Include a uint8 class property in check-qom-proplist unit tests,
to ensure the feature is working.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: "Daniel P. Berrangé" <berrange@redhat.com>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: qemu-devel@nongnu.org
---
include/qom/object.h | 8 ++++----
qom/object.c | 36 +++++++++++++++++++++++-------------
tests/check-qom-proplist.c | 10 ++++++++--
3 files changed, 35 insertions(+), 19 deletions(-)
diff --git a/include/qom/object.h b/include/qom/object.h
index d378f13a11..1634294e4f 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -1747,7 +1747,7 @@ ObjectProperty *object_property_add_uint8_ptr(Object *obj, const char *name,
ObjectProperty *object_class_property_add_uint8_ptr(ObjectClass *klass,
const char *name,
- const uint8_t *v,
+ ptrdiff_t offset,
ObjectPropertyFlags flags);
/**
@@ -1768,7 +1768,7 @@ ObjectProperty *object_property_add_uint16_ptr(Object *obj, const char *name,
ObjectProperty *object_class_property_add_uint16_ptr(ObjectClass *klass,
const char *name,
- const uint16_t *v,
+ ptrdiff_t offset,
ObjectPropertyFlags flags);
/**
@@ -1789,7 +1789,7 @@ ObjectProperty *object_property_add_uint32_ptr(Object *obj, const char *name,
ObjectProperty *object_class_property_add_uint32_ptr(ObjectClass *klass,
const char *name,
- const uint32_t *v,
+ ptrdiff_t offset,
ObjectPropertyFlags flags);
/**
@@ -1810,7 +1810,7 @@ ObjectProperty *object_property_add_uint64_ptr(Object *obj, const char *name,
ObjectProperty *object_class_property_add_uint64_ptr(ObjectClass *klass,
const char *name,
- const uint64_t *v,
+ ptrdiff_t offset,
ObjectPropertyFlags flags);
/**
diff --git a/qom/object.c b/qom/object.c
index 17692ed5c3..bb32f5d3ad 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -2450,13 +2450,22 @@ static char *object_get_type(Object *obj, Error **errp)
}
typedef struct {
- /* Pointer to property value */
- void *ptr;
+ bool is_offset;
+ union {
+ /* Pointer to property value. Valid if is_offset=false */
+ void *ptr;
+ /* Offset in Object struct. Valid if is_offset=true */
+ ptrdiff_t offset;
+ };
} PointerProperty;
static void *pointer_property_get_ptr(Object *obj, PointerProperty *prop)
{
- return prop->ptr;
+ if (prop->is_offset) {
+ return (void *)obj + prop->offset;
+ } else {
+ return prop->ptr;
+ }
}
static void property_get_uint8_ptr(Object *obj, Visitor *v, const char *name,
@@ -2573,10 +2582,11 @@ object_class_property_add_uint_ptr(ObjectClass *oc, const char *name,
ObjectPropertyAccessor getter,
ObjectPropertyAccessor setter,
ObjectPropertyFlags flags,
- void *ptr)
+ ptrdiff_t offset)
{
PointerProperty *prop = g_new0(PointerProperty, 1);
- prop->ptr = ptr;
+ prop->is_offset = true;
+ prop->offset = offset;
return object_class_property_add(oc, name, type,
(flags & OBJ_PROP_FLAG_READ) ? getter : NULL,
(flags & OBJ_PROP_FLAG_WRITE) ? setter : NULL,
@@ -2597,13 +2607,13 @@ object_property_add_uint8_ptr(Object *obj, const char *name,
ObjectProperty *
object_class_property_add_uint8_ptr(ObjectClass *klass, const char *name,
- const uint8_t *v,
+ ptrdiff_t offset,
ObjectPropertyFlags flags)
{
return object_class_property_add_uint_ptr(klass, name, "uint8",
property_get_uint8_ptr,
property_set_uint8_ptr,
- flags, (void *)v);
+ flags, offset);
}
ObjectProperty *
@@ -2620,13 +2630,13 @@ object_property_add_uint16_ptr(Object *obj, const char *name,
ObjectProperty *
object_class_property_add_uint16_ptr(ObjectClass *klass, const char *name,
- const uint16_t *v,
+ ptrdiff_t offset,
ObjectPropertyFlags flags)
{
return object_class_property_add_uint_ptr(klass, name, "uint16",
property_get_uint16_ptr,
property_set_uint16_ptr,
- flags, (void *)v);
+ flags, offset);
}
ObjectProperty *
@@ -2643,13 +2653,13 @@ object_property_add_uint32_ptr(Object *obj, const char *name,
ObjectProperty *
object_class_property_add_uint32_ptr(ObjectClass *klass, const char *name,
- const uint32_t *v,
+ ptrdiff_t offset,
ObjectPropertyFlags flags)
{
return object_class_property_add_uint_ptr(klass, name, "uint32",
property_get_uint32_ptr,
property_set_uint32_ptr,
- flags, (void *)v);
+ flags, offset);
}
ObjectProperty *
@@ -2666,13 +2676,13 @@ object_property_add_uint64_ptr(Object *obj, const char *name,
ObjectProperty *
object_class_property_add_uint64_ptr(ObjectClass *klass, const char *name,
- const uint64_t *v,
+ ptrdiff_t offset,
ObjectPropertyFlags flags)
{
return object_class_property_add_uint_ptr(klass, name, "uint64",
property_get_uint64_ptr,
property_set_uint64_ptr,
- flags, (void *)v);
+ flags, offset);
}
typedef struct {
diff --git a/tests/check-qom-proplist.c b/tests/check-qom-proplist.c
index 1b76581980..fba30c20b2 100644
--- a/tests/check-qom-proplist.c
+++ b/tests/check-qom-proplist.c
@@ -61,6 +61,7 @@ struct DummyObject {
bool bv;
DummyAnimal av;
char *sv;
+ uint8_t u8v;
};
struct DummyObjectClass {
@@ -141,6 +142,9 @@ static void dummy_class_init(ObjectClass *cls, void *data)
&dummy_animal_map,
dummy_get_av,
dummy_set_av);
+ object_class_property_add_uint8_ptr(cls, "u8v",
+ offsetof(DummyObject, u8v),
+ OBJ_PROP_FLAG_READWRITE);
}
@@ -385,12 +389,14 @@ static void test_dummy_createlist(void)
"bv", "yes",
"sv", "Hiss hiss hiss",
"av", "platypus",
+ "u8v", "42",
NULL));
g_assert(err == NULL);
g_assert_cmpstr(dobj->sv, ==, "Hiss hiss hiss");
g_assert(dobj->bv == true);
g_assert(dobj->av == DUMMY_PLATYPUS);
+ g_assert_cmpint(dobj->u8v, ==, 42);
g_assert(object_resolve_path_component(parent, "dummy0")
== OBJECT(dobj));
@@ -531,7 +537,7 @@ static void test_dummy_iterator(void)
{
const char *expected[] = {
"type", /* inherited from TYPE_OBJECT */
- "sv", "av", /* class properties */
+ "sv", "av", "u8v", /* class properties */
"bv"}; /* instance property */
Object *parent = object_get_objects_root();
DummyObject *dobj = DUMMY_OBJECT(
@@ -552,7 +558,7 @@ static void test_dummy_iterator(void)
static void test_dummy_class_iterator(void)
{
- const char *expected[] = { "type", "av", "sv" };
+ const char *expected[] = { "type", "av", "sv", "u8v" };
ObjectPropertyIterator iter;
ObjectClass *klass = object_class_by_name(TYPE_DUMMY);
--
2.26.2
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 04/12] sev: Use class properties
2020-10-09 16:01 [PATCH 00/12] qom: Make all -object types use only class properties Eduardo Habkost
` (2 preceding siblings ...)
2020-10-09 16:01 ` [PATCH 03/12] qom: Make object_class_property_add_uint*_ptr() get offset Eduardo Habkost
@ 2020-10-09 16:01 ` Eduardo Habkost
2020-10-09 16:01 ` [PATCH 05/12] rng: " Eduardo Habkost
` (7 subsequent siblings)
11 siblings, 0 replies; 28+ messages in thread
From: Eduardo Habkost @ 2020-10-09 16:01 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Markus Armbruster, Paolo Bonzini,
Marc-André Lureau, John Snow, Richard Henderson
Instance properties make introspection hard and are not shown by
"-object ...,help". Convert them to class properties.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: qemu-devel@nongnu.org
---
target/i386/sev.c | 22 +++++++++++++---------
1 file changed, 13 insertions(+), 9 deletions(-)
diff --git a/target/i386/sev.c b/target/i386/sev.c
index 93c4d60b82..d1c9247c14 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -298,6 +298,19 @@ sev_guest_class_init(ObjectClass *oc, void *data)
sev_guest_set_session_file);
object_class_property_set_description(oc, "session-file",
"guest owners session parameters (encoded with base64)");
+
+ object_class_property_add_uint32_ptr(oc, "policy",
+ offsetof(SevGuestState, policy),
+ OBJ_PROP_FLAG_READWRITE);
+ object_class_property_add_uint32_ptr(oc, "handle",
+ offsetof(SevGuestState, handle),
+ OBJ_PROP_FLAG_READWRITE);
+ object_class_property_add_uint32_ptr(oc, "cbitpos",
+ offsetof(SevGuestState, cbitpos),
+ OBJ_PROP_FLAG_READWRITE);
+ object_class_property_add_uint32_ptr(oc, "reduced-phys-bits",
+ offsetof(SevGuestState, reduced_phys_bits),
+ OBJ_PROP_FLAG_READWRITE);
}
static void
@@ -307,15 +320,6 @@ sev_guest_instance_init(Object *obj)
sev->sev_device = g_strdup(DEFAULT_SEV_DEVICE);
sev->policy = DEFAULT_GUEST_POLICY;
- object_property_add_uint32_ptr(obj, "policy", &sev->policy,
- OBJ_PROP_FLAG_READWRITE);
- object_property_add_uint32_ptr(obj, "handle", &sev->handle,
- OBJ_PROP_FLAG_READWRITE);
- object_property_add_uint32_ptr(obj, "cbitpos", &sev->cbitpos,
- OBJ_PROP_FLAG_READWRITE);
- object_property_add_uint32_ptr(obj, "reduced-phys-bits",
- &sev->reduced_phys_bits,
- OBJ_PROP_FLAG_READWRITE);
}
/* sev guest info */
--
2.26.2
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 05/12] rng: Use class properties
2020-10-09 16:01 [PATCH 00/12] qom: Make all -object types use only class properties Eduardo Habkost
` (3 preceding siblings ...)
2020-10-09 16:01 ` [PATCH 04/12] sev: Use class properties Eduardo Habkost
@ 2020-10-09 16:01 ` Eduardo Habkost
2020-10-09 16:01 ` [PATCH 06/12] can_host: " Eduardo Habkost
` (6 subsequent siblings)
11 siblings, 0 replies; 28+ messages in thread
From: Eduardo Habkost @ 2020-10-09 16:01 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Laurent Vivier, Amit Shah, Markus Armbruster,
Marc-André Lureau, John Snow
Instance properties make introspection hard and are not shown by
"-object ...,help". Convert them to class properties.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Cc: Laurent Vivier <lvivier@redhat.com>
Cc: Amit Shah <amit@kernel.org>
Cc: qemu-devel@nongnu.org
---
backends/rng-egd.c | 10 +++-------
backends/rng-random.c | 8 ++++----
backends/rng.c | 6 +++---
3 files changed, 10 insertions(+), 14 deletions(-)
diff --git a/backends/rng-egd.c b/backends/rng-egd.c
index 20198ff26e..600eef3f75 100644
--- a/backends/rng-egd.c
+++ b/backends/rng-egd.c
@@ -135,12 +135,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);
-}
-
static void rng_egd_finalize(Object *obj)
{
RngEgd *s = RNG_EGD(obj);
@@ -153,6 +147,9 @@ static void rng_egd_class_init(ObjectClass *klass, void *data)
{
RngBackendClass *rbc = RNG_BACKEND_CLASS(klass);
+ object_class_property_add_str(klass, "chardev",
+ rng_egd_get_chardev, rng_egd_set_chardev);
+
rbc->request_entropy = rng_egd_request_entropy;
rbc->opened = rng_egd_opened;
}
@@ -162,7 +159,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 245b12ab24..27152b8cdb 100644
--- a/backends/rng-random.c
+++ b/backends/rng-random.c
@@ -108,10 +108,6 @@ static void rng_random_init(Object *obj)
{
RngRandom *s = RNG_RANDOM(obj);
- object_property_add_str(obj, "filename",
- rng_random_get_filename,
- rng_random_set_filename);
-
s->filename = g_strdup("/dev/urandom");
s->fd = -1;
}
@@ -132,6 +128,10 @@ static void rng_random_class_init(ObjectClass *klass, void *data)
{
RngBackendClass *rbc = RNG_BACKEND_CLASS(klass);
+ object_class_property_add_str(klass, "filename",
+ rng_random_get_filename,
+ rng_random_set_filename);
+
rbc->request_entropy = rng_random_request_entropy;
rbc->opened = rng_random_opened;
}
diff --git a/backends/rng.c b/backends/rng.c
index 484f04e891..f018c380da 100644
--- a/backends/rng.c
+++ b/backends/rng.c
@@ -106,9 +106,6 @@ static void rng_backend_init(Object *obj)
QSIMPLEQ_INIT(&s->requests);
- object_property_add_bool(obj, "opened",
- rng_backend_prop_get_opened,
- rng_backend_prop_set_opened);
}
static void rng_backend_finalize(Object *obj)
@@ -122,6 +119,9 @@ static void rng_backend_class_init(ObjectClass *oc, void *data)
{
UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc);
+ object_class_property_add_bool(oc, "opened",
+ rng_backend_prop_get_opened,
+ rng_backend_prop_set_opened);
ucc->complete = rng_backend_complete;
}
--
2.26.2
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 06/12] can_host: Use class properties
2020-10-09 16:01 [PATCH 00/12] qom: Make all -object types use only class properties Eduardo Habkost
` (4 preceding siblings ...)
2020-10-09 16:01 ` [PATCH 05/12] rng: " Eduardo Habkost
@ 2020-10-09 16:01 ` Eduardo Habkost
2020-10-12 14:52 ` Pavel Pisa
2020-10-09 16:01 ` [PATCH 07/12] colo: " Eduardo Habkost
` (5 subsequent siblings)
11 siblings, 1 reply; 28+ messages in thread
From: Eduardo Habkost @ 2020-10-09 16:01 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Vikram Garhwal, Jason Wang, Markus Armbruster,
Marc-André Lureau, John Snow, Pavel Pisa
Instance properties make introspection hard and are not shown by
"-object ...,help". Convert them to class properties.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Cc: Pavel Pisa <pisa@cmp.felk.cvut.cz>
Cc: Vikram Garhwal <fnu.vikram@xilinx.com>
Cc: Jason Wang <jasowang@redhat.com>
Cc: qemu-devel@nongnu.org
---
net/can/can_host.c | 16 +++++-----------
1 file changed, 5 insertions(+), 11 deletions(-)
diff --git a/net/can/can_host.c b/net/can/can_host.c
index be4547d913..bec88b9620 100644
--- a/net/can/can_host.c
+++ b/net/can/can_host.c
@@ -72,21 +72,16 @@ static void can_host_complete(UserCreatable *uc, Error **errp)
can_host_connect(CAN_HOST(uc), errp);
}
-static void can_host_instance_init(Object *obj)
-{
- CanHostState *ch = CAN_HOST(obj);
-
- object_property_add_link(obj, "canbus", TYPE_CAN_BUS,
- (Object **)&ch->bus,
- object_property_allow_set_link,
- OBJ_PROP_LINK_STRONG);
-}
-
static void can_host_class_init(ObjectClass *klass,
void *class_data G_GNUC_UNUSED)
{
UserCreatableClass *uc_klass = USER_CREATABLE_CLASS(klass);
+ object_class_property_add_link(klass, "canbus", TYPE_CAN_BUS,
+ offsetof(CanHostState, bus),
+ object_property_allow_set_link,
+ OBJ_PROP_LINK_STRONG);
+
klass->unparent = can_host_unparent;
uc_klass->complete = can_host_complete;
}
@@ -97,7 +92,6 @@ static const TypeInfo can_host_info = {
.instance_size = sizeof(CanHostState),
.class_size = sizeof(CanHostClass),
.abstract = true,
- .instance_init = can_host_instance_init,
.class_init = can_host_class_init,
.interfaces = (InterfaceInfo[]) {
{ TYPE_USER_CREATABLE },
--
2.26.2
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 07/12] colo: Use class properties
2020-10-09 16:01 [PATCH 00/12] qom: Make all -object types use only class properties Eduardo Habkost
` (5 preceding siblings ...)
2020-10-09 16:01 ` [PATCH 06/12] can_host: " Eduardo Habkost
@ 2020-10-09 16:01 ` Eduardo Habkost
2020-10-09 16:01 ` [PATCH 08/12] netfilter: Reorder functions Eduardo Habkost
` (4 subsequent siblings)
11 siblings, 0 replies; 28+ messages in thread
From: Eduardo Habkost @ 2020-10-09 16:01 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Li Zhijian, Jason Wang, Markus Armbruster,
Zhang Chen, Marc-André Lureau, John Snow
Instance properties make introspection hard and are not shown by
"-object ...,help". Convert them to class properties.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Cc: Zhang Chen <chen.zhang@intel.com>
Cc: Li Zhijian <lizhijian@cn.fujitsu.com>
Cc: Jason Wang <jasowang@redhat.com>
Cc: qemu-devel@nongnu.org
---
net/colo-compare.c | 57 +++++++++++++++++++++++-----------------------
1 file changed, 29 insertions(+), 28 deletions(-)
diff --git a/net/colo-compare.c b/net/colo-compare.c
index 3a45d64175..017e82dd8b 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -1375,6 +1375,35 @@ static void colo_compare_class_init(ObjectClass *oc, void *data)
{
UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc);
+ object_class_property_add_str(oc, "primary_in",
+ compare_get_pri_indev, compare_set_pri_indev);
+ object_class_property_add_str(oc, "secondary_in",
+ compare_get_sec_indev, compare_set_sec_indev);
+ object_class_property_add_str(oc, "outdev",
+ compare_get_outdev, compare_set_outdev);
+ object_class_property_add_link(oc, "iothread", TYPE_IOTHREAD,
+ offsetof(CompareState, iothread),
+ object_property_allow_set_link,
+ OBJ_PROP_LINK_STRONG);
+ /* This parameter just for Xen COLO */
+ object_class_property_add_str(oc, "notify_dev",
+ compare_get_notify_dev, compare_set_notify_dev);
+
+ object_class_property_add(oc, "compare_timeout", "uint32",
+ compare_get_timeout,
+ compare_set_timeout, NULL, NULL);
+
+ object_class_property_add(oc, "expired_scan_cycle", "uint32",
+ compare_get_expired_scan_cycle,
+ compare_set_expired_scan_cycle, NULL, NULL);
+
+ object_class_property_add(oc, "max_queue_size", "uint32",
+ get_max_queue_size,
+ set_max_queue_size, NULL, NULL);
+
+ object_class_property_add_bool(oc, "vnet_hdr_support", compare_get_vnet_hdr,
+ compare_set_vnet_hdr);
+
ucc->complete = colo_compare_complete;
}
@@ -1382,35 +1411,7 @@ static void colo_compare_init(Object *obj)
{
CompareState *s = COLO_COMPARE(obj);
- object_property_add_str(obj, "primary_in",
- compare_get_pri_indev, compare_set_pri_indev);
- object_property_add_str(obj, "secondary_in",
- compare_get_sec_indev, compare_set_sec_indev);
- object_property_add_str(obj, "outdev",
- compare_get_outdev, compare_set_outdev);
- object_property_add_link(obj, "iothread", TYPE_IOTHREAD,
- (Object **)&s->iothread,
- object_property_allow_set_link,
- OBJ_PROP_LINK_STRONG);
- /* This parameter just for Xen COLO */
- object_property_add_str(obj, "notify_dev",
- compare_get_notify_dev, compare_set_notify_dev);
-
- object_property_add(obj, "compare_timeout", "uint32",
- compare_get_timeout,
- compare_set_timeout, NULL, NULL);
-
- object_property_add(obj, "expired_scan_cycle", "uint32",
- compare_get_expired_scan_cycle,
- compare_set_expired_scan_cycle, NULL, NULL);
-
- object_property_add(obj, "max_queue_size", "uint32",
- get_max_queue_size,
- set_max_queue_size, NULL, NULL);
-
s->vnet_hdr = false;
- object_property_add_bool(obj, "vnet_hdr_support", compare_get_vnet_hdr,
- compare_set_vnet_hdr);
}
static void colo_compare_finalize(Object *obj)
--
2.26.2
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 08/12] netfilter: Reorder functions
2020-10-09 16:01 [PATCH 00/12] qom: Make all -object types use only class properties Eduardo Habkost
` (6 preceding siblings ...)
2020-10-09 16:01 ` [PATCH 07/12] colo: " Eduardo Habkost
@ 2020-10-09 16:01 ` Eduardo Habkost
2020-10-09 16:01 ` [PATCH 09/12] netfilter: Use class properties Eduardo Habkost
` (3 subsequent siblings)
11 siblings, 0 replies; 28+ messages in thread
From: Eduardo Habkost @ 2020-10-09 16:01 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Li Zhijian, Jason Wang, Markus Armbruster,
Zhang Chen, Marc-André Lureau, John Snow
Trivial code reordering in some filter backends, to make the next
changes easier to review.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Cc: Jason Wang <jasowang@redhat.com>
Cc: Zhang Chen <chen.zhang@intel.com>
Cc: Li Zhijian <lizhijian@cn.fujitsu.com>
Cc: qemu-devel@nongnu.org
---
net/filter-buffer.c | 20 ++++++++++----------
net/filter-mirror.c | 36 ++++++++++++++++++------------------
2 files changed, 28 insertions(+), 28 deletions(-)
diff --git a/net/filter-buffer.c b/net/filter-buffer.c
index d8392be53c..95e384865f 100644
--- a/net/filter-buffer.c
+++ b/net/filter-buffer.c
@@ -144,16 +144,6 @@ static void filter_buffer_status_changed(NetFilterState *nf, Error **errp)
}
}
-static void filter_buffer_class_init(ObjectClass *oc, void *data)
-{
- NetFilterClass *nfc = NETFILTER_CLASS(oc);
-
- nfc->setup = filter_buffer_setup;
- nfc->cleanup = filter_buffer_cleanup;
- nfc->receive_iov = filter_buffer_receive_iov;
- nfc->status_changed = filter_buffer_status_changed;
-}
-
static void filter_buffer_get_interval(Object *obj, Visitor *v,
const char *name, void *opaque,
Error **errp)
@@ -182,6 +172,16 @@ static void filter_buffer_set_interval(Object *obj, Visitor *v,
s->interval = value;
}
+static void filter_buffer_class_init(ObjectClass *oc, void *data)
+{
+ NetFilterClass *nfc = NETFILTER_CLASS(oc);
+
+ nfc->setup = filter_buffer_setup;
+ nfc->cleanup = filter_buffer_cleanup;
+ nfc->receive_iov = filter_buffer_receive_iov;
+ nfc->status_changed = filter_buffer_status_changed;
+}
+
static void filter_buffer_init(Object *obj)
{
object_property_add(obj, "interval", "uint32",
diff --git a/net/filter-mirror.c b/net/filter-mirror.c
index 088d4dcace..26b783011a 100644
--- a/net/filter-mirror.c
+++ b/net/filter-mirror.c
@@ -284,24 +284,6 @@ static void filter_redirector_setup(NetFilterState *nf, Error **errp)
}
}
-static void filter_mirror_class_init(ObjectClass *oc, void *data)
-{
- NetFilterClass *nfc = NETFILTER_CLASS(oc);
-
- nfc->setup = filter_mirror_setup;
- nfc->cleanup = filter_mirror_cleanup;
- nfc->receive_iov = filter_mirror_receive_iov;
-}
-
-static void filter_redirector_class_init(ObjectClass *oc, void *data)
-{
- NetFilterClass *nfc = NETFILTER_CLASS(oc);
-
- nfc->setup = filter_redirector_setup;
- nfc->cleanup = filter_redirector_cleanup;
- nfc->receive_iov = filter_redirector_receive_iov;
-}
-
static char *filter_redirector_get_indev(Object *obj, Error **errp)
{
MirrorState *s = FILTER_REDIRECTOR(obj);
@@ -388,6 +370,24 @@ static void filter_redirector_set_vnet_hdr(Object *obj,
s->vnet_hdr = value;
}
+static void filter_mirror_class_init(ObjectClass *oc, void *data)
+{
+ NetFilterClass *nfc = NETFILTER_CLASS(oc);
+
+ nfc->setup = filter_mirror_setup;
+ nfc->cleanup = filter_mirror_cleanup;
+ nfc->receive_iov = filter_mirror_receive_iov;
+}
+
+static void filter_redirector_class_init(ObjectClass *oc, void *data)
+{
+ NetFilterClass *nfc = NETFILTER_CLASS(oc);
+
+ nfc->setup = filter_redirector_setup;
+ nfc->cleanup = filter_redirector_cleanup;
+ nfc->receive_iov = filter_redirector_receive_iov;
+}
+
static void filter_mirror_init(Object *obj)
{
MirrorState *s = FILTER_MIRROR(obj);
--
2.26.2
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 09/12] netfilter: Use class properties
2020-10-09 16:01 [PATCH 00/12] qom: Make all -object types use only class properties Eduardo Habkost
` (7 preceding siblings ...)
2020-10-09 16:01 ` [PATCH 08/12] netfilter: Reorder functions Eduardo Habkost
@ 2020-10-09 16:01 ` Eduardo Habkost
2020-10-09 16:01 ` [PATCH 10/12] input: " Eduardo Habkost
` (2 subsequent siblings)
11 siblings, 0 replies; 28+ messages in thread
From: Eduardo Habkost @ 2020-10-09 16:01 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Li Zhijian, Jason Wang, Markus Armbruster,
Zhang Chen, Marc-André Lureau, John Snow
Instance properties make introspection hard and are not shown by
"-object ...,help". Convert them to class properties.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Cc: Jason Wang <jasowang@redhat.com>
Cc: Zhang Chen <chen.zhang@intel.com>
Cc: Li Zhijian <lizhijian@cn.fujitsu.com>
Cc: qemu-devel@nongnu.org
---
net/dump.c | 10 +++++-----
net/filter-buffer.c | 12 ++++--------
net/filter-mirror.c | 28 ++++++++++++++--------------
net/filter-rewriter.c | 7 ++++---
net/filter.c | 24 ++++++++++++------------
5 files changed, 39 insertions(+), 42 deletions(-)
diff --git a/net/dump.c b/net/dump.c
index 7fd448d2e1..4d538d82a6 100644
--- a/net/dump.c
+++ b/net/dump.c
@@ -224,11 +224,6 @@ static void filter_dump_instance_init(Object *obj)
NetFilterDumpState *nfds = FILTER_DUMP(obj);
nfds->maxlen = 65536;
-
- object_property_add(obj, "maxlen", "uint32", filter_dump_get_maxlen,
- filter_dump_set_maxlen, NULL, NULL);
- object_property_add_str(obj, "file", file_dump_get_filename,
- file_dump_set_filename);
}
static void filter_dump_instance_finalize(Object *obj)
@@ -242,6 +237,11 @@ static void filter_dump_class_init(ObjectClass *oc, void *data)
{
NetFilterClass *nfc = NETFILTER_CLASS(oc);
+ object_class_property_add(oc, "maxlen", "uint32", filter_dump_get_maxlen,
+ filter_dump_set_maxlen, NULL, NULL);
+ object_class_property_add_str(oc, "file", file_dump_get_filename,
+ file_dump_set_filename);
+
nfc->setup = filter_dump_setup;
nfc->cleanup = filter_dump_cleanup;
nfc->receive_iov = filter_dump_receive_iov;
diff --git a/net/filter-buffer.c b/net/filter-buffer.c
index 95e384865f..283dc9cbe6 100644
--- a/net/filter-buffer.c
+++ b/net/filter-buffer.c
@@ -176,24 +176,20 @@ static void filter_buffer_class_init(ObjectClass *oc, void *data)
{
NetFilterClass *nfc = NETFILTER_CLASS(oc);
+ object_class_property_add(oc, "interval", "uint32",
+ filter_buffer_get_interval,
+ filter_buffer_set_interval, NULL, NULL);
+
nfc->setup = filter_buffer_setup;
nfc->cleanup = filter_buffer_cleanup;
nfc->receive_iov = filter_buffer_receive_iov;
nfc->status_changed = filter_buffer_status_changed;
}
-static void filter_buffer_init(Object *obj)
-{
- object_property_add(obj, "interval", "uint32",
- filter_buffer_get_interval,
- filter_buffer_set_interval, NULL, NULL);
-}
-
static const TypeInfo filter_buffer_info = {
.name = TYPE_FILTER_BUFFER,
.parent = TYPE_NETFILTER,
.class_init = filter_buffer_class_init,
- .instance_init = filter_buffer_init,
.instance_size = sizeof(FilterBufferState),
};
diff --git a/net/filter-mirror.c b/net/filter-mirror.c
index 26b783011a..f8e65007c0 100644
--- a/net/filter-mirror.c
+++ b/net/filter-mirror.c
@@ -374,6 +374,12 @@ static void filter_mirror_class_init(ObjectClass *oc, void *data)
{
NetFilterClass *nfc = NETFILTER_CLASS(oc);
+ object_class_property_add_str(oc, "outdev", filter_mirror_get_outdev,
+ filter_mirror_set_outdev);
+ object_class_property_add_bool(oc, "vnet_hdr_support",
+ filter_mirror_get_vnet_hdr,
+ filter_mirror_set_vnet_hdr);
+
nfc->setup = filter_mirror_setup;
nfc->cleanup = filter_mirror_cleanup;
nfc->receive_iov = filter_mirror_receive_iov;
@@ -383,6 +389,14 @@ static void filter_redirector_class_init(ObjectClass *oc, void *data)
{
NetFilterClass *nfc = NETFILTER_CLASS(oc);
+ object_class_property_add_str(oc, "indev", filter_redirector_get_indev,
+ filter_redirector_set_indev);
+ object_class_property_add_str(oc, "outdev", filter_redirector_get_outdev,
+ filter_redirector_set_outdev);
+ object_class_property_add_bool(oc, "vnet_hdr_support",
+ filter_redirector_get_vnet_hdr,
+ filter_redirector_set_vnet_hdr);
+
nfc->setup = filter_redirector_setup;
nfc->cleanup = filter_redirector_cleanup;
nfc->receive_iov = filter_redirector_receive_iov;
@@ -392,28 +406,14 @@ static void filter_mirror_init(Object *obj)
{
MirrorState *s = FILTER_MIRROR(obj);
- object_property_add_str(obj, "outdev", filter_mirror_get_outdev,
- filter_mirror_set_outdev);
-
s->vnet_hdr = false;
- object_property_add_bool(obj, "vnet_hdr_support",
- filter_mirror_get_vnet_hdr,
- filter_mirror_set_vnet_hdr);
}
static void filter_redirector_init(Object *obj)
{
MirrorState *s = FILTER_REDIRECTOR(obj);
- object_property_add_str(obj, "indev", filter_redirector_get_indev,
- filter_redirector_set_indev);
- object_property_add_str(obj, "outdev", filter_redirector_get_outdev,
- filter_redirector_set_outdev);
-
s->vnet_hdr = false;
- object_property_add_bool(obj, "vnet_hdr_support",
- filter_redirector_get_vnet_hdr,
- filter_redirector_set_vnet_hdr);
}
static void filter_mirror_fini(Object *obj)
diff --git a/net/filter-rewriter.c b/net/filter-rewriter.c
index dc3c27a489..ae358059d9 100644
--- a/net/filter-rewriter.c
+++ b/net/filter-rewriter.c
@@ -416,15 +416,16 @@ static void filter_rewriter_init(Object *obj)
s->vnet_hdr = false;
s->failover_mode = FAILOVER_MODE_OFF;
- object_property_add_bool(obj, "vnet_hdr_support",
- filter_rewriter_get_vnet_hdr,
- filter_rewriter_set_vnet_hdr);
}
static void colo_rewriter_class_init(ObjectClass *oc, void *data)
{
NetFilterClass *nfc = NETFILTER_CLASS(oc);
+ object_class_property_add_bool(oc, "vnet_hdr_support",
+ filter_rewriter_get_vnet_hdr,
+ filter_rewriter_set_vnet_hdr);
+
nfc->setup = colo_rewriter_setup;
nfc->cleanup = colo_rewriter_cleanup;
nfc->receive_iov = colo_rewriter_receive_iov;
diff --git a/net/filter.c b/net/filter.c
index eac8ba1e9c..3fe88fa43f 100644
--- a/net/filter.c
+++ b/net/filter.c
@@ -212,18 +212,6 @@ static void netfilter_init(Object *obj)
nf->on = true;
nf->insert_before_flag = false;
nf->position = g_strdup("tail");
-
- object_property_add_str(obj, "netdev",
- netfilter_get_netdev_id, netfilter_set_netdev_id);
- object_property_add_enum(obj, "queue", "NetFilterDirection",
- &NetFilterDirection_lookup,
- netfilter_get_direction, netfilter_set_direction);
- object_property_add_str(obj, "status",
- netfilter_get_status, netfilter_set_status);
- object_property_add_str(obj, "position",
- netfilter_get_position, netfilter_set_position);
- object_property_add_str(obj, "insert",
- netfilter_get_insert, netfilter_set_insert);
}
static void netfilter_complete(UserCreatable *uc, Error **errp)
@@ -350,6 +338,18 @@ static void netfilter_class_init(ObjectClass *oc, void *data)
UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc);
NetFilterClass *nfc = NETFILTER_CLASS(oc);
+ object_class_property_add_str(oc, "netdev",
+ netfilter_get_netdev_id, netfilter_set_netdev_id);
+ object_class_property_add_enum(oc, "queue", "NetFilterDirection",
+ &NetFilterDirection_lookup,
+ netfilter_get_direction, netfilter_set_direction);
+ object_class_property_add_str(oc, "status",
+ netfilter_get_status, netfilter_set_status);
+ object_class_property_add_str(oc, "position",
+ netfilter_get_position, netfilter_set_position);
+ object_class_property_add_str(oc, "insert",
+ netfilter_get_insert, netfilter_set_insert);
+
ucc->complete = netfilter_complete;
nfc->handle_event = default_handle_event;
}
--
2.26.2
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 10/12] input: Use class properties
2020-10-09 16:01 [PATCH 00/12] qom: Make all -object types use only class properties Eduardo Habkost
` (8 preceding siblings ...)
2020-10-09 16:01 ` [PATCH 09/12] netfilter: Use class properties Eduardo Habkost
@ 2020-10-09 16:01 ` Eduardo Habkost
2020-10-13 12:54 ` Gerd Hoffmann
2020-10-09 16:01 ` [PATCH 11/12] [RFC] qom: Property lock mechanism Eduardo Habkost
2020-10-09 16:01 ` [PATCH 12/12] [RFC] qom: Lock properties of all TYPE_USER_CREATABLE types Eduardo Habkost
11 siblings, 1 reply; 28+ messages in thread
From: Eduardo Habkost @ 2020-10-09 16:01 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Marc-André Lureau, John Snow, Markus Armbruster,
Gerd Hoffmann
Instance properties make introspection hard and are not shown by
"-object ...,help". Convert them to class properties.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: qemu-devel@nongnu.org
---
ui/input-barrier.c | 44 ++++++++++++++++++++++----------------------
ui/input-linux.c | 27 ++++++++++++++-------------
2 files changed, 36 insertions(+), 35 deletions(-)
diff --git a/ui/input-barrier.c b/ui/input-barrier.c
index a047919fde..8e886a6495 100644
--- a/ui/input-barrier.c
+++ b/ui/input-barrier.c
@@ -689,34 +689,34 @@ static void input_barrier_instance_init(Object *obj)
ib->y_origin = 0;
ib->width = 1920;
ib->height = 1080;
-
- object_property_add_str(obj, "name",
- input_barrier_get_name,
- input_barrier_set_name);
- object_property_add_str(obj, "server",
- input_barrier_get_server,
- input_barrier_set_server);
- object_property_add_str(obj, "port",
- input_barrier_get_port,
- input_barrier_set_port);
- object_property_add_str(obj, "x-origin",
- input_barrier_get_x_origin,
- input_barrier_set_x_origin);
- object_property_add_str(obj, "y-origin",
- input_barrier_get_y_origin,
- input_barrier_set_y_origin);
- object_property_add_str(obj, "width",
- input_barrier_get_width,
- input_barrier_set_width);
- object_property_add_str(obj, "height",
- input_barrier_get_height,
- input_barrier_set_height);
}
static void input_barrier_class_init(ObjectClass *oc, void *data)
{
UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc);
+ object_class_property_add_str(oc, "name",
+ input_barrier_get_name,
+ input_barrier_set_name);
+ object_class_property_add_str(oc, "server",
+ input_barrier_get_server,
+ input_barrier_set_server);
+ object_class_property_add_str(oc, "port",
+ input_barrier_get_port,
+ input_barrier_set_port);
+ object_class_property_add_str(oc, "x-origin",
+ input_barrier_get_x_origin,
+ input_barrier_set_x_origin);
+ object_class_property_add_str(oc, "y-origin",
+ input_barrier_get_y_origin,
+ input_barrier_set_y_origin);
+ object_class_property_add_str(oc, "width",
+ input_barrier_get_width,
+ input_barrier_set_width);
+ object_class_property_add_str(oc, "height",
+ input_barrier_get_height,
+ input_barrier_set_height);
+
ucc->complete = input_barrier_complete;
}
diff --git a/ui/input-linux.c b/ui/input-linux.c
index ab351a4187..df3dff8898 100644
--- a/ui/input-linux.c
+++ b/ui/input-linux.c
@@ -489,25 +489,26 @@ static void input_linux_set_grab_toggle(Object *obj, int value,
static void input_linux_instance_init(Object *obj)
{
- object_property_add_str(obj, "evdev",
- input_linux_get_evdev,
- input_linux_set_evdev);
- object_property_add_bool(obj, "grab_all",
- input_linux_get_grab_all,
- input_linux_set_grab_all);
- object_property_add_bool(obj, "repeat",
- input_linux_get_repeat,
- input_linux_set_repeat);
- object_property_add_enum(obj, "grab-toggle", "GrabToggleKeys",
- &GrabToggleKeys_lookup,
- input_linux_get_grab_toggle,
- input_linux_set_grab_toggle);
}
static void input_linux_class_init(ObjectClass *oc, void *data)
{
UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc);
+ object_class_property_add_str(oc, "evdev",
+ input_linux_get_evdev,
+ input_linux_set_evdev);
+ object_class_property_add_bool(oc, "grab_all",
+ input_linux_get_grab_all,
+ input_linux_set_grab_all);
+ object_class_property_add_bool(oc, "repeat",
+ input_linux_get_repeat,
+ input_linux_set_repeat);
+ object_class_property_add_enum(oc, "grab-toggle", "GrabToggleKeys",
+ &GrabToggleKeys_lookup,
+ input_linux_get_grab_toggle,
+ input_linux_set_grab_toggle);
+
ucc->complete = input_linux_complete;
}
--
2.26.2
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 11/12] [RFC] qom: Property lock mechanism
2020-10-09 16:01 [PATCH 00/12] qom: Make all -object types use only class properties Eduardo Habkost
` (9 preceding siblings ...)
2020-10-09 16:01 ` [PATCH 10/12] input: " Eduardo Habkost
@ 2020-10-09 16:01 ` Eduardo Habkost
2020-10-09 16:01 ` [PATCH 12/12] [RFC] qom: Lock properties of all TYPE_USER_CREATABLE types Eduardo Habkost
11 siblings, 0 replies; 28+ messages in thread
From: Eduardo Habkost @ 2020-10-09 16:01 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Daniel P. Berrangé,
Markus Armbruster, Paolo Bonzini, Marc-André Lureau,
John Snow
Add a mechanism to allow QOM types to prevent writable instance
properties from being registered. This will be used by types
that expose all QOM properties in user-visible interfaces like
object-add and device_add, to ensure our external interfaces are
not affected by dynamic QOM properties.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: "Daniel P. Berrangé" <berrange@redhat.com>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: qemu-devel@nongnu.org
---
include/qom/object.h | 17 +++++++++
qom/object.c | 28 ++++++++++++++
tests/test-qdev-global-props.c | 70 ++++++++++++++++++++++++++++++++++
3 files changed, 115 insertions(+)
diff --git a/include/qom/object.h b/include/qom/object.h
index 1634294e4f..a124cf897d 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -137,6 +137,8 @@ struct ObjectClass
ObjectUnparent *unparent;
GHashTable *properties;
+ /* instance properties locked. See object_class_lock_properties() */
+ bool properties_locked;
};
/**
@@ -1867,6 +1869,21 @@ void object_property_set_description(Object *obj, const char *name,
void object_class_property_set_description(ObjectClass *klass, const char *name,
const char *description);
+/**
+ * object_class_lock_properties:
+ * @oc: the object class to have properties locked
+ *
+ * Prevent all subtypes of @oc from having writeable instance
+ * properties. If @oc is an interface type, this also affects all
+ * classes implementing the interface.
+ *
+ * This can be used by QOM types that have all QOM properties
+ * exposed to the external world (e.g. #TYPE_USER_CREATABLE) to
+ * ensure all user-writable properties are introspectable at the
+ * class level.
+ */
+void object_class_lock_properties(ObjectClass *oc);
+
/**
* object_child_foreach:
* @obj: the object whose children will be navigated
diff --git a/qom/object.c b/qom/object.c
index bb32f5d3ad..73f27b8b7e 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -498,6 +498,27 @@ static void object_class_property_init_all(Object *obj)
}
}
+void object_class_lock_properties(ObjectClass *oc)
+{
+ oc->properties_locked = true;
+}
+
+static bool object_class_properties_locked(ObjectClass *oc)
+{
+ GSList *i = NULL;
+
+ if (oc->properties_locked) {
+ return true;
+ }
+ for (i = oc->interfaces; i; i = i->next) {
+ ObjectClass *ic = i->data;
+ if (ic->properties_locked) {
+ return true;
+ }
+ }
+ return false;
+}
+
static void object_initialize_with_type(Object *obj, size_t size, TypeImpl *type)
{
type_initialize(type);
@@ -1192,8 +1213,15 @@ object_property_try_add(Object *obj, const char *name, const char *type,
void *opaque, Error **errp)
{
ObjectProperty *prop;
+ ObjectClass *oc = object_get_class(obj);
size_t name_len = strlen(name);
+ if (set && object_class_properties_locked(oc)) {
+ error_setg(errp, "writable instance property not allowed for type %s",
+ object_class_get_name(oc));
+ return NULL;
+ }
+
if (name_len >= 3 && !memcmp(name + name_len - 3, "[*]", 4)) {
int i;
ObjectProperty *ret;
diff --git a/tests/test-qdev-global-props.c b/tests/test-qdev-global-props.c
index c8862cac5f..590c916c4b 100644
--- a/tests/test-qdev-global-props.c
+++ b/tests/test-qdev-global-props.c
@@ -58,6 +58,9 @@ static void static_prop_class_init(ObjectClass *oc, void *data)
dc->realize = NULL;
device_class_set_props(dc, static_props);
+
+ /* test_proplist_lock() will check if property locking works */
+ object_class_lock_properties(oc);
}
static const TypeInfo static_prop_type = {
@@ -213,6 +216,69 @@ static const TypeInfo nondevice_type = {
.parent = TYPE_OBJECT,
};
+static void locked_interface_class_base_init(ObjectClass *klass, void *data)
+{
+ object_class_lock_properties(klass);
+}
+
+#define TYPE_LOCKED_INTERFACE "locked-interface"
+static const TypeInfo locked_interface_type = {
+ .name = TYPE_LOCKED_INTERFACE,
+ .parent = TYPE_INTERFACE,
+ .class_base_init = locked_interface_class_base_init,
+};
+
+#define TYPE_LOCKED_BY_INTERFACE "locked-by-interface"
+static const TypeInfo locked_by_interface_type = {
+ .name = TYPE_LOCKED_BY_INTERFACE,
+ .parent = TYPE_OBJECT,
+ .interfaces = (InterfaceInfo[]) {
+ { TYPE_LOCKED_INTERFACE },
+ { },
+ },
+};
+
+/* Make sure QOM property locking works as expected */
+static void test_proplist_lock(void)
+{
+ g_autoptr(Object) dynamic_obj = object_new(TYPE_DYNAMIC_PROPS);
+ g_autoptr(Object) static_obj = object_new(TYPE_STATIC_PROPS);
+ g_autoptr(Object) locked = object_new(TYPE_LOCKED_BY_INTERFACE);
+ Error *err = NULL;
+
+ /* read-only property: should always work */
+ object_property_try_add(dynamic_obj, "dynamic-prop-ro", "uint32",
+ prop1_accessor, NULL,
+ NULL, NULL, &error_abort);
+ object_property_try_add(static_obj, "dynamic-prop-ro", "uint32",
+ prop1_accessor, NULL,
+ NULL, NULL, &error_abort);
+ object_property_try_add(locked, "dynamic-prop-ro", "uint32",
+ prop1_accessor, NULL,
+ NULL, NULL, &error_abort);
+
+
+ /* read-write property: */
+
+ /* TYPE_DYNAMIC_PROPS is not locked */
+ object_property_try_add(dynamic_obj, "dynamic-prop-rw", "uint32",
+ prop1_accessor, prop1_accessor,
+ NULL, NULL, &error_abort);
+
+ /* TYPE_STATIC_PROPS is locked */
+ object_property_try_add(static_obj, "dynamic-prop-rw", "uint32",
+ prop1_accessor, prop1_accessor,
+ NULL, NULL, &err);
+ error_free_or_abort(&err);
+
+ /* TYPE_LOCKED_BY_INTERFACE is locked by interface type */
+ object_property_try_add(locked, "dynamic-prop-rw", "uint32",
+ prop1_accessor, prop1_accessor,
+ NULL, NULL, &err);
+ error_free_or_abort(&err);
+}
+
+
/* Test setting of dynamic properties using global properties */
static void test_dynamic_globalprop_subprocess(void)
{
@@ -294,6 +360,10 @@ int main(int argc, char **argv)
type_register_static(&hotplug_type);
type_register_static(&nohotplug_type);
type_register_static(&nondevice_type);
+ type_register_static(&locked_interface_type);
+ type_register_static(&locked_by_interface_type);
+
+ g_test_add_func("/qdev/properties/locking", test_proplist_lock);
g_test_add_func("/qdev/properties/static/default/subprocess",
test_static_prop_subprocess);
--
2.26.2
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 12/12] [RFC] qom: Lock properties of all TYPE_USER_CREATABLE types
2020-10-09 16:01 [PATCH 00/12] qom: Make all -object types use only class properties Eduardo Habkost
` (10 preceding siblings ...)
2020-10-09 16:01 ` [PATCH 11/12] [RFC] qom: Property lock mechanism Eduardo Habkost
@ 2020-10-09 16:01 ` Eduardo Habkost
2020-10-09 21:31 ` [PATCH] check-qom-proplist: Don't register instance props for user-creatable type Eduardo Habkost
11 siblings, 1 reply; 28+ messages in thread
From: Eduardo Habkost @ 2020-10-09 16:01 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Daniel P. Berrangé,
Markus Armbruster, Paolo Bonzini, Marc-André Lureau,
John Snow
TYPE_USER_CREATABLE types expose all QOM properties through
`-object` and `object-add`. Lock the properties on
TYPE_USER_CREATABLE so we will never add new writable instance
properties to those types.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: "Daniel P. Berrangé" <berrange@redhat.com>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: qemu-devel@nongnu.org
---
qom/object_interfaces.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
index e8e1523960..40123d4b50 100644
--- a/qom/object_interfaces.c
+++ b/qom/object_interfaces.c
@@ -293,6 +293,16 @@ bool user_creatable_del(const char *id, Error **errp)
return true;
}
+static void user_creatable_class_init(ObjectClass *klass, void *data)
+{
+ /*
+ * User-creatable QOM types expose all writable QOM properties
+ * to the external world through `-object` and `object-add`,
+ * so all writable properties must be registered at class level.
+ */
+ object_class_lock_properties(klass);
+}
+
void user_creatable_cleanup(void)
{
object_unparent(object_get_objects_root());
@@ -304,6 +314,7 @@ static void register_types(void)
.name = TYPE_USER_CREATABLE,
.parent = TYPE_INTERFACE,
.class_size = sizeof(UserCreatableClass),
+ .class_init = user_creatable_class_init,
};
type_register_static(&uc_interface_info);
--
2.26.2
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH 03/12] qom: Make object_class_property_add_uint*_ptr() get offset
2020-10-09 16:01 ` [PATCH 03/12] qom: Make object_class_property_add_uint*_ptr() get offset Eduardo Habkost
@ 2020-10-09 17:24 ` Eric Blake
2020-10-09 17:31 ` Eduardo Habkost
2020-10-21 12:24 ` Igor Mammedov
1 sibling, 1 reply; 28+ messages in thread
From: Eric Blake @ 2020-10-09 17:24 UTC (permalink / raw)
To: Eduardo Habkost, qemu-devel
Cc: Kevin Wolf, Daniel P. Berrangé,
Markus Armbruster, Marc-André Lureau, Paolo Bonzini,
John Snow
[-- Attachment #1.1: Type: text/plain, Size: 1707 bytes --]
On 10/9/20 11:01 AM, Eduardo Habkost wrote:
> The existing object_class_property_add_uint*_ptr() functions are
> not very useful, because they need a pointer to the property
> value, which can't really be provided before the object is
> created.
>
> Replace the pointer parameter in those functions with a
> `ptrdiff_t offset` parameter.
>
> Include a uint8 class property in check-qom-proplist unit tests,
> to ensure the feature is working.
>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> static void *pointer_property_get_ptr(Object *obj, PointerProperty *prop)
> {
> - return prop->ptr;
> + if (prop->is_offset) {
> + return (void *)obj + prop->offset;
Addition on void* is a gcc extension. Does clang support it as well, or
should you be casting to char* instead?
> + } else {
> + return prop->ptr;
> + }
> }
>
> +++ b/tests/check-qom-proplist.c
> @@ -61,6 +61,7 @@ struct DummyObject {
> bool bv;
> DummyAnimal av;
> char *sv;
> + uint8_t u8v;
> };
>
> struct DummyObjectClass {
> @@ -141,6 +142,9 @@ static void dummy_class_init(ObjectClass *cls, void *data)
> &dummy_animal_map,
> dummy_get_av,
> dummy_set_av);
> + object_class_property_add_uint8_ptr(cls, "u8v",
> + offsetof(DummyObject, u8v),
> + OBJ_PROP_FLAG_READWRITE);
I like how it turns out.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 03/12] qom: Make object_class_property_add_uint*_ptr() get offset
2020-10-09 17:24 ` Eric Blake
@ 2020-10-09 17:31 ` Eduardo Habkost
0 siblings, 0 replies; 28+ messages in thread
From: Eduardo Habkost @ 2020-10-09 17:31 UTC (permalink / raw)
To: Eric Blake
Cc: Kevin Wolf, Daniel P. Berrangé,
qemu-devel, Markus Armbruster, Marc-André Lureau,
Paolo Bonzini, John Snow
On Fri, Oct 09, 2020 at 12:24:19PM -0500, Eric Blake wrote:
> On 10/9/20 11:01 AM, Eduardo Habkost wrote:
> > The existing object_class_property_add_uint*_ptr() functions are
> > not very useful, because they need a pointer to the property
> > value, which can't really be provided before the object is
> > created.
> >
> > Replace the pointer parameter in those functions with a
> > `ptrdiff_t offset` parameter.
> >
> > Include a uint8 class property in check-qom-proplist unit tests,
> > to ensure the feature is working.
> >
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > ---
>
> > static void *pointer_property_get_ptr(Object *obj, PointerProperty *prop)
> > {
> > - return prop->ptr;
> > + if (prop->is_offset) {
> > + return (void *)obj + prop->offset;
>
> Addition on void* is a gcc extension. Does clang support it as well, or
> should you be casting to char* instead?
Both object_link_get_targetp() and object_link_get_targetp()
already use it, so it should be OK.
--
Eduardo
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH] check-qom-proplist: Don't register instance props for user-creatable type
2020-10-09 16:01 ` [PATCH 12/12] [RFC] qom: Lock properties of all TYPE_USER_CREATABLE types Eduardo Habkost
@ 2020-10-09 21:31 ` Eduardo Habkost
0 siblings, 0 replies; 28+ messages in thread
From: Eduardo Habkost @ 2020-10-09 21:31 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Daniel P. Berrangé,
Markus Armbruster, Paolo Bonzini, Marc-André Lureau,
John Snow
user-creatable types will now be forbidden from registering
instance properties, but check-qom-proplist reuse the same type
for testing user_creatable_add_opts() and for testing
class/instance property enumeration.
To address those conflicting requirements, add two subclasses of
TYPE_DUMMY: one that's user-creatable and another one that has
instance properties. Most test that set the "bv" property will
use the new TYPE_DUMMY_WITH_INSTANCE_PROPS type, but
test_dummy_createcmdl() will now TYPE_DUMMY_USER_CREATABLE.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
The patch
[RFC] qom: Lock properties of all TYPE_USER_CREATABLE types
will break check-qom-proplist, unless this patch is applied
first.
---
tests/check-qom-proplist.c | 59 +++++++++++++++++++++++++++-----------
1 file changed, 43 insertions(+), 16 deletions(-)
diff --git a/tests/check-qom-proplist.c b/tests/check-qom-proplist.c
index fba30c20b2..e9d0eec0c2 100644
--- a/tests/check-qom-proplist.c
+++ b/tests/check-qom-proplist.c
@@ -124,14 +124,6 @@ static char *dummy_get_sv(Object *obj,
}
-static void dummy_init(Object *obj)
-{
- object_property_add_bool(obj, "bv",
- dummy_get_bv,
- dummy_set_bv);
-}
-
-
static void dummy_class_init(ObjectClass *cls, void *data)
{
object_class_property_add_str(cls, "sv",
@@ -155,15 +147,48 @@ static void dummy_finalize(Object *obj)
g_free(dobj->sv);
}
-
static const TypeInfo dummy_info = {
.name = TYPE_DUMMY,
.parent = TYPE_OBJECT,
.instance_size = sizeof(DummyObject),
- .instance_init = dummy_init,
.instance_finalize = dummy_finalize,
.class_size = sizeof(DummyObjectClass),
.class_init = dummy_class_init,
+};
+
+static void dummy_with_instance_props_init(Object *obj)
+{
+ object_property_add_bool(obj, "bv",
+ dummy_get_bv,
+ dummy_set_bv);
+}
+
+/* Subclass of TYPE_DUMMY, but with a instance-level "bv" property */
+#define TYPE_DUMMY_WITH_INSTANCE_PROPS "qemu-dummy-with-intance-props"
+
+static const TypeInfo dummy_with_instance_props_info = {
+ .name = TYPE_DUMMY_WITH_INSTANCE_PROPS,
+ .parent = TYPE_DUMMY,
+ .instance_init = dummy_with_instance_props_init,
+};
+
+static void dummy_user_creatable_class_init(ObjectClass *cls, void *data)
+{
+ object_class_property_add_bool(cls, "bv",
+ dummy_get_bv,
+ dummy_set_bv);
+}
+
+/*
+ * Subclass of TYPE_DUMMY, but user-creatable and with a class-level
+ * "bv" property
+ */
+#define TYPE_DUMMY_USER_CREATABLE "qemu-dummy-user-creatable"
+
+static const TypeInfo dummy_user_creatable_info = {
+ .name = TYPE_DUMMY_USER_CREATABLE,
+ .parent = TYPE_DUMMY,
+ .class_init = dummy_user_creatable_class_init,
.interfaces = (InterfaceInfo[]) {
{ TYPE_USER_CREATABLE },
{ }
@@ -341,7 +366,7 @@ static void test_dummy_createv(void)
Error *err = NULL;
Object *parent = object_get_objects_root();
DummyObject *dobj = DUMMY_OBJECT(
- object_new_with_props(TYPE_DUMMY,
+ object_new_with_props(TYPE_DUMMY_WITH_INSTANCE_PROPS,
parent,
"dummy0",
&err,
@@ -370,7 +395,7 @@ static Object *new_helper(Error **errp,
Object *obj;
va_start(vargs, parent);
- obj = object_new_with_propv(TYPE_DUMMY,
+ obj = object_new_with_propv(TYPE_DUMMY_WITH_INSTANCE_PROPS,
parent,
"dummy0",
errp,
@@ -409,7 +434,7 @@ static void test_dummy_createcmdl(void)
QemuOpts *opts;
DummyObject *dobj;
Error *err = NULL;
- const char *params = TYPE_DUMMY \
+ const char *params = TYPE_DUMMY_USER_CREATABLE \
",id=dev0," \
"bv=yes,sv=Hiss hiss hiss,av=platypus";
@@ -449,7 +474,7 @@ static void test_dummy_badenum(void)
Error *err = NULL;
Object *parent = object_get_objects_root();
Object *dobj =
- object_new_with_props(TYPE_DUMMY,
+ object_new_with_props(TYPE_DUMMY_WITH_INSTANCE_PROPS,
parent,
"dummy0",
&err,
@@ -541,7 +566,7 @@ static void test_dummy_iterator(void)
"bv"}; /* instance property */
Object *parent = object_get_objects_root();
DummyObject *dobj = DUMMY_OBJECT(
- object_new_with_props(TYPE_DUMMY,
+ object_new_with_props(TYPE_DUMMY_WITH_INSTANCE_PROPS,
parent,
"dummy0",
&error_abort,
@@ -560,7 +585,7 @@ static void test_dummy_class_iterator(void)
{
const char *expected[] = { "type", "av", "sv", "u8v" };
ObjectPropertyIterator iter;
- ObjectClass *klass = object_class_by_name(TYPE_DUMMY);
+ ObjectClass *klass = object_class_by_name(TYPE_DUMMY_WITH_INSTANCE_PROPS);
object_class_property_iter_init(&iter, klass);
test_dummy_prop_iterator(&iter, expected, ARRAY_SIZE(expected));
@@ -626,6 +651,8 @@ int main(int argc, char **argv)
module_call_init(MODULE_INIT_QOM);
type_register_static(&dummy_info);
+ type_register_static(&dummy_with_instance_props_info);
+ type_register_static(&dummy_user_creatable_info);
type_register_static(&dummy_dev_info);
type_register_static(&dummy_bus_info);
type_register_static(&dummy_backend_info);
--
2.26.2
--
Eduardo
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH 06/12] can_host: Use class properties
2020-10-09 16:01 ` [PATCH 06/12] can_host: " Eduardo Habkost
@ 2020-10-12 14:52 ` Pavel Pisa
0 siblings, 0 replies; 28+ messages in thread
From: Pavel Pisa @ 2020-10-12 14:52 UTC (permalink / raw)
To: Eduardo Habkost
Cc: Kevin Wolf, Vikram Garhwal, Jason Wang, qemu-devel,
Markus Armbruster, Marc-André Lureau, John Snow
Hello Eduardo,
On Friday 09 of October 2020 18:01:16 Eduardo Habkost wrote:
> Instance properties make introspection hard and are not shown by
> "-object ...,help". Convert them to class properties.
>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> Cc: Pavel Pisa <pisa@cmp.felk.cvut.cz>
> Cc: Vikram Garhwal <fnu.vikram@xilinx.com>
> Cc: Jason Wang <jasowang@redhat.com>
> Cc: qemu-devel@nongnu.org
> ---
> net/can/can_host.c | 16 +++++-----------
> 1 file changed, 5 insertions(+), 11 deletions(-)
>
You can add my
Tested-by: Pavel Pisa <pisa@cmp.felk.cvut.cz>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 10/12] input: Use class properties
2020-10-09 16:01 ` [PATCH 10/12] input: " Eduardo Habkost
@ 2020-10-13 12:54 ` Gerd Hoffmann
0 siblings, 0 replies; 28+ messages in thread
From: Gerd Hoffmann @ 2020-10-13 12:54 UTC (permalink / raw)
To: Eduardo Habkost
Cc: Kevin Wolf, Marc-André Lureau, John Snow, qemu-devel,
Markus Armbruster
On Fri, Oct 09, 2020 at 12:01:20PM -0400, Eduardo Habkost wrote:
> Instance properties make introspection hard and are not shown by
> "-object ...,help". Convert them to class properties.
>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 03/12] qom: Make object_class_property_add_uint*_ptr() get offset
2020-10-09 16:01 ` [PATCH 03/12] qom: Make object_class_property_add_uint*_ptr() get offset Eduardo Habkost
2020-10-09 17:24 ` Eric Blake
@ 2020-10-21 12:24 ` Igor Mammedov
2020-10-21 13:30 ` Eduardo Habkost
1 sibling, 1 reply; 28+ messages in thread
From: Igor Mammedov @ 2020-10-21 12:24 UTC (permalink / raw)
To: Eduardo Habkost
Cc: Kevin Wolf, Daniel P. Berrangé,
qemu-devel, Markus Armbruster, Marc-André Lureau,
Paolo Bonzini, John Snow
On Fri, 9 Oct 2020 12:01:13 -0400
Eduardo Habkost <ehabkost@redhat.com> wrote:
> The existing object_class_property_add_uint*_ptr() functions are
> not very useful, because they need a pointer to the property
> value, which can't really be provided before the object is
> created.
>
> Replace the pointer parameter in those functions with a
> `ptrdiff_t offset` parameter.
>
> Include a uint8 class property in check-qom-proplist unit tests,
> to ensure the feature is working.
Not sure I like approach, it's reinventing qdev pointer properties in QOM form.
I had an impression that Paolo wanted qdev pointer properties be gone
and replaced by something like link properties.
object_property_add_uintXX_ptr() were introduced as a quick hack,
when ACPI code generation was moved from Seabios, to avoid more
code shuffling in device models and adding more boiler plate in
form of custom setters/getters (the later didn't seem to bother
us everywhere else where we use object_[class_]property_add() ).
Then it spread little bit to another places.
I'd rather get rid of object_property_add_uintXX_ptr() API altogether
in favor of object_[class_]property_add() like it is used in other places
to handle intXX properties.
Adding helpers similar to object_property_add_bool() for intXX
could reduce boiler plate need for converting current instances of
_ptr(), and such helpers would also help with reducing boilerplate
for the rest of instances where object_[class_]property_add()
currently is used for dealing with integers.
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: "Daniel P. Berrangé" <berrange@redhat.com>
> Cc: Eduardo Habkost <ehabkost@redhat.com>
> Cc: qemu-devel@nongnu.org
> ---
> include/qom/object.h | 8 ++++----
> qom/object.c | 36 +++++++++++++++++++++++-------------
> tests/check-qom-proplist.c | 10 ++++++++--
> 3 files changed, 35 insertions(+), 19 deletions(-)
>
> diff --git a/include/qom/object.h b/include/qom/object.h
> index d378f13a11..1634294e4f 100644
> --- a/include/qom/object.h
> +++ b/include/qom/object.h
> @@ -1747,7 +1747,7 @@ ObjectProperty *object_property_add_uint8_ptr(Object *obj, const char *name,
>
> ObjectProperty *object_class_property_add_uint8_ptr(ObjectClass *klass,
> const char *name,
> - const uint8_t *v,
> + ptrdiff_t offset,
> ObjectPropertyFlags flags);
>
> /**
> @@ -1768,7 +1768,7 @@ ObjectProperty *object_property_add_uint16_ptr(Object *obj, const char *name,
>
> ObjectProperty *object_class_property_add_uint16_ptr(ObjectClass *klass,
> const char *name,
> - const uint16_t *v,
> + ptrdiff_t offset,
> ObjectPropertyFlags flags);
>
> /**
> @@ -1789,7 +1789,7 @@ ObjectProperty *object_property_add_uint32_ptr(Object *obj, const char *name,
>
> ObjectProperty *object_class_property_add_uint32_ptr(ObjectClass *klass,
> const char *name,
> - const uint32_t *v,
> + ptrdiff_t offset,
> ObjectPropertyFlags flags);
>
> /**
> @@ -1810,7 +1810,7 @@ ObjectProperty *object_property_add_uint64_ptr(Object *obj, const char *name,
>
> ObjectProperty *object_class_property_add_uint64_ptr(ObjectClass *klass,
> const char *name,
> - const uint64_t *v,
> + ptrdiff_t offset,
> ObjectPropertyFlags flags);
>
> /**
> diff --git a/qom/object.c b/qom/object.c
> index 17692ed5c3..bb32f5d3ad 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -2450,13 +2450,22 @@ static char *object_get_type(Object *obj, Error **errp)
> }
>
> typedef struct {
> - /* Pointer to property value */
> - void *ptr;
> + bool is_offset;
> + union {
> + /* Pointer to property value. Valid if is_offset=false */
> + void *ptr;
> + /* Offset in Object struct. Valid if is_offset=true */
> + ptrdiff_t offset;
> + };
> } PointerProperty;
>
> static void *pointer_property_get_ptr(Object *obj, PointerProperty *prop)
> {
> - return prop->ptr;
> + if (prop->is_offset) {
> + return (void *)obj + prop->offset;
> + } else {
> + return prop->ptr;
> + }
> }
>
> static void property_get_uint8_ptr(Object *obj, Visitor *v, const char *name,
> @@ -2573,10 +2582,11 @@ object_class_property_add_uint_ptr(ObjectClass *oc, const char *name,
> ObjectPropertyAccessor getter,
> ObjectPropertyAccessor setter,
> ObjectPropertyFlags flags,
> - void *ptr)
> + ptrdiff_t offset)
> {
> PointerProperty *prop = g_new0(PointerProperty, 1);
> - prop->ptr = ptr;
> + prop->is_offset = true;
> + prop->offset = offset;
> return object_class_property_add(oc, name, type,
> (flags & OBJ_PROP_FLAG_READ) ? getter : NULL,
> (flags & OBJ_PROP_FLAG_WRITE) ? setter : NULL,
> @@ -2597,13 +2607,13 @@ object_property_add_uint8_ptr(Object *obj, const char *name,
>
> ObjectProperty *
> object_class_property_add_uint8_ptr(ObjectClass *klass, const char *name,
> - const uint8_t *v,
> + ptrdiff_t offset,
> ObjectPropertyFlags flags)
> {
> return object_class_property_add_uint_ptr(klass, name, "uint8",
> property_get_uint8_ptr,
> property_set_uint8_ptr,
> - flags, (void *)v);
> + flags, offset);
> }
>
> ObjectProperty *
> @@ -2620,13 +2630,13 @@ object_property_add_uint16_ptr(Object *obj, const char *name,
>
> ObjectProperty *
> object_class_property_add_uint16_ptr(ObjectClass *klass, const char *name,
> - const uint16_t *v,
> + ptrdiff_t offset,
> ObjectPropertyFlags flags)
> {
> return object_class_property_add_uint_ptr(klass, name, "uint16",
> property_get_uint16_ptr,
> property_set_uint16_ptr,
> - flags, (void *)v);
> + flags, offset);
> }
>
> ObjectProperty *
> @@ -2643,13 +2653,13 @@ object_property_add_uint32_ptr(Object *obj, const char *name,
>
> ObjectProperty *
> object_class_property_add_uint32_ptr(ObjectClass *klass, const char *name,
> - const uint32_t *v,
> + ptrdiff_t offset,
> ObjectPropertyFlags flags)
> {
> return object_class_property_add_uint_ptr(klass, name, "uint32",
> property_get_uint32_ptr,
> property_set_uint32_ptr,
> - flags, (void *)v);
> + flags, offset);
> }
>
> ObjectProperty *
> @@ -2666,13 +2676,13 @@ object_property_add_uint64_ptr(Object *obj, const char *name,
>
> ObjectProperty *
> object_class_property_add_uint64_ptr(ObjectClass *klass, const char *name,
> - const uint64_t *v,
> + ptrdiff_t offset,
> ObjectPropertyFlags flags)
> {
> return object_class_property_add_uint_ptr(klass, name, "uint64",
> property_get_uint64_ptr,
> property_set_uint64_ptr,
> - flags, (void *)v);
> + flags, offset);
> }
>
> typedef struct {
> diff --git a/tests/check-qom-proplist.c b/tests/check-qom-proplist.c
> index 1b76581980..fba30c20b2 100644
> --- a/tests/check-qom-proplist.c
> +++ b/tests/check-qom-proplist.c
> @@ -61,6 +61,7 @@ struct DummyObject {
> bool bv;
> DummyAnimal av;
> char *sv;
> + uint8_t u8v;
> };
>
> struct DummyObjectClass {
> @@ -141,6 +142,9 @@ static void dummy_class_init(ObjectClass *cls, void *data)
> &dummy_animal_map,
> dummy_get_av,
> dummy_set_av);
> + object_class_property_add_uint8_ptr(cls, "u8v",
> + offsetof(DummyObject, u8v),
> + OBJ_PROP_FLAG_READWRITE);
> }
>
>
> @@ -385,12 +389,14 @@ static void test_dummy_createlist(void)
> "bv", "yes",
> "sv", "Hiss hiss hiss",
> "av", "platypus",
> + "u8v", "42",
> NULL));
>
> g_assert(err == NULL);
> g_assert_cmpstr(dobj->sv, ==, "Hiss hiss hiss");
> g_assert(dobj->bv == true);
> g_assert(dobj->av == DUMMY_PLATYPUS);
> + g_assert_cmpint(dobj->u8v, ==, 42);
>
> g_assert(object_resolve_path_component(parent, "dummy0")
> == OBJECT(dobj));
> @@ -531,7 +537,7 @@ static void test_dummy_iterator(void)
> {
> const char *expected[] = {
> "type", /* inherited from TYPE_OBJECT */
> - "sv", "av", /* class properties */
> + "sv", "av", "u8v", /* class properties */
> "bv"}; /* instance property */
> Object *parent = object_get_objects_root();
> DummyObject *dobj = DUMMY_OBJECT(
> @@ -552,7 +558,7 @@ static void test_dummy_iterator(void)
>
> static void test_dummy_class_iterator(void)
> {
> - const char *expected[] = { "type", "av", "sv" };
> + const char *expected[] = { "type", "av", "sv", "u8v" };
> ObjectPropertyIterator iter;
> ObjectClass *klass = object_class_by_name(TYPE_DUMMY);
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 03/12] qom: Make object_class_property_add_uint*_ptr() get offset
2020-10-21 12:24 ` Igor Mammedov
@ 2020-10-21 13:30 ` Eduardo Habkost
2020-10-22 5:06 ` Markus Armbruster
2020-10-23 15:33 ` Igor Mammedov
0 siblings, 2 replies; 28+ messages in thread
From: Eduardo Habkost @ 2020-10-21 13:30 UTC (permalink / raw)
To: Igor Mammedov
Cc: Kevin Wolf, Daniel P. Berrangé,
qemu-devel, Markus Armbruster, Marc-André Lureau,
Paolo Bonzini, John Snow
On Wed, Oct 21, 2020 at 02:24:08PM +0200, Igor Mammedov wrote:
> On Fri, 9 Oct 2020 12:01:13 -0400
> Eduardo Habkost <ehabkost@redhat.com> wrote:
>
> > The existing object_class_property_add_uint*_ptr() functions are
> > not very useful, because they need a pointer to the property
> > value, which can't really be provided before the object is
> > created.
> >
> > Replace the pointer parameter in those functions with a
> > `ptrdiff_t offset` parameter.
> >
> > Include a uint8 class property in check-qom-proplist unit tests,
> > to ensure the feature is working.
>
>
> Not sure I like approach, it's reinventing qdev pointer properties in QOM form.
Yes, and that's on purpose. If we want to eventually merge the
two competing APIs into a single one, we need to make them
converge.
> I had an impression that Paolo wanted qdev pointer properties be gone
> and replaced by something like link properties.
This is completely unrelated to qdev pointer properties and link
properties. The properties that use object_property_add_uint*_ptr()
today are not qdev pointer properties and will never be link
properties. They are just integer properties.
>
> object_property_add_uintXX_ptr() were introduced as a quick hack,
> when ACPI code generation was moved from Seabios, to avoid more
> code shuffling in device models and adding more boiler plate in
> form of custom setters/getters (the later didn't seem to bother
> us everywhere else where we use object_[class_]property_add() ).
> Then it spread little bit to another places.
>
> I'd rather get rid of object_property_add_uintXX_ptr() API altogether
> in favor of object_[class_]property_add() like it is used in other places
> to handle intXX properties.
> Adding helpers similar to object_property_add_bool() for intXX
> could reduce boiler plate need for converting current instances of
> _ptr(), and such helpers would also help with reducing boilerplate
> for the rest of instances where object_[class_]property_add()
> currently is used for dealing with integers.
I find object_property_add_bool() terrible. It requires too much
boilerplate. I actually have plans to introduce
object*_property_add_bool_ptr() to simplify existing
object_property_add_bool() callers.
I don't love object*_property_add_*_ptr() either. I consider the
qdev property API better. But we need a reasonable alternative,
because the qdev API can't be used by non-device objects yet.
I don't think object*_property_add() and
object*_property_add_bool() are reasonable alternatives.
>
>
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > ---
> > Cc: Paolo Bonzini <pbonzini@redhat.com>
> > Cc: "Daniel P. Berrangé" <berrange@redhat.com>
> > Cc: Eduardo Habkost <ehabkost@redhat.com>
> > Cc: qemu-devel@nongnu.org
> > ---
> > include/qom/object.h | 8 ++++----
> > qom/object.c | 36 +++++++++++++++++++++++-------------
> > tests/check-qom-proplist.c | 10 ++++++++--
> > 3 files changed, 35 insertions(+), 19 deletions(-)
> >
> > diff --git a/include/qom/object.h b/include/qom/object.h
> > index d378f13a11..1634294e4f 100644
> > --- a/include/qom/object.h
> > +++ b/include/qom/object.h
> > @@ -1747,7 +1747,7 @@ ObjectProperty *object_property_add_uint8_ptr(Object *obj, const char *name,
> >
> > ObjectProperty *object_class_property_add_uint8_ptr(ObjectClass *klass,
> > const char *name,
> > - const uint8_t *v,
> > + ptrdiff_t offset,
> > ObjectPropertyFlags flags);
> >
> > /**
> > @@ -1768,7 +1768,7 @@ ObjectProperty *object_property_add_uint16_ptr(Object *obj, const char *name,
> >
> > ObjectProperty *object_class_property_add_uint16_ptr(ObjectClass *klass,
> > const char *name,
> > - const uint16_t *v,
> > + ptrdiff_t offset,
> > ObjectPropertyFlags flags);
> >
> > /**
> > @@ -1789,7 +1789,7 @@ ObjectProperty *object_property_add_uint32_ptr(Object *obj, const char *name,
> >
> > ObjectProperty *object_class_property_add_uint32_ptr(ObjectClass *klass,
> > const char *name,
> > - const uint32_t *v,
> > + ptrdiff_t offset,
> > ObjectPropertyFlags flags);
> >
> > /**
> > @@ -1810,7 +1810,7 @@ ObjectProperty *object_property_add_uint64_ptr(Object *obj, const char *name,
> >
> > ObjectProperty *object_class_property_add_uint64_ptr(ObjectClass *klass,
> > const char *name,
> > - const uint64_t *v,
> > + ptrdiff_t offset,
> > ObjectPropertyFlags flags);
> >
> > /**
> > diff --git a/qom/object.c b/qom/object.c
> > index 17692ed5c3..bb32f5d3ad 100644
> > --- a/qom/object.c
> > +++ b/qom/object.c
> > @@ -2450,13 +2450,22 @@ static char *object_get_type(Object *obj, Error **errp)
> > }
> >
> > typedef struct {
> > - /* Pointer to property value */
> > - void *ptr;
> > + bool is_offset;
> > + union {
> > + /* Pointer to property value. Valid if is_offset=false */
> > + void *ptr;
> > + /* Offset in Object struct. Valid if is_offset=true */
> > + ptrdiff_t offset;
> > + };
> > } PointerProperty;
> >
> > static void *pointer_property_get_ptr(Object *obj, PointerProperty *prop)
> > {
> > - return prop->ptr;
> > + if (prop->is_offset) {
> > + return (void *)obj + prop->offset;
> > + } else {
> > + return prop->ptr;
> > + }
> > }
> >
> > static void property_get_uint8_ptr(Object *obj, Visitor *v, const char *name,
> > @@ -2573,10 +2582,11 @@ object_class_property_add_uint_ptr(ObjectClass *oc, const char *name,
> > ObjectPropertyAccessor getter,
> > ObjectPropertyAccessor setter,
> > ObjectPropertyFlags flags,
> > - void *ptr)
> > + ptrdiff_t offset)
> > {
> > PointerProperty *prop = g_new0(PointerProperty, 1);
> > - prop->ptr = ptr;
> > + prop->is_offset = true;
> > + prop->offset = offset;
> > return object_class_property_add(oc, name, type,
> > (flags & OBJ_PROP_FLAG_READ) ? getter : NULL,
> > (flags & OBJ_PROP_FLAG_WRITE) ? setter : NULL,
> > @@ -2597,13 +2607,13 @@ object_property_add_uint8_ptr(Object *obj, const char *name,
> >
> > ObjectProperty *
> > object_class_property_add_uint8_ptr(ObjectClass *klass, const char *name,
> > - const uint8_t *v,
> > + ptrdiff_t offset,
> > ObjectPropertyFlags flags)
> > {
> > return object_class_property_add_uint_ptr(klass, name, "uint8",
> > property_get_uint8_ptr,
> > property_set_uint8_ptr,
> > - flags, (void *)v);
> > + flags, offset);
> > }
> >
> > ObjectProperty *
> > @@ -2620,13 +2630,13 @@ object_property_add_uint16_ptr(Object *obj, const char *name,
> >
> > ObjectProperty *
> > object_class_property_add_uint16_ptr(ObjectClass *klass, const char *name,
> > - const uint16_t *v,
> > + ptrdiff_t offset,
> > ObjectPropertyFlags flags)
> > {
> > return object_class_property_add_uint_ptr(klass, name, "uint16",
> > property_get_uint16_ptr,
> > property_set_uint16_ptr,
> > - flags, (void *)v);
> > + flags, offset);
> > }
> >
> > ObjectProperty *
> > @@ -2643,13 +2653,13 @@ object_property_add_uint32_ptr(Object *obj, const char *name,
> >
> > ObjectProperty *
> > object_class_property_add_uint32_ptr(ObjectClass *klass, const char *name,
> > - const uint32_t *v,
> > + ptrdiff_t offset,
> > ObjectPropertyFlags flags)
> > {
> > return object_class_property_add_uint_ptr(klass, name, "uint32",
> > property_get_uint32_ptr,
> > property_set_uint32_ptr,
> > - flags, (void *)v);
> > + flags, offset);
> > }
> >
> > ObjectProperty *
> > @@ -2666,13 +2676,13 @@ object_property_add_uint64_ptr(Object *obj, const char *name,
> >
> > ObjectProperty *
> > object_class_property_add_uint64_ptr(ObjectClass *klass, const char *name,
> > - const uint64_t *v,
> > + ptrdiff_t offset,
> > ObjectPropertyFlags flags)
> > {
> > return object_class_property_add_uint_ptr(klass, name, "uint64",
> > property_get_uint64_ptr,
> > property_set_uint64_ptr,
> > - flags, (void *)v);
> > + flags, offset);
> > }
> >
> > typedef struct {
> > diff --git a/tests/check-qom-proplist.c b/tests/check-qom-proplist.c
> > index 1b76581980..fba30c20b2 100644
> > --- a/tests/check-qom-proplist.c
> > +++ b/tests/check-qom-proplist.c
> > @@ -61,6 +61,7 @@ struct DummyObject {
> > bool bv;
> > DummyAnimal av;
> > char *sv;
> > + uint8_t u8v;
> > };
> >
> > struct DummyObjectClass {
> > @@ -141,6 +142,9 @@ static void dummy_class_init(ObjectClass *cls, void *data)
> > &dummy_animal_map,
> > dummy_get_av,
> > dummy_set_av);
> > + object_class_property_add_uint8_ptr(cls, "u8v",
> > + offsetof(DummyObject, u8v),
> > + OBJ_PROP_FLAG_READWRITE);
> > }
> >
> >
> > @@ -385,12 +389,14 @@ static void test_dummy_createlist(void)
> > "bv", "yes",
> > "sv", "Hiss hiss hiss",
> > "av", "platypus",
> > + "u8v", "42",
> > NULL));
> >
> > g_assert(err == NULL);
> > g_assert_cmpstr(dobj->sv, ==, "Hiss hiss hiss");
> > g_assert(dobj->bv == true);
> > g_assert(dobj->av == DUMMY_PLATYPUS);
> > + g_assert_cmpint(dobj->u8v, ==, 42);
> >
> > g_assert(object_resolve_path_component(parent, "dummy0")
> > == OBJECT(dobj));
> > @@ -531,7 +537,7 @@ static void test_dummy_iterator(void)
> > {
> > const char *expected[] = {
> > "type", /* inherited from TYPE_OBJECT */
> > - "sv", "av", /* class properties */
> > + "sv", "av", "u8v", /* class properties */
> > "bv"}; /* instance property */
> > Object *parent = object_get_objects_root();
> > DummyObject *dobj = DUMMY_OBJECT(
> > @@ -552,7 +558,7 @@ static void test_dummy_iterator(void)
> >
> > static void test_dummy_class_iterator(void)
> > {
> > - const char *expected[] = { "type", "av", "sv" };
> > + const char *expected[] = { "type", "av", "sv", "u8v" };
> > ObjectPropertyIterator iter;
> > ObjectClass *klass = object_class_by_name(TYPE_DUMMY);
> >
>
--
Eduardo
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 03/12] qom: Make object_class_property_add_uint*_ptr() get offset
2020-10-21 13:30 ` Eduardo Habkost
@ 2020-10-22 5:06 ` Markus Armbruster
2020-10-22 21:34 ` Eduardo Habkost
2020-10-23 15:33 ` Igor Mammedov
1 sibling, 1 reply; 28+ messages in thread
From: Markus Armbruster @ 2020-10-22 5:06 UTC (permalink / raw)
To: Eduardo Habkost
Cc: Kevin Wolf, Daniel P. Berrangé,
qemu-devel, Marc-André Lureau, Paolo Bonzini, Igor Mammedov,
John Snow
Eduardo Habkost <ehabkost@redhat.com> writes:
> On Wed, Oct 21, 2020 at 02:24:08PM +0200, Igor Mammedov wrote:
>> On Fri, 9 Oct 2020 12:01:13 -0400
>> Eduardo Habkost <ehabkost@redhat.com> wrote:
>>
>> > The existing object_class_property_add_uint*_ptr() functions are
>> > not very useful, because they need a pointer to the property
>> > value, which can't really be provided before the object is
>> > created.
>> >
>> > Replace the pointer parameter in those functions with a
>> > `ptrdiff_t offset` parameter.
>> >
>> > Include a uint8 class property in check-qom-proplist unit tests,
>> > to ensure the feature is working.
>>
>>
>> Not sure I like approach, it's reinventing qdev pointer properties in QOM form.
>
> Yes, and that's on purpose. If we want to eventually merge the
> two competing APIs into a single one, we need to make them
> converge.
>
>> I had an impression that Paolo wanted qdev pointer properties be gone
>> and replaced by something like link properties.
>
> This is completely unrelated to qdev pointer properties and link
> properties. The properties that use object_property_add_uint*_ptr()
> today are not qdev pointer properties and will never be link
> properties. They are just integer properties.
>
>>
>> object_property_add_uintXX_ptr() were introduced as a quick hack,
>> when ACPI code generation was moved from Seabios, to avoid more
>> code shuffling in device models and adding more boiler plate in
>> form of custom setters/getters (the later didn't seem to bother
>> us everywhere else where we use object_[class_]property_add() ).
>> Then it spread little bit to another places.
>>
>> I'd rather get rid of object_property_add_uintXX_ptr() API altogether
>> in favor of object_[class_]property_add() like it is used in other places
>> to handle intXX properties.
>> Adding helpers similar to object_property_add_bool() for intXX
>> could reduce boiler plate need for converting current instances of
>> _ptr(), and such helpers would also help with reducing boilerplate
>> for the rest of instances where object_[class_]property_add()
>> currently is used for dealing with integers.
>
> I find object_property_add_bool() terrible. It requires too much
> boilerplate. I actually have plans to introduce
> object*_property_add_bool_ptr() to simplify existing
> object_property_add_bool() callers.
>
> I don't love object*_property_add_*_ptr() either. I consider the
> qdev property API better. But we need a reasonable alternative,
> because the qdev API can't be used by non-device objects yet.
Emphasis on *yet*: we should be able to lift it up into QOM, shouldn't
we?
> I don't think object*_property_add() and
> object*_property_add_bool() are reasonable alternatives.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 03/12] qom: Make object_class_property_add_uint*_ptr() get offset
2020-10-22 5:06 ` Markus Armbruster
@ 2020-10-22 21:34 ` Eduardo Habkost
0 siblings, 0 replies; 28+ messages in thread
From: Eduardo Habkost @ 2020-10-22 21:34 UTC (permalink / raw)
To: Markus Armbruster
Cc: Kevin Wolf, Daniel P. Berrangé,
qemu-devel, Marc-André Lureau, Paolo Bonzini, Igor Mammedov,
John Snow
On Thu, Oct 22, 2020 at 07:06:58AM +0200, Markus Armbruster wrote:
[...]
> > I don't love object*_property_add_*_ptr() either. I consider the
> > qdev property API better. But we need a reasonable alternative,
> > because the qdev API can't be used by non-device objects yet.
>
> Emphasis on *yet*: we should be able to lift it up into QOM, shouldn't
> we?
We should, yes. My plan is to make object_property_*_ptr() and
PropertyInfo code converge until they look exactly the same and
become a single API.
--
Eduardo
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 03/12] qom: Make object_class_property_add_uint*_ptr() get offset
2020-10-21 13:30 ` Eduardo Habkost
2020-10-22 5:06 ` Markus Armbruster
@ 2020-10-23 15:33 ` Igor Mammedov
2020-10-27 22:18 ` Eduardo Habkost
2020-10-28 15:22 ` Paolo Bonzini
1 sibling, 2 replies; 28+ messages in thread
From: Igor Mammedov @ 2020-10-23 15:33 UTC (permalink / raw)
To: Eduardo Habkost
Cc: Kevin Wolf, Daniel P. Berrangé,
qemu-devel, Markus Armbruster, Paolo Bonzini,
Marc-André Lureau, John Snow
On Wed, 21 Oct 2020 09:30:41 -0400
Eduardo Habkost <ehabkost@redhat.com> wrote:
> On Wed, Oct 21, 2020 at 02:24:08PM +0200, Igor Mammedov wrote:
> > On Fri, 9 Oct 2020 12:01:13 -0400
> > Eduardo Habkost <ehabkost@redhat.com> wrote:
> >
> > > The existing object_class_property_add_uint*_ptr() functions are
> > > not very useful, because they need a pointer to the property
> > > value, which can't really be provided before the object is
> > > created.
> > >
> > > Replace the pointer parameter in those functions with a
> > > `ptrdiff_t offset` parameter.
> > >
> > > Include a uint8 class property in check-qom-proplist unit tests,
> > > to ensure the feature is working.
> >
> >
> > Not sure I like approach, it's reinventing qdev pointer properties in QOM form.
>
> Yes, and that's on purpose. If we want to eventually merge the
> two competing APIs into a single one, we need to make them
> converge.
>
> > I had an impression that Paolo wanted qdev pointer properties be gone
> > and replaced by something like link properties.
>
> This is completely unrelated to qdev pointer properties and link
> properties. The properties that use object_property_add_uint*_ptr()
> today are not qdev pointer properties and will never be link
> properties. They are just integer properties.
right, _prt confused me for a while.
>
> >
> > object_property_add_uintXX_ptr() were introduced as a quick hack,
> > when ACPI code generation was moved from Seabios, to avoid more
> > code shuffling in device models and adding more boiler plate in
> > form of custom setters/getters (the later didn't seem to bother
> > us everywhere else where we use object_[class_]property_add() ).
> > Then it spread little bit to another places.
> >
> > I'd rather get rid of object_property_add_uintXX_ptr() API altogether
> > in favor of object_[class_]property_add() like it is used in other places
> > to handle intXX properties.
> > Adding helpers similar to object_property_add_bool() for intXX
> > could reduce boiler plate need for converting current instances of
> > _ptr(), and such helpers would also help with reducing boilerplate
> > for the rest of instances where object_[class_]property_add()
> > currently is used for dealing with integers.
>
> I find object_property_add_bool() terrible. It requires too much
> boilerplate. I actually have plans to introduce
> object*_property_add_bool_ptr() to simplify existing
> object_property_add_bool() callers.
But boiler-plate related to QOM properties set/get methods was considered
tolerable back then.
It was a long time ago, so I don't recall why we decided to abandon
qdev properties API.
> I don't love object*_property_add_*_ptr() either. I consider the
> qdev property API better. But we need a reasonable alternative,
> because the qdev API can't be used by non-device objects yet.
> I don't think object*_property_add() and
> object*_property_add_bool() are reasonable alternatives.
I also like old qdev API as it can be introspected (it's just data at
class level), very concise when used and has default values.
Instead of duplicating all that pointer arithmetic from qdev properties
in QOM API, it could be better to fix qdev properties so that they
would work for Object as well.
At least all that thrown away type safety would stay constrained/hidden
inside of qdev property macros, instead of being opencoded (offsets) all
over the place.
How hard it would be make qdev properties to work with Object and what
makes you duplicate ugly part of it in QOM instead of making them to
handle Object strait away?
That would also result in huge removal of boiler plate of current QOM
properties.
That should suit your goal to make (most) properties introspectable
and statically described.
[...]
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 03/12] qom: Make object_class_property_add_uint*_ptr() get offset
2020-10-23 15:33 ` Igor Mammedov
@ 2020-10-27 22:18 ` Eduardo Habkost
2020-10-28 15:22 ` Paolo Bonzini
1 sibling, 0 replies; 28+ messages in thread
From: Eduardo Habkost @ 2020-10-27 22:18 UTC (permalink / raw)
To: Igor Mammedov
Cc: Kevin Wolf, Daniel P. Berrangé,
qemu-devel, Markus Armbruster, Paolo Bonzini,
Marc-André Lureau, John Snow
On Fri, Oct 23, 2020 at 05:33:14PM +0200, Igor Mammedov wrote:
> On Wed, 21 Oct 2020 09:30:41 -0400
> Eduardo Habkost <ehabkost@redhat.com> wrote:
[...]
> > I don't love object*_property_add_*_ptr() either. I consider the
> > qdev property API better. But we need a reasonable alternative,
> > because the qdev API can't be used by non-device objects yet.
> > I don't think object*_property_add() and
> > object*_property_add_bool() are reasonable alternatives.
>
> I also like old qdev API as it can be introspected (it's just data at
> class level), very concise when used and has default values.
>
> Instead of duplicating all that pointer arithmetic from qdev properties
> in QOM API, it could be better to fix qdev properties so that they
> would work for Object as well.
> At least all that thrown away type safety would stay constrained/hidden
> inside of qdev property macros, instead of being opencoded (offsets) all
> over the place.
>
> How hard it would be make qdev properties to work with Object and what
> makes you duplicate ugly part of it in QOM instead of making them to
> handle Object strait away?
It is doable, but lots of work. I'm working on this right now.
> That would also result in huge removal of boiler plate of current QOM
> properties.
Yep.
>
> That should suit your goal to make (most) properties introspectable
> and statically described.
That's correct. I just don't want a huge qdev refactor to be a
reason to delay important work in other areas.
--
Eduardo
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 03/12] qom: Make object_class_property_add_uint*_ptr() get offset
2020-10-23 15:33 ` Igor Mammedov
2020-10-27 22:18 ` Eduardo Habkost
@ 2020-10-28 15:22 ` Paolo Bonzini
2020-10-28 15:53 ` Igor Mammedov
2020-10-29 12:56 ` Eduardo Habkost
1 sibling, 2 replies; 28+ messages in thread
From: Paolo Bonzini @ 2020-10-28 15:22 UTC (permalink / raw)
To: Igor Mammedov, Eduardo Habkost
Cc: Kevin Wolf, Daniel P. Berrangé,
qemu-devel, Markus Armbruster, Marc-André Lureau, John Snow
On 23/10/20 17:33, Igor Mammedov wrote:
> On Wed, 21 Oct 2020 09:30:41 -0400
> Eduardo Habkost <ehabkost@redhat.com> wrote:
>
>> On Wed, Oct 21, 2020 at 02:24:08PM +0200, Igor Mammedov wrote:
>>> On Fri, 9 Oct 2020 12:01:13 -0400
>>> Eduardo Habkost <ehabkost@redhat.com> wrote:
>>>
>>>> The existing object_class_property_add_uint*_ptr() functions are
>>>> not very useful, because they need a pointer to the property
>>>> value, which can't really be provided before the object is
>>>> created.
>>>>
>>>> Replace the pointer parameter in those functions with a
>>>> `ptrdiff_t offset` parameter.
>>>>
>>>> Include a uint8 class property in check-qom-proplist unit tests,
>>>> to ensure the feature is working.
>>>
>>>
>>> Not sure I like approach, it's reinventing qdev pointer properties in QOM form.
>>
>> Yes, and that's on purpose. If we want to eventually merge the
>> two competing APIs into a single one, we need to make them
>> converge.
>>
>>> I had an impression that Paolo wanted qdev pointer properties be gone
>>> and replaced by something like link properties.
>>
>> This is completely unrelated to qdev pointer properties and link
>> properties. The properties that use object_property_add_uint*_ptr()
>> today are not qdev pointer properties and will never be link
>> properties. They are just integer properties.
I think this series a step in the right direction, but please take more
"inspiration" from link properties, which are done right. In
particular, properties should have an optional check function and be
read-only unless the check function is there.
You can make the check function take an uint64_t for simplicity, so that
all the check functions for uint properties have the same prototype.
For example a single "property_check_uint_allow" function can allow
setting the property (which is almost always wrong, but an easy cop out
for this series).
Paolo
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 03/12] qom: Make object_class_property_add_uint*_ptr() get offset
2020-10-28 15:22 ` Paolo Bonzini
@ 2020-10-28 15:53 ` Igor Mammedov
2020-10-29 12:56 ` Eduardo Habkost
1 sibling, 0 replies; 28+ messages in thread
From: Igor Mammedov @ 2020-10-28 15:53 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Kevin Wolf, Daniel P. Berrangé,
Eduardo Habkost, Markus Armbruster, qemu-devel,
Marc-André Lureau, John Snow
On Wed, 28 Oct 2020 16:22:40 +0100
Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 23/10/20 17:33, Igor Mammedov wrote:
> > On Wed, 21 Oct 2020 09:30:41 -0400
> > Eduardo Habkost <ehabkost@redhat.com> wrote:
> >
> >> On Wed, Oct 21, 2020 at 02:24:08PM +0200, Igor Mammedov wrote:
> >>> On Fri, 9 Oct 2020 12:01:13 -0400
> >>> Eduardo Habkost <ehabkost@redhat.com> wrote:
> >>>
> >>>> The existing object_class_property_add_uint*_ptr() functions are
> >>>> not very useful, because they need a pointer to the property
> >>>> value, which can't really be provided before the object is
> >>>> created.
> >>>>
> >>>> Replace the pointer parameter in those functions with a
> >>>> `ptrdiff_t offset` parameter.
> >>>>
> >>>> Include a uint8 class property in check-qom-proplist unit tests,
> >>>> to ensure the feature is working.
> >>>
> >>>
> >>> Not sure I like approach, it's reinventing qdev pointer properties in QOM form.
> >>
> >> Yes, and that's on purpose. If we want to eventually merge the
> >> two competing APIs into a single one, we need to make them
> >> converge.
> >>
> >>> I had an impression that Paolo wanted qdev pointer properties be gone
> >>> and replaced by something like link properties.
> >>
> >> This is completely unrelated to qdev pointer properties and link
> >> properties. The properties that use object_property_add_uint*_ptr()
> >> today are not qdev pointer properties and will never be link
> >> properties. They are just integer properties.
>
> I think this series a step in the right direction, but please take more
> "inspiration" from link properties, which are done right. In
> particular, properties should have an optional check function and be
> read-only unless the check function is there.
object_class_property_add_uint*_ptr() is similar to what we have in QDEV
properties already implemented. But that is all hidden behind macro
magic, so users aren't using it directly.
But what I dislike the most is adding _class_ variants of those with
offsets exposed to users call site without any type checking.
It might be easier and safer to make current QDEV properties to work
with Object in one go, instead of duplication small parts of it in
object_foo() API.
But then I haven't actually tried so ...
> You can make the check function take an uint64_t for simplicity, so that
> all the check functions for uint properties have the same prototype.
> For example a single "property_check_uint_allow" function can allow
> setting the property (which is almost always wrong, but an easy cop out
> for this series).
>
> Paolo
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 03/12] qom: Make object_class_property_add_uint*_ptr() get offset
2020-10-28 15:22 ` Paolo Bonzini
2020-10-28 15:53 ` Igor Mammedov
@ 2020-10-29 12:56 ` Eduardo Habkost
2020-10-29 13:37 ` Igor Mammedov
1 sibling, 1 reply; 28+ messages in thread
From: Eduardo Habkost @ 2020-10-29 12:56 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Kevin Wolf, Daniel P. Berrangé,
qemu-devel, Markus Armbruster, Marc-André Lureau,
Igor Mammedov, John Snow
On Wed, Oct 28, 2020 at 04:22:40PM +0100, Paolo Bonzini wrote:
> On 23/10/20 17:33, Igor Mammedov wrote:
> > On Wed, 21 Oct 2020 09:30:41 -0400
> > Eduardo Habkost <ehabkost@redhat.com> wrote:
> >
> >> On Wed, Oct 21, 2020 at 02:24:08PM +0200, Igor Mammedov wrote:
> >>> On Fri, 9 Oct 2020 12:01:13 -0400
> >>> Eduardo Habkost <ehabkost@redhat.com> wrote:
> >>>
> >>>> The existing object_class_property_add_uint*_ptr() functions are
> >>>> not very useful, because they need a pointer to the property
> >>>> value, which can't really be provided before the object is
> >>>> created.
> >>>>
> >>>> Replace the pointer parameter in those functions with a
> >>>> `ptrdiff_t offset` parameter.
> >>>>
> >>>> Include a uint8 class property in check-qom-proplist unit tests,
> >>>> to ensure the feature is working.
> >>>
> >>>
> >>> Not sure I like approach, it's reinventing qdev pointer properties in QOM form.
> >>
> >> Yes, and that's on purpose. If we want to eventually merge the
> >> two competing APIs into a single one, we need to make them
> >> converge.
> >>
> >>> I had an impression that Paolo wanted qdev pointer properties be gone
> >>> and replaced by something like link properties.
> >>
> >> This is completely unrelated to qdev pointer properties and link
> >> properties. The properties that use object_property_add_uint*_ptr()
> >> today are not qdev pointer properties and will never be link
> >> properties. They are just integer properties.
>
> I think this series a step in the right direction, but please take more
> "inspiration" from link properties, which are done right. In
> particular, properties should have an optional check function and be
> read-only unless the check function is there.
>
> You can make the check function take an uint64_t for simplicity, so that
> all the check functions for uint properties have the same prototype.
> For example a single "property_check_uint_allow" function can allow
> setting the property (which is almost always wrong, but an easy cop out
> for this series).
A property check callback that needs the property value is a more
complex use case, and would require too much property-type-specific
boilerplate today. I plan to address it, but not right now.
In my next series that makes static properties usable by any QOM
object, I will add a separate "allow_set" callback to the
internal QOM property API, which will not take the property value
as argument. This would be enough for the dev->realized checks
that are currently in qdev.
Interestingly, there is only one link property check callback
function in the QEMU tree that actually cares about the property
value: isa_ipmi_bmc_check(). All other cases either don't care
about the property value at all (qdev_prop_allow_set_link_before_realize(),
object_property_allow_set_link()), or are being misused for
something other than property checking (xlnx_dp_set_dpdma()).
--
Eduardo
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 03/12] qom: Make object_class_property_add_uint*_ptr() get offset
2020-10-29 12:56 ` Eduardo Habkost
@ 2020-10-29 13:37 ` Igor Mammedov
0 siblings, 0 replies; 28+ messages in thread
From: Igor Mammedov @ 2020-10-29 13:37 UTC (permalink / raw)
To: Eduardo Habkost
Cc: Kevin Wolf, Daniel P. Berrangé,
qemu-devel, Markus Armbruster, Marc-André Lureau,
Paolo Bonzini, John Snow
On Thu, 29 Oct 2020 08:56:34 -0400
Eduardo Habkost <ehabkost@redhat.com> wrote:
> On Wed, Oct 28, 2020 at 04:22:40PM +0100, Paolo Bonzini wrote:
> > On 23/10/20 17:33, Igor Mammedov wrote:
> > > On Wed, 21 Oct 2020 09:30:41 -0400
> > > Eduardo Habkost <ehabkost@redhat.com> wrote:
> > >
> > >> On Wed, Oct 21, 2020 at 02:24:08PM +0200, Igor Mammedov wrote:
> > >>> On Fri, 9 Oct 2020 12:01:13 -0400
> > >>> Eduardo Habkost <ehabkost@redhat.com> wrote:
> > >>>
> > >>>> The existing object_class_property_add_uint*_ptr() functions are
> > >>>> not very useful, because they need a pointer to the property
> > >>>> value, which can't really be provided before the object is
> > >>>> created.
> > >>>>
> > >>>> Replace the pointer parameter in those functions with a
> > >>>> `ptrdiff_t offset` parameter.
> > >>>>
> > >>>> Include a uint8 class property in check-qom-proplist unit tests,
> > >>>> to ensure the feature is working.
> > >>>
> > >>>
> > >>> Not sure I like approach, it's reinventing qdev pointer properties in QOM form.
> > >>
> > >> Yes, and that's on purpose. If we want to eventually merge the
> > >> two competing APIs into a single one, we need to make them
> > >> converge.
> > >>
> > >>> I had an impression that Paolo wanted qdev pointer properties be gone
> > >>> and replaced by something like link properties.
> > >>
> > >> This is completely unrelated to qdev pointer properties and link
> > >> properties. The properties that use object_property_add_uint*_ptr()
> > >> today are not qdev pointer properties and will never be link
> > >> properties. They are just integer properties.
> >
> > I think this series a step in the right direction, but please take more
> > "inspiration" from link properties, which are done right. In
> > particular, properties should have an optional check function and be
> > read-only unless the check function is there.
> >
> > You can make the check function take an uint64_t for simplicity, so that
> > all the check functions for uint properties have the same prototype.
> > For example a single "property_check_uint_allow" function can allow
> > setting the property (which is almost always wrong, but an easy cop out
> > for this series).
>
> A property check callback that needs the property value is a more
> complex use case, and would require too much property-type-specific
> boilerplate today. I plan to address it, but not right now.
sounds good to me,
as long as user don't have deal with offsets directly and macro does
its type check thing.
> In my next series that makes static properties usable by any QOM
> object, I will add a separate "allow_set" callback to the
> internal QOM property API, which will not take the property value
> as argument. This would be enough for the dev->realized checks
> that are currently in qdev.
>
> Interestingly, there is only one link property check callback
> function in the QEMU tree that actually cares about the property
> value: isa_ipmi_bmc_check(). All other cases either don't care
> about the property value at all (qdev_prop_allow_set_link_before_realize(),
> object_property_allow_set_link()), or are being misused for
> something other than property checking (xlnx_dp_set_dpdma()).
>
^ permalink raw reply [flat|nested] 28+ messages in thread
end of thread, other threads:[~2020-10-29 13:38 UTC | newest]
Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-09 16:01 [PATCH 00/12] qom: Make all -object types use only class properties Eduardo Habkost
2020-10-09 16:01 ` [PATCH 01/12] qom: Helpers for pointer properties Eduardo Habkost
2020-10-09 16:01 ` [PATCH 02/12] qom: Introduce PointerProperty struct Eduardo Habkost
2020-10-09 16:01 ` [PATCH 03/12] qom: Make object_class_property_add_uint*_ptr() get offset Eduardo Habkost
2020-10-09 17:24 ` Eric Blake
2020-10-09 17:31 ` Eduardo Habkost
2020-10-21 12:24 ` Igor Mammedov
2020-10-21 13:30 ` Eduardo Habkost
2020-10-22 5:06 ` Markus Armbruster
2020-10-22 21:34 ` Eduardo Habkost
2020-10-23 15:33 ` Igor Mammedov
2020-10-27 22:18 ` Eduardo Habkost
2020-10-28 15:22 ` Paolo Bonzini
2020-10-28 15:53 ` Igor Mammedov
2020-10-29 12:56 ` Eduardo Habkost
2020-10-29 13:37 ` Igor Mammedov
2020-10-09 16:01 ` [PATCH 04/12] sev: Use class properties Eduardo Habkost
2020-10-09 16:01 ` [PATCH 05/12] rng: " Eduardo Habkost
2020-10-09 16:01 ` [PATCH 06/12] can_host: " Eduardo Habkost
2020-10-12 14:52 ` Pavel Pisa
2020-10-09 16:01 ` [PATCH 07/12] colo: " Eduardo Habkost
2020-10-09 16:01 ` [PATCH 08/12] netfilter: Reorder functions Eduardo Habkost
2020-10-09 16:01 ` [PATCH 09/12] netfilter: Use class properties Eduardo Habkost
2020-10-09 16:01 ` [PATCH 10/12] input: " Eduardo Habkost
2020-10-13 12:54 ` Gerd Hoffmann
2020-10-09 16:01 ` [PATCH 11/12] [RFC] qom: Property lock mechanism Eduardo Habkost
2020-10-09 16:01 ` [PATCH 12/12] [RFC] qom: Lock properties of all TYPE_USER_CREATABLE types Eduardo Habkost
2020-10-09 21:31 ` [PATCH] check-qom-proplist: Don't register instance props for user-creatable type Eduardo Habkost
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.