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

Aline Manera alinefm at linux.vnet.ibm.com
Fri Jan 29 17:12:37 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"
> +                },

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.

>                   "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




More information about the Kimchi-devel mailing list