Re: [Qemu-devel] [PATCH] hw/misc/auxbus.c: Mark the aux-to-i2c-bridge device as non-hotpluggable

[Thomas Huth]

On 22.08.2017 17:12, Peter Maydell wrote:
> On 22 August 2017 at 16:06, Thomas Huth <address@hidden> wrote:
>> QEMU currently aborts if the user tries to do something like this:
>>
>> $ aarch64-softmmu/qemu-system-aarch64 -S -M integratorcp -nographic
>> QEMU 2.9.93 monitor - type 'help' for more information
>> (qemu) device_add aux-to-i2c-bridge,id=x
>> (qemu) device_del x
>> **
>> ERROR:qemu/qdev-monitor.c:872:qdev_unplug: assertion failed: (hotplug_ctrl)
>> Aborted (core dumped)
>>
>> Looks like the device is not hot-pluggable, so let's mark it
>> accordingly.
>>
>> Signed-off-by: Thomas Huth <address@hidden>
>> ---
>>  hw/misc/auxbus.c | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/hw/misc/auxbus.c b/hw/misc/auxbus.c
>> index 8a90ddd..2c62515 100644
>> --- a/hw/misc/auxbus.c
>> +++ b/hw/misc/auxbus.c
>> @@ -222,9 +222,17 @@ static inline I2CBus 
>> *aux_bridge_get_i2c_bus(AUXTOI2CState *bridge)
>>      return bridge->i2c_bus;
>>  }
>>
>> +static void aux_bridge_class_init(ObjectClass *oc, void *data)
>> +{
>> +    DeviceClass *dc = DEVICE_CLASS(oc);
>> +
>> +    dc->hotpluggable = false;
>> +}
> 
> Why is our default "hotpluggable" rather than "not hotpluggable" ?
> We must have way more non-hotpluggable devices than hotpluggable
> ones, and it takes active effort to make a hotpluggable device
> model, so it seems like it would be much less bug-prone to
> require hotpluggable devices to set dc->hotpluggable true
> rather than all the non-hotpluggable ones to set it false...

I think most devices are already non-hotpluggable automatically because
they sit on a bus that is not hot-pluggable (e.g. sysbus devices). The
problematic ones are the devices with .parent = TYPE_DEVICE. And for
these, I think it is quite hard to say whether they should be
hot-pluggable by default or not?
Anyway, according to my tests (I'm currently working on a test that
automatically does device_add + device_del for all devices, as you might
have guessed already), there are not that many devices that cause
problems here, so I guess marking some few with hotpluggable = false
should be OK?

 Thomas