[Qemu-devel] [PATCH v4 0/2] Add generic can_be_deleted to UserCreatableClass.

[Qemu-devel] [PATCH v4 0/2] Add generic can_be_deleted to UserCreatableClass.

[Lin Ma]

The patchset adds a generic can_be_deleted callback to UserCreatableClass.
It prevents removing a usercreatable object if the callback returns false.

Backends could implement the callback if it shoudn't be removed while it's
in use.

Thank Peter Crosthwaite, Paolo Bonzini, Andreas Färber and Igor Mammedov for
helping review.


ChangeLog:
V4:
Add function user_creatable_can_be_deleted to wrapper can_be_deleted method.
Remove some unnecessary code.

V3:
Move the callback to the correct place: UserCreatableClass.
Move the backend callback implementation to hostmem.c.

V2:
Make it generic, add the can_be_deleted callback to TypeInfo and TypeImpl.
Implement the callback in hostmem-file.c and hostmem-ram.c.

V1:
Initial version, hard coded in object_del to prevent removing an in-use
host memory backend.


Lin Ma (2):
  qom: Add can_be_deleted callback to UserCreatableClass
  hostmem: Prevent removing an in-use memory backend object

 backends/hostmem.c              | 14 ++++++++++++++
 include/qom/object_interfaces.h | 14 ++++++++++++++
 qmp.c                           |  5 +++++
 qom/object_interfaces.c         | 15 +++++++++++++++
 4 files changed, 48 insertions(+)

-- 
2.1.4

[Qemu-devel] [PATCH v4 1/2] qom: Add can_be_deleted callback to UserCreatableClass

[Lin Ma]

If backends implement the can_be_deleted and it returns false,
Then the qmp_object_del won't delete the given backends.

Signed-off-by: Lin Ma <lma@suse.com>
---
 include/qom/object_interfaces.h | 14 ++++++++++++++
 qmp.c                           |  5 +++++
 qom/object_interfaces.c         | 15 +++++++++++++++
 3 files changed, 34 insertions(+)

diff --git a/include/qom/object_interfaces.h b/include/qom/object_interfaces.h
index b792283..012b653 100644
--- a/include/qom/object_interfaces.h
+++ b/include/qom/object_interfaces.h
@@ -25,6 +25,8 @@ typedef struct UserCreatable {
  * UserCreatableClass:
  * @parent_class: the base class
  * @complete: callback to be called after @obj's properties are set.
+ * @can_be_deleted: callback to be called before an object is removed
+ * to check if @obj can be removed safely.
  *
  * Interface is designed to work with -object/object-add/object_add
  * commands.
@@ -47,6 +49,7 @@ typedef struct UserCreatableClass {
 
     /* <public> */
     void (*complete)(UserCreatable *uc, Error **errp);
+    bool (*can_be_deleted)(UserCreatable *uc, Error **errp);
 } UserCreatableClass;
 
 /**
@@ -59,4 +62,15 @@ typedef struct UserCreatableClass {
  * nothing.
  */
 void user_creatable_complete(Object *obj, Error **errp);
+
+/**
+ * user_creatable_can_be_deleted:
+ * @obj: the object whose can_be_deleted() method is called if defined
+ * @errp: if an error occurs, a pointer to an area to store the error
+ *
+ * Wrapper to call can_be_deleted() method if one of types it's inherited
+ * from implements USER_CREATABLE interface, otherwise the call does
+ * nothing.
+ */
+bool user_creatable_can_be_deleted(Object *obj, Error **errp);
 #endif
diff --git a/qmp.c b/qmp.c
index c479e77..dc61da1 100644
--- a/qmp.c
+++ b/qmp.c
@@ -711,6 +711,11 @@ void qmp_object_del(const char *id, Error **errp)
         error_setg(errp, "object id not found");
         return;
     }
+
+    if (!user_creatable_can_be_deleted(obj, errp)) {
+        error_setg(errp, "%s is in used, can not be deleted", id);
+        return;
+    }
     object_unparent(obj);
 }
 
diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
index 6360818..25ae6ca 100644
--- a/qom/object_interfaces.c
+++ b/qom/object_interfaces.c
@@ -18,6 +18,21 @@ void user_creatable_complete(Object *obj, Error **errp)
     }
 }
 
+bool user_creatable_can_be_deleted(Object *obj, Error **errp)
+{
+
+    UserCreatableClass *ucc;
+    UserCreatable *uc =
+        (UserCreatable *)object_dynamic_cast(obj, TYPE_USER_CREATABLE);
+
+    ucc = USER_CREATABLE_GET_CLASS(uc);
+    if (ucc->can_be_deleted) {
+        return ucc->can_be_deleted(uc, errp);
+    } else {
+        return true;
+    }
+}
+
 static void register_types(void)
 {
     static const TypeInfo uc_interface_info = {
-- 
2.1.4

[Qemu-devel] [PATCH v4 2/2] hostmem: Prevent removing an in-use memory backend object

[Lin Ma]

showing a memory device whose memdev is removed leads an assert:

(qemu) object_add memory-backend-ram,id=ram0,size=128M
(qemu) device_add pc-dimm,id=d0,memdev=ram0
(qemu) object_del ram0
(qemu) info memory-devices
**
ERROR:qom/object.c:1274:object_get_canonical_path_component:\
                            assertion failed: (obj->parent != NULL)
Aborted

The patch prevents removing an in-use mem backend and error out.

Signed-off-by: Lin Ma <lma@suse.com>
---
 backends/hostmem.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/backends/hostmem.c b/backends/hostmem.c
index 99e8f99..dbf94a9 100644
--- a/backends/hostmem.c
+++ b/backends/hostmem.c
@@ -335,12 +335,26 @@ host_memory_backend_memory_complete(UserCreatable *uc, Error **errp)
     }
 }
 
+static bool
+host_memory_backend_can_be_deleted(UserCreatable *uc, Error **errp)
+{
+    MemoryRegion *mr;
+    mr = host_memory_backend_get_memory(MEMORY_BACKEND(uc), errp);
+    if (memory_region_is_mapped(mr)) {
+        return false;
+    } else {
+        return true;
+    }
+}
+
 static void
 host_memory_backend_class_init(ObjectClass *oc, void *data)
 {
     UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc);
 
     ucc->complete = host_memory_backend_memory_complete;
+
+    ucc->can_be_deleted = host_memory_backend_can_be_deleted;
 }
 
 static const TypeInfo host_memory_backend_info = {
-- 
2.1.4

Re: [Qemu-devel] [PATCH v4 1/2] qom: Add can_be_deleted callback to UserCreatableClass

[Igor Mammedov]

On Fri, 27 Mar 2015 13:36:11 +0800
Lin Ma <lma@suse.com> wrote:

> If backends implement the can_be_deleted and it returns false,
> Then the qmp_object_del won't delete the given backends.
> 
> Signed-off-by: Lin Ma <lma@suse.com>
> ---
>  include/qom/object_interfaces.h | 14 ++++++++++++++
>  qmp.c                           |  5 +++++
>  qom/object_interfaces.c         | 15 +++++++++++++++
>  3 files changed, 34 insertions(+)
> 
> diff --git a/include/qom/object_interfaces.h b/include/qom/object_interfaces.h
> index b792283..012b653 100644
> --- a/include/qom/object_interfaces.h
> +++ b/include/qom/object_interfaces.h
> @@ -25,6 +25,8 @@ typedef struct UserCreatable {
>   * UserCreatableClass:
>   * @parent_class: the base class
>   * @complete: callback to be called after @obj's properties are set.
> + * @can_be_deleted: callback to be called before an object is removed
> + * to check if @obj can be removed safely.
>   *
>   * Interface is designed to work with -object/object-add/object_add
>   * commands.
> @@ -47,6 +49,7 @@ typedef struct UserCreatableClass {
>  
>      /* <public> */
>      void (*complete)(UserCreatable *uc, Error **errp);
> +    bool (*can_be_deleted)(UserCreatable *uc, Error **errp);
>  } UserCreatableClass;
>  
>  /**
> @@ -59,4 +62,15 @@ typedef struct UserCreatableClass {
>   * nothing.
>   */
>  void user_creatable_complete(Object *obj, Error **errp);
> +
> +/**
> + * user_creatable_can_be_deleted:
> + * @obj: the object whose can_be_deleted() method is called if defined
> + * @errp: if an error occurs, a pointer to an area to store the error
> + *
> + * Wrapper to call can_be_deleted() method if one of types it's inherited
> + * from implements USER_CREATABLE interface, otherwise the call does
> + * nothing.
> + */
> +bool user_creatable_can_be_deleted(Object *obj, Error **errp);
>  #endif
> diff --git a/qmp.c b/qmp.c
> index c479e77..dc61da1 100644
> --- a/qmp.c
> +++ b/qmp.c
> @@ -711,6 +711,11 @@ void qmp_object_del(const char *id, Error **errp)
>          error_setg(errp, "object id not found");
>          return;
>      }
> +
> +    if (!user_creatable_can_be_deleted(obj, errp)) {
> +        error_setg(errp, "%s is in used, can not be deleted", id);
                                ^^^ is in use || is used
> +        return;
> +    }
>      object_unparent(obj);
>  }
>  
> diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
> index 6360818..25ae6ca 100644
> --- a/qom/object_interfaces.c
> +++ b/qom/object_interfaces.c
> @@ -18,6 +18,21 @@ void user_creatable_complete(Object *obj, Error **errp)
>      }
>  }
>  
> +bool user_creatable_can_be_deleted(Object *obj, Error **errp)
> +{
> +
> +    UserCreatableClass *ucc;
> +    UserCreatable *uc =
> +        (UserCreatable *)object_dynamic_cast(obj, TYPE_USER_CREATABLE);
direc cast is wron, pls use type specific caster with QOM objects
in this case you can even drop dynamic cast since /objects holds
only TYPE_USER_CREATABLE objects and just do
  UserCreatable *uc = USER_CREATABLE(obj)
you don't check for null pointer anyway

> +
> +    ucc = USER_CREATABLE_GET_CLASS(uc);
> +    if (ucc->can_be_deleted) {
> +        return ucc->can_be_deleted(uc, errp);
> +    } else {
> +        return true;
> +    }
> +}
> +
>  static void register_types(void)
>  {
>      static const TypeInfo uc_interface_info = {

Re: [Qemu-devel] [PATCH v4 2/2] hostmem: Prevent removing an in-use memory backend object

[Igor Mammedov]

On Fri, 27 Mar 2015 13:36:12 +0800
Lin Ma <lma@suse.com> wrote:

> showing a memory device whose memdev is removed leads an assert:
> 
> (qemu) object_add memory-backend-ram,id=ram0,size=128M
> (qemu) device_add pc-dimm,id=d0,memdev=ram0
> (qemu) object_del ram0
> (qemu) info memory-devices
> **
> ERROR:qom/object.c:1274:object_get_canonical_path_component:\
>                             assertion failed: (obj->parent != NULL)
> Aborted
> 
> The patch prevents removing an in-use mem backend and error out.
> 
> Signed-off-by: Lin Ma <lma@suse.com>
> ---
>  backends/hostmem.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/backends/hostmem.c b/backends/hostmem.c
> index 99e8f99..dbf94a9 100644
> --- a/backends/hostmem.c
> +++ b/backends/hostmem.c
> @@ -335,12 +335,26 @@ host_memory_backend_memory_complete(UserCreatable *uc, Error **errp)
>      }
>  }
>  
> +static bool
> +host_memory_backend_can_be_deleted(UserCreatable *uc, Error **errp)
> +{
> +    MemoryRegion *mr;
add separating line here

> +    mr = host_memory_backend_get_memory(MEMORY_BACKEND(uc), errp);
> +    if (memory_region_is_mapped(mr)) {
> +        return false;
> +    } else {
> +        return true;
> +    }
> +}
> +
>  static void
>  host_memory_backend_class_init(ObjectClass *oc, void *data)
>  {
>      UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc);
>  
>      ucc->complete = host_memory_backend_memory_complete;
> +
unnecessary line, pls drop.

> +    ucc->can_be_deleted = host_memory_backend_can_be_deleted;
>  }
>  
>  static const TypeInfo host_memory_backend_info = {

Re: [Qemu-devel] [PATCH v4 0/2] Add generic can_be_deleted to UserCreatableClass.

[Igor Mammedov]

On Fri, 27 Mar 2015 13:36:10 +0800
Lin Ma <lma@suse.com> wrote:

> The patchset adds a generic can_be_deleted callback to UserCreatableClass.
> It prevents removing a usercreatable object if the callback returns false.
> 
> Backends could implement the callback if it shoudn't be removed while it's
> in use.
> 
> Thank Peter Crosthwaite, Paolo Bonzini, Andreas Färber and Igor Mammedov for
> helping review.
perhaps it's better to take this into 2.3


> 
> 
> ChangeLog:
> V4:
> Add function user_creatable_can_be_deleted to wrapper can_be_deleted method.
> Remove some unnecessary code.
> 
> V3:
> Move the callback to the correct place: UserCreatableClass.
> Move the backend callback implementation to hostmem.c.
> 
> V2:
> Make it generic, add the can_be_deleted callback to TypeInfo and TypeImpl.
> Implement the callback in hostmem-file.c and hostmem-ram.c.
> 
> V1:
> Initial version, hard coded in object_del to prevent removing an in-use
> host memory backend.
> 
> 
> Lin Ma (2):
>   qom: Add can_be_deleted callback to UserCreatableClass
>   hostmem: Prevent removing an in-use memory backend object
> 
>  backends/hostmem.c              | 14 ++++++++++++++
>  include/qom/object_interfaces.h | 14 ++++++++++++++
>  qmp.c                           |  5 +++++
>  qom/object_interfaces.c         | 15 +++++++++++++++
>  4 files changed, 48 insertions(+)
> 

Re: [Qemu-devel] [PATCH v4 1/2] qom: Add can_be_deleted callback to UserCreatableClass

[Lin Ma]

在 2015年03月27日 22:35, Igor Mammedov 写道:
> On Fri, 27 Mar 2015 13:36:11 +0800
> Lin Ma <lma@suse.com> wrote:
>
>> If backends implement the can_be_deleted and it returns false,
>> Then the qmp_object_del won't delete the given backends.
>>
>> Signed-off-by: Lin Ma <lma@suse.com>
>> ---
>>   include/qom/object_interfaces.h | 14 ++++++++++++++
>>   qmp.c                           |  5 +++++
>>   qom/object_interfaces.c         | 15 +++++++++++++++
>>   3 files changed, 34 insertions(+)
>>
>> diff --git a/include/qom/object_interfaces.h b/include/qom/object_interfaces.h
>> index b792283..012b653 100644
>> --- a/include/qom/object_interfaces.h
>> +++ b/include/qom/object_interfaces.h
>> @@ -25,6 +25,8 @@ typedef struct UserCreatable {
>>    * UserCreatableClass:
>>    * @parent_class: the base class
>>    * @complete: callback to be called after @obj's properties are set.
>> + * @can_be_deleted: callback to be called before an object is removed
>> + * to check if @obj can be removed safely.
>>    *
>>    * Interface is designed to work with -object/object-add/object_add
>>    * commands.
>> @@ -47,6 +49,7 @@ typedef struct UserCreatableClass {
>>   
>>       /* <public> */
>>       void (*complete)(UserCreatable *uc, Error **errp);
>> +    bool (*can_be_deleted)(UserCreatable *uc, Error **errp);
>>   } UserCreatableClass;
>>   
>>   /**
>> @@ -59,4 +62,15 @@ typedef struct UserCreatableClass {
>>    * nothing.
>>    */
>>   void user_creatable_complete(Object *obj, Error **errp);
>> +
>> +/**
>> + * user_creatable_can_be_deleted:
>> + * @obj: the object whose can_be_deleted() method is called if defined
>> + * @errp: if an error occurs, a pointer to an area to store the error
>> + *
>> + * Wrapper to call can_be_deleted() method if one of types it's inherited
>> + * from implements USER_CREATABLE interface, otherwise the call does
>> + * nothing.
>> + */
>> +bool user_creatable_can_be_deleted(Object *obj, Error **errp);
>>   #endif
>> diff --git a/qmp.c b/qmp.c
>> index c479e77..dc61da1 100644
>> --- a/qmp.c
>> +++ b/qmp.c
>> @@ -711,6 +711,11 @@ void qmp_object_del(const char *id, Error **errp)
>>           error_setg(errp, "object id not found");
>>           return;
>>       }
>> +
>> +    if (!user_creatable_can_be_deleted(obj, errp)) {
>> +        error_setg(errp, "%s is in used, can not be deleted", id);
>                                  ^^^ is in use || is used
                                                  Thanks for catching 
this. I'll modify it to 'in use'
>> +        return;
>> +    }
>>       object_unparent(obj);
>>   }
>>   
>> diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
>> index 6360818..25ae6ca 100644
>> --- a/qom/object_interfaces.c
>> +++ b/qom/object_interfaces.c
>> @@ -18,6 +18,21 @@ void user_creatable_complete(Object *obj, Error **errp)
>>       }
>>   }
>>   
>> +bool user_creatable_can_be_deleted(Object *obj, Error **errp)
>> +{
>> +
>> +    UserCreatableClass *ucc;
>> +    UserCreatable *uc =
>> +        (UserCreatable *)object_dynamic_cast(obj, TYPE_USER_CREATABLE);
> direc cast is wron, pls use type specific caster with QOM objects
> in this case you can even drop dynamic cast since /objects holds
> only TYPE_USER_CREATABLE objects and just do
>    UserCreatable *uc = USER_CREATABLE(obj)
will do.
> you don't check for null pointer anyway
May I have your suggestion about checking the null pointer? says:
if (!uc) {
     return false;  // is this make sense?
}
>
>> +
>> +    ucc = USER_CREATABLE_GET_CLASS(uc);
>> +    if (ucc->can_be_deleted) {
>> +        return ucc->can_be_deleted(uc, errp);
>> +    } else {
>> +        return true;
>> +    }
>> +}
>> +
>>   static void register_types(void)
>>   {
>>       static const TypeInfo uc_interface_info = {
>