Re: [Qemu-devel] [PATCH 1/2] object: Add can_be_deleted callback to TypeInfo and TypeImpl

[Peter Crosthwaite]

On Mon, Mar 23, 2015 at 3:40 PM, Lin Ma <address@hidden> wrote:
> Add can_be_deleted callback, If it is not null and returns false,
> The qmp_object_del won't delete the given object.
>
> Signed-off-by: Lin Ma <address@hidden>
> ---
>  include/qom/object.h | 12 ++++++++++++
>  qmp.c                |  7 +++++++
>  qom/object.c         | 12 ++++++++++++
>  3 files changed, 31 insertions(+)
>
> diff --git a/include/qom/object.h b/include/qom/object.h
> index d2d7748..6e78cb0 100644
> --- a/include/qom/object.h
> +++ b/include/qom/object.h
> @@ -428,6 +428,8 @@ struct Object
>   *   function.
>   * @abstract: If this field is true, then the class is considered abstract 
> and
>   *   cannot be directly instantiated.
> + * @can_be_deleted: If this function returns true, then the object can be
> +     deleted safely.
>   * @class_size: The size of the class object (derivative of #ObjectClass)
>   *   for this object.  If @class_size is 0, then the size of the class will 
> be
>   *   assumed to be the size of the parent class.  This allows a type to avoid
> @@ -463,6 +465,8 @@ struct TypeInfo
>      bool abstract;
>      size_t class_size;
>
> +    bool (*can_be_deleted)(Object *obj);
> +

I don't think TypeInfo is the right place for this. You can however
define function hooks for Object in ObjectClass. See the unparent
field of ObjectClass for a precedent.

>      void (*class_init)(ObjectClass *klass, void *data);
>      void (*class_base_init)(ObjectClass *klass, void *data);
>      void (*class_finalize)(ObjectClass *klass, void *data);
> @@ -671,6 +675,14 @@ ObjectClass *object_get_class(Object *obj);
>  const char *object_get_typename(Object *obj);
>
>  /**
> + * object_can_be_deleted:
> + * @obj: The object to obtain the deletion for.
> + *
> + * Returns: %true if @obj can be deleted safely, %false otherwise.
> + */
> +bool object_can_be_deleted(Object *obj);
> +
> +/**
>   * type_register_static:
>   * @info: The #TypeInfo of the new type.
>   *
> diff --git a/qmp.c b/qmp.c
> index c479e77..dbbcb37 100644
> --- a/qmp.c
> +++ b/qmp.c
> @@ -711,6 +711,13 @@ void qmp_object_del(const char *id, Error **errp)
>          error_setg(errp, "object id not found");
>          return;
>      }
> +
> +    if (!object_can_be_deleted(obj)) {
> +        char *path = object_get_canonical_path_component(obj);
> +        error_setg(errp, "%s is in used, can not be deleted", path);
> +        g_free(path);
> +        return;
> +    }
>      object_unparent(obj);

But is a better way to do this to add error handling to
object_unparent API and override object_unparent for your device in
question to throw the error? Then your change doesn't have to be
limited to QMP.

Regards,
Peter

>  }
>
> diff --git a/qom/object.c b/qom/object.c
> index d167038..dcec108 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -57,6 +57,8 @@ struct TypeImpl
>
>      bool abstract;
>
> +    bool (*can_be_deleted)(Object *obj);
> +
>      const char *parent;
>      TypeImpl *parent_type;
>
> @@ -121,6 +123,8 @@ static TypeImpl *type_new(const TypeInfo *info)
>
>      ti->abstract = info->abstract;
>
> +    ti->can_be_deleted = info->can_be_deleted;
> +
>      for (i = 0; info->interfaces && info->interfaces[i].type; i++) {
>          ti->interfaces[i].typename = g_strdup(info->interfaces[i].type);
>      }
> @@ -584,6 +588,14 @@ const char *object_get_typename(Object *obj)
>      return obj->class->type->name;
>  }
>
> +bool object_can_be_deleted(Object *obj)
> +{
> +    if (obj->class->type->can_be_deleted)
> +        return obj->class->type->can_be_deleted(obj);
> +    else
> +        return true;
> +}
> +
>  ObjectClass *object_get_class(Object *obj)
>  {
>      return obj->class;
> --
> 2.1.4
>
>