Re: [Qemu-devel] [PATCH] cpu: Register QOM links at /machine/cpus/<index>

[Eduardo Habkost <ehabkost@redhat.com>]

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