Re: [PATCH 2/3] qdev: check callback takes Object **target as third argument

Paolo Bonzini
2015-04-28T11:39:23Z

On 28/04/2015 11:18, Eric Auger wrote:
> Check callback now takes as third argument an Object **. In
> object_set_link_property, we pass the property child as argument.
> We also assign the *child before the check call so that enhanced
> check can be performed in the callback. In case the check fails,
> the old value is restored and ref count is left unchanged.
>
> This typically makes possible to do checks both on the *child
> content (for instance a qemu_irq) and also perform some actions/
> checks on its container, which was not possible before.
>
> This is typically useful for starting irqfd setup in vfio platform
> use case.

s/typically/for example/

I can't say that "starting irqfd setup in vfio platform" is typical. :)

> diff --git a/include/qom/object.h b/include/qom/object.h
> index 4687fa1..0a7daff 100644
> --- a/include/qom/object.h
> +++ b/include/qom/object.h
> @@ -34,7 +34,7 @@ typedef struct InterfaceClass InterfaceClass;
> typedef struct InterfaceInfo InterfaceInfo;
>
> typedef void (*object_property_set_link_t)(Object *, const char *,
> - Object *, Error **);
> + Object **, Error **);

Let's make the new argument "Object * const*", and rename the typedef to
LinkPropertySetter.

Ok with that change.

Paolo

>
> #define TYPE_OBJECT "object"
>
> @@ -1136,7 +1136,7 @@ typedef enum {
> * an error.
> */
> void object_property_allow_set_link(Object *, const char *,
> - Object *, Error **);
> + Object **, Error **);
>
> /**
> * object_property_add_link:
> @@ -1168,8 +1168,7 @@ void object_property_allow_set_link(Object *, const char *,
> */
> void object_property_add_link(Object *obj, const char *name,
> const char *type, Object **child,
> - void (*check)(Object *obj, const char *name,
> - Object *val, Error **errp),
> + object_property_set_link_t check,
> ObjectPropertyLinkFlags flags,
> Error **errp);
>
> diff --git a/qom/object.c b/qom/object.c
> index b8dff43..cc9ed87 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -1112,14 +1112,14 @@ out:
> }
>
> void object_property_allow_set_link(Object *obj, const char *name,
> - Object *val, Error **errp)
> + Object **target, Error **errp)
> {
> /* Allow the link to be set, always */
> }
>
> typedef struct {
> Object **child;
> - void (*check)(Object *, const char *, Object *, Error **);
> + void (*check)(Object *, const char *, Object **, Error **);
> ObjectPropertyLinkFlags flags;
> } LinkProperty;
>
> @@ -1201,14 +1201,17 @@ static void object_set_link_property(Object *obj, Visitor *v, void *opaque,
> return;
> }
>
> - prop->check(obj, name, new_target, &local_err);
> + object_ref(new_target);
> + *child = new_target;
> +
> + prop->check(obj, name, child, &local_err);
> if (local_err) {
> error_propagate(errp, local_err);
> + *child = old_target;
> + object_ref(new_target);
> return;
> }
>
> - object_ref(new_target);
> - *child = new_target;
> object_unref(old_target);
> }
>
> @@ -1233,7 +1236,7 @@ static void object_release_link_property(Object *obj, const char *name,
> void object_property_add_link(Object *obj, const char *name,
> const char *type, Object **child,
> void (*check)(Object *, const char *,
> - Object *, Error **),
> + Object **, Error **),
> ObjectPropertyLinkFlags flags,
> Error **errp)
> {
>