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

[Thomas Huth <thuth@redhat.com>]

On 22.08.2017 17:30, Peter Maydell wrote:
> On 22 August 2017 at 16:26, Thomas Huth <thuth@redhat.com> wrote:
>> On 22.08.2017 17:12, Peter Maydell wrote:
>>> 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?
> 
> To implement hotplug you need to write extra code (notably some
> kind of unrealize method to undo whatever you did in realize),
> at which point also setting the hotpluggable flag is trivial.
> 
>> 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?
> 
> The problem is not the devices we have today but the ones we're
> going to write tomorrow. Having hotpluggable default to false
> "fails safe" -- the worst that happens is that somebody writing
> a hotpluggable device finds in their testing that they need to
> set the flag to make it work. Having it default to true fails
> non-safely -- as you've found, people writing devices that
> were never expected to be hotplugged don't try to test the
> error case of attempting hotplug anyway, and in code review
> the absence of a line of code is very hard to reliably spot,
> so we get devices in tree that crash QEMU if you try to
> hotplug them. If we make the default be not-hotpluggable
> we fix this not just today but forever.

OK, you've got a point. I'll ponder about this a little bit and will try
to come up with a patch / some patches ...

 Thomas