On Mon, May 04, 2015 at 05:53:59PM +0200, Paolo Bonzini wrote: > > > On 04/05/2015 16:05, Eduardo Habkost wrote: > > On Mon, May 04, 2015 at 03:19:32PM +0200, Paolo Bonzini wrote: > >> > >> > >> On 04/05/2015 15:16, Igor Mammedov wrote: > >>>>> Can we use the APIC id then? Perhaps wrapped with a CPUState-level > >>>>> method get_stable_processor_id()? > >>> We have CPUClass->get_arch_id() which results in APIC id for > >>> target-i386. > >>> But I'd rather see an arbitrary DEVICE->id as index/name, that way > >>> when -device cpu-foo,id=cpuXXX becomes functional we would have > >>> 1:1 mapping between CLI and /machine/cpus/ view. > >> > >> CPUs would already be available at /machine/peripheral. I think aliases > >> should provide alternative indexing whenever possible---not simply > >> filter by device type. > > > > [1] Is there anybody or any document that can explain to me what all the > > containers inside /machine mean? I see /machine/peripheral, > > /machine/peripheral-anon, /machine/unattached, here, and I don't > > know what they mean. > > /machine/peripheral/XYZ holds devices created with -device id=XYZ
So, if you know the ID of the CPU, we already have an easy mechanism to look it up, and having device ID on /machine/cpus would be pointless (and make the solution more complex because today the CPUs don't have any device ID set). > > /machine/peripheral-anon/device[NN] holds devices created without an id > > /machine/unattached holds devices created by the board and not added > elsewhere through object_property_add_child. > > > Could you clarify what you mean by "alternative indexing"? > > A way to lookup devices of a particular kind. An example of > "alternative indexing" is using pci.0/child[NN] to look up children of > the first PCI bus. > > > All I am trying to provide right now is having a predictable path for > > CPUs, it doesn't matter if using -device, device_add, -smp, or cpu-add. > > Filtering by device type is not what I need, here. > > Ok, so we're on the same page. I would use any of: > > - /machine/cpus/NN (your choice) > > - /machine/cpu[NN] (Peter's choice) > > - /machine/cpus/cpu[NN] (hybrid, resembles /machine/peripheral-anon or > /machine/unattached more) > > I'm not sure if "NN" should be a random progressive number (in that case > you can use cpu[*] to let the QOM core pick the number) or the APIC ID. > You know the domain better than I do. You made a good point below, which make me want to use the APIC ID: > > > Making the path depend on guest-visible bits that can change depending > > on the architeture or machine would make the path less predictable. > > You can still list all children of /machine/cpus. That's true, and that's enough for clients that don't want/need to be aware of the arch-specific ID. Most clients should treat the QOM path as an arbitrary string, and in this case get_arch_id() is the simplest (and more well-behaved) identifier we have. > The disadvantage of > the APIC ID is that IDs may have holes even without doing any > hot-unplug; the advantage is that, from a set of online CPUs in the > guest, you can predict the paths in /machine/cpus. The other disavantage I was worrying about is that it doesn't let clients predict the full QOM path of a CPU without knowing how to calculate the arch-specific ID. But this is solved by simply listing all children of /machine/cpus. > > With cpu[*] instead you can have different contents of /machine/cpus > after for example > > cpu_add 3 # adds /machine/cpus/cpu[2] pointing to CPU 3 > cpu_add 2 # adds /machine/cpus/cpu[3] pointing to CPU 2 > > vs. > > cpu_add 2 # adds /machine/cpus/cpu[2] pointing to CPU 2 > cpu_add 3 # adds /machine/cpus/cpu[3] pointing to CPU 3 > > > > I have an alternative patch that simply adds a "qom-path" field to > > query-cpus. If we find out that making commitments about QOM paths is > > too hard, I can submit it instead. > > I don't think it's too hard, but this alternative patch may also make sense. Well, the patch may be useful even with predictable paths: query-cpus doesn't show the APIC ID, so adding a "qom-path" field would be useful to correctly match QOM objects and information from query-cpus. And if we add the qom-path field to query-cpus, we don't have the immediate need for predictable QOM paths for CPUs anymore. Maybe we should forget about /machine/cpus (because it will be osboleted by topology aware CPU enumeration mechanisms in the future) and just apply the qom-path patch. -- Eduardo
