[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] hw/misc/auxbus.c: Mark the aux-to-i2c-bridge de
From: |
Thomas Huth |
Subject: |
Re: [Qemu-devel] [PATCH] hw/misc/auxbus.c: Mark the aux-to-i2c-bridge device as non-hotpluggable |
Date: |
Tue, 22 Aug 2017 17:42:59 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.0 |
On 22.08.2017 17:30, Peter Maydell wrote:
> On 22 August 2017 at 16:26, Thomas Huth <address@hidden> 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