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
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 -- 2.26.2
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. 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? Michal 1: https://gitlab.com/MichalPrivoznik/libvirt/-/tree/hmat 2: https://www.redhat.com/archives/libvir-list/2020-January/msg00422.html
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. Thanks, DHB > > Michal > > 1: https://gitlab.com/MichalPrivoznik/libvirt/-/tree/hmat > 2: https://www.redhat.com/archives/libvir-list/2020-January/msg00422.html >
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@redhat.com> Do you think it's worth documenting in the release notes too? Michal
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 [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. Thanks for the tweaks before before pushing! > > Reviewed-by: Michal Privoznik <mprivozn@redhat.com> > > Do you think it's worth documenting in the release notes too? In the "Improvements" section perhaps. I just sent a patch. Thanks, DHB > > Michal >
© 2016 - 2024 Red Hat, Inc.