Re: [Qemu-devel] [RFC PATCH v2.1 05/12] qdev: hotplug: Introduce HotplugHandler.pre_plug() callback

[David Gibson]

On Thu, Mar 31, 2016 at 02:09:14PM +0530, Bharata B Rao wrote:
> From: Igor Mammedov <address@hidden>
> 
> pre_plug callback is to be called before device.realize() is executed.
> This would allow to check/set device's properties from HotplugHandler.
> 
> Signed-off-by: Igor Mammedov <address@hidden>
> Signed-off-by: Bharata B Rao <address@hidden>

Reviewed-by: David Gibson <address@hidden>

It would be really nice to get some opinion on this from Andreas or
Paolo.

> ---
>  hw/core/hotplug.c    | 11 +++++++++++
>  hw/core/qdev.c       |  9 ++++++++-
>  include/hw/hotplug.h | 14 +++++++++++++-
>  3 files changed, 32 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/core/hotplug.c b/hw/core/hotplug.c
> index 645cfca..17ac986 100644
> --- a/hw/core/hotplug.c
> +++ b/hw/core/hotplug.c
> @@ -13,6 +13,17 @@
>  #include "hw/hotplug.h"
>  #include "qemu/module.h"
>  
> +void hotplug_handler_pre_plug(HotplugHandler *plug_handler,
> +                              DeviceState *plugged_dev,
> +                              Error **errp)
> +{
> +    HotplugHandlerClass *hdc = HOTPLUG_HANDLER_GET_CLASS(plug_handler);
> +
> +    if (hdc->pre_plug) {
> +        hdc->pre_plug(plug_handler, plugged_dev, errp);
> +    }
> +}
> +
>  void hotplug_handler_plug(HotplugHandler *plug_handler,
>                            DeviceState *plugged_dev,
>                            Error **errp)
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index db41aa1..a0b3aad 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -1062,6 +1062,14 @@ static void device_set_realized(Object *obj, bool 
> value, Error **errp)
>              g_free(name);
>          }
>  
> +        hotplug_ctrl = qdev_get_hotplug_handler(dev);
> +        if (hotplug_ctrl) {
> +            hotplug_handler_pre_plug(hotplug_ctrl, dev, &local_err);
> +            if (local_err != NULL) {
> +                goto fail;
> +            }
> +        }
> +
>          if (dc->realize) {
>              dc->realize(dev, &local_err);
>          }
> @@ -1072,7 +1080,6 @@ static void device_set_realized(Object *obj, bool 
> value, Error **errp)
>  
>          DEVICE_LISTENER_CALL(realize, Forward, dev);
>  
> -        hotplug_ctrl = qdev_get_hotplug_handler(dev);
>          if (hotplug_ctrl) {
>              hotplug_handler_plug(hotplug_ctrl, dev, &local_err);
>          }
> diff --git a/include/hw/hotplug.h b/include/hw/hotplug.h
> index 2db025d..50d84e9 100644
> --- a/include/hw/hotplug.h
> +++ b/include/hw/hotplug.h
> @@ -46,7 +46,8 @@ typedef void (*hotplug_fn)(HotplugHandler *plug_handler,
>   * hardware (un)plug functions.
>   *
>   * @parent: Opaque parent interface.
> - * @plug: plug callback.
> + * @pre_plug: pre plug callback called at start of device.realize(true)
> + * @plug: plug callback called at end of device.realize(true).
>   * @unplug_request: unplug request callback.
>   *                  Used as a means to initiate device unplug for devices 
> that
>   *                  require asynchronous unplug handling.
> @@ -59,6 +60,7 @@ typedef struct HotplugHandlerClass {
>      InterfaceClass parent;
>  
>      /* <public> */
> +    hotplug_fn pre_plug;
>      hotplug_fn plug;
>      hotplug_fn unplug_request;
>      hotplug_fn unplug;
> @@ -74,6 +76,16 @@ void hotplug_handler_plug(HotplugHandler *plug_handler,
>                            Error **errp);
>  
>  /**
> + * hotplug_handler_pre_plug:
> + *
> + * Call #HotplugHandlerClass.pre_plug callback of @plug_handler.
> + */
> +void hotplug_handler_pre_plug(HotplugHandler *plug_handler,
> +                              DeviceState *plugged_dev,
> +                              Error **errp);
> +
> +
> +/**
>   * hotplug_handler_unplug_request:
>   *
>   * Calls #HotplugHandlerClass.unplug_request callback of @plug_handler.

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

signature.asc
Description: PGP signature