[Kimchi-devel] [PATCH] [Kimchi] Bug fix #1072 - changing vpus verification

[Aline Manera]

Hi Daniel,

I am not sure this is the root cause of the issue.
I created a guest with topology and tried to change maxvcpus and vcpus 
values on UI and I got the same error reported on github.

The test I did:
- create a new guest with topology set

|"cpu_info":{ ||"maxvcpus":4, ||"vcpus":4, ||"topology":{ ||"cores":2, ||"threads":2, ||"sockets":1 ||} ||}|


- try to change maxvcpus to 8
- try to change vcpus to 8




I believe the problem is because, on UI, there is no information about 
topology, only about maxvcpus and vcpus.
When user sets maxvcpus (and topology is set), the topology must be 
changed according to the new maxvcpus value.
We will need to update the UI to show and let the user change the 
topology instead of maxvcpus (as maxvcpus must be cores * threads * 
sockets).

On 11/04/2016 04:15 PM, dhbarboza82 at gmail.com wrote:
> From: Daniel Henrique Barboza <danielhb at linux.vnet.ibm.com>
>
> This patch changes the vcpu verification when
> editing a turned off VM, if the VM has a topology
> set, to comply with the current libvirt schema: vcpus
> must be a multiple of the 'threads' value.
>
> Unit tests were adapted to reflect these changes.
>
> Signed-off-by: Daniel Henrique Barboza <danielhb at linux.vnet.ibm.com>
> ---
>   i18n.py             |  2 +-
>   model/cpuinfo.py    |  2 +-
>   tests/test_model.py | 12 ++++++------
>   3 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/i18n.py b/i18n.py
> index 03929e5..2ec0e78 100644
> --- a/i18n.py
> +++ b/i18n.py
> @@ -357,7 +357,7 @@ messages = {
>       "KCHCPUINF0002E": _("When CPU topology is defined, maximum number of vCPUs must be a product of sockets, cores, and threads."),
>       "KCHCPUINF0003E": _("This host (or current configuration) does not allow CPU topology."),
>       "KCHCPUINF0004E": _("The maximum number of vCPUs is too large for this system."),
> -    "KCHCPUINF0005E": _("When CPU topology is defined, vCPUs must be a multiple of a product of cores and threads."),
> +    "KCHCPUINF0005E": _("When CPU topology is defined, vCPUs must be a multiple of the 'threads' number defined."),
>       "KCHCPUINF0006E": _("The number of threads is too large for this system."),
>       "KCHCPUINF0007E": _("When CPU topology is specified, sockets, cores and threads are required paramaters."),
>       "KCHCPUINF0008E": _("Parameter 'cpu_info' expects an object with fields among: 'vcpus', 'maxvcpus', 'topology'."),
> diff --git a/model/cpuinfo.py b/model/cpuinfo.py
> index 3707a02..5f82320 100644
> --- a/model/cpuinfo.py
> +++ b/model/cpuinfo.py
> @@ -138,7 +138,7 @@ class CPUInfoModel(object):
>                   raise InvalidParameter("KCHCPUINF0006E")
>               if maxvcpus != sockets * cores * threads:
>                   raise InvalidParameter("KCHCPUINF0002E")
> -            if vcpus % (cores * threads) != 0:
> +            if vcpus % threads != 0:
>                   raise InvalidParameter("KCHCPUINF0005E")
>
>           if maxvcpus > self.get_host_max_vcpus():
> diff --git a/tests/test_model.py b/tests/test_model.py
> index 05d7415..1246be6 100644
> --- a/tests/test_model.py
> +++ b/tests/test_model.py
> @@ -1298,23 +1298,23 @@ class ModelTests(unittest.TestCase):
>
>               # define CPU topology
>               inst.vm_update(u'kimchi-vm1', {'cpu_info': {'topology': {
> -                           'sockets': 4, 'cores': 2, 'threads': 1}}})
> +                           'sockets': 2, 'cores': 2, 'threads': 2}}})
>               vm_info = inst.vm_lookup(u'kimchi-vm1')
> -            self.assertEquals({'sockets': 4, 'cores': 2, 'threads': 1},
> +            self.assertEquals({'sockets': 2, 'cores': 2, 'threads': 2},
>                                 vm_info['cpu_info']['topology'])
>
> -            # vcpus not a multiple of (cores * threads)
> +            # vcpus not a multiple of threads
>               self.assertRaises(InvalidParameter, inst.vm_update, u'kimchi-vm1',
> -                              {'cpu_info': {'vcpus': 1}})
> +                              {'cpu_info': {'vcpus': 5}})
>
>               # maxvcpus different of (sockets * cores * threads)
>               self.assertRaises(InvalidParameter, inst.vm_update, u'kimchi-vm1',
>                                 {'cpu_info': {'maxvcpus': 4}})
>
> -            # topology does not match maxvcpus (8 != 2 * 2 * 1)
> +            # topology does not match maxvcpus (8 != 3 * 2 * 2)
>               self.assertRaises(InvalidParameter, inst.vm_update, u'kimchi-vm1',
>                                 {'cpu_info': {'topology': {
> -                               'sockets': 2, 'cores': 2, 'threads': 1}}})
> +                               'sockets': 3, 'cores': 2, 'threads': 2}}})
>
>               # undefine CPU topology
>               inst.vm_update(u'kimchi-vm1', {'cpu_info': {'topology': {}}})

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ovirt.org/pipermail/kimchi-devel/attachments/20161104/54b23e8f/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: odbkdhheeioeaokd.jpg
Type: image/jpeg
Size: 33670 bytes
Desc: not available
URL: <http://lists.ovirt.org/pipermail/kimchi-devel/attachments/20161104/54b23e8f/attachment.jpg>