| Message ID | 1430335224-6716-12-git-send-email-mdroth@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
On 29/04/2015 21:20, Michael Roth wrote: > 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); > } > I think it's okay to just change the type of qbus_set_hotplug_handler's handler argument, and get rid altogether of qbus_set_hotplug_handler_internal. Paolo
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); >
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);
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> --- hw/core/qdev.c | 9 ++++----- include/hw/qdev-core.h | 2 ++ 2 files changed, 6 insertions(+), 5 deletions(-)