| Message ID | 1402505369-12526-2-git-send-email-pbonzini@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Thu, Jun 12, 2014 at 2:49 AM, Paolo Bonzini <pbonzini@redhat.com> wrote: > It may be desirable to have custom link<> properties that do more > than just store an object. Even the addition of a "check" > function is not enough if setting the link has side effects > or if a non-standard reference counting is preferrable. > > Avoid the assumption that the opaque field of a link<> is a > LinkProperty struct, by adding a generic "resolve" callback > to ObjectProperty. This fixes aliases of link properties. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> I commented them before, but there are two 80 char lines. Otherwise: Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com> > --- > include/qom/object.h | 49 ++++++++++++++++++++++++++++++++++++++++++++++ > qom/object.c | 55 ++++++++++++++++++++++++++++++++++------------------ > 2 files changed, 85 insertions(+), 19 deletions(-) > > diff --git a/include/qom/object.h b/include/qom/object.h > index a641dcd..f8ab845 100644 > --- a/include/qom/object.h > +++ b/include/qom/object.h > @@ -304,6 +304,23 @@ typedef void (ObjectPropertyAccessor)(Object *obj, > Error **errp); > > /** > + * ObjectPropertyResolve: > + * @obj: the object that owns the property > + * @opaque: the opaque registered with the property > + * @part: the name of the property > + * > + * If @path is the path that led to @obj, the function should > + * return the Object corresponding to "@path/@part". If #NULL > + * is returned, "@path/@part" is not a valid object path. > + * > + * The returned object can also be used as a starting point > + * to resolve a relative path starting with "@part". > + */ > +typedef Object *(ObjectPropertyResolve)(Object *obj, > + void *opaque, > + const char *part); > + > +/** > * ObjectPropertyRelease: > * @obj: the object that owns the property > * @name: the name of the property > @@ -321,6 +338,7 @@ typedef struct ObjectProperty > gchar *type; > ObjectPropertyAccessor *get; > ObjectPropertyAccessor *set; > + ObjectPropertyResolve *resolve; > ObjectPropertyRelease *release; > void *opaque; > > @@ -769,6 +787,37 @@ void object_ref(Object *obj); > void object_unref(Object *obj); > > /** > + * object_property_add_full: > + * @obj: the object 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. > + * @resolve: called when the property name is used as part of an object > + * path. This is meant for cases when you want to have custom link > + * properties. If it is NULL, the property name cannot be used as part > + * of a valid object path. > + * @release: called when the property is removed from the object. This is > + * meant to allow a property to free its opaque upon object > + * 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 object_property_add_full(Object *obj, const char *name, const char *type, > + ObjectPropertyAccessor *get, > + ObjectPropertyAccessor *set, > + ObjectPropertyResolve *resolve, > + ObjectPropertyRelease *release, > + void *opaque, Error **errp); > + > +/** > * object_property_add: > * @obj: the object to add a property to > * @name: the name of the property. This can contain any character except for > diff --git a/qom/object.c b/qom/object.c > index e42b254..fcdd0da 100644 > --- a/qom/object.c > +++ b/qom/object.c > @@ -355,11 +355,6 @@ static inline bool object_property_is_child(ObjectProperty *prop) > return strstart(prop->type, "child<", NULL); > } > > -static inline bool object_property_is_link(ObjectProperty *prop) > -{ > - return strstart(prop->type, "link<", NULL); > -} > - > static void object_property_del_all(Object *obj) > { > while (!QTAILQ_EMPTY(&obj->properties)) { > @@ -727,9 +722,10 @@ void object_unref(Object *obj) > } > } > > -void object_property_add(Object *obj, const char *name, const char *type, > +void object_property_add_full(Object *obj, const char *name, const char *type, > ObjectPropertyAccessor *get, > ObjectPropertyAccessor *set, > + ObjectPropertyResolve *resolve, > ObjectPropertyRelease *release, > void *opaque, Error **errp) > { > @@ -751,12 +747,23 @@ void object_property_add(Object *obj, const char *name, const char *type, > > prop->get = get; > prop->set = set; > + prop->resolve = resolve; > prop->release = release; > prop->opaque = opaque; > > QTAILQ_INSERT_TAIL(&obj->properties, prop, node); > } > > +void object_property_add(Object *obj, const char *name, const char *type, > + ObjectPropertyAccessor *get, > + ObjectPropertyAccessor *set, > + ObjectPropertyRelease *release, > + void *opaque, Error **errp) > +{ > + object_property_add_full(obj, name, type, get, set, NULL, release, > + opaque, errp); > +} > + > ObjectProperty *object_property_find(Object *obj, const char *name, > Error **errp) > { > @@ -993,6 +1000,11 @@ static void object_get_child_property(Object *obj, Visitor *v, void *opaque, > g_free(path); > } > > +static Object *object_resolve_child_property(Object *parent, void *opaque, const gchar *part) > +{ > + return opaque; > +} > + > static void object_finalize_child_property(Object *obj, const char *name, > void *opaque) > { > @@ -1009,8 +1021,9 @@ void object_property_add_child(Object *obj, const char *name, > > type = g_strdup_printf("child<%s>", object_get_typename(OBJECT(child))); > > - object_property_add(obj, name, type, object_get_child_property, NULL, > - object_finalize_child_property, child, &local_err); > + object_property_add_full(obj, name, type, object_get_child_property, NULL, > + object_resolve_child_property, > + object_finalize_child_property, child, &local_err); > if (local_err) { > error_propagate(errp, local_err); > goto out; > @@ -1128,6 +1141,12 @@ static void object_set_link_property(Object *obj, Visitor *v, void *opaque, > } > } > > +static Object *object_resolve_link_property(Object *parent, void *opaque, const gchar *part) > +{ > + LinkProperty *lprop = opaque; > + return *lprop->child; > +} > + > static void object_release_link_property(Object *obj, const char *name, > void *opaque) > { > @@ -1156,12 +1175,13 @@ void object_property_add_link(Object *obj, const char *name, > > full_type = g_strdup_printf("link<%s>", type); > > - object_property_add(obj, name, full_type, > - object_get_link_property, > - check ? object_set_link_property : NULL, > - object_release_link_property, > - prop, > - &local_err); > + object_property_add_full(obj, name, full_type, > + object_get_link_property, > + check ? object_set_link_property : NULL, > + object_resolve_link_property, > + object_release_link_property, > + prop, > + &local_err); > if (local_err) { > error_propagate(errp, local_err); > g_free(prop); > @@ -1225,11 +1245,8 @@ Object *object_resolve_path_component(Object *parent, const gchar *part) > return NULL; > } > > - if (object_property_is_link(prop)) { > - LinkProperty *lprop = prop->opaque; > - return *lprop->child; > - } else if (object_property_is_child(prop)) { > - return prop->opaque; > + if (prop->resolve) { > + return prop->resolve(parent, prop->opaque, part); > } else { > return NULL; > } > -- > 1.8.3.1 > > >
Am 11.06.2014 18:49, schrieb Paolo Bonzini: > It may be desirable to have custom link<> properties that do more > than just store an object. Even the addition of a "check" > function is not enough if setting the link has side effects > or if a non-standard reference counting is preferrable. > > Avoid the assumption that the opaque field of a link<> is a > LinkProperty struct, by adding a generic "resolve" callback > to ObjectProperty. This fixes aliases of link properties. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > include/qom/object.h | 49 ++++++++++++++++++++++++++++++++++++++++++++++ > qom/object.c | 55 ++++++++++++++++++++++++++++++++++------------------ > 2 files changed, 85 insertions(+), 19 deletions(-) After reading through multiple times, this does seem to make sense... ;) > diff --git a/include/qom/object.h b/include/qom/object.h > index a641dcd..f8ab845 100644 > --- a/include/qom/object.h > +++ b/include/qom/object.h > @@ -304,6 +304,23 @@ typedef void (ObjectPropertyAccessor)(Object *obj, > Error **errp); > > /** > + * ObjectPropertyResolve: > + * @obj: the object that owns the property > + * @opaque: the opaque registered with the property > + * @part: the name of the property > + * > + * If @path is the path that led to @obj, the function should > + * return the Object corresponding to "@path/@part". If #NULL > + * is returned, "@path/@part" is not a valid object path. > + * > + * The returned object can also be used as a starting point > + * to resolve a relative path starting with "@part". Minor: I would propose something like: * Resolves the #Object corresponding to property @part. * * The returned object can also be used as a starting point * to resolve a relative path starting with "@part". * * Returns: If @path is the path that led to @obj, the function * returns the #Object corresponding to "@path/@part". * If "@path/@part" is not a valid object path, it returns #NULL. I.e. inverting the two sentences to fit into a gtk-doc "Returns:" section, and either Object -> #Object or object. > + */ > +typedef Object *(ObjectPropertyResolve)(Object *obj, > + void *opaque, > + const char *part); > + > +/** > * ObjectPropertyRelease: > * @obj: the object that owns the property > * @name: the name of the property > @@ -321,6 +338,7 @@ typedef struct ObjectProperty > gchar *type; > ObjectPropertyAccessor *get; > ObjectPropertyAccessor *set; > + ObjectPropertyResolve *resolve; > ObjectPropertyRelease *release; > void *opaque; > > @@ -769,6 +787,37 @@ void object_ref(Object *obj); > void object_unref(Object *obj); > > /** > + * object_property_add_full: > + * @obj: the object 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. > + * @resolve: called when the property name is used as part of an object > + * path. This is meant for cases when you want to have custom link > + * properties. If it is NULL, the property name cannot be used as part > + * of a valid object path. > + * @release: called when the property is removed from the object. This is > + * meant to allow a property to free its opaque upon object > + * 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 object_property_add_full(Object *obj, const char *name, const char *type, > + ObjectPropertyAccessor *get, > + ObjectPropertyAccessor *set, > + ObjectPropertyResolve *resolve, > + ObjectPropertyRelease *release, > + void *opaque, Error **errp); > + > +/** > * object_property_add: > * @obj: the object to add a property to > * @name: the name of the property. This can contain any character except for I'm a bit concerned about the duplication going on here, e.g. of the forbidden characters. I think we should either just add the new argument to object_property_add() and add NULL arguments at old call sites as done before, or we should (to avoid future _really_full, _really_really_full versions) return the resulting ObjectProperty * for modification by the caller (in this case: ->resolve = foo). > diff --git a/qom/object.c b/qom/object.c > index e42b254..fcdd0da 100644 > --- a/qom/object.c > +++ b/qom/object.c > @@ -355,11 +355,6 @@ static inline bool object_property_is_child(ObjectProperty *prop) > return strstart(prop->type, "child<", NULL); > } > > -static inline bool object_property_is_link(ObjectProperty *prop) > -{ > - return strstart(prop->type, "link<", NULL); > -} > - (Scratch my earlier comment re macros, this is the function I meant.) > static void object_property_del_all(Object *obj) > { > while (!QTAILQ_EMPTY(&obj->properties)) { > @@ -727,9 +722,10 @@ void object_unref(Object *obj) > } > } > > -void object_property_add(Object *obj, const char *name, const char *type, > +void object_property_add_full(Object *obj, const char *name, const char *type, > ObjectPropertyAccessor *get, > ObjectPropertyAccessor *set, > + ObjectPropertyResolve *resolve, > ObjectPropertyRelease *release, > void *opaque, Error **errp) > { > @@ -751,12 +747,23 @@ void object_property_add(Object *obj, const char *name, const char *type, > > prop->get = get; > prop->set = set; > + prop->resolve = resolve; > prop->release = release; > prop->opaque = opaque; > > QTAILQ_INSERT_TAIL(&obj->properties, prop, node); > } > > +void object_property_add(Object *obj, const char *name, const char *type, > + ObjectPropertyAccessor *get, > + ObjectPropertyAccessor *set, > + ObjectPropertyRelease *release, > + void *opaque, Error **errp) > +{ > + object_property_add_full(obj, name, type, get, set, NULL, release, > + opaque, errp); > +} > + > ObjectProperty *object_property_find(Object *obj, const char *name, > Error **errp) > { > @@ -993,6 +1000,11 @@ static void object_get_child_property(Object *obj, Visitor *v, void *opaque, > g_free(path); > } > > +static Object *object_resolve_child_property(Object *parent, void *opaque, const gchar *part) > +{ > + return opaque; > +} > + > static void object_finalize_child_property(Object *obj, const char *name, > void *opaque) > { > @@ -1009,8 +1021,9 @@ void object_property_add_child(Object *obj, const char *name, > > type = g_strdup_printf("child<%s>", object_get_typename(OBJECT(child))); > > - object_property_add(obj, name, type, object_get_child_property, NULL, > - object_finalize_child_property, child, &local_err); > + object_property_add_full(obj, name, type, object_get_child_property, NULL, > + object_resolve_child_property, > + object_finalize_child_property, child, &local_err); > if (local_err) { > error_propagate(errp, local_err); > goto out; > @@ -1128,6 +1141,12 @@ static void object_set_link_property(Object *obj, Visitor *v, void *opaque, > } > } > > +static Object *object_resolve_link_property(Object *parent, void *opaque, const gchar *part) > +{ > + LinkProperty *lprop = opaque; > + return *lprop->child; > +} > + > static void object_release_link_property(Object *obj, const char *name, > void *opaque) > { > @@ -1156,12 +1175,13 @@ void object_property_add_link(Object *obj, const char *name, > > full_type = g_strdup_printf("link<%s>", type); > > - object_property_add(obj, name, full_type, > - object_get_link_property, > - check ? object_set_link_property : NULL, > - object_release_link_property, > - prop, > - &local_err); > + object_property_add_full(obj, name, full_type, > + object_get_link_property, > + check ? object_set_link_property : NULL, > + object_resolve_link_property, > + object_release_link_property, > + prop, > + &local_err); > if (local_err) { > error_propagate(errp, local_err); > g_free(prop); > @@ -1225,11 +1245,8 @@ Object *object_resolve_path_component(Object *parent, const gchar *part) > return NULL; > } > > - if (object_property_is_link(prop)) { > - LinkProperty *lprop = prop->opaque; > - return *lprop->child; > - } else if (object_property_is_child(prop)) { > - return prop->opaque; > + if (prop->resolve) { > + return prop->resolve(parent, prop->opaque, part); > } else { > return NULL; > } The _add vs. _add_full issue aside, the implementation looks good. Regards, Andreas
Il 17/06/2014 16:18, Andreas Färber ha scritto: >> /** >> + * ObjectPropertyResolve: >> + * @obj: the object that owns the property >> + * @opaque: the opaque registered with the property >> + * @part: the name of the property >> + * >> + * If @path is the path that led to @obj, the function should >> + * return the Object corresponding to "@path/@part". If #NULL >> + * is returned, "@path/@part" is not a valid object path. >> + * >> + * The returned object can also be used as a starting point >> + * to resolve a relative path starting with "@part". > > Minor: I would propose something like: > > * Resolves the #Object corresponding to property @part. > * > * The returned object can also be used as a starting point > * to resolve a relative path starting with "@part". > * > * Returns: If @path is the path that led to @obj, the function > * returns the #Object corresponding to "@path/@part". > * If "@path/@part" is not a valid object path, it returns #NULL. > > I.e. inverting the two sentences to fit into a gtk-doc "Returns:" > section, and either Object -> #Object or object. Good suggestion. >> +void object_property_add_full(Object *obj, const char *name, const char *type, >> + ObjectPropertyAccessor *get, >> + ObjectPropertyAccessor *set, >> + ObjectPropertyResolve *resolve, >> + ObjectPropertyRelease *release, >> + void *opaque, Error **errp); > > I'm a bit concerned about the duplication going on here, e.g. of the > forbidden characters. I think we should either just add the new argument > to object_property_add() and add NULL arguments at old call sites as > done before, or we should (to avoid future _really_full, > _really_really_full versions) return the resulting ObjectProperty * for > modification by the caller (in this case: ->resolve = foo). The reason I went with "_full" is that the new argument is really needed only in a minority of cases. There are ~50 occurrences right now, and I expect only a handful of them to need a ->resolve callback (and so far all of them would be in qom/object.c). There are many examples in glib's GSource (g_timeout_add_full, g_main_context_invoke_full, etc.) or elsewhere in glib (g_format_size_full). Since we do not have an ABI to follow, we could add arguments to the _full routine while keeping the shorthand version as is. I can change the 50 occurrences if you want though. Paolo >> -void object_property_add(Object *obj, const char *name, const char *type, >> +void object_property_add_full(Object *obj, const char *name, const char *type, >> ObjectPropertyAccessor *get, >> ObjectPropertyAccessor *set, >> + ObjectPropertyResolve *resolve, >> ObjectPropertyRelease *release, >> void *opaque, Error **errp) >> { >> @@ -751,12 +747,23 @@ void object_property_add(Object *obj, const char *name, const char *type, >> >> prop->get = get; >> prop->set = set; >> + prop->resolve = resolve; >> prop->release = release; >> prop->opaque = opaque; >> >> QTAILQ_INSERT_TAIL(&obj->properties, prop, node); >> } >> >> +void object_property_add(Object *obj, const char *name, const char *type, >> + ObjectPropertyAccessor *get, >> + ObjectPropertyAccessor *set, >> + ObjectPropertyRelease *release, >> + void *opaque, Error **errp) >> +{ >> + object_property_add_full(obj, name, type, get, set, NULL, release, >> + opaque, errp); >> +} >> + >> ObjectProperty *object_property_find(Object *obj, const char *name, >> Error **errp) >> { >> @@ -993,6 +1000,11 @@ static void object_get_child_property(Object *obj, Visitor *v, void *opaque, >> g_free(path); >> } >> >> +static Object *object_resolve_child_property(Object *parent, void *opaque, const gchar *part) >> +{ >> + return opaque; >> +} >> + >> static void object_finalize_child_property(Object *obj, const char *name, >> void *opaque) >> { >> @@ -1009,8 +1021,9 @@ void object_property_add_child(Object *obj, const char *name, >> >> type = g_strdup_printf("child<%s>", object_get_typename(OBJECT(child))); >> >> - object_property_add(obj, name, type, object_get_child_property, NULL, >> - object_finalize_child_property, child, &local_err); >> + object_property_add_full(obj, name, type, object_get_child_property, NULL, >> + object_resolve_child_property, >> + object_finalize_child_property, child, &local_err); >> if (local_err) { >> error_propagate(errp, local_err); >> goto out; >> @@ -1128,6 +1141,12 @@ static void object_set_link_property(Object *obj, Visitor *v, void *opaque, >> } >> } >> >> +static Object *object_resolve_link_property(Object *parent, void *opaque, const gchar *part) >> +{ >> + LinkProperty *lprop = opaque; >> + return *lprop->child; >> +} >> + >> static void object_release_link_property(Object *obj, const char *name, >> void *opaque) >> { >> @@ -1156,12 +1175,13 @@ void object_property_add_link(Object *obj, const char *name, >> >> full_type = g_strdup_printf("link<%s>", type); >> >> - object_property_add(obj, name, full_type, >> - object_get_link_property, >> - check ? object_set_link_property : NULL, >> - object_release_link_property, >> - prop, >> - &local_err); >> + object_property_add_full(obj, name, full_type, >> + object_get_link_property, >> + check ? object_set_link_property : NULL, >> + object_resolve_link_property, >> + object_release_link_property, >> + prop, >> + &local_err); >> if (local_err) { >> error_propagate(errp, local_err); >> g_free(prop); >> @@ -1225,11 +1245,8 @@ Object *object_resolve_path_component(Object *parent, const gchar *part) >> return NULL; >> } >> >> - if (object_property_is_link(prop)) { >> - LinkProperty *lprop = prop->opaque; >> - return *lprop->child; >> - } else if (object_property_is_child(prop)) { >> - return prop->opaque; >> + if (prop->resolve) { >> + return prop->resolve(parent, prop->opaque, part); >> } else { >> return NULL; >> } > > The _add vs. _add_full issue aside, the implementation looks good. > > Regards, > Andreas >
Am 17.06.2014 16:18, schrieb Andreas Färber: > Am 11.06.2014 18:49, schrieb Paolo Bonzini: >> diff --git a/qom/object.c b/qom/object.c >> index e42b254..fcdd0da 100644 >> --- a/qom/object.c >> +++ b/qom/object.c [...] >> @@ -1128,6 +1141,12 @@ static void object_set_link_property(Object *obj, Visitor *v, void *opaque, >> } >> } >> >> +static Object *object_resolve_link_property(Object *parent, void *opaque, const gchar *part) >> +{ >> + LinkProperty *lprop = opaque; Forgot: White line preferred even for single-line statements for consistency. >> + return *lprop->child; >> +} >> + >> static void object_release_link_property(Object *obj, const char *name, >> void *opaque) >> {
Am 17.06.2014 17:07, schrieb Paolo Bonzini: > Il 17/06/2014 16:18, Andreas Färber ha scritto: >>> +void object_property_add_full(Object *obj, const char *name, const >>> char *type, >>> + ObjectPropertyAccessor *get, >>> + ObjectPropertyAccessor *set, >>> + ObjectPropertyResolve *resolve, >>> + ObjectPropertyRelease *release, >>> + void *opaque, Error **errp); >> >> I'm a bit concerned about the duplication going on here, e.g. of the >> forbidden characters. I think we should either just add the new argument >> to object_property_add() and add NULL arguments at old call sites as >> done before, or we should (to avoid future _really_full, >> _really_really_full versions) return the resulting ObjectProperty * for >> modification by the caller (in this case: ->resolve = foo). > > The reason I went with "_full" is that the new argument is really needed > only in a minority of cases. There are ~50 occurrences right now, and I > expect only a handful of them to need a ->resolve callback (and so far > all of them would be in qom/object.c). > > There are many examples in glib's GSource (g_timeout_add_full, > g_main_context_invoke_full, etc.) or elsewhere in glib > (g_format_size_full). > > Since we do not have an ABI to follow, we could add arguments to the > _full routine while keeping the shorthand version as is. > > I can change the 50 occurrences if you want though. So what about my alternative suggestion of changing _add's void -> ObjectProperty*? That would limit future updating to the struct itself while still avoiding to touch the 50. Andreas
Il 17/06/2014 17:19, Andreas Färber ha scritto: > So what about my alternative suggestion of changing _add's void -> > ObjectProperty*? That would limit future updating to the struct itself > while still avoiding to touch the 50. That's fine by me too. Paolo
diff --git a/include/qom/object.h b/include/qom/object.h index a641dcd..f8ab845 100644 --- a/include/qom/object.h +++ b/include/qom/object.h @@ -304,6 +304,23 @@ typedef void (ObjectPropertyAccessor)(Object *obj, Error **errp); /** + * ObjectPropertyResolve: + * @obj: the object that owns the property + * @opaque: the opaque registered with the property + * @part: the name of the property + * + * If @path is the path that led to @obj, the function should + * return the Object corresponding to "@path/@part". If #NULL + * is returned, "@path/@part" is not a valid object path. + * + * The returned object can also be used as a starting point + * to resolve a relative path starting with "@part". + */ +typedef Object *(ObjectPropertyResolve)(Object *obj, + void *opaque, + const char *part); + +/** * ObjectPropertyRelease: * @obj: the object that owns the property * @name: the name of the property @@ -321,6 +338,7 @@ typedef struct ObjectProperty gchar *type; ObjectPropertyAccessor *get; ObjectPropertyAccessor *set; + ObjectPropertyResolve *resolve; ObjectPropertyRelease *release; void *opaque; @@ -769,6 +787,37 @@ void object_ref(Object *obj); void object_unref(Object *obj); /** + * object_property_add_full: + * @obj: the object 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. + * @resolve: called when the property name is used as part of an object + * path. This is meant for cases when you want to have custom link + * properties. If it is NULL, the property name cannot be used as part + * of a valid object path. + * @release: called when the property is removed from the object. This is + * meant to allow a property to free its opaque upon object + * 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 object_property_add_full(Object *obj, const char *name, const char *type, + ObjectPropertyAccessor *get, + ObjectPropertyAccessor *set, + ObjectPropertyResolve *resolve, + ObjectPropertyRelease *release, + void *opaque, Error **errp); + +/** * object_property_add: * @obj: the object to add a property to * @name: the name of the property. This can contain any character except for diff --git a/qom/object.c b/qom/object.c index e42b254..fcdd0da 100644 --- a/qom/object.c +++ b/qom/object.c @@ -355,11 +355,6 @@ static inline bool object_property_is_child(ObjectProperty *prop) return strstart(prop->type, "child<", NULL); } -static inline bool object_property_is_link(ObjectProperty *prop) -{ - return strstart(prop->type, "link<", NULL); -} - static void object_property_del_all(Object *obj) { while (!QTAILQ_EMPTY(&obj->properties)) { @@ -727,9 +722,10 @@ void object_unref(Object *obj) } } -void object_property_add(Object *obj, const char *name, const char *type, +void object_property_add_full(Object *obj, const char *name, const char *type, ObjectPropertyAccessor *get, ObjectPropertyAccessor *set, + ObjectPropertyResolve *resolve, ObjectPropertyRelease *release, void *opaque, Error **errp) { @@ -751,12 +747,23 @@ void object_property_add(Object *obj, const char *name, const char *type, prop->get = get; prop->set = set; + prop->resolve = resolve; prop->release = release; prop->opaque = opaque; QTAILQ_INSERT_TAIL(&obj->properties, prop, node); } +void object_property_add(Object *obj, const char *name, const char *type, + ObjectPropertyAccessor *get, + ObjectPropertyAccessor *set, + ObjectPropertyRelease *release, + void *opaque, Error **errp) +{ + object_property_add_full(obj, name, type, get, set, NULL, release, + opaque, errp); +} + ObjectProperty *object_property_find(Object *obj, const char *name, Error **errp) { @@ -993,6 +1000,11 @@ static void object_get_child_property(Object *obj, Visitor *v, void *opaque, g_free(path); } +static Object *object_resolve_child_property(Object *parent, void *opaque, const gchar *part) +{ + return opaque; +} + static void object_finalize_child_property(Object *obj, const char *name, void *opaque) { @@ -1009,8 +1021,9 @@ void object_property_add_child(Object *obj, const char *name, type = g_strdup_printf("child<%s>", object_get_typename(OBJECT(child))); - object_property_add(obj, name, type, object_get_child_property, NULL, - object_finalize_child_property, child, &local_err); + object_property_add_full(obj, name, type, object_get_child_property, NULL, + object_resolve_child_property, + object_finalize_child_property, child, &local_err); if (local_err) { error_propagate(errp, local_err); goto out; @@ -1128,6 +1141,12 @@ static void object_set_link_property(Object *obj, Visitor *v, void *opaque, } } +static Object *object_resolve_link_property(Object *parent, void *opaque, const gchar *part) +{ + LinkProperty *lprop = opaque; + return *lprop->child; +} + static void object_release_link_property(Object *obj, const char *name, void *opaque) { @@ -1156,12 +1175,13 @@ void object_property_add_link(Object *obj, const char *name, full_type = g_strdup_printf("link<%s>", type); - object_property_add(obj, name, full_type, - object_get_link_property, - check ? object_set_link_property : NULL, - object_release_link_property, - prop, - &local_err); + object_property_add_full(obj, name, full_type, + object_get_link_property, + check ? object_set_link_property : NULL, + object_resolve_link_property, + object_release_link_property, + prop, + &local_err); if (local_err) { error_propagate(errp, local_err); g_free(prop); @@ -1225,11 +1245,8 @@ Object *object_resolve_path_component(Object *parent, const gchar *part) return NULL; } - if (object_property_is_link(prop)) { - LinkProperty *lprop = prop->opaque; - return *lprop->child; - } else if (object_property_is_child(prop)) { - return prop->opaque; + if (prop->resolve) { + return prop->resolve(parent, prop->opaque, part); } else { return NULL; }
It may be desirable to have custom link<> properties that do more than just store an object. Even the addition of a "check" function is not enough if setting the link has side effects or if a non-standard reference counting is preferrable. Avoid the assumption that the opaque field of a link<> is a LinkProperty struct, by adding a generic "resolve" callback to ObjectProperty. This fixes aliases of link properties. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- include/qom/object.h | 49 ++++++++++++++++++++++++++++++++++++++++++++++ qom/object.c | 55 ++++++++++++++++++++++++++++++++++------------------ 2 files changed, 85 insertions(+), 19 deletions(-)