[Kimchi-devel] [PATCH] [Kimchi 1/3] Add maxvcpus attribute to templates

[Aline Manera]

On 01/29/2016 03:25 PM, Lucio Correia wrote:
> On 29-01-2016 15:12, Aline Manera wrote:
>>
>>
>> On 01/27/2016 07:14 PM, Lucio Correia wrote:
>>> - Update defalut values for maxvcpus and cpus
>>> - Rename check_topology() to check_cpu_info() to also verify cpus
>>> and maxvcpus and use it for simplification of all CPU parameter
>>> validations
>>> - Unduplicate and move _get_host_maxcpu() to CPUInfoModel as
>>> get_host_max_vcpus()
>>> - Update docs and APIs with maxvcpus attribute
>>>
>>> Signed-off-by: Lucio Correia <luciojhc at linux.vnet.ibm.com>
>>> ---
>>>   API.json             | 12 ++++++++++
>>>   control/templates.py |  3 ++-
>>>   docs/API.md          | 24 +++++++++++++++----
>>>   i18n.py              | 10 ++++----
>>>   model/cpuinfo.py     | 59 
>>> ++++++++++++++++++++++++++++++++-------------
>>>   model/templates.py   | 67
>>> ++++++++++++++++++++++++++++------------------------
>>>   model/vms.py         | 25 +-------------------
>>>   vmtemplate.py        |  6 ++++-
>>>   8 files changed, 125 insertions(+), 81 deletions(-)
>>>
>>> diff --git a/API.json b/API.json
>>> index 2b64d07..4876cc0 100644
>>> --- a/API.json
>>> +++ b/API.json
>>> @@ -454,6 +454,12 @@
>>>                       "minimum": 1,
>>>                       "error": "KCHTMPL0012E"
>>>                   },
>>
>>> +                "maxvcpus": {
>>> +                    "description": "The maximum number of virtual
>>> CPUs that can be assigned to the VMs created from the template",
>>> +                    "type": "integer",
>>> +                    "minimum": 1,
>>> +                    "error": "KCHTMPL0012E"
>>> +                },
>>
>> It is better to concentrate all the CPU information in the same object
>>
>> That way the 'cpus' would be changed to 'cpu' with 'current' and 'max'
>> values
>>
>> {.. cpu: {current: ..., max:...}}
>>
>> It would be good to also include 'cpu_info[topology]' in the same data
>> structure.
>>
>> {... cpu: {current:..., max:..., topology: ...}
>>
>> To include topology in the same data structure will require a lot of
>> changes. So it is better to do in a separated patch after getting this
>> one (related to max cpus) merged.
>>
>
> Yes, I talked to Rodrigo about this. I preferred not to do it along 
> with the new feature due to it being a big change.
>
> There are some implications on this. Each of cpus, maxvcpus and 
> topology values depend on each other:
> maxvcpus = sockets * cores * threads
> cpus     =       x * cores * threads (where 1 <= x <= sockets)
>
> Which means that, if the client (UI or API) wants to do cpu hotplug, 
> it will have to inform the entire struct again (not only cpus new 
> value). My patches try to be flexible and accept cpus-only or 
> maxvcpus-only updates. All values are required only if topology is 
> being changed.
>
You can continue doing that way even changing the data structure.