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

[Igor Mammedov <imammedo@redhat.com>]

On Fri, 21 Mar 2014 12:45:01 +0100
Paolo Bonzini <pbonzini@redhat.com> wrote:

> Il 21/03/2014 11:35, Igor Mammedov ha scritto:
> > On Thu, 20 Mar 2014 19:03:57 +0100
> > Paolo Bonzini <pbonzini@redhat.com> 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?
Ok for memory hotplug, I'll try to add and use get_hotplug_handler() method
to /machine as a minimal solution to the problem.

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