qdev: Mark devices as non-hotpluggable by default

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 <[email protected]>

---
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(-)

hw/core/qdev.c +0 -1

index 606ab53..c4f1902 100644
@@ -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,

hw/core/qdev.c +0 -0

      dc->user_creatable = true;
  }
 

hw/cpu/core.c +1 -0

index bd578ab..01660aa 100644
@@ -82,6 +82,7 @@ static void cpu_core_class_init(ObjectClass *oc, void *data)
      DeviceClass *dc = DEVICE_CLASS(oc);
 
      set_bit(DEVICE_CATEGORY_CPU, dc->categories);
+    dc->hotpluggable = true;
  }
 
  static const TypeInfo cpu_core_type_info = {

hw/mem/nvdimm.c +3 -0

index 952fce5..02be9f3 100644
@@ -148,9 +148,12 @@ static MemoryRegion *nvdimm_get_vmstate_memory_region(PCDIMMDevice *dimm)
 
  static void nvdimm_class_init(ObjectClass *oc, void *data)
  {
+    DeviceClass *dc = DEVICE_CLASS(oc);
      PCDIMMDeviceClass *ddc = PC_DIMM_CLASS(oc);
      NVDIMMClass *nvc = NVDIMM_CLASS(oc);
 
+    dc->hotpluggable = true;
+
      ddc->realize = nvdimm_realize;
      ddc->get_memory_region = nvdimm_get_memory_region;
      ddc->get_vmstate_memory_region = nvdimm_get_vmstate_memory_region;

hw/mem/pc-dimm.c +1 -0

index 66eace5..1f78567 100644
@@ -443,6 +443,7 @@ static void pc_dimm_class_init(ObjectClass *oc, void *data)
      dc->unrealize = pc_dimm_unrealize;
      dc->props = pc_dimm_properties;
      dc->desc = "DIMM memory module";
+    dc->hotpluggable = true;
 
      ddc->get_memory_region = pc_dimm_get_memory_region;
      ddc->get_vmstate_memory_region = pc_dimm_get_vmstate_memory_region;

hw/pci/pci.c +1 -0

index 1e6fb88..8db380d 100644
@@ -2534,6 +2534,7 @@ static void pci_device_class_init(ObjectClass *klass, void *data)
      k->unrealize = pci_qdev_unrealize;
      k->bus_type = TYPE_PCI_BUS;
      k->props = pci_props;
+    k->hotpluggable = true;
      pc->realize = pci_default_realize;
  }
 

hw/s390x/ccw-device.c +1 -0

index f9bfa15..d1b6e6f 100644
@@ -48,6 +48,7 @@ static void ccw_device_class_init(ObjectClass *klass, void *data)
      k->realize = ccw_device_realize;
      k->refill_ids = ccw_device_refill_ids;
      dc->props = ccw_device_properties;
+    dc->hotpluggable = true;
  }
 
  const VMStateDescription vmstate_ccw_dev = {

hw/scsi/scsi-bus.c +1 -0

index e364410..338180d 100644
@@ -2123,6 +2123,7 @@ static void scsi_device_class_init(ObjectClass *klass, void *data)
      k->realize   = scsi_qdev_realize;
      k->unrealize = scsi_qdev_unrealize;
      k->props     = scsi_props;
+    k->hotpluggable = true;
  }
 
  static void scsi_dev_instance_init(Object *obj)

hw/usb/bus.c +1 -0

index d910f84..16701aa 100644
@@ -793,6 +793,7 @@ static void usb_device_class_init(ObjectClass *klass, void *data)
      k->realize  = usb_qdev_realize;
      k->unrealize = usb_qdev_unrealize;
      k->props    = usb_props;
+    k->hotpluggable = true;
  }
 
  static const TypeInfo usb_device_type_info = {

Hi Thomas,

Quoted 49 lines · Thomas Huth

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 <[email protected]>

---
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(-)

hw/core/qdev.c +2 -4

index 606ab53..c4f1902 100644
@@ -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

hw/core/qdev.c static void device_class_init(ObjectClass *class, void *data) · line 1127

Quoted 4 lines

hw/core/qdev.c +2 -1

+     * 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...

Quoted 53 lines · Marcel Apfelbaum

Hi Thomas,

Quoted 49 lines · Thomas Huth

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 <[email protected]>

---
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(-)

hw/core/qdev.c +2 -4

index 606ab53..c4f1902 100644
@@ -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

Quoted 8 lines

hw/core/qdev.c static void device_class_init(ObjectClass *class, void *data) · line 1127

Quoted 4 lines

hw/core/qdev.c +2 -1

+     * 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

Quoted 70 lines · Peter Maydell

Quoted 67 lines · Marcel Apfelbaum

Hi Thomas,

Quoted 54 lines · Thomas Huth

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 <[email protected]>

---
   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(-)

hw/core/qdev.c +2 -4

index 606ab53..c4f1902 100644
@@ -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

Quoted 5 lines

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...

Quoted 5 lines

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.

Eduardo came up with some cool automated tests not long ago. Eduardo, can your tests help?

Quoted 1 line · Marcel Apfelbaum

On 20/09/2017 13:07, Peter Maydell wrote:

Quoted 8 lines

Quoted 5 lines

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.

Eduardo came up with some cool automated tests not long ago. Eduardo, can your tests help?

You mean the scripts/device-crash-test script? That's only for cold-plugging ... but I could maybe use the device_add HMP test (see
https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg00817.html ) to exercise the hotplugging of all devices and add some debug code to qdev_device_add() to see which devices are dc->hotpluggable and have a hotplug handler at the same time... I'll give it a try.

Quoted 2 lines · Thomas Huth

Quoted 1 line · Marcel Apfelbaum

On 20/09/2017 13:07, Peter Maydell wrote:

[...]

Quoted 9 lines

Quoted 8 lines

Quoted 5 lines

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.

Eduardo came up with some cool automated tests not long ago. Eduardo, can your tests help?

Quoted 6 lines

You mean the scripts/device-crash-test script? That's only for cold-plugging ... but I could maybe use the device_add HMP test (see
https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg00817.html ) to exercise the hotplugging of all devices and add some debug code to qdev_device_add() to see which devices are dc->hotpluggable and have a hotplug handler at the same time... I'll give it a try.

Ok, done that now, and it looks like I missed devices of type TYPE_X86_CPU and TYPE_S390_CPU at least... Unlike the ppc64 CPUs, they are not derived from TYPE_CPU_CORE, but from TYPE_CPU ... is that on purpose? I thought hot-pluggable CPUs would need to be derived from TYPE_CPU_CORE... well, maybe I just understood that wrong. I'll send a new version of my patch.

Thomas

Quoted 2 lines · Thomas Huth

Quoted 1 line · Marcel Apfelbaum

On 20/09/2017 13:07, Peter Maydell wrote:

[...]

Quoted 9 lines

Quoted 8 lines

Quoted 5 lines

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.

Eduardo came up with some cool automated tests not long ago. Eduardo, can your tests help?

Quoted 6 lines

You mean the scripts/device-crash-test script? That's only for cold-plugging ... but I could maybe use the device_add HMP test (see
https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg00817.html ) to exercise the hotplugging of all devices and add some debug code to qdev_device_add() to see which devices are dc->hotpluggable and have a hotplug handler at the same time... I'll give it a try.

Ok, done that now, and it looks like I missed devices of type TYPE_X86_CPU and TYPE_S390_CPU at least... Unlike the ppc64 CPUs, they are not derived from TYPE_CPU_CORE, but from TYPE_CPU ... is that on purpose? I thought hot-pluggable CPUs would need to be derived from TYPE_CPU_CORE... well, maybe I just understood that wrong. I'll send a new version of my patch.

Thomas

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 <[email protected]>

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 <[email protected]>

---
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(-)

hw/core/qdev.c +4 -6

index 606ab53..c4f1902 100644
@@ -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;
     dc->user_creatable = true;
 }
 

hw/cpu/core.c +1 -0

index bd578ab..01660aa 100644
@@ -82,6 +82,7 @@ static void cpu_core_class_init(ObjectClass *oc, void *data)
     DeviceClass *dc = DEVICE_CLASS(oc);
 
     set_bit(DEVICE_CATEGORY_CPU, dc->categories);
+    dc->hotpluggable = true;
 }
 
 static const TypeInfo cpu_core_type_info = {

hw/mem/nvdimm.c +3 -0

index 952fce5..02be9f3 100644
@@ -148,9 +148,12 @@ static MemoryRegion *nvdimm_get_vmstate_memory_region(PCDIMMDevice *dimm)
 
 static void nvdimm_class_init(ObjectClass *oc, void *data)
 {
+    DeviceClass *dc = DEVICE_CLASS(oc);
     PCDIMMDeviceClass *ddc = PC_DIMM_CLASS(oc);
     NVDIMMClass *nvc = NVDIMM_CLASS(oc);
 
+    dc->hotpluggable = true;
+
     ddc->realize = nvdimm_realize;
     ddc->get_memory_region = nvdimm_get_memory_region;
     ddc->get_vmstate_memory_region = nvdimm_get_vmstate_memory_region;

hw/mem/pc-dimm.c +1 -0

index 66eace5..1f78567 100644
@@ -443,6 +443,7 @@ static void pc_dimm_class_init(ObjectClass *oc, void *data)
     dc->unrealize = pc_dimm_unrealize;
     dc->props = pc_dimm_properties;
     dc->desc = "DIMM memory module";
+    dc->hotpluggable = true;
 
     ddc->get_memory_region = pc_dimm_get_memory_region;
     ddc->get_vmstate_memory_region = pc_dimm_get_vmstate_memory_region;

hw/pci/pci.c +1 -0

index 1e6fb88..8db380d 100644
@@ -2534,6 +2534,7 @@ static void pci_device_class_init(ObjectClass *klass, void *data)
     k->unrealize = pci_qdev_unrealize;
     k->bus_type = TYPE_PCI_BUS;
     k->props = pci_props;
+    k->hotpluggable = true;
     pc->realize = pci_default_realize;
 }
 

hw/s390x/ccw-device.c +1 -0

index f9bfa15..d1b6e6f 100644
@@ -48,6 +48,7 @@ static void ccw_device_class_init(ObjectClass *klass, void *data)
     k->realize = ccw_device_realize;
     k->refill_ids = ccw_device_refill_ids;
     dc->props = ccw_device_properties;
+    dc->hotpluggable = true;
 }
 
 const VMStateDescription vmstate_ccw_dev = {

hw/scsi/scsi-bus.c +1 -0

index e364410..338180d 100644
@@ -2123,6 +2123,7 @@ static void scsi_device_class_init(ObjectClass *klass, void *data)
     k->realize   = scsi_qdev_realize;
     k->unrealize = scsi_qdev_unrealize;
     k->props     = scsi_props;
+    k->hotpluggable = true;
 }
 
 static void scsi_dev_instance_init(Object *obj)

hw/usb/bus.c +1 -0

index d910f84..16701aa 100644
@@ -793,6 +793,7 @@ static void usb_device_class_init(ObjectClass *klass, void *data)
     k->realize  = usb_qdev_realize;
     k->unrealize = usb_qdev_unrealize;
     k->props    = usb_props;
+    k->hotpluggable = true;
 }
 
 static const TypeInfo usb_device_type_info = {

Signature · 1 line

1.8.3.1

Quoted 11 lines · Thomas Huth

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.

These seem to be missing:
* TYPE_CPU (or at least TYPE_X86_CPU and TYPE_S390_CPU)

* TYPE_VIRTIO_SERIAL_PORT
* TYPE_CCID_CARD
* TYPE_XENSYSDEV

Also, I don't think we need to set it for TYPE_CPU_CORE, just for TYPE_SPAPR_CPU_CORE.

hw/s390x/s390-virtio-ccw.c:446: mc->get_hotplug_handler = s390_get_hotplug_handler;
s390_get_hotplug_handler accepts:
* TYPE_CPU

hw/s390x/css-bridge.c=94=VirtualCssBus *virtual_css_bus_init(void)
hw/s390x/css-bridge.c:110: qbus_set_hotplug_handler(bus, dev, &error_abort);
* This seems to be for TYPE_CCW_DEVICE

hw/s390x/s390-pci-bus.c=548=static int s390_pcihost_init(SysBusDevice *dev)
hw/s390x/s390-pci-bus.c:564: qbus_set_hotplug_handler(bus, DEVICE(dev), NULL);
hw/s390x/s390-pci-bus.c:568: qbus_set_hotplug_handler(BUS(s->bus), DEVICE(s), NULL);
hw/s390x/s390-pci-bus.c=660=static void s390_pcihost_hot_plug(HotplugHandler *hotplug_dev,
hw/s390x/s390-pci-bus.c:676: qbus_set_hotplug_handler(bus, DEVICE(s), errp);
* TYPE_PCI only, it seems.

Quoted 13 lines · On Thu, 21 Sep 2017 15:50:28 -0300 Eduardo Habkost <[email protected]> wrote:

Quoted 11 lines · Thomas Huth

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.

Quoted 2 lines

These seem to be missing:
* TYPE_CPU (or at least TYPE_X86_CPU and TYPE_S390_CPU)

I think it would be better to set it for TYPE_CPU (and have architectures override if needed).

Quoted 11 lines · Thomas Huth

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.

These seem to be missing:
* TYPE_CPU (or at least TYPE_X86_CPU and TYPE_S390_CPU)
* TYPE_VIRTIO_SERIAL_PORT
* TYPE_CCID_CARD
* TYPE_XENSYSDEV

Also, I don't think we need to set it for TYPE_CPU_CORE, just for TYPE_SPAPR_CPU_CORE.

Quoted 13 lines · Eduardo Habkost

Quoted 11 lines · Thomas Huth

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.

Quoted 5 lines

These seem to be missing:
* TYPE_CPU (or at least TYPE_X86_CPU and TYPE_S390_CPU)
* TYPE_VIRTIO_SERIAL_PORT
* TYPE_CCID_CARD
* TYPE_XENSYSDEV

Thanks for the detailed examination, Eduardo! I'll rework my patch accordingly...

Quoted 2 lines

Also, I don't think we need to set it for TYPE_CPU_CORE, just for TYPE_SPAPR_CPU_CORE.

Ok - you're likely right. There is one other consumer of TYPE_CPU_CORE beside spapr, which is the pnv machine, and as far as I can see, it does not support CPU hotplugging yet. So it indeed makes more sense to set this in TYPE_SPAPR_CPU_CORE only.