On Wed, 11 Dec 2013 18:02:11 +0100
Paolo Bonzini <pbonz...@redhat.com> wrote:
> Il 11/12/2013 17:57, Igor Mammedov ha scritto:
> > On Wed, 11 Dec 2013 17:29:06 +0100
> > Paolo Bonzini <pbonz...@redhat.com> wrote:
> >
> >> Il 11/12/2013 02:01, Igor Mammedov ha scritto:
> >>> Currently it's possible to make PCIDevice not hotplugable 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 "hotplugable" readonly property, to make
> >>> it possible to get it via QOM interface.
> >>>
> >>> Signed-off-by: Igor Mammedov <imamm...@redhat.com>
> >>> ---
> >>> hw/core/qdev.c | 8 ++++++++
> >>> include/hw/qdev-core.h | 3 +++
> >>> 2 files changed, 11 insertions(+), 0 deletions(-)
> >>>
> >>> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> >>> index 25c2d2c..1b1a684 100644
> >>> --- a/hw/core/qdev.c
> >>> +++ b/hw/core/qdev.c
> >>> @@ -719,6 +719,12 @@ static void device_set_realized(Object *obj, bool
> >>> value, Error **err)
> >>> dev->realized = value;
> >>> }
> >>>
> >>> +static bool device_get_hotplugable(Object *obj, Error **err)
> >>> +{
> >>> + DeviceClass *dc = DEVICE_GET_CLASS(obj);
> >>> + return dc->hotplugable;
> >>> +}
> >>> +
> >>> static void device_initfn(Object *obj)
> >>> {
> >>> DeviceState *dev = DEVICE(obj);
> >>> @@ -736,6 +742,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, "hotplugable",
> >>> + device_get_hotplugable, NULL, NULL);
> >>>
> >>> class = object_get_class(OBJECT(dev));
> >>> do {
> >>> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> >>> index 684a5da..5d4a9c8 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.
> >>> + * @hotplugable: booleean indicating if #DeviceClass is hotplugable,
> >>> available
> >>> + * as readonly "hotplugable" 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 hotplugable;
> >>>
> >>> /* callbacks */
> >>> void (*reset)(DeviceState *dev);
> >>>
> >>
> >> Does this patch break SCSI and virtio-serial hotplug?
> > Ouch,
> > this patch doesn't but
> > "[PATCH 07/11] qdev:pci: refactor PCIDevice to use generic "hotplugable"
> > property" does. See 2 hunks in hw/core/qdev.c
>
> If you set dc->hotpluggable = true, those two hunks could also be moved
> here.
sure. posting as reply to this thread so not to spam list with series respin.
tested with CPU hotplug which was broken too.
>
> Paolo
>
> >>
> >> Perhaps you could initialize dc->hotpluggable to true, and return
> > that's probably the easiest way, since before series all devices were
> > considered hotplugable from qdev POV, so it makes sense to make DeviceClass
> > hotplugable by default to avoid unexpected regressions.
> >
> >> "dc->hotplugable && device->parent_bus->allow_hotplug" from the property
> >> getter?
> > sure, since there is no bus-less hotlpug it won't hurt.
> > I'll respin amended patch as reply to this thread if it's ok.
> >
> >> Or just set the class to true in the common superclasses of all
> >> buses that support hotplug, but that's a bit harder.
> > perhaps exercise for the later.
> >
> >>
> >> Paolo
> >
> >
>
--
Regards,
Igor
