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

[Corey Minyard]

On 07/21/2016 10:05 AM, Corey Minyard wrote:
> On 07/21/2016 09:49 AM, Marc-André Lureau wrote:
> > Hi
> >
> > ----- Original Message -----
> > > From: Corey Minyard <address@hidden>
> > >
> > > Add an unrealize function to free the timer allocated in the
> > > realize function.
> > >
> > > Signed-off-by: Corey Minyard <address@hidden>
> > > Cc: Marc-André Lureau <address@hidden>
> > > ---
> > >   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()?

Never mind, I misunderstood that code.

-corey

> > +    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