[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC PATCH] qdev: Mark devices as non-hotpluggable by d
From: |
Thomas Huth |
Subject: |
Re: [Qemu-devel] [RFC PATCH] qdev: Mark devices as non-hotpluggable by default |
Date: |
Wed, 20 Sep 2017 13:17:58 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 |
On 20.09.2017 12:57, Marcel Apfelbaum wrote:
> On 20/09/2017 13:07, Peter Maydell wrote:
>> On 20 September 2017 at 08:50, Marcel Apfelbaum <address@hidden>
>> wrote:
>>> Hi Thomas,
>>>
>>>
>>> On 19/09/2017 11:55, 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.
>>>>
>>>> Signed-off-by: Thomas Huth <address@hidden>
>>>> ---
>>>> Note: I've marked the patch as RFC since I'm not 100% sure
>>>> whether I've
>>>> correctly identified all devices that should still be marked as hot-
>>>> pluggable. Feedback is welcome!
>>>>
>>>> hw/core/qdev.c | 10 ++++------
>>>> hw/cpu/core.c | 1 +
>>>> hw/mem/nvdimm.c | 3 +++
>>>> hw/mem/pc-dimm.c | 1 +
>>>> hw/pci/pci.c | 1 +
>>>> hw/s390x/ccw-device.c | 1 +
>>>> hw/scsi/scsi-bus.c | 1 +
>>>> hw/usb/bus.c | 1 +
>>>> 8 files changed, 13 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
>>>> index 606ab53..c4f1902 100644
>>>> --- a/hw/core/qdev.c
>>>> +++ b/hw/core/qdev.c
>>>> @@ -1120,13 +1120,11 @@ static void device_class_init(ObjectClass
>>>> *class,
>>>> void *data)
>>>> dc->realize = device_realize;
>>>> dc->unrealize = device_unrealize;
>>>> - /* by default all devices were considered as hotpluggable,
>>>> - * so with intent to check it in generic qdev_unplug() /
>>>> - * device_set_realized() functions make every device
>>>> - * hotpluggable. Devices that shouldn't be hotpluggable,
>>>> - * should override it in their class_init()
>>>> + /*
>>>> + * All devices are considered as cold-pluggable by default. The
>>>> devices
>>>> + * that are hotpluggable should override it in their class_init().
>>>> */
>>>> - dc->hotpluggable = true;
>>>> + dc->hotpluggable = false;
>>>
>>>
>>> I agree with the defensive mode as long as we don't
>>> introduce any regression...
>>
>> Is it possible to hack together some kind of test code that
>> can give us a list of the devices compiled in that have
>> hotpluggable enabled? Then we could compare before and
>> after to see which devices we've changed.
>>
>
> Eduardo came up with some cool automated tests not long ago.
> Eduardo, can your tests help?
You mean the scripts/device-crash-test script? That's only for
cold-plugging ... but I could maybe use the device_add HMP test (see
https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg00817.html ) to
exercise the hotplugging of all devices and add some debug code to
qdev_device_add() to see which devices are dc->hotpluggable and have a
hotplug handler at the same time... I'll give it a try.
Thomas
Re: [Qemu-devel] [RFC PATCH] qdev: Mark devices as non-hotpluggable by default, Anthony PERARD, 2017/09/21
Re: [Qemu-devel] [RFC PATCH] qdev: Mark devices as non-hotpluggable by default, Eduardo Habkost, 2017/09/21