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@xxxxxxxxxx>
Do you think it's worth documenting in the release notes too?
Michal
