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

[Paolo Bonzini]

Il 17/12/2013 20:38, Anthony Liguori ha scritto:
> On Tue, Dec 17, 2013 at 4:38 AM, Paolo Bonzini <address@hidden> 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.
> 
> 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".

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 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