On 11.09.2017 14:53, Igor Mammedov wrote:
> On Thu, 7 Sep 2017 11:22:42 +0200
> Thomas Huth <th...@redhat.com> wrote:
>
>> qdev_unplug() bails out with an assertion if the user tries to device_del
>> a hot-plugged device that does not have a hotplug controller. Unfortunately,
>> our devices are all marked with hotpluggable = true by default (see the
>> device_class_init() function in qdev.c), so it currently can happen that
>> the user runs into this situation and QEMU gets terminated unexpectedly:
>>
>> $ qemu-system-aarch64 -M virt -nographic -nodefaults -monitor stdio -S
>> QEMU 2.10.50 monitor - type 'help' for more information
>> (qemu) device_add aux-to-i2c-bridge,id=x
>> (qemu) device_del x
>> **
>> ERROR:qdev-monitor.c:872:qdev_unplug: assertion failed: (hotplug_ctrl)
>> Aborted (core dumped)
>> Hotplugging devices without a hotplug controller does not make much sense,
>> so we should disallow this during the device_add process already!
>>
>> Suggested-by: Paolo Bonzini <pbonz...@redhat.com>
>> Signed-off-by: Thomas Huth <th...@redhat.com>
>> ---
>> hw/core/qdev.c | 5 +++++
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
>> index 606ab53..d9ccce6 100644
>> --- a/hw/core/qdev.c
>> +++ b/hw/core/qdev.c
>> @@ -908,6 +908,11 @@ static void device_set_realized(Object *obj, bool
>> value, Error **errp)
>> if (local_err != NULL) {
>> goto fail;
>> }
>> + } else if (dev->hotplugged) {
>> + /* Hot-plugged device without hotplug controller? No way! */
>> + error_setg(&local_err, QERR_DEVICE_NO_HOTPLUG,
>> + object_get_typename(obj));
>> + goto fail;
>> }
>>
>> if (dc->realize) {
>
> maybe it should be other way around, i.e, fix device so that following would
> work
>
> device_set_realized()
> if (dev->hotplugged && !dc->hotpluggable) {
>
> error_setg(errp, QERR_DEVICE_NO_HOTPLUG, object_get_typename(obj));
>
> return;
>
> }
>
> instead of leaving device broken, like in yours
> 84ebd3e watchdog/wdt_diag288: Mark diag288 watchdog as non-hotpluggable
No, that apparently does not work right for new devices since people
keep forgetting to set hotpluggable = false there. Both, Paolo and Peter
suggested that we should not allow hot-plugging if there's no hot plug
controller - it indeed does not make sense, so we should not allow it.
Thomas
