Re: [Qemu-devel] [PATCH 30/30] qdev: HotplugHandler: add support for unplugging BUS-less devices

[Paolo Bonzini]

Il 24/09/2014 13:48, Igor Mammedov ha scritto:
> Signed-off-by: Igor Mammedov <address@hidden>
> ---
>  hw/core/qdev.c | 46 +++++++++++++++++++++++++++-------------------
>  1 file changed, 27 insertions(+), 19 deletions(-)
> 
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index 4182cc3..e309b0e 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -209,9 +209,27 @@ void qdev_set_legacy_instance_id(DeviceState *dev, int 
> alias_id,
>      dev->alias_required_for_version = required_for_version;
>  }
>  
> +static HotplugHandler *qdev_get_hotplug_handler(DeviceState *dev)
> +{
> +    HotplugHandler *hotplug_ctrl = NULL;
> +
> +    if (dev->parent_bus && dev->parent_bus->hotplug_handler) {
> +        hotplug_ctrl = dev->parent_bus->hotplug_handler;
> +    } else if (object_dynamic_cast(qdev_get_machine(), TYPE_MACHINE)) {
> +        MachineState *machine = MACHINE(qdev_get_machine());
> +        MachineClass *mc = MACHINE_GET_CLASS(machine);
> +
> +        if (mc->get_hotplug_handler) {
> +            hotplug_ctrl = mc->get_hotplug_handler(machine, dev);
> +        }
> +    }
> +    return hotplug_ctrl;
> +}
> +
>  void qdev_unplug(DeviceState *dev, Error **errp)
>  {
>      DeviceClass *dc = DEVICE_GET_CLASS(dev);
> +    HotplugHandler *hotplug_ctrl;
>  
>      if (dev->parent_bus && !qbus_is_hotpluggable(dev->parent_bus)) {
>          error_set(errp, QERR_BUS_NO_HOTPLUG, dev->parent_bus->name);
> @@ -226,17 +244,17 @@ void qdev_unplug(DeviceState *dev, Error **errp)
>  
>      qdev_hot_removed = true;
>  
> -    if (dev->parent_bus && dev->parent_bus->hotplug_handler) {
> +    hotplug_ctrl = qdev_get_hotplug_handler(dev);
> +    if (hotplug_ctrl) {
>          HotplugHandlerClass *hdc;
>  
>          /* If device supports async unplug just request it to be done,
>           * otherwise just remove it synchronously */
> -        hdc = HOTPLUG_HANDLER_GET_CLASS(dev->parent_bus->hotplug_handler);
> +        hdc = HOTPLUG_HANDLER_GET_CLASS(hotplug_ctrl);
>          if (hdc->unplug_request) {
> -            hotplug_handler_unplug_request(dev->parent_bus->hotplug_handler,
> -                                           dev, errp);
> +            hotplug_handler_unplug_request(hotplug_ctrl, dev, errp);
>          } else {
> -            hotplug_handler_unplug(dev->parent_bus->hotplug_handler, dev, 
> errp);
> +            hotplug_handler_unplug(hotplug_ctrl, dev, errp);
>          }
>      } else {
>          assert(0);
> @@ -817,6 +835,7 @@ static void device_set_realized(Object *obj, bool value, 
> Error **errp)
>  {
>      DeviceState *dev = DEVICE(obj);
>      DeviceClass *dc = DEVICE_GET_CLASS(dev);
> +    HotplugHandler *hotplug_ctrl;
>      BusState *bus;
>      Error *local_err = NULL;
>  
> @@ -844,20 +863,9 @@ static void device_set_realized(Object *obj, bool value, 
> Error **errp)
>              goto fail;
>          }
>  
> -        if (dev->parent_bus && dev->parent_bus->hotplug_handler) {
> -            hotplug_handler_plug(dev->parent_bus->hotplug_handler,
> -                                 dev, &local_err);
> -        } else if (object_dynamic_cast(qdev_get_machine(), TYPE_MACHINE)) {
> -            HotplugHandler *hotplug_ctrl;
> -            MachineState *machine = MACHINE(qdev_get_machine());
> -            MachineClass *mc = MACHINE_GET_CLASS(machine);
> -
> -            if (mc->get_hotplug_handler) {
> -                hotplug_ctrl = mc->get_hotplug_handler(machine, dev);
> -                if (hotplug_ctrl) {
> -                    hotplug_handler_plug(hotplug_ctrl, dev, &local_err);
> -                }
> -            }
> +        hotplug_ctrl = qdev_get_hotplug_handler(dev);
> +        if (hotplug_ctrl) {
> +            hotplug_handler_plug(hotplug_ctrl, dev, &local_err);
>          }
>  
>          if (local_err != NULL) {
> 

Let's start from the end. :)

Reviewed-by: Paolo Bonzini <address@hidden>