From e1c8866163a2861f79a08ce4e1487fc84b7d1ec1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= Date: Fri, 29 Oct 2021 12:45:48 +0100 Subject: [PATCH] virtinst: validate that the CPU topology is sane MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The product of sockets * dies * cores * threads must be equal to the vCPU count. While libvirt and QEMU will report this error scenario, it makes sense to catch it in virt-install, so we can test our local logic for setting defaults for topology. This exposes some inconsistent configurations in the test suite. Signed-off-by: Daniel P. Berrangé --- .../cli/compare/virt-install-singleton-config-1.xml | 2 +- .../cli/compare/virt-install-singleton-config-3.xml | 2 +- tests/test_cli.py | 4 ++-- tests/test_misc.py | 12 ++++++++++-- virtinst/domain/cpu.py | 12 ++++++++++++ 5 files changed, 26 insertions(+), 6 deletions(-) diff --git a/tests/data/cli/compare/virt-install-singleton-config-1.xml b/tests/data/cli/compare/virt-install-singleton-config-1.xml index 05e80f3a8..cf8f0eb6c 100644 --- a/tests/data/cli/compare/virt-install-singleton-config-1.xml +++ b/tests/data/cli/compare/virt-install-singleton-config-1.xml @@ -37,7 +37,7 @@ Broadwell Intel - + diff --git a/tests/data/cli/compare/virt-install-singleton-config-3.xml b/tests/data/cli/compare/virt-install-singleton-config-3.xml index 0dc7b78ce..f903c2c98 100644 --- a/tests/data/cli/compare/virt-install-singleton-config-3.xml +++ b/tests/data/cli/compare/virt-install-singleton-config-3.xml @@ -12,7 +12,7 @@ - 4 + 6 Acme LLC diff --git a/tests/test_cli.py b/tests/test_cli.py index 4cc831da4..4d9070c26 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -467,7 +467,7 @@ c = vinst.add_category("xml-comparsion", "--connect %(URI-KVM)s --noautoconsole c.add_compare(""" --memory 1024 --uuid 12345678-12F4-1234-1234-123456789AFA ---vcpus 4,cores=2,threads=2,dies=1,sockets=2 --cpuset=1,3-5 +--vcpus 4,cores=2,threads=1,dies=1,sockets=2 --cpuset=1,3-5 --cpu host-copy --description \"foobar & baz\" --boot uefi,smbios_mode=emulate,boot1.dev=hd,boot.dev=network,initarg1=bar=baz,initarg=foo @@ -573,7 +573,7 @@ memnode0.cellid=1,memnode0.mode=strict,memnode0.nodeset=2 # Test the implied defaults for gl=yes setting virgl=on c.add_compare(""" ---vcpus vcpu.current=3,maxvcpus=4,vcpu.placement=auto +--vcpus vcpu.current=3,maxvcpus=6,vcpu.placement=auto --memory hotplugmemorymax=2048,hotplugmemoryslots=2 --disk none --features apic.eoi=off,hap=on,hyperv.synic.state=on,hyperv.reset.state=off,hyperv.spinlocks.state=on,hyperv.spinlocks.retries=5678,pae=on,pmu.state=on,pvspinlock.state=off,smm.state=off,viridian=on,vmcoreinfo.state=on,vmport.state=off,kvm.hidden.state=on,hyperv.vapic.state=off,hyperv.relaxed.state=off,gic.version=host,kvm.hint-dedicated.state=on,kvm.poll-control.state=on,ioapic.driver=qemu diff --git a/tests/test_misc.py b/tests/test_misc.py index 01f650c37..9b8c95efe 100644 --- a/tests/test_misc.py +++ b/tests/test_misc.py @@ -42,14 +42,22 @@ def test_misc_cpu_topology(): cpu = virtinst.DomainCpu(conn) cpu.topology.cores = "4" - cpu.set_topology_defaults(9) + cpu.set_topology_defaults(8) assert get_top(cpu) == [2, 1, 4, 1] cpu = virtinst.DomainCpu(conn) cpu.topology.threads = "3" - cpu.set_topology_defaults(14) + cpu.set_topology_defaults(12) assert get_top(cpu) == [4, 1, 1, 3] + cpu = virtinst.DomainCpu(conn) + cpu.topology.threads = "3" + try: + cpu.set_topology_defaults(14) + assert False, "Topology unexpectedly validated" + except ValueError: + pass + cpu = virtinst.DomainCpu(conn) cpu.topology.sockets = 5 cpu.topology.cores = 2 diff --git a/virtinst/domain/cpu.py b/virtinst/domain/cpu.py index 16f7a5888..f2d0c79f0 100644 --- a/virtinst/domain/cpu.py +++ b/virtinst/domain/cpu.py @@ -41,6 +41,18 @@ class _CPUTopology(XMLBuilder): if not self.threads: self.threads = vcpus // self.total_vcpus() + if self.total_vcpus() != vcpus: + raise ValueError(_("Total CPUs implied by topology " + "(sockets=%(sockets)d * dies=%(dies)d * cores=%(cores)d * threads=%(threads)d == %(total)d) " + "does not match vCPU count %(vcpus)d") % { + "sockets": self.sockets, + "dies": self.dies, + "cores": self.cores, + "threads": self.threads, + "total": self.total_vcpus(), + "vcpus": vcpus, + }) + return def total_vcpus(self):