[Kimchi-devel] [PATCH v2] Parts to allow Kimchi to configure the cpu topology.

Christy Perez christy at linux.vnet.ibm.com
Tue Nov 11 20:24:59 UTC 2014



On 11/10/2014 11:59 PM, Christy Perez wrote:
> Signed-off-by: Christy Perez <christy at linux.vnet.ibm.com>
> ---
>  docs/API.md                   |   2 +
>  src/kimchi/API.json           |   6 ++
>  src/kimchi/control/cpuinfo.py |  35 +++++++++
>  src/kimchi/i18n.py            |   2 +
>  src/kimchi/model/cpuinfo.py   | 169 ++++++++++++++++++++++++++++++++++++++++++
>  src/kimchi/model/host.py      |   1 +
>  src/kimchi/model/templates.py |  15 +++-
>  7 files changed, 229 insertions(+), 1 deletion(-)
>  create mode 100644 src/kimchi/control/cpuinfo.py
>  create mode 100644 src/kimchi/model/cpuinfo.py
> 
> diff --git a/docs/API.md b/docs/API.md
> index 9b866f3..865c728 100644
> --- a/docs/API.md
> +++ b/docs/API.md
> @@ -233,6 +233,8 @@ Represents a snapshot of the Virtual Machine's primary monitor.
>              * threads - The number of threads per core.
>              If specifying both cpus and CPU topology, make sure cpus is
>              equal to the product of sockets, cores, and threads.
> +        * auto-topology: Allow Kimchi to determine the best topology
> +            based on desired vCPUs and host capabilities.
> 
>  ### Sub-Collection: Virtual Machine Network Interfaces
> 
> diff --git a/src/kimchi/API.json b/src/kimchi/API.json
> index 0ad36ab..b405889 100644
> --- a/src/kimchi/API.json
> +++ b/src/kimchi/API.json
> @@ -54,6 +54,12 @@
>                              "error": "KCHTMPL0026E"
>                          }
>                      }
> +                },
> +                "auto_topology": {
> +                    "description": "Auto-configure guest CPU topology.",
> +                    "type": "boolean",
> +                    "required": false,
> +                    "error": "KCHTMPL0028E"
>                  }
>              }
>          }
> diff --git a/src/kimchi/control/cpuinfo.py b/src/kimchi/control/cpuinfo.py
> new file mode 100644
> index 0000000..db38b2d
> --- /dev/null
> +++ b/src/kimchi/control/cpuinfo.py
> @@ -0,0 +1,35 @@
> +#
> +# Project Kimchi
> +#
> +# Copyright IBM, Corp. 2014
> +#
> +# This library is free software; you can redistribute it and/or
> +# modify it under the terms of the GNU Lesser General Public
> +# License as published by the Free Software Foundation; either
> +# version 2.1 of the License, or (at your option) any later version.
> +#
> +# This library is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +# Lesser General Public License for more details.
> +#
> +# You should have received a copy of the GNU Lesser General Public
> +# License along with this library; if not, write to the Free Software
> +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301 USA
> +
> +from kimchi.control.base import Resource
> +
> +
> +class CPUInfo(Resource):
> +    def __init__(self, model):
> +        super(CPUInfo, self).__init__(model)
> +        self.uri_fmt = "/host/cpu_info"
> +
> +    @property
> +    def data(self):
> +        return {'threading_enabled': self.info['guest_threads_enabled'],
> +                'sockets': self.info['sockets'],
> +                'cores_present': self.info['cores_present'],
> +                'cores_online': self.info['cores_available'],
> +                'threads_per_core': self.info['threads_per_core']
> +                }
> diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py
> index e823f2b..1ace42c 100644
> --- a/src/kimchi/i18n.py
> +++ b/src/kimchi/i18n.py
> @@ -150,6 +150,8 @@
>      "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": _("The auto_topology value must be either True or False."),
> +    "KCHTMPL0029E": _("This host (or current configuration) does not allow CPU topology."),
> 
>      "KCHPOOL0001E": _("Storage pool %(name)s already exists"),
>      "KCHPOOL0002E": _("Storage pool %(name)s does not exist"),
> diff --git a/src/kimchi/model/cpuinfo.py b/src/kimchi/model/cpuinfo.py
> new file mode 100644
> index 0000000..93ee8d1
> --- /dev/null
> +++ b/src/kimchi/model/cpuinfo.py
> @@ -0,0 +1,169 @@
> +# modify it under the terms of the GNU Lesser General Public
> +# License as published by the Free Software Foundation; either
> +# version 2.1 of the License, or (at your option) any later version.
> +#
> +# This library is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +# Lesser General Public License for more details.
> +#
> +# You should have received a copy of the GNU Lesser General Public
> +# License along with this library; if not, write to the Free Software
> +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301 USA
> +
> +import platform
> +import psutil
> +from xml.etree import ElementTree as ET
> +
> +from kimchi.exception import InvalidParameter
> +from kimchi.utils import kimchi_log, run_command
> +
> +
> +class CPUInfoModel(object):
> +    """
> +    Get information about a CPU for hyperthreading (on x86)
> +    or SMT (on POWER) for logic when creating templates and VMs.
> +    """
> +
> +    def __init__(self, **kargs):
> +        self.conn = kargs['conn']
> +
> +        """
> +        Since there are so many similar-seeming variables:
> +        - arch = either ppc or x86 (Intel and AMD)
> +        - guest_threads_enabled = Can a user specify topology?
> +        - sockets = total number of sockets in the system. While nothing
> +            uses this value, it's part of topology so we'll track it.
> +        - cores_present = total number of cores in the system
> +            Note: Cores is often synonymous with CPUs.
> +        - cores_available = cores online (in case some were offlined)
> +        - cores_per_socket = max cores value for topology
> +        - threads_per_core = max threads value for topology
> +        """
> +
> +        self.conn = kargs['conn']
> +
> +        libvirt_topology = None
> +        try:
> +            connect = self.conn.get()
> +        except Exception as e:
> +            raise Exception("Unable to get qemu connection: %s" % e.message)
> +        try:
> +            xml = connect.getCapabilities()
> +            capabilities = ET.fromstring(xml)
> +            libvirt_topology = capabilities.find('host').find('cpu').\
> +                find('topology')
> +            if libvirt_topology is None:
> +                kimchi_log.info("cpu_info topology not supported.")
> +                self.guest_threads_enabled = False
> +                self.sockets = 0
> +                self.cores_present = 0
> +                self.cores_available = 0
> +                self.cores_per_socket = 0
> +                self.threads_per_core = 0
> +                self.max_threads = 0
> +                return
> +        except Exception as e:
> +            raise("Unable to get CPU topology capabilities: %s" % e.message)
> +
> +        if platform.machine().startswith('ppc'):
> +            # IBM PowerPC
> +            self.arch = 'ppc'
> +            self.guest_threads_enabled = True
> +            out, error, rc = run_command(['ppc64_cpu', '--smt'])
> +            if rc or 'on' in out:
> +                # SMT has to be disabled for guest to use threads as CPUs.
> +                # rc is always zero, whether SMT is off or on.
> +                self.guest_threads_enabled = False
> +            out, error, rc = run_command(['ppc64_cpu', '--cores-present'])
> +            if not rc:
> +                self.cores_present = int(out.split()[-1])
> +            out, error, rc = run_command(['ppc64_cpu', '--cores-on'])
> +            if not rc:
> +                self.cores_available = int(out.split()[-1])
> +            out, error, rc = run_command(['ppc64_cpu', '--threads-per-core'])
> +            if not rc:
> +                self.threads_per_core = int(out.split()[-1])
> +            self.sockets = self.cores_present/self.threads_per_core
> +            self.cores_per_socket = self.cores_present/self.sockets
> +        else:
> +            # Intel or AMD
> +            self.arch = 'x86'
> +            self.guest_threads_enabled = True
> +            self.sockets = int(libvirt_topology.get('sockets'))
> +            self.cores_per_socket = int(libvirt_topology.get('cores'))
> +            self.cores_present = self.cores_per_socket * self.sockets
> +            self.cores_available = psutil.NUM_CPUS
> +            self.threads_per_core = int(libvirt_topology.get('threads'))
> +
> +    def lookup(self):
> +
> +        return {
> +            'guest_threads_enabled': self.guest_threads_enabled,
> +            'sockets': self.sockets,
> +            'cores_per_socket': self.cores_per_socket,
> +            'cores_present': self.cores_present,
> +            'cores_availalble': self.cores_available,
> +            'threads_per_core': self.threads_per_core,
> +            }
> +
> +    def get_rec_topology(self, vcpus, guest=None, smtx=0):
> +        """
> +            Kimchi will provide a recommended topoology based on the
> +            number of virtual CPUs desired and guest OS.
> +
> +            param vcpus: should be an integer
> +            param guest: should be in the form 'distro:version'
> +            param thread_pref: a power of 2, or 0, if no preference (x86).
> +            return: topology numbers. If all zero, SMT/hyperthreading not
> +            supported.
> +        """
> +
> +        """
> +            Note to remove later ... This will be called from the UI,
> +            If Power, smtx should be set.
> +
> +            On x86, spreading threads over cores is better than grouping.
> +            IOW, for vcpus = 2, sockets = 1, cores = 2, threads = 1 is
> +            better than vcpus=2, sockets = 1, cores = 1, threads = 2.
> +        """
> +
> +        if not self.guest_threads_enabled:
> +            return None
> +        if vcpus > self.sockets * self.cores_available * \
> +            self.threads_per_core:
> +             # This check should go into template create too?
> +             # Or should we allow this?
> +             raise InvalidParameter("The number of vcpus is too large \
> +                                    for this system")  # @TODO: msgid
> +
> +        sockets = 1
> +        cores = 1
> +        threads = 1
> +        best_vcpus = vcpus
> +
> +        if best_vcpus == 1:
> +            return {'topology': {'sockets': sockets, 'cores': cores,
> +                                 'threads': threads},
> +                    'vcpus': best_vcpus}
> +
> +        if self.arch is 'ppc':
> +            if best_vcpus % self.threads_per_core:
> +                # Having fewer threads than threads/core will give poor
> +                # performance. Since we're auto-configuring things,
> +                # let's give the user a better vcpu value.
> +                best_vcpus = best_vcpus + (self.threads_per_core - \
> +                    (best_vcpus % self.threads_per_core))
> +
> +        if best_vcpus <= self.threads_per_core:
> +            threads = best_vcpus
> +            return {'topology': {'sockets': sockets, 'cores': cores,
> +                                 'threads': threads},
> +                    'vcpus': best_vcpus}
> +
> +        # creating a VM using REST APIs
> +        if self.arch is 'ppc' and smtx == 0:
> +            smtx = self.threads_per_core
> +
> +        if smtx:  # Power
> +            cores = best_vcpus/smtx

Somehow some lines were deleted from my local branch, so this should
have an 'else' and a return. That explains why larger values of vcpus
were resulting in the message that topology isn't supported.

Doing some more testing and I'll re-send a different version today.

> diff --git a/src/kimchi/model/host.py b/src/kimchi/model/host.py
> index 8cddcdc..7912311 100644
> --- a/src/kimchi/model/host.py
> +++ b/src/kimchi/model/host.py
> @@ -53,6 +53,7 @@ def __init__(self, **kargs):
>          self.task = TaskModel(**kargs)
>          self.host_info = self._get_host_info()
> 
> +    # @TODO: Think about moving this to the new cpuinfo class
>      def _get_ppc_cpu_info(self):
>          res = {}
>          with open('/proc/cpuinfo') as f:
> diff --git a/src/kimchi/model/templates.py b/src/kimchi/model/templates.py
> index ff1070d..4189c28 100644
> --- a/src/kimchi/model/templates.py
> +++ b/src/kimchi/model/templates.py
> @@ -25,6 +25,8 @@
>  from kimchi.exception import InvalidOperation, InvalidParameter
>  from kimchi.exception import NotFoundError, OperationFailed
>  from kimchi.kvmusertests import UserTests
> +from kimchi.model.cpuinfo import CPUInfoModel
> +from kimchi.osinfo import common_spec
>  from kimchi.utils import pool_name_from_uri
>  from kimchi.utils import probe_file_permission_as_user
>  from kimchi.vmtemplate import VMTemplate
> @@ -50,18 +52,29 @@ def create(self, params):
> 
>          cpu_info = params.get('cpu_info')
>          if cpu_info:
> +            vcpus = params.get('cpus')
>              topology = cpu_info.get('topology')
> +            auto_topology = cpu_info.get('auto_topology')
>              # Check, even though currently only topology
>              #   is supported.
>              if topology:
>                  sockets = topology['sockets']
>                  cores = topology['cores']
>                  threads = topology['threads']
> -                vcpus = params.get('cpus')
>                  if vcpus is None:
>                      params['cpus'] = sockets * cores * threads
>                  elif vcpus != sockets * cores * threads:
>                      raise InvalidParameter("KCHTMPL0025E")
> +            elif auto_topology and auto_topology is True:
> +                if vcpus is None:
> +                    vcpus = common_spec['cpus']
> +                topology = CPUInfoModel(conn=self.conn).get_rec_topology(vcpus)
> +                if topology:
> +                    params['cpus'] = topology['vcpus']
> +                    params['cpu_info']['topology'] = topology
> +                else:
> +                    raise InvalidOperation("KCHTMPL0029E")
> +
>          else:
>              params['cpu_info'] = dict()
> 




More information about the Kimchi-devel mailing list