From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51582) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bQFeC-00045V-UK for qemu-devel@nongnu.org; Thu, 21 Jul 2016 11:12:25 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bQFe8-0005kK-Iq for qemu-devel@nongnu.org; Thu, 21 Jul 2016 11:12:23 -0400 Received: from mail-oi0-x242.google.com ([2607:f8b0:4003:c06::242]:33027) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bQFe8-0005k5-Dv for qemu-devel@nongnu.org; Thu, 21 Jul 2016 11:12:20 -0400 Received: by mail-oi0-x242.google.com with SMTP id l9so7964038oih.0 for ; Thu, 21 Jul 2016 08:12:20 -0700 (PDT) Sender: Corey Minyard Reply-To: minyard@acm.org References: <1469112292-30548-1-git-send-email-minyard@acm.org> <568261880.7102035.1469112560328.JavaMail.zimbra@redhat.com> <5790E4A3.3000607@acm.org> From: Corey Minyard Message-ID: <5790E651.2090006@acm.org> Date: Thu, 21 Jul 2016 10:12:17 -0500 MIME-Version: 1.0 In-Reply-To: <5790E4A3.3000607@acm.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH 1/3] ipmi_bmc_sim: Free timer List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?UTF-8?Q?Marc-Andr=c3=a9_Lureau?= Cc: qemu-devel@nongnu.org, 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 >>> >>> Add an unrealize function to free the timer allocated in the >>> realize function. >>> >>> Signed-off-by: Corey Minyard >>> Cc: Marc-André Lureau >>> --- >>> 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 >>> >>> >