Patchwork qom: if @instance_size==0, assign size of object to parent object size

login
register
mail settings
Submitter Mitsyanko Igor
Date Feb. 28, 2012, 7:18 a.m.
Message ID <1330413510-9289-1-git-send-email-i.mitsyanko@samsung.com>
Download mbox | patch
Permalink /patch/143364/
State New
Headers show

Comments

Mitsyanko Igor - Feb. 28, 2012, 7:18 a.m.
QOM documentation states that for objects of type with @instance_size == 0 size
will be assigned to match parent object's size. But currently this feauture is
not implemented and qemu asserts during creation of object with zero instance_size.
This patch adjusts actual behaviour in accordance with documentation.

Signed-off-by: Igor Mitsyanko <i.mitsyanko@samsung.com>
---
 qom/object.c |   15 +++++++++++++++
 1 files changed, 15 insertions(+), 0 deletions(-)
Paolo Bonzini - Feb. 28, 2012, 8:39 a.m.
Il 28/02/2012 08:18, Igor Mitsyanko ha scritto:
> QOM documentation states that for objects of type with @instance_size == 0 size
> will be assigned to match parent object's size. But currently this feauture is
> not implemented and qemu asserts during creation of object with zero instance_size.
> This patch adjusts actual behaviour in accordance with documentation.

You can do it just once, in type_get_parent instead.

Paolo
Mitsyanko Igor - Feb. 28, 2012, 9:43 a.m.
On 02/28/2012 12:39 PM, Paolo Bonzini wrote:
> Il 28/02/2012 08:18, Igor Mitsyanko ha scritto:
>> QOM documentation states that for objects of type with @instance_size == 0 size
>> will be assigned to match parent object's size. But currently this feauture is
>> not implemented and qemu asserts during creation of object with zero instance_size.
>> This patch adjusts actual behaviour in accordance with documentation.
>
> You can do it just once, in type_get_parent instead.
>
> Paolo
>
>
>
Can't understand how type_get_parent() is relevant to this, we can call 
object_initialize() or object_new_with_type() and these two functions 
must set instance_size before calling object_initialize_with_type(). Up 
to this point there are no calls to type_get_parent().

Mitsyanko Igor
ASWG, Moscow R&D center, Samsung Electronics
email: i.mitsyanko@samsung.com
Paolo Bonzini - Feb. 28, 2012, 10:03 a.m.
Il 28/02/2012 10:43, Igor Mitsyanko ha scritto:
> On 02/28/2012 12:39 PM, Paolo Bonzini wrote:
>> Il 28/02/2012 08:18, Igor Mitsyanko ha scritto:
>>> QOM documentation states that for objects of type with @instance_size
>>> == 0 size
>>> will be assigned to match parent object's size. But currently this
>>> feauture is
>>> not implemented and qemu asserts during creation of object with zero
>>> instance_size.
>>> This patch adjusts actual behaviour in accordance with documentation.
>>
>> You can do it just once, in type_get_parent instead.

Sorry, rewind.  "You can do it in type_class_init instead" (you are
obviously doing it just once since you assign to type->instance_size).
type_class_init mostly deals with class initialization, but it's really
the place where a type is hooked up with its parent.  Perhaps
type_late_init would be a better name.

I think the problem is misplaced type_class_init calls.

 void object_initialize(void *data, const char *typename)
 {
     TypeImpl *type = type_get_by_name(typename);

+    type->instance_size = object_get_instance_size(type);
     object_initialize_with_type(data, type);
 }

object_initialize_with_type needs to call type_class_init before testing
type->instance_size, not after.

@@ -357,6 +371,7 @@ Object *object_new_with_type(Type type)

     g_assert(type != NULL);

+    type->instance_size = object_get_instance_size(type);

And this should also be a call to type_class_init.

     obj = g_malloc(type->instance_size);
     object_initialize_with_type(obj, type);
     object_ref(obj);

Paolo
Mitsyanko Igor - Feb. 28, 2012, 11:09 a.m.
On 02/28/2012 02:03 PM, Paolo Bonzini wrote:
> Il 28/02/2012 10:43, Igor Mitsyanko ha scritto:
>> On 02/28/2012 12:39 PM, Paolo Bonzini wrote:
>>> Il 28/02/2012 08:18, Igor Mitsyanko ha scritto:
>>>> QOM documentation states that for objects of type with @instance_size
>>>> == 0 size
>>>> will be assigned to match parent object's size. But currently this
>>>> feauture is
>>>> not implemented and qemu asserts during creation of object with zero
>>>> instance_size.
>>>> This patch adjusts actual behaviour in accordance with documentation.
>>>
>>> You can do it just once, in type_get_parent instead.
>
> Sorry, rewind.  "You can do it in type_class_init instead" (you are
> obviously doing it just once since you assign to type->instance_size).
> type_class_init mostly deals with class initialization, but it's really
> the place where a type is hooked up with its parent...

Ok, that's obviously a much better approach.

   Perhaps
> type_late_init would be a better name.

How about simple type_initialization()?
Paolo Bonzini - Feb. 28, 2012, 11:21 a.m.
Il 28/02/2012 12:09, Igor Mitsyanko ha scritto:
> 
>   Perhaps
>> type_late_init would be a better name.
> 
> How about simple type_initialization()?

We use verbs, so it would be type_initialize, but yes.

Paolo

Patch

diff --git a/qom/object.c b/qom/object.c
index aa037d2..3cc2849 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -275,10 +275,24 @@  void object_initialize_with_type(void *data, TypeImpl *type)
     object_init_with_type(obj, type);
 }
 
+static size_t object_get_instance_size(TypeImpl *ti)
+{
+    if (ti->instance_size) {
+        return ti->instance_size;
+    }
+
+    if (type_has_parent(ti)) {
+        return object_get_instance_size(type_get_parent(ti));
+    }
+
+    return 0;
+}
+
 void object_initialize(void *data, const char *typename)
 {
     TypeImpl *type = type_get_by_name(typename);
 
+    type->instance_size = object_get_instance_size(type);
     object_initialize_with_type(data, type);
 }
 
@@ -357,6 +371,7 @@  Object *object_new_with_type(Type type)
 
     g_assert(type != NULL);
 
+    type->instance_size = object_get_instance_size(type);
     obj = g_malloc(type->instance_size);
     object_initialize_with_type(obj, type);
     object_ref(obj);