Re: [Qemu-devel] [PATCH 13/30] qdev: hotplug: set handler only if HOTPLUG_HANDLER interface is supported

[Paolo Bonzini]

Il 24/09/2014 13:48, Igor Mammedov ha scritto:
> Check if 'handler' implements HOTPLUG_HANDLER interface
> before setting it, if it's not then do nothing and leave
> bus not hotpluggable.
> 
> That would allow to reuse the same code for creating bus
> for example 'scsi_bus_new()' for both hotpluggable and not
> hotpluggable controllers.
> 
> Signed-off-by: Igor Mammedov <address@hidden>
> ---
> PS:
> Keep allow_hotplug = 1 before check to preserve legacy
> behaviour for incremental conversion without breaking
> bisect-ability. I will be removed later in series.
> ---
>  include/hw/qdev-core.h | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> index ba812c5..653ff4a 100644
> --- a/include/hw/qdev-core.h
> +++ b/include/hw/qdev-core.h
> @@ -366,9 +366,14 @@ char *qdev_get_dev_path(DeviceState *dev);
>  static inline void qbus_set_hotplug_handler(BusState *bus, DeviceState 
> *handler,
>                                              Error **errp)
>  {
> +    bus->allow_hotplug = 1;
> +
> +    if (!object_dynamic_cast(OBJECT(handler), TYPE_HOTPLUG_HANDLER)) {
> +        return;
> +    }
> +
>      object_property_set_link(OBJECT(bus), OBJECT(handler),
>                               QDEV_HOTPLUG_HANDLER_PROPERTY, errp);
> -    bus->allow_hotplug = 1;
>  }
>  
>  static inline bool qbus_is_hotpluggable(BusState *bus)
> 

Ok, so scsi won't error_abort, but you are actually breaking
hotplug/unplug on HBAs other than virtio-scsi and pvscsi.  On those HBAs
you can actually do surprise removal and hotplug, but you have to
manually rescan the bus in the virtual machine.

Reviewed-by: Paolo Bonzini <address@hidden>