Re: [Qemu-devel] [PATCH] hw/core/qdev: Do not allow hot-plugging without hotplug controller

[Igor Mammedov]

On Thu,  7 Sep 2017 11:22:42 +0200
Thomas Huth <address@hidden> 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 <address@hidden>
> Signed-off-by: Thomas Huth <address@hidden>
> ---
>  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