[Kimchi-devel] [PATCH] [Kimchi 1/3] Add maxvcpus attribute to templates
Aline Manera
alinefm at linux.vnet.ibm.com
Fri Jan 29 17:15:38 UTC 2016
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:...}
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
>
More information about the Kimchi-devel
mailing list