Hi
----- Original Message -----
> From: Corey Minyard <cminy...@mvista.com>
>
> Add an unrealize function to free the timer allocated in the
> realize function, unregsiter the vmstate, and free any
> pending messages.
I don't know how to test this either, the device seems to be hotpluggable, but
doing device_del crashes qemu. Looks like it would be worth fixing that too
(even better would be to automate this kind of test for all devices, but that's
just some thought)
> Also, get rid of the unnecessary mutex, it was a vestige
> of something else that was not done. That way we don't
> have to free it.
You may want to split this in a seperate patch
>
> Signed-off-by: Corey Minyard <cminy...@mvista.com>
> Cc: Marc-André Lureau <mlur...@redhat.com>
> ---
> hw/ipmi/ipmi_bmc_sim.c | 22 ++++++++++++++++------
> 1 file changed, 16 insertions(+), 6 deletions(-)
>
> diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c
> index dc9c14c..fe92b93 100644
> --- a/hw/ipmi/ipmi_bmc_sim.c
> +++ b/hw/ipmi/ipmi_bmc_sim.c
> @@ -217,7 +217,6 @@ struct IPMIBmcSim {
> /* Odd netfns are for responses, so we only need the even ones. */
> const IPMINetfn *netfns[MAX_NETFNS / 2];
>
> - QemuMutex lock;
> /* We allow one event in the buffer */
> uint8_t evtbuf[16];
>
> @@ -940,7 +939,6 @@ static void get_msg(IPMIBmcSim *ibs,
> {
> IPMIRcvBufEntry *msg;
>
> - qemu_mutex_lock(&ibs->lock);
> if (QTAILQ_EMPTY(&ibs->rcvbufs)) {
> rsp_buffer_set_error(rsp, 0x80); /* Queue empty */
> goto out;
> @@ -960,7 +958,6 @@ static void get_msg(IPMIBmcSim *ibs,
> }
>
> out:
> - qemu_mutex_unlock(&ibs->lock);
> return;
> }
>
> @@ -1055,11 +1052,9 @@ static void send_msg(IPMIBmcSim *ibs,
> end_msg:
> msg->buf[msg->len] = ipmb_checksum(msg->buf, msg->len, 0);
> msg->len++;
> - qemu_mutex_lock(&ibs->lock);
> QTAILQ_INSERT_TAIL(&ibs->rcvbufs, msg, entry);
> ibs->msg_flags |= IPMI_BMC_MSG_FLAG_RCV_MSG_QUEUE;
> k->set_atn(s, 1, attn_irq_enabled(ibs));
> - qemu_mutex_unlock(&ibs->lock);
> }
>
> static void do_watchdog_reset(IPMIBmcSim *ibs)
> @@ -1753,7 +1748,6 @@ static void ipmi_sim_realize(DeviceState *dev, Error
> **errp)
> unsigned int i;
> IPMIBmcSim *ibs = IPMI_BMC_SIMULATOR(b);
>
> - qemu_mutex_init(&ibs->lock);
> QTAILQ_INIT(&ibs->rcvbufs);
>
> ibs->bmc_global_enables = (1 << IPMI_BMC_EVENT_LOG_BIT);
> @@ -1786,12 +1780,28 @@ 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);
> + IPMIRcvBufEntry *msg, *tmp;
> + IPMIBmcSim *ibs = IPMI_BMC_SIMULATOR(b);
> +
> + vmstate_unregister(NULL, &vmstate_ipmi_sim, ibs);
> + 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);
> + }
> +}
> +
Otherwise, for completeness, this looks good so
Reviewed-by: Marc-André Lureau <marcandre.lur...@redhat.com>
> 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
>
>
