Re: [Qemu-devel] [RFC PATCH 11/15] qdev: add qbus_set_hotplug_handler_generic()

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 3244 bytes --]

On Wed, Apr 29, 2015 at 02:20:20PM -0500, Michael Roth wrote:
> Certain devices types, like memory/CPU, are now being handled using a
> hotplug interface provided by a top-level MachineClass. Hotpluggable
> host bridges are another such device where it makes sense to use a
> machine-level hotplug handler. However, unlike those devices,
> host-bridges have a parent bus (the main system bus), and devices with
> a parent bus use a different mechanism for registering their hotplug
> handlers: qbus_set_hotplug_handler(). This interface currently expects
> a handler to be a subclass of DeviceClass, but this is not the case
> for MachineClass, which derives directly from ObjectClass.
> 
> Internally, the interface only requires an ObjectClass, so expose that
> support via a new qbus_set_hotplug_handler_generic().
> 
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Eduardo Habkost <ehabkost@redhat.com>
> Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>

Seems odd to have to interfaces to me.  Why not just change
qbus_set_hotplug_handler() to take any ObjectClass?

> ---
>  hw/core/qdev.c         | 9 ++++-----
>  include/hw/qdev-core.h | 2 ++
>  2 files changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index fda1d2f..8fd9320 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -108,22 +108,21 @@ void qdev_set_parent_bus(DeviceState *dev, BusState *bus)
>      bus_add_child(bus, dev);
>  }
>  
> -static void qbus_set_hotplug_handler_internal(BusState *bus, Object *handler,
> -                                              Error **errp)
> +void qbus_set_hotplug_handler_generic(BusState *bus, Object *handler,
> +                                      Error **errp)
>  {
> -
>      object_property_set_link(OBJECT(bus), OBJECT(handler),
>                               QDEV_HOTPLUG_HANDLER_PROPERTY, errp);
>  }
>  
>  void qbus_set_hotplug_handler(BusState *bus, DeviceState *handler, Error **errp)
>  {
> -    qbus_set_hotplug_handler_internal(bus, OBJECT(handler), errp);
> +    qbus_set_hotplug_handler_generic(bus, OBJECT(handler), errp);
>  }
>  
>  void qbus_set_bus_hotplug_handler(BusState *bus, Error **errp)
>  {
> -    qbus_set_hotplug_handler_internal(bus, OBJECT(bus), errp);
> +    qbus_set_hotplug_handler_generic(bus, OBJECT(bus), errp);
>  }
>  
>  /* Create a new device.  This only initializes the device state structure
> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> index 17f805e..3b210bf 100644
> --- a/include/hw/qdev-core.h
> +++ b/include/hw/qdev-core.h
> @@ -377,6 +377,8 @@ GSList *qdev_build_hotpluggable_device_list(Object *peripheral);
>  
>  void qbus_set_hotplug_handler(BusState *bus, DeviceState *handler,
>                                Error **errp);
> +void qbus_set_hotplug_handler_generic(BusState *bus, Object *handler,
> +                                      Error **errp);
>  
>  void qbus_set_bus_hotplug_handler(BusState *bus, Error **errp);
>  

-- 
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

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]