Re: [Qemu-devel] [PATCH 1/3] ipmi_bmc_sim: Free timer

[Corey Minyard <minyard@acm.org>]

On 07/21/2016 09:49 AM, Marc-André Lureau wrote:
> Hi
>
> ----- Original Message -----
>> From: Corey Minyard <cminyard@mvista.com>
>>
>> Add an unrealize function to free the timer allocated in the
>> realize function.
>>
>> Signed-off-by: Corey Minyard <cminyard@mvista.com>
>> Cc: Marc-André Lureau <mlureau@redhat.com>
>> ---
>>   hw/ipmi/ipmi_bmc_sim.c | 10 ++++++++++
>>   1 file changed, 10 insertions(+)
>>
>> diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c
>> index dc9c14c..c83adf8 100644
>> --- a/hw/ipmi/ipmi_bmc_sim.c
>> +++ b/hw/ipmi/ipmi_bmc_sim.c
>> @@ -1786,12 +1786,22 @@ static void ipmi_sim_realize(DeviceState *dev, Error
>> **errp)
>>       vmstate_register(NULL, 0, &vmstate_ipmi_sim, ibs);
>>   }
>>   
>> +static void ipmi_sim_unrealize(DeviceState *dev, Error **errp)
>> +{
>> +    IPMIBmc *b = IPMI_BMC(dev);
>> +    IPMIBmcSim *ibs = IPMI_BMC_SIMULATOR(b);
>> +
>> +    timer_del(ibs->timer);
>> +    timer_free(ibs->timer);
>> +}
>> +
> I think we may want to unrealize more here:
>
> +static void ipmi_sim_unrealize(DeviceState *dev, Error **errp)
> +{
> +    IPMIBmcSim *ibs = IPMI_BMC_SIMULATOR(dev);
> +    IPMIRcvBufEntry *msg, *tmp;
> +
> +    vmstate_unregister(NULL, &vmstate_ipmi_sim, ibs);

Isn't this already done automatically in device_set_realized()?

> +    timer_del(ibs->timer);
> +    timer_free(ibs->timer);
> +    QTAILQ_FOREACH_SAFE(msg, &ibs->rcvbufs, entry, tmp) {
> +        QTAILQ_REMOVE(&ibs->rcvbufs, msg, entry);
> +        g_free(msg);
> +    }
> +    qemu_mutex_destroy(&ibs->lock);

That mutex should probably go away, it was a vestige of
something else that is no longer there.

Thanks,

-corey

>>   static void ipmi_sim_class_init(ObjectClass *oc, void *data)
>>   {
>>       DeviceClass *dc = DEVICE_CLASS(oc);
>>       IPMIBmcClass *bk = IPMI_BMC_CLASS(oc);
>>   
>>       dc->realize = ipmi_sim_realize;
>> +    dc->unrealize = ipmi_sim_unrealize;
>>       bk->handle_command = ipmi_sim_handle_command;
>>   }
>>   
>> --
>> 2.7.4
>>
>>