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