Re: [PATCH 1/2] hw/ipmi/bmc: Delay timer_new_ns() from init to realize to avoid memleaks

[Philippe Mathieu-Daudé]

On 2/17/20 6:12 PM, Philippe Mathieu-Daudé wrote:
> On 2/17/20 5:32 PM, Peter Maydell wrote:
> > On Mon, 17 Feb 2020 at 16:15, Philippe Mathieu-Daudé 
[...]
> > > - hotpluggable devices missing unrealize()
> > >
> > >       a15mpcore_priv
> > >       a9mpcore_priv
> >
> > Most of these are not really hotpluggable. What is
> > confusing your test code is that sysbus devices get
> > the default 'hotpluggable = true' setting, but other
> > conditions usually prevent hotplugging. (The reason
> > hotpluggable is true is the default is because of
> > handling of 'dynamic sysbus' devices which live on
> > the 'platform bus'.) In particular, I think that
> > anything with !dc->user_creatable can't be hotplugged
> > unless board code specifically tries it, which would
> > be a bug for most of these devices.
[...]
> > Also, if there isn't anything for a device's unrealize
> > method to do, it doesn't need to provide one.
>
> This point is hard to check with simply with qtest.
>
> Thanks for your comments, it helped sorting few things out.

Quick check with TYPE_BITBAND which is a SysBus device, we have:

static void bitband_realize(DeviceState *dev, Error **errp)
{
    BitBandState *s = BITBAND(dev);

    if (!s->source_memory) {
        error_setg(errp, "source-memory property not set");
        return;
    }

    address_space_init(&s->source_as, s->source_memory, "bitband-source");
}

Do we need the equivalent:

static void bitband_unrealize(DeviceState *dev, Error **errp)
{
    BitBandState *s = BITBAND(dev);

    address_space_destroy(&s->source_as);
}

Or instead mark the device user_creatable=false because of the link to a 
TYPE_MEMORY_REGION?

> > thanks
> > -- PMM