Message ID | 20180514100023.12542-4-david@redhat.com |
---|---|
State | New |
Headers | show |
Series | MemoryDevice: use multi stage hotplug handlers | expand |
On Mon, May 14, 2018 at 12:00:08PM +0200, David Hildenbrand wrote: > From: Igor Mammedov <imammedo@redhat.com> > > it will allow to return another hotplug handler than the default > one for a specific bus based device type. Which is needed to handle > non trivial plug/unplug sequences that need the access to resources > configured outside of bus where device is attached. > > That will allow for returned hotplug handler to orchestrate wiring > in arbitrary order, by chaining other hotplug handlers when > it's needed. > > PS: > It could be used for hybrid virtio-mem and virtio-pmem devices > where it will return machine as hotplug handler which will do > necessary wiring at machine level and then pass control down > the chain to bus specific hotplug handler. > > Example of top level hotplug handler override and custom plug sequence: > > some_machine_get_hotplug_handler(machine){ > if (object_dynamic_cast(OBJECT(dev), TYPE_SOME_BUS_DEVICE)) { > return HOTPLUG_HANDLER(machine); > } > return NULL; > } > > some_machine_device_plug(hotplug_dev, dev) { > if (object_dynamic_cast(OBJECT(dev), TYPE_SOME_BUS_DEVICE)) { > /* do machine specific initialization */ > some_machine_init_special_device(dev) > > /* pass control to bus specific handler */ > hotplug_handler_plug(dev->parent_bus->hotplug_handler, dev) > } > } Hi, David. I might be misreading, but isn't it more like the following? some_machine_device_plug(hotplug_dev, dev) { if (object_dynamic_cast(OBJECT(dev), TYPE_SOME_BUS_DEVICE)) { /* do machine specific initialization */ some_machine_init_special_device(dev) } else { /* pass control to bus specific handler */ hotplug_handler_plug(dev->parent_bus->hotplug_handler, dev) } } > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > Signed-off-by: David Hildenbrand <david@redhat.com> > --- > hw/core/qdev.c | 6 ++---- > include/hw/qdev-core.h | 11 +++++++++++ > 2 files changed, 13 insertions(+), 4 deletions(-) > > diff --git a/hw/core/qdev.c b/hw/core/qdev.c > index f6f92473b8..885286f579 100644 > --- a/hw/core/qdev.c > +++ b/hw/core/qdev.c > @@ -261,12 +261,10 @@ HotplugHandler *qdev_get_machine_hotplug_handler(DeviceState *dev) > > HotplugHandler *qdev_get_hotplug_handler(DeviceState *dev) > { > - HotplugHandler *hotplug_ctrl; > + HotplugHandler *hotplug_ctrl = qdev_get_machine_hotplug_handler(dev); > > - if (dev->parent_bus && dev->parent_bus->hotplug_handler) { > + if (hotplug_ctrl == NULL && dev->parent_bus) { > hotplug_ctrl = dev->parent_bus->hotplug_handler; > - } else { > - hotplug_ctrl = qdev_get_machine_hotplug_handler(dev); > } > return hotplug_ctrl; > } > diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h > index 9453588160..e6a8eca558 100644 > --- a/include/hw/qdev-core.h > +++ b/include/hw/qdev-core.h > @@ -286,6 +286,17 @@ void qdev_init_nofail(DeviceState *dev); > void qdev_set_legacy_instance_id(DeviceState *dev, int alias_id, > int required_for_version); > HotplugHandler *qdev_get_machine_hotplug_handler(DeviceState *dev); > +/** > + * qdev_get_hotplug_handler: Get handler responsible for device wiring > + * > + * Find HOTPLUG_HANDLER for @dev that provides [pre|un]plug callbacks for it. > + * > + * Note: in case @dev has a parent bus, it will be returned as handler unless > + * machine handler overrides it. > + * > + * Returns: pointer to object that implements TYPE_HOTPLUG_HANDLER interface > + * or NULL if there aren't any. > + */ > HotplugHandler *qdev_get_hotplug_handler(DeviceState *dev); > void qdev_unplug(DeviceState *dev, Error **errp); > void qdev_simple_device_unplug_cb(HotplugHandler *hotplug_dev, > -- > 2.14.3 > >
On 15.05.2018 02:00, Murilo Opsfelder Araujo wrote: > On Mon, May 14, 2018 at 12:00:08PM +0200, David Hildenbrand wrote: >> From: Igor Mammedov <imammedo@redhat.com> >> >> it will allow to return another hotplug handler than the default >> one for a specific bus based device type. Which is needed to handle >> non trivial plug/unplug sequences that need the access to resources >> configured outside of bus where device is attached. >> >> That will allow for returned hotplug handler to orchestrate wiring >> in arbitrary order, by chaining other hotplug handlers when >> it's needed. >> >> PS: >> It could be used for hybrid virtio-mem and virtio-pmem devices >> where it will return machine as hotplug handler which will do >> necessary wiring at machine level and then pass control down >> the chain to bus specific hotplug handler. >> >> Example of top level hotplug handler override and custom plug sequence: >> >> some_machine_get_hotplug_handler(machine){ >> if (object_dynamic_cast(OBJECT(dev), TYPE_SOME_BUS_DEVICE)) { >> return HOTPLUG_HANDLER(machine); >> } >> return NULL; >> } >> >> some_machine_device_plug(hotplug_dev, dev) { >> if (object_dynamic_cast(OBJECT(dev), TYPE_SOME_BUS_DEVICE)) { >> /* do machine specific initialization */ >> some_machine_init_special_device(dev) >> >> /* pass control to bus specific handler */ >> hotplug_handler_plug(dev->parent_bus->hotplug_handler, dev) >> } >> } > > Hi, David. > > I might be misreading, but isn't it more like the following? > > some_machine_device_plug(hotplug_dev, dev) { > if (object_dynamic_cast(OBJECT(dev), TYPE_SOME_BUS_DEVICE)) { > /* do machine specific initialization */ > some_machine_init_special_device(dev) > } else { > /* pass control to bus specific handler */ > hotplug_handler_plug(dev->parent_bus->hotplug_handler, dev) > } > } There are various ways how to implement it ,this example if from Igor and most probably not complete (only showcases how things are passed on to the next handler). I decided to implement it slightly different, to make it more flexible. Best you have a look at the following patches in this series. Thanks! > >> Signed-off-by: Igor Mammedov <imammedo@redhat.com> >> Signed-off-by: David Hildenbrand <david@redhat.com> >> --- >> hw/core/qdev.c | 6 ++---- >> include/hw/qdev-core.h | 11 +++++++++++ >> 2 files changed, 13 insertions(+), 4 deletions(-) >> >> diff --git a/hw/core/qdev.c b/hw/core/qdev.c >> index f6f92473b8..885286f579 100644 >> --- a/hw/core/qdev.c >> +++ b/hw/core/qdev.c >> @@ -261,12 +261,10 @@ HotplugHandler *qdev_get_machine_hotplug_handler(DeviceState *dev) >> >> HotplugHandler *qdev_get_hotplug_handler(DeviceState *dev) >> { >> - HotplugHandler *hotplug_ctrl; >> + HotplugHandler *hotplug_ctrl = qdev_get_machine_hotplug_handler(dev); >> >> - if (dev->parent_bus && dev->parent_bus->hotplug_handler) { >> + if (hotplug_ctrl == NULL && dev->parent_bus) { >> hotplug_ctrl = dev->parent_bus->hotplug_handler; >> - } else { >> - hotplug_ctrl = qdev_get_machine_hotplug_handler(dev); >> } >> return hotplug_ctrl; >> } >> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h >> index 9453588160..e6a8eca558 100644 >> --- a/include/hw/qdev-core.h >> +++ b/include/hw/qdev-core.h >> @@ -286,6 +286,17 @@ void qdev_init_nofail(DeviceState *dev); >> void qdev_set_legacy_instance_id(DeviceState *dev, int alias_id, >> int required_for_version); >> HotplugHandler *qdev_get_machine_hotplug_handler(DeviceState *dev); >> +/** >> + * qdev_get_hotplug_handler: Get handler responsible for device wiring >> + * >> + * Find HOTPLUG_HANDLER for @dev that provides [pre|un]plug callbacks for it. >> + * >> + * Note: in case @dev has a parent bus, it will be returned as handler unless >> + * machine handler overrides it. >> + * >> + * Returns: pointer to object that implements TYPE_HOTPLUG_HANDLER interface >> + * or NULL if there aren't any. >> + */ >> HotplugHandler *qdev_get_hotplug_handler(DeviceState *dev); >> void qdev_unplug(DeviceState *dev, Error **errp); >> void qdev_simple_device_unplug_cb(HotplugHandler *hotplug_dev, >> -- >> 2.14.3 >> >> >
diff --git a/hw/core/qdev.c b/hw/core/qdev.c index f6f92473b8..885286f579 100644 --- a/hw/core/qdev.c +++ b/hw/core/qdev.c @@ -261,12 +261,10 @@ HotplugHandler *qdev_get_machine_hotplug_handler(DeviceState *dev) HotplugHandler *qdev_get_hotplug_handler(DeviceState *dev) { - HotplugHandler *hotplug_ctrl; + HotplugHandler *hotplug_ctrl = qdev_get_machine_hotplug_handler(dev); - if (dev->parent_bus && dev->parent_bus->hotplug_handler) { + if (hotplug_ctrl == NULL && dev->parent_bus) { hotplug_ctrl = dev->parent_bus->hotplug_handler; - } else { - hotplug_ctrl = qdev_get_machine_hotplug_handler(dev); } return hotplug_ctrl; } diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h index 9453588160..e6a8eca558 100644 --- a/include/hw/qdev-core.h +++ b/include/hw/qdev-core.h @@ -286,6 +286,17 @@ void qdev_init_nofail(DeviceState *dev); void qdev_set_legacy_instance_id(DeviceState *dev, int alias_id, int required_for_version); HotplugHandler *qdev_get_machine_hotplug_handler(DeviceState *dev); +/** + * qdev_get_hotplug_handler: Get handler responsible for device wiring + * + * Find HOTPLUG_HANDLER for @dev that provides [pre|un]plug callbacks for it. + * + * Note: in case @dev has a parent bus, it will be returned as handler unless + * machine handler overrides it. + * + * Returns: pointer to object that implements TYPE_HOTPLUG_HANDLER interface + * or NULL if there aren't any. + */ HotplugHandler *qdev_get_hotplug_handler(DeviceState *dev); void qdev_unplug(DeviceState *dev, Error **errp); void qdev_simple_device_unplug_cb(HotplugHandler *hotplug_dev,