From 9a578e1ac58eccd7b7a8123ce7fe22cbf8080b3a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= Date: Fri, 29 Oct 2021 13:27:35 +0100 Subject: [PATCH] virtinst: prefer cores when exposing topology to the guest MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In real world silicon though it is rare to have high socket/die counts, but common to have huge core counts. Some OS will even refuse to use sockets over a certain count. Thus we prefer to expose cores to the guest rather than sockets as the default for missing fields. This matches a recent change made in QEMU for new machine types commit 4a0af2930a4e4f64ce551152fdb4b9e7be106408 Author: Yanan Wang Date: Wed Sep 29 10:58:09 2021 +0800 machine: Prefer cores over sockets in smp parsing since 6.2 Closes: https://github.com/virt-manager/virt-manager/issues/155 Signed-off-by: Daniel P. Berrangé --- man/virt-install.rst | 6 ++++-- tests/test_misc.py | 4 ++-- virtinst/domain/cpu.py | 17 ++++++++++++++--- 3 files changed, 20 insertions(+), 7 deletions(-) diff --git a/man/virt-install.rst b/man/virt-install.rst index c51303cc..85070314 100644 --- a/man/virt-install.rst +++ b/man/virt-install.rst @@ -290,8 +290,10 @@ the guest will be able to hotplug up to MAX vcpus while the guest is running, but will startup with VCPUS. CPU topology can additionally be specified with sockets, dies, cores, and threads. -If values are omitted, the rest will be autofilled preferring sockets over -cores over threads. +If values are omitted, the rest will be autofilled preferring cores over sockets +over threads. Cores are preferred because this matches the characteristics of +modern real world silicon and thus a better fit for what guest OS will be +expecting to deal with. 'cpuset' sets which physical cpus the guest can use. ``CPUSET`` is a comma separated list of numbers, which can also be specified in ranges or cpus diff --git a/tests/test_misc.py b/tests/test_misc.py index 9b8c95ef..25e12f94 100644 --- a/tests/test_misc.py +++ b/tests/test_misc.py @@ -38,7 +38,7 @@ def test_misc_cpu_topology(): cpu = virtinst.DomainCpu(conn) cpu.topology.dies = "3" cpu.set_topology_defaults(9) - assert get_top(cpu) == [3, 3, 1, 1] + assert get_top(cpu) == [1, 3, 3, 1] cpu = virtinst.DomainCpu(conn) cpu.topology.cores = "4" @@ -48,7 +48,7 @@ def test_misc_cpu_topology(): cpu = virtinst.DomainCpu(conn) cpu.topology.threads = "3" cpu.set_topology_defaults(12) - assert get_top(cpu) == [4, 1, 1, 3] + assert get_top(cpu) == [1, 1, 4, 3] cpu = virtinst.DomainCpu(conn) cpu.topology.threads = "3" diff --git a/virtinst/domain/cpu.py b/virtinst/domain/cpu.py index f2d0c79f..f6f9eabb 100644 --- a/virtinst/domain/cpu.py +++ b/virtinst/domain/cpu.py @@ -29,15 +29,26 @@ class _CPUTopology(XMLBuilder): # While `dies` is optional and defaults to 1 if omitted, # `sockets`, `cores`, and `threads` are mandatory. def set_defaults_from_vcpus(self, vcpus): + # The hierarchy is sockets > dies > cores > threads. + # + # In real world silicon though it is rare to have + # high socket/die counts, but common to have huge + # core counts. + # + # Some OS will even refuse to use sockets over a + # a certain count. + # + # Thus we prefer to expose cores to the guest rather + # than sockets as the default for missing fields + if not self.cores: + self.cores = vcpus // self.total_vcpus() + if not self.sockets: self.sockets = vcpus // self.total_vcpus() if not self.dies: self.dies = 1 - if not self.cores: - self.cores = vcpus // self.total_vcpus() - if not self.threads: self.threads = vcpus // self.total_vcpus()