[Kimchi-devel] [PATCH] [Kimchi 1/3] Add maxvcpus attribute to templates
Aline Manera
alinefm at linux.vnet.ibm.com
Fri Jan 29 17:16:59 UTC 2016
On 01/29/2016 03:15 PM, Aline Manera wrote:
>
>
> On 01/29/2016 03:12 PM, 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:...}}
>>
>
> Or extend the current cpu_info to include current and max values for cpu.
>
> cpu_info: {topology:..., current:..., max:...}
s/current/cpus
s/max/maxvcpus
=)
>
> Maybe it turns on a few changes needed to have all the CPU details in
> one data structure.
>
>> 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.
>>
>>> "memory": {
>>> "description": "Memory (MB) for the template",
>>> "type": "integer",
>>> @@ -637,6 +643,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"
>>> + },
>>> "memory": {
>>> "description": "Memory (MB) for the template",
>>> "type": "integer",
>>> diff --git a/control/templates.py b/control/templates.py
>>> index 4cd70c2..fcc298e 100644
>>> --- a/control/templates.py
>>> +++ b/control/templates.py
>>> @@ -1,7 +1,7 @@
>>> #
>>> # Project Kimchi
>>> #
>>> -# Copyright IBM, Corp. 2013-2015
>>> +# Copyright IBM, Corp. 2013-2016
>>> #
>>> # This library is free software; you can redistribute it and/or
>>> # modify it under the terms of the GNU Lesser General Public
>>> @@ -47,6 +47,7 @@ class Template(Resource):
>>> 'os_distro': self.info['os_distro'],
>>> 'os_version': self.info['os_version'],
>>> 'cpus': self.info['cpus'],
>>> + 'maxvcpus': self.info.get('maxvcpus'),
>>> 'memory': self.info['memory'],
>>> 'cdrom': self.info.get('cdrom', None),
>>> 'disks': self.info['disks'],
>>> diff --git a/docs/API.md b/docs/API.md
>>> index 5122a0c..4790d08 100644
>>> --- a/docs/API.md
>>> +++ b/docs/API.md
>>> @@ -276,8 +276,11 @@ Represents a snapshot of the Virtual Machine's
>>> primary monitor.
>>> * os_distro *(optional)*: The operating system distribution
>>> * os_version *(optional)*: The version of the operating system
>>> distribution
>>> * cpus *(optional)*: The number of CPUs assigned to the VM.
>>> - Default is 1, unlees specifying a cpu topology. In that
>>> case, cpus
>>> - will default to a product of the topology values (see
>>> cpu_info).
>>> + Default is 1, unless a CPU topology is specified. In that
>>> case, cpus
>>> + will default to maxvcpus value.
>>> + * maxvcpus *(optional)*: The maximum number of CPUs that can be
>>> assigned to
>>> + the VM. If a CPU topology is specified (see cpu_info),
>>> maxvcpus must
>>> + be a product of sockets, cores and threads.
>>> * memory *(optional)*: The amount of memory assigned to the VM.
>>> Default is 1024M.
>>> * cdrom *(optional)*: A volume name or URI to an ISO image.
>>> @@ -304,10 +307,10 @@ Represents a snapshot of the Virtual Machine's
>>> primary monitor.
>>> * topology: Specify sockets, threads, and cores to run the
>>> virtual CPU
>>> threads on.
>>> All three are required in order to specify cpu topology.
>>> - * sockets - The number of sockets to use.
>>> + * sockets - The maximum number of sockets to use.
>>> * cores - The number of cores per socket.
>>> * threads - The number of threads per core.
>>> - If specifying both cpus and CPU topology, make sure
>>> cpus is
>>> + If specifying both maxvcpus and CPU topology, make sure
>>> maxvcpus is
>>> equal to the product of sockets, cores, and threads.
>>>
>>> ### Sub-Collection: Virtual Machine Network Interfaces
>>> @@ -379,6 +382,7 @@ A interface represents available network
>>> interface on VM.
>>> * os_distro: The operating system distribution
>>> * os_version: The version of the operating system distribution
>>> * cpus: The number of CPUs assigned to the VM
>>> + * maxvcpus: The maximum number of CPUs that can be assigned to
>>> the VM
>>> * memory: The amount of memory assigned to the VM in the unit
>>> of MB
>>> * cdrom: A volume name or URI to an ISO image
>>> * storagepool: URI of the storagepool where template allocates
>>> vm storage.
>>> @@ -416,6 +420,7 @@ A interface represents available network
>>> interface on VM.
>>> * os_distro: The operating system distribution
>>> * os_version: The version of the operating system distribution
>>> * cpus: The number of CPUs assigned to the VM
>>> + * maxvcpus: The maximum number of CPUs that can be assigned to
>>> the VM
>>> * memory: The amount of memory assigned to the VM
>>> * cdrom: A volume name or URI to an ISO image
>>> * networks *(optional)*: list of networks will be assigned to
>>> the new VM.
>>> @@ -435,6 +440,17 @@ A interface represents available network
>>> interface on VM.
>>> Independent Computing Environments
>>> * null: Graphics is disabled or type not supported
>>> * listen: The network which the vnc/spice server listens on.
>>> + * cpu_info: CPU-specific information.
>>> + * topology: Specify sockets, threads, and cores to run the
>>> virtual CPU
>>> + threads on.
>>> + All three are required in order to specify cpu topology.
>>> + * sockets - The maximum number of sockets to use.
>>> + * cores - The number of cores per socket.
>>> + * threads - The number of threads per core.
>>> + If CPU topology is specified, *cpus* and *maxvcpus*
>>> also must be
>>> + specified. Make sure *maxvcpus* is equal to the product
>>> of sockets,
>>> + cores and threads, and *cpus* is a multiple of a
>>> product of cores
>>> + and threads.
>>>
>>> **Actions (POST):**
>>>
>>> diff --git a/i18n.py b/i18n.py
>>> index a575922..bb6f3d1 100644
>>> --- a/i18n.py
>>> +++ b/i18n.py
>>> @@ -1,7 +1,7 @@
>>> #
>>> # Project Kimchi
>>> #
>>> -# Copyright IBM, Corp. 2014-2015
>>> +# Copyright IBM, Corp. 2014-2016
>>> #
>>> # This library is free software; you can redistribute it and/or
>>> # modify it under the terms of the GNU Lesser General Public
>>> @@ -175,7 +175,6 @@ messages = {
>>> "KCHTMPL0022E": _("Disk size must be an integer greater than
>>> 1GB."),
>>> "KCHTMPL0023E": _("Template base image must be a valid local
>>> image file"),
>>> "KCHTMPL0024E": _("Cannot identify base image %(path)s format"),
>>> - "KCHTMPL0025E": _("When specifying CPU topology, VCPUs must be
>>> a product of sockets, cores, and threads."),
>>> "KCHTMPL0026E": _("When specifying CPU topology, each element
>>> must be an integer greater than zero."),
>>> "KCHTMPL0027E": _("Invalid disk image format. Valid formats:
>>> bochs, cloop, cow, dmg, qcow, qcow2, qed, raw, vmdk, vpc."),
>>> "KCHTMPL0028E": _("When setting template disks, following
>>> parameters are required: 'index', 'pool name', 'format', 'size' or
>>> 'volume' (for scsi/iscsi pools)"),
>>> @@ -311,9 +310,12 @@ messages = {
>>> "KCHSNAP0009E": _("Unable to revert virtual machine '%(vm)s'
>>> to snapshot '%(name)s'. Details: %(err)s"),
>>> "KCHSNAP0010E": _("Unable to create snapshot of virtual
>>> machine '%(vm)s' because it contains a disk with format
>>> '%(format)s'; only 'qcow2' is supported."),
>>>
>>> - "KCHCPUINF0001E": _("The number of vCPUs is too large for this
>>> system."),
>>> - "KCHCPUINF0002E": _("Invalid vCPU/topology combination."),
>>> + "KCHCPUINF0001E": _("The number of vCPUs must be less than or
>>> equal the maximum number of vCPUs specified."),
>>> + "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."),
>>> + "KCHCPUINF0006E": _("The number of threads is too large for
>>> this system."),
>>>
>>> "KCHLVMS0001E": _("Invalid volume group name parameter:
>>> %(name)s."),
>>>
>>> diff --git a/model/cpuinfo.py b/model/cpuinfo.py
>>> index 299e445..83e4ec2 100644
>>> --- a/model/cpuinfo.py
>>> +++ b/model/cpuinfo.py
>>> @@ -1,7 +1,7 @@
>>> #
>>> # Project Kimchi
>>> #
>>> -# Copyright IBM, Corp. 2014-2015
>>> +# Copyright IBM, Corp. 2014-2016
>>> #
>>> # This library is free software; you can redistribute it and/or
>>> # modify it under the terms of the GNU Lesser General Public
>>> @@ -25,6 +25,7 @@ from wok.utils import run_command, wok_log
>>>
>>>
>>> ARCH = 'power' if platform.machine().startswith('ppc') else 'x86'
>>> +MAX_PPC_VCPUS = 255
>>>
>>>
>>> def get_topo_capabilities(connect):
>>> @@ -106,21 +107,47 @@ class CPUInfoModel(object):
>>> 'threads_per_core': self.threads_per_core,
>>> }
>>>
>>> - def check_topology(self, vcpus, topology):
>>> + def check_cpu_info(self, params):
>>> """
>>> - param vcpus: should be an integer
>>> - param iso_path: the path of the guest ISO
>>> - param topology: {'sockets': x, 'cores': x, 'threads': x}
>>> + param params: dict containing:
>>> + maxvcpus: integer
>>> + vcpus: integer
>>> + cpu_info: None if topology undefined or topology
>>> definition
>>> + dict: {'topology': {
>>> + 'sockets': x,
>>> + 'cores': x,
>>> + 'threads': x
>>> + }
>>> + }
>>> """
>>> - sockets = topology['sockets']
>>> - cores = topology['cores']
>>> - threads = topology['threads']
>>> -
>>> - if not self.guest_threads_enabled:
>>> - raise InvalidOperation("KCHCPUINF0003E")
>>> - if vcpus != sockets * cores * threads:
>>> - raise InvalidParameter("KCHCPUINF0002E")
>>> - if vcpus > self.cores_available * self.threads_per_core:
>>> + maxvcpus = params.get('maxvcpus')
>>> + vcpus = params.get('cpus')
>>> + topology = params.get('cpu_info', {}).get('topology', None)
>>> + if topology:
>>> + sockets = topology['sockets']
>>> + cores = topology['cores']
>>> + threads = topology['threads']
>>> +
>>> + if not self.guest_threads_enabled:
>>> + raise InvalidOperation("KCHCPUINF0003E")
>>> + if threads > self.threads_per_core:
>>> + raise InvalidParameter("KCHCPUINF0006E")
>>> + if maxvcpus != sockets * cores * threads:
>>> + raise InvalidParameter("KCHCPUINF0002E")
>>> + if vcpus % (cores * threads) != 0:
>>> + raise InvalidParameter("KCHCPUINF0005E")
>>> +
>>> + if maxvcpus > self.get_host_max_vcpus():
>>> + raise InvalidParameter("KCHCPUINF0004E")
>>> + if vcpus > maxvcpus:
>>> raise InvalidParameter("KCHCPUINF0001E")
>>> - if threads > self.threads_per_core:
>>> - raise InvalidParameter("KCHCPUINF0002E")
>>> +
>>> + def get_host_max_vcpus(self):
>>> + if ARCH == 'power':
>>> + max_vcpus = self.cores_available * self.threads_per_core
>>> + if max_vcpus > MAX_PPC_VCPUS:
>>> + max_vcpus = MAX_PPC_VCPUS
>>> + else:
>>> + max_vcpus = self.conn.get().getMaxVcpus('kvm')
>>> +
>>> + return max_vcpus
>>> diff --git a/model/templates.py b/model/templates.py
>>> index c9b11c3..1f29bd9 100644
>>> --- a/model/templates.py
>>> +++ b/model/templates.py
>>> @@ -1,7 +1,7 @@
>>> #
>>> # Project Kimchi
>>> #
>>> -# Copyright IBM, Corp. 2014-2015
>>> +# Copyright IBM, Corp. 2014-2016
>>> #
>>> # This library is free software; you can redistribute it and/or
>>> # modify it under the terms of the GNU Lesser General Public
>>> @@ -54,22 +54,6 @@ class TemplatesModel(object):
>>> {'filename': iso,
>>> 'user': user,
>>> 'err': excp})
>>>
>>> - cpu_info = params.get('cpu_info')
>>> - if cpu_info:
>>> - topology = cpu_info.get('topology')
>>> - # Check, even though currently only topology
>>> - # is supported.
>>> - if topology:
>>> - sockets = topology['sockets']
>>> - cores = topology['cores']
>>> - threads = topology['threads']
>>> - if params.get('cpus') is None:
>>> - params['cpus'] = sockets * cores * threads
>>> - # check_topoology will raise the appropriate
>>> - # exception if a topology is invalid.
>>> - CPUInfoModel(conn=self.conn).\
>>> - check_topology(params['cpus'], topology)
>>> -
>>> conn = self.conn.get()
>>> for net_name in params.get(u'networks', []):
>>> try:
>>> @@ -82,6 +66,9 @@ class TemplatesModel(object):
>>> # will be raised here
>>> t = LibvirtVMTemplate(params, scan=True, conn=self.conn)
>>>
>>> + # Validate cpu info
>>> + t.cpuinfo_validate()
>>> +
>>> # Validate max memory
>>> maxMem = (t._get_max_memory(t.info.get('memory')) >> 10)
>>> if t.info.get('memory') > maxMem:
>>> @@ -180,9 +167,6 @@ class TemplateModel(object):
>>> new_t = copy.copy(old_t)
>>> new_t.update(params)
>>>
>>> - if not self._validate_updated_cpu_params(new_t):
>>> - raise InvalidParameter('KCHTMPL0025E')
>>> -
>>> for net_name in params.get(u'networks', []):
>>> try:
>>> conn = self.conn.get()
>>> @@ -199,22 +183,18 @@ class TemplateModel(object):
>>> raise
>>> return ident
>>>
>>> - def _validate_updated_cpu_params(self, info):
>>> - # Note: cpu_info is the parent of topology. cpus is vcpus
>>> - vcpus = info['cpus']
>>> - cpu_info = info.get('cpu_info')
>>> - # cpu_info will always be at least an empty dict
>>> - topology = cpu_info.get('topology')
>>> - if topology is None:
>>> - return True
>>> - return vcpus == topology['sockets'] * topology['cores'] * \
>>> - topology['threads']
>>> -
>>>
>>> class LibvirtVMTemplate(VMTemplate):
>>> def __init__(self, args, scan=False, conn=None):
>>> self.conn = conn
>>> VMTemplate.__init__(self, args, scan)
>>> + self.set_cpu_info(args)
>>> +
>>> + def cpuinfo_validate(self):
>>> + cpu_model = CPUInfoModel(conn=self.conn)
>>> +
>>> + # validate CPU info values - will raise appropriate exceptions
>>> + cpu_model.check_cpu_info(self.info)
>>>
>>> def _storage_validate(self, pool_uri):
>>> pool_name = pool_name_from_uri(pool_uri)
>>> @@ -283,3 +263,28 @@ class LibvirtVMTemplate(VMTemplate):
>>> except libvirt.libvirtError as e:
>>> raise OperationFailed("KCHVMSTOR0008E", {'error':
>>> e.message})
>>> return vol_list
>>> +
>>> + def set_cpu_info(self, params):
>>> + # undefined topology: consider these values to calculate
>>> maxvcpus
>>> + sockets = 1
>>> + cores = 1
>>> + threads = 1
>>> +
>>> + # get topology values
>>> + topology = self.info.get('cpu_info', {}).get('topology', None)
>>> + if topology:
>>> + sockets = topology['sockets']
>>> + cores = topology['cores']
>>> + threads = topology['threads']
>>> +
>>> + # maxvcpus not specified: use defaults
>>> + if 'maxvcpus' not in params:
>>> + cpus = self.info.get('cpus', None)
>>> + if cpus and not topology:
>>> + self.info['maxvcpus'] = cpus
>>> + else:
>>> + self.info['maxvcpus'] = sockets * cores * threads
>>> +
>>> + # current vcpus not specified: defaults is maxvcpus
>>> + if 'cpus' not in params:
>>> + self.info['cpus'] = self.info['maxvcpus']
>>> diff --git a/model/vms.py b/model/vms.py
>>> index 1203c71..9aaf530 100644
>>> --- a/model/vms.py
>>> +++ b/model/vms.py
>>> @@ -107,17 +107,6 @@ class VMsModel(object):
>>> self.caps = CapabilitiesModel(**kargs)
>>> self.task = TaskModel(**kargs)
>>>
>>> - def _get_host_maxcpu(self):
>>> - if os.uname()[4] in ['ppc', 'ppc64', 'ppc64le']:
>>> - cpu_model = CPUInfoModel(conn=self.conn)
>>> - max_vcpu_val = (cpu_model.cores_available *
>>> - cpu_model.threads_per_core)
>>> - if max_vcpu_val > 255:
>>> - max_vcpu_val = 255
>>> - else:
>>> - max_vcpu_val = self.conn.get().getMaxVcpus('kvm')
>>> - return max_vcpu_val
>>> -
>>> def create(self, params):
>>> t_name = template_name_from_uri(params['template'])
>>> vm_list = self.get_list()
>>> @@ -178,8 +167,7 @@ class VMsModel(object):
>>> stream_protocols = self.caps.libvirt_stream_protocols
>>> xml = t.to_vm_xml(name, vm_uuid,
>>> libvirt_stream_protocols=stream_protocols,
>>> - graphics=graphics,
>>> - max_vcpus=self._get_host_maxcpu())
>>> + graphics=graphics)
>>>
>>> cb('Defining new VM')
>>> try:
>>> @@ -940,17 +928,6 @@ class VMModel(object):
>>>
>>> return ET.tostring(root, encoding="utf-8")
>>>
>>> - def _get_host_maxcpu(self):
>>> - if os.uname()[4] in ['ppc', 'ppc64', 'ppc64le']:
>>> - cpu_model = CPUInfoModel(conn=self.conn)
>>> - max_vcpu_val = (cpu_model.cores_available *
>>> - cpu_model.threads_per_core)
>>> - if max_vcpu_val > 255:
>>> - max_vcpu_val = 255
>>> - else:
>>> - max_vcpu_val = self.conn.get().getMaxVcpus('kvm')
>>> - return max_vcpu_val
>>> -
>>> def _live_vm_update(self, dom, params):
>>> self._vm_update_access_metadata(dom, params)
>>> if 'memory' in params and dom.isActive():
>>> diff --git a/vmtemplate.py b/vmtemplate.py
>>> index d629226..2f524a7 100644
>>> --- a/vmtemplate.py
>>> +++ b/vmtemplate.py
>>> @@ -1,7 +1,7 @@
>>> #
>>> # Project Kimchi
>>> #
>>> -# Copyright IBM, Corp. 2013-2015
>>> +# Copyright IBM, Corp. 2013-2016
>>> #
>>> # This library is free software; you can redistribute it and/or
>>> # modify it under the terms of the GNU Lesser General Public
>>> @@ -466,6 +466,10 @@ class VMTemplate(object):
>>> self._storage_validate(pool_uri)
>>> self._network_validate()
>>> self._iso_validate()
>>> + self.cpuinfo_validate()
>>> +
>>> + def cpuinfo_validate(self):
>>> + pass
>>>
>>> def _iso_validate(self):
>>> pass
>>
>> _______________________________________________
>> Kimchi-devel mailing list
>> Kimchi-devel at ovirt.org
>> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
>>
>
> _______________________________________________
> Kimchi-devel mailing list
> Kimchi-devel at ovirt.org
> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
>
More information about the Kimchi-devel
mailing list