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

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

[Fam Zheng]

Link properties of devices created with object_property_add_link() are not
reflected in HMP "info qtree". For example, whether a virtio-blk device has an
iothread (i.e. has enabled data plane) can not be introspected easily.

Introduce a new type of qdev property to fix that.

Because QOM is not my most familiar area, only virtio devices are converted in
this version. If the new API is okay, other devices can be worked on either in
v2 or in a follow up series.

Fam Zheng (7):
  qom: Make link property API public
  qom: Handle property lookup failure in object_resolve_link
  qom: Save type name in LinkProperty
  qdev: Introduce DEFINE_PROP_LINK
  virtio-blk: Use DEFINE_PROP_LINK
  virtio-scsi: Use DEFINE_PROP_LINK
  virtio-rng: Use DEFINE_PROP_LINK

 hw/block/dataplane/virtio-blk.c |  2 +-
 hw/block/virtio-blk.c           |  7 ++--
 hw/core/qdev-properties.c       | 81 +++++++++++++++++++++++++++++++++++++++++
 hw/scsi/virtio-scsi-dataplane.c |  2 +-
 hw/scsi/virtio-scsi.c           | 15 ++------
 hw/virtio/virtio-pci.c          |  6 ---
 hw/virtio/virtio-rng.c          | 16 ++------
 include/hw/qdev-core.h          |  3 ++
 include/hw/qdev-properties.h    | 11 ++++++
 include/hw/virtio/virtio-blk.h  |  2 +-
 include/hw/virtio/virtio-rng.h  |  2 +-
 include/hw/virtio/virtio-scsi.h |  2 +-
 include/qom/object.h            | 14 +++++++
 qom/object.c                    | 24 ++++++------
 14 files changed, 136 insertions(+), 51 deletions(-)

-- 
2.9.4

[Qemu-devel] [PATCH 1/7] qom: Make link property API public

[Fam Zheng]

The get/set pair and the struct will be reused by qdev link prop, make
them public.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 include/qom/object.h | 13 +++++++++++++
 qom/object.c         | 18 ++++++------------
 2 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/include/qom/object.h b/include/qom/object.h
index abaeb8c..4659e6a 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -1613,4 +1613,17 @@ Object *container_get(Object *root, const char *path);
  * Returns the instance_size of the given @typename.
  */
 size_t object_type_get_instance_size(const char *typename);
+
+typedef struct {
+    Object **child;
+    void (*check)(Object *, const char *, Object *, Error **);
+    ObjectPropertyLinkFlags flags;
+} LinkProperty;
+
+void object_get_link_property(Object *obj, Visitor *v,
+                              const char *name, void *opaque,
+                              Error **errp);
+void object_set_link_property(Object *obj, Visitor *v,
+                              const char *name, void *opaque,
+                              Error **errp);
 #endif
diff --git a/qom/object.c b/qom/object.c
index 5f6fdfa..3868370 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -1434,15 +1434,9 @@ void object_property_allow_set_link(Object *obj, const char *name,
     /* Allow the link to be set, always */
 }
 
-typedef struct {
-    Object **child;
-    void (*check)(Object *, const char *, Object *, Error **);
-    ObjectPropertyLinkFlags flags;
-} LinkProperty;
-
-static void object_get_link_property(Object *obj, Visitor *v,
-                                     const char *name, void *opaque,
-                                     Error **errp)
+void object_get_link_property(Object *obj, Visitor *v,
+                              const char *name, void *opaque,
+                              Error **errp)
 {
     LinkProperty *lprop = opaque;
     Object **child = lprop->child;
@@ -1498,9 +1492,9 @@ static Object *object_resolve_link(Object *obj, const char *name,
     return target;
 }
 
-static void object_set_link_property(Object *obj, Visitor *v,
-                                     const char *name, void *opaque,
-                                     Error **errp)
+void object_set_link_property(Object *obj, Visitor *v,
+                              const char *name, void *opaque,
+                              Error **errp)
 {
     Error *local_err = NULL;
     LinkProperty *prop = opaque;
-- 
2.9.4

[Qemu-devel] [PATCH 2/7] qom: Handle property lookup failure in object_resolve_link

[Fam Zheng]

Since we have made object_set_link_property a public function, it is
possible that it will be called with a nonexistent property name.  Let's
survive this error case and report error to avoid segfault in the future.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 qom/object.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/qom/object.c b/qom/object.c
index 3868370..dd9c3fa 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -1470,7 +1470,10 @@ static Object *object_resolve_link(Object *obj, const char *name,
     Object *target;
 
     /* Go from link<FOO> to FOO.  */
-    type = object_property_get_type(obj, name, NULL);
+    type = object_property_get_type(obj, name, errp);
+    if (!type) {
+        return NULL;
+    }
     target_type = g_strndup(&type[5], strlen(type) - 6);
     target = object_resolve_path_type(path, target_type, &ambiguous);
 
-- 
2.9.4

[Qemu-devel] [PATCH 3/7] qom: Save type name in LinkProperty

[Fam Zheng]

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 include/qom/object.h | 1 +
 qom/object.c         | 1 +
 2 files changed, 2 insertions(+)

diff --git a/include/qom/object.h b/include/qom/object.h
index 4659e6a..262cd44 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -1618,6 +1618,7 @@ typedef struct {
     Object **child;
     void (*check)(Object *, const char *, Object *, Error **);
     ObjectPropertyLinkFlags flags;
+    const char *type;
 } LinkProperty;
 
 void object_get_link_property(Object *obj, Visitor *v,
diff --git a/qom/object.c b/qom/object.c
index dd9c3fa..b3073a6 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -1562,6 +1562,7 @@ void object_property_add_link(Object *obj, const char *name,
     prop->child = child;
     prop->check = check;
     prop->flags = flags;
+    prop->type = type;
 
     full_type = g_strdup_printf("link<%s>", type);
 
-- 
2.9.4

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

[Fam Zheng]

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

[Qemu-devel] [PATCH 5/7] virtio-blk: Use DEFINE_PROP_LINK

[Fam Zheng]

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 hw/block/dataplane/virtio-blk.c | 2 +-
 hw/block/virtio-blk.c           | 7 +++----
 hw/virtio/virtio-pci.c          | 2 --
 include/hw/virtio/virtio-blk.h  | 2 +-
 4 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
index 5556f0e..6fdc6f6 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -116,7 +116,7 @@ void virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *conf,
     s->conf = conf;
 
     if (conf->iothread) {
-        s->iothread = conf->iothread;
+        s->iothread = IOTHREAD(conf->iothread);
         object_ref(OBJECT(s->iothread));
         s->ctx = iothread_get_aio_context(s->iothread);
     } else {
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 604d37d..aa2c38c 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -981,10 +981,6 @@ static void virtio_blk_instance_init(Object *obj)
 {
     VirtIOBlock *s = VIRTIO_BLK(obj);
 
-    object_property_add_link(obj, "iothread", TYPE_IOTHREAD,
-                             (Object **)&s->conf.iothread,
-                             qdev_prop_allow_set_link_before_realize,
-                             OBJ_PROP_LINK_UNREF_ON_RELEASE, NULL);
     device_add_bootindex_property(obj, &s->conf.conf.bootindex,
                                   "bootindex", "/disk@0,0",
                                   DEVICE(obj), NULL);
@@ -1012,6 +1008,9 @@ static Property virtio_blk_properties[] = {
     DEFINE_PROP_BIT("request-merging", VirtIOBlock, conf.request_merging, 0,
                     true),
     DEFINE_PROP_UINT16("num-queues", VirtIOBlock, conf.num_queues, 1),
+    DEFINE_PROP_LINK("iothread", VirtIOBlock, conf.iothread, TYPE_IOTHREAD,
+                     qdev_prop_allow_set_link_before_realize,
+                     OBJ_PROP_LINK_UNREF_ON_RELEASE),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 20d6a08..e6960ae 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1996,8 +1996,6 @@ static void virtio_blk_pci_instance_init(Object *obj)
 
     virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
                                 TYPE_VIRTIO_BLK);
-    object_property_add_alias(obj, "iothread", OBJECT(&dev->vdev),"iothread",
-                              &error_abort);
     object_property_add_alias(obj, "bootindex", OBJECT(&dev->vdev),
                               "bootindex", &error_abort);
 }
diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h
index d3c8a6f..94a9f0c 100644
--- a/include/hw/virtio/virtio-blk.h
+++ b/include/hw/virtio/virtio-blk.h
@@ -33,7 +33,7 @@ struct virtio_blk_inhdr
 struct VirtIOBlkConf
 {
     BlockConf conf;
-    IOThread *iothread;
+    Object *iothread;
     char *serial;
     uint32_t scsi;
     uint32_t config_wce;
-- 
2.9.4

[Qemu-devel] [PATCH 6/7] virtio-scsi: Use DEFINE_PROP_LINK

[Fam Zheng]

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 hw/scsi/virtio-scsi-dataplane.c |  2 +-
 hw/scsi/virtio-scsi.c           | 15 ++++-----------
 hw/virtio/virtio-pci.c          |  2 --
 include/hw/virtio/virtio-scsi.h |  2 +-
 4 files changed, 6 insertions(+), 15 deletions(-)

diff --git a/hw/scsi/virtio-scsi-dataplane.c b/hw/scsi/virtio-scsi-dataplane.c
index 944ea4e..887c100 100644
--- a/hw/scsi/virtio-scsi-dataplane.c
+++ b/hw/scsi/virtio-scsi-dataplane.c
@@ -40,7 +40,7 @@ void virtio_scsi_dataplane_setup(VirtIOSCSI *s, Error **errp)
             error_setg(errp, "ioeventfd is required for iothread");
             return;
         }
-        s->ctx = iothread_get_aio_context(vs->conf.iothread);
+        s->ctx = iothread_get_aio_context(IOTHREAD(vs->conf.iothread));
     } else {
         if (!virtio_device_ioeventfd_enabled(vdev)) {
             return;
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index f46f06d..afe4389 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -897,16 +897,6 @@ static void virtio_scsi_device_realize(DeviceState *dev, Error **errp)
     virtio_scsi_dataplane_setup(s, errp);
 }
 
-static void virtio_scsi_instance_init(Object *obj)
-{
-    VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(obj);
-
-    object_property_add_link(obj, "iothread", TYPE_IOTHREAD,
-                             (Object **)&vs->conf.iothread,
-                             qdev_prop_allow_set_link_before_realize,
-                             OBJ_PROP_LINK_UNREF_ON_RELEASE, &error_abort);
-}
-
 void virtio_scsi_common_unrealize(DeviceState *dev, Error **errp)
 {
     VirtIODevice *vdev = VIRTIO_DEVICE(dev);
@@ -934,6 +924,10 @@ static Property virtio_scsi_properties[] = {
                                            VIRTIO_SCSI_F_HOTPLUG, true),
     DEFINE_PROP_BIT("param_change", VirtIOSCSI, host_features,
                                                 VIRTIO_SCSI_F_CHANGE, true),
+    DEFINE_PROP_LINK("iothread", VirtIOSCSI, parent_obj.conf.iothread,
+                     TYPE_IOTHREAD,
+                     qdev_prop_allow_set_link_before_realize,
+                     OBJ_PROP_LINK_UNREF_ON_RELEASE),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -988,7 +982,6 @@ static const TypeInfo virtio_scsi_info = {
     .name = TYPE_VIRTIO_SCSI,
     .parent = TYPE_VIRTIO_SCSI_COMMON,
     .instance_size = sizeof(VirtIOSCSI),
-    .instance_init = virtio_scsi_instance_init,
     .class_init = virtio_scsi_class_init,
     .interfaces = (InterfaceInfo[]) {
         { TYPE_HOTPLUG_HANDLER },
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index e6960ae..eb03ba5 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -2065,8 +2065,6 @@ static void virtio_scsi_pci_instance_init(Object *obj)
 
     virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
                                 TYPE_VIRTIO_SCSI);
-    object_property_add_alias(obj, "iothread", OBJECT(&dev->vdev), "iothread",
-                              &error_abort);
 }
 
 static const TypeInfo virtio_scsi_pci_info = {
diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h
index de6ae5a..1decc40 100644
--- a/include/hw/virtio/virtio-scsi.h
+++ b/include/hw/virtio/virtio-scsi.h
@@ -56,7 +56,7 @@ struct VirtIOSCSIConf {
 #endif
     CharBackend chardev;
     uint32_t boot_tpgt;
-    IOThread *iothread;
+    Object *iothread;
 };
 
 struct VirtIOSCSI;
-- 
2.9.4

[Qemu-devel] [PATCH 7/7] virtio-rng: Use DEFINE_PROP_LINK

[Fam Zheng]

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 hw/virtio/virtio-pci.c         |  2 --
 hw/virtio/virtio-rng.c         | 16 ++++------------
 include/hw/virtio/virtio-rng.h |  2 +-
 3 files changed, 5 insertions(+), 15 deletions(-)

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index eb03ba5..0938db4 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -2459,8 +2459,6 @@ static void virtio_rng_initfn(Object *obj)
 
     virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
                                 TYPE_VIRTIO_RNG);
-    object_property_add_alias(obj, "rng", OBJECT(&dev->vdev), "rng",
-                              &error_abort);
 }
 
 static const TypeInfo virtio_rng_pci_info = {
diff --git a/hw/virtio/virtio-rng.c b/hw/virtio/virtio-rng.c
index a6ee501..218778b 100644
--- a/hw/virtio/virtio-rng.c
+++ b/hw/virtio/virtio-rng.c
@@ -199,7 +199,7 @@ static void virtio_rng_device_realize(DeviceState *dev, Error **errp)
                                  "rng", NULL);
     }
 
-    vrng->rng = vrng->conf.rng;
+    vrng->rng = RNG_BACKEND(vrng->conf.rng);
     if (vrng->rng == NULL) {
         error_setg(errp, "'rng' parameter expects a valid object");
         return;
@@ -246,6 +246,9 @@ static Property virtio_rng_properties[] = {
      */
     DEFINE_PROP_UINT64("max-bytes", VirtIORNG, conf.max_bytes, INT64_MAX),
     DEFINE_PROP_UINT32("period", VirtIORNG, conf.period_ms, 1 << 16),
+    DEFINE_PROP_LINK("rng", VirtIORNG, conf.rng, TYPE_RNG_BACKEND,
+                     qdev_prop_allow_set_link_before_realize,
+                     OBJ_PROP_LINK_UNREF_ON_RELEASE),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -262,21 +265,10 @@ static void virtio_rng_class_init(ObjectClass *klass, void *data)
     vdc->get_features = get_features;
 }
 
-static void virtio_rng_initfn(Object *obj)
-{
-    VirtIORNG *vrng = VIRTIO_RNG(obj);
-
-    object_property_add_link(obj, "rng", TYPE_RNG_BACKEND,
-                             (Object **)&vrng->conf.rng,
-                             qdev_prop_allow_set_link_before_realize,
-                             OBJ_PROP_LINK_UNREF_ON_RELEASE, NULL);
-}
-
 static const TypeInfo virtio_rng_info = {
     .name = TYPE_VIRTIO_RNG,
     .parent = TYPE_VIRTIO_DEVICE,
     .instance_size = sizeof(VirtIORNG),
-    .instance_init = virtio_rng_initfn,
     .class_init = virtio_rng_class_init,
 };
 
diff --git a/include/hw/virtio/virtio-rng.h b/include/hw/virtio/virtio-rng.h
index 922dce7..8d45597 100644
--- a/include/hw/virtio/virtio-rng.h
+++ b/include/hw/virtio/virtio-rng.h
@@ -23,7 +23,7 @@
         OBJECT_GET_PARENT_CLASS(obj, TYPE_VIRTIO_RNG)
 
 struct VirtIORNGConf {
-    RngBackend *rng;
+    Object *rng;
     uint64_t max_bytes;
     uint32_t period_ms;
     RngRandom *default_backend;
-- 
2.9.4

Re: [Qemu-devel] [PATCH 1/7] qom: Make link property API public

[Paolo Bonzini]

On 28/06/2017 14:48, Fam Zheng wrote:
> The get/set pair and the struct will be reused by qdev link prop, make
> them public.
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>

Maybe it's better to make it a separate header file.  No other objections.

Paolo

> ---
>  include/qom/object.h | 13 +++++++++++++
>  qom/object.c         | 18 ++++++------------
>  2 files changed, 19 insertions(+), 12 deletions(-)
> 
> diff --git a/include/qom/object.h b/include/qom/object.h
> index abaeb8c..4659e6a 100644
> --- a/include/qom/object.h
> +++ b/include/qom/object.h
> @@ -1613,4 +1613,17 @@ Object *container_get(Object *root, const char *path);
>   * Returns the instance_size of the given @typename.
>   */
>  size_t object_type_get_instance_size(const char *typename);
> +
> +typedef struct {
> +    Object **child;
> +    void (*check)(Object *, const char *, Object *, Error **);
> +    ObjectPropertyLinkFlags flags;
> +} LinkProperty;
> +
> +void object_get_link_property(Object *obj, Visitor *v,
> +                              const char *name, void *opaque,
> +                              Error **errp);
> +void object_set_link_property(Object *obj, Visitor *v,
> +                              const char *name, void *opaque,
> +                              Error **errp);
>  #endif
> diff --git a/qom/object.c b/qom/object.c
> index 5f6fdfa..3868370 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -1434,15 +1434,9 @@ void object_property_allow_set_link(Object *obj, const char *name,
>      /* Allow the link to be set, always */
>  }
>  
> -typedef struct {
> -    Object **child;
> -    void (*check)(Object *, const char *, Object *, Error **);
> -    ObjectPropertyLinkFlags flags;
> -} LinkProperty;
> -
> -static void object_get_link_property(Object *obj, Visitor *v,
> -                                     const char *name, void *opaque,
> -                                     Error **errp)
> +void object_get_link_property(Object *obj, Visitor *v,
> +                              const char *name, void *opaque,
> +                              Error **errp)
>  {
>      LinkProperty *lprop = opaque;
>      Object **child = lprop->child;
> @@ -1498,9 +1492,9 @@ static Object *object_resolve_link(Object *obj, const char *name,
>      return target;
>  }
>  
> -static void object_set_link_property(Object *obj, Visitor *v,
> -                                     const char *name, void *opaque,
> -                                     Error **errp)
> +void object_set_link_property(Object *obj, Visitor *v,
> +                              const char *name, void *opaque,
> +                              Error **errp)
>  {
>      Error *local_err = NULL;
>      LinkProperty *prop = opaque;
> 

Re: [Qemu-devel] [PATCH 1/7] qom: Make link property API public

[Andreas Färber]

Am 28.06.2017 um 15:32 schrieb Paolo Bonzini:
> On 28/06/2017 14:48, Fam Zheng wrote:
>> The get/set pair and the struct will be reused by qdev link prop, make
>> them public.
>>
>> Signed-off-by: Fam Zheng <famz@redhat.com>
> 
> Maybe it's better to make it a separate header file.  No other objections.

So you think it should be added?

The alternative would be moving info qom-tree forward and finally
deprecating info qtree. There was a patch of mine to make it list
properties with -v or so, but there were StringOutputVisitor
dependencies where it got stuck in review.

Regards,
Andreas

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

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

[Paolo Bonzini]

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

[Fam Zheng]

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

[Paolo Bonzini]

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 1/7] qom: Make link property API public

[Fam Zheng]

On Wed, 06/28 15:36, Andreas Färber wrote:
> The alternative would be moving info qom-tree forward and finally
> deprecating info qtree. There was a patch of mine to make it list
> properties with -v or so, but there were StringOutputVisitor
> dependencies where it got stuck in review.

No objection to enhancing info qom-tree, but also no idea how to help. :)

BTW, will it print much more information than "info qtree"? If so deprecating it
sounds like a bad idea for me. "info qtree" output is already lengthy. As HMP is
for human, it would be nice if the output is easy to extract interesting
information with eyeball, rather than dumping everything.

Fam

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

[Fam Zheng]

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