[Qemu-devel] [PATCH v3 00/20] qom: dynamic properties and composition tree

[Qemu-devel] [PATCH v3 00/20] qom: dynamic properties and composition tree

[Anthony Liguori]

This is a follow up to my previous series to get us started in the QOM
direction.  A few things are different this time around.  Most notably:

 1) Devices no longer have names.  Instead, path names are always used to
    identify devices.

 2) In order to support (1), dynamic properties are now supported.

 3) The concept of a "root device" has been introduced.  The root device is
    roughly modelling the motherboard of a machine.  This type is a container
    type and it's best to think of it as something like a PCB board I guess.

See my other mail for a description of my merge plan for QOM.

To try it out, here's an example session:

Launch:

anthony@titi:~/build/qemu$ x86_64-softmmu/qemu-system-x86_64 -hda ~/images/linux.img -snapshot -device virtio-balloon-pci,id=foo -qmp unix:/tmp/server.sock,server,nowait

Explore the object model:

anthony@titi:~/git/qemu/QMP$ ./qom-list /
peripheral/
i440fx/
anthony@titi:~/git/qemu/QMP$ ./qom-list /i440fx/
piix3/
anthony@titi:~/git/qemu/QMP$ ./qom-list /i440fx/piix3
rtc/
anthony@titi:~/git/qemu/QMP$ ./qom-list /i440fx/piix3/rtc
date
base_year
anthony@titi:~/git/qemu/QMP$ ./qom-get /i440fx/piix3/rtc.date
tm_sec: 33
tm_hour: 21
tm_mday: 30
tm_year: 111
tm_mon: 10
tm_min: 2
anthony@titi:~/git/qemu/QMP$ ./qom-get rtc.date
tm_sec: 38
tm_hour: 21
tm_mday: 30
tm_year: 111
tm_mon: 10
tm_min: 2
anthony@titi:~/git/qemu/QMP$ ./qom-list /peripheral
foo/
anthony@titi:~/git/qemu/QMP$ ./qom-list /peripheral/foo
event_idx
indirect_desc

Anthony Liguori (20):
  qom: add a reference count to qdev objects
  qom: add new dynamic property infrastructure based on Visitors (v2)
  qom: register legacy properties as new style properties (v2)
  qom: introduce root device
  qdev: provide an interface to return canonical path from root (v2)
  qdev: provide a path resolution (v2)
  qom: add child properties (composition) (v3)
  qom: add link properties (v2)
  qapi: allow a 'gen' key to suppress code generation
  qmp: add qom-list command
  qom: qom_{get,set} monitor commands (v2)
  qdev: add explicitly named devices to the root complex
  dev: add an anonymous peripheral container
  rtc: make piix3 set the rtc as a child (v2)
  rtc: add a dynamic property for retrieving the date
  qom: optimize qdev_get_canonical_path using a parent link
  qom: add vga node to the pc composition tree
  qom: add string property type
  qdev: add a qdev_get_type() function and expose as a 'type' property
  pc: fill out most of the composition tree

 Makefile.objs            |    2 +-
 hw/acpi_piix4.c          |    7 +-
 hw/cirrus_vga.c          |    8 +-
 hw/container.c           |   20 ++
 hw/ide/pci.c             |   11 +-
 hw/kvmclock.c            |    5 +-
 hw/kvmclock.h            |    5 +-
 hw/mc146818rtc.c         |   27 +++
 hw/mips_malta.c          |    5 +-
 hw/pc.c                  |   75 +++++--
 hw/pc.h                  |   51 +++--
 hw/pc_piix.c             |   61 ++++--
 hw/piix_pci.c            |   11 +-
 hw/qdev.c                |  555 +++++++++++++++++++++++++++++++++++++++++++++-
 hw/qdev.h                |  281 +++++++++++++++++++++++
 hw/usb-uhci.c            |    4 +-
 hw/usb-uhci.h            |    2 +-
 hw/vga-pci.c             |    5 +-
 hw/vmware_vga.h          |    6 +-
 monitor.h                |    4 +
 qapi-schema.json         |  107 +++++++++
 qerror.c                 |    4 +
 qerror.h                 |    3 +
 qmp-commands.hx          |   18 ++
 qmp.c                    |   93 ++++++++
 scripts/qapi-commands.py |    1 +
 scripts/qapi-types.py    |    1 +
 27 files changed, 1289 insertions(+), 83 deletions(-)
 create mode 100644 hw/container.c

-- 
1.7.4.1

[Qemu-devel] [PATCH v3 01/20] qom: add a reference count to qdev objects

[Anthony Liguori]

To ensure that a device isn't removed from the graph until all of its links are
broken.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 hw/qdev.c |   16 ++++++++++++++++
 hw/qdev.h |   26 ++++++++++++++++++++++++++
 2 files changed, 42 insertions(+), 0 deletions(-)

diff --git a/hw/qdev.c b/hw/qdev.c
index 106407f..fdc9843 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -323,6 +323,11 @@ int qdev_unplug(DeviceState *dev)
     }
     assert(dev->info->unplug != NULL);
 
+    if (dev->ref != 0) {
+        qerror_report(QERR_DEVICE_IN_USE, dev->id?:"");
+        return -1;
+    }
+
     qdev_hot_removed = true;
 
     return dev->info->unplug(dev);
@@ -962,3 +967,14 @@ char* qdev_get_fw_dev_path(DeviceState *dev)
 
     return strdup(path);
 }
+
+void qdev_ref(DeviceState *dev)
+{
+    dev->ref++;
+}
+
+void qdev_unref(DeviceState *dev)
+{
+    g_assert(dev->ref > 0);
+    dev->ref--;
+}
diff --git a/hw/qdev.h b/hw/qdev.h
index 36a4198..2397b4e 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -45,6 +45,12 @@ struct DeviceState {
     QTAILQ_ENTRY(DeviceState) sibling;
     int instance_id_alias;
     int alias_required_for_version;
+
+    /**
+     * This tracks the number of references between devices.  See @qdev_ref for
+     * more information.
+     */
+    uint32_t ref;
 };
 
 typedef void (*bus_dev_printfn)(Monitor *mon, DeviceState *dev, int indent);
@@ -329,4 +335,24 @@ char *qdev_get_fw_dev_path(DeviceState *dev);
 /* This is a nasty hack to allow passing a NULL bus to qdev_create.  */
 extern struct BusInfo system_bus_info;
 
+/**
+ * @qdev_ref
+ *
+ * Increase the reference count of a device.  A device cannot be freed as long
+ * as its reference count is greater than zero.
+ *
+ * @dev - the device
+ */
+void qdev_ref(DeviceState *dev);
+
+/**
+ * @qdef_unref
+ *
+ * Decrease the reference count of a device.  A device cannot be freed as long
+ * as its reference count is greater than zero.
+ *
+ * @dev - the device
+ */
+void qdev_unref(DeviceState *dev);
+
 #endif
-- 
1.7.4.1

[Qemu-devel] [PATCH v3 02/20] qom: add new dynamic property infrastructure based on Visitors (v2)

[Anthony Liguori]

qdev properties are settable only during construction and static to classes.
This isn't flexible enough for QOM.

This patch introduces a property interface for qdev that provides dynamic
properties that are tied to objects, instead of classes.  These properties are
Visitor based instead of string based too.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
v1 -> v2
 - Etter -> Accessor (Stefan)
 - spelling mistakes (Stefan)
 - switch to qemu-queue (Kevin/Gerd)
---
 hw/qdev.c |   97 +++++++++++++++++++++++++++++++++++++++++++++++++
 hw/qdev.h |  120 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 qerror.c  |    4 ++
 qerror.h  |    3 ++
 4 files changed, 224 insertions(+), 0 deletions(-)

diff --git a/hw/qdev.c b/hw/qdev.c
index fdc9843..94f14c1 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -98,6 +98,7 @@ static DeviceState *qdev_create_from_info(BusState *bus, DeviceInfo *info)
         qdev_hot_added = true;
     }
     dev->instance_id_alias = -1;
+    QTAILQ_INIT(&dev->properties);
     dev->state = DEV_STATE_CREATED;
     return dev;
 }
@@ -395,12 +396,31 @@ void qdev_init_nofail(DeviceState *dev)
     }
 }
 
+static void qdev_property_del_all(DeviceState *dev)
+{
+    while (!QTAILQ_EMPTY(&dev->properties)) {
+        DeviceProperty *prop = QTAILQ_FIRST(&dev->properties);
+
+        QTAILQ_REMOVE(&dev->properties, prop, node);
+
+        if (prop->release) {
+            prop->release(dev, prop->name, prop->opaque);
+        }
+
+        g_free(prop->name);
+        g_free(prop->type);
+        g_free(prop);
+    }
+}
+
 /* Unlink device from bus and free the structure.  */
 void qdev_free(DeviceState *dev)
 {
     BusState *bus;
     Property *prop;
 
+    qdev_property_del_all(dev);
+
     if (dev->state == DEV_STATE_INITIALIZED) {
         while (dev->num_child_bus) {
             bus = QLIST_FIRST(&dev->child_bus);
@@ -978,3 +998,80 @@ void qdev_unref(DeviceState *dev)
     g_assert(dev->ref > 0);
     dev->ref--;
 }
+
+void qdev_property_add(DeviceState *dev, const char *name, const char *type,
+                       DevicePropertyAccessor *get, DevicePropertyAccessor *set,
+                       DevicePropertyRelease *release,
+                       void *opaque, Error **errp)
+{
+    DeviceProperty *prop = g_malloc0(sizeof(*prop));
+
+    prop->name = g_strdup(name);
+    prop->type = g_strdup(type);
+
+    prop->get = get;
+    prop->set = set;
+    prop->release = release;
+    prop->opaque = opaque;
+
+    QTAILQ_INSERT_TAIL(&dev->properties, prop, node);
+}
+
+static DeviceProperty *qdev_property_find(DeviceState *dev, const char *name)
+{
+    DeviceProperty *prop;
+
+    QTAILQ_FOREACH(prop, &dev->properties, node) {
+        if (strcmp(prop->name, name) == 0) {
+            return prop;
+        }
+    }
+
+    return NULL;
+}
+
+void qdev_property_get(DeviceState *dev, Visitor *v, const char *name,
+                       Error **errp)
+{
+    DeviceProperty *prop = qdev_property_find(dev, name);
+
+    if (prop == NULL) {
+        error_set(errp, QERR_PROPERTY_NOT_FOUND, dev->id?:"", name);
+        return;
+    }
+
+    if (!prop->get) {
+        error_set(errp, QERR_PERMISSION_DENIED);
+    } else {
+        prop->get(dev, v, prop->opaque, name, errp);
+    }
+}
+
+void qdev_property_set(DeviceState *dev, Visitor *v, const char *name,
+                       Error **errp)
+{
+    DeviceProperty *prop = qdev_property_find(dev, name);
+
+    if (prop == NULL) {
+        error_set(errp, QERR_PROPERTY_NOT_FOUND, dev->id?:"", name);
+        return;
+    }
+
+    if (!prop->set) {
+        error_set(errp, QERR_PERMISSION_DENIED);
+    } else {
+        prop->set(dev, prop->opaque, v, name, errp);
+    }
+}
+
+const char *qdev_property_get_type(DeviceState *dev, const char *name, Error **errp)
+{
+    DeviceProperty *prop = qdev_property_find(dev, name);
+
+    if (prop == NULL) {
+        error_set(errp, QERR_PROPERTY_NOT_FOUND, dev->id?:"", name);
+        return NULL;
+    }
+
+    return prop->type;
+}
diff --git a/hw/qdev.h b/hw/qdev.h
index 2397b4e..2df3bb7 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -5,6 +5,7 @@
 #include "qemu-queue.h"
 #include "qemu-char.h"
 #include "qemu-option.h"
+#include "qapi/qapi-visit-core.h"
 
 typedef struct Property Property;
 
@@ -27,6 +28,44 @@ enum {
     DEV_NVECTORS_UNSPECIFIED = -1,
 };
 
+/**
+ * @DevicePropertyAccessor - called when trying to get/set a property
+ *
+ * @dev the device that owns the property
+ * @v the visitor that contains the property data
+ * @opaque the device property opaque
+ * @name the name of the property
+ * @errp a pointer to an Error that is filled if getting/setting fails.
+ */
+typedef void (DevicePropertyAccessor)(DeviceState *dev,
+                                      Visitor *v,
+                                      void *opaque,
+                                      const char *name,
+                                      Error **errp);
+
+/**
+ * @DevicePropertyRelease - called when a property is removed from a device
+ *
+ * @dev the device that owns the property
+ * @name the name of the property
+ * @opaque the opaque registered with the property
+ */
+typedef void (DevicePropertyRelease)(DeviceState *dev,
+                                     const char *name,
+                                     void *opaque);
+
+typedef struct DeviceProperty
+{
+    gchar *name;
+    gchar *type;
+    DevicePropertyAccessor *get;
+    DevicePropertyAccessor *set;
+    DevicePropertyRelease *release;
+    void *opaque;
+
+    QTAILQ_ENTRY(DeviceProperty) node;
+} DeviceProperty;
+
 /* This structure should not be accessed directly.  We declare it here
    so that it can be embedded in individual device state structures.  */
 struct DeviceState {
@@ -51,6 +90,8 @@ struct DeviceState {
      * more information.
      */
     uint32_t ref;
+
+    QTAILQ_HEAD(, DeviceProperty) properties;
 };
 
 typedef void (*bus_dev_printfn)(Monitor *mon, DeviceState *dev, int indent);
@@ -355,4 +396,83 @@ void qdev_ref(DeviceState *dev);
  */
 void qdev_unref(DeviceState *dev);
 
+/**
+ * @qdev_property_add - add a new property to a device
+ *
+ * @dev - the device to add a property to
+ *
+ * @name - the name of the property.  This can contain any character except for
+ *         a forward slash.  In general, you should use hyphens '-' instead of
+ *         underscores '_' when naming properties.
+ *
+ * @type - the type name of the property.  This namespace is pretty loosely
+ *         defined.  Sub namespaces are constructed by using a prefix and then
+ *         to angle brackets.  For instance, the type 'virtio-net-pci' in the
+ *         'link' namespace would be 'link<virtio-net-pci>'.
+ *
+ * @get - the getter to be called to read a property.  If this is NULL, then
+ *        the property cannot be read.
+ *
+ * @set - the setter to be called to write a property.  If this is NULL,
+ *        then the property cannot be written.
+ *
+ * @release - called when the property is removed from the device.  This is
+ *            meant to allow a property to free its opaque upon device
+ *            destruction.  This may be NULL.
+ *
+ * @opaque - an opaque pointer to pass to the callbacks for the property
+ *
+ * @errp - returns an error if this function fails
+ */
+void qdev_property_add(DeviceState *dev, const char *name, const char *type,
+                       DevicePropertyAccessor *get, DevicePropertyAccessor *set,
+                       DevicePropertyRelease *release,
+                       void *opaque, Error **errp);
+
+/**
+ * @qdev_property_get - reads a property from a device
+ *
+ * @dev - the device
+ *
+ * @v - the visitor that will receive the property value.  This should be an
+ *      Output visitor and the data will be written with @name as the name.
+ *
+ * @name - the name of the property
+ *
+ * @errp - returns an error if this function fails
+ */
+void qdev_property_get(DeviceState *dev, Visitor *v, const char *name,
+                       Error **errp);
+
+/**
+ * @qdev_property_set - writes a property to a device
+ *
+ * @dev - the device
+ *
+ * @v - the visitor that will be used to write the property value.  This should
+ *      be an Input visitor and the data will be first read with @name as the
+ *      name and then written as the property value.
+ *
+ * @name - the name of the property
+ *
+ * @errp - returns an error if this function fails
+ */
+void qdev_property_set(DeviceState *dev, Visitor *v, const char *name,
+                       Error **errp);
+
+/**
+ * @qdev_property_get_type - returns the type of a property
+ *
+ * @dev - the device
+ *
+ * @name - the name of the property
+ *
+ * @errp - returns an error if this function fails
+ *
+ * Returns:
+ *   The type name of the property.
+ */
+const char *qdev_property_get_type(DeviceState *dev, const char *name,
+                                   Error **errp);
+
 #endif
diff --git a/qerror.c b/qerror.c
index b544ced..ad491d1 100644
--- a/qerror.c
+++ b/qerror.c
@@ -182,6 +182,10 @@ static const QErrorStringTable qerror_table[] = {
         .desc      = "Could not open '%(filename)'",
     },
     {
+        .error_fmt = QERR_PERMISSION_DENIED,
+        .desc      = "Insufficient permission to perform this operation",
+    },
+    {
         .error_fmt = QERR_PROPERTY_NOT_FOUND,
         .desc      = "Property '%(device).%(property)' not found",
     },
diff --git a/qerror.h b/qerror.h
index 1fee39d..89780c5 100644
--- a/qerror.h
+++ b/qerror.h
@@ -153,6 +153,9 @@ QError *qobject_to_qerror(const QObject *obj);
 #define QERR_OPEN_FILE_FAILED \
     "{ 'class': 'OpenFileFailed', 'data': { 'filename': %s } }"
 
+#define QERR_PERMISSION_DENIED \
+    "{ 'class': 'PermissionDenied', 'data': {} }"
+
 #define QERR_PROPERTY_NOT_FOUND \
     "{ 'class': 'PropertyNotFound', 'data': { 'device': %s, 'property': %s } }"
 
-- 
1.7.4.1

[Qemu-devel] [PATCH v3 03/20] qom: register legacy properties as new style properties (v2)

[Anthony Liguori]

Expose all legacy properties through the new QOM property mechanism.  The qdev
property types are exposed through the 'legacy<>' namespace.  They are always
visited as strings since they do their own string parsing.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
v1 -> v2
 - add bus properties (Gerd)
---
 hw/qdev.c |   86 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/qdev.h |    7 +++++
 2 files changed, 93 insertions(+), 0 deletions(-)

diff --git a/hw/qdev.c b/hw/qdev.c
index 94f14c1..6f77af9 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -83,6 +83,7 @@ static DeviceInfo *qdev_find_info(BusInfo *bus_info, const char *name)
 static DeviceState *qdev_create_from_info(BusState *bus, DeviceInfo *info)
 {
     DeviceState *dev;
+    Property *prop;
 
     assert(bus->info == info->bus_info);
     dev = g_malloc0(info->size);
@@ -100,6 +101,15 @@ static DeviceState *qdev_create_from_info(BusState *bus, DeviceInfo *info)
     dev->instance_id_alias = -1;
     QTAILQ_INIT(&dev->properties);
     dev->state = DEV_STATE_CREATED;
+
+    for (prop = dev->info->props; prop && prop->name; prop++) {
+        qdev_property_add_legacy(dev, prop, NULL);
+    }
+
+    for (prop = dev->info->bus_info->props; prop && prop->name; prop++) {
+        qdev_property_add_legacy(dev, prop, NULL);
+    }
+
     return dev;
 }
 
@@ -1075,3 +1085,79 @@ const char *qdev_property_get_type(DeviceState *dev, const char *name, Error **e
 
     return prop->type;
 }
+
+/**
+ * Legacy property handling
+ */
+
+static void qdev_get_legacy_property(DeviceState *dev, Visitor *v, void *opaque,
+                                     const char *name, Error **errp)
+{
+    Property *prop = opaque;
+
+    if (prop->info->print) {
+        char buffer[1024];
+        char *ptr = buffer;
+
+        prop->info->print(dev, prop, buffer, sizeof(buffer));
+        visit_type_str(v, &ptr, name, errp);
+    } else {
+        error_set(errp, QERR_PERMISSION_DENIED);
+    }
+}
+
+static void qdev_set_legacy_property(DeviceState *dev, Visitor *v, void *opaque,
+                                     const char *name, Error **errp)
+{
+    Property *prop = opaque;
+
+    if (dev->state != DEV_STATE_CREATED) {
+        error_set(errp, QERR_PERMISSION_DENIED);
+        return;
+    }
+
+    if (prop->info->parse) {
+        Error *local_err = NULL;
+        char *ptr = NULL;
+
+        visit_type_str(v, &ptr, name, &local_err);
+        if (!local_err) {
+            int ret;
+            ret = prop->info->parse(dev, prop, ptr);
+            if (ret != 0) {
+                error_set(errp, QERR_INVALID_PARAMETER_VALUE,
+                          name, prop->info->name);
+            }
+            g_free(ptr);
+        } else {
+            error_propagate(errp, local_err);
+        }
+    } else {
+        error_set(errp, QERR_PERMISSION_DENIED);
+    }
+}
+
+/**
+ * @qdev_add_legacy_property - adds a legacy property
+ *
+ * Do not use this is new code!  Properties added through this interface will
+ * be given types in the "legacy<>" type namespace.
+ *
+ * Legacy properties are always processed as strings.  The format of the string
+ * depends on the property type.
+ */
+void qdev_property_add_legacy(DeviceState *dev, Property *prop,
+                              Error **errp)
+{
+    gchar *type;
+
+    type = g_strdup_printf("legacy<%s>", prop->info->name);
+
+    qdev_property_add(dev, prop->name, type,
+                      qdev_get_legacy_property,
+                      qdev_set_legacy_property,
+                      NULL,
+                      prop, errp);
+
+    g_free(type);
+}
diff --git a/hw/qdev.h b/hw/qdev.h
index 2df3bb7..3b629d4 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -475,4 +475,11 @@ void qdev_property_set(DeviceState *dev, Visitor *v, const char *name,
 const char *qdev_property_get_type(DeviceState *dev, const char *name,
                                    Error **errp);
 
+/**
+ * @qdev_property_add_legacy - add a legacy @Property to a device
+ *
+ * DO NOT USE THIS IN NEW CODE!
+ */
+void qdev_property_add_legacy(DeviceState *dev, Property *prop, Error **errp);
+
 #endif
-- 
1.7.4.1

[Qemu-devel] [PATCH v3 04/20] qom: introduce root device

[Anthony Liguori]

This is based on Jan's suggestion for how to do unique naming.  The root device
is the root of composition.  All devices are reachable via child<> links from
this device.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 Makefile.objs  |    2 +-
 hw/container.c |   20 ++++++++++++++++++++
 hw/qdev.c      |   12 ++++++++++++
 hw/qdev.h      |    8 ++++++++
 4 files changed, 41 insertions(+), 1 deletions(-)
 create mode 100644 hw/container.c

diff --git a/Makefile.objs b/Makefile.objs
index 3a699ee..2290515 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -279,7 +279,7 @@ hw-obj-$(CONFIG_LSI_SCSI_PCI) += lsi53c895a.o
 hw-obj-$(CONFIG_ESP) += esp.o
 
 hw-obj-y += dma-helpers.o sysbus.o isa-bus.o
-hw-obj-y += qdev-addr.o
+hw-obj-y += qdev-addr.o container.o
 
 # VGA
 hw-obj-$(CONFIG_VGA_PCI) += vga-pci.o
diff --git a/hw/container.c b/hw/container.c
new file mode 100644
index 0000000..9cbf399
--- /dev/null
+++ b/hw/container.c
@@ -0,0 +1,20 @@
+#include "sysbus.h"
+
+static int container_initfn(SysBusDevice *dev)
+{
+    return 0;
+}
+
+static SysBusDeviceInfo container_info = {
+    .init = container_initfn,
+    .qdev.name = "container",
+    .qdev.size = sizeof(SysBusDevice),
+    .qdev.no_user = 1,
+};
+
+static void container_init(void)
+{
+    sysbus_register_withprop(&container_info);
+}
+
+device_init(container_init);
diff --git a/hw/qdev.c b/hw/qdev.c
index 6f77af9..bb0b9f7 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -1161,3 +1161,15 @@ void qdev_property_add_legacy(DeviceState *dev, Property *prop,
 
     g_free(type);
 }
+
+DeviceState *qdev_get_root(void)
+{
+    static DeviceState *qdev_root;
+
+    if (!qdev_root) {
+        qdev_root = qdev_create(NULL, "container");
+        qdev_init_nofail(qdev_root);
+    }
+
+    return qdev_root;
+}
diff --git a/hw/qdev.h b/hw/qdev.h
index 3b629d4..52aadd2 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -482,4 +482,12 @@ const char *qdev_property_get_type(DeviceState *dev, const char *name,
  */
 void qdev_property_add_legacy(DeviceState *dev, Property *prop, Error **errp);
 
+/**
+ * @qdev_get_root - returns the root device of the composition tree
+ *
+ * Returns:
+ *   The root of the composition tree.
+ */
+DeviceState *qdev_get_root(void);
+
 #endif
-- 
1.7.4.1

[Qemu-devel] [PATCH v3 05/20] qdev: provide an interface to return canonical path from root (v2)

[Anthony Liguori]

The canonical path is the path in the composition tree from the root to the
device.  This is effectively the name of the device.

This is an incredibly unefficient implementation that will be optimized in
a future patch.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
v1 -> v2
 - change from gslist to qemu-queue (Kevin/Gerd)
---
 hw/qdev.c |   48 ++++++++++++++++++++++++++++++++++++++++++++++++
 hw/qdev.h |    9 +++++++++
 2 files changed, 57 insertions(+), 0 deletions(-)

diff --git a/hw/qdev.c b/hw/qdev.c
index bb0b9f7..79849c9 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -1173,3 +1173,51 @@ DeviceState *qdev_get_root(void)
 
     return qdev_root;
 }
+
+static gchar *qdev_get_path_in(DeviceState *parent, DeviceState *dev)
+{
+    DeviceProperty *prop;
+
+    if (parent == dev) {
+        return g_strdup("");
+    }
+
+    QTAILQ_FOREACH(prop, &parent->properties, node) {
+        gchar *subpath;
+
+        if (!strstart(prop->type, "child<", NULL)) {
+            continue;
+        }
+
+        /* Check to see if the device is one of parent's children */
+        if (prop->opaque == dev) {
+            return g_strdup(prop->name);
+        }
+
+        /* Check to see if the device is a child of our child */
+        subpath = qdev_get_path_in(prop->opaque, dev);
+        if (subpath) {
+            gchar *path;
+
+            path = g_strdup_printf("%s/%s", prop->name, subpath);
+            g_free(subpath);
+
+            return path;
+        }
+    }
+
+    return NULL;
+}
+
+gchar *qdev_get_canonical_path(DeviceState *dev)
+{
+    gchar *path, *newpath;
+
+    path = qdev_get_path_in(qdev_get_root(), dev);
+    g_assert(path != NULL);
+
+    newpath = g_strdup_printf("/%s", path);
+    g_free(path);
+
+    return newpath;
+}
diff --git a/hw/qdev.h b/hw/qdev.h
index 52aadd2..0f00497 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -490,4 +490,13 @@ void qdev_property_add_legacy(DeviceState *dev, Property *prop, Error **errp);
  */
 DeviceState *qdev_get_root(void);
 
+/**
+ * @qdev_get_canonical_path - returns the canonical path for a device.  This
+ * is the path within the composition tree starting from the root.
+ *
+ * Returns:
+ *   The canonical path in the composition tree.
+ */
+gchar *qdev_get_canonical_path(DeviceState *dev);
+
 #endif
-- 
1.7.4.1

[Qemu-devel] [PATCH v3 06/20] qdev: provide a path resolution (v2)

[Anthony Liguori]

There are two types of supported paths--absolute paths and partial paths.

Absolute paths are derived from the root device and can follow child<> or
link<> properties.  Since they can follow link<> properties, they can be
arbitrarily long.  Absolute paths look like absolute filenames and are prefixed
with a leading slash.

Partial paths are look like relative filenames.  They do not begin with a
prefix.  The matching rules for partial paths are subtle but designed to make
specifying devices easy.  At each level of the composition tree, the partial
path is matched as an absolute path.  The first match is not returned.  At
least two matches are searched for.  A successful result is only returned if
only one match is founded.  If more than one match is found, a flag is returned
to indicate that the match was ambiguous.

At the end of the day, partial path support means that if you create a device
called 'ide0', you can just say 'ide0' as the path name and it will Just Work.
If we internally create a device called 'i440fx', you can just say 'i440fx' and
it will Just Work and long as you don't do anything silly.

A management tool should probably always use absolute paths since then they
don't have to deal with the possibility of ambiguity.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
v1 -> v2
 - fix comments (Stefan)
 - always initialize ambiguous to false (Stefan)
 - change from gslist to qemu-queue (Kevin/Gerd)
---
 hw/qdev.c |  103 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/qdev.h |   28 ++++++++++++++++
 2 files changed, 131 insertions(+), 0 deletions(-)

diff --git a/hw/qdev.c b/hw/qdev.c
index 79849c9..2519f00 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -1221,3 +1221,106 @@ gchar *qdev_get_canonical_path(DeviceState *dev)
 
     return newpath;
 }
+
+static DeviceState *qdev_resolve_abs_path(DeviceState *parent,
+                                          gchar **parts,
+                                          int index)
+{
+    DeviceProperty *prop;
+    DeviceState *child;
+
+    if (parts[index] == NULL) {
+        return parent;
+    }
+
+    if (strcmp(parts[index], "") == 0) {
+        return qdev_resolve_abs_path(parent, parts, index + 1);
+    }
+
+    prop = qdev_property_find(parent, parts[index]);
+    if (prop == NULL) {
+        return NULL;
+    }
+
+    child = NULL;
+    if (strstart(prop->type, "link<", NULL)) {
+        DeviceState **pchild = prop->opaque;
+        if (*pchild) {
+            child = *pchild;
+        }
+    } else if (strstart(prop->type, "child<", NULL)) {
+        child = prop->opaque;
+    }
+
+    if (!child) {
+        return NULL;
+    }
+
+    return qdev_resolve_abs_path(child, parts, index + 1);
+}
+
+static DeviceState *qdev_resolve_partial_path(DeviceState *parent,
+                                              gchar **parts,
+                                              bool *ambiguous)
+{
+    DeviceState *dev;
+    DeviceProperty *prop;
+
+    dev = qdev_resolve_abs_path(parent, parts, 0);
+
+    QTAILQ_FOREACH(prop, &parent->properties, node) {
+        DeviceState *found;
+
+        if (!strstart(prop->type, "child<", NULL)) {
+            continue;
+        }
+
+        found = qdev_resolve_partial_path(prop->opaque, parts, ambiguous);
+        if (found) {
+            if (dev) {
+                if (ambiguous) {
+                    *ambiguous = true;
+                }
+                return NULL;
+            }
+            dev = found;
+        }
+
+        if (ambiguous && *ambiguous) {
+            return NULL;
+        }
+    }
+
+    return dev;
+}
+
+DeviceState *qdev_resolve_path(const char *path, bool *ambiguous)
+{
+    bool partial_path = true;
+    DeviceState *dev;
+    gchar **parts;
+
+    parts = g_strsplit(path, "/", 0);
+    if (parts == NULL || parts[0] == NULL) {
+        g_strfreev(parts);
+        return qdev_get_root();
+    }
+
+    if (strcmp(parts[0], "") == 0) {
+        partial_path = false;
+    }
+
+    if (partial_path) {
+        if (ambiguous) {
+            *ambiguous = false;
+        }
+        dev = qdev_resolve_partial_path(qdev_get_root(), parts, ambiguous);
+    } else {
+        dev = qdev_resolve_abs_path(qdev_get_root(), parts, 1);
+    }
+
+    g_strfreev(parts);
+
+    return dev;
+}
+
diff --git a/hw/qdev.h b/hw/qdev.h
index 0f00497..641d134 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -499,4 +499,32 @@ DeviceState *qdev_get_root(void);
  */
 gchar *qdev_get_canonical_path(DeviceState *dev);
 
+/**
+ * @qdev_resolve_path - resolves a path returning a device
+ *
+ * There are two types of supported paths--absolute paths and partial paths.
+ * 
+ * Absolute paths are derived from the root device and can follow child<> or
+ * link<> properties.  Since they can follow link<> properties, they can be
+ * arbitrarily long.  Absolute paths look like absolute filenames and are
+ * prefixed with a leading slash.
+ * 
+ * Partial paths look like relative filenames.  They do not begin with a
+ * prefix.  The matching rules for partial paths are subtle but designed to make
+ * specifying devices easy.  At each level of the composition tree, the partial
+ * path is matched as an absolute path.  The first match is not returned.  At
+ * least two matches are searched for.  A successful result is only returned if
+ * only one match is founded.  If more than one match is found, a flag is
+ * return to indicate that the match was ambiguous.
+ *
+ * @path - the path to resolve
+ *
+ * @ambiguous - returns true if the path resolution failed because of an
+ *              ambiguous match
+ *
+ * Returns:
+ *   The matched device or NULL on path lookup failure.
+ */
+DeviceState *qdev_resolve_path(const char *path, bool *ambiguous);
+
 #endif
-- 
1.7.4.1

[Qemu-devel] [PATCH v3 07/20] qom: add child properties (composition) (v3)

[Anthony Liguori]

Child properties express a relationship of composition.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
v1 -> v2
 - fix comments (Kevin)
 - add a reference when adding a child property (Kevin)
 - fix bug spotted by Paolo on reference
---
 hw/qdev.c |   26 ++++++++++++++++++++++++++
 hw/qdev.h |   20 ++++++++++++++++++++
 2 files changed, 46 insertions(+), 0 deletions(-)

diff --git a/hw/qdev.c b/hw/qdev.c
index 2519f00..167f459 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -1174,6 +1174,32 @@ DeviceState *qdev_get_root(void)
     return qdev_root;
 }
 
+static void qdev_get_child_property(DeviceState *dev, Visitor *v, void *opaque,
+                                    const char *name, Error **errp)
+{
+    DeviceState *child = opaque;
+    gchar *path;
+
+    path = qdev_get_canonical_path(child);
+    visit_type_str(v, &path, name, errp);
+    g_free(path);
+}
+
+void qdev_property_add_child(DeviceState *dev, const char *name,
+                             DeviceState *child, Error **errp)
+{
+    gchar *type;
+
+    type = g_strdup_printf("child<%s>", child->info->name);
+
+    qdev_property_add(dev, name, type, qdev_get_child_property,
+                      NULL, NULL, child, errp);
+
+    qdev_ref(child);
+
+    g_free(type);
+}
+
 static gchar *qdev_get_path_in(DeviceState *parent, DeviceState *dev)
 {
     DeviceProperty *prop;
diff --git a/hw/qdev.h b/hw/qdev.h
index 641d134..38b36e8 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -527,4 +527,24 @@ gchar *qdev_get_canonical_path(DeviceState *dev);
  */
 DeviceState *qdev_resolve_path(const char *path, bool *ambiguous);
 
+/**
+ * @qdev_property_add_child - Add a child property to a device
+ *
+ * Child properties form the composition tree.  All devices need to be a child
+ * of another device.  Devices can only be a child of one device.
+ *
+ * There is no way for a child to determine what its parent is.  It is not
+ * a bidirectional relationship.  This is by design.
+ *
+ * @dev - the device to add a property to
+ *
+ * @name - the name of the property
+ *
+ * @child - the child device
+ *
+ * @errp - if an error occurs, a pointer to an area to store the area
+ */
+void qdev_property_add_child(DeviceState *dev, const char *name,
+                             DeviceState *child, Error **errp);
+
 #endif
-- 
1.7.4.1

[Qemu-devel] [PATCH v3 08/20] qom: add link properties (v2)

[Anthony Liguori]

Links represent an ephemeral relationship between devices.  They are meant to
replace the qdev concept of busses by allowing more informal relationships
between devices.

Links are fairly limited in their usefulness without implementing QOM-style
subclassing and interfaces.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
v1 -> v2
 - comments (Stefan)
 - maintain a reference when adding/removing a link property (Kevin)
---
 hw/qdev.c |   74 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/qdev.h |   23 +++++++++++++++++++
 2 files changed, 97 insertions(+), 0 deletions(-)

diff --git a/hw/qdev.c b/hw/qdev.c
index 167f459..d448275 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -1200,6 +1200,80 @@ void qdev_property_add_child(DeviceState *dev, const char *name,
     g_free(type);
 }
 
+static void qdev_get_link_property(DeviceState *dev, Visitor *v, void *opaque,
+                                   const char *name, Error **errp)
+{
+    DeviceState **child = opaque;
+    gchar *path;
+
+    if (*child) {
+        path = qdev_get_canonical_path(*child);
+        visit_type_str(v, &path, name, errp);
+        g_free(path);
+    } else {
+        path = (gchar *)"";
+        visit_type_str(v, &path, name, errp);
+    }
+}
+
+static void qdev_set_link_property(DeviceState *dev, Visitor *v, void *opaque,
+                                   const char *name, Error **errp)
+{
+    DeviceState **child = opaque;
+    bool ambiguous = false;
+    const char *type;
+    char *path;
+
+    type = qdev_property_get_type(dev, name, NULL);
+
+    visit_type_str(v, &path, name, errp);
+
+    if (*child) {
+        qdev_unref(*child);
+    }
+
+    if (strcmp(path, "") != 0) {
+        DeviceState *target;
+
+        target = qdev_resolve_path(path, &ambiguous);
+        if (target) {
+            gchar *target_type;
+
+            target_type = g_strdup_printf("link<%s>", target->info->name);
+            if (strcmp(target_type, type) == 0) {
+                *child = target;
+                qdev_ref(target);
+            } else {
+                error_set(errp, QERR_INVALID_PARAMETER_TYPE, name, type);
+            }
+
+            g_free(target_type);
+        } else {
+            error_set(errp, QERR_DEVICE_NOT_FOUND, path);
+        }
+    } else {
+        *child = NULL;
+    }
+
+    g_free(path);
+}
+
+void qdev_property_add_link(DeviceState *dev, const char *name,
+                            const char *type, DeviceState **child,
+                            Error **errp)
+{
+    gchar *full_type;
+
+    full_type = g_strdup_printf("link<%s>", type);
+
+    qdev_property_add(dev, name, full_type,
+                      qdev_get_link_property,
+                      qdev_set_link_property,
+                      NULL, child, errp);
+
+    g_free(full_type);
+}
+
 static gchar *qdev_get_path_in(DeviceState *parent, DeviceState *dev)
 {
     DeviceProperty *prop;
diff --git a/hw/qdev.h b/hw/qdev.h
index 38b36e8..4351e2e 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -547,4 +547,27 @@ DeviceState *qdev_resolve_path(const char *path, bool *ambiguous);
 void qdev_property_add_child(DeviceState *dev, const char *name,
                              DeviceState *child, Error **errp);
 
+/**
+ * @qdev_property_add_link - Add a link property to a device
+ *
+ * Links establish relationships between devices.  Links are unidirectional
+ * although two links can be combined to form a bidirectional relationship
+ * between devices.
+ *
+ * Links form the graph in the device model.
+ *
+ * @dev - the device to add a property to
+ *
+ * @name - the name of the property
+ *
+ * @type - the qdev type of the link
+ *
+ * @child - a pointer to where the link device reference is stored
+ *
+ * @errp - if an error occurs, a pointer to an area to store the area
+ */
+void qdev_property_add_link(DeviceState *dev, const char *name,
+                            const char *type, DeviceState **child,
+                            Error **errp);
+
 #endif
-- 
1.7.4.1

[Qemu-devel] [PATCH v3 09/20] qapi: allow a 'gen' key to suppress code generation

[Anthony Liguori]

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 scripts/qapi-commands.py |    1 +
 scripts/qapi-types.py    |    1 +
 2 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index f7def16..54d1f5d 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -405,6 +405,7 @@ except os.error, e:
 
 exprs = parse_schema(sys.stdin)
 commands = filter(lambda expr: expr.has_key('command'), exprs)
+commands = filter(lambda expr: not expr.has_key('gen'), commands)
 
 if dispatch_type == "sync":
     fdecl = open(h_file, 'w')
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index f64d84c..267cb49 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -238,6 +238,7 @@ fdecl.write(mcgen('''
                   guard=guardname(h_file)))
 
 exprs = parse_schema(sys.stdin)
+exprs = filter(lambda expr: not expr.has_key('gen'), exprs)
 
 for expr in exprs:
     ret = "\n"
-- 
1.7.4.1

[Qemu-devel] [PATCH v3 10/20] qmp: add qom-list command

[Anthony Liguori]

This can be used to list properties in the device model.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 qapi-schema.json |   48 ++++++++++++++++++++++++++++++++++++++++++++++++
 qmp-commands.hx  |    6 ++++++
 qmp.c            |   28 ++++++++++++++++++++++++++++
 3 files changed, 82 insertions(+), 0 deletions(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index f358b49..7c979b2 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1168,3 +1168,51 @@
 # Since: 0.14.0
 ##
 { 'command': 'migrate_set_speed', 'data': {'value': 'int'} }
+
+##
+# @DevicePropertyInfo:
+#
+# @name: the name of the property
+#
+# @type: the type of the property.  This will typically come in one of four
+#        forms:
+#
+#        1) A primitive type such as 'u8', 'u16', 'bool', 'str', or 'double'.
+#           These types are mapped to the appropriate JSON type.
+#
+#        2) A legacy type in the form 'legacy<subtype>' where subtype is the
+#           legacy qdev typename.  These types are always treated as strings.
+#
+#        3) A child type in the form 'child<subtype>' where subtype is a qdev
+#           device type name.  Child properties create the composition tree.
+#
+#        4) A link type in the form 'link<subtype>' where subtype is a qdev
+#           device type name.  Link properties form the device model graph.
+#
+# Since: 1.1
+#
+# Notes: This type is experimental.  Its syntax may change in future releases.
+##
+{ 'type': 'DevicePropertyInfo',
+  'data': { 'name': 'str', 'type': 'str' } }
+
+##
+# @qom-list:
+#
+# This command will list any properties of a device given a path in the device
+# model.
+#
+# @path: the path within the device model.  See @qom-get for a description of
+#        this parameter.
+#
+# Returns: a list of @DevicePropertyInfo that describe the properties of the
+#          device.
+#
+# Since: 1.1
+#
+# Notes: This command is experimental.  It's syntax may change in future
+#        releases.
+##
+{ 'command': 'qom-list',
+  'data': { 'path': 'str' },
+  'returns': [ 'DevicePropertyInfo' ] }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 002e7e8..c1f0718 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -2009,3 +2009,9 @@ EQMP
         .args_type  = "",
         .mhandler.cmd_new = qmp_marshal_input_query_balloon,
     },
+
+    {
+        .name       = "qom-list",
+        .args_type  = "path:s",
+        .mhandler.cmd_new = qmp_marshal_input_qom_list,
+    },
diff --git a/qmp.c b/qmp.c
index d71ceb4..8e9a595 100644
--- a/qmp.c
+++ b/qmp.c
@@ -16,6 +16,7 @@
 #include "qmp-commands.h"
 #include "kvm.h"
 #include "arch_init.h"
+#include "hw/qdev.h"
 
 NameInfo *qmp_query_name(Error **errp)
 {
@@ -154,3 +155,30 @@ void qmp_cont(Error **errp)
 
     vm_start();
 }
+
+DevicePropertyInfoList *qmp_qom_list(const char *path, Error **errp)
+{
+    DeviceState *dev;
+    bool ambiguous = false;
+    DevicePropertyInfoList *props = NULL;
+    DeviceProperty *prop;
+
+    dev = qdev_resolve_path(path, &ambiguous);
+    if (dev == NULL) {
+        error_set(errp, QERR_DEVICE_NOT_FOUND, path);
+        return NULL;
+    }
+
+    QTAILQ_FOREACH(prop, &dev->properties, node) {
+        DevicePropertyInfoList *entry = g_malloc0(sizeof(*entry));
+
+        entry->value = g_malloc0(sizeof(DevicePropertyInfo));
+        entry->next = props;
+        props = entry;
+
+        entry->value->name = g_strdup(prop->name);
+        entry->value->type = g_strdup(prop->type);
+    }
+
+    return props;
+}
-- 
1.7.4.1

[Qemu-devel] [PATCH v3 11/20] qom: qom_{get, set} monitor commands (v2)

[Anthony Liguori]

This allows clients to read and write device model properties through QMP.  QAPI
doesn't support Visitor types yet and these commands are special in that they
don't work with fixed types.

I've added a documentation stub to qapi-schema.json so we can keep consistency
there.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
v1 -> v2
 - comments (Stefan)
---
 monitor.h        |    4 +++
 qapi-schema.json |   59 +++++++++++++++++++++++++++++++++++++++++++++++++
 qmp-commands.hx  |   12 ++++++++++
 qmp.c            |   65 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 140 insertions(+), 0 deletions(-)

diff --git a/monitor.h b/monitor.h
index 052f1cb..cfa2f67 100644
--- a/monitor.h
+++ b/monitor.h
@@ -67,4 +67,8 @@ typedef void (MonitorCompletion)(void *opaque, QObject *ret_data);
 
 void monitor_set_error(Monitor *mon, QError *qerror);
 
+int qmp_qom_set(Monitor *mon, const QDict *qdict, QObject **ret);
+
+int qmp_qom_get(Monitor *mon, const QDict *qdict, QObject **ret);
+
 #endif /* !MONITOR_H */
diff --git a/qapi-schema.json b/qapi-schema.json
index 7c979b2..44cf764 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1216,3 +1216,62 @@
 { 'command': 'qom-list',
   'data': { 'path': 'str' },
   'returns': [ 'DevicePropertyInfo' ] }
+
+##
+# @qom-get:
+#
+# This command will get a property from a device model path and return the
+# value.
+#
+# @path: The path within the device model.  There are two forms of supported
+#        paths--absolute and partial paths.
+#
+#        Absolute paths are derived from the root device and can follow child<>
+#        or link<> properties.  Since they can follow link<> properties, they
+#        can be arbitrarily long.  Absolute paths look like absolute filenames
+#        and are prefixed  with a leading slash.
+#
+#        Partial paths look like relative filenames.  They do not begin
+#        with a prefix.  The matching rules for partial paths are subtle but
+#        designed to make specifying devices easy.  At each level of the
+#        composition tree, the partial path is matched as an absolute path.
+#        The first match is not returned.  At least two matches are searched
+#        for.  A successful result is only returned if only one match is
+#        found.  If more than one match is found, a flag is return to
+#        indicate that the match was ambiguous.
+#
+# @property: The property name to read
+#
+# Returns: The property value.  The type depends on the property type.  legacy<>
+#          properties are returned as #str.  child<> and link<> properties are
+#          returns as #str pathnames.  All integer property types (u8, u16, etc)
+#          are returned as #int.
+#
+# Since: 1.1
+#
+# Notes: This command is experimental and may change syntax in future releases.
+##
+{ 'command': 'qom-get',
+  'data': { 'path': 'str', 'property': 'str' },
+  'returns': 'visitor',
+  'gen': 'no' }
+
+##
+# @qom-set:
+#
+# This command will set a property from a device model path.
+#
+# @path: see @qom-get for a description of this parameter
+#
+# @property: the property name to set
+#
+# @value: a value who's type is appropriate for the property type.  See @qom-get
+#         for a description of type mapping.
+#
+# Since: 1.1
+#
+# Notes: This command is experimental and may change syntax in future releases.
+##
+{ 'command': 'qom-set',
+  'data': { 'path': 'str', 'property': 'str', 'value': 'visitor' },
+  'gen': 'no' }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index c1f0718..7e3f4b9 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -2015,3 +2015,15 @@ EQMP
         .args_type  = "path:s",
         .mhandler.cmd_new = qmp_marshal_input_qom_list,
     },
+
+    {
+        .name       = "qom-set",
+	.args_type  = "path:s,property:s,opts:O",
+	.mhandler.cmd_new = qmp_qom_set,
+    },
+
+    {
+        .name       = "qom-get",
+	.args_type  = "path:s,property:s",
+	.mhandler.cmd_new = qmp_qom_get,
+    },
diff --git a/qmp.c b/qmp.c
index 8e9a595..5e09b41 100644
--- a/qmp.c
+++ b/qmp.c
@@ -17,6 +17,8 @@
 #include "kvm.h"
 #include "arch_init.h"
 #include "hw/qdev.h"
+#include "qapi/qmp-input-visitor.h"
+#include "qapi/qmp-output-visitor.h"
 
 NameInfo *qmp_query_name(Error **errp)
 {
@@ -182,3 +184,66 @@ DevicePropertyInfoList *qmp_qom_list(const char *path, Error **errp)
 
     return props;
 }
+
+/* FIXME: teach qapi about how to pass through Visitors */
+int qmp_qom_set(Monitor *mon, const QDict *qdict, QObject **ret)
+{
+    const char *path = qdict_get_str(qdict, "path");
+    const char *property = qdict_get_str(qdict, "property");
+    QObject *value = qdict_get(qdict, "value");
+    Error *local_err = NULL;
+    QmpInputVisitor *mi;
+    DeviceState *dev;
+
+    dev = qdev_resolve_path(path, NULL);
+    if (!dev) {
+        error_set(&local_err, QERR_DEVICE_NOT_FOUND, path);
+        goto out;
+    }
+
+    mi = qmp_input_visitor_new(value);
+    qdev_property_set(dev, qmp_input_get_visitor(mi), property, &local_err);
+
+    qmp_input_visitor_cleanup(mi);
+
+out:
+    if (local_err) {
+        qerror_report_err(local_err);
+        error_free(local_err);
+        return -1;
+    }
+
+    return 0;
+}
+
+int qmp_qom_get(Monitor *mon, const QDict *qdict, QObject **ret)
+{
+    const char *path = qdict_get_str(qdict, "path");
+    const char *property = qdict_get_str(qdict, "property");
+    Error *local_err = NULL;
+    QmpOutputVisitor *mo;
+    DeviceState *dev;
+
+    dev = qdev_resolve_path(path, NULL);
+    if (!dev) {
+        error_set(&local_err, QERR_DEVICE_NOT_FOUND, path);
+        goto out;
+    }
+
+    mo = qmp_output_visitor_new();
+    qdev_property_get(dev, qmp_output_get_visitor(mo), property, &local_err);
+    if (!local_err) {
+        *ret = qmp_output_get_qobject(mo);
+    }
+
+    qmp_output_visitor_cleanup(mo);
+
+out:
+    if (local_err) {
+        qerror_report_err(local_err);
+        error_free(local_err);
+        return -1;
+    }
+
+    return 0;
+}
-- 
1.7.4.1

[Qemu-devel] [PATCH v3 12/20] qdev: add explicitly named devices to the root complex

[Anthony Liguori]

We first add a 'peripheral' container to the root device that we add user
created devices to.  This provides all user created devices with a unique and
isolated namespace.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 hw/qdev.c |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/hw/qdev.c b/hw/qdev.c
index d448275..cb3fc6e 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -227,6 +227,19 @@ int qdev_device_help(QemuOpts *opts)
     return 1;
 }
 
+static DeviceState *qdev_get_peripheral(void)
+{
+    static DeviceState *dev;
+
+    if (dev == NULL) {
+        dev = qdev_create(NULL, "container");
+        qdev_property_add_child(qdev_get_root(), "peripheral", dev, NULL);
+        qdev_init_nofail(dev);
+    }
+
+    return dev;
+}
+
 DeviceState *qdev_device_add(QemuOpts *opts)
 {
     const char *driver, *path, *id;
@@ -278,6 +291,7 @@ DeviceState *qdev_device_add(QemuOpts *opts)
     id = qemu_opts_id(opts);
     if (id) {
         qdev->id = id;
+        qdev_property_add_child(qdev_get_peripheral(), qdev->id, qdev, NULL);
     }
     if (qemu_opt_foreach(opts, set_property, qdev, 1) != 0) {
         qdev_free(qdev);
-- 
1.7.4.1

[Qemu-devel] [PATCH v3 13/20] dev: add an anonymous peripheral container

[Anthony Liguori]

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 hw/qdev.c |   21 ++++++++++++++++++++-
 1 files changed, 20 insertions(+), 1 deletions(-)

diff --git a/hw/qdev.c b/hw/qdev.c
index cb3fc6e..4d491f1 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -240,6 +240,19 @@ static DeviceState *qdev_get_peripheral(void)
     return dev;
 }
 
+static DeviceState *qdev_get_peripheral_anon(void)
+{
+    static DeviceState *dev;
+
+    if (dev == NULL) {
+        dev = qdev_create(NULL, "container");
+        qdev_property_add_child(qdev_get_root(), "peripheral-anon", dev, NULL);
+        qdev_init_nofail(dev);
+    }
+
+    return dev;
+}
+
 DeviceState *qdev_device_add(QemuOpts *opts)
 {
     const char *driver, *path, *id;
@@ -292,7 +305,13 @@ DeviceState *qdev_device_add(QemuOpts *opts)
     if (id) {
         qdev->id = id;
         qdev_property_add_child(qdev_get_peripheral(), qdev->id, qdev, NULL);
-    }
+    } else {
+        static int anon_count;
+        gchar *name = g_strdup_printf("device[%d]", anon_count++);
+        qdev_property_add_child(qdev_get_peripheral_anon(), name,
+                                qdev, NULL);
+        g_free(name);
+    }        
     if (qemu_opt_foreach(opts, set_property, qdev, 1) != 0) {
         qdev_free(qdev);
         return NULL;
-- 
1.7.4.1

[Qemu-devel] [PATCH v3 14/20] rtc: make piix3 set the rtc as a child (v2)

[Anthony Liguori]

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
v1 -> v2
 - comments (Stefan)
---
 hw/pc_piix.c  |   11 +++++++++++
 hw/piix_pci.c |    3 +++
 2 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index 970f43c..2d5ea2c 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -205,6 +205,17 @@ static void pc_init1(MemoryRegion *system_memory,
         }
     }
 
+    /* FIXME there's some major spaghetti here.  Somehow we create the devices
+     * on the PIIX before we actually create it.  We create the PIIX3 deep in
+     * the recess of the i440fx creation too and then lose the DeviceState.
+     *
+     * For now, let's "fix" this by making judicious use of paths.  This is not
+     * generally the right way to do this.
+     */
+
+    qdev_property_add_child(qdev_resolve_path("/i440fx/piix3", NULL),
+                            "rtc", (DeviceState *)rtc_state, NULL);
+
     audio_init(gsi, pci_enabled ? pci_bus : NULL);
 
     pc_cmos_init(below_4g_mem_size, above_4g_mem_size, boot_device,
diff --git a/hw/piix_pci.c b/hw/piix_pci.c
index d183443..d785d4b 100644
--- a/hw/piix_pci.c
+++ b/hw/piix_pci.c
@@ -288,6 +288,7 @@ static PCIBus *i440fx_common_init(const char *device_name,
                     address_space_io, 0);
     s->bus = b;
     qdev_init_nofail(dev);
+    qdev_property_add_child(qdev_get_root(), "i440fx", dev, NULL);
 
     d = pci_create_simple(b, 0, device_name);
     *pi440fx_state = DO_UPCAST(PCII440FXState, dev, d);
@@ -323,6 +324,8 @@ static PCIBus *i440fx_common_init(const char *device_name,
                 pci_create_simple_multifunction(b, -1, true, "PIIX3"));
         pci_bus_irqs(b, piix3_set_irq, pci_slot_get_pirq, piix3,
                 PIIX_NUM_PIRQS);
+
+        qdev_property_add_child(dev, "piix3", &piix3->dev.qdev, NULL);
     }
     piix3->pic = pic;
 
-- 
1.7.4.1

[Qemu-devel] [PATCH v3 15/20] rtc: add a dynamic property for retrieving the date

[Anthony Liguori]

This really shows the power of dynamic object properties compared to qdev
static properties.

This property represents a complex structure who's format is preserved over the
wire.  This is enabled by visitors.

It also shows an entirely synthetic property that is not tied to device state.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 hw/mc146818rtc.c |   27 +++++++++++++++++++++++++++
 1 files changed, 27 insertions(+), 0 deletions(-)

diff --git a/hw/mc146818rtc.c b/hw/mc146818rtc.c
index 2aaca2f..0c23cb0 100644
--- a/hw/mc146818rtc.c
+++ b/hw/mc146818rtc.c
@@ -614,6 +614,29 @@ static const MemoryRegionOps cmos_ops = {
     .old_portio = cmos_portio
 };
 
+// FIXME add int32 visitor
+static void visit_type_int32(Visitor *v, int *value, const char *name, Error **errp)
+{
+    int64_t val = *value;
+    visit_type_int(v, &val, name, errp);
+}
+
+static void rtc_get_date(DeviceState *dev, Visitor *v, void *opaque,
+                         const char *name, Error **errp)
+{
+    ISADevice *isa = DO_UPCAST(ISADevice, qdev, dev);
+    RTCState *s = DO_UPCAST(RTCState, dev, isa);
+
+    visit_start_struct(v, NULL, "struct tm", name, 0, errp);
+    visit_type_int32(v, &s->current_tm.tm_year, "tm_year", errp);
+    visit_type_int32(v, &s->current_tm.tm_mon, "tm_mon", errp);
+    visit_type_int32(v, &s->current_tm.tm_mday, "tm_mday", errp);
+    visit_type_int32(v, &s->current_tm.tm_hour, "tm_hour", errp);
+    visit_type_int32(v, &s->current_tm.tm_min, "tm_min", errp);
+    visit_type_int32(v, &s->current_tm.tm_sec, "tm_sec", errp);
+    visit_end_struct(v, errp);
+}
+
 static int rtc_initfn(ISADevice *dev)
 {
     RTCState *s = DO_UPCAST(RTCState, dev, dev);
@@ -647,6 +670,10 @@ static int rtc_initfn(ISADevice *dev)
 
     qdev_set_legacy_instance_id(&dev->qdev, base, 2);
     qemu_register_reset(rtc_reset, s);
+
+    qdev_property_add(&s->dev.qdev, "date", "struct tm",
+                      rtc_get_date, NULL, NULL, s, NULL);
+
     return 0;
 }
 
-- 
1.7.4.1

[Qemu-devel] [PATCH v3 16/20] qom: optimize qdev_get_canonical_path using a parent link

[Anthony Liguori]

The full tree search was a bit unreasonable.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 hw/qdev.c |   56 ++++++++++++++++++++++++--------------------------------
 hw/qdev.h |    4 ++++
 2 files changed, 28 insertions(+), 32 deletions(-)

diff --git a/hw/qdev.c b/hw/qdev.c
index 4d491f1..4004860 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -1229,6 +1229,8 @@ void qdev_property_add_child(DeviceState *dev, const char *name,
                       NULL, NULL, child, errp);
 
     qdev_ref(child);
+    g_assert(child->parent == NULL);
+    child->parent = dev;
 
     g_free(type);
 }
@@ -1307,48 +1309,38 @@ void qdev_property_add_link(DeviceState *dev, const char *name,
     g_free(full_type);
 }
 
-static gchar *qdev_get_path_in(DeviceState *parent, DeviceState *dev)
+gchar *qdev_get_canonical_path(DeviceState *dev)
 {
-    DeviceProperty *prop;
+    DeviceState *root = qdev_get_root();
+    char *newpath = NULL, *path = NULL;
 
-    if (parent == dev) {
-        return g_strdup("");
-    }
+    while (dev != root) {
+        DeviceProperty *prop = NULL;
 
-    QTAILQ_FOREACH(prop, &parent->properties, node) {
-        gchar *subpath;
+        g_assert(dev->parent != NULL);
 
-        if (!strstart(prop->type, "child<", NULL)) {
-            continue;
-        }
+        QTAILQ_FOREACH(prop, &dev->parent->properties, node) {
+            if (!strstart(prop->type, "child<", NULL)) {
+                continue;
+            }
 
-        /* Check to see if the device is one of parent's children */
-        if (prop->opaque == dev) {
-            return g_strdup(prop->name);
+            if (prop->opaque == dev) {
+                if (path) {
+                    newpath = g_strdup_printf("%s/%s", prop->name, path);
+                    g_free(path);
+                    path = newpath;
+                } else {
+                    path = g_strdup(prop->name);
+                }
+                break;
+            }
         }
 
-        /* Check to see if the device is a child of our child */
-        subpath = qdev_get_path_in(prop->opaque, dev);
-        if (subpath) {
-            gchar *path;
+        g_assert(prop != NULL);
 
-            path = g_strdup_printf("%s/%s", prop->name, subpath);
-            g_free(subpath);
-
-            return path;
-        }
+        dev = dev->parent;
     }
 
-    return NULL;
-}
-
-gchar *qdev_get_canonical_path(DeviceState *dev)
-{
-    gchar *path, *newpath;
-
-    path = qdev_get_path_in(qdev_get_root(), dev);
-    g_assert(path != NULL);
-
     newpath = g_strdup_printf("/%s", path);
     g_free(path);
 
diff --git a/hw/qdev.h b/hw/qdev.h
index 4351e2e..fdab848 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -92,6 +92,10 @@ struct DeviceState {
     uint32_t ref;
 
     QTAILQ_HEAD(, DeviceProperty) properties;
+
+    /* Do not, under any circumstance, use this parent link below anywhere
+     * outside of qdev.c.  You have been warned. */
+    DeviceState *parent;
 };
 
 typedef void (*bus_dev_printfn)(Monitor *mon, DeviceState *dev, int indent);
-- 
1.7.4.1

[Qemu-devel] [PATCH v3 17/20] qom: add vga node to the pc composition tree

[Anthony Liguori]

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 hw/cirrus_vga.c |    8 +++++---
 hw/pc.c         |   26 ++++++++++++++++----------
 hw/pc.h         |   14 +++++++-------
 hw/pc_piix.c    |    6 +++++-
 hw/vga-pci.c    |    5 ++---
 hw/vmware_vga.h |    6 +++---
 6 files changed, 38 insertions(+), 27 deletions(-)

diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c
index c7e365b..c0db315 100644
--- a/hw/cirrus_vga.c
+++ b/hw/cirrus_vga.c
@@ -2899,7 +2899,7 @@ static void cirrus_init_common(CirrusVGAState * s, int device_id, int is_pci,
  *
  ***************************************/
 
-void isa_cirrus_vga_init(MemoryRegion *system_memory)
+DeviceState *isa_cirrus_vga_init(MemoryRegion *system_memory)
 {
     CirrusVGAState *s;
 
@@ -2913,6 +2913,8 @@ void isa_cirrus_vga_init(MemoryRegion *system_memory)
     vmstate_register(NULL, 0, &vmstate_cirrus_vga, s);
     rom_add_vga(VGABIOS_CIRRUS_FILENAME);
     /* XXX ISA-LFB support */
+    /* FIXME not qdev yet */
+    return NULL;
 }
 
 /***************************************
@@ -2955,9 +2957,9 @@ static int pci_cirrus_vga_initfn(PCIDevice *dev)
      return 0;
 }
 
-void pci_cirrus_vga_init(PCIBus *bus)
+DeviceState *pci_cirrus_vga_init(PCIBus *bus)
 {
-    pci_create_simple(bus, -1, "cirrus-vga");
+    return &pci_create_simple(bus, -1, "cirrus-vga")->qdev;
 }
 
 static PCIDeviceInfo cirrus_vga_info = {
diff --git a/hw/pc.c b/hw/pc.c
index 33778fe..2fd124a 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -1069,38 +1069,44 @@ qemu_irq *pc_allocate_cpu_irq(void)
     return qemu_allocate_irqs(pic_irq_request, NULL, 1);
 }
 
-void pc_vga_init(PCIBus *pci_bus)
+DeviceState *pc_vga_init(PCIBus *pci_bus)
 {
+    DeviceState *dev = NULL;
+
     if (cirrus_vga_enabled) {
         if (pci_bus) {
-            pci_cirrus_vga_init(pci_bus);
+            dev = pci_cirrus_vga_init(pci_bus);
         } else {
-            isa_cirrus_vga_init(get_system_memory());
+            dev = isa_cirrus_vga_init(get_system_memory());
         }
     } else if (vmsvga_enabled) {
         if (pci_bus) {
-            if (!pci_vmsvga_init(pci_bus)) {
+            dev = pci_vmsvga_init(pci_bus);
+            if (!dev) {
                 fprintf(stderr, "Warning: vmware_vga not available,"
                         " using standard VGA instead\n");
-                pci_vga_init(pci_bus);
+                dev = pci_vga_init(pci_bus);
             }
         } else {
             fprintf(stderr, "%s: vmware_vga: no PCI bus\n", __FUNCTION__);
         }
 #ifdef CONFIG_SPICE
     } else if (qxl_enabled) {
-        if (pci_bus)
-            pci_create_simple(pci_bus, -1, "qxl-vga");
-        else
+        if (pci_bus) {
+            dev = &pci_create_simple(pci_bus, -1, "qxl-vga")->qdev;
+        } else {
             fprintf(stderr, "%s: qxl: no PCI bus\n", __FUNCTION__);
+        }
 #endif
     } else if (std_vga_enabled) {
         if (pci_bus) {
-            pci_vga_init(pci_bus);
+            dev = pci_vga_init(pci_bus);
         } else {
-            isa_vga_init();
+            dev = isa_vga_init();
         }
     }
+
+    return dev;
 }
 
 static void cpu_request_exit(void *opaque, int irq, int level)
diff --git a/hw/pc.h b/hw/pc.h
index b7b7e40..b2000e7 100644
--- a/hw/pc.h
+++ b/hw/pc.h
@@ -140,7 +140,7 @@ void pc_memory_init(MemoryRegion *system_memory,
                     MemoryRegion *rom_memory,
                     MemoryRegion **ram_memory);
 qemu_irq *pc_allocate_cpu_irq(void);
-void pc_vga_init(PCIBus *pci_bus);
+DeviceState *pc_vga_init(PCIBus *pci_bus);
 void pc_basic_device_init(qemu_irq *gsi,
                           ISADevice **rtc_state,
                           ISADevice **floppy,
@@ -205,27 +205,27 @@ enum vga_retrace_method {
 
 extern enum vga_retrace_method vga_retrace_method;
 
-static inline int isa_vga_init(void)
+static inline DeviceState *isa_vga_init(void)
 {
     ISADevice *dev;
 
     dev = isa_try_create("isa-vga");
     if (!dev) {
         fprintf(stderr, "Warning: isa-vga not available\n");
-        return 0;
+        return NULL;
     }
     qdev_init_nofail(&dev->qdev);
-    return 1;
+    return &dev->qdev;
 }
 
-int pci_vga_init(PCIBus *bus);
+DeviceState *pci_vga_init(PCIBus *bus);
 int isa_vga_mm_init(target_phys_addr_t vram_base,
                     target_phys_addr_t ctrl_base, int it_shift,
                     MemoryRegion *address_space);
 
 /* cirrus_vga.c */
-void pci_cirrus_vga_init(PCIBus *bus);
-void isa_cirrus_vga_init(MemoryRegion *address_space);
+DeviceState *pci_cirrus_vga_init(PCIBus *bus);
+DeviceState *isa_cirrus_vga_init(MemoryRegion *address_space);
 
 /* ne2000.c */
 static inline bool isa_ne2000_init(int base, int irq, NICInfo *nd)
diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index 2d5ea2c..166c2fc 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -99,6 +99,7 @@ static void pc_init1(MemoryRegion *system_memory,
     MemoryRegion *ram_memory;
     MemoryRegion *pci_memory;
     MemoryRegion *rom_memory;
+    DeviceState *dev;
 
     pc_cpus_init(cpu_model);
 
@@ -168,7 +169,10 @@ static void pc_init1(MemoryRegion *system_memory,
 
     pc_register_ferr_irq(gsi[13]);
 
-    pc_vga_init(pci_enabled? pci_bus: NULL);
+    dev = pc_vga_init(pci_enabled? pci_bus: NULL);
+    if (dev) {
+        qdev_property_add_child(qdev_get_root(), "vga", dev, NULL);
+    }
 
     if (xen_enabled()) {
         pci_create_simple(pci_bus, -1, "xen-platform");
diff --git a/hw/vga-pci.c b/hw/vga-pci.c
index 14bfadb..a75dbf3 100644
--- a/hw/vga-pci.c
+++ b/hw/vga-pci.c
@@ -70,10 +70,9 @@ static int pci_vga_initfn(PCIDevice *dev)
      return 0;
 }
 
-int pci_vga_init(PCIBus *bus)
+DeviceState *pci_vga_init(PCIBus *bus)
 {
-    pci_create_simple(bus, -1, "VGA");
-    return 0;
+    return &pci_create_simple(bus, -1, "VGA")->qdev;
 }
 
 static PCIDeviceInfo vga_info = {
diff --git a/hw/vmware_vga.h b/hw/vmware_vga.h
index 5132573..db11cbf 100644
--- a/hw/vmware_vga.h
+++ b/hw/vmware_vga.h
@@ -4,15 +4,15 @@
 #include "qemu-common.h"
 
 /* vmware_vga.c */
-static inline bool pci_vmsvga_init(PCIBus *bus)
+static inline DeviceState *pci_vmsvga_init(PCIBus *bus)
 {
     PCIDevice *dev;
 
     dev = pci_try_create(bus, -1, "vmware-svga");
     if (!dev || qdev_init(&dev->qdev) < 0) {
-        return false;
+        return NULL;
     } else {
-        return true;
+        return &dev->qdev;
     }
 }
 
-- 
1.7.4.1

[Qemu-devel] [PATCH v3 18/20] qom: add string property type

[Anthony Liguori]

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 hw/qdev.c |   59 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/qdev.h |   22 ++++++++++++++++++++++
 2 files changed, 81 insertions(+), 0 deletions(-)

diff --git a/hw/qdev.c b/hw/qdev.c
index 4004860..0fc20fc 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -1449,3 +1449,62 @@ DeviceState *qdev_resolve_path(const char *path, bool *ambiguous)
     return dev;
 }
 
+typedef struct StringProperty
+{
+    char *(*get)(DeviceState *, Error **);
+    void (*set)(DeviceState *, const char *, Error **);
+} StringProperty;
+
+static void qdev_property_get_str(DeviceState *dev, Visitor *v, void *opaque,
+                                  const char *name, Error **errp)
+{
+    StringProperty *prop = opaque;
+    char *value;
+
+    value = prop->get(dev, errp);
+    if (value) {
+        visit_type_str(v, &value, name, errp);
+        g_free(value);
+    }
+}
+
+static void qdev_property_set_str(DeviceState *dev, Visitor *v, void *opaque,
+                                  const char *name, Error **errp)
+{
+    StringProperty *prop = opaque;
+    char *value;
+    Error *local_err = NULL;
+
+    visit_type_str(v, &value, name, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return;
+    }
+
+    prop->set(dev, value, errp);
+    g_free(value);
+}
+
+static void qdev_property_release_str(DeviceState *dev, const char *name,
+                                      void *opaque)
+{
+    StringProperty *prop = opaque;
+    g_free(prop);
+}
+
+void qdev_property_add_str(DeviceState *dev, const char *name,
+                           char *(*get)(DeviceState *, Error **),
+                           void (*set)(DeviceState *, const char *, Error **),
+                           Error **errp)
+{
+    StringProperty *prop = g_malloc0(sizeof(*prop));
+
+    prop->get = get;
+    prop->set = set;
+
+    qdev_property_add(dev, name, "string",
+                      get ? qdev_property_get_str : NULL,
+                      set ? qdev_property_set_str : NULL,
+                      qdev_property_release_str,
+                      prop, errp);
+}
diff --git a/hw/qdev.h b/hw/qdev.h
index fdab848..9faf2ee 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -574,4 +574,26 @@ void qdev_property_add_link(DeviceState *dev, const char *name,
                             const char *type, DeviceState **child,
                             Error **errp);
 
+/**
+ * @qdev_property_add_str
+ *
+ * Add a string property using getters/setters.  This function will add a
+ * property of type 'string'.
+ *
+ * @dev - the device to add a property to
+ *
+ * @name - the name of the property
+ *
+ * @get - the getter or NULL if the property is write-only.  This function must
+ *        return a string to be freed by @g_free().
+ *
+ * @set - the setter or NULL if the property is read-only
+ *
+ * @errp - if an error occurs, a pointer to an area to store the error
+ */
+void qdev_property_add_str(DeviceState *dev, const char *name,
+                           char *(*get)(DeviceState *, Error **),
+                           void (*set)(DeviceState *, const char *, Error **),
+                           Error **errp);
+
 #endif
-- 
1.7.4.1

[Qemu-devel] [PATCH v3 19/20] qdev: add a qdev_get_type() function and expose as a 'type' property

[Anthony Liguori]

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 hw/qdev.c |    7 +++++++
 hw/qdev.h |   14 ++++++++++++++
 2 files changed, 21 insertions(+), 0 deletions(-)

diff --git a/hw/qdev.c b/hw/qdev.c
index 0fc20fc..83913c7 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -110,6 +110,8 @@ static DeviceState *qdev_create_from_info(BusState *bus, DeviceInfo *info)
         qdev_property_add_legacy(dev, prop, NULL);
     }
 
+    qdev_property_add_str(dev, "type", qdev_get_type, NULL, NULL);
+
     return dev;
 }
 
@@ -1031,6 +1033,11 @@ char* qdev_get_fw_dev_path(DeviceState *dev)
     return strdup(path);
 }
 
+char *qdev_get_type(DeviceState *dev, Error **errp)
+{
+    return g_strdup(dev->info->name);
+}
+
 void qdev_ref(DeviceState *dev)
 {
     dev->ref++;
diff --git a/hw/qdev.h b/hw/qdev.h
index 9faf2ee..6e18427 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -596,4 +596,18 @@ void qdev_property_add_str(DeviceState *dev, const char *name,
                            void (*set)(DeviceState *, const char *, Error **),
                            Error **errp);
 
+/**
+ * @qdev_get_type
+ *
+ * Returns the string representation of the type of this object.
+ *
+ * @dev - the device
+ *
+ * @errp - if an error occurs, a pointer to an area to store the error
+ *
+ * Returns: a string representing the type.  This must be freed by the caller
+ *          with g_free().
+ */
+char *qdev_get_type(DeviceState *dev, Error **errp);
+
 #endif
-- 
1.7.4.1

Re: [Qemu-devel] [PATCH v3 00/20] qom: dynamic properties and composition tree

[Anthony Liguori]

On 12/12/2011 02:29 PM, Anthony Liguori wrote:
> This is a follow up to my previous series to get us started in the QOM
> direction.  A few things are different this time around.  Most notably:
>
>   1) Devices no longer have names.  Instead, path names are always used to
>      identify devices.
>
>   2) In order to support (1), dynamic properties are now supported.
>
>   3) The concept of a "root device" has been introduced.  The root device is
>      roughly modelling the motherboard of a machine.  This type is a container
>      type and it's best to think of it as something like a PCB board I guess.
>
> See my other mail for a description of my merge plan for QOM.

Applied.

Regards,

Anthony Liguori

>
> To try it out, here's an example session:
>
> Launch:
>
> anthony@titi:~/build/qemu$ x86_64-softmmu/qemu-system-x86_64 -hda ~/images/linux.img -snapshot -device virtio-balloon-pci,id=foo -qmp unix:/tmp/server.sock,server,nowait
>
> Explore the object model:
>
> anthony@titi:~/git/qemu/QMP$ ./qom-list /
> peripheral/
> i440fx/
> anthony@titi:~/git/qemu/QMP$ ./qom-list /i440fx/
> piix3/
> anthony@titi:~/git/qemu/QMP$ ./qom-list /i440fx/piix3
> rtc/
> anthony@titi:~/git/qemu/QMP$ ./qom-list /i440fx/piix3/rtc
> date
> base_year
> anthony@titi:~/git/qemu/QMP$ ./qom-get /i440fx/piix3/rtc.date
> tm_sec: 33
> tm_hour: 21
> tm_mday: 30
> tm_year: 111
> tm_mon: 10
> tm_min: 2
> anthony@titi:~/git/qemu/QMP$ ./qom-get rtc.date
> tm_sec: 38
> tm_hour: 21
> tm_mday: 30
> tm_year: 111
> tm_mon: 10
> tm_min: 2
> anthony@titi:~/git/qemu/QMP$ ./qom-list /peripheral
> foo/
> anthony@titi:~/git/qemu/QMP$ ./qom-list /peripheral/foo
> event_idx
> indirect_desc
>
> Anthony Liguori (20):
>    qom: add a reference count to qdev objects
>    qom: add new dynamic property infrastructure based on Visitors (v2)
>    qom: register legacy properties as new style properties (v2)
>    qom: introduce root device
>    qdev: provide an interface to return canonical path from root (v2)
>    qdev: provide a path resolution (v2)
>    qom: add child properties (composition) (v3)
>    qom: add link properties (v2)
>    qapi: allow a 'gen' key to suppress code generation
>    qmp: add qom-list command
>    qom: qom_{get,set} monitor commands (v2)
>    qdev: add explicitly named devices to the root complex
>    dev: add an anonymous peripheral container
>    rtc: make piix3 set the rtc as a child (v2)
>    rtc: add a dynamic property for retrieving the date
>    qom: optimize qdev_get_canonical_path using a parent link
>    qom: add vga node to the pc composition tree
>    qom: add string property type
>    qdev: add a qdev_get_type() function and expose as a 'type' property
>    pc: fill out most of the composition tree
>
>   Makefile.objs            |    2 +-
>   hw/acpi_piix4.c          |    7 +-
>   hw/cirrus_vga.c          |    8 +-
>   hw/container.c           |   20 ++
>   hw/ide/pci.c             |   11 +-
>   hw/kvmclock.c            |    5 +-
>   hw/kvmclock.h            |    5 +-
>   hw/mc146818rtc.c         |   27 +++
>   hw/mips_malta.c          |    5 +-
>   hw/pc.c                  |   75 +++++--
>   hw/pc.h                  |   51 +++--
>   hw/pc_piix.c             |   61 ++++--
>   hw/piix_pci.c            |   11 +-
>   hw/qdev.c                |  555 +++++++++++++++++++++++++++++++++++++++++++++-
>   hw/qdev.h                |  281 +++++++++++++++++++++++
>   hw/usb-uhci.c            |    4 +-
>   hw/usb-uhci.h            |    2 +-
>   hw/vga-pci.c             |    5 +-
>   hw/vmware_vga.h          |    6 +-
>   monitor.h                |    4 +
>   qapi-schema.json         |  107 +++++++++
>   qerror.c                 |    4 +
>   qerror.h                 |    3 +
>   qmp-commands.hx          |   18 ++
>   qmp.c                    |   93 ++++++++
>   scripts/qapi-commands.py |    1 +
>   scripts/qapi-types.py    |    1 +
>   27 files changed, 1289 insertions(+), 83 deletions(-)
>   create mode 100644 hw/container.c
>

Re: [Qemu-devel] [PATCH v3 00/20] qom: dynamic properties and composition tree

[Kevin Wolf]

Am 15.12.2011 19:10, schrieb Anthony Liguori:
> On 12/12/2011 02:29 PM, Anthony Liguori wrote:
>> This is a follow up to my previous series to get us started in the QOM
>> direction.  A few things are different this time around.  Most notably:
>>
>>   1) Devices no longer have names.  Instead, path names are always used to
>>      identify devices.
>>
>>   2) In order to support (1), dynamic properties are now supported.
>>
>>   3) The concept of a "root device" has been introduced.  The root device is
>>      roughly modelling the motherboard of a machine.  This type is a container
>>      type and it's best to think of it as something like a PCB board I guess.
>>
>> See my other mail for a description of my merge plan for QOM.
> 
> Applied.
> 
> Regards,
> 
> Anthony Liguori

So you pushed this with qdev_property_add_child() calls spread all over
the place instead of being treated like other properties? :-(

Kevin

Re: [Qemu-devel] [PATCH v3 00/20] qom: dynamic properties and composition tree

[Paolo Bonzini]

On 12/16/2011 10:42 AM, Kevin Wolf wrote:
>> >
>> >  Applied.
>> >
>> >  Regards,
>> >
>> >  Anthony Liguori
> So you pushed this with qdev_property_add_child() calls spread all over
> the place instead of being treated like other properties?:-(

I think actually this is not the biggest problem.  child properties are 
dynamic, and it's not a problem IMO if they are created like that.

I don't like that _link_ properties are spread all over the place 
instead of being treated like other properties.  Link properties are 
static, and PROP_PTR properties could often be converted to links.

I'm playing with separating the "legacy" and "static" property concepts 
so that we can head in that direction.  I hope to send out an RFC in an 
hour or so.

Paolo

Re: [Qemu-devel] [PATCH v3 00/20] qom: dynamic properties and composition tree

[Kevin Wolf]

Am 16.12.2011 11:17, schrieb Paolo Bonzini:
> On 12/16/2011 10:42 AM, Kevin Wolf wrote:
>>>>
>>>>  Applied.
>>>>
>>>>  Regards,
>>>>
>>>>  Anthony Liguori
>> So you pushed this with qdev_property_add_child() calls spread all over
>> the place instead of being treated like other properties?:-(
> 
> I think actually this is not the biggest problem.  child properties are 
> dynamic, and it's not a problem IMO if they are created like that.

That they are added in an init function is an indicator that they aren't
really dynamic. Some of them are conditional, some aren't. For those
which are, I think having a child property that can be NULL could be
reasonable. In the end you want to have this kind of things configurable
via QOM instead of hard-coded in the devices, right?

> I don't like that _link_ properties are spread all over the place 
> instead of being treated like other properties.  Link properties are 
> static, and PROP_PTR properties could often be converted to links.

Yes, links too.

Kevin

Re: [Qemu-devel] [PATCH v3 00/20] qom: dynamic properties and composition tree

[Paolo Bonzini]

On 12/16/2011 11:36 AM, Kevin Wolf wrote:
> >  I think actually this is not the biggest problem.  child properties are
> >  dynamic, and it's not a problem IMO if they are created like that.
>
> That they are added in an init function is an indicator that they aren't
> really dynamic.

That's true.  However, another indicator is that anything that does not 
have a struct field is also not really static. :)

So right now, child properties are all "dynamic" in this sense.  This 
could change when Anthony converts buses to QOM.  The bus right now is 
embedded into the HBA's struct, is not a pointer.  This likely would 
change when buses are QOM-ized, but then the bus would indeed be a 100% 
static child.

> I think having a child property that can be NULL could be
> reasonable.

I think Anthony convinced me this is not the case (unlike links).  Even 
if buses and similar objects are changed to pointers because the 
implementation needs that, those pointers should never be NULL (or if 
they can, the child property should not exist when they are NULL).

Paolo

Re: [Qemu-devel] [PATCH v3 00/20] qom: dynamic properties and composition tree

[Kevin Wolf]

Am 16.12.2011 13:24, schrieb Paolo Bonzini:
> On 12/16/2011 11:36 AM, Kevin Wolf wrote:
>>>  I think actually this is not the biggest problem.  child properties are
>>>  dynamic, and it's not a problem IMO if they are created like that.
>>
>> That they are added in an init function is an indicator that they aren't
>> really dynamic.
> 
> That's true.  However, another indicator is that anything that does not 
> have a struct field is also not really static. :)

I don't think that not remembering the child device because you don't
need to reference it any more makes it any less static. You could easily
add the struct member, assign it once and then it matches your
definition of static.

And it's not even true of all child devices, for example PCII440FXState
does have a pointer to its piix3 already.

> So right now, child properties are all "dynamic" in this sense.  This 
> could change when Anthony converts buses to QOM.  The bus right now is 
> embedded into the HBA's struct, is not a pointer.  This likely would 
> change when buses are QOM-ized, but then the bus would indeed be a 100% 
> static child.
> 
>> I think having a child property that can be NULL could be
>> reasonable.
> 
> I think Anthony convinced me this is not the case (unlike links).  Even 
> if buses and similar objects are changed to pointers because the 
> implementation needs that, those pointers should never be NULL (or if 
> they can, the child property should not exist when they are NULL).

So child properties are never configurable, and if a device is optional
or you can choose between multiple devices then it should be a link
instead. That is, device composition in term of "child devices" happens
only hard-coded and the user doesn't do it.

Is this a reasonably accurate description of the intention?

Kevin

Re: [Qemu-devel] [PATCH v3 00/20] qom: dynamic properties and composition tree

[Paolo Bonzini]

On 12/16/2011 01:55 PM, Kevin Wolf wrote:
> I don't think that not remembering the child device because you don't
> need to reference it any more makes it any less static. You could easily
> add the struct member, assign it once and then it matches your
> definition of static.

I think in that case you would add a link property, not a child.

> And it's not even true of all child devices, for example PCII440FXState
> does have a pointer to its piix3 already.

It's unused, actually.  You're right, even though I'm not sure whether a 
child<Pin> will be stored in the parent as a Pin or as a qemu_irq.

> >  I think Anthony convinced me this is not the case (unlike links).  Even
> >  if buses and similar objects are changed to pointers because the
> >  implementation needs that, those pointers should never be NULL (or if
> >  they can, the child property should not exist when they are NULL).
>
> So child properties are never configurable, and if a device is optional
> or you can choose between multiple devices then it should be a link
> instead. That is, device composition in term of "child devices" happens
> only hard-coded and the user doesn't do it.
>
> Is this a reasonably accurate description of the intention?

It does sound accurate.  In addition, a child property should really 
model a parent-child relationship and should create a tree.

As an example, say you want to join two devices ("A" and "B"), by adding 
a connection from B to A.  Assuming you cannot simply add A as a 
child<A> to B, you can do it in several ways:

1) directly add a link<A> property to B.

    Examples:

    - a local APIC has a link<CPUX86State>

2) generalizing (1), you can add a link<X> property to B, where X can be 
any superclass of A or an interface implemented by A.

    Examples:

    - an object representing a PCI bus has a link<PCIDevice> for each
      slot (PCIDevice is an abstract class)

    - a SCSIBus can have a link<SCSIBusAdapter> to the parent (assuming
      SCSIBus is a class and SCSIBusAdapter is an interface)

3) using composition ("has-a") instead of inheritance, you can add a 
child<X> property to A and a link<X> property to B.

     Example:

     - a SCSI bus adapter is a device that has a child<SCSIBus>.
       A SCSIDevice doesn't have a link to the adapter, but only
       a link<SCSIBus>.

     - instead of representing PCIDevice as a class, you could have a
       child<PCIConnector> in the children, and a link<PCIConnector> in
       the bus.  (I would actually prefer this to an abstract PCIDevice
       class, but we agreed that it would be too much churn for now).

     - X can be simply a "Pin".

     All of these examples match your definition, I tihnk.

For bidirectional links you can have any combination of the above.

Paolo

Re: [Qemu-devel] [PATCH v3 00/20] qom: dynamic properties and composition tree

[Anthony Liguori]

On 12/16/2011 03:42 AM, Kevin Wolf wrote:
> Am 15.12.2011 19:10, schrieb Anthony Liguori:
>> On 12/12/2011 02:29 PM, Anthony Liguori wrote:
>>> This is a follow up to my previous series to get us started in the QOM
>>> direction.  A few things are different this time around.  Most notably:
>>>
>>>    1) Devices no longer have names.  Instead, path names are always used to
>>>       identify devices.
>>>
>>>    2) In order to support (1), dynamic properties are now supported.
>>>
>>>    3) The concept of a "root device" has been introduced.  The root device is
>>>       roughly modelling the motherboard of a machine.  This type is a container
>>>       type and it's best to think of it as something like a PCB board I guess.
>>>
>>> See my other mail for a description of my merge plan for QOM.
>>
>> Applied.
>>
>> Regards,
>>
>> Anthony Liguori
>
> So you pushed this with qdev_property_add_child() calls spread all over
> the place instead of being treated like other properties? :-(

I would really like to be able to do something like:

struct ISASerial {
   Device parent;

   UART uart;
};

static Properties isa_serial_properties[] = {
     DEFINE_PROP_CHILD(ISASerial, uart),
     ...
};

Actually, I'd really like to do:

struct ISASerial {
    Device parent;

    UART _child uart;
};

And use an IDL compiler to generate the property table.

But at any rate, to get there, we need to first refactor the device model so 
that child devices are being created and registered in the device init 
functions.  We also need to a bunch more of the qom series so that we can do 
in-place initialization of objects.

Right now, a lot of things that are "children" are created in deep corners of 
the machine init often times long before the parent is ever created.

Regards,

Anthony Liguori

>
> Kevin
>

Re: [Qemu-devel] [PATCH v3 00/20] qom: dynamic properties and composition tree

[Anthony Liguori]

On 12/16/2011 04:17 AM, Paolo Bonzini wrote:
> On 12/16/2011 10:42 AM, Kevin Wolf wrote:
>>> >
>>> > Applied.
>>> >
>>> > Regards,
>>> >
>>> > Anthony Liguori
>> So you pushed this with qdev_property_add_child() calls spread all over
>> the place instead of being treated like other properties?:-(
>
> I think actually this is not the biggest problem. child properties are dynamic,
> and it's not a problem IMO if they are created like that.
>
> I don't like that _link_ properties are spread all over the place instead of
> being treated like other properties. Link properties are static, and PROP_PTR
> properties could often be converted to links.

With your new series, we should be able to add a DEFINE_PROP_LINK() that does 
the right thing I guess.  I've got no objection to that.

Regards,

Anthony Liguori

>
> I'm playing with separating the "legacy" and "static" property concepts so that
> we can head in that direction. I hope to send out an RFC in an hour or so.
>
> Paolo
>

Re: [Qemu-devel] [PATCH v3 00/20] qom: dynamic properties and composition tree

[Anthony Liguori]

On 12/16/2011 06:24 AM, Paolo Bonzini wrote:
> On 12/16/2011 11:36 AM, Kevin Wolf wrote:
>> > I think actually this is not the biggest problem. child properties are
>> > dynamic, and it's not a problem IMO if they are created like that.
>>
>> That they are added in an init function is an indicator that they aren't
>> really dynamic.
>
> That's true. However, another indicator is that anything that does not have a
> struct field is also not really static. :)
>
> So right now, child properties are all "dynamic" in this sense. This could
> change when Anthony converts buses to QOM. The bus right now is embedded into
> the HBA's struct, is not a pointer. This likely would change when buses are
> QOM-ized, but then the bus would indeed be a 100% static child.
>
>> I think having a child property that can be NULL could be
>> reasonable.
>
> I think Anthony convinced me this is not the case (unlike links). Even if buses
> and similar objects are changed to pointers because the implementation needs
> that, those pointers should never be NULL (or if they can, the child property
> should not exist when they are NULL).

What I would like to get to eventually is:

struct ISASerial {
    Device parent;

    UART _child uart;
    ISABus _link *bus;
};

A child should be able to be part of the parent devices memory with its life 
cycle bound to the parents life cycle.  This is why a child property shouldn't 
be nullable.

Regards,

Anthony Liguori

>
> Paolo
>

Re: [Qemu-devel] [PATCH v3 00/20] qom: dynamic properties and composition tree

[Anthony Liguori]

On 12/16/2011 07:28 AM, Paolo Bonzini wrote:
> On 12/16/2011 01:55 PM, Kevin Wolf wrote:
>> I don't think that not remembering the child device because you don't
>> need to reference it any more makes it any less static. You could easily
>> add the struct member, assign it once and then it matches your
>> definition of static.
>
> I think in that case you would add a link property, not a child.
>
>> And it's not even true of all child devices, for example PCII440FXState
>> does have a pointer to its piix3 already.
>
> It's unused, actually. You're right, even though I'm not sure whether a
> child<Pin> will be stored in the parent as a Pin or as a qemu_irq.
>
>> > I think Anthony convinced me this is not the case (unlike links). Even
>> > if buses and similar objects are changed to pointers because the
>> > implementation needs that, those pointers should never be NULL (or if
>> > they can, the child property should not exist when they are NULL).
>>
>> So child properties are never configurable, and if a device is optional
>> or you can choose between multiple devices then it should be a link
>> instead. That is, device composition in term of "child devices" happens
>> only hard-coded and the user doesn't do it.
>>
>> Is this a reasonably accurate description of the intention?
>
> It does sound accurate. In addition, a child property should really model a
> parent-child relationship and should create a tree.
>
> As an example, say you want to join two devices ("A" and "B"), by adding a
> connection from B to A. Assuming you cannot simply add A as a child<A> to B, you
> can do it in several ways:
>
> 1) directly add a link<A> property to B.
>
> Examples:
>
> - a local APIC has a link<CPUX86State>
>
> 2) generalizing (1), you can add a link<X> property to B, where X can be any
> superclass of A or an interface implemented by A.
>
> Examples:
>
> - an object representing a PCI bus has a link<PCIDevice> for each
> slot (PCIDevice is an abstract class)
>
> - a SCSIBus can have a link<SCSIBusAdapter> to the parent (assuming
> SCSIBus is a class and SCSIBusAdapter is an interface)
>
> 3) using composition ("has-a") instead of inheritance, you can add a child<X>
> property to A and a link<X> property to B.
>
> Example:
>
> - a SCSI bus adapter is a device that has a child<SCSIBus>.
> A SCSIDevice doesn't have a link to the adapter, but only
> a link<SCSIBus>.
>
> - instead of representing PCIDevice as a class, you could have a
> child<PCIConnector> in the children, and a link<PCIConnector> in
> the bus. (I would actually prefer this to an abstract PCIDevice
> class, but we agreed that it would be too much churn for now).
>
> - X can be simply a "Pin".
>
> All of these examples match your definition, I tihnk.
>
> For bidirectional links you can have any combination of the above.

Yes, exactly.

Regards,

Anthony Liguori

>
> Paolo
>

Re: [Qemu-devel] [PATCH v3 00/20] qom: dynamic properties and composition tree

[Paolo Bonzini]

On 12/16/2011 02:51 PM, Anthony Liguori wrote:
> struct ISASerial {
>     Device parent;
>
>     UART _child uart;
>     ISABus _link *bus;
> };
>
> A child should be able to be part of the parent devices memory with its
> life cycle bound to the parents life cycle.  This is why a child
> property shouldn't be nullable.

100% agreed in principle.

But technically, if you do not make them pointers how do you handle 
children that, for some reason (broken reference counting for example) 
outlive the parent?  Aborting could be a very good answer. :)

Paolo

Re: [Qemu-devel] [PATCH v3 00/20] qom: dynamic properties and composition tree

[Anthony Liguori]

On 12/16/2011 07:52 AM, Paolo Bonzini wrote:
> On 12/16/2011 02:51 PM, Anthony Liguori wrote:
>> struct ISASerial {
>> Device parent;
>>
>> UART _child uart;
>> ISABus _link *bus;
>> };
>>
>> A child should be able to be part of the parent devices memory with its
>> life cycle bound to the parents life cycle. This is why a child
>> property shouldn't be nullable.
>
> 100% agreed in principle.
>
> But technically, if you do not make them pointers how do you handle children
> that, for some reason (broken reference counting for example) outlive the
> parent? Aborting could be a very good answer. :)

Yes, aborting would be the answer.

Regards,

Anthony Liguori

>
> Paolo
>

Re: [Qemu-devel] [PATCH v3 00/20] qom: dynamic properties and composition tree

[Kevin Wolf]

Am 16.12.2011 14:51, schrieb Anthony Liguori:
> On 12/16/2011 06:24 AM, Paolo Bonzini wrote:
>> On 12/16/2011 11:36 AM, Kevin Wolf wrote:
>>>> I think actually this is not the biggest problem. child properties are
>>>> dynamic, and it's not a problem IMO if they are created like that.
>>>
>>> That they are added in an init function is an indicator that they aren't
>>> really dynamic.
>>
>> That's true. However, another indicator is that anything that does not have a
>> struct field is also not really static. :)
>>
>> So right now, child properties are all "dynamic" in this sense. This could
>> change when Anthony converts buses to QOM. The bus right now is embedded into
>> the HBA's struct, is not a pointer. This likely would change when buses are
>> QOM-ized, but then the bus would indeed be a 100% static child.
>>
>>> I think having a child property that can be NULL could be
>>> reasonable.
>>
>> I think Anthony convinced me this is not the case (unlike links). Even if buses
>> and similar objects are changed to pointers because the implementation needs
>> that, those pointers should never be NULL (or if they can, the child property
>> should not exist when they are NULL).
> 
> What I would like to get to eventually is:
> 
> struct ISASerial {
>     Device parent;
> 
>     UART _child uart;
>     ISABus _link *bus;
> };
> 
> A child should be able to be part of the parent devices memory with its life 
> cycle bound to the parents life cycle.  This is why a child property shouldn't 
> be nullable.

I don't think being bound to the life cycle (that is, from realize on)
implies anything about being nullable.

For example, imagine two different types of UARTs with a compatible
interface, and you could choose whether to have one or the other on the
board. Maybe you could even use none at all (probably doesn't make a lot
of sense in this example, but I figure it might in other contexts).

So even though once the device is realized, the UART is bound to the
life cycle of your ISASerial, you wouldn't want to have the UART type
hard-coded, but leave the user a choice. Would this be modelled as a
link rather than a child?

Kevin

Re: [Qemu-devel] [PATCH v3 00/20] qom: dynamic properties and composition tree

[Anthony Liguori]

On 12/16/2011 08:18 AM, Kevin Wolf wrote:
> Am 16.12.2011 14:51, schrieb Anthony Liguori:
>> On 12/16/2011 06:24 AM, Paolo Bonzini wrote:
>>> On 12/16/2011 11:36 AM, Kevin Wolf wrote:
>>>>> I think actually this is not the biggest problem. child properties are
>>>>> dynamic, and it's not a problem IMO if they are created like that.
>>>>
>>>> That they are added in an init function is an indicator that they aren't
>>>> really dynamic.
>>>
>>> That's true. However, another indicator is that anything that does not have a
>>> struct field is also not really static. :)
>>>
>>> So right now, child properties are all "dynamic" in this sense. This could
>>> change when Anthony converts buses to QOM. The bus right now is embedded into
>>> the HBA's struct, is not a pointer. This likely would change when buses are
>>> QOM-ized, but then the bus would indeed be a 100% static child.
>>>
>>>> I think having a child property that can be NULL could be
>>>> reasonable.
>>>
>>> I think Anthony convinced me this is not the case (unlike links). Even if buses
>>> and similar objects are changed to pointers because the implementation needs
>>> that, those pointers should never be NULL (or if they can, the child property
>>> should not exist when they are NULL).
>>
>> What I would like to get to eventually is:
>>
>> struct ISASerial {
>>      Device parent;
>>
>>      UART _child uart;
>>      ISABus _link *bus;
>> };
>>
>> A child should be able to be part of the parent devices memory with its life
>> cycle bound to the parents life cycle.  This is why a child property shouldn't
>> be nullable.
>
> I don't think being bound to the life cycle (that is, from realize on)
> implies anything about being nullable.
>
> For example, imagine two different types of UARTs with a compatible
> interface, and you could choose whether to have one or the other on the
> board. Maybe you could even use none at all (probably doesn't make a lot
> of sense in this example, but I figure it might in other contexts).

What you're describing is what a link<> is.  Whenever you want the ability to 
make a choice (including the choice of None), a link<> is the type of property 
to use.

But too much choice can be a bad thing.  In many cases, you just want to have a 
child device for the purposes code sharing.  An ISA serial device embedding a 
UART is a good example of this.

Yes, you could make a UARTInterface and have the ISA serial device expose a 
link<UARTInterface> but that's roughly equivalent to having every chip on your 
motherboard be connected with a DIP package instead of being soldered on the 
board.  You could do it, but it would be very expensive and cumbersome.

> So even though once the device is realized, the UART is bound to the
> life cycle of your ISASerial, you wouldn't want to have the UART type
> hard-coded, but leave the user a choice. Would this be modelled as a
> link rather than a child?

Yes.  I'm not terribly sure how this would work yet.  A link and a child 
property both acquire references to a device and release a reference to a device 
at destruction time.

For a child property, the reference held by the parent is the only reference in 
existing.  For non-child properties, the 'peripheral' container also holds a 
reference (since you want to be able to assign the device somewhere else in the 
device model).

I'm not sure tying life cycles for a user created device makes sense.  If a user 
creates a device, IMO, the user should be the one to destroy the device.

Regards,

Anthony Liguori

>
> Kevin
>

Re: [Qemu-devel] [PATCH v3 00/20] qom: dynamic properties and composition tree

[Paolo Bonzini]

On 12/16/2011 03:54 PM, Anthony Liguori wrote:
>
> Yes.  I'm not terribly sure how this would work yet.  A link and a child
> property both acquire references to a device and release a reference to
> a device at destruction time.
>
> For a child property, the reference held by the parent is the only
> reference in existing.  For non-child properties, the 'peripheral'
> container also holds a reference (since you want to be able to assign
> the device somewhere else in the device model).
>
> I'm not sure tying life cycles for a user created device makes sense.
> If a user creates a device, IMO, the user should be the one to destroy
> the device.

I agree.

Paolo

Re: [Qemu-devel] [PATCH v3 00/20] qom: dynamic properties and composition tree

[Kevin Wolf]

Am 16.12.2011 15:54, schrieb Anthony Liguori:
> On 12/16/2011 08:18 AM, Kevin Wolf wrote:
>> Am 16.12.2011 14:51, schrieb Anthony Liguori:
>>> What I would like to get to eventually is:
>>>
>>> struct ISASerial {
>>>      Device parent;
>>>
>>>      UART _child uart;
>>>      ISABus _link *bus;
>>> };
>>>
>>> A child should be able to be part of the parent devices memory with its life
>>> cycle bound to the parents life cycle.  This is why a child property shouldn't
>>> be nullable.
>>
>> I don't think being bound to the life cycle (that is, from realize on)
>> implies anything about being nullable.
>>
>> For example, imagine two different types of UARTs with a compatible
>> interface, and you could choose whether to have one or the other on the
>> board. Maybe you could even use none at all (probably doesn't make a lot
>> of sense in this example, but I figure it might in other contexts).
> 
> What you're describing is what a link<> is.  Whenever you want the ability to 
> make a choice (including the choice of None), a link<> is the type of property 
> to use.
> 
> But too much choice can be a bad thing.  In many cases, you just want to have a 
> child device for the purposes code sharing.  An ISA serial device embedding a 
> UART is a good example of this.
> 
> Yes, you could make a UARTInterface and have the ISA serial device expose a 
> link<UARTInterface> but that's roughly equivalent to having every chip on your 
> motherboard be connected with a DIP package instead of being soldered on the 
> board.  You could do it, but it would be very expensive and cumbersome.

Sure, I'm not saying it's a practical thing to do in this case, I just
wanted to understand the way things are supposed to be modelled. I think
I understand now when it should be a child and when a link.

>> So even though once the device is realized, the UART is bound to the
>> life cycle of your ISASerial, you wouldn't want to have the UART type
>> hard-coded, but leave the user a choice. Would this be modelled as a
>> link rather than a child?
> 
> Yes.  I'm not terribly sure how this would work yet.  A link and a child 
> property both acquire references to a device and release a reference to a device 
> at destruction time.
> 
> For a child property, the reference held by the parent is the only reference in 
> existing.  For non-child properties, the 'peripheral' container also holds a 
> reference (since you want to be able to assign the device somewhere else in the 
> device model).
> 
> I'm not sure tying life cycles for a user created device makes sense.  If a user 
> creates a device, IMO, the user should be the one to destroy the device.

Yes, that might be the most consistent.

Kevin