On Thu, Apr 17, 2014 at 3:20 AM, Andreas Färber <afaer...@suse.de> wrote: > Am 15.04.2014 23:39, schrieb Peter Crosthwaite: >> On Wed, Apr 16, 2014 at 2:16 AM, Andreas Färber <afaer...@suse.de> wrote: >>> Am 15.04.2014 04:21, schrieb Peter Crosthwaite: >>>> So clients can set the top level id string. >>>> >>>> Signed-off-by: Peter Crosthwaite <peter.crosthwa...@xilinx.com> >>> >>> Anthony had nack'ed Paolo's attempt to generalize the qdev id to QOM, so >>> I'm not sure if we should really do this even if just on device level. >>> The id= is used to construct the canonical path, and we can't change >>> that through a qdev setter. >> >> How can we change it? The problem I am trying to solve, is getting >> meaningful device instance names instead of the anonymous defaults. >> This includes in the canonical path. I am completely open to proposals >> :) > > Possibly this is just a mixup, see below. On yesterday's KVM call > "instance names" referred to VMState names and instance_ids and > qdev/qbus paths. Then there's the device IDs that this patch was > touching. And there's the canonical QOM composition tree paths, that for > user-created devices may include the device ID. > >> Using a dynamic property might allow us to >>> unparent while keeping a reference and then add it as a child<> again, >>> but that still feels awkward. >> >> Eww. >> >> Having it as a getter only seem more >>> secure and predictable. >>> >> >> Sure, once it's committed to the canonical path it definitely needs a >> read-only semantic. > >> Shouldn't be hugely different to the >> writable-until-realize semantic of qdev props though? > > It is to some degree. The canonical paths get set up as part of or right > after instance_init. Only legacy devices without canonical path get a > path as part of realize, but that needs to be fixed for recursive > realization: Devices need to be accessible somewhere in the tree to be > user-modified via qom-set, and they will need to be in the tree to be > realized. > >>> Another issue is bus naming. If supplied NULL, the bus will be based on >>> ID. If the ID changes, bus naming will be inconsistent. >>> >> >> Or rather - what the user intends. If the board has 2 USB busses named >> "foo" and "bar", then those should be both the canonical path and bus >> names. Rather than the homogenised default names "usb0", "usb1". > > Let's not discuss this here again, it's being discussed between ppc and > libvirt already. If the device supplies a hardcoded bus name then (while > it may not be unique) we don't have to care in the context of IDs here. > The interesting case is if we create a device with id=foo, and the bus > uses NULL as indicated above, then it will get foo.0, foo.1, etc. unless > I've missed something. IDs are unique, so if you have only one bus per > device you get foo.0 and bar.0; if a device author chooses not to use > NULL as bus name and rolls their own then we can't help them on the > DeviceState level. > >>> Why would clients set the ID after having chosen a different ID >> >> I'm lost here - what is the mechanism by which you can validly set >> per-instance IDs? > > Sorry, maybe I don't fully get your question... > > Either the user specifies ,id=foo on command line or HMP, or she > doesn't. The corresponding mechanism in config files is [device "foo"], > see docs/q35-chipset.cfg for an example. > > qdev paths used for VMState currently still depend on a bus, which Alex > or Alexey wanted to look into fixing. By default -1 is supplied by > DeviceState for registering dc->vmsd, which can be overridden by > inlining the corresponding qdev.c code or more conveniently - Alex' > suggestion yesterday - by moving that default value of -1 into an > overrideable instance field. > > As for canonical QOM paths, it's the job of machines and devices to set > up those - unless a peripheral device is added by the user, in which > case see previous explanations by Markus and me. > > You might remember my big MPCore refactoring? That was supposed to be > the building block for you to create a proper SoC container device for > Zynq and thereby constructing meaningful canonical paths. I.e., a > ZedBoard or MicroZed or Parallella board instantiates the SoC as > /machine/zynq, the Zynq SoC makes available /machine/zynq/cortex or > whatever (-> PMM; recursively ./scu etc.) plus memory-controller (this > series), uart and whatever Zynq peripherals.
I am aware of this and has been on my radar for quite some time. You can't expect me to > dictate paths for each model, that's no generic task; you need to take > care of naming your favorite device models yourself so that *no* > /machine/unassigned/device[n] remains. This may involve, such as in > Anthony's case of i440fx or in case of function-driven Exynos/Zynq/... > code, refactoring parts of devices into child<> devices for aggregation > rather than just assigning names to existing devices on /machine. > > BTW that's also why - e.g. in the Xilinx context where you discussed > inlining with Edgar - having qdev-style void create-foo wrappers hiding > the object creation is undesirable since, however wrapped into helpers, > we need to access the Object for adding as child<> property from its > caller for uniqueness, usually. > My FWIW my motivations were different there but I see the point. > Hope that explains all device naming. > > Now, the unanswered question is what use case do you need meaningful > names for? Command line? QOM tree? Migration? Something else? > I guess the main one (in the context of this series) is propagation of a meaningful name to the memory region instantiator. It will be something of a regression if instead of the MR names being "zynq.ram" and "zynq.ocm" they become homogenized to "ram", "ram". I could just work around this by adding a memory-region-name property specific to my new device (see P2), but this soln. seemed more concise as the one name is now shared and consistent between qtree and mtree. Regards, Peter > Regards, > Andreas > > -- > SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany > GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg >
