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

[Thomas Huth]

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