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

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(a)linux.vnet.ibm.com&gt; >> --- >> 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.