Re: [Qemu-devel] [RFC PATCH] qdev: Mark devices as non-hotpluggable by default

[Cornelia Huck]

On Thu, 21 Sep 2017 15:50:28 -0300
Eduardo Habkost <address@hidden> wrote:

> On Tue, Sep 19, 2017 at 10:55:53AM +0200, 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.  
> 
> 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).

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

Nod.

> 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

Yes, it is.

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

Yes.