Re: [Qemu-devel] [PATCH 4/9] object: remove object_finalize

[Anthony Liguori]

Andreas Färber <address@hidden> writes:

> Am 26.08.2012 17:51, schrieb Anthony Liguori:
>> Callers should just use object_unref
>> 
>> Signed-off-by: Anthony Liguori <address@hidden>
>> ---
>>  hw/qdev.c             |    4 ----
>>  include/qemu/object.h |    9 ---------
>>  qom/object.c          |    2 +-
>>  3 files changed, 1 insertions(+), 14 deletions(-)
>> 
>> diff --git a/hw/qdev.c b/hw/qdev.c
>> index 6b61daa..fdee91f 100644
>> --- a/hw/qdev.c
>> +++ b/hw/qdev.c
>> @@ -678,13 +678,9 @@ static void device_initfn(Object *obj)
>>  static void device_finalize(Object *obj)
>>  {
>>      DeviceState *dev = DEVICE(obj);
>> -    BusState *bus;
>>      DeviceClass *dc = DEVICE_GET_CLASS(dev);
>>  
>>      if (dev->state == DEV_STATE_INITIALIZED) {
>> -        while (dev->num_child_bus) {
>> -            bus = QLIST_FIRST(&dev->child_bus);
>> -        }
>>          if (qdev_get_vmsd(dev)) {
>>              vmstate_unregister(dev, qdev_get_vmsd(dev), dev);
>>          }
>
> This seems to answer my remark on 3/9, should've been squashed into that
> one.
>
>> diff --git a/include/qemu/object.h b/include/qemu/object.h
>> index 487adcd..8bc9935 100644
>> --- a/include/qemu/object.h
>> +++ b/include/qemu/object.h
>> @@ -490,15 +490,6 @@ void object_initialize_with_type(void *data, Type type);
>>  void object_initialize(void *obj, const char *typename);
>>  
>>  /**
>> - * object_finalize:
>> - * @obj: The object to finalize.
>> - *
>> - * This function destroys and object without freeing the memory associated 
>> with
>> - * it.
>> - */
>> -void object_finalize(void *obj);
>> -
>> -/**
>>   * object_dynamic_cast:
>>   * @obj: The object to cast.
>>   * @typename: The @typename to cast to.
>> diff --git a/qom/object.c b/qom/object.c
>> index 44135c3..1144f79 100644
>> --- a/qom/object.c
>> +++ b/qom/object.c
>> @@ -375,7 +375,7 @@ static void object_deinit(Object *obj, TypeImpl *type)
>>      }
>>  }
>>  
>> -void object_finalize(void *data)
>> +static void object_finalize(void *data)
>>  {
>>      Object *obj = data;
>>      TypeImpl *ti = obj->class->type;
>
> This is what I was referring to with breaking the encapsulation on 3/9:
> When we have a PHB with embedded PCIDevice on its PCIBus, as
> demonstrated with i440fx and prep_pci, then when doing object_delete()
> on the whole thing I expect the main object's finalizer to call
> object_finalize() on its embedded childs, forcing their uninit or an
> assert if a programming error. Not just an unref that might or might not
> finalize it.
>
> If however finalize is called only at refcount 0 then who will unref the
> self-created children? Finalize would never be called due to pending
> references by its children...

The refs are stored as properties and when properties are removed during
object destruction, those references are dropped.

That is what takes the refcount to 0.  We could add a
g_assert(child->ref == 1) before removing the property to catch
programming errors.

Regards,

Anthony Liguori

>
> Andreas
>
> -- 
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg