[Qemu-devel] [PATCH 0/4] qdev: give all devices a canonical QOM path

[Qemu-devel] [PATCH 0/4] qdev: give all devices a canonical QOM path

[Paolo Bonzini]

A strong limitation of QOM right now is that unconverted ports
(e.g. all...) do not give a canonical path to devices that are
part of the board.  This in turn makes it impossible to replace
PROP_PTR with a QOM link for example.

This series fixes this by putting all such devices under a
/unattached container.

The problem is when to do this.  qdev_init is a good place, but it
requires the composition tree to be in order by the time we call it.
This is not the case so far, but it can be changed easily (patch 2) at
the cost of removing most of the few composition examples that are in the
tree.  This change is also necessary so that a single set of the topmost
realize property can propagate all the way down the composition tree,
as agreed on the mailing list.

At the end of the series, all devices are moved to /machine
in order to avoid cluttering too much the QOM root.

Paolo Bonzini (4):
  qom: add container_get
  qdev: add children before qdev_init
  qdev: give all devices a canonical path
  qdev: put all devices under /machine

 hw/pc_piix.c          |   18 +-----------------
 hw/piix_pci.c         |    3 +--
 hw/ppc_prep.c         |    2 +-
 hw/qdev-monitor.c     |   16 ++++++----------
 hw/qdev.c             |   16 +++++++++++++---
 include/qemu/object.h |   24 ++++++++++++++++++++++++
 qom/container.c       |   23 +++++++++++++++++++++++
 qom/object.c          |   33 +++++++++++++++++----------------
 8 files changed, 86 insertions(+), 49 deletions(-)

-- 
1.7.9.1

[Qemu-devel] [PATCH 1/4] qom: add container_get

[Paolo Bonzini]

This is QOM "mkdir -p".  It is useful when referring to
container objects such as "/machine".

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/qdev-monitor.c     |    8 ++------
 include/qemu/object.h |   24 ++++++++++++++++++++++++
 qom/container.c       |   23 +++++++++++++++++++++++
 qom/object.c          |   33 +++++++++++++++++----------------
 4 files changed, 66 insertions(+), 22 deletions(-)

diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c
index a310cc7..2e82962 100644
--- a/hw/qdev-monitor.c
+++ b/hw/qdev-monitor.c
@@ -180,9 +180,7 @@ static Object *qdev_get_peripheral(void)
     static Object *dev;
 
     if (dev == NULL) {
-        dev = object_new("container");
-        object_property_add_child(object_get_root(), "peripheral",
-                                  OBJECT(dev), NULL);
+        dev = container_get("/peripheral");
     }
 
     return dev;
@@ -193,9 +191,7 @@ static Object *qdev_get_peripheral_anon(void)
     static Object *dev;
 
     if (dev == NULL) {
-        dev = object_new("container");
-        object_property_add_child(object_get_root(), "peripheral-anon",
-                                  OBJECT(dev), NULL);
+        dev = container_get("/peripheral-anon");
     }
 
     return dev;
diff --git a/include/qemu/object.h b/include/qemu/object.h
index e8fc126..a675937 100644
--- a/include/qemu/object.h
+++ b/include/qemu/object.h
@@ -837,6 +837,18 @@ Object *object_resolve_path_type(const char *path, const char *typename,
                                  bool *ambiguous);
 
 /**
+ * object_resolve_path_component:
+ * @parent: the object in which to resolve the path
+ * @part: the component to resolve.
+ *
+ * This is similar to object_resolve_path with an absolute path, but it
+ * only resolves one element (@part) and takes the others from @parent.
+ *
+ * Returns: The resolved object or NULL on path lookup failure.
+ */
+Object *object_resolve_path_component(Object *parent, gchar *part);
+
+/**
  * object_property_add_child:
  * @obj: the object to add a property to
  * @name: the name of the property
@@ -891,4 +903,16 @@ void object_property_add_str(Object *obj, const char *name,
                              void (*set)(Object *, const char *, struct Error **),
                              struct Error **errp);
 
+/**
+ * container_get:
+ * @path: path to the container
+ *
+ * Return a container object whose path is @path.  Create more containers
+ * along the path if necessary.
+ *
+ * Returns: the container object.
+ */
+Object *container_get(const char *path);
+
+
 #endif
diff --git a/qom/container.c b/qom/container.c
index f107208..67e9e8a 100644
--- a/qom/container.c
+++ b/qom/container.c
@@ -12,6 +12,7 @@
 
 #include "qemu/object.h"
 #include "module.h"
+#include <assert.h>
 
 static TypeInfo container_info = {
     .name          = "container",
@@ -24,4 +25,26 @@ static void container_register_types(void)
     type_register_static(&container_info);
 }
 
+Object *container_get(const char *path)
+{
+    Object *obj, *child;
+    gchar **parts;
+    int i;
+
+    parts = g_strsplit(path, "/", 0);
+    assert(parts != NULL && parts[0] != NULL && !parts[0][0]);
+    obj = object_get_root();
+
+    for (i = 1; parts[i] != NULL; i++, obj = child) {
+        child = object_resolve_path_component(obj, parts[i]);
+        if (!child) {
+            child = object_new("container");
+            object_property_add_child(obj, parts[i], child, NULL);
+        }
+    }
+
+    return obj;
+}
+
+
 type_init(container_register_types)
diff --git a/qom/object.c b/qom/object.c
index 9cd9506..e721fc2 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -1022,12 +1022,27 @@ gchar *object_get_canonical_path(Object *obj)
     return newpath;
 }
 
+Object *object_resolve_path_component(Object *parent, gchar *part)
+{
+    ObjectProperty *prop = object_property_find(parent, part);
+    if (prop == NULL) {
+        return NULL;
+    }
+
+    if (strstart(prop->type, "link<", NULL)) {
+        return *(Object **)prop->opaque;
+    } else if (strstart(prop->type, "child<", NULL)) {
+        return prop->opaque;
+    } else {
+        return NULL;
+    }
+}
+
 static Object *object_resolve_abs_path(Object *parent,
                                           gchar **parts,
                                           const char *typename,
                                           int index)
 {
-    ObjectProperty *prop;
     Object *child;
 
     if (parts[index] == NULL) {
@@ -1038,21 +1053,7 @@ static Object *object_resolve_abs_path(Object *parent,
         return object_resolve_abs_path(parent, parts, typename, index + 1);
     }
 
-    prop = object_property_find(parent, parts[index]);
-    if (prop == NULL) {
-        return NULL;
-    }
-
-    child = NULL;
-    if (strstart(prop->type, "link<", NULL)) {
-        Object **pchild = prop->opaque;
-        if (*pchild) {
-            child = *pchild;
-        }
-    } else if (strstart(prop->type, "child<", NULL)) {
-        child = prop->opaque;
-    }
-
+    child = object_resolve_path_component(parent, parts[index]);
     if (!child) {
         return NULL;
     }
-- 
1.7.9.1

Re: [Qemu-devel] [PATCH 1/4] qom: add container_get

[Anthony Liguori]

On 03/27/2012 11:38 AM, Paolo Bonzini wrote:
> This is QOM "mkdir -p".  It is useful when referring to
> container objects such as "/machine".

That's a very interesting analogy...

Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>

With the big caveat of let's not abuse how easy this makes it to add depth to 
the hierarchy.

Regards,

Anthony Liguori

>
> Signed-off-by: Paolo Bonzini<pbonzini@redhat.com>
> ---
>   hw/qdev-monitor.c     |    8 ++------
>   include/qemu/object.h |   24 ++++++++++++++++++++++++
>   qom/container.c       |   23 +++++++++++++++++++++++
>   qom/object.c          |   33 +++++++++++++++++----------------
>   4 files changed, 66 insertions(+), 22 deletions(-)
>
> diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c
> index a310cc7..2e82962 100644
> --- a/hw/qdev-monitor.c
> +++ b/hw/qdev-monitor.c
> @@ -180,9 +180,7 @@ static Object *qdev_get_peripheral(void)
>       static Object *dev;
>
>       if (dev == NULL) {
> -        dev = object_new("container");
> -        object_property_add_child(object_get_root(), "peripheral",
> -                                  OBJECT(dev), NULL);
> +        dev = container_get("/peripheral");
>       }
>
>       return dev;
> @@ -193,9 +191,7 @@ static Object *qdev_get_peripheral_anon(void)
>       static Object *dev;
>
>       if (dev == NULL) {
> -        dev = object_new("container");
> -        object_property_add_child(object_get_root(), "peripheral-anon",
> -                                  OBJECT(dev), NULL);
> +        dev = container_get("/peripheral-anon");
>       }
>
>       return dev;
> diff --git a/include/qemu/object.h b/include/qemu/object.h
> index e8fc126..a675937 100644
> --- a/include/qemu/object.h
> +++ b/include/qemu/object.h
> @@ -837,6 +837,18 @@ Object *object_resolve_path_type(const char *path, const char *typename,
>                                    bool *ambiguous);
>
>   /**
> + * object_resolve_path_component:
> + * @parent: the object in which to resolve the path
> + * @part: the component to resolve.
> + *
> + * This is similar to object_resolve_path with an absolute path, but it
> + * only resolves one element (@part) and takes the others from @parent.
> + *
> + * Returns: The resolved object or NULL on path lookup failure.
> + */
> +Object *object_resolve_path_component(Object *parent, gchar *part);
> +
> +/**
>    * object_property_add_child:
>    * @obj: the object to add a property to
>    * @name: the name of the property
> @@ -891,4 +903,16 @@ void object_property_add_str(Object *obj, const char *name,
>                                void (*set)(Object *, const char *, struct Error **),
>                                struct Error **errp);
>
> +/**
> + * container_get:
> + * @path: path to the container
> + *
> + * Return a container object whose path is @path.  Create more containers
> + * along the path if necessary.
> + *
> + * Returns: the container object.
> + */
> +Object *container_get(const char *path);
> +
> +
>   #endif
> diff --git a/qom/container.c b/qom/container.c
> index f107208..67e9e8a 100644
> --- a/qom/container.c
> +++ b/qom/container.c
> @@ -12,6 +12,7 @@
>
>   #include "qemu/object.h"
>   #include "module.h"
> +#include<assert.h>
>
>   static TypeInfo container_info = {
>       .name          = "container",
> @@ -24,4 +25,26 @@ static void container_register_types(void)
>       type_register_static(&container_info);
>   }
>
> +Object *container_get(const char *path)
> +{
> +    Object *obj, *child;
> +    gchar **parts;
> +    int i;
> +
> +    parts = g_strsplit(path, "/", 0);
> +    assert(parts != NULL&&  parts[0] != NULL&&  !parts[0][0]);
> +    obj = object_get_root();
> +
> +    for (i = 1; parts[i] != NULL; i++, obj = child) {
> +        child = object_resolve_path_component(obj, parts[i]);
> +        if (!child) {
> +            child = object_new("container");
> +            object_property_add_child(obj, parts[i], child, NULL);
> +        }
> +    }
> +
> +    return obj;
> +}
> +
> +
>   type_init(container_register_types)
> diff --git a/qom/object.c b/qom/object.c
> index 9cd9506..e721fc2 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -1022,12 +1022,27 @@ gchar *object_get_canonical_path(Object *obj)
>       return newpath;
>   }
>
> +Object *object_resolve_path_component(Object *parent, gchar *part)
> +{
> +    ObjectProperty *prop = object_property_find(parent, part);
> +    if (prop == NULL) {
> +        return NULL;
> +    }
> +
> +    if (strstart(prop->type, "link<", NULL)) {
> +        return *(Object **)prop->opaque;
> +    } else if (strstart(prop->type, "child<", NULL)) {
> +        return prop->opaque;
> +    } else {
> +        return NULL;
> +    }
> +}
> +
>   static Object *object_resolve_abs_path(Object *parent,
>                                             gchar **parts,
>                                             const char *typename,
>                                             int index)
>   {
> -    ObjectProperty *prop;
>       Object *child;
>
>       if (parts[index] == NULL) {
> @@ -1038,21 +1053,7 @@ static Object *object_resolve_abs_path(Object *parent,
>           return object_resolve_abs_path(parent, parts, typename, index + 1);
>       }
>
> -    prop = object_property_find(parent, parts[index]);
> -    if (prop == NULL) {
> -        return NULL;
> -    }
> -
> -    child = NULL;
> -    if (strstart(prop->type, "link<", NULL)) {
> -        Object **pchild = prop->opaque;
> -        if (*pchild) {
> -            child = *pchild;
> -        }
> -    } else if (strstart(prop->type, "child<", NULL)) {
> -        child = prop->opaque;
> -    }
> -
> +    child = object_resolve_path_component(parent, parts[index]);
>       if (!child) {
>           return NULL;
>       }

[Qemu-devel] [PATCH 2/4] qdev: add children before qdev_init

[Paolo Bonzini]

We want the composition tree to to be in order by the time we call
qdev_init, so that a single set of the toplevel realize property can
propagate all the way down the composition tree.

This is not the case so far.  Unfortunately, this is incompatible
with calling qdev_init in the constructor wrappers for devices,
so for now we need to unattach some devices that are created through
those wrappers.  This will be fixed by removing qdev_init and instead
setting the toplevel realize property after machine init.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/pc_piix.c      |   18 +-----------------
 hw/piix_pci.c     |    3 +--
 hw/ppc_prep.c     |    2 +-
 hw/qdev-monitor.c |    8 ++++----
 4 files changed, 7 insertions(+), 24 deletions(-)

diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index 3f99f9a..747c40f 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -146,7 +146,6 @@ static void pc_init1(MemoryRegion *system_memory,
     MemoryRegion *ram_memory;
     MemoryRegion *pci_memory;
     MemoryRegion *rom_memory;
-    DeviceState *dev;
 
     pc_cpus_init(cpu_model);
 
@@ -224,11 +223,7 @@ static void pc_init1(MemoryRegion *system_memory,
 
     pc_register_ferr_irq(gsi[13]);
 
-    dev = pc_vga_init(isa_bus, pci_enabled ? pci_bus : NULL);
-    if (dev) {
-        object_property_add_child(object_get_root(), "vga", OBJECT(dev), NULL);
-    }
-
+    pc_vga_init(isa_bus, pci_enabled ? pci_bus : NULL);
     if (xen_enabled()) {
         pci_create_simple(pci_bus, -1, "xen-platform");
     }
@@ -255,17 +250,6 @@ static void pc_init1(MemoryRegion *system_memory,
         }
         idebus[0] = qdev_get_child_bus(&dev->qdev, "ide.0");
         idebus[1] = qdev_get_child_bus(&dev->qdev, "ide.1");
-
-        /* 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.
-         */
-        object_property_add_child(object_resolve_path("/i440fx/piix3", NULL),
-                                  "rtc", (Object *)rtc_state, NULL);
     } else {
         for(i = 0; i < MAX_IDE_BUS; i++) {
             ISADevice *dev;
diff --git a/hw/piix_pci.c b/hw/piix_pci.c
index e0268fe..9017565 100644
--- a/hw/piix_pci.c
+++ b/hw/piix_pci.c
@@ -276,8 +276,8 @@ static PCIBus *i440fx_common_init(const char *device_name,
     b = pci_bus_new(&s->busdev.qdev, NULL, pci_address_space,
                     address_space_io, 0);
     s->bus = b;
-    qdev_init_nofail(dev);
     object_property_add_child(object_get_root(), "i440fx", OBJECT(dev), NULL);
+    qdev_init_nofail(dev);
 
     d = pci_create_simple(b, 0, device_name);
     *pi440fx_state = DO_UPCAST(PCII440FXState, dev, d);
@@ -316,7 +316,6 @@ static PCIBus *i440fx_common_init(const char *device_name,
         pci_bus_irqs(b, piix3_set_irq, pci_slot_get_pirq, piix3,
                 PIIX_NUM_PIRQS);
     }
-    object_property_add_child(OBJECT(dev), "piix3", OBJECT(piix3), NULL);
     piix3->pic = pic;
     *isa_bus = DO_UPCAST(ISABus, qbus,
                          qdev_get_child_bus(&piix3->dev.qdev, "isa.0"));
diff --git a/hw/ppc_prep.c b/hw/ppc_prep.c
index 06d589d..86c9336 100644
--- a/hw/ppc_prep.c
+++ b/hw/ppc_prep.c
@@ -615,8 +615,8 @@ static void ppc_prep_init (ram_addr_t ram_size,
     sys = sysbus_from_qdev(dev);
     pcihost = DO_UPCAST(PCIHostState, busdev, sys);
     pcihost->address_space = get_system_memory();
-    qdev_init_nofail(dev);
     object_property_add_child(object_get_root(), "raven", OBJECT(dev), NULL);
+    qdev_init_nofail(dev);
     pci_bus = (PCIBus *)qdev_get_child_bus(dev, "pci.0");
     if (pci_bus == NULL) {
         fprintf(stderr, "Couldn't create PCI host controller.\n");
diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c
index 2e82962..031cb83 100644
--- a/hw/qdev-monitor.c
+++ b/hw/qdev-monitor.c
@@ -458,10 +458,6 @@ DeviceState *qdev_device_add(QemuOpts *opts)
         qdev_free(qdev);
         return NULL;
     }
-    if (qdev_init(qdev) < 0) {
-        qerror_report(QERR_DEVICE_INIT_FAILED, driver);
-        return NULL;
-    }
     if (qdev->id) {
         object_property_add_child(qdev_get_peripheral(), qdev->id,
                                   OBJECT(qdev), NULL);
@@ -472,6 +468,10 @@ DeviceState *qdev_device_add(QemuOpts *opts)
                                   OBJECT(qdev), NULL);
         g_free(name);
     }        
+    if (qdev_init(qdev) < 0) {
+        qerror_report(QERR_DEVICE_INIT_FAILED, driver);
+        return NULL;
+    }
     qdev->opts = opts;
     return qdev;
 }
-- 
1.7.9.1

Re: [Qemu-devel] [PATCH 2/4] qdev: add children before qdev_init

[Anthony Liguori]

On 03/27/2012 11:38 AM, Paolo Bonzini wrote:
> We want the composition tree to to be in order by the time we call
> qdev_init, so that a single set of the toplevel realize property can
> propagate all the way down the composition tree.
>
> This is not the case so far.  Unfortunately, this is incompatible
> with calling qdev_init in the constructor wrappers for devices,
> so for now we need to unattach some devices that are created through
> those wrappers.  This will be fixed by removing qdev_init and instead
> setting the toplevel realize property after machine init.
>
> Signed-off-by: Paolo Bonzini<pbonzini@redhat.com>

Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>

Regards,

Anthony Liguori

> ---
>   hw/pc_piix.c      |   18 +-----------------
>   hw/piix_pci.c     |    3 +--
>   hw/ppc_prep.c     |    2 +-
>   hw/qdev-monitor.c |    8 ++++----
>   4 files changed, 7 insertions(+), 24 deletions(-)
>
> diff --git a/hw/pc_piix.c b/hw/pc_piix.c
> index 3f99f9a..747c40f 100644
> --- a/hw/pc_piix.c
> +++ b/hw/pc_piix.c
> @@ -146,7 +146,6 @@ static void pc_init1(MemoryRegion *system_memory,
>       MemoryRegion *ram_memory;
>       MemoryRegion *pci_memory;
>       MemoryRegion *rom_memory;
> -    DeviceState *dev;
>
>       pc_cpus_init(cpu_model);
>
> @@ -224,11 +223,7 @@ static void pc_init1(MemoryRegion *system_memory,
>
>       pc_register_ferr_irq(gsi[13]);
>
> -    dev = pc_vga_init(isa_bus, pci_enabled ? pci_bus : NULL);
> -    if (dev) {
> -        object_property_add_child(object_get_root(), "vga", OBJECT(dev), NULL);
> -    }
> -
> +    pc_vga_init(isa_bus, pci_enabled ? pci_bus : NULL);
>       if (xen_enabled()) {
>           pci_create_simple(pci_bus, -1, "xen-platform");
>       }
> @@ -255,17 +250,6 @@ static void pc_init1(MemoryRegion *system_memory,
>           }
>           idebus[0] = qdev_get_child_bus(&dev->qdev, "ide.0");
>           idebus[1] = qdev_get_child_bus(&dev->qdev, "ide.1");
> -
> -        /* 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.
> -         */
> -        object_property_add_child(object_resolve_path("/i440fx/piix3", NULL),
> -                                  "rtc", (Object *)rtc_state, NULL);
>       } else {
>           for(i = 0; i<  MAX_IDE_BUS; i++) {
>               ISADevice *dev;
> diff --git a/hw/piix_pci.c b/hw/piix_pci.c
> index e0268fe..9017565 100644
> --- a/hw/piix_pci.c
> +++ b/hw/piix_pci.c
> @@ -276,8 +276,8 @@ static PCIBus *i440fx_common_init(const char *device_name,
>       b = pci_bus_new(&s->busdev.qdev, NULL, pci_address_space,
>                       address_space_io, 0);
>       s->bus = b;
> -    qdev_init_nofail(dev);
>       object_property_add_child(object_get_root(), "i440fx", OBJECT(dev), NULL);
> +    qdev_init_nofail(dev);
>
>       d = pci_create_simple(b, 0, device_name);
>       *pi440fx_state = DO_UPCAST(PCII440FXState, dev, d);
> @@ -316,7 +316,6 @@ static PCIBus *i440fx_common_init(const char *device_name,
>           pci_bus_irqs(b, piix3_set_irq, pci_slot_get_pirq, piix3,
>                   PIIX_NUM_PIRQS);
>       }
> -    object_property_add_child(OBJECT(dev), "piix3", OBJECT(piix3), NULL);
>       piix3->pic = pic;
>       *isa_bus = DO_UPCAST(ISABus, qbus,
>                            qdev_get_child_bus(&piix3->dev.qdev, "isa.0"));
> diff --git a/hw/ppc_prep.c b/hw/ppc_prep.c
> index 06d589d..86c9336 100644
> --- a/hw/ppc_prep.c
> +++ b/hw/ppc_prep.c
> @@ -615,8 +615,8 @@ static void ppc_prep_init (ram_addr_t ram_size,
>       sys = sysbus_from_qdev(dev);
>       pcihost = DO_UPCAST(PCIHostState, busdev, sys);
>       pcihost->address_space = get_system_memory();
> -    qdev_init_nofail(dev);
>       object_property_add_child(object_get_root(), "raven", OBJECT(dev), NULL);
> +    qdev_init_nofail(dev);
>       pci_bus = (PCIBus *)qdev_get_child_bus(dev, "pci.0");
>       if (pci_bus == NULL) {
>           fprintf(stderr, "Couldn't create PCI host controller.\n");
> diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c
> index 2e82962..031cb83 100644
> --- a/hw/qdev-monitor.c
> +++ b/hw/qdev-monitor.c
> @@ -458,10 +458,6 @@ DeviceState *qdev_device_add(QemuOpts *opts)
>           qdev_free(qdev);
>           return NULL;
>       }
> -    if (qdev_init(qdev)<  0) {
> -        qerror_report(QERR_DEVICE_INIT_FAILED, driver);
> -        return NULL;
> -    }
>       if (qdev->id) {
>           object_property_add_child(qdev_get_peripheral(), qdev->id,
>                                     OBJECT(qdev), NULL);
> @@ -472,6 +468,10 @@ DeviceState *qdev_device_add(QemuOpts *opts)
>                                     OBJECT(qdev), NULL);
>           g_free(name);
>       }
> +    if (qdev_init(qdev)<  0) {
> +        qerror_report(QERR_DEVICE_INIT_FAILED, driver);
> +        return NULL;
> +    }
>       qdev->opts = opts;
>       return qdev;
>   }

[Qemu-devel] [PATCH 3/4] qdev: give all devices a canonical path

[Paolo Bonzini]

A strong limitation of QOM right now is that unconverted ports
(e.g. all...) do not give a canonical path to devices that are
part of the board.  This in turn makes it impossible to replace
PROP_PTR with a QOM link for example.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/qdev.c |   16 +++++++++++++---
 1 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/hw/qdev.c b/hw/qdev.c
index ee21d90..f5c716e 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -113,14 +113,14 @@ DeviceState *qdev_create(BusState *bus, const char *name)
     return dev;
 }
 
-DeviceState *qdev_try_create(BusState *bus, const char *name)
+DeviceState *qdev_try_create(BusState *bus, const char *type)
 {
     DeviceState *dev;
 
-    if (object_class_by_name(name) == NULL) {
+    if (object_class_by_name(type) == NULL) {
         return NULL;
     }
-    dev = DEVICE(object_new(name));
+    dev = DEVICE(object_new(type));
     if (!dev) {
         return NULL;
     }
@@ -152,6 +152,16 @@ int qdev_init(DeviceState *dev)
         qdev_free(dev);
         return rc;
     }
+
+    if (!OBJECT(dev)->parent) {
+        static int unattached_count = 0;
+        gchar *name = g_strdup_printf("device[%d]", unattached_count++);
+
+        object_property_add_child(container_get("/unattached"), name,
+                                  OBJECT(dev), NULL);
+        g_free(name);
+    }
+
     if (qdev_get_vmsd(dev)) {
         vmstate_register_with_alias_id(dev, -1, qdev_get_vmsd(dev), dev,
                                        dev->instance_id_alias,
-- 
1.7.9.1

Re: [Qemu-devel] [PATCH 3/4] qdev: give all devices a canonical path

[Anthony Liguori]

On 03/27/2012 11:38 AM, Paolo Bonzini wrote:
> A strong limitation of QOM right now is that unconverted ports
> (e.g. all...) do not give a canonical path to devices that are
> part of the board.  This in turn makes it impossible to replace
> PROP_PTR with a QOM link for example.
>
> Signed-off-by: Paolo Bonzini<pbonzini@redhat.com>

Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>

Regards,

Anthony Liguori

> ---
>   hw/qdev.c |   16 +++++++++++++---
>   1 files changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/hw/qdev.c b/hw/qdev.c
> index ee21d90..f5c716e 100644
> --- a/hw/qdev.c
> +++ b/hw/qdev.c
> @@ -113,14 +113,14 @@ DeviceState *qdev_create(BusState *bus, const char *name)
>       return dev;
>   }
>
> -DeviceState *qdev_try_create(BusState *bus, const char *name)
> +DeviceState *qdev_try_create(BusState *bus, const char *type)
>   {
>       DeviceState *dev;
>
> -    if (object_class_by_name(name) == NULL) {
> +    if (object_class_by_name(type) == NULL) {
>           return NULL;
>       }
> -    dev = DEVICE(object_new(name));
> +    dev = DEVICE(object_new(type));
>       if (!dev) {
>           return NULL;
>       }
> @@ -152,6 +152,16 @@ int qdev_init(DeviceState *dev)
>           qdev_free(dev);
>           return rc;
>       }
> +
> +    if (!OBJECT(dev)->parent) {
> +        static int unattached_count = 0;
> +        gchar *name = g_strdup_printf("device[%d]", unattached_count++);
> +
> +        object_property_add_child(container_get("/unattached"), name,
> +                                  OBJECT(dev), NULL);
> +        g_free(name);
> +    }
> +
>       if (qdev_get_vmsd(dev)) {
>           vmstate_register_with_alias_id(dev, -1, qdev_get_vmsd(dev), dev,
>                                          dev->instance_id_alias,

[Qemu-devel] [PATCH 4/4] qdev: put all devices under /machine

[Paolo Bonzini]

Avoid cluttering too much the QOM root.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/piix_pci.c     |    2 +-
 hw/ppc_prep.c     |    2 +-
 hw/qdev-monitor.c |    4 ++--
 hw/qdev.c         |    2 +-
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/hw/piix_pci.c b/hw/piix_pci.c
index 9017565..bd20a16 100644
--- a/hw/piix_pci.c
+++ b/hw/piix_pci.c
@@ -276,7 +276,7 @@ static PCIBus *i440fx_common_init(const char *device_name,
     b = pci_bus_new(&s->busdev.qdev, NULL, pci_address_space,
                     address_space_io, 0);
     s->bus = b;
-    object_property_add_child(object_get_root(), "i440fx", OBJECT(dev), NULL);
+    object_property_add_child(container_get("/machine"), "i440fx", OBJECT(dev), NULL);
     qdev_init_nofail(dev);
 
     d = pci_create_simple(b, 0, device_name);
diff --git a/hw/ppc_prep.c b/hw/ppc_prep.c
index 86c9336..d06fc5e 100644
--- a/hw/ppc_prep.c
+++ b/hw/ppc_prep.c
@@ -615,7 +615,7 @@ static void ppc_prep_init (ram_addr_t ram_size,
     sys = sysbus_from_qdev(dev);
     pcihost = DO_UPCAST(PCIHostState, busdev, sys);
     pcihost->address_space = get_system_memory();
-    object_property_add_child(object_get_root(), "raven", OBJECT(dev), NULL);
+    object_property_add_child(container_get("/machine", "raven", OBJECT(dev), NULL);
     qdev_init_nofail(dev);
     pci_bus = (PCIBus *)qdev_get_child_bus(dev, "pci.0");
     if (pci_bus == NULL) {
diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c
index 031cb83..4783366 100644
--- a/hw/qdev-monitor.c
+++ b/hw/qdev-monitor.c
@@ -180,7 +180,7 @@ static Object *qdev_get_peripheral(void)
     static Object *dev;
 
     if (dev == NULL) {
-        dev = container_get("/peripheral");
+        dev = container_get("/machine/peripheral");
     }
 
     return dev;
@@ -191,7 +191,7 @@ static Object *qdev_get_peripheral_anon(void)
     static Object *dev;
 
     if (dev == NULL) {
-        dev = container_get("/peripheral-anon");
+        dev = container_get("/machine/peripheral-anon");
     }
 
     return dev;
diff --git a/hw/qdev.c b/hw/qdev.c
index f5c716e..60e5081 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -157,7 +157,7 @@ int qdev_init(DeviceState *dev)
         static int unattached_count = 0;
         gchar *name = g_strdup_printf("device[%d]", unattached_count++);
 
-        object_property_add_child(container_get("/unattached"), name,
+        object_property_add_child(container_get("/machine/unattached"), name,
                                   OBJECT(dev), NULL);
         g_free(name);
     }
-- 
1.7.9.1

Re: [Qemu-devel] [PATCH 4/4] qdev: put all devices under /machine

[Anthony Liguori]

On 03/27/2012 11:38 AM, Paolo Bonzini wrote:
> Avoid cluttering too much the QOM root.
>
> Signed-off-by: Paolo Bonzini<pbonzini@redhat.com>

Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>

Regards,

Anthony Liguori

> ---
>   hw/piix_pci.c     |    2 +-
>   hw/ppc_prep.c     |    2 +-
>   hw/qdev-monitor.c |    4 ++--
>   hw/qdev.c         |    2 +-
>   4 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/hw/piix_pci.c b/hw/piix_pci.c
> index 9017565..bd20a16 100644
> --- a/hw/piix_pci.c
> +++ b/hw/piix_pci.c
> @@ -276,7 +276,7 @@ static PCIBus *i440fx_common_init(const char *device_name,
>       b = pci_bus_new(&s->busdev.qdev, NULL, pci_address_space,
>                       address_space_io, 0);
>       s->bus = b;
> -    object_property_add_child(object_get_root(), "i440fx", OBJECT(dev), NULL);
> +    object_property_add_child(container_get("/machine"), "i440fx", OBJECT(dev), NULL);
>       qdev_init_nofail(dev);
>
>       d = pci_create_simple(b, 0, device_name);
> diff --git a/hw/ppc_prep.c b/hw/ppc_prep.c
> index 86c9336..d06fc5e 100644
> --- a/hw/ppc_prep.c
> +++ b/hw/ppc_prep.c
> @@ -615,7 +615,7 @@ static void ppc_prep_init (ram_addr_t ram_size,
>       sys = sysbus_from_qdev(dev);
>       pcihost = DO_UPCAST(PCIHostState, busdev, sys);
>       pcihost->address_space = get_system_memory();
> -    object_property_add_child(object_get_root(), "raven", OBJECT(dev), NULL);
> +    object_property_add_child(container_get("/machine", "raven", OBJECT(dev), NULL);
>       qdev_init_nofail(dev);
>       pci_bus = (PCIBus *)qdev_get_child_bus(dev, "pci.0");
>       if (pci_bus == NULL) {
> diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c
> index 031cb83..4783366 100644
> --- a/hw/qdev-monitor.c
> +++ b/hw/qdev-monitor.c
> @@ -180,7 +180,7 @@ static Object *qdev_get_peripheral(void)
>       static Object *dev;
>
>       if (dev == NULL) {
> -        dev = container_get("/peripheral");
> +        dev = container_get("/machine/peripheral");
>       }
>
>       return dev;
> @@ -191,7 +191,7 @@ static Object *qdev_get_peripheral_anon(void)
>       static Object *dev;
>
>       if (dev == NULL) {
> -        dev = container_get("/peripheral-anon");
> +        dev = container_get("/machine/peripheral-anon");
>       }
>
>       return dev;
> diff --git a/hw/qdev.c b/hw/qdev.c
> index f5c716e..60e5081 100644
> --- a/hw/qdev.c
> +++ b/hw/qdev.c
> @@ -157,7 +157,7 @@ int qdev_init(DeviceState *dev)
>           static int unattached_count = 0;
>           gchar *name = g_strdup_printf("device[%d]", unattached_count++);
>
> -        object_property_add_child(container_get("/unattached"), name,
> +        object_property_add_child(container_get("/machine/unattached"), name,
>                                     OBJECT(dev), NULL);
>           g_free(name);
>       }

Re: [Qemu-devel] [PATCH 4/4] qdev: put all devices under /machine

[Andreas Färber]

Am 27.03.2012 23:11, schrieb Anthony Liguori:
> On 03/27/2012 11:38 AM, Paolo Bonzini wrote:
>> Avoid cluttering too much the QOM root.
>>
>> Signed-off-by: Paolo Bonzini<pbonzini@redhat.com>
> 
> Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>

Not terribly convinced, but no better suggestion.

Could you please not hardcode this everywhere but supply a machine_get()
helper? My idea would be that what is now just a container gets replaced
by the future QOM machine object.

I.e. object_get_root() -> machine_get() or whatever we want to call it,
calling container_get("/machine").

Andreas

> 
> Regards,
> 
> Anthony Liguori
> 
>> ---
>>   hw/piix_pci.c     |    2 +-
>>   hw/ppc_prep.c     |    2 +-
>>   hw/qdev-monitor.c |    4 ++--
>>   hw/qdev.c         |    2 +-
>>   4 files changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/hw/piix_pci.c b/hw/piix_pci.c
>> index 9017565..bd20a16 100644
>> --- a/hw/piix_pci.c
>> +++ b/hw/piix_pci.c
>> @@ -276,7 +276,7 @@ static PCIBus *i440fx_common_init(const char
>> *device_name,
>>       b = pci_bus_new(&s->busdev.qdev, NULL, pci_address_space,
>>                       address_space_io, 0);
>>       s->bus = b;
>> -    object_property_add_child(object_get_root(), "i440fx",
>> OBJECT(dev), NULL);
>> +    object_property_add_child(container_get("/machine"), "i440fx",
>> OBJECT(dev), NULL);
>>       qdev_init_nofail(dev);
>>
>>       d = pci_create_simple(b, 0, device_name);
>> diff --git a/hw/ppc_prep.c b/hw/ppc_prep.c
>> index 86c9336..d06fc5e 100644
>> --- a/hw/ppc_prep.c
>> +++ b/hw/ppc_prep.c
>> @@ -615,7 +615,7 @@ static void ppc_prep_init (ram_addr_t ram_size,
>>       sys = sysbus_from_qdev(dev);
>>       pcihost = DO_UPCAST(PCIHostState, busdev, sys);
>>       pcihost->address_space = get_system_memory();
>> -    object_property_add_child(object_get_root(), "raven",
>> OBJECT(dev), NULL);
>> +    object_property_add_child(container_get("/machine", "raven",
>> OBJECT(dev), NULL);
>>       qdev_init_nofail(dev);
>>       pci_bus = (PCIBus *)qdev_get_child_bus(dev, "pci.0");
>>       if (pci_bus == NULL) {
>> diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c
>> index 031cb83..4783366 100644
>> --- a/hw/qdev-monitor.c
>> +++ b/hw/qdev-monitor.c
>> @@ -180,7 +180,7 @@ static Object *qdev_get_peripheral(void)
>>       static Object *dev;
>>
>>       if (dev == NULL) {
>> -        dev = container_get("/peripheral");
>> +        dev = container_get("/machine/peripheral");
>>       }
>>
>>       return dev;
>> @@ -191,7 +191,7 @@ static Object *qdev_get_peripheral_anon(void)
>>       static Object *dev;
>>
>>       if (dev == NULL) {
>> -        dev = container_get("/peripheral-anon");
>> +        dev = container_get("/machine/peripheral-anon");
>>       }
>>
>>       return dev;
>> diff --git a/hw/qdev.c b/hw/qdev.c
>> index f5c716e..60e5081 100644
>> --- a/hw/qdev.c
>> +++ b/hw/qdev.c
>> @@ -157,7 +157,7 @@ int qdev_init(DeviceState *dev)
>>           static int unattached_count = 0;
>>           gchar *name = g_strdup_printf("device[%d]",
>> unattached_count++);
>>
>> -        object_property_add_child(container_get("/unattached"), name,
>> +       
>> object_property_add_child(container_get("/machine/unattached"), name,
>>                                     OBJECT(dev), NULL);
>>           g_free(name);
>>       }
> 


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

[Qemu-devel] [PATCH v2 4/4] qdev: put all devices under /machine

[Paolo Bonzini]

Avoid cluttering too much the QOM root.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
	v1->v2: add qdev_get_machine() and use it.

 hw/piix_pci.c     |    2 +-
 hw/ppc_prep.c     |    2 +-
 hw/qdev-monitor.c |    4 ++--
 hw/qdev.c         |   13 ++++++++++++-
 hw/qdev.h         |    2 ++
 5 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/hw/piix_pci.c b/hw/piix_pci.c
index 9017565..179d9a6 100644
--- a/hw/piix_pci.c
+++ b/hw/piix_pci.c
@@ -276,7 +276,7 @@ static PCIBus *i440fx_common_init(const char *device_name,
     b = pci_bus_new(&s->busdev.qdev, NULL, pci_address_space,
                     address_space_io, 0);
     s->bus = b;
-    object_property_add_child(object_get_root(), "i440fx", OBJECT(dev), NULL);
+    object_property_add_child(qdev_get_machine(), "i440fx", OBJECT(dev), NULL);
     qdev_init_nofail(dev);
 
     d = pci_create_simple(b, 0, device_name);
diff --git a/hw/ppc_prep.c b/hw/ppc_prep.c
index 86c9336..9d8e659 100644
--- a/hw/ppc_prep.c
+++ b/hw/ppc_prep.c
@@ -615,7 +615,7 @@ static void ppc_prep_init (ram_addr_t ram_size,
     sys = sysbus_from_qdev(dev);
     pcihost = DO_UPCAST(PCIHostState, busdev, sys);
     pcihost->address_space = get_system_memory();
-    object_property_add_child(object_get_root(), "raven", OBJECT(dev), NULL);
+    object_property_add_child(qdev_get_machine(), "raven", OBJECT(dev), NULL);
     qdev_init_nofail(dev);
     pci_bus = (PCIBus *)qdev_get_child_bus(dev, "pci.0");
     if (pci_bus == NULL) {
diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c
index 031cb83..4783366 100644
--- a/hw/qdev-monitor.c
+++ b/hw/qdev-monitor.c
@@ -180,7 +180,7 @@ static Object *qdev_get_peripheral(void)
     static Object *dev;
 
     if (dev == NULL) {
-        dev = container_get("/peripheral");
+        dev = container_get("/machine/peripheral");
     }
 
     return dev;
@@ -191,7 +191,7 @@ static Object *qdev_get_peripheral_anon(void)
     static Object *dev;
 
     if (dev == NULL) {
-        dev = container_get("/peripheral-anon");
+        dev = container_get("/machine/peripheral-anon");
     }
 
     return dev;
diff --git a/hw/qdev.c b/hw/qdev.c
index f5c716e..0d3c0fc 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -157,7 +157,7 @@ int qdev_init(DeviceState *dev)
         static int unattached_count = 0;
         gchar *name = g_strdup_printf("device[%d]", unattached_count++);
 
-        object_property_add_child(container_get("/unattached"), name,
+        object_property_add_child(container_get("/machine/unattached"), name,
                                   OBJECT(dev), NULL);
         g_free(name);
     }
@@ -668,6 +668,17 @@ void device_reset(DeviceState *dev)
     }
 }
 
+Object *qdev_get_machine(void)
+{
+    static Object *dev;
+
+    if (dev == NULL) {
+        dev = container_get("/machine");
+    }
+
+    return dev;
+}
+
 static TypeInfo device_type_info = {
     .name = TYPE_DEVICE,
     .parent = TYPE_OBJECT,
diff --git a/hw/qdev.h b/hw/qdev.h
index 9cc3f98..a8df42f 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -349,6 +349,8 @@ BusInfo *qdev_get_bus_info(DeviceState *dev);
 
 Property *qdev_get_props(DeviceState *dev);
 
+Object *qdev_get_machine(void);
+
 /* FIXME: make this a link<> */
 void qdev_set_parent_bus(DeviceState *dev, BusState *bus);
 
-- 
1.7.9.1

Re: [Qemu-devel] [PATCH v2 4/4] qdev: put all devices under /machine

[Andreas Färber]

Am 28.03.2012 16:34, schrieb Paolo Bonzini:
> Avoid cluttering too much the QOM root.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> 	v1->v2: add qdev_get_machine() and use it.

Thanks,

> 
>  hw/piix_pci.c     |    2 +-
>  hw/ppc_prep.c     |    2 +-
>  hw/qdev-monitor.c |    4 ++--
>  hw/qdev.c         |   13 ++++++++++++-
>  hw/qdev.h         |    2 ++
>  5 files changed, 18 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/piix_pci.c b/hw/piix_pci.c
> index 9017565..179d9a6 100644
> --- a/hw/piix_pci.c
> +++ b/hw/piix_pci.c
> @@ -276,7 +276,7 @@ static PCIBus *i440fx_common_init(const char *device_name,
>      b = pci_bus_new(&s->busdev.qdev, NULL, pci_address_space,
>                      address_space_io, 0);
>      s->bus = b;
> -    object_property_add_child(object_get_root(), "i440fx", OBJECT(dev), NULL);
> +    object_property_add_child(qdev_get_machine(), "i440fx", OBJECT(dev), NULL);
>      qdev_init_nofail(dev);
>  
>      d = pci_create_simple(b, 0, device_name);
> diff --git a/hw/ppc_prep.c b/hw/ppc_prep.c
> index 86c9336..9d8e659 100644
> --- a/hw/ppc_prep.c
> +++ b/hw/ppc_prep.c
> @@ -615,7 +615,7 @@ static void ppc_prep_init (ram_addr_t ram_size,
>      sys = sysbus_from_qdev(dev);
>      pcihost = DO_UPCAST(PCIHostState, busdev, sys);
>      pcihost->address_space = get_system_memory();
> -    object_property_add_child(object_get_root(), "raven", OBJECT(dev), NULL);
> +    object_property_add_child(qdev_get_machine(), "raven", OBJECT(dev), NULL);
>      qdev_init_nofail(dev);
>      pci_bus = (PCIBus *)qdev_get_child_bus(dev, "pci.0");
>      if (pci_bus == NULL) {
> diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c
> index 031cb83..4783366 100644
> --- a/hw/qdev-monitor.c
> +++ b/hw/qdev-monitor.c
> @@ -180,7 +180,7 @@ static Object *qdev_get_peripheral(void)
>      static Object *dev;
>  
>      if (dev == NULL) {
> -        dev = container_get("/peripheral");
> +        dev = container_get("/machine/peripheral");

I was kinda hoping we could even do something like this in 1/4:
container_get_relative(qdev_get_machine(), "peripheral");
w/ container_get(bla) -> container_get_relative(object_get_root(), bla).

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH v2 4/4] qdev: put all devices under /machine

[Paolo Bonzini]

Il 28/03/2012 17:10, Andreas Färber ha scritto:
>> >      if (dev == NULL) {
>> > -        dev = container_get("/peripheral");
>> > +        dev = container_get("/machine/peripheral");
> I was kinda hoping we could even do something like this in 1/4:
> container_get_relative(qdev_get_machine(), "peripheral");
> w/ container_get(bla) -> container_get_relative(object_get_root(), bla).

Follow-up? O:-)

Paolo

[Qemu-devel] [PATCH] qom: Refine container_get() to allow using a custom root

[Andreas Färber]

Specify the root to search from as argument. This avoids hardcoding
"/machine" in some places and makes it more flexible.

Signed-off-by: Andreas Färber <afaerber@suse.de>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Anthony Liguori <anthony@codemonkey.ws>
---
 hw/qdev-monitor.c     |    4 ++--
 hw/qdev.c             |    7 ++++---
 include/qemu/object.h |    3 ++-
 qom/container.c       |    4 ++--
 4 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c
index 4783366..67f296b 100644
--- a/hw/qdev-monitor.c
+++ b/hw/qdev-monitor.c
@@ -180,7 +180,7 @@ static Object *qdev_get_peripheral(void)
     static Object *dev;
 
     if (dev == NULL) {
-        dev = container_get("/machine/peripheral");
+        dev = container_get(qdev_get_machine(), "/peripheral");
     }
 
     return dev;
@@ -191,7 +191,7 @@ static Object *qdev_get_peripheral_anon(void)
     static Object *dev;
 
     if (dev == NULL) {
-        dev = container_get("/machine/peripheral-anon");
+        dev = container_get(qdev_get_machine(), "/peripheral-anon");
     }
 
     return dev;
diff --git a/hw/qdev.c b/hw/qdev.c
index 0d3c0fc..efa4c5d 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -157,8 +157,9 @@ int qdev_init(DeviceState *dev)
         static int unattached_count = 0;
         gchar *name = g_strdup_printf("device[%d]", unattached_count++);
 
-        object_property_add_child(container_get("/machine/unattached"), name,
-                                  OBJECT(dev), NULL);
+        object_property_add_child(container_get(qdev_get_machine(),
+                                                "/unattached"),
+                                  name, OBJECT(dev), NULL);
         g_free(name);
     }
 
@@ -673,7 +674,7 @@ Object *qdev_get_machine(void)
     static Object *dev;
 
     if (dev == NULL) {
-        dev = container_get("/machine");
+        dev = container_get(object_get_root(), "/machine");
     }
 
     return dev;
diff --git a/include/qemu/object.h b/include/qemu/object.h
index a675937..ca1649c 100644
--- a/include/qemu/object.h
+++ b/include/qemu/object.h
@@ -905,6 +905,7 @@ void object_property_add_str(Object *obj, const char *name,
 
 /**
  * container_get:
+ * @root: root of the #path, e.g., object_get_root()
  * @path: path to the container
  *
  * Return a container object whose path is @path.  Create more containers
@@ -912,7 +913,7 @@ void object_property_add_str(Object *obj, const char *name,
  *
  * Returns: the container object.
  */
-Object *container_get(const char *path);
+Object *container_get(Object *root, const char *path);
 
 
 #endif
diff --git a/qom/container.c b/qom/container.c
index 67e9e8a..c9940ab 100644
--- a/qom/container.c
+++ b/qom/container.c
@@ -25,7 +25,7 @@ static void container_register_types(void)
     type_register_static(&container_info);
 }
 
-Object *container_get(const char *path)
+Object *container_get(Object *root, const char *path)
 {
     Object *obj, *child;
     gchar **parts;
@@ -33,7 +33,7 @@ Object *container_get(const char *path)
 
     parts = g_strsplit(path, "/", 0);
     assert(parts != NULL && parts[0] != NULL && !parts[0][0]);
-    obj = object_get_root();
+    obj = root;
 
     for (i = 1; parts[i] != NULL; i++, obj = child) {
         child = object_resolve_path_component(obj, parts[i]);
-- 
1.7.7

Re: [Qemu-devel] [PATCH] qom: Refine container_get() to allow using a custom root

[Paolo Bonzini]

Il 05/04/2012 13:21, Andreas Färber ha scritto:
> Specify the root to search from as argument. This avoids hardcoding
> "/machine" in some places and makes it more flexible.
> 
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Anthony Liguori <anthony@codemonkey.ws>

Looks good, thanks.

Paolo

> ---
>  hw/qdev-monitor.c     |    4 ++--
>  hw/qdev.c             |    7 ++++---
>  include/qemu/object.h |    3 ++-
>  qom/container.c       |    4 ++--
>  4 files changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c
> index 4783366..67f296b 100644
> --- a/hw/qdev-monitor.c
> +++ b/hw/qdev-monitor.c
> @@ -180,7 +180,7 @@ static Object *qdev_get_peripheral(void)
>      static Object *dev;
>  
>      if (dev == NULL) {
> -        dev = container_get("/machine/peripheral");
> +        dev = container_get(qdev_get_machine(), "/peripheral");
>      }
>  
>      return dev;
> @@ -191,7 +191,7 @@ static Object *qdev_get_peripheral_anon(void)
>      static Object *dev;
>  
>      if (dev == NULL) {
> -        dev = container_get("/machine/peripheral-anon");
> +        dev = container_get(qdev_get_machine(), "/peripheral-anon");
>      }
>  
>      return dev;
> diff --git a/hw/qdev.c b/hw/qdev.c
> index 0d3c0fc..efa4c5d 100644
> --- a/hw/qdev.c
> +++ b/hw/qdev.c
> @@ -157,8 +157,9 @@ int qdev_init(DeviceState *dev)
>          static int unattached_count = 0;
>          gchar *name = g_strdup_printf("device[%d]", unattached_count++);
>  
> -        object_property_add_child(container_get("/machine/unattached"), name,
> -                                  OBJECT(dev), NULL);
> +        object_property_add_child(container_get(qdev_get_machine(),
> +                                                "/unattached"),
> +                                  name, OBJECT(dev), NULL);
>          g_free(name);
>      }
>  
> @@ -673,7 +674,7 @@ Object *qdev_get_machine(void)
>      static Object *dev;
>  
>      if (dev == NULL) {
> -        dev = container_get("/machine");
> +        dev = container_get(object_get_root(), "/machine");
>      }
>  
>      return dev;
> diff --git a/include/qemu/object.h b/include/qemu/object.h
> index a675937..ca1649c 100644
> --- a/include/qemu/object.h
> +++ b/include/qemu/object.h
> @@ -905,6 +905,7 @@ void object_property_add_str(Object *obj, const char *name,
>  
>  /**
>   * container_get:
> + * @root: root of the #path, e.g., object_get_root()
>   * @path: path to the container
>   *
>   * Return a container object whose path is @path.  Create more containers
> @@ -912,7 +913,7 @@ void object_property_add_str(Object *obj, const char *name,
>   *
>   * Returns: the container object.
>   */
> -Object *container_get(const char *path);
> +Object *container_get(Object *root, const char *path);
>  
>  
>  #endif
> diff --git a/qom/container.c b/qom/container.c
> index 67e9e8a..c9940ab 100644
> --- a/qom/container.c
> +++ b/qom/container.c
> @@ -25,7 +25,7 @@ static void container_register_types(void)
>      type_register_static(&container_info);
>  }
>  
> -Object *container_get(const char *path)
> +Object *container_get(Object *root, const char *path)
>  {
>      Object *obj, *child;
>      gchar **parts;
> @@ -33,7 +33,7 @@ Object *container_get(const char *path)
>  
>      parts = g_strsplit(path, "/", 0);
>      assert(parts != NULL && parts[0] != NULL && !parts[0][0]);
> -    obj = object_get_root();
> +    obj = root;
>  
>      for (i = 1; parts[i] != NULL; i++, obj = child) {
>          child = object_resolve_path_component(obj, parts[i]);

Re: [Qemu-devel] [PATCH 0/4] qdev: give all devices a canonical QOM path

[Anthony Liguori]

On 03/27/2012 11:38 AM, Paolo Bonzini wrote:
> A strong limitation of QOM right now is that unconverted ports
> (e.g. all...) do not give a canonical path to devices that are
> part of the board.  This in turn makes it impossible to replace
> PROP_PTR with a QOM link for example.
>
> This series fixes this by putting all such devices under a
> /unattached container.

Excellent!  Thanks for doing this.

Regards,

Anthony Liguori

>
> The problem is when to do this.  qdev_init is a good place, but it
> requires the composition tree to be in order by the time we call it.
> This is not the case so far, but it can be changed easily (patch 2) at
> the cost of removing most of the few composition examples that are in the
> tree.  This change is also necessary so that a single set of the topmost
> realize property can propagate all the way down the composition tree,
> as agreed on the mailing list.
>
> At the end of the series, all devices are moved to /machine
> in order to avoid cluttering too much the QOM root.
>
> Paolo Bonzini (4):
>    qom: add container_get
>    qdev: add children before qdev_init
>    qdev: give all devices a canonical path
>    qdev: put all devices under /machine
>
>   hw/pc_piix.c          |   18 +-----------------
>   hw/piix_pci.c         |    3 +--
>   hw/ppc_prep.c         |    2 +-
>   hw/qdev-monitor.c     |   16 ++++++----------
>   hw/qdev.c             |   16 +++++++++++++---
>   include/qemu/object.h |   24 ++++++++++++++++++++++++
>   qom/container.c       |   23 +++++++++++++++++++++++
>   qom/object.c          |   33 +++++++++++++++++----------------
>   8 files changed, 86 insertions(+), 49 deletions(-)
>

Re: [Qemu-devel] [PATCH 0/4] qdev: give all devices a canonical QOM path

[Anthony Liguori]

On 03/27/2012 11:38 AM, Paolo Bonzini wrote:
> A strong limitation of QOM right now is that unconverted ports
> (e.g. all...) do not give a canonical path to devices that are
> part of the board.  This in turn makes it impossible to replace
> PROP_PTR with a QOM link for example.
>
> This series fixes this by putting all such devices under a
> /unattached container.
>
> The problem is when to do this.  qdev_init is a good place, but it
> requires the composition tree to be in order by the time we call it.
> This is not the case so far, but it can be changed easily (patch 2) at
> the cost of removing most of the few composition examples that are in the
> tree.  This change is also necessary so that a single set of the topmost
> realize property can propagate all the way down the composition tree,
> as agreed on the mailing list.
>
> At the end of the series, all devices are moved to /machine
> in order to avoid cluttering too much the QOM root.

Applied all (including v2 of 4/4).  Thanks.

Regards,

Anthony Liguori

>
> Paolo Bonzini (4):
>    qom: add container_get
>    qdev: add children before qdev_init
>    qdev: give all devices a canonical path
>    qdev: put all devices under /machine
>
>   hw/pc_piix.c          |   18 +-----------------
>   hw/piix_pci.c         |    3 +--
>   hw/ppc_prep.c         |    2 +-
>   hw/qdev-monitor.c     |   16 ++++++----------
>   hw/qdev.c             |   16 +++++++++++++---
>   include/qemu/object.h |   24 ++++++++++++++++++++++++
>   qom/container.c       |   23 +++++++++++++++++++++++
>   qom/object.c          |   33 +++++++++++++++++----------------
>   8 files changed, 86 insertions(+), 49 deletions(-)
>