On Sat, 14 Dec 2013 17:10:35 +1000 Peter Crosthwaite <peter.crosthwa...@xilinx.com> wrote:
> On Fri, Dec 13, 2013 at 10:44 PM, Igor Mammedov <imamm...@redhat.com> wrote: > > Currently it's possible to make PCIDevice not hotpluggable by using > > no_hotplug field of PCIDeviceClass. However it limits this > > only to PCI devices and prevents from generalizing hotplug code. > > > > So add similar field to DeviceClass so it could be reused with other > > Devices and would allow to replace PCI specific hotplug callbacks > > with generic implementation. > > > > In addition expose field as "hotpluggable" readonly property, to make > > it possible to get it via QOM interface. > > > > Make DeviceClass hotpluggable by default as it was assumed before. > > > > Signed-off-by: Igor Mammedov <imamm...@redhat.com> > > --- > > v4: > > * s/hotplugable/hotpluggable/ > > > > v3: > > * make DeviceClass hotpluggable by default > > Since PCIDevice still uses internal no_hotlpug checks it shouldn't > > reggress. And follow up patch that converts PCIDevices to use > > "hotpluggable" property will take care about not hotpluggable PCI > > devices explicitly setting "hotpluggable" to false in their class_init(). > > > > * move generic hotplug checks from > > "7/11 qdev:pci: refactor PCIDevice to use generic "hotplugable" property" > > to this patch > > --- > > hw/core/qdev.c | 29 +++++++++++++++++++++++++++++ > > include/hw/qdev-core.h | 3 +++ > > 2 files changed, 32 insertions(+) > > > > diff --git a/hw/core/qdev.c b/hw/core/qdev.c > > index 25c2d2c..9418fea 100644 > > --- a/hw/core/qdev.c > > +++ b/hw/core/qdev.c > > @@ -215,6 +215,12 @@ void qdev_unplug(DeviceState *dev, Error **errp) > > } > > assert(dc->unplug != NULL); > > > > + if (!dc->hotpluggable) { > > + error_set(errp, QERR_DEVICE_NO_HOTPLUG, > > + object_get_typename(OBJECT(dev))); > > + return; > > + } > > + > > qdev_hot_removed = true; > > > > if (dc->unplug(dev) < 0) { > > @@ -679,6 +685,11 @@ static void device_set_realized(Object *obj, bool > > value, Error **err) > > DeviceClass *dc = DEVICE_GET_CLASS(dev); > > Error *local_err = NULL; > > > > + if (dev->hotplugged && !dc->hotpluggable) { > > + error_set(err, QERR_DEVICE_NO_HOTPLUG, object_get_typename(obj)); > > + return; > > + } > > + > > if (value && !dev->realized) { > > if (!obj->parent && local_err == NULL) { > > static int unattached_count; > > @@ -719,6 +730,14 @@ static void device_set_realized(Object *obj, bool > > value, Error **err) > > dev->realized = value; > > } > > > > +static bool device_get_hotpluggable(Object *obj, Error **err) > > +{ > > + DeviceClass *dc = DEVICE_GET_CLASS(obj); > > + DeviceState *dev = DEVICE(obj); > > + > > + return dc->hotpluggable && dev->parent_bus->allow_hotplug; > > +} > > + > > static void device_initfn(Object *obj) > > { > > DeviceState *dev = DEVICE(obj); > > @@ -736,6 +755,8 @@ static void device_initfn(Object *obj) > > > > object_property_add_bool(obj, "realized", > > device_get_realized, device_set_realized, > > NULL); > > + object_property_add_bool(obj, "hotpluggable", > > + device_get_hotpluggable, NULL, NULL); > > > > class = object_get_class(OBJECT(dev)); > > do { > > @@ -784,6 +805,14 @@ static void device_class_base_init(ObjectClass *class, > > void *data) > > * so do not propagate them to the subclasses. > > */ > > klass->props = NULL; > > + > > + /* by default all devices were considered as hotpluggable, > > + * so with intent to check it in generic qdev_unplug() / > > + * device_set_realized() functions make every device > > + * hotpluggable. Devices that shouldn't be hoplugable, > > + * should override it in their class_init() > > + */ > > + klass->hotpluggable = true; > > } > > > > static void device_unparent(Object *obj) > > diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h > > index 684a5da..04bbef4 100644 > > --- a/include/hw/qdev-core.h > > +++ b/include/hw/qdev-core.h > > @@ -50,6 +50,8 @@ struct VMStateDescription; > > * is changed to %true. Deprecated, new types inheriting directly from > > * TYPE_DEVICE should use @realize instead, new leaf types should consult > > * their respective parent type. > > + * @hotpluggable: booleean indicating if #DeviceClass is hotpluggable, > > available > > "boolean". Although the fact that its bool is implicit just by saying: > > "@hotpluggable: indicates if #DeviceClass is hotpluggable ... sure > > Regards, > Peter > > > + * as readonly "hotpluggable" property of #DeviceState instance > > * > > * # Realization # > > * Devices are constructed in two stages, > > @@ -99,6 +101,7 @@ typedef struct DeviceClass { > > const char *desc; > > Property *props; > > int no_user; > > + bool hotpluggable; > > > > /* callbacks */ > > void (*reset)(DeviceState *dev); > > -- > > 1.8.3.1 > > > >