[Qemu-devel] [PATCH 4/7] qdev: Introduce DEFINE_PROP_LINK

Fam Zheng posted 7 patches 8 years, 7 months ago
There is a newer version of this series
[Qemu-devel] [PATCH 4/7] qdev: Introduce DEFINE_PROP_LINK
Posted by Fam Zheng 8 years, 7 months ago
This property can be used to replace the object_property_add_link in
device code, to add a link to other objects.

It's implemented by creating a wrapper property that basically forwards
operations to a QOM "link-FOO" property, which handles the check
callback and flags. The feature that is missing from QOM is the dynamic
child pointer which is done in the added code with the usual "offset"
approach.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 hw/core/qdev-properties.c    | 81 ++++++++++++++++++++++++++++++++++++++++++++
 include/hw/qdev-core.h       |  3 ++
 include/hw/qdev-properties.h | 11 ++++++
 3 files changed, 95 insertions(+)

diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 68cd653..3801379 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -1192,3 +1192,84 @@ PropertyInfo qdev_prop_size = {
     .set = set_size,
     .set_default_value = set_default_value_uint,
 };
+
+/* --- object link property --- */
+
+static ObjectProperty *link_property_get_or_create(Object *obj, Property *prop,
+                                                   Error **errp)
+{
+    Error *local_err = NULL;
+    Object **child = qdev_get_prop_ptr(DEVICE(obj), prop);
+    char *link_prop_name = g_strdup_printf("link-%s", prop->name);
+    ObjectProperty *op = object_property_find(obj, link_prop_name, NULL);
+
+    if (op) {
+        goto out;
+    }
+    object_property_add_link(obj, link_prop_name, prop->link.type,
+                             child, prop->link.check,
+                             prop->link.flags, &local_err);
+
+    if (local_err) {
+        error_propagate(errp, local_err);
+        goto out;
+    }
+    op = object_property_find(obj, link_prop_name, errp);
+out:
+    g_free(link_prop_name);
+    return op;
+}
+
+static void get_link_property(Object *obj, Visitor *v,
+                              const char *name, void *opaque,
+                              Error **errp)
+{
+    Property *prop = opaque;
+    ObjectProperty *op = link_property_get_or_create(obj, prop, NULL);
+    char *link_prop_name;
+
+    if (!op) {
+        return;
+    }
+    link_prop_name = g_strdup_printf("link-%s", name);
+    object_get_link_property(obj, v, link_prop_name, op->opaque, errp);
+    g_free(link_prop_name);
+}
+
+static void set_link_property(Object *obj, Visitor *v,
+                              const char *name, void *opaque,
+                              Error **errp)
+{
+    Object *target;
+    Property *prop = opaque;
+    Object **link_ptr = qdev_get_prop_ptr(DEVICE(obj), prop);
+    ObjectProperty *op = link_property_get_or_create(obj, prop, errp);
+    char *link_prop_name;
+    Error *local_err = NULL;
+
+    if (!op) {
+        return;
+    }
+
+    link_prop_name = g_strdup_printf("link-%s", name);
+    object_set_link_property(obj, v, link_prop_name, op->opaque, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        goto out;
+    }
+    target = object_property_get_link(obj, link_prop_name, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        goto out;
+    }
+    *link_ptr = target;
+
+out:
+    g_free(link_prop_name);
+}
+
+PropertyInfo qdev_prop_link = {
+    .name = "link",
+    .get = get_link_property,
+    .set = set_link_property,
+};
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 9d7c1c0..a520226 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -233,6 +233,9 @@ struct Property {
     int          arrayoffset;
     PropertyInfo *arrayinfo;
     int          arrayfieldsize;
+    /* Only @check and @flags are used; @child is unuseful because we need a
+     * dynamic pointer in @obj as derived from @offset. */
+    LinkProperty link;
 };
 
 struct PropertyInfo {
diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
index 39bf4b2..c9ccc9e 100644
--- a/include/hw/qdev-properties.h
+++ b/include/hw/qdev-properties.h
@@ -30,6 +30,7 @@ extern PropertyInfo qdev_prop_pci_devfn;
 extern PropertyInfo qdev_prop_blocksize;
 extern PropertyInfo qdev_prop_pci_host_devaddr;
 extern PropertyInfo qdev_prop_arraylen;
+extern PropertyInfo qdev_prop_link;
 
 #define DEFINE_PROP(_name, _state, _field, _prop, _type) { \
         .name      = (_name),                                    \
@@ -117,6 +118,16 @@ extern PropertyInfo qdev_prop_arraylen;
         .arrayoffset = offsetof(_state, _arrayfield),                   \
         }
 
+#define DEFINE_PROP_LINK(_name, _state, _field, _type, _check, _flags) {\
+        .name = (_name),                                                \
+        .info = &(qdev_prop_link),                                      \
+        .offset = offsetof(_state, _field)                              \
+            + type_check(Object *, typeof_field(_state, _field)),       \
+        .link.check = _check,                                           \
+        .link.flags = _flags,                                           \
+        .link.type  = _type,                                            \
+        }
+
 #define DEFINE_PROP_UINT8(_n, _s, _f, _d)                       \
     DEFINE_PROP_UNSIGNED(_n, _s, _f, _d, qdev_prop_uint8, uint8_t)
 #define DEFINE_PROP_UINT16(_n, _s, _f, _d)                      \
-- 
2.9.4


Re: [Qemu-devel] [PATCH 4/7] qdev: Introduce DEFINE_PROP_LINK
Posted by Paolo Bonzini 8 years, 7 months ago

On 28/06/2017 14:48, Fam Zheng wrote:
> 
> It's implemented by creating a wrapper property that basically forwards
> operations to a QOM "link-FOO" property, which handles the check
> callback and flags. The feature that is missing from QOM is the dynamic
> child pointer which is done in the added code with the usual "offset"
> approach.

Would it be possible to add a ".create" field to PropertyInfo?  Then
there's no need to introduce the wrapper.

Also, I think _check and _flags can be always
qdev_prop_allow_set_link_before_realize and
OBJ_PROP_LINK_UNREF_ON_RELEASE for DEFINE_PROP_LINK.

Paolo

Re: [Qemu-devel] [PATCH 4/7] qdev: Introduce DEFINE_PROP_LINK
Posted by Fam Zheng 8 years, 7 months ago
On Wed, 06/28 15:38, Paolo Bonzini wrote:
> 
> 
> On 28/06/2017 14:48, Fam Zheng wrote:
> > 
> > It's implemented by creating a wrapper property that basically forwards
> > operations to a QOM "link-FOO" property, which handles the check
> > callback and flags. The feature that is missing from QOM is the dynamic
> > child pointer which is done in the added code with the usual "offset"
> > approach.
> 
> Would it be possible to add a ".create" field to PropertyInfo?  Then
> there's no need to introduce the wrapper.

QOM setter/getter want a LinkProperty opaque pointer, but qdev uses a Property
pointer. I don't see a way to adapt that with .create(). Can you elaborate?

> 
> Also, I think _check and _flags can be always
> qdev_prop_allow_set_link_before_realize and
> OBJ_PROP_LINK_UNREF_ON_RELEASE for DEFINE_PROP_LINK.

A quick grep gave me some object_property_add_link() callers using different
parameters, but I don't know if they should use DEFINE_PROP_LINK.

Fam

Re: [Qemu-devel] [PATCH 4/7] qdev: Introduce DEFINE_PROP_LINK
Posted by Paolo Bonzini 8 years, 7 months ago

On 28/06/2017 16:02, Fam Zheng wrote:
>>> It's implemented by creating a wrapper property that basically forwards
>>> operations to a QOM "link-FOO" property, which handles the check
>>> callback and flags. The feature that is missing from QOM is the dynamic
>>> child pointer which is done in the added code with the usual "offset"
>>> approach.
>> Would it be possible to add a ".create" field to PropertyInfo?  Then
>> there's no need to introduce the wrapper.
> QOM setter/getter want a LinkProperty opaque pointer, but qdev uses a Property
> pointer. I don't see a way to adapt that with .create(). Can you elaborate?

The .create() callback would call object_property_add_link directly.
There would be no change in the properties at the QOM level, but the
PropertyInfo lets "info qtree" show the property.

Paolo

Re: [Qemu-devel] [PATCH 4/7] qdev: Introduce DEFINE_PROP_LINK
Posted by Fam Zheng 8 years, 7 months ago
On Wed, 06/28 16:06, Paolo Bonzini wrote:
> 
> 
> On 28/06/2017 16:02, Fam Zheng wrote:
> >>> It's implemented by creating a wrapper property that basically forwards
> >>> operations to a QOM "link-FOO" property, which handles the check
> >>> callback and flags. The feature that is missing from QOM is the dynamic
> >>> child pointer which is done in the added code with the usual "offset"
> >>> approach.
> >> Would it be possible to add a ".create" field to PropertyInfo?  Then
> >> there's no need to introduce the wrapper.
> > QOM setter/getter want a LinkProperty opaque pointer, but qdev uses a Property
> > pointer. I don't see a way to adapt that with .create(). Can you elaborate?
> 
> The .create() callback would call object_property_add_link directly.
> There would be no change in the properties at the QOM level, but the
> PropertyInfo lets "info qtree" show the property.

Thanks, I see, I'll try it in v2 if Andreas agrees on the general idea.

Fam