Re: [Qemu-devel] [PATCH v2] qdev: Mark devices as non-hotpluggable by default
On 26 September 2017 at 18:27, Thomas Huthwrote: > On 25.09.2017 19:59, Eduardo Habkost wrote: >> On Mon, Sep 25, 2017 at 07:42:13PM +0200, Thomas Huth wrote: >> "make check" found a few candidates: >> https://travis-ci.org/ehabkost/qemu/jobs/278743999 >> >> Initialization of device dpcd failed: Device 'dpcd' does not support >> hotplugging >> [...] >> Initialization of device nand failed: Device 'nand' does not support >> hotplugging > > I've now had a look at those, and I now feel like the whole > "hotpluggable = false by default" idea was either just wrong or there > are other smart ideas necessary to solve this issue: These devices are > created during the instance_init() function of another device, e.g. > "dpcd" is created in the xlnx_dp_init() function, which is the > instance_init of TYPE_XLNX_DP. Now, instance_init() can be called at any > time during runtime, even without really adding the device, e.g. for > parameter introspection (try "device_add xlnx.v-dp,help" at the HMP > prompt for example). This is a bit weird though, because it means we have a lot of devices which are "it's OK to instance_init this but not to actually create (realize) it". I don't think that that should count as hotpluggable. (Is it actually useful to be able to call "help" for a device that it is not possible to create?) > But just setting "hotpluggable = true" for a device (e.g. "dpcd") which > could be created during instance_init also does not sound very > attractive to me... Not sure about any good alternative way to tackle > this right now, maybe I've eventually got to check user_creatable in > device_set_realized, too, or move the hotpluggable checks to > qmp_device_add() instead... I'll think about that for a while... > suggestions welcome! I think the general principle of defaulting to "you need to specifically mark the device if you want it to be permitted to hotplug it" seems right -- even devices which are created under the hood by the real hotpluggable device still need to be written to support it (ie by having a finalize method or whatever to clean up). So they should probably be marked as "hotpluggable but not user createable" or similar. We just need to get the details right. thanks -- PMM
Re: [Qemu-devel] [PATCH v2] qdev: Mark devices as non-hotpluggable by default
On 25.09.2017 19:59, Eduardo Habkost wrote: > On Mon, Sep 25, 2017 at 07:42:13PM +0200, Thomas Huth wrote: >> On 25.09.2017 17:26, Peter Maydell wrote: >>> On 25 September 2017 at 16:19, Thomas Huthwrote: Not sure whether this works for the virtio-xxx-device devices, though, since they are marked as user_creatable = true currently... >>> >>> That's deliberate -- for the arm boards with virtio-mmio >>> the board creates a bunch of virtio-mmio transports and the >>> virtio-foo-device can be user created to plug into those. >> >> Yes, I know ... I'm just wondering whether the virtio-xxx-device devices >> should be non-user_creatable on the non-ARM targets, since they >> apparently can't be used with "-device" there...? > > I wouldn't like to make DeviceClass fields depend on the target. > Being user-creatable doesn't mean they will work on all machines, > anyway. If user/management need more specific info, they need > something like the query-device-slots command I've proposed some > time ago. > >> >>> If overall trying to do the 'right thing' is tricky here >>> then perhaps we can start by flipping the default to >>> not-hotpluggable and marking some devices hotpluggable >>> that in theory shouldn't be with a comment about why. >> >> Yes, if Eduardo's idea to move the test to qmp_device_add() does not >> work out (I'll try that next), your suggestion is certainly the best >> thing we can do right now. > > I think it would work, but we would lose the feature Peter > mentions below: > >> >>> Incidentally I think there's still some advantage in the >>> "created as part of some other device" devices having >>> to be explicitly marked hotpluggable, because those >>> devices do still need some specific code in them to >>> handle it (ie code to release the resources that are >>> created in the device's realize method). >> >> Ok ... by the way, does anybody know more devices like >> virtio-xxx-device, i.e. devices that are indirectly plugged when adding >> other devices? > > "make check" found a few candidates: > https://travis-ci.org/ehabkost/qemu/jobs/278743999 > > Initialization of device dpcd failed: Device 'dpcd' does not support > hotplugging > [...] > Initialization of device nand failed: Device 'nand' does not support > hotplugging I've now had a look at those, and I now feel like the whole "hotpluggable = false by default" idea was either just wrong or there are other smart ideas necessary to solve this issue: These devices are created during the instance_init() function of another device, e.g. "dpcd" is created in the xlnx_dp_init() function, which is the instance_init of TYPE_XLNX_DP. Now, instance_init() can be called at any time during runtime, even without really adding the device, e.g. for parameter introspection (try "device_add xlnx.v-dp,help" at the HMP prompt for example). But just setting "hotpluggable = true" for a device (e.g. "dpcd") which could be created during instance_init also does not sound very attractive to me... Not sure about any good alternative way to tackle this right now, maybe I've eventually got to check user_creatable in device_set_realized, too, or move the hotpluggable checks to qmp_device_add() instead... I'll think about that for a while... suggestions welcome! Thomas
Re: [Qemu-devel] [PATCH v2] qdev: Mark devices as non-hotpluggable by default
On 26 September 2017 at 03:59, Eduardo Habkostwrote: > On Mon, Sep 25, 2017 at 07:05:26PM +0100, Peter Maydell wrote: >> On 25 September 2017 at 18:59, Eduardo Habkost wrote: >> If we just diff "list of devices marked hotplug before this patch" >> against "list of devices marked hotplug after this patch" how >> big is the list? Can we just eyeball it to see what needs >> to be specialcased? > > So, the full list quite big, ~1800 device types are affected by > this patch: Thanks for producing the list. These arm ones definitely shouldn't be hotpluggable, certainly: aspeed-soc, allwinner-a10, fsl,imx21, fsl,imx6, fsl,imx25, fsl,imx31, xlnx,zynqmp, digic thanks -- PMM
Re: [Qemu-devel] [PATCH v2] qdev: Mark devices as non-hotpluggable by default
On 26.09.2017 07:26, Bharata B Rao wrote: > On Fri, Sep 22, 2017 at 11:16:34AM +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. > > I see that the discussion has moved on, but want to note here that > CPU hotplug on pseries breaks with this patch. > > (qemu) device_add host-spapr-cpu-core,core-id=8,id=core8 > Device 'host-powerpc64-cpu' does not support hotplugging > > (qemu) device_add POWER8E_v2.1-spapr-cpu-core,id=core8,core-id=8 > Device 'POWER8E_v2.1-powerpc64-cpu' does not support hotplugging Sorry, my bad (again) - I missed that the spapr-cpu-core tries to internally create some more devices, i.e. the powerpc64-cpu device and a icp device. So they need to be marked as hotpluggable, too. I'll fix this in my next version of the patch... Anyway, thanks a lot for testing! (and memo ty myself: Do more testing on your own before sending patches like this...) > Hope I am not missing anything. You just missed to add a proper qtest for "make check" for hot-pluggable CPUs (hint, hint!) ;-) Apart from that, this failure was completely my fault... Cheers, Thomas
Re: [Qemu-devel] [PATCH v2] qdev: Mark devices as non-hotpluggable by default
On Fri, Sep 22, 2017 at 11:16:34AM +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. I see that the discussion has moved on, but want to note here that CPU hotplug on pseries breaks with this patch. (qemu) device_add host-spapr-cpu-core,core-id=8,id=core8 Device 'host-powerpc64-cpu' does not support hotplugging (qemu) device_add POWER8E_v2.1-spapr-cpu-core,id=core8,core-id=8 Device 'POWER8E_v2.1-powerpc64-cpu' does not support hotplugging Hope I am not missing anything. Regards, Bharata.
Re: [Qemu-devel] [PATCH v2] qdev: Mark devices as non-hotpluggable by default
On 26.09.2017 04:59, Eduardo Habkost wrote: > On Mon, Sep 25, 2017 at 07:05:26PM +0100, Peter Maydell wrote: >> On 25 September 2017 at 18:59, Eduardo Habkostwrote: >>> Finding the full list of devices that can be instantiated >>> internally at hotplug-time sounds tricky. >> >> If we just diff "list of devices marked hotplug before this patch" >> against "list of devices marked hotplug after this patch" how >> big is the list? Can we just eyeball it to see what needs >> to be specialcased? > > So, the full list quite big, ~1800 device types are affected by > this patch: > > https://gist.github.com/ehabkost/bd8e25c6811ac81d947ad8ad5b557f5c#file-dev-types-diff-json > > If we ignore the "-cpu" classes, there ~640 affected device > types. > > However, if we look only at the direct children of TYPE_DEVICE, > we have: OK, thanks a lot for that list! But do you think that we can assume that the devices which are not direct children of TYPE_DEVICE can not be hotplugged internally during runtime? ... I currently don't think so (but at least they are good candidates which need to be examined more carefully). But anyway, how can we continue here? Set hotpluggable = true on all 640 (or even all 1800) affected devices? That sounds very, very ugly to me. Maybe we should just do it for the virtio-*-device devices and the others that break during "make check" (btw. sorry for not noticing this ... I normally run "make check" regularly, but this time I apparently missed to run it for aarch64), and if we later notice some more problems, we know that we lack a "make check" test for that case, too, and we should add such a test? That would at least eventually improve our test coverage a little bit, I guess... Thomas
Re: [Qemu-devel] [PATCH v2] qdev: Mark devices as non-hotpluggable by default
On Mon, Sep 25, 2017 at 07:05:26PM +0100, Peter Maydell wrote: > On 25 September 2017 at 18:59, Eduardo Habkostwrote: > > Finding the full list of devices that can be instantiated > > internally at hotplug-time sounds tricky. > > If we just diff "list of devices marked hotplug before this patch" > against "list of devices marked hotplug after this patch" how > big is the list? Can we just eyeball it to see what needs > to be specialcased? So, the full list quite big, ~1800 device types are affected by this patch: https://gist.github.com/ehabkost/bd8e25c6811ac81d947ad8ad5b557f5c#file-dev-types-diff-json If we ignore the "-cpu" classes, there ~640 affected device types. However, if we look only at the direct children of TYPE_DEVICE, we have: $ grep '"parent": "device"' dev-types-diff.json -{"return": {"abstract": true, "name": "adb-device", "parent": "device", "user-creatable": true, "hotpluggable": true}} -{"return": {"abstract": true, "name": "apic-common", "parent": "device", "user-creatable": false, "hotpluggable": true}} -{"return": {"abstract": true, "name": "aspeed-soc", "parent": "device", "user-creatable": true, "hotpluggable": true}} -{"return": {"abstract": true, "name": "aux-slave", "parent": "device", "user-creatable": true, "hotpluggable": true}} +{"return": {"abstract": true, "name": "adb-device", "parent": "device", "user-creatable": true, "hotpluggable": false}} +{"return": {"abstract": true, "name": "apic-common", "parent": "device", "user-creatable": false, "hotpluggable": false}} +{"return": {"abstract": true, "name": "aspeed-soc", "parent": "device", "user-creatable": true, "hotpluggable": false}} +{"return": {"abstract": true, "name": "aux-slave", "parent": "device", "user-creatable": true, "hotpluggable": false}} {"return": {"abstract": true, "name": "ccid-card", "parent": "device", "user-creatable": true, "hotpluggable": true}} {"return": {"abstract": true, "name": "ccw-device", "parent": "device", "user-creatable": true, "hotpluggable": true}} -{"return": {"abstract": true, "name": "cpu-core", "parent": "device", "user-creatable": true, "hotpluggable": true}} -{"return": {"abstract": true, "name": "cpu", "parent": "device", "user-creatable": false, "hotpluggable": true}} +{"return": {"abstract": true, "name": "cpu-core", "parent": "device", "user-creatable": true, "hotpluggable": false}} +{"return": {"abstract": true, "name": "cpu", "parent": "device", "user-creatable": false, "hotpluggable": false}} -{"return": {"abstract": true, "name": "hda-codec", "parent": "device", "user-creatable": true, "hotpluggable": true}} -{"return": {"abstract": true, "name": "i2c-slave", "parent": "device", "user-creatable": true, "hotpluggable": true}} +{"return": {"abstract": true, "name": "hda-codec", "parent": "device", "user-creatable": true, "hotpluggable": false}} +{"return": {"abstract": true, "name": "i2c-slave", "parent": "device", "user-creatable": true, "hotpluggable": false}} -{"return": {"abstract": true, "name": "ics-base", "parent": "device", "user-creatable": true, "hotpluggable": true}} -{"return": {"abstract": true, "name": "ide-device", "parent": "device", "user-creatable": true, "hotpluggable": true}} +{"return": {"abstract": true, "name": "ics-base", "parent": "device", "user-creatable": true, "hotpluggable": false}} +{"return": {"abstract": true, "name": "ide-device", "parent": "device", "user-creatable": true, "hotpluggable": false}} -{"return": {"abstract": true, "name": "ipack-device", "parent": "device", "user-creatable": true, "hotpluggable": true}} -{"return": {"abstract": true, "name": "ipmi-bmc", "parent": "device", "user-creatable": true, "hotpluggable": true}} -{"return": {"abstract": true, "name": "isa-device", "parent": "device", "user-creatable": true, "hotpluggable": true}} +{"return": {"abstract": true, "name": "ipack-device", "parent": "device", "user-creatable": true, "hotpluggable": false}} +{"return": {"abstract": true, "name": "ipmi-bmc", "parent": "device", "user-creatable": true, "hotpluggable": false}} +{"return": {"abstract": true, "name": "isa-device", "parent": "device", "user-creatable": true, "hotpluggable": false}} {"return": {"abstract": true, "name": "pci-device", "parent": "device", "user-creatable": true, "hotpluggable": true}} -{"return": {"abstract": true, "name": "pcmcia-card", "parent": "device", "user-creatable": true, "hotpluggable": true}} +{"return": {"abstract": true, "name": "pcmcia-card", "parent": "device", "user-creatable": true, "hotpluggable": false}} -{"return": {"abstract": true, "name": "s390-sclp-event-type", "parent": "device", "user-creatable": true, "hotpluggable": true}} +{"return": {"abstract": true, "name": "s390-sclp-event-type", "parent": "device", "user-creatable": true, "hotpluggable": false}} {"return": {"abstract": true, "name": "s390-skeys", "parent": "device", "user-creatable": true, "hotpluggable": false}} {"return": {"abstract": true, "name":
Re: [Qemu-devel] [PATCH v2] qdev: Mark devices as non-hotpluggable by default
On 25 September 2017 at 19:20, Eduardo Habkostwrote: > On Mon, Sep 25, 2017 at 07:02:06PM +0100, Peter Maydell wrote: >> On 25 September 2017 at 18:51, Eduardo Habkost wrote: >> > On Mon, Sep 25, 2017 at 06:45:15PM +0100, Peter Maydell wrote: >> >> You should be able to on the command line for x86 do something >> >> like -device virtio-pci,... -device virtio-foo-device,... >> >> to manually create the pci transport and the backend. >> > >> > virtio-pci is abstract, so this is not possible. (The same >> > applies to virtio-ccw-device). >> >> Did I use the wrong device name? I meant the transport >> layer device (which virtio-pci-blk creates along with >> virtio-blk-device internally), not the abstract device >> that's a base class for the pci devices. > > AFAIK, virtio-pci/virtio-pci-blk itself is the transport layer > device. Internally, it creates two objects: a virtio-pci-bus > (which is a 1-device bus, not creatable using -device), and a > virtio-blk-device attached to that bus. Hmm, I thought the way we structured this was that virtio-pci-blk is the convenience wrapper device, which creates both the endpoint device (virtio-blk-device) and the transport device (which is the thing that has a PCI bus interface on one end and a virtio bus on the other). But maybe we didn't restructure the pci virtio devices to do that... thanks -- PMM
Re: [Qemu-devel] [PATCH v2] qdev: Mark devices as non-hotpluggable by default
On Mon, Sep 25, 2017 at 07:02:06PM +0100, Peter Maydell wrote: > On 25 September 2017 at 18:51, Eduardo Habkostwrote: > > On Mon, Sep 25, 2017 at 06:45:15PM +0100, Peter Maydell wrote: > >> You should be able to on the command line for x86 do something > >> like -device virtio-pci,... -device virtio-foo-device,... > >> to manually create the pci transport and the backend. > > > > virtio-pci is abstract, so this is not possible. (The same > > applies to virtio-ccw-device). > > Did I use the wrong device name? I meant the transport > layer device (which virtio-pci-blk creates along with > virtio-blk-device internally), not the abstract device > that's a base class for the pci devices. AFAIK, virtio-pci/virtio-pci-blk itself is the transport layer device. Internally, it creates two objects: a virtio-pci-bus (which is a 1-device bus, not creatable using -device), and a virtio-blk-device attached to that bus. -- Eduardo
Re: [Qemu-devel] [PATCH v2] qdev: Mark devices as non-hotpluggable by default
On Mon, Sep 25, 2017 at 07:05:26PM +0100, Peter Maydell wrote: > On 25 September 2017 at 18:59, Eduardo Habkostwrote: > > Finding the full list of devices that can be instantiated > > internally at hotplug-time sounds tricky. > > If we just diff "list of devices marked hotplug before this patch" > against "list of devices marked hotplug after this patch" how > big is the list? Can we just eyeball it to see what needs > to be specialcased? I was working on a query-device-type QMP command some time ago. I will rebase the work in progress and use it to build those lists. -- Eduardo
Re: [Qemu-devel] [PATCH v2] qdev: Mark devices as non-hotpluggable by default
On 25 September 2017 at 18:59, Eduardo Habkostwrote: > Finding the full list of devices that can be instantiated > internally at hotplug-time sounds tricky. If we just diff "list of devices marked hotplug before this patch" against "list of devices marked hotplug after this patch" how big is the list? Can we just eyeball it to see what needs to be specialcased? thanks -- PMM
Re: [Qemu-devel] [PATCH v2] qdev: Mark devices as non-hotpluggable by default
On 25 September 2017 at 18:51, Eduardo Habkostwrote: > On Mon, Sep 25, 2017 at 06:45:15PM +0100, Peter Maydell wrote: >> You should be able to on the command line for x86 do something >> like -device virtio-pci,... -device virtio-foo-device,... >> to manually create the pci transport and the backend. > > virtio-pci is abstract, so this is not possible. (The same > applies to virtio-ccw-device). Did I use the wrong device name? I meant the transport layer device (which virtio-pci-blk creates along with virtio-blk-device internally), not the abstract device that's a base class for the pci devices. thanks -- PMM
Re: [Qemu-devel] [PATCH v2] qdev: Mark devices as non-hotpluggable by default
On Mon, Sep 25, 2017 at 07:42:13PM +0200, Thomas Huth wrote: > On 25.09.2017 17:26, Peter Maydell wrote: > > On 25 September 2017 at 16:19, Thomas Huthwrote: > >> Not sure whether this works for the virtio-xxx-device devices, > >> though, since they are marked as user_creatable = true currently... > > > > That's deliberate -- for the arm boards with virtio-mmio > > the board creates a bunch of virtio-mmio transports and the > > virtio-foo-device can be user created to plug into those. > > Yes, I know ... I'm just wondering whether the virtio-xxx-device devices > should be non-user_creatable on the non-ARM targets, since they > apparently can't be used with "-device" there...? I wouldn't like to make DeviceClass fields depend on the target. Being user-creatable doesn't mean they will work on all machines, anyway. If user/management need more specific info, they need something like the query-device-slots command I've proposed some time ago. > > > If overall trying to do the 'right thing' is tricky here > > then perhaps we can start by flipping the default to > > not-hotpluggable and marking some devices hotpluggable > > that in theory shouldn't be with a comment about why. > > Yes, if Eduardo's idea to move the test to qmp_device_add() does not > work out (I'll try that next), your suggestion is certainly the best > thing we can do right now. I think it would work, but we would lose the feature Peter mentions below: > > > Incidentally I think there's still some advantage in the > > "created as part of some other device" devices having > > to be explicitly marked hotpluggable, because those > > devices do still need some specific code in them to > > handle it (ie code to release the resources that are > > created in the device's realize method). > > Ok ... by the way, does anybody know more devices like > virtio-xxx-device, i.e. devices that are indirectly plugged when adding > other devices? "make check" found a few candidates: https://travis-ci.org/ehabkost/qemu/jobs/278743999 Initialization of device dpcd failed: Device 'dpcd' does not support hotplugging [...] Initialization of device nand failed: Device 'nand' does not support hotplugging Finding the full list of devices that can be instantiated internally at hotplug-time sounds tricky. -- Eduardo
Re: [Qemu-devel] [PATCH v2] qdev: Mark devices as non-hotpluggable by default
On Mon, Sep 25, 2017 at 06:45:15PM +0100, Peter Maydell wrote: > On 25 September 2017 at 18:42, Thomas Huthwrote: > > On 25.09.2017 17:26, Peter Maydell wrote: > >> On 25 September 2017 at 16:19, Thomas Huth wrote: > >>> Not sure whether this works for the virtio-xxx-device devices, > >>> though, since they are marked as user_creatable = true currently... > >> > >> That's deliberate -- for the arm boards with virtio-mmio > >> the board creates a bunch of virtio-mmio transports and the > >> virtio-foo-device can be user created to plug into those. > > > > Yes, I know ... I'm just wondering whether the virtio-xxx-device devices > > should be non-user_creatable on the non-ARM targets, since they > > apparently can't be used with "-device" there...? > > You should be able to on the command line for x86 do something > like -device virtio-pci,... -device virtio-foo-device,... > to manually create the pci transport and the backend. virtio-pci is abstract, so this is not possible. (The same applies to virtio-ccw-device). -- Eduardo
Re: [Qemu-devel] [PATCH v2] qdev: Mark devices as non-hotpluggable by default
On Mon, Sep 25, 2017 at 04:26:44PM +0100, Peter Maydell wrote: > On 25 September 2017 at 16:19, Thomas Huthwrote: > > Not sure whether this works for the virtio-xxx-device devices, > > though, since they are marked as user_creatable = true currently... > > That's deliberate -- for the arm boards with virtio-mmio > the board creates a bunch of virtio-mmio transports and the > virtio-foo-device can be user created to plug into those. Do they support device_add virtio-xxx-device, though? If they don't, should we report it as hotpluggable on a future QMP query-device-type command? > > If overall trying to do the 'right thing' is tricky here > then perhaps we can start by flipping the default to > not-hotpluggable and marking some devices hotpluggable > that in theory shouldn't be with a comment about why. Finding the full list of those devices is the tricky part. > > Incidentally I think there's still some advantage in the > "created as part of some other device" devices having > to be explicitly marked hotpluggable, because those > devices do still need some specific code in them to > handle it (ie code to release the resources that are > created in the device's realize method). I agree this is still useful. I was trying represent something different: I was looking for a flag meaning "supports device_add", while the current meaning is "supports late instantiation". On all devices, device_add support requires late instantiation. On most user-creatable devices, late instantiation support implies device_add support (virtio-*-device is the only exception). This confusion can be solved by documenting DeviceClass::hotpluggable as "may support device_add, as long as the machine and bus lets you do it", so there's no harm in setting it to true on virtio-*-device. This won't allow us to automatically tell if a device can be safely instantiated without a hotplug controller, though. -- Eduardo
Re: [Qemu-devel] [PATCH v2] qdev: Mark devices as non-hotpluggable by default
On 25 September 2017 at 18:42, Thomas Huthwrote: > On 25.09.2017 17:26, Peter Maydell wrote: >> On 25 September 2017 at 16:19, Thomas Huth wrote: >>> Not sure whether this works for the virtio-xxx-device devices, >>> though, since they are marked as user_creatable = true currently... >> >> That's deliberate -- for the arm boards with virtio-mmio >> the board creates a bunch of virtio-mmio transports and the >> virtio-foo-device can be user created to plug into those. > > Yes, I know ... I'm just wondering whether the virtio-xxx-device devices > should be non-user_creatable on the non-ARM targets, since they > apparently can't be used with "-device" there...? You should be able to on the command line for x86 do something like -device virtio-pci,... -device virtio-foo-device,... to manually create the pci transport and the backend. thanks -- PMM
Re: [Qemu-devel] [PATCH v2] qdev: Mark devices as non-hotpluggable by default
On 25.09.2017 17:26, Peter Maydell wrote: > On 25 September 2017 at 16:19, Thomas Huthwrote: >> Not sure whether this works for the virtio-xxx-device devices, >> though, since they are marked as user_creatable = true currently... > > That's deliberate -- for the arm boards with virtio-mmio > the board creates a bunch of virtio-mmio transports and the > virtio-foo-device can be user created to plug into those. Yes, I know ... I'm just wondering whether the virtio-xxx-device devices should be non-user_creatable on the non-ARM targets, since they apparently can't be used with "-device" there...? > If overall trying to do the 'right thing' is tricky here > then perhaps we can start by flipping the default to > not-hotpluggable and marking some devices hotpluggable > that in theory shouldn't be with a comment about why. Yes, if Eduardo's idea to move the test to qmp_device_add() does not work out (I'll try that next), your suggestion is certainly the best thing we can do right now. > Incidentally I think there's still some advantage in the > "created as part of some other device" devices having > to be explicitly marked hotpluggable, because those > devices do still need some specific code in them to > handle it (ie code to release the resources that are > created in the device's realize method). Ok ... by the way, does anybody know more devices like virtio-xxx-device, i.e. devices that are indirectly plugged when adding other devices? Thomas
Re: [Qemu-devel] [PATCH v2] qdev: Mark devices as non-hotpluggable by default
On 25 September 2017 at 16:19, Thomas Huthwrote: > Not sure whether this works for the virtio-xxx-device devices, > though, since they are marked as user_creatable = true currently... That's deliberate -- for the arm boards with virtio-mmio the board creates a bunch of virtio-mmio transports and the virtio-foo-device can be user created to plug into those. If overall trying to do the 'right thing' is tricky here then perhaps we can start by flipping the default to not-hotpluggable and marking some devices hotpluggable that in theory shouldn't be with a comment about why. Incidentally I think there's still some advantage in the "created as part of some other device" devices having to be explicitly marked hotpluggable, because those devices do still need some specific code in them to handle it (ie code to release the resources that are created in the device's realize method). thanks -- PMM
Re: [Qemu-devel] [PATCH v2] qdev: Mark devices as non-hotpluggable by default
On 25.09.2017 16:34, Eduardo Habkost wrote: > On Mon, Sep 25, 2017 at 03:46:47PM +0200, Igor Mammedov wrote: >> On Mon, 25 Sep 2017 10:31:53 -0300 >> Eduardo Habkostwrote: >> >>> On Mon, Sep 25, 2017 at 01:53:16PM +0200, Cornelia Huck wrote: On Fri, 22 Sep 2017 11:16:34 +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. > > Signed-off-by: Thomas Huth > --- > v2: Add missing devices and dropped "RFC" status. See Eduardo's reply on > the previous version of this patch for the rationale which devices need > to be hotpluggable: > https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg06128.html > > hw/char/virtio-serial-bus.c | 1 + > hw/core/qdev.c| 10 -- > hw/mem/nvdimm.c | 3 +++ > hw/mem/pc-dimm.c | 1 + > hw/pci/pci.c | 1 + > hw/ppc/spapr_cpu_core.c | 1 + > hw/s390x/ccw-device.c | 1 + > hw/scsi/scsi-bus.c| 1 + > hw/usb/bus.c | 1 + > hw/usb/dev-smartcard-reader.c | 1 + > hw/xen/xen_backend.c | 1 + > target/i386/cpu.c | 4 ++-- > target/s390x/cpu.c| 1 + > 13 files changed, 19 insertions(+), 8 deletions(-) Hm, this seems to break hotplug of virtio devices: (qemu) device_add virtio-net-pci,id=xxx Device 'virtio-net-device' does not support hotplugging (qemu) device_add virtio-net-ccw,id=yyy Device 'virtio-net-device' does not support hotplugging We probably need to enable hotplug for virtio devices as well? D'oh, I only checked "normal" PCI devices, not the ones like virtio devices that consist of two devices internally :-/ >>> I've seen a similar case broken by "[PATCH] hw/core/qdev: Do not >>> allow hot-plugging without hotplug controller", because we're >>> blocking creation of devices created internally by other devices, >>> not just the ones created by device_add. I need to find the >>> exact reproducer again. >>> >>> It would probably simplify things if we move the check for >>> DeviceClass::hotpluggable to qmp_device_add(), instead of >>> device_set_realized(). >> if we would have to do that, than we are probably doing >> something wrong and not using property right. Perhaps we >> should fix property instead of trying find a place to check >> it where it would hurt less. > > I disagree. It depends on the purpose and semantics we define > for DeviceClass::hotpluggable. My goal is to have a reliable way > to tell the user if a device really supports device_add, and I > think it wouldn't make sense to report hotpluggable=true on a > device that is never instantiated directly by the user and only > used internally. I think an alternative was would maybe be to check for "user_creatable" in device_set_realized() in addition to "hotpluggable": If user_creatable is false, we can be sure that it is an internal plugging only. Not sure whether this works for the virtio-xxx-device devices, though, since they are marked as user_creatable = true currently... Thomas
Re: [Qemu-devel] [PATCH v2] qdev: Mark devices as non-hotpluggable by default
On Mon, Sep 25, 2017 at 03:46:47PM +0200, Igor Mammedov wrote: > On Mon, 25 Sep 2017 10:31:53 -0300 > Eduardo Habkostwrote: > > > On Mon, Sep 25, 2017 at 01:53:16PM +0200, Cornelia Huck wrote: > > > On Fri, 22 Sep 2017 11:16:34 +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. > > > > > > > > Signed-off-by: Thomas Huth > > > > --- > > > > v2: Add missing devices and dropped "RFC" status. See Eduardo's reply > > > > on > > > > the previous version of this patch for the rationale which devices need > > > > to be hotpluggable: > > > > https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg06128.html > > > > > > > > hw/char/virtio-serial-bus.c | 1 + > > > > hw/core/qdev.c| 10 -- > > > > hw/mem/nvdimm.c | 3 +++ > > > > hw/mem/pc-dimm.c | 1 + > > > > hw/pci/pci.c | 1 + > > > > hw/ppc/spapr_cpu_core.c | 1 + > > > > hw/s390x/ccw-device.c | 1 + > > > > hw/scsi/scsi-bus.c| 1 + > > > > hw/usb/bus.c | 1 + > > > > hw/usb/dev-smartcard-reader.c | 1 + > > > > hw/xen/xen_backend.c | 1 + > > > > target/i386/cpu.c | 4 ++-- > > > > target/s390x/cpu.c| 1 + > > > > 13 files changed, 19 insertions(+), 8 deletions(-) > > > > > > Hm, this seems to break hotplug of virtio devices: > > > > > > (qemu) device_add virtio-net-pci,id=xxx > > > Device 'virtio-net-device' does not support hotplugging > > > > > > (qemu) device_add virtio-net-ccw,id=yyy > > > Device 'virtio-net-device' does not support hotplugging > > > > > > We probably need to enable hotplug for virtio devices as well? > > > > I've seen a similar case broken by "[PATCH] hw/core/qdev: Do not > > allow hot-plugging without hotplug controller", because we're > > blocking creation of devices created internally by other devices, > > not just the ones created by device_add. I need to find the > > exact reproducer again. > > > > It would probably simplify things if we move the check for > > DeviceClass::hotpluggable to qmp_device_add(), instead of > > device_set_realized(). > if we would have to do that, than we are probably doing > something wrong and not using property right. Perhaps we > should fix property instead of trying find a place to check > it where it would hurt less. I disagree. It depends on the purpose and semantics we define for DeviceClass::hotpluggable. My goal is to have a reliable way to tell the user if a device really supports device_add, and I think it wouldn't make sense to report hotpluggable=true on a device that is never instantiated directly by the user and only used internally. -- Eduardo
Re: [Qemu-devel] [PATCH v2] qdev: Mark devices as non-hotpluggable by default
On Mon, 25 Sep 2017 10:31:53 -0300 Eduardo Habkostwrote: > On Mon, Sep 25, 2017 at 01:53:16PM +0200, Cornelia Huck wrote: > > On Fri, 22 Sep 2017 11:16:34 +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. > > > > > > Signed-off-by: Thomas Huth > > > --- > > > v2: Add missing devices and dropped "RFC" status. See Eduardo's reply on > > > the previous version of this patch for the rationale which devices need > > > to be hotpluggable: > > > https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg06128.html > > > > > > hw/char/virtio-serial-bus.c | 1 + > > > hw/core/qdev.c| 10 -- > > > hw/mem/nvdimm.c | 3 +++ > > > hw/mem/pc-dimm.c | 1 + > > > hw/pci/pci.c | 1 + > > > hw/ppc/spapr_cpu_core.c | 1 + > > > hw/s390x/ccw-device.c | 1 + > > > hw/scsi/scsi-bus.c| 1 + > > > hw/usb/bus.c | 1 + > > > hw/usb/dev-smartcard-reader.c | 1 + > > > hw/xen/xen_backend.c | 1 + > > > target/i386/cpu.c | 4 ++-- > > > target/s390x/cpu.c| 1 + > > > 13 files changed, 19 insertions(+), 8 deletions(-) > > > > Hm, this seems to break hotplug of virtio devices: > > > > (qemu) device_add virtio-net-pci,id=xxx > > Device 'virtio-net-device' does not support hotplugging > > > > (qemu) device_add virtio-net-ccw,id=yyy > > Device 'virtio-net-device' does not support hotplugging > > > > We probably need to enable hotplug for virtio devices as well? > > I've seen a similar case broken by "[PATCH] hw/core/qdev: Do not > allow hot-plugging without hotplug controller", because we're > blocking creation of devices created internally by other devices, > not just the ones created by device_add. I need to find the > exact reproducer again. > > It would probably simplify things if we move the check for > DeviceClass::hotpluggable to qmp_device_add(), instead of > device_set_realized(). if we would have to do that, than we are probably doing something wrong and not using property right. Perhaps we should fix property instead of trying find a place to check it where it would hurt less.
Re: [Qemu-devel] [PATCH v2] qdev: Mark devices as non-hotpluggable by default
On Mon, Sep 25, 2017 at 01:53:16PM +0200, Cornelia Huck wrote: > On Fri, 22 Sep 2017 11:16:34 +0200 > Thomas Huthwrote: > > > 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 > > --- > > v2: Add missing devices and dropped "RFC" status. See Eduardo's reply on > > the previous version of this patch for the rationale which devices need > > to be hotpluggable: > > https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg06128.html > > > > hw/char/virtio-serial-bus.c | 1 + > > hw/core/qdev.c| 10 -- > > hw/mem/nvdimm.c | 3 +++ > > hw/mem/pc-dimm.c | 1 + > > hw/pci/pci.c | 1 + > > hw/ppc/spapr_cpu_core.c | 1 + > > hw/s390x/ccw-device.c | 1 + > > hw/scsi/scsi-bus.c| 1 + > > hw/usb/bus.c | 1 + > > hw/usb/dev-smartcard-reader.c | 1 + > > hw/xen/xen_backend.c | 1 + > > target/i386/cpu.c | 4 ++-- > > target/s390x/cpu.c| 1 + > > 13 files changed, 19 insertions(+), 8 deletions(-) > > Hm, this seems to break hotplug of virtio devices: > > (qemu) device_add virtio-net-pci,id=xxx > Device 'virtio-net-device' does not support hotplugging > > (qemu) device_add virtio-net-ccw,id=yyy > Device 'virtio-net-device' does not support hotplugging > > We probably need to enable hotplug for virtio devices as well? I've seen a similar case broken by "[PATCH] hw/core/qdev: Do not allow hot-plugging without hotplug controller", because we're blocking creation of devices created internally by other devices, not just the ones created by device_add. I need to find the exact reproducer again. It would probably simplify things if we move the check for DeviceClass::hotpluggable to qmp_device_add(), instead of device_set_realized(). -- Eduardo
Re: [Qemu-devel] [PATCH v2] qdev: Mark devices as non-hotpluggable by default
On Fri, 22 Sep 2017 11:16:34 +0200 Thomas Huthwrote: > 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 > --- > v2: Add missing devices and dropped "RFC" status. See Eduardo's reply on > the previous version of this patch for the rationale which devices need > to be hotpluggable: > https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg06128.html > > hw/char/virtio-serial-bus.c | 1 + > hw/core/qdev.c| 10 -- > hw/mem/nvdimm.c | 3 +++ > hw/mem/pc-dimm.c | 1 + > hw/pci/pci.c | 1 + > hw/ppc/spapr_cpu_core.c | 1 + > hw/s390x/ccw-device.c | 1 + > hw/scsi/scsi-bus.c| 1 + > hw/usb/bus.c | 1 + > hw/usb/dev-smartcard-reader.c | 1 + > hw/xen/xen_backend.c | 1 + > target/i386/cpu.c | 4 ++-- > target/s390x/cpu.c| 1 + > 13 files changed, 19 insertions(+), 8 deletions(-) Hm, this seems to break hotplug of virtio devices: (qemu) device_add virtio-net-pci,id=xxx Device 'virtio-net-device' does not support hotplugging (qemu) device_add virtio-net-ccw,id=yyy Device 'virtio-net-device' does not support hotplugging We probably need to enable hotplug for virtio devices as well?
Re: [Qemu-devel] [PATCH v2] qdev: Mark devices as non-hotpluggable by default
On 22/09/2017 12:16, 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--- v2: Add missing devices and dropped "RFC" status. See Eduardo's reply on the previous version of this patch for the rationale which devices need to be hotpluggable: https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg06128.html hw/char/virtio-serial-bus.c | 1 + hw/core/qdev.c| 10 -- hw/mem/nvdimm.c | 3 +++ hw/mem/pc-dimm.c | 1 + hw/pci/pci.c | 1 + hw/ppc/spapr_cpu_core.c | 1 + hw/s390x/ccw-device.c | 1 + hw/scsi/scsi-bus.c| 1 + hw/usb/bus.c | 1 + hw/usb/dev-smartcard-reader.c | 1 + hw/xen/xen_backend.c | 1 + target/i386/cpu.c | 4 ++-- target/s390x/cpu.c| 1 + 13 files changed, 19 insertions(+), 8 deletions(-) diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c index 17a1bb0..da26fc2 100644 --- a/hw/char/virtio-serial-bus.c +++ b/hw/char/virtio-serial-bus.c @@ -1097,6 +1097,7 @@ static void virtio_serial_port_class_init(ObjectClass *klass, void *data) k->realize = virtser_port_device_realize; k->unrealize = virtser_port_device_unrealize; k->props = virtser_props; +k->hotpluggable = true; } static const TypeInfo virtio_serial_port_type_info = { diff --git a/hw/core/qdev.c b/hw/core/qdev.c index d9ccce6..28fd92f 100644 --- a/hw/core/qdev.c +++ b/hw/core/qdev.c @@ -1125,13 +1125,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; } diff --git a/hw/mem/nvdimm.c b/hw/mem/nvdimm.c index 952fce5..02be9f3 100644 --- a/hw/mem/nvdimm.c +++ b/hw/mem/nvdimm.c @@ -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; diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c index 66eace5..1f78567 100644 --- a/hw/mem/pc-dimm.c +++ b/hw/mem/pc-dimm.c @@ -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; diff --git a/hw/pci/pci.c b/hw/pci/pci.c index 1e6fb88..8db380d 100644 --- a/hw/pci/pci.c +++ b/hw/pci/pci.c @@ -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; } diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c index c08ee75..720284e 100644 --- a/hw/ppc/spapr_cpu_core.c +++ b/hw/ppc/spapr_cpu_core.c @@ -325,6 +325,7 @@ void spapr_cpu_core_class_init(ObjectClass *oc, void *data) dc->realize = spapr_cpu_core_realize; dc->unrealize = spapr_cpu_core_unrealizefn; dc->props = spapr_cpu_core_properties; +dc->hotpluggable = true; scc->cpu_class = cpu_class_by_name(TYPE_POWERPC_CPU, data); g_assert(scc->cpu_class); } diff
Re: [Qemu-devel] [PATCH v2] qdev: Mark devices as non-hotpluggable by default
On Fri, Sep 22, 2017 at 11:16:34AM +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. > > Signed-off-by: Thomas HuthSeems reasonable to me. Reviewed-by: David Gibson > --- > v2: Add missing devices and dropped "RFC" status. See Eduardo's reply on > the previous version of this patch for the rationale which devices need > to be hotpluggable: > https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg06128.html > > hw/char/virtio-serial-bus.c | 1 + > hw/core/qdev.c| 10 -- > hw/mem/nvdimm.c | 3 +++ > hw/mem/pc-dimm.c | 1 + > hw/pci/pci.c | 1 + > hw/ppc/spapr_cpu_core.c | 1 + > hw/s390x/ccw-device.c | 1 + > hw/scsi/scsi-bus.c| 1 + > hw/usb/bus.c | 1 + > hw/usb/dev-smartcard-reader.c | 1 + > hw/xen/xen_backend.c | 1 + > target/i386/cpu.c | 4 ++-- > target/s390x/cpu.c| 1 + > 13 files changed, 19 insertions(+), 8 deletions(-) > > diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c > index 17a1bb0..da26fc2 100644 > --- a/hw/char/virtio-serial-bus.c > +++ b/hw/char/virtio-serial-bus.c > @@ -1097,6 +1097,7 @@ static void virtio_serial_port_class_init(ObjectClass > *klass, void *data) > k->realize = virtser_port_device_realize; > k->unrealize = virtser_port_device_unrealize; > k->props = virtser_props; > +k->hotpluggable = true; > } > > static const TypeInfo virtio_serial_port_type_info = { > diff --git a/hw/core/qdev.c b/hw/core/qdev.c > index d9ccce6..28fd92f 100644 > --- a/hw/core/qdev.c > +++ b/hw/core/qdev.c > @@ -1125,13 +1125,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; > } > > diff --git a/hw/mem/nvdimm.c b/hw/mem/nvdimm.c > index 952fce5..02be9f3 100644 > --- a/hw/mem/nvdimm.c > +++ b/hw/mem/nvdimm.c > @@ -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; > diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c > index 66eace5..1f78567 100644 > --- a/hw/mem/pc-dimm.c > +++ b/hw/mem/pc-dimm.c > @@ -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; > diff --git a/hw/pci/pci.c b/hw/pci/pci.c > index 1e6fb88..8db380d 100644 > --- a/hw/pci/pci.c > +++ b/hw/pci/pci.c > @@ -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; > } > > diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c > index c08ee75..720284e 100644 > --- a/hw/ppc/spapr_cpu_core.c > +++ b/hw/ppc/spapr_cpu_core.c > @@ -325,6 +325,7 @@ void spapr_cpu_core_class_init(ObjectClass *oc, void