[Kimchi-devel] [PATCH] [Kimchi 1/3] Add maxvcpus attribute to templates
Paulo Ricardo Paz Vital
pvital at linux.vnet.ibm.com
Fri Jan 29 16:54:29 UTC 2016
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"
> + },
> "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
In API.json you said that maxvcpu is "The maximum number of virtual CPUs
that can be assigned to the VMs..." and here looks like you missed the
*virtual* word. Is it correct?
> + 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
Same as my first comment.
> * 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
Same here.
> * 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
>
More information about the Kimchi-devel
mailing list