Re: [Qemu-devel] [PATCH 0/7] spapr: rework memory nodes

[Nishanth Aravamudan <nacc@linux.vnet.ibm.com>]

On 17.06.2014 [16:22:33 -0300], Eduardo Habkost wrote:
> On Tue, Jun 17, 2014 at 11:38:16AM -0700, Nishanth Aravamudan wrote:
> > On 17.06.2014 [11:07:00 -0300], Eduardo Habkost wrote:
> > <snip>
> > > > If it is canonical and kosher way of using NUMA in QEMU, ok, we can use it.
> > > > I just fail to see why we need a requirement for nodes to go consequently
> > > > here. And it confuses me as a user a bit if I can add "-numa
> > > > node,nodeid=22" (no memory, no cpus) but do not get to see it in the guest.
> > > 
> > > I agree with you it is confusing. But before we support that use case,
> > > we need to make sure auto-allocation is handled properly, because it
> > > would be hard to fix it later without breaking compatibility.
> > > 
> > > We probably just need a "present" field on struct NodeInfo, so
> > > machine-specific code and auto-allocation code can differentiate nodes
> > > that are not present on the command-line from empty nodes that were
> > > specified in the command-line.
> > 
> > What/where is struct NodeInfo?
> 
> It was introduced very recently. See the pull request at:
> 
>   From: "Michael S. Tsirkin" <mst@redhat.com>
>   Message-ID: <1403021756-15960-1-git-send-email-mst@redhat.com>
>   Subject: [Qemu-devel] [PULL 000/103] pc, pci, virtio, hotplug fixes, enhancements for 2.1
> 
>   git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream

Ah thank you very much!

Before I get cracking on some patches, wanted to clarify some things:

1) We need something like a "present" field to deal with
auto-allocation, which indicates a user-specified NUMA node.

2) We need something like a "defined" field to indicate which entries
are actually valid and which aren't just 0 because they weren't ever set
in order to support sparse node numbering.
	2a) We could add a defined field to indicate the defined
	entries, iterate over the entire array and skip those not
	defined [keeps index:nodeid mapping, changes all loops]
	2b) We could add a nodeid field to indicate the defined entries,
	iterate over only nb_numa_nodes [breaks index:nodeid, keeps
	loops the same, but requires using the nodeid member in the
	loops, not guaranteed for the array to be sorted on nodeid]

I'm currently in favor of 2b, with perhaps a call to qsort on the array
after parsing to sort by node id? I'd have to audit the users of the
array to make sure they use the nodeid member and not the index, but
that should be straightforward.

These patches would probably need to go in before Alexey's series
(Alexey, I can rebase your patches on top, if you want).

Thanks,
Nish