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

[Paolo Bonzini]

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)
>  {
>