[libvirt] [PATCH 0/2] Sanitize vcpu topology checking
See patch 1. Peter Krempa (2): conf: Sanitize cpu topology numbers qemu: Reuse virDomainDeGetVcpusTopology to calculate total vcpu count src/conf/domain_conf.c | 39 +++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 2 ++ src/libvirt_private.syms | 1 + src/qemu/qemu_domain.c | 18 ++++++++---------- src/qemu/qemu_driver.c | 14 ++++++-------- 5 files changed, 56 insertions(+), 18 deletions(-) -- 2.10.0
Make sure that the topology results into a sane number of cpus (up to UINT_MAX) so that it can be sanely compared to the vcpu count of the VM. Additionally the helper added in this patch allows to fetch the total number the topology results to so that it does not have to be reimplemented later. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1378290 --- src/conf/domain_conf.c | 39 +++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 2 ++ src/libvirt_private.syms | 1 + 3 files changed, 42 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index f562323..44752b9 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1570,6 +1570,42 @@ virDomainDefGetVcpuPinInfoHelper(virDomainDefPtr def, } +/** + * virDomainDeGetVcpusTopology: + * @def: domain definition + * @maxvcpus: optionally filled with number of vcpus the domain topology describes + * + * Calculates and validates that the vcpu topology is in sane bounds and + * optionally returns the total number of vcpus described by given topology. + * + * Returns 0 on success, 1 if topology is not configured and -1 on error. + */ +int +virDomainDefGetVcpusTopology(const virDomainDef *def, + unsigned int *maxvcpus) +{ + unsigned long long tmp; + + if (!def->cpu || def->cpu->sockets == 0) + return 1; + + tmp = def->cpu->sockets; + + /* multiplication of 32bit numbers fits into a 64bit variable */ + if ((tmp *= def->cpu->cores) > UINT_MAX || + (tmp *= def->cpu->threads) > UINT_MAX) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("cpu topology results in more than %u cpus"), UINT_MAX); + return -1; + } + + if (maxvcpus) + *maxvcpus = tmp; + + return 0; +} + + virDomainDiskDefPtr virDomainDiskDefNew(virDomainXMLOptionPtr xmlopt) { @@ -4786,6 +4822,9 @@ virDomainDefValidateInternal(const virDomainDef *def) if (virDomainDefCheckDuplicateDiskInfo(def) < 0) return -1; + if (virDomainDefGetVcpusTopology(def, NULL) < 0) + return -1; + return 0; } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index a70bc21..54c9502 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2488,6 +2488,8 @@ virBitmapPtr virDomainDefGetOnlineVcpumap(const virDomainDef *def); virDomainVcpuDefPtr virDomainDefGetVcpu(virDomainDefPtr def, unsigned int vcpu) ATTRIBUTE_RETURN_CHECK; void virDomainDefVcpuOrderClear(virDomainDefPtr def); +int virDomainDefGetVcpusTopology(const virDomainDef *def, + unsigned int *maxvcpus); virDomainObjPtr virDomainObjNew(virDomainXMLOptionPtr caps) ATTRIBUTE_NONNULL(1); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 923afd1..233cf6b 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -247,6 +247,7 @@ virDomainDefGetVcpu; virDomainDefGetVcpuPinInfoHelper; virDomainDefGetVcpus; virDomainDefGetVcpusMax; +virDomainDefGetVcpusTopology; virDomainDefHasDeviceAddress; virDomainDefHasMemballoon; virDomainDefHasMemoryHotplug; -- 2.10.0
Rather than multiplying sockets, cores, and threads use the new helper for getting the vcpu count resulting from the topology. --- src/qemu/qemu_domain.c | 18 ++++++++---------- src/qemu/qemu_driver.c | 14 ++++++-------- 2 files changed, 14 insertions(+), 18 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 2b24c01..83ac389 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2401,7 +2401,7 @@ qemuDomainDefValidate(const virDomainDef *def, { virQEMUDriverPtr driver = opaque; virQEMUCapsPtr qemuCaps = NULL; - size_t topologycpus; + unsigned int topologycpus; int ret = -1; if (!(qemuCaps = virQEMUCapsCacheLookup(caps, @@ -2443,15 +2443,13 @@ qemuDomainDefValidate(const virDomainDef *def, } /* 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; - } + if (virDomainDefGetVcpusTopology(def, &topologycpus) == 0 && + 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; } } diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 29a7e3f..e6f845d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4718,6 +4718,7 @@ qemuDomainSetVcpusMax(virQEMUDriverPtr driver, unsigned int nvcpus) { virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + unsigned int topologycpus; int ret = -1; if (def) { @@ -4733,16 +4734,13 @@ qemuDomainSetVcpusMax(virQEMUDriverPtr driver, goto cleanup; } - if (persistentDef->cpu && persistentDef->cpu->sockets) { + if (virDomainDefGetVcpusTopology(persistentDef, &topologycpus) == 0 && + nvcpus != topologycpus) { /* allow setting a valid vcpu count for the topology so an invalid * setting may be corrected via this API */ - if (nvcpus != persistentDef->cpu->sockets * - persistentDef->cpu->cores * - persistentDef->cpu->threads) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("CPU topology doesn't match the desired vcpu count")); - goto cleanup; - } + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("CPU topology doesn't match the desired vcpu count")); + goto cleanup; } /* ordering information may become invalid, thus clear it */ -- 2.10.0
On 10.10.2016 22:01, Peter Krempa wrote:
See patch 1.
Peter Krempa (2): conf: Sanitize cpu topology numbers qemu: Reuse virDomainDeGetVcpusTopology to calculate total vcpu count
src/conf/domain_conf.c | 39 +++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 2 ++ src/libvirt_private.syms | 1 + src/qemu/qemu_domain.c | 18 ++++++++---------- src/qemu/qemu_driver.c | 14 ++++++-------- 5 files changed, 56 insertions(+), 18 deletions(-)
ACK to both. Michal
participants (2)
-
Michal Privoznik -
Peter Krempa