On Fri, Aug 19, 2016 at 13:10:09 -0400, John Ferlan wrote:
On 08/19/2016 10:38 AM, Peter Krempa wrote:
As of qemu commit: commit a32ef3bfc12c8d0588f43f74dcc5280885bbdb30 Author: Thomas Huth <thuth@redhat.com> Date: Wed Jul 22 15:59:50 2015 +0200
vl: Add another sanity check to smp_parse() function
v2.4.0-952-ga32ef3b
configuration where the maximum CPU count doesn't match the topology is rejected. Prior to that only configurations where the topology would contain more cpus than the maximum count would be rejected.
Use QEMU_CAPS_QUERY_HOTPLUGGABLE_CPUS as a relevant recent enough witness to avoid breaking old configs. ---
Notes: v2: -fixed typo in commit message - already ACKed
src/qemu/qemu_domain.c | 38 ++++++++++++++++++++++++++++++++------ 1 file changed, 32 insertions(+), 6 deletions(-)
[...]
+ /* qemu as of 2.5.0 rejects SMP topologies that don't match the cpu count */ + if (def->cpu && def->cpu->sockets) { + topologycpus = def->cpu->sockets * def->cpu->cores * def->cpu->threads; + if (topologycpus != virDomainDefGetVcpusMax(def)) { + /* presence of query-hotpluggable-cpus should be a good enough witness */ + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_HOTPLUGGABLE_CPUS)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("CPU topology doesn't match maximum vcpu count")); + goto cleanup; + }
Would it be a good idea to add a VIR_WARN or VIR_INFO (something) that will give a heads up that the configuration may prevent some future start? Especially since you went through all the math before detecting the capability...
Well, adding a warning would be cerainly easy, but as it was discussed earlier, using VIR_WARN or VIR_INFO doesn't make much sense since it's improbable that the user will be reading logs after defining a VM. Peter