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

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

[Fam Zheng]

v2: Create a new header for link properties. [Paolo]
    Don't wrap, use PropertyInfo.create() (much better diffstat, yay!).
    [Paolo]

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 macro to fix that.

Fam Zheng (7):
  qom: Make link property API public
  qom: Handle property lookup failure in object_resolve_link
  qdev: Introduce PropertyInfo.create
  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       | 16 ++++++++++++++++
 hw/core/qdev.c                  | 31 +++++++++++++++++++------------
 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          |  6 ++++++
 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/link-property.h     | 31 +++++++++++++++++++++++++++++++
 qom/object.c                    | 24 +++++++++++-------------
 15 files changed, 110 insertions(+), 63 deletions(-)
 create mode 100644 include/qom/link-property.h

-- 
2.9.4

[Qemu-devel] [PATCH v2 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/link-property.h | 31 +++++++++++++++++++++++++++++++
 qom/object.c                | 19 +++++++------------
 2 files changed, 38 insertions(+), 12 deletions(-)
 create mode 100644 include/qom/link-property.h

diff --git a/include/qom/link-property.h b/include/qom/link-property.h
new file mode 100644
index 0000000..f3c3816
--- /dev/null
+++ b/include/qom/link-property.h
@@ -0,0 +1,31 @@
+/*
+ * QOM link property
+ *
+ * Copyright IBM, Corp. 2011
+ *
+ * Authors:
+ *  Anthony Liguori   <aliguori@us.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#ifndef QOM_LINK_PROPERTY_H
+#define QOM_LINK_PROPERTY_H
+
+#include "qom/object.h"
+
+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..fdb8f0d 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -28,6 +28,7 @@
 #include "qapi/qmp/qobject.h"
 #include "qapi/qmp/qbool.h"
 #include "qapi/qmp/qstring.h"
+#include "qom/link-property.h"
 
 #define MAX_INTERFACES 32
 
@@ -1434,15 +1435,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 +1493,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 v2 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 fdb8f0d..7ce97d9 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -1471,7 +1471,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 v2 3/7] qdev: Introduce PropertyInfo.create

[Fam Zheng]

This allows property implementation to provide a specialized property
creation method.

Update conditions guarding property types accordingly.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 hw/core/qdev.c         | 31 +++++++++++++++++++------------
 include/hw/qdev-core.h |  1 +
 2 files changed, 20 insertions(+), 12 deletions(-)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 849952a..ec63fe0 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -744,6 +744,10 @@ static void qdev_property_add_legacy(DeviceState *dev, Property *prop,
         return;
     }
 
+    if (prop->info->create) {
+        return;
+    }
+
     name = g_strdup_printf("legacy-%s", prop->name);
     object_property_add(OBJECT(dev), name, "str",
                         prop->info->print ? qdev_get_legacy_property : prop->info->get,
@@ -770,20 +774,23 @@ void qdev_property_add_static(DeviceState *dev, Property *prop,
     Error *local_err = NULL;
     Object *obj = OBJECT(dev);
 
-    /*
-     * TODO qdev_prop_ptr does not have getters or setters.  It must
-     * go now that it can be replaced with links.  The test should be
-     * removed along with it: all static properties are read/write.
-     */
-    if (!prop->info->get && !prop->info->set) {
-        return;
+    if (prop->info->create) {
+        prop->info->create(obj, prop, &local_err);
+    } else {
+        /*
+         * TODO qdev_prop_ptr does not have getters or setters.  It must
+         * go now that it can be replaced with links.  The test should be
+         * removed along with it: all static properties are read/write.
+         */
+        if (!prop->info->get && !prop->info->set) {
+            return;
+        }
+        object_property_add(obj, prop->name, prop->info->name,
+                            prop->info->get, prop->info->set,
+                            prop->info->release,
+                            prop, &local_err);
     }
 
-    object_property_add(obj, prop->name, prop->info->name,
-                        prop->info->get, prop->info->set,
-                        prop->info->release,
-                        prop, &local_err);
-
     if (local_err) {
         error_propagate(errp, local_err);
         return;
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 9d7c1c0..33518ee 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -241,6 +241,7 @@ struct PropertyInfo {
     const char * const *enum_table;
     int (*print)(DeviceState *dev, Property *prop, char *dest, size_t len);
     void (*set_default_value)(Object *obj, const Property *prop);
+    void (*create)(Object *obj, Property *prop, Error **errp);
     ObjectPropertyAccessor *get;
     ObjectPropertyAccessor *set;
     ObjectPropertyRelease *release;
-- 
2.9.4

[Qemu-devel] [PATCH v2 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, which is a common pattern.

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

diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 68cd653..7c11eb8 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -1192,3 +1192,19 @@ PropertyInfo qdev_prop_size = {
     .set = set_size,
     .set_default_value = set_default_value_uint,
 };
+
+/* --- object link property --- */
+
+static void create_link_property(Object *obj, Property *prop, Error **errp)
+{
+    Object **child = qdev_get_prop_ptr(DEVICE(obj), prop);
+
+    object_property_add_link(obj, prop->name, prop->link_type,
+                             child, prop->link.check,
+                             prop->link.flags, errp);
+}
+
+PropertyInfo qdev_prop_link = {
+    .name = "link",
+    .create = create_link_property,
+};
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 33518ee..40afb3d 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -5,6 +5,7 @@
 #include "qemu/option.h"
 #include "qemu/bitmap.h"
 #include "qom/object.h"
+#include "qom/link-property.h"
 #include "hw/irq.h"
 #include "hw/hotplug.h"
 
@@ -233,6 +234,10 @@ 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;
+    const char   *link_type;
 };
 
 struct PropertyInfo {
diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
index 39bf4b2..767c10b 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 v2 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 v2 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 v2 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 v2 4/7] qdev: Introduce DEFINE_PROP_LINK

[Igor Mammedov]

On Thu, 29 Jun 2017 16:04:49 +0800
Fam Zheng <famz@redhat.com> wrote:

> This property can be used to replace the object_property_add_link in
> device code, to add a link to other objects, which is a common pattern.
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  hw/core/qdev-properties.c    | 16 ++++++++++++++++
>  include/hw/qdev-core.h       |  5 +++++
>  include/hw/qdev-properties.h | 11 +++++++++++
>  3 files changed, 32 insertions(+)
> 
> diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
> index 68cd653..7c11eb8 100644
> --- a/hw/core/qdev-properties.c
> +++ b/hw/core/qdev-properties.c
> @@ -1192,3 +1192,19 @@ PropertyInfo qdev_prop_size = {
>      .set = set_size,
>      .set_default_value = set_default_value_uint,
>  };
> +
> +/* --- object link property --- */
> +
> +static void create_link_property(Object *obj, Property *prop, Error **errp)
> +{
> +    Object **child = qdev_get_prop_ptr(DEVICE(obj), prop);
> +
> +    object_property_add_link(obj, prop->name, prop->link_type,
> +                             child, prop->link.check,
> +                             prop->link.flags, errp);
> +}
> +
> +PropertyInfo qdev_prop_link = {
> +    .name = "link",
> +    .create = create_link_property,
> +};
> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> index 33518ee..40afb3d 100644
> --- a/include/hw/qdev-core.h
> +++ b/include/hw/qdev-core.h
> @@ -5,6 +5,7 @@
>  #include "qemu/option.h"
>  #include "qemu/bitmap.h"
>  #include "qom/object.h"
> +#include "qom/link-property.h"
>  #include "hw/irq.h"
>  #include "hw/hotplug.h"
>  
> @@ -233,6 +234,10 @@ 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. */
 @check, @flags, @child, @obj are not fields of struct Property so it's
not clear what doc comments talks about. Maybe adding prefixes would help,
for example:
  @link.child

> +    LinkProperty link;
> +    const char   *link_type;
>  };
>  
>  struct PropertyInfo {
> diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
> index 39bf4b2..767c10b 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,                                           \
maybe we shouldn't have custom _check and _flags fields as majority of devices
use qdev_prop_allow_set_link_before_realize + OBJ_PROP_LINK_UNREF_ON_RELEASE
policies. That will save us ~2LOC per property of boiler plate code.

I've looked at current device link usage and there is only few that actually
use or need custom check/flags and several are misusing object_property_allow_set_link()
where they should use qdev_prop_allow_set_link_before_realize().

We could leave alone exceptions that require custom check/flags as is
or provide DEFINE_PROP_LINK_CUSTOM() for a few that actually need it and fix
ones that misuse it.

There is a bunch of board code that uses links but that's not for Device
so it's not related to this macro anyways.

> +        .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)                      \

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

[Fam Zheng]

On Thu, 06/29 12:40, Igor Mammedov wrote:
> On Thu, 29 Jun 2017 16:04:49 +0800
> Fam Zheng <famz@redhat.com> wrote:
> 
> > This property can be used to replace the object_property_add_link in
> > device code, to add a link to other objects, which is a common pattern.
> > 
> > Signed-off-by: Fam Zheng <famz@redhat.com>
> > ---
> >  hw/core/qdev-properties.c    | 16 ++++++++++++++++
> >  include/hw/qdev-core.h       |  5 +++++
> >  include/hw/qdev-properties.h | 11 +++++++++++
> >  3 files changed, 32 insertions(+)
> > 
> > diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
> > index 68cd653..7c11eb8 100644
> > --- a/hw/core/qdev-properties.c
> > +++ b/hw/core/qdev-properties.c
> > @@ -1192,3 +1192,19 @@ PropertyInfo qdev_prop_size = {
> >      .set = set_size,
> >      .set_default_value = set_default_value_uint,
> >  };
> > +
> > +/* --- object link property --- */
> > +
> > +static void create_link_property(Object *obj, Property *prop, Error **errp)
> > +{
> > +    Object **child = qdev_get_prop_ptr(DEVICE(obj), prop);
> > +
> > +    object_property_add_link(obj, prop->name, prop->link_type,
> > +                             child, prop->link.check,
> > +                             prop->link.flags, errp);
> > +}
> > +
> > +PropertyInfo qdev_prop_link = {
> > +    .name = "link",
> > +    .create = create_link_property,
> > +};
> > diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> > index 33518ee..40afb3d 100644
> > --- a/include/hw/qdev-core.h
> > +++ b/include/hw/qdev-core.h
> > @@ -5,6 +5,7 @@
> >  #include "qemu/option.h"
> >  #include "qemu/bitmap.h"
> >  #include "qom/object.h"
> > +#include "qom/link-property.h"
> >  #include "hw/irq.h"
> >  #include "hw/hotplug.h"
> >  
> > @@ -233,6 +234,10 @@ 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. */
>  @check, @flags, @child, @obj are not fields of struct Property so it's
> not clear what doc comments talks about. Maybe adding prefixes would help,
> for example:
>   @link.child

Good point, I was being too stingy with words.

> 
> > +    LinkProperty link;
> > +    const char   *link_type;
> >  };
> >  
> >  struct PropertyInfo {
> > diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
> > index 39bf4b2..767c10b 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,                                           \
> maybe we shouldn't have custom _check and _flags fields as majority of devices
> use qdev_prop_allow_set_link_before_realize + OBJ_PROP_LINK_UNREF_ON_RELEASE
> policies. That will save us ~2LOC per property of boiler plate code.
> 
> I've looked at current device link usage and there is only few that actually
> use or need custom check/flags and several are misusing object_property_allow_set_link()
> where they should use qdev_prop_allow_set_link_before_realize().
> 
> We could leave alone exceptions that require custom check/flags as is
> or provide DEFINE_PROP_LINK_CUSTOM() for a few that actually need it and fix
> ones that misuse it.
> 
> There is a bunch of board code that uses links but that's not for Device
> so it's not related to this macro anyways.

OK, I wasn't sure about this when Paolo pointed out in v1, but since you've now
taken a look, I will simplify it.

I will also try to cover more devices in v3. Thanks!

Fam

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

[Paolo Bonzini]

On 29/06/2017 10:04, Fam Zheng wrote:
> +#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,                                            \
> +        }
> +

Still unsure about _check; qdev_prop_allow_set_link_before_realize is
mimicking the same behavior of any other qdev property, so it should be
always okay for DEFINE_PROP_LINK.

Paolo

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

[Fam Zheng]

On Thu, 06/29 13:51, Paolo Bonzini wrote:
> 
> 
> On 29/06/2017 10:04, Fam Zheng wrote:
> > +#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,                                            \
> > +        }
> > +
> 
> Still unsure about _check; qdev_prop_allow_set_link_before_realize is
> mimicking the same behavior of any other qdev property, so it should be
> always okay for DEFINE_PROP_LINK.

OK, let's drop Property.link and use constant check and flags. A
DEFINE_PROP_LINK_FULL can always be added later.

Fam