On Tue, 18 Feb 2014 17:35:16 +0100
Andreas Färber <afaer...@suse.de> wrote:
> Am 10.02.2014 17:48, schrieb Michael S. Tsirkin:
> > From: Igor Mammedov <imamm...@redhat.com>
> >
> > 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. Following
> > patches will replace PCIDeviceClass.no_hotplug with this
> > new property.
> >
> > In addition expose field as "hotpluggable" readonly property,
> > to make it possible to read its value via QOM interface.
> >
> > Make DeviceClass hotpluggable by default as it was assumed
> > before.
> >
> > Signed-off-by: Igor Mammedov <imamm...@redhat.com>
> > Reviewed-by: Michael S. Tsirkin <m...@redhat.com>
> > Signed-off-by: Michael S. Tsirkin <m...@redhat.com>
> > ---
> > include/hw/qdev-core.h | 3 +++
> > hw/core/qdev.c | 29 +++++++++++++++++++++++++++++
> > 2 files changed, 32 insertions(+)
> >
> > diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> > index 41ec533..08d329d 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: indicates if #DeviceClass is hotpluggable, available
> > + * as readonly "hotpluggable" property of #DeviceState instance
> > *
> > * # Realization #
> > * Devices are constructed in two stages,
> > @@ -110,6 +112,7 @@ typedef struct DeviceClass {
> > * TODO remove once we're there
> > */
> > bool cannot_instantiate_with_device_add_yet;
> > + bool hotpluggable;
> >
> > /* callbacks */
> > void (*reset)(DeviceState *dev);
> > diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> > index c9f0c33..5c864db 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) {
> > @@ -694,6 +700,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;
> > @@ -734,6 +745,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);
> > @@ -750,6 +769,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 {
> > @@ -786,6 +807,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 hotpluggable,
> > + * should override it in their class_init()
> > + */
> > + klass->hotpluggable = true;
>
> Is this intentionally being added to class_base_init or should this have
> gone into class_init? As seen above, class_base_init has so far been
> used to "erase" stuff set by the parent classes, not to set random defaults.
You are right it should be in class_init(), I'll post fix shortly.
> Regards,
> Andreas
>
> > }
> >
> > static void device_unparent(Object *obj)
>
