Re: [Qemu-devel] [PATCH 04/16] qom: add QObject-based property get/set wrappers

[Anthony Liguori <anthony@codemonkey.ws>]

On 02/02/2012 10:45 AM, Paolo Bonzini wrote:
> Move the creation of QmpInputVisitor and QmpOutputVisitor from
> qmp.c to qom/object.c, since it's the only practical way to access
> object properties.
>
> Signed-off-by: Paolo Bonzini<pbonzini@redhat.com>
> ---
>   include/qemu/object.h |   24 ++++++++++++++++++++++++
>   qmp.c                 |   17 ++---------------
>   qom/object.c          |   29 +++++++++++++++++++++++++++++
>   3 files changed, 55 insertions(+), 15 deletions(-)

I don't want object.h to have a dependency on QObject.  We need to phase out 
QObject.

Couple things:

1) We shouldn't use generic interfaces to read/write properties from objects. 
We should use type-safe accessors provided by the types themselves.

2) If we want to get fancy, we can add property_set_int, etc. and then implement 
(1) via header files that just call these functions.

Regards,

Anthony Liguori

>
> diff --git a/include/qemu/object.h b/include/qemu/object.h
> index 947cf29..71090f2 100644
> --- a/include/qemu/object.h
> +++ b/include/qemu/object.h
> @@ -542,6 +542,18 @@ void object_property_get(Object *obj, struct Visitor *v, const char *name,
>                            struct Error **errp);
>
>   /**
> + * object_property_get_qobject:
> + * @obj: the object
> + * @name: the name of the property
> + * @errp: returns an error if this function fails
> + *
> + * Returns: the value of the property, converted to QObject, or NULL if
> + * an error occurs.
> + */
> +struct QObject *object_property_get_qobject(Object *obj, const char *name,
> +                                            struct Error **errp);
> +
> +/**
>    * object_property_set:
>    * @obj: the object
>    * @v: the visitor that will be used to write the property value.  This should
> @@ -556,6 +568,18 @@ void object_property_set(Object *obj, struct Visitor *v, const char *name,
>                            struct Error **errp);
>
>   /**
> + * object_property_set_qobject:
> + * @obj: the object
> + * @ret: The value that will be written to the property.
> + * @name: the name of the property
> + * @errp: returns an error if this function fails
> + *
> + * Writes a property to a object.
> + */
> +void object_property_set_qobject(Object *obj, struct QObject *qobj,
> +                                 const char *name, struct Error **errp);
> +
> +/**
>    * @object_property_get_type:
>    * @obj: the object
>    * @name: the name of the property
> diff --git a/qmp.c b/qmp.c
> index 45052cc..c7a81cc 100644
> --- a/qmp.c
> +++ b/qmp.c
> @@ -21,8 +21,6 @@
>   #include "kvm.h"
>   #include "arch_init.h"
>   #include "hw/qdev.h"
> -#include "qapi/qmp-input-visitor.h"
> -#include "qapi/qmp-output-visitor.h"
>   #include "blockdev.h"
>
>   NameInfo *qmp_query_name(Error **errp)
> @@ -198,7 +196,6 @@ int qmp_qom_set(Monitor *mon, const QDict *qdict, QObject **ret)
>       const char *property = qdict_get_str(qdict, "property");
>       QObject *value = qdict_get(qdict, "value");
>       Error *local_err = NULL;
> -    QmpInputVisitor *mi;
>       Object *obj;
>
>       obj = object_resolve_path(path, NULL);
> @@ -207,10 +204,7 @@ int qmp_qom_set(Monitor *mon, const QDict *qdict, QObject **ret)
>           goto out;
>       }
>
> -    mi = qmp_input_visitor_new(value);
> -    object_property_set(obj, qmp_input_get_visitor(mi), property,&local_err);
> -
> -    qmp_input_visitor_cleanup(mi);
> +    object_property_set_qobject(obj, value, property,&local_err);
>
>   out:
>       if (local_err) {
> @@ -227,7 +221,6 @@ 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;
>       Object *obj;
>
>       obj = object_resolve_path(path, NULL);
> @@ -236,13 +229,7 @@ int qmp_qom_get(Monitor *mon, const QDict *qdict, QObject **ret)
>           goto out;
>       }
>
> -    mo = qmp_output_visitor_new();
> -    object_property_get(obj, qmp_output_get_visitor(mo), property,&local_err);
> -    if (!local_err) {
> -        *ret = qmp_output_get_qobject(mo);
> -    }
> -
> -    qmp_output_visitor_cleanup(mo);
> +    *ret = object_property_get_qobject(obj, property,&local_err);
>
>   out:
>       if (local_err) {
> diff --git a/qom/object.c b/qom/object.c
> index 299e146..13c8bec 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -13,6 +13,8 @@
>   #include "qemu/object.h"
>   #include "qemu-common.h"
>   #include "qapi/qapi-visit-core.h"
> +#include "qapi/qmp-input-visitor.h"
> +#include "qapi/qmp-output-visitor.h"
>
>   #define MAX_INTERFACES 32
>
> @@ -646,6 +648,33 @@ void object_property_set(Object *obj, Visitor *v, const char *name,
>       }
>   }
>
> +void object_property_set_qobject(Object *obj, QObject *value,
> +                                 const char *name, Error **errp)
> +{
> +    QmpInputVisitor *mi;
> +    mi = qmp_input_visitor_new(value);
> +    object_property_set(obj, qmp_input_get_visitor(mi), name, errp);
> +
> +    qmp_input_visitor_cleanup(mi);
> +}
> +
> +QObject *object_property_get_qobject(Object *obj, const char *name,
> +                                     Error **errp)
> +{
> +    QObject *ret = NULL;
> +    Error *local_err = NULL;
> +    QmpOutputVisitor *mo;
> +
> +    mo = qmp_output_visitor_new();
> +    object_property_get(obj, qmp_output_get_visitor(mo), name,&local_err);
> +    if (!local_err) {
> +        ret = qmp_output_get_qobject(mo);
> +    }
> +    error_propagate(errp, local_err);
> +    qmp_output_visitor_cleanup(mo);
> +    return ret;
> +}
> +
>   const char *object_property_get_type(Object *obj, const char *name, Error **errp)
>   {
>       ObjectProperty *prop = object_property_find(obj, name);