On Fri, 21 Mar 2014 12:45:01 +0100 Paolo Bonzini <pbonz...@redhat.com> wrote:
> Il 21/03/2014 11:35, Igor Mammedov ha scritto: > > On Thu, 20 Mar 2014 19:03:57 +0100 > > Paolo Bonzini <pbonz...@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
