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 <address@hidden>

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<address@hidden>
> ---
>   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;
>       }