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

- 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" + }, "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