[25/25] qdev: split part of device_finalize to device_unrealize

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/qdev.c |   35 ++++++++++++++++++++---------------
 1 file changed, 20 insertions(+), 15 deletions(-)

On 04/03/2012 06:15 AM, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini<pbonzini@redhat.com>
> ---
>   hw/qdev.c |   35 ++++++++++++++++++++---------------
>   1 file changed, 20 insertions(+), 15 deletions(-)
>
> diff --git a/hw/qdev.c b/hw/qdev.c
> index e674248..45f1133 100644
> --- a/hw/qdev.c
> +++ b/hw/qdev.c
> @@ -601,27 +601,31 @@ static void device_initfn(Object *obj)
>   }
>
>   /* Unlink device from bus and free the structure.  */
> -static void device_finalize(Object *obj)
> +static void device_unrealize(Object *obj)
>   {
>       DeviceState *dev = DEVICE(obj);
>       BusState *bus;
>       DeviceClass *dc = DEVICE_GET_CLASS(dev);
>
> -    if (object_is_realized(obj)) {
> -        while (dev->num_child_bus) {
> -            bus = QLIST_FIRST(&dev->child_bus);
> -            qbus_free(bus);
> -        }
> -        if (qdev_get_vmsd(dev)) {
> -            vmstate_unregister(dev, qdev_get_vmsd(dev), dev);
> -        }
> -        if (dc->exit) {
> -            dc->exit(dev);
> -        }
> -        if (dev->opts) {
> -            qemu_opts_del(dev->opts);
> -        }
> +    while (dev->num_child_bus) {
> +        bus = QLIST_FIRST(&dev->child_bus);
> +        qbus_free(bus);
> +    }

This doesn't seem symmetric to me.  We don't create busses in realize(), or at 
least we shouldn't IMHO.  We should create child busses in instance_init(). 
Should be remove them in finalize?

Regards,

Anthony Liguori

> +    if (qdev_get_vmsd(dev)) {
> +        vmstate_unregister(dev, qdev_get_vmsd(dev), dev);
>       }
> +    if (dc->exit) {
> +        dc->exit(dev);
> +    }
> +    if (dev->opts) {
> +        qemu_opts_del(dev->opts);
> +    }
> +}
> +
> +static void device_finalize(Object *obj)
> +{
> +    DeviceState *dev = DEVICE(obj);
> +
>       QTAILQ_REMOVE(&dev->parent_bus->children, dev, sibling);
>   }
>
> @@ -649,6 +653,7 @@ static void device_class_init(ObjectClass *klass, void *data)
>   {
>       klass->realize = device_realize;
>       klass->realize_children = device_realize_children;
> +    klass->unrealize = device_unrealize;
>   }
>
>   static TypeInfo device_type_info = {

Il 20/04/2012 19:56, Anthony Liguori ha scritto:
> On 04/03/2012 06:15 AM, Paolo Bonzini wrote:
>> Signed-off-by: Paolo Bonzini<pbonzini@redhat.com>
>> ---
>>   hw/qdev.c |   35 ++++++++++++++++++++---------------
>>   1 file changed, 20 insertions(+), 15 deletions(-)
>>
>> diff --git a/hw/qdev.c b/hw/qdev.c
>> index e674248..45f1133 100644
>> --- a/hw/qdev.c
>> +++ b/hw/qdev.c
>> @@ -601,27 +601,31 @@ static void device_initfn(Object *obj)
>>   }
>>
>>   /* Unlink device from bus and free the structure.  */
>> -static void device_finalize(Object *obj)
>> +static void device_unrealize(Object *obj)
>>   {
>>       DeviceState *dev = DEVICE(obj);
>>       BusState *bus;
>>       DeviceClass *dc = DEVICE_GET_CLASS(dev);
>>
>> -    if (object_is_realized(obj)) {
>> -        while (dev->num_child_bus) {
>> -            bus = QLIST_FIRST(&dev->child_bus);
>> -            qbus_free(bus);
>> -        }
>> -        if (qdev_get_vmsd(dev)) {
>> -            vmstate_unregister(dev, qdev_get_vmsd(dev), dev);
>> -        }
>> -        if (dc->exit) {
>> -            dc->exit(dev);
>> -        }
>> -        if (dev->opts) {
>> -            qemu_opts_del(dev->opts);
>> -        }
>> +    while (dev->num_child_bus) {
>> +        bus = QLIST_FIRST(&dev->child_bus);
>> +        qbus_free(bus);
>> +    }
> 
> This doesn't seem symmetric to me.  We don't create busses in realize(),
> or at least we shouldn't IMHO.  We should create child busses in
> instance_init(). Should be remove them in finalize?

Perhaps yes, I'll split the patch in two.

Paolo