Re: [libvirt] [PATCH 1/1] Added check for maximum number of vcpus exceeding topology limit
On 01/11/2012 09:10 AM, Martin Kletzander wrote:
Earlier, when the number of vcpus was greater than the topology
allowed,
libvirt didn't raise an error and continued, resulting in running qemu
with parameters making no sense. Even though qemu did not report any
error itself, the number of vcpus was set to maximum allowed by the
topology.
---
src/conf/domain_conf.c | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 180dd2b..06ddd02 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -8010,6 +8010,13 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps,
if (def->cpu == NULL)
goto error;
+ if (def->maxvcpus >
+ def->cpu->sockets * def->cpu->cores * def->cpu->threads)
{
+ virDomainReportError(VIR_ERR_XML_DETAIL, "%s",
+ _("Maximum CPUs greater than topology
limit"));
+ goto error;
+ }
Won't work as-is, since topology is optional, in which case the product
of sockets*cores*threads will be 0 and trigger the error message.
I think our end goal should be that if the user provided neither
topology nor maxvcpus, then we should behave as if they requested 1
vcpu. If they provided only one of the two values, we should compute
the other value (in the case of providing only topology, maxvcpus is
easy to compute; in the reverse direction, I wonder if a sane default is
having sockets and cores be 1, and threads equal to maxvcpus). Finally,
if the user provides both topology and maxvcpus, then we need to
correlate the two and ensure a sane mix (your patch was trying to
address this point, but failed to address the other situations) - I'm
not sure whether it makes sense to allow maxvcpus less than the product,
or whether we should insist that it is equal to the product.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 620 bytes)