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

[Markus Armbruster]

Peter Maydell <address@hidden> writes:

> On Mon, 17 Feb 2020 at 16:15, Philippe Mathieu-Daudé <address@hidden> wrote:
>> Per this comment in qdev.c, unrealize() is the expected default:
>>
>>      /* by default all devices were considered as hotpluggable,
>>       * so with intent to check it in generic qdev_unplug() /
>>       * device_set_realized() functions make every device
>>       * hotpluggable. Devices that shouldn't be hotpluggable,
>>       * should override it in their class_init()
>>       */
>>      dc->hotpluggable = true;

Please note that "hot-pluggable" is *not* required for "need unrealize()
to work", at least in theory.  Cold plug exists, and cold unplug could
exist.

> This comment sounds like it's documenting what was the
> previous default ("were considered") and making a minimal
> change to avoid breaking existing code where a device
> does want to be hotpluggable but isn't explicitly saying so.

Commit 1a37eca107, six years ago:

    qdev: add "hotpluggable" property to Device
    
    Currently it's possible to make PCIDevice not hotpluggable
    by using no_hotplug field of PCIDeviceClass. However it
    limits this only to PCI devices and prevents from
    generalizing hotplug code.
    
    So add similar field to DeviceClass so it could be reused
    with other Devices and would allow to replace PCI specific
    hotplug callbacks with generic implementation. Following
    patches will replace PCIDeviceClass.no_hotplug with this
    new property.
    
    In addition expose field as "hotpluggable" readonly property,
    to make it possible to read its value via QOM interface.
    
    Make DeviceClass hotpluggable by default as it was assumed
    before.

> I forget how exactly it works (the mechanism has changed
> several times) but in practice a sysbus device is generally
> not hotpluggable, and that's what most devices are.

A device's "hot-pluggability" comes into play only when both bus and
machine support hot-plug.  And before we even get there, the device
needs to be "pluggable", i.e. dc->user_creatable.

Bus types supporting hot plug include PCI, SCSI, USB, virtio-serial-bus.
Grep for qbus_set_hotplug_handler().