[PATCH v2 0/4] NUMA CPUs 'auto-fill' for incomplete topologies
Daniel Henrique Barboza
danielhb413 at gmail.com
Thu Jun 18 11:13:59 UTC 2020
On 6/18/20 7:34 AM, Michal Privoznik wrote:
> 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  (because of HMAT ). 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.
Thanks for the tweaks before before pushing!
> Reviewed-by: Michal Privoznik <mprivozn at redhat.com>
> Do you think it's worth documenting in the release notes too?
In the "Improvements" section perhaps. I just sent a patch.
More information about the libvir-list