[PATCH v2 0/4] NUMA CPUs 'auto-fill' for incomplete topologies

Michal Privoznik mprivozn at redhat.com
Thu Jun 18 10:34:27 UTC 2020


On 6/17/20 10:08 PM, Daniel Henrique Barboza wrote:
> 
> 
> On 6/17/20 4:19 PM, Michal Privoznik wrote:
>> On 6/10/20 8:35 PM, Daniel Henrique Barboza wrote:
>>> changes in v2:
>>> - removed patch 5/5
>>>
>>> Gitlab link: https://gitlab.com/danielhb/libvirt/-/tree/vcpus_numa_v2
>>>
>>> v1 link: 
>>> https://www.redhat.com/archives/libvir-list/2020-June/msg00016.html
>>>
>>>
>>> Daniel Henrique Barboza (4):
>>>    numa_conf.c: add helper functions for cpumap operations
>>>    qemu_domain.c: NUMA CPUs auto-fill for incomplete topologies
>>>    qemuxml2xmltest.c: add NUMA vcpus auto fill tests
>>>    formatdomain.html.in: document the NUMA cpus auto fill feature
>>>
>>>   docs/formatdomain.html.in                     | 11 ++++-
>>>   src/conf/numa_conf.c                          | 46 ++++++++++++++++++
>>>   src/conf/numa_conf.h                          |  3 ++
>>>   src/libvirt_private.syms                      |  1 +
>>>   src/qemu/qemu_domain.c                        | 47 +++++++++++++++++++
>>>   src/qemu/qemu_domain.h                        |  4 ++
>>>   src/qemu/qemu_driver.c                        |  9 ++++
>>>   .../numavcpus-topology-mismatch.xml           | 37 +++++++++++++++
>>>   ...avcpus-topology-mismatch.x86_64-latest.xml | 38 +++++++++++++++
>>>   tests/qemuxml2xmltest.c                       |  1 +
>>>   10 files changed, 196 insertions(+), 1 deletion(-)
>>>   create mode 100644 
>>> tests/qemuxml2argvdata/numavcpus-topology-mismatch.xml
>>>   create mode 100644 
>>> tests/qemuxml2xmloutdata/numavcpus-topology-mismatch.x86_64-latest.xml
>>>
>>
>> Patches look good to me.
> 
> Thanks for the review!
> 
>>
>> My only concern is that I plan to introduce vCPU-less NUMA nodes [1] 
>> (because of HMAT [2]). But I guess if user assigns vCPUs to NUMA nodes 
>> fully, then we still can have vCPU-less nodes because your code would 
>> be NOP, right?
> 
> It'll be a NOP because the sum of CPUs in the NUMA topology would be 
> equal to the
> maxcpus declared in <vcpus>
> 
> Now, for the new use case you're going to introduce, you'll need to either
> (1) forbid incomplete NUMA nodes entirely for this case or (2) check how 
> QEMU
> fills in the vcpus in this scenario.
> 
> For (2) my brutal uneducated guess is that the behavior would be 
> similar, but populating
> the first non-cpuless NUMA node (which wouldn't be necessarily node0). 
> This can be
> arranged by creating a function that returns whether a node is cpu-less 
> and using the
> first non-cpuless cpu in the qemuDomainDefNumaCPUsRectify() function 
> (patch 2) instead
> of node0. You'll want to check it with QEMU first (Igor Mammedov 
> perhaps?) to ensure
> that this is what QEMU would do in these cases.
> 
> TBH I believe that cpu-less NUMA nodes is quite an advanced feature and 
> (1) is
> a good approach for that, specially because there is no existing guests 
> in the
> wild that would be impacted by this restriction since Libvirt does not 
> support
> it yet.

Yes, for qemu 2.7+ in qemuValidateDomainDef() if topology is specified 
then we require full vCPU to NUMA assignment. Well, we warn users if it 
is not the case (see QEMU_CAPS_QUERY_HOTPLUGGABLE_CPUS check and code 
around).


Anyways, as I said your code is okay (I'm fixing couple of small nits) 
and pushing.

Reviewed-by: Michal Privoznik <mprivozn at redhat.com>

Do you think it's worth documenting in the release notes too?

Michal




More information about the libvir-list mailing list