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

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

[Lin Ma]

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 <lma@suse.com>
---
 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);
+
     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);
 }
 
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

[Qemu-devel] [PATCH 2/2] memory-backend: Add can_be_deleted impl for ram-backend and file-backend

[Lin Ma]

The can_be_deleted callback returns false if the backend is in used,
Otherwise returns true.

Signed-off-by: Lin Ma <lma@suse.com>
---
 backends/hostmem-file.c | 12 ++++++++++++
 backends/hostmem-ram.c  | 12 ++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
index 5179994..33ad8e7 100644
--- a/backends/hostmem-file.c
+++ b/backends/hostmem-file.c
@@ -118,10 +118,22 @@ file_backend_instance_init(Object *o)
                             set_mem_path, NULL);
 }
 
+static bool
+file_backend_can_be_deleted(Object *obj)
+{
+    MemoryRegion *mr;
+    mr = host_memory_backend_get_memory(MEMORY_BACKEND(obj), &error_abort);
+    if (memory_region_is_mapped(mr))
+        return false;
+    else
+        return true;
+}
+
 static const TypeInfo file_backend_info = {
     .name = TYPE_MEMORY_BACKEND_FILE,
     .parent = TYPE_MEMORY_BACKEND,
     .class_init = file_backend_class_init,
+    .can_be_deleted = file_backend_can_be_deleted,
     .instance_init = file_backend_instance_init,
     .instance_size = sizeof(HostMemoryBackendFile),
 };
diff --git a/backends/hostmem-ram.c b/backends/hostmem-ram.c
index a67a134..8724ad1 100644
--- a/backends/hostmem-ram.c
+++ b/backends/hostmem-ram.c
@@ -39,10 +39,22 @@ ram_backend_class_init(ObjectClass *oc, void *data)
     bc->alloc = ram_backend_memory_alloc;
 }
 
+static bool
+ram_backend_can_be_deleted(Object *obj)
+{
+    MemoryRegion *mr;
+    mr = host_memory_backend_get_memory(MEMORY_BACKEND(obj), &error_abort);
+    if (memory_region_is_mapped(mr))
+        return false;
+    else
+        return true;
+}
+
 static const TypeInfo ram_backend_info = {
     .name = TYPE_MEMORY_BACKEND_RAM,
     .parent = TYPE_MEMORY_BACKEND,
     .class_init = ram_backend_class_init,
+    .can_be_deleted = ram_backend_can_be_deleted,
 };
 
 static void register_types(void)
-- 
2.1.4

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 <lma@suse.com> 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 <lma@suse.com>
> ---
>  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
>
>

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

[Paolo Bonzini]

On 23/03/2015 11:36, Peter Crosthwaite wrote:
> 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.

In this case, the right place could be UserCreatable.  Alternatively...

> 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.

... this is also a good choice.

Paolo

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

[Andreas Färber]

Hi Lin,

Am 23.03.2015 um 11:10 schrieb Lin Ma:
> 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 <lma@suse.com>
> ---
>  include/qom/object.h | 12 ++++++++++++
>  qmp.c                |  7 +++++++
>  qom/object.c         | 12 ++++++++++++
>  3 files changed, 31 insertions(+)

Why am I not being CC'ed here as QOM maintainer? You can use
scripts/get_maintainer.pl to assist you, or read MAINTAINERS file.

Also, when sending more than one patch, always include a cover letter
for grouping.

It further seems that this is the second version already, so you
should've used --subject-prefix="PATCH v2" and included a change log in
the cover letter.

Cf. http://wiki.qemu-project.org/Contribute/SubmitAPatch

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu,
Graham Norton; HRB 21284 (AG Nürnberg)

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

[Andreas Färber]

Hi,

For consistency in git-log, please use "qom:" rather than "object:".

Am 23.03.2015 um 13:06 schrieb Paolo Bonzini:
> On 23/03/2015 11:36, Peter Crosthwaite wrote:
>> 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.

Agree.

> In this case, the right place could be UserCreatable.

Maybe, not so familiar with that interface myself. Does object_del allow
to delete non-UserCreatable objects? Then it wouldn't help much.

>  Alternatively...
> 
>> 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.
> 
> ... this is also a good choice.

Well, I have doubts about asking someone who's not ultimately familiar
with that code to refactor the API. For instance, we wouldn't want QEMU
on shutdown or in error cases refusing to unparent some object.

Doing it at QMP level (ObjectClass/UserCreatable) seems safer, given
that Chun Yan's trivial block option fix ended up respinning a QemuOpts
refactoring some twenty times before it got merged.

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu,
Graham Norton; HRB 21284 (AG Nürnberg)

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

[Igor Mammedov]

On Mon, 23 Mar 2015 14:13:07 +0100
Andreas Färber <afaerber@suse.de> wrote:

> Hi,
> 
> For consistency in git-log, please use "qom:" rather than "object:".
> 
> Am 23.03.2015 um 13:06 schrieb Paolo Bonzini:
> > On 23/03/2015 11:36, Peter Crosthwaite wrote:
> >> 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.
> 
> Agree.
> 
> > In this case, the right place could be UserCreatable.
> 
> Maybe, not so familiar with that interface myself. Does object_del allow
> to delete non-UserCreatable objects? Then it wouldn't help much.
object_del() works only with /objects children, and so far the only way
that child placed there is via object_add() which requires a new object
to have TYPE_USER_CREATABLE interface.
I'd go this way rather than overhaul object_unparent() in this case.

> 
> >  Alternatively...
> > 
> >> 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.
> > 
> > ... this is also a good choice.
> 
> Well, I have doubts about asking someone who's not ultimately familiar
> with that code to refactor the API. For instance, we wouldn't want QEMU
> on shutdown or in error cases refusing to unparent some object.
> 
> Doing it at QMP level (ObjectClass/UserCreatable) seems safer, given
> that Chun Yan's trivial block option fix ended up respinning a QemuOpts
> refactoring some twenty times before it got merged.
> 
> Regards,
> Andreas
> 

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

[Lin Ma]

在 2015年03月23日 20:52, Andreas Färber 写道:
> Hi Lin,
>
> Am 23.03.2015 um 11:10 schrieb Lin Ma:
>> 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 <lma@suse.com>
>> ---
>>   include/qom/object.h | 12 ++++++++++++
>>   qmp.c                |  7 +++++++
>>   qom/object.c         | 12 ++++++++++++
>>   3 files changed, 31 insertions(+)
> Why am I not being CC'ed here as QOM maintainer? You can use
> scripts/get_maintainer.pl to assist you, or read MAINTAINERS file.
Really sorry about that.
>
> Also, when sending more than one patch, always include a cover letter
> for grouping.
Thanks for reminding.
> It further seems that this is the second version already, so you
> should've used --subject-prefix="PATCH v2" and included a change log in
> the cover letter.
>
> Cf. http://wiki.qemu-project.org/Contribute/SubmitAPatch
>
> Regards,
> Andreas
>
Because the patch v1 is a single patch and not generic, So I dropped it.
That's why I didn't used --subject-prefix here. But I'll add all the 
info & format that you mentioned in patch v3.

Thanks,
Lin

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

[Lin Ma]

在 2015年03月23日 20:06, Paolo Bonzini 写道:
>
> On 23/03/2015 11:36, Peter Crosthwaite wrote:
>> 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.
> In this case, the right place could be UserCreatable.  Alternatively...
Well... Because I'm not familiar with qom very much, Let me dig more 
details about UserCreatable first :-)
>
>> 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.
> ... this is also a good choice.
>
> Paolo
>

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

[Lin Ma]

在 2015年03月23日 21:30, Igor Mammedov 写道:
> On Mon, 23 Mar 2015 14:13:07 +0100
> Andreas Färber <afaerber@suse.de> wrote:
>
>> Hi,
>>
>> For consistency in git-log, please use "qom:" rather than "object:".
>>
>> Am 23.03.2015 um 13:06 schrieb Paolo Bonzini:
>>> On 23/03/2015 11:36, Peter Crosthwaite wrote:
>>>> 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.
>> Agree.
>>
>>> In this case, the right place could be UserCreatable.
>> Maybe, not so familiar with that interface myself. Does object_del allow
>> to delete non-UserCreatable objects? Then it wouldn't help much.
> object_del() works only with /objects children, and so far the only way
> that child placed there is via object_add() which requires a new object
> to have TYPE_USER_CREATABLE interface.
> I'd go this way rather than overhaul object_unparent() in this case.
What about these changes?
1. Add a callback function named 'ObjectCanBeDeleted *can_be_deleted' to 
struct ObjectClass in object.h
2. Call the function in qmp_object_del of qmp.c, says:
     ObjectClass *obj_class = object_get_class(obj);
         if (obj_class->can_be_deleted)
             if (!obj_class->can_be_deleted(obj))
                 error out

3. Then implement can_be_deleted callback in backends, says:
     static bool host_memory_backend_can_be_deleted(Object *obj) {......}

     static void host_memory_backend_class_init(ObjectClass *oc, void 
*data) {
         ......
         oc->can_be_deleted = host_memory_backend_can_be_deleted;
     }
>
>>>   Alternatively...
>>>
>>>> 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.
>>> ... this is also a good choice.
>> Well, I have doubts about asking someone who's not ultimately familiar
>> with that code to refactor the API. For instance, we wouldn't want QEMU
>> on shutdown or in error cases refusing to unparent some object.
>>
>> Doing it at QMP level (ObjectClass/UserCreatable) seems safer, given
>> that Chun Yan's trivial block option fix ended up respinning a QemuOpts
>> refactoring some twenty times before it got merged.
>>
>> Regards,
>> Andreas
>>
>
>

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

[Igor Mammedov]

On Wed, 25 Mar 2015 23:47:46 +0800
Lin Ma <lma@suse.com> wrote:

> 
> 在 2015年03月23日 21:30, Igor Mammedov 写道:
> > On Mon, 23 Mar 2015 14:13:07 +0100
> > Andreas Färber <afaerber@suse.de> wrote:
> >
> >> Hi,
> >>
> >> For consistency in git-log, please use "qom:" rather than "object:".
> >>
> >> Am 23.03.2015 um 13:06 schrieb Paolo Bonzini:
> >>> On 23/03/2015 11:36, Peter Crosthwaite wrote:
> >>>> 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.
> >> Agree.
> >>
> >>> In this case, the right place could be UserCreatable.
> >> Maybe, not so familiar with that interface myself. Does object_del allow
> >> to delete non-UserCreatable objects? Then it wouldn't help much.
> > object_del() works only with /objects children, and so far the only way
> > that child placed there is via object_add() which requires a new object
> > to have TYPE_USER_CREATABLE interface.
> > I'd go this way rather than overhaul object_unparent() in this case.
> What about these changes?
> 1. Add a callback function named 'ObjectCanBeDeleted *can_be_deleted' to 
> struct ObjectClass in object.h
> 2. Call the function in qmp_object_del of qmp.c, says:
>      ObjectClass *obj_class = object_get_class(obj);
>          if (obj_class->can_be_deleted)
>              if (!obj_class->can_be_deleted(obj))
>                  error out
> 
> 3. Then implement can_be_deleted callback in backends, says:
>      static bool host_memory_backend_can_be_deleted(Object *obj) {......}
> 
>      static void host_memory_backend_class_init(ObjectClass *oc, void 
> *data) {
>          ......
>          oc->can_be_deleted = host_memory_backend_can_be_deleted;
>      }

all backends currently implement TYPE_USER_CREATABLE interface,
so I'd rather extend UserCreatableClass with:

     bool can_be_deleted(UserCreatable *uc)

callback, which backends could implement if needed.

> >
> >>>   Alternatively...
> >>>
> >>>> 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.
> >>> ... this is also a good choice.
> >> Well, I have doubts about asking someone who's not ultimately familiar
> >> with that code to refactor the API. For instance, we wouldn't want QEMU
> >> on shutdown or in error cases refusing to unparent some object.
> >>
> >> Doing it at QMP level (ObjectClass/UserCreatable) seems safer, given
> >> that Chun Yan's trivial block option fix ended up respinning a QemuOpts
> >> refactoring some twenty times before it got merged.
> >>
> >> Regards,
> >> Andreas
> >>
> >
> >
> 

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

[Andreas Färber]

Am 26.03.2015 um 11:05 schrieb Igor Mammedov:
> On Wed, 25 Mar 2015 23:47:46 +0800
> Lin Ma <lma@suse.com> wrote:
>> 在 2015年03月23日 21:30, Igor Mammedov 写道:
>>> On Mon, 23 Mar 2015 14:13:07 +0100
>>> Andreas Färber <afaerber@suse.de> wrote:
>>>> Am 23.03.2015 um 13:06 schrieb Paolo Bonzini:
>>>>> On 23/03/2015 11:36, Peter Crosthwaite wrote:
>>>>>> 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.
>>>> Agree.
>>>>
>>>>> In this case, the right place could be UserCreatable.
>>>> Maybe, not so familiar with that interface myself. Does object_del allow
>>>> to delete non-UserCreatable objects? Then it wouldn't help much.
>>> object_del() works only with /objects children, and so far the only way
>>> that child placed there is via object_add() which requires a new object
>>> to have TYPE_USER_CREATABLE interface.
>>> I'd go this way rather than overhaul object_unparent() in this case.
>> What about these changes?
>> 1. Add a callback function named 'ObjectCanBeDeleted *can_be_deleted' to 
>> struct ObjectClass in object.h
>> 2. Call the function in qmp_object_del of qmp.c, says:
>>      ObjectClass *obj_class = object_get_class(obj);
>>          if (obj_class->can_be_deleted)
>>              if (!obj_class->can_be_deleted(obj))
>>                  error out
>>
>> 3. Then implement can_be_deleted callback in backends, says:
>>      static bool host_memory_backend_can_be_deleted(Object *obj) {......}
>>
>>      static void host_memory_backend_class_init(ObjectClass *oc, void 
>> *data) {
>>          ......
>>          oc->can_be_deleted = host_memory_backend_can_be_deleted;
>>      }
> 
> all backends currently implement TYPE_USER_CREATABLE interface,
> so I'd rather extend UserCreatableClass with:
> 
>      bool can_be_deleted(UserCreatable *uc)
> 
> callback, which backends could implement if needed.

As for the implementation, could we simply look at Object::ref field?

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu,
Graham Norton; HRB 21284 (AG Nürnberg)

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

[Igor Mammedov]

On Thu, 26 Mar 2015 11:07:27 +0100
Andreas Färber <afaerber@suse.de> wrote:

> Am 26.03.2015 um 11:05 schrieb Igor Mammedov:
> > On Wed, 25 Mar 2015 23:47:46 +0800
> > Lin Ma <lma@suse.com> wrote:
> >> 在 2015年03月23日 21:30, Igor Mammedov 写道:
> >>> On Mon, 23 Mar 2015 14:13:07 +0100
> >>> Andreas Färber <afaerber@suse.de> wrote:
> >>>> Am 23.03.2015 um 13:06 schrieb Paolo Bonzini:
> >>>>> On 23/03/2015 11:36, Peter Crosthwaite wrote:
> >>>>>> 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.
> >>>> Agree.
> >>>>
> >>>>> In this case, the right place could be UserCreatable.
> >>>> Maybe, not so familiar with that interface myself. Does object_del allow
> >>>> to delete non-UserCreatable objects? Then it wouldn't help much.
> >>> object_del() works only with /objects children, and so far the only way
> >>> that child placed there is via object_add() which requires a new object
> >>> to have TYPE_USER_CREATABLE interface.
> >>> I'd go this way rather than overhaul object_unparent() in this case.
> >> What about these changes?
> >> 1. Add a callback function named 'ObjectCanBeDeleted *can_be_deleted' to 
> >> struct ObjectClass in object.h
> >> 2. Call the function in qmp_object_del of qmp.c, says:
> >>      ObjectClass *obj_class = object_get_class(obj);
> >>          if (obj_class->can_be_deleted)
> >>              if (!obj_class->can_be_deleted(obj))
> >>                  error out
> >>
> >> 3. Then implement can_be_deleted callback in backends, says:
> >>      static bool host_memory_backend_can_be_deleted(Object *obj) {......}
> >>
> >>      static void host_memory_backend_class_init(ObjectClass *oc, void 
> >> *data) {
> >>          ......
> >>          oc->can_be_deleted = host_memory_backend_can_be_deleted;
> >>      }
> > 
> > all backends currently implement TYPE_USER_CREATABLE interface,
> > so I'd rather extend UserCreatableClass with:
> > 
> >      bool can_be_deleted(UserCreatable *uc)
> > 
> > callback, which backends could implement if needed.
> 
> As for the implementation, could we simply look at Object::ref field?
What value of ref, one would use to decide if deletion is possible?

In generic case object can have ref > 1 but still be eligible for deleting
via object-del.

> 
> Regards,
> Andreas
> 

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

[Paolo Bonzini]

On 26/03/2015 11:29, Igor Mammedov wrote:
> What value of ref, one would use to decide if deletion is possible?
> 
> In generic case object can have ref > 1 but still be eligible for deleting
> via object-del.

Right, for example devices are unparented with ref > 1.

Still, it could be a sane default implementation.  It doesn't even need
a function pointer until someone comes up with an object that has
different needs.

Paolo

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

[Igor Mammedov]

On Thu, 26 Mar 2015 14:37:13 +0100
Paolo Bonzini <pbonzini@redhat.com> wrote:

> 
> 
> On 26/03/2015 11:29, Igor Mammedov wrote:
> > What value of ref, one would use to decide if deletion is possible?
> > 
> > In generic case object can have ref > 1 but still be eligible for deleting
> > via object-del.
> 
> Right, for example devices are unparented with ref > 1.
> 
> Still, it could be a sane default implementation.  It doesn't even need
> a function pointer until someone comes up with an object that has
> different needs.
Not in case of memory backend, its 'ref' in unused state is 2 since
it contains at least one child, backend object is owner of MemoryRegion.

> Paolo