Re: [Qemu-devel] [PATCH 00/11 v3] Refactor PCI/SHPC/PCIE hotplug to use a more generic hotplug API

[Igor Mammedov <imammedo@redhat.com>]

On Wed, 18 Dec 2013 11:36:52 +0100
Paolo Bonzini <pbonzini@redhat.com> wrote:

> Il 17/12/2013 20:38, Anthony Liguori ha scritto:
> > On Tue, Dec 17, 2013 at 4:38 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> >> Il 17/12/2013 00:26, Anthony Liguori ha scritto:
> >>> Sharing hot plug code is a good thing.  Making hotplug a qdev-level
> >>> concept seems like a bad thing to me.
> >>
> >> Can you explain what you mean?
> > 
> > The question is whether "hotpluggable" as a property applies to all
> > devices or not.
I think Andreas asked me to provide "hotpluggable" property to
distinguish hotpluggable vs not hotpluggable DimmDevice via qom interface.

> > 
> > But hotplug is strictly a bus level concept.  It's a sequence of
> > events that correspond to what happens when you add a new device to a
> > bus after power on.
> 
> Hotplugging a device is a special case of plugging a device.  If a bus
> or device only supports cold-plug, that can be done using
> "bc->allow_hotplug = false" or "dc->hotpluggable = false".
Do we need per instance ability to set "hotpluggable" property?
For example board might want to mark some CPUs as not hotpluggable.

> 
> Igor's interface applies just as well to the case of plugging a device
> at startup; I think separating the two makes little sense.  And once you
> have cold-plug and hot-plug in qdev core, it makes sense to add unplug
> as well.  Also because we already have surprise removal in qdev core
> (that's unparent) and we have some kind of unplug request support
> (device_del/dc->unplug).
> 
> One possibility that remains is to put cold/hot-plug in a "BusDevice"
> class rather than in the core qdev:
> 
>     Device
>       BusDevice    <-- can be cold/hot-plugged
> 
> but this adds more complication.  For example, the same CPU can be
> hotpluggable or not depending on the board model, should the superclass
> be Device or BusDevice.  And if we ever have multi-CPU targets, with the
> "core" CPU not hotpluggable and additional hotpluggable ones (e.g. for
> GPUs) what would be the superclass of the common CPU superclass?
> 
> > The question is whether there can be code sharing without touching the
> > base class.  You could certainly have a HotpluggableBusState and then
> > a HotpluggableDeviceState.
> > 
> > Interfaces would be another option too.
> 
> Interfaces are fine, but the question is who finds them and calls them.
>  In this case, the discovery mechanism is a link property, and the
> calling mechanism is an explicit hook in the "realized" property.
If we don't need per instance "hotpluggable" state and we can call
interfaces from generic qdev/device code, then we would need at first
only TYPE_HOTPLUGGABLE_BUS_DEVICE_IF and later for link<> based hotplug
we could add just TYPE_HOTPLUGGABLE_DEVICE_IF. Difference would be in
the way they get access to hotplug device link, former one will use bus
for it and second some other way.

> 
> If we had aspect-oriented programming, we would be marking join points
> instead of writing "if (dev->foo) bar(dev->foo)" conditionals.  But the
> idea is the same.
> 
> > The general concern is about polluting widely used base classes.  It's
> > better if we can avoid adding things to DeviceState and Object
> > whenever possible.
> 
> I agree.  At the same time we should make base classes as small as
> possible, but not smaller than that.
> 
> Paolo
>