qemu-trivial
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-trivial] [PATCH] hw/misc/auxbus.c: Mark the aux-to-i2c-bridge


From: Peter Maydell
Subject: Re: [Qemu-trivial] [PATCH] hw/misc/auxbus.c: Mark the aux-to-i2c-bridge device as non-hotpluggable
Date: Tue, 22 Aug 2017 16:30:24 +0100

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.

thanks
-- PMM



reply via email to

[Prev in Thread] Current Thread [Next in Thread]