| Message ID | 1469112292-30548-1-git-send-email-minyard@acm.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 >> >>
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 <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()? 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 >>> >>> >
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); +} + 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; }