Am 08.08.2013 05:10, schrieb Alexey Kardashevskiy: > On 08/08/2013 12:22 AM, Andreas Färber wrote: >> Setting a canonical path is done via object_property_add_child() after >> instantiating and before realizing a device. >> For x86 the northbridge is used as name, i.e. "i440fx" for pc and "q35" >> for q35. Suggest to discuss with Anthony how to structure the >> composition tree for sPAPR. > > I tried inserting this: > object_property_add_child(qdev_get_machine(), busname, OBJECT(dev), NULL); > > in TYPE_SPAPR_PCI_HOST_BRIDGE's spapr_phb_init() (I thought this is what > i440fx_init() does in the very beginning) but when I do that, spapr_phb > already has a parent of a "container" type so assert happens.
See above: After instantiating (instance_init) and before realizing (realize/init). It needs to be done by the parent, not by the device itself - unless I'm completely misunderstanding what you're trying, in absence of code to look at. > What is the aim of all of this? Is it not to have "unattached" in a path > starting with /machine? Correct. To have a canonical path for management tools, qtest, etc. similar to /sys filesystem in Linux. > btw I have added notes in the previous response against splitting ICS > initialization into 2 functions and you skipped it. Does this mean you do > not want to waste more of your time persuading me and this is the real > stopped or you just gave up? :) Thanks. "Never give up, never surrender!" :P I believe I had already answered to that: I have no clue what ICS, ICP, XICS acronyms actually stand for, but I explained friendly and verbose why doing QOM things the way I asked you to makes sense. Ultimately I was told by Anthony when and how to do these things, and I do not have all day to argue with you, so if you don't like my feedback then please discuss with your IBM colleague directly. There is by definition a functional split between instance_init and realize, not my invention, and management tools should be able to tweak properties of objects, such as you setting nr_servers to 1 on machine init and QMP qom-set bumping it to 2. It is IMO not essential to implement that from the start because having a performant interrupt implementation is more important than our QOM'ish refactorings, therefore I said error_setg() should be okay, but my feedback is targeted at keeping our design future-proof, which means that instantiation of a single object that was in instance_init before should not be moved into a property setter that may be called multiple times because it would then need to be moved back anyway. Use of object_initialize() was requested by Anthony to have the QOM composition reflected in the allocation model, i.e. in this case g_try_malloc0(sizeof(XICSState)) including the ICS(?) thingy. So since it was created once independent of properties before your patch, I assume we should keep that behavior, while only changing the storage location. Still a whole week left for cleaning up 1.7 patches! :) Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
