[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] qemu: match vcpu topology to maximum vcpu numbers.

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

Attachment: signature.asc
Description: OpenPGP digital signature

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]