[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 1/2] qom: Implement object_property_add_const
From: |
Eduardo Habkost |
Subject: |
Re: [Qemu-devel] [PATCH v2 1/2] qom: Implement object_property_add_const_link() |
Date: |
Tue, 5 May 2015 13:06:54 -0300 |
User-agent: |
Mutt/1.5.23 (2014-03-12) |
On Fri, May 01, 2015 at 08:15:24PM +0200, Paolo Bonzini wrote:
> On 01/05/2015 16:09, Eduardo Habkost wrote:
> > +void object_property_add_const_link(Object *obj, const char *name,
> > + const char *type, Object *child,
> > + ObjectPropertyLinkFlags flags,
> > + Error **errp)
> > +{
> > + Object **childp = g_new0(Object*, 1);
> > +
> > + *childp = child;
> > + object_property_add_link(obj, name, type, childp, NULL,
> > + flags | OBJ_PROP_LINK_FREE_CHILD_POINTER,
> > errp);
> > +}
> > +
>
> This works, but is the extra functionality needed, compared to
> an alias? Namely, when is flags going to be != 0?
Flags is going to be != 0 if the caller grabs a reference to the target
object (to ensure it won't disappear) and wants it to be automatically
dropped when the property is removed. It is not strictly necessary, but
I thought it could be useful.
But to be honest, I don't love the flags argument in
object_property_add_link(), either. I mean: why do we need flags in
object_property_add_link() and not in object_property_add_alias()?
>
> FWIW, here is my ./.. patch. I'm all for adding a helper like
> object_property_add_const_link on top if we go for it.
Looks good to me.
>
> Another possibility is to not introduce any of our patches and reuse
> the child<> getter and resolve functions in
> object_property_add_const_link.
Using the "." property would allow object_property_add_const_link() to
be a one-liner, so it sounds better to me.
>
> -------------------- 8< ---------------
> From 653ca80e93fcaa94c7fa172edbaef75457d4952d Mon Sep 17 00:00:00 2001
> From: Paolo Bonzini <address@hidden>
> Date: Tue, 31 Mar 2015 13:56:24 +0200
> Subject: [PATCH] qom: add . and .. properties
>
> Signed-off-by: Paolo Bonzini <address@hidden>
> ---
> qom/object.c | 42 ++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 42 insertions(+)
>
> diff --git a/qom/object.c b/qom/object.c
> index b8dff43..3dd87b7 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -1783,9 +1783,51 @@ void object_property_set_description(Object *obj,
> const char *name,
> op->description = g_strdup(description);
> }
>
> +static void property_get_dot(Object *obj, struct Visitor *v, void *opaque,
> + const char *name, Error **errp)
> +{
> + gchar *path = object_get_canonical_path(obj);
> + visit_type_str(v, &path, name, errp);
> + g_free(path);
> +}
> +
> +static Object *property_resolve_dot(Object *obj, void *opaque,
> + const gchar *part)
> +{
> + return obj;
> +}
> +
> +static void property_get_dotdot(Object *obj, struct Visitor *v, void *opaque,
> + const char *name, Error **errp)
> +{
> + if (obj->parent) {
> + gchar *path = object_get_canonical_path(obj);
> + visit_type_str(v, &path, name, errp);
> + g_free(path);
> + } else {
> + gchar *path = (gchar *)"";
> + visit_type_str(v, &path, name, errp);
> + }
> +}
> +
> +static Object *property_resolve_dotdot(Object *obj, void *opaque,
> + const gchar *part)
> +{
> + return obj->parent;
> +}
> +
> static void object_instance_init(Object *obj)
> {
> + ObjectProperty *op;
> object_property_add_str(obj, "type", qdev_get_type, NULL, NULL);
> +
> + op = object_property_add(obj, ".", "link<Object>", property_get_dot,
> + NULL, NULL, NULL, NULL);
> + op->resolve = property_resolve_dot;
> +
> + op = object_property_add(obj, "..", "link<Object>", property_get_dotdot,
> + NULL, NULL, NULL, NULL);
> + op->resolve = property_resolve_dotdot;
> }
>
> static void register_types(void)
> --
> 2.3.5
>
--
Eduardo