On 09/11/2018 04:28 PM, Peng Hao wrote:

...

For interleave mode,sometimes we want to allocate mmeory regularly in all nodes on the host. But different hosts has different node number. So we add nodeset='all' for interleave mode and if nodeset=NULL default nodeset is 'all' for interleave mode.

Signed-off-by: Peng Hao <peng.hao2@zte.com.cn> --- src/conf/numa_conf.c | 73 ++++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 57 insertions(+), 16 deletions(-)

Firstly, this patch does not pass 'syntax-check'. Secondly, it breaks qemuxml2argvtest.

I will pay attention to this.

...

+ numa->memory.allnode = true; + } else { Any patch that changes accepted XML needs to go hand in hand with documentation and RNG update and a test case. I will add next.

...

+ if (placement == VIR_DOMAIN_NUMATUNE_PLACEMENT_STATIC && + mode == VIR_DOMAIN_NUMATUNE_MEM_INTERLEAVE && + numa->memory.allnode) { + if ((bitmap = virBitmapNew(VIR_DOMAIN_CPUMASK_LEN)) == NULL) + goto cleanup; + virBitmapClearAll(bitmap); + maxnode = numa_max_node();

So, you're including numa.h to get this function. What if:

a) numa.h is not available? b) what is wrong with virNumaGetMaxNode()?

...

+ for (i = 0; i <= maxnode; i++) { + if (virBitmapSetBit(bitmap, i) < 0) { + virBitmapFree(bitmap); + goto cleanup; + } + } + if (numa->memory.nodeset) + virBitmapFree(numa->memory.nodeset); + numa->memory.nodeset = bitmap; }

/* setting nodeset when placement auto is invalid */

But more importantly, why is this patch needed? I might be missing something, but:

a) you can just not pin the memory to avoid mismatch of NUMA nodes on migration, b) supply new domain XML on migration where NUMA nodes match the destination

Isn't pinning memory to all NUMA nodes equivalent to no pinning at all? I would use 'interlaeve' to let virtual machine's memory distribute evenly in all nodes. And 'interleave' setting ask for providing 'nodeset'. I think it is not so convenient. Michal