From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59334) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WFnxE-0006uI-7i for qemu-devel@nongnu.org; Tue, 18 Feb 2014 11:55:37 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WFnx7-00009h-E9 for qemu-devel@nongnu.org; Tue, 18 Feb 2014 11:55:32 -0500 Received: from mx1.redhat.com ([209.132.183.28]:1546) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WFnx7-00009N-6f for qemu-devel@nongnu.org; Tue, 18 Feb 2014 11:55:25 -0500 Date: Tue, 18 Feb 2014 17:55:18 +0100 From: Igor Mammedov Message-ID: <20140218175518.5db1e78d@nial.usersys.redhat.com> In-Reply-To: <53038BC4.8070800@suse.de> References: <1392050814-31814-1-git-send-email-mst@redhat.com> <1392050814-31814-14-git-send-email-mst@redhat.com> <53038BC4.8070800@suse.de> MIME-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PULL 13/20] qdev: add "hotpluggable" property to Device List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Andreas =?ISO-8859-1?B?RuRyYmVy?= Cc: Peter Maydell , Paolo Bonzini , qemu-devel@nongnu.org, Anthony Liguori , "Michael S. Tsirkin" On Tue, 18 Feb 2014 17:35:16 +0100 Andreas F=E4rber wrote: > Am 10.02.2014 17:48, schrieb Michael S. Tsirkin: > > From: Igor Mammedov > >=20 > > 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. > >=20 > > 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. > >=20 > > In addition expose field as "hotpluggable" readonly property, > > to make it possible to read its value via QOM interface. > >=20 > > Make DeviceClass hotpluggable by default as it was assumed > > before. > >=20 > > Signed-off-by: Igor Mammedov > > Reviewed-by: Michael S. Tsirkin > > Signed-off-by: Michael S. Tsirkin > > --- > > include/hw/qdev-core.h | 3 +++ > > hw/core/qdev.c | 29 +++++++++++++++++++++++++++++ > > 2 files changed, 32 insertions(+) > >=20 > > 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 cons= ult > > * 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; > > =20 > > /* 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 !=3D NULL); > > =20 > > + if (!dc->hotpluggable) { > > + error_set(errp, QERR_DEVICE_NO_HOTPLUG, > > + object_get_typename(OBJECT(dev))); > > + return; > > + } > > + > > qdev_hot_removed =3D true; > > =20 > > if (dc->unplug(dev) < 0) { > > @@ -694,6 +700,11 @@ static void device_set_realized(Object *obj, bool = value, Error **err) > > DeviceClass *dc =3D DEVICE_GET_CLASS(dev); > > Error *local_err =3D NULL; > > =20 > > + 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 =3D=3D NULL) { > > static int unattached_count; > > @@ -734,6 +745,14 @@ static void device_set_realized(Object *obj, bool = value, Error **err) > > dev->realized =3D value; > > } > > =20 > > +static bool device_get_hotpluggable(Object *obj, Error **err) > > +{ > > + DeviceClass *dc =3D DEVICE_GET_CLASS(obj); > > + DeviceState *dev =3D DEVICE(obj); > > + > > + return dc->hotpluggable && dev->parent_bus->allow_hotplug; > > +} > > + > > static void device_initfn(Object *obj) > > { > > DeviceState *dev =3D DEVICE(obj); > > @@ -750,6 +769,8 @@ static void device_initfn(Object *obj) > > =20 > > object_property_add_bool(obj, "realized", > > device_get_realized, device_set_realized,= NULL); > > + object_property_add_bool(obj, "hotpluggable", > > + device_get_hotpluggable, NULL, NULL); > > =20 > > class =3D object_get_class(OBJECT(dev)); > > do { > > @@ -786,6 +807,14 @@ static void device_class_base_init(ObjectClass *cl= ass, void *data) > > * so do not propagate them to the subclasses. > > */ > > klass->props =3D 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 =3D true; >=20 > 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 defaul= ts. You are right it should be in class_init(), I'll post fix shortly. > Regards, > Andreas >=20 > > } > > =20 > > static void device_unparent(Object *obj) >=20