Re: [Qemu-devel] [RFC 4/8] qdev: link based hotplug

[Paolo Bonzini]

Il 21/03/2014 11:35, Igor Mammedov ha scritto:
> On Thu, 20 Mar 2014 19:03:57 +0100
> Paolo Bonzini <address@hidden> wrote:
> 
>> Il 20/03/2014 17:20, Igor Mammedov ha scritto:
>>>>>
>>>>> What about just looking up on the QOM tree until you find a
>>>>> HotplugHandler, if the device doesn't have a bus or the bus doesn't have
>>>>> a hotplug handler link itself?  This is similar to how FWPathProvider 
>>>>> works.
>>> it does so "hotplug_handler_get_from_path()",
>>> above just provides option to specify lookup path. See 6/8 where PC board
>>> allocates links and sets its own board specific path for generic DimmDevice.
>>
>> Yeah, but I'm not sure why you need the links.  Why can't you just start 
>> from the canonical path, such as /machine/peripheral/dimm-0 for -device 
>> dimm,id=dimm-0, and walk up from there?
> That would work in this particular case since /machine is hotplug handler,
> but in generic case hotplug handler can be located somewhere else in QOM tree
> i.e. it isn't a parent of hotplugged device.

Yes, it removes more flexibility than I thought.  Hotplugged devices 
are by definition at /machine/peripheral, so there's not much "walking 
up" that we can do.

However...

> Allowing to customize lookup path via DeviceClass.hotplug_path() makes
> wiring hotplug handler flexible. For example:
>  * A target that decides to use DimmDevice cloud have hotplug handler
>    located at /machine/peripheral/ec_foo, walking down from
>    /machine/peripheral/dimm-0 or /machine/unassigned/dimm-0 (in hotplug case)
>    won't allow to find  hotplug handler. But overriding 
> DeviceClass.hotplug_path()
>    the target machine could provide means to locate necessary hotplug handler

... since this is meant for "monkeypatching" in the target machine 
(which we don't really do elsewhere, do we?), perhaps /machine itself 
could grow a get_hotplug_handler() or get_hotplug_path() method?

The machine object then can return itself if you want to implement 
HotplugHandler in /machine, or it could return the PM device, or some 
other controller.

Or even simpler (perhaps too simple) you could just check if /machine 
implements HotplugHandler if the hotplug device is busless.

> I've added link<>s as an attempt to visualize Andreas' idea to use them for
> hotplug and mgmt.

Yes, links for hotplug/management are a nice idea.  I think however
we're still in an early phase of that, and we've already made memory 
hotplug depend on a lot of infrastructure!

> leaves are link<>s which are prebuilt at startup and set when device
> is added. It provides an easy to enumerate interface for mgmt and also
> gives us a quite informative path that encodes topology and later
> we could use it instead of custom properties. For example:
> 
>   device_add x86cpu,path=/machine/node[1]/cpu[0]/core[3]/thread[2]
> vs
>   device_add x86cpu,apic-id=[who knows how it's calculated]
> 
> or
>   device_add dimm,path=/machine/node[0]/dimm[5]
> vs
>   device_add dimm,node=0,slot=5
> 
> i.e. being added device could decode all needed information from above
> provided path instead of creating a bunch of custom properties.

So "path" would mean "look for a link there and set it to myself".
In turn the link setter would take care of setting the device's
address based on the property name, as well as (if applicable)
adding the device on the bus.

It's a nice alternative to the bus+addr, and one we could consider to 
move device creation to -object.  Anthony in the past had mentioned 
something like

    -object dimm,id=foo
    -set machine/node[0].dimm[5]=foo
    -set dimm.realize=true

I think I like your proposal better.

We now have moved towards UserCreatable instead of setting magic 
properties manually, and I think "path" fits in the "UserCreatable" 
scheme better than "-set".

It could be extended to PCI, as basically a version of bus+addr
with QOM paths:

    -object e1000,id=e1000,path=/machine/pci[0]/slot[03.0]

Here is an example of configuring a PCIe switch with relative paths:

    -object ioh3420,id=port4,path=pci[0]/slot[1c.0]
    -object x3130-upstream,id=up,path=port4/pci[0]/slot[00.0],chassis=1
    -object 
xio3130-downstream,id=down0,multifunction=on,path=up/pci[0]/slot[00.0],chassis=2
    -object 
xio3130-downstream,id=down1,multifunction=on,path=up/pci[0]/slot[00.1],chassis=3
    -object e1000,id=e1000,path=down0/pci[0]/slot[00.0]

It's a bit verbose.  It doesn't allow for automatically attributing
slots within a bus, which is the main drawback compared to bus+addr.
Quite frankly I'm not sure I would like it as a user, even though it's
likely superior for management and for complicated configurations
such as the above PCIe example.

In the past we stalled on how to create the properties, since there is 
the problem of requiring pre-creation of links.  On SCSI you would have 
thousands of links.

Since interfaces are fancy now, perhaps we could use them here too 
(DynamicPropertyCreator?).  object_property_find would pass missing 
property names to the interface if implemented, and the object would 
use it to validate dynamic property names and create them dynamically.

Thanks for throwing up these ideas.  Even if we end up with a vastly 
simplified mechanism for memory hotplug, it's good to  get a fresh
view on old concepts!

Paolo