On 20 September 2017 at 08:50, Marcel Apfelbaum <mar...@redhat.com> wrote: > Hi Thomas, > > > On 19/09/2017 11:55, Thomas Huth wrote: >> >> Historically we've marked all devices as hotpluggable by default. However, >> most devices are not hotpluggable, and you also need a HotplugHandler to >> support these devices. So if the user tries to "device_add" or >> "device_del" >> such a non-hotpluggable device during runtime, either nothing really >> usable >> happens, or QEMU even crashes/aborts unexpectedly (see for example commit >> 84ebd3e8c7d4fe955b - "Mark diag288 watchdog as non-hotpluggable"). >> So let's change this dangerous default behaviour and mark the devices as >> non-hotpluggable by default. Certain parent devices classes which are >> known >> as hotpluggable (e.g. PCI, USB, etc.) are marked with "hotpluggable = >> true", >> so that devices that are derived from these classes continue to work as >> expected. >> >> Signed-off-by: Thomas Huth <th...@redhat.com> >> --- >> Note: I've marked the patch as RFC since I'm not 100% sure whether I've >> correctly identified all devices that should still be marked as hot- >> pluggable. Feedback is welcome! >> >> hw/core/qdev.c | 10 ++++------ >> hw/cpu/core.c | 1 + >> hw/mem/nvdimm.c | 3 +++ >> hw/mem/pc-dimm.c | 1 + >> hw/pci/pci.c | 1 + >> hw/s390x/ccw-device.c | 1 + >> hw/scsi/scsi-bus.c | 1 + >> hw/usb/bus.c | 1 + >> 8 files changed, 13 insertions(+), 6 deletions(-) >> >> diff --git a/hw/core/qdev.c b/hw/core/qdev.c >> index 606ab53..c4f1902 100644 >> --- a/hw/core/qdev.c >> +++ b/hw/core/qdev.c >> @@ -1120,13 +1120,11 @@ static void device_class_init(ObjectClass *class, >> void *data) >> dc->realize = device_realize; >> dc->unrealize = device_unrealize; >> - /* 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() >> + /* >> + * All devices are considered as cold-pluggable by default. The >> devices >> + * that are hotpluggable should override it in their class_init(). >> */ >> - dc->hotpluggable = true; >> + dc->hotpluggable = false; > > > I agree with the defensive mode as long as we don't > introduce any regression...
Is it possible to hack together some kind of test code that can give us a list of the devices compiled in that have hotpluggable enabled? Then we could compare before and after to see which devices we've changed. thanks -- PMM