4:59 a.m.
For qemu, if the -smp N or the value of maxcpus is given, it define
the number of vcpu of guest whenever the vcpu topology is defined or
not. But if the -smp N and maxcpus are missing, the topology can
compute and define vcpus for guest automatically by math:
vcpu number = sockets*cores*threads
For libvirt, as <vcpu> is always mandatory, so we can ask topology
to match maximum vcpu numbers.
---
src/conf/domain_conf.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index a9656af..ffdc6da 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11815,10 +11815,10 @@ virDomainDefParseXML(xmlDocPtr xml,
goto error;
if (def->cpu->sockets &&
- def->maxvcpus >
+ def->maxvcpus !=
def->cpu->sockets * def->cpu->cores * def->cpu->threads) {
virReportError(VIR_ERR_XML_DETAIL, "%s",
- _("Maximum CPUs greater than topology limit"));
+ _("Topology limit does not match maximum CPUs"));
goto error;
}
--
1.8.1.4
3:20 p.m.
On 05/28/2013 03:59 AM, Guannan Ren wrote:
For qemu, if the -smp N or the value of maxcpus is given, it define
the number of vcpu of guest whenever the vcpu topology is defined or
not. But if the -smp N and maxcpus are missing, the topology can
compute and define vcpus for guest automatically by math:
vcpu number = sockets*cores*threads
For libvirt, as <vcpu> is always mandatory, so we can ask topology
to match maximum vcpu numbers.
---
src/conf/domain_conf.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index a9656af..ffdc6da 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11815,10 +11815,10 @@ virDomainDefParseXML(xmlDocPtr xml,
goto error;
if (def->cpu->sockets &&
- def->maxvcpus >
+ def->maxvcpus !=
def->cpu->sockets * def->cpu->cores * def->cpu->threads)
{
virReportError(VIR_ERR_XML_DETAIL, "%s",
- _("Maximum CPUs greater than topology limit"));
+ _("Topology limit does not match maximum CPUs"));
Is this going to reject XML that was previously accepted? Is there a
bugzilla showing what happens if this patch is not incorporated? I'm
worried about introducing an unintentional regression if we include this
in 1.0.6 without more justification.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
8:32 p.m.
On 05/29/2013 04:20 AM, Eric Blake wrote:
On 05/28/2013 03:59 AM, Guannan Ren wrote:
> For qemu, if the -smp N or the value of maxcpus is given, it define
> the number of vcpu of guest whenever the vcpu topology is defined or
> not. But if the -smp N and maxcpus are missing, the topology can
> compute and define vcpus for guest automatically by math:
>
> vcpu number = sockets*cores*threads
>
> For libvirt, as <vcpu> is always mandatory, so we can ask topology
> to match maximum vcpu numbers.
> ---
> src/conf/domain_conf.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index a9656af..ffdc6da 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -11815,10 +11815,10 @@ virDomainDefParseXML(xmlDocPtr xml,
> goto error;
>
> if (def->cpu->sockets &&
> - def->maxvcpus >
> + def->maxvcpus !=
> def->cpu->sockets * def->cpu->cores *
def->cpu->threads) {
> virReportError(VIR_ERR_XML_DETAIL, "%s",
> - _("Maximum CPUs greater than topology
limit"));
> + _("Topology limit does not match maximum
CPUs"));
Is this going to reject XML that was previously accepted? Is there a
bugzilla showing what happens if this patch is not incorporated? I'm
worried about introducing an unintentional regression if we include this
in 1.0.6 without more justification.
Yes, it make the cpu topology limit setting more strict.
Alternatively, we can add more comments for <topology> to make it clear that
<vcpu> decide the number of vcpus, the sockets*cores*threads had better
be equal to vcpu numbers.
The bz:https://bugzilla.redhat.com/show_bug.cgi?id=880017
Description of problem:
libvirt should check if vcpu topology is right. If the wrong vcpu topology is given in xml
, the wrong arguments also be passed to qemu-kvm.
vcpu number = sockets*cores*threads
Steps to Reproduce:
1.# virsh start vm
Domain vm started
2.# virsh dumpxml vm
<domain type='kvm' id='104'>
.......
<vcpu placement='static'>4</vcpu>
......
<cpu>
<topology sockets='1' cores='4' threads='2'/>
</cpu>
3.# ps -ef|grep qemu-kvm
qemu 21296 1 14 16:41 ? 00:00:17 /usr/libexec/qemu-kvm -name vm -S -M
rhel6.4.0 -enable-kvm -m 1024 -smp 4,sockets=1,cores=4,threads=2
......
Actual results:
Wrong vcpu topology can be given in xml and passed to qemu-kvm
Expected results:
libvirt should check if vcpu topology is right.
10:11 p.m.
On 05/28/2013 07:32 PM, Guannan Ren wrote:
>> if (def->cpu->sockets &&
>> - def->maxvcpus >
>> + def->maxvcpus !=
>> def->cpu->sockets * def->cpu->cores *
def->cpu->threads) {
>> virReportError(VIR_ERR_XML_DETAIL, "%s",
>> - _("Maximum CPUs greater than topology
>> limit"));
>> + _("Topology limit does not match maximum
>> CPUs"));
> Is this going to reject XML that was previously accepted? Is there a
> bugzilla showing what happens if this patch is not incorporated? I'm
> worried about introducing an unintentional regression if we include this
> in 1.0.6 without more justification.
>
Yes, it make the cpu topology limit setting more strict.
I'm okay with making things more strict, if it makes sense. But isn't
it feasible to support a topology that has more slots than the maximum?
That is, since a bare metal machine can have two sockets but only
populate one, why can't a virtual machine have 2 sockets but set a max
vcpu of 1 so that only one socket can be populated? Does qemu itself
enforce that there be a match?
That's why I'm worried about enforcing that the user can't have a
mismatch between maxvcpus vs. topology, if they specify both. I'm also
worried that this check might fire if the user specifies only topology
or only maxvcpus; I want to make sure that we only do the sanity
checking if both numbers are provided.
Alternatively, we can add more comments for <topology> to make
it clear
that
<vcpu> decide the number of vcpus, the sockets*cores*threads had better
be equal to vcpu numbers.
If there is a problem with a mismatch, then we should document that a
match is required, and fail on mismatch. Merely documenting that a
match is required without enforcing it is too weak; and enforcing it
without documentation is a disservice. I haven't ruled out enforcing a
match, but want to make sure that we aren't artificially crippling
something that might prove useful in reality, when compared with what
bare-metal setups can provide.
The bz:https://bugzilla.redhat.com/show_bug.cgi?id=880017
Description of problem:
libvirt should check if vcpu topology is right. If the wrong vcpu
topology is given in xml , the wrong arguments also be passed to qemu-kvm.
vcpu number = sockets*cores*threads
Steps to Reproduce:
1.# virsh start vm
Domain vm started
2.# virsh dumpxml vm
<domain type='kvm' id='104'>
.......
<vcpu placement='static'>4</vcpu>
......
<cpu>
<topology sockets='1' cores='4' threads='2'/>
</cpu>
3.# ps -ef|grep qemu-kvm
qemu 21296 1 14 16:41 ? 00:00:17 /usr/libexec/qemu-kvm
-name vm -S -M rhel6.4.0 -enable-kvm -m 1024 -smp
4,sockets=1,cores=4,threads=2
......
Actual results:
Wrong vcpu topology can be given in xml and passed to qemu-kvm
Expected results:
libvirt should check if vcpu topology is right.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
9:53 p.m.
New subject: [libvirt] [PATCH] doc: cpu topology values should be equal to maxmium vcpus
---
docs/formatdomain.html.in | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 755d084..27c1580 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -941,7 +941,9 @@
virtual CPU provided to the guest. Three non-zero values have to be
given for <code>sockets</code>, <code>cores</code>, and
<code>threads</code>: total number of CPU sockets, number of cores
per
- socket, and number of threads per core, respectively.</dd>
+ socket, and number of threads per core, respectively. The maximum number
+ of virtual CPUs is defined by <code>vcpu</code> element, the product
of
+ the three valuses should be equal to the maximum vcpus.</dd>
<dt><code>feature</code></dt>
<dd>The <code>cpu</code> element can contain zero or more
--
1.8.1.4
10:14 p.m.
New subject: [libvirt] [PATCH] doc: cpu topology values should be equal to maxmium vcpus
On 05/28/2013 08:53 PM, Guannan Ren wrote:
---
docs/formatdomain.html.in | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 755d084..27c1580 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -941,7 +941,9 @@
virtual CPU provided to the guest. Three non-zero values have to be
given for <code>sockets</code>, <code>cores</code>, and
<code>threads</code>: total number of CPU sockets, number of cores
per
- socket, and number of threads per core, respectively.</dd>
+ socket, and number of threads per core, respectively. The maximum number
+ of virtual CPUs is defined by <code>vcpu</code> element, the product
of
+ the three valuses should be equal to the maximum vcpus.</dd>
s/valuses/values/
Again, if we are going to document that they must be equal, then the
code should match; otherwise, if we are going to allow a difference
between the two, then the wording should be a bit nicer ("It is
recommended that if both maxvcpu and topology are provided, then the two
numbers should match"). Or maybe a one-sided restriction makes sense,
where maxvcpus can be less than or equal, but not greater than, an
explicit topology.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
7:48 a.m.
New subject: [libvirt] [PATCH] doc: cpu topology values should be equal to maxmium vcpus
On 05/29/2013 11:14 AM, Eric Blake wrote:
On 05/28/2013 08:53 PM, Guannan Ren wrote:
> ---
> docs/formatdomain.html.in | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
There are little document about how to set values of sockets, cores,
threads properly.
After going through qemu code, I summarize the package/cores/threads/
algorithm
which can be verified by 'cpuid' tool.
The following only explains the case of all the four number are given in
qemu commandline.
smp = n (argument to -smp)
nr_threads = smp / (cores * sockets)
nr_cores = cores
-smp 4, sockets=1,cores=4,threads=2 leads to: 1(physical cpu) *4(cores)
*1(thread)
One physical cpu contains four cores log processor which each contains
one thread logical processor.
0 means only one physical cpu package
pkg_id = cpu_index/nr_threads/nr_cores
0 = 0 / 1 / 4
0 = 1 / 1 / 4
0 = 2 / 1 / 4
0 = 3 / 1 / 4
3 means there are four multi-cores logical processor
core_id = cpu_index/nr_threads % nr_cores
0 = 0 / 1 % 4
1 = 1 / 1 % 4
2 = 2 / 1 % 4
3 = 3 / 1 % 4
0 means only one thread logical processor
smt_id = cpu_index % nr_threads
0 = 0 % 1
0 = 1 % 1
0 = 2 % 1
0 = 3 % 1
And the result could be verified by 'cpuid' tool running in fedora guest.
1st vcpu: (APIC synth): PKG_ID=0 CORE_ID=0 SMT_ID=0
2nd vcpu: (APIC synth): PKG_ID=0 CORE_ID=1 SMT_ID=0
3rd vcpu: (APIC synth): PKG_ID=0 CORE_ID=2 SMT_ID=0
4th vcpu: (APIC synth): PKG_ID=0 CORE_ID=3 SMT_ID=0
I tried it on several cpu topology:
-smp 16, sockets=2,cores=2,threads=1 leads to 2 * 2 * 4
There are two physical cpu package. Each of them contains two cores
logical processor and
each of the cores contains four hyper-threading logical processor.
The result could be verified by cpuid tool.
If the calculation is true, I think my patch is self-acked.
Guannan
4195
days inactive
4196
days old
7 comments
2 participants
participants (2)
-
Eric Blake -
Guannan Ren