Re: [Kimchi-devel] [PATCH v4] Parts to allow Kimchi to configure the cpu topology.

On 2014年11月13日 07:00, Christy Perez wrote: > Signed-off-by: Christy Perez <christy(a)linux.vnet.ibm.com&gt; > --- > docs/API.md | 28 +++++++ > src/kimchi/API.json | 11 +-- > src/kimchi/control/cpuinfo.py | 37 +++++++++ > src/kimchi/control/host.py | 2 + > src/kimchi/i18n.py | 3 +- > src/kimchi/model/cpuinfo.py | 172 > ++++++++++++++++++++++++++++++++++++++++++ > src/kimchi/model/host.py | 1 + > src/kimchi/model/templates.py | 25 ++++-- > 8 files changed, 263 insertions(+), 16 deletions(-) > 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..2f434b6 100644 > --- a/docs/API.md > +++ b/docs/API.md > @@ -233,6 +233,10 @@ 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. > + Only threads is required. '0' should be passed in for > users to > + take advantage of this auto-sizing feature. Then, kimchi > will create > + a topology based on vcpus (if specified) and the host's > capabilities. > + To find the host's capabilties, see the /host/cpuinfo > documentation. > > ### Sub-Collection: Virtual Machine Network Interfaces > > @@ -868,6 +872,30 @@ Contains the host sample data. > > *No actions defined* > > +### Resource: HostStats > + > +**URI:** /host/cpuinfo > + > +The cores and sockets of a hosts's CPU. Useful when sizing VMs to take > +advantages of the perforamance benefits of SMT (Power) or > Hyper-Threading (Intel). > + > +**Methods:** > + > +* **GET**: Retreives the sockets, cores, and threads values. > + * threading_enabled: Whether CPU topology is supported on this > system. > + * sockets: The number of total sockets on a system. > + * cores: The total number of cores per socket. > + * threads_per_core: The threads per core. > + > +**Actions (PUT):** > + > +*No actions defined* > + > +**Actions (POST):** > + > +*No actions defined* > + > + > ### Resource: HostStatsHistory > > **URI:** /host/stats/history > diff --git a/src/kimchi/API.json b/src/kimchi/API.json > index 0ad36ab..fb28723 100644 > --- a/src/kimchi/API.json > +++ b/src/kimchi/API.json > @@ -37,20 +37,15 @@ > "properties": { > "sockets": { > "type": "integer", > - "required": true, > - "minimum": 1, > - "error": "KCHTMPL0026E" > + "minimum": 1 > }, > "cores": { > "type": "integer", > - "required": true, > - "minimum": 1, > - "error": "KCHTMPL0026E" > + "minimum": 1 > }, > "threads": { > - "type": "integer", > "required": true, > - "minimum": 1, > + "type": "integer", > "error": "KCHTMPL0026E" > } > } > diff --git a/src/kimchi/control/cpuinfo.py > b/src/kimchi/control/cpuinfo.py > new file mode 100644 > index 0000000..415dd3d > --- /dev/null > +++ b/src/kimchi/control/cpuinfo.py > @@ -0,0 +1,37 @@ > +# > +# 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.admin_methods = ['GET'] > + self.role_key = 'host' > + self.uri_fmt = "/host/cpuinfo" > + > + @property > + def data(self): > + return {'threading_enabled': self.info['guest_threads_enabled'], > + 'sockets': self.info['sockets'], > + 'cores': self.info['cores_available'], > + 'threads_per_core': self.info['threads_per_core'] > + } > diff --git a/src/kimchi/control/host.py b/src/kimchi/control/host.py > index 4362da7..9f73653 100644 > --- a/src/kimchi/control/host.py > +++ b/src/kimchi/control/host.py > @@ -17,6 +17,7 @@ > # 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.cpuinfo import CPUInfo > from kimchi.control.base import Collection, Resource, SimpleCollection > from kimchi.control.utils import UrlSubNode > from kimchi.exception import NotFoundError > @@ -39,6 +40,7 @@ def __init__(self, model, id=None): > self.users = Users(self.model) > self.groups = Groups(self.model) > self.swupdate = self.generate_action_handler_task('swupdate') > + self.cpuinfo = CPUInfo(self.model) > > @property > def data(self): > diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py > index e823f2b..139a3dc 100644 > --- a/src/kimchi/i18n.py > +++ b/src/kimchi/i18n.py > @@ -148,8 +148,9 @@ > "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."), > + "KCHTMPL0026E": _("When specifying CPU topology, threads is > required."), > "KCHTMPL0027E": _("Invalid disk image format. Valid formats: > bochs, cloop, cow, dmg, qcow, qcow2, qed, raw, vmdk, vpc."), > + "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..b104fa3 > --- /dev/null > +++ b/src/kimchi/model/cpuinfo.py > @@ -0,0 +1,172 @@ > +# 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 > +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 = self.cores_present > + self.threads_per_core = int(libvirt_topology.get('threads')) > + > + def lookup(self, ident): > + return { > + 'guest_threads_enabled': self.guest_threads_enabled, > + 'sockets': self.sockets, > + 'cores_per_socket': self.cores_per_socket, > + 'cores_present': self.cores_present, > + 'cores_available': self.cores_available, > + 'threads_per_core': self.threads_per_core, > + } > + > + def get_rec_topology(self, vcpus, 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 (0 if x86). > + return: topology numbers. None, if SMT/HT not enabled. > + """ > + > + if not self.guest_threads_enabled: > + return None > + if vcpus > 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 smtx == 0: > + smtx = self.threads_per_core > + if best_vcpus % smtx: > + # 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 + \ > + (smtx- (best_vcpus % smtx)) > + > + if best_vcpus <= self.threads_per_core: > + threads = best_vcpus > + return {'topology': {'sockets': sockets, 'cores': cores, > + 'threads': threads}, > + 'vcpus': best_vcpus} > + > + if self.arch is 'ppc': > + cores = best_vcpus/threads > + threads = smtx Can we do build time platform type probe instead of run time, after all no one have two arch on the same machine.

> + else: # x86 > + # Because of the check above, we know that more vcpus > + # than will fit on one core were requested. Spread > + # them over as many cores as needed, as opposed to packing > + # threads onto fewer cores. > + # cores= best_vcpus/self.cores_available > + # threads = self.threads_per_core > + cores = min(self.cores_available, best_vcpus) > + threads = best_vcpus / cores > + # In case vcpus was something like '3' on a dual-thread proc, > + # make it a legal vcpu number: > + best_vcpus = cores * threads > + > + return {'topology': {'sockets': sockets, 'cores': cores, > + 'threads': threads}, > + 'vcpus': best_vcpus} > 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..dec9a43 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,15 +52,24 @@ def create(self, params): > > cpu_info = params.get('cpu_info') > if cpu_info: > + vcpus = params.get('cpus') > topology = cpu_info.get('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: > + sockets = topology.get('sockets') > + cores = topology.get('cores') > + threads = topology.get('threads') > + if sockets is None and cores is None: > + # The user wants kimchi to decide > + if vcpus is None: > + vcpus = max(common_spec['cpus'], threads) > + rec_topology = CPUInfoModel( > + conn=self.conn).get_rec_topology(vcpus, threads) > + if rec_topology: > + params['cpus'] = rec_topology['vcpus'] > + params['cpu_info']['topology'] = > rec_topology['topology'] > + else: > + raise InvalidOperation("KCHTMPL0029E") > + elif vcpus is None: > params['cpus'] = sockets * cores * threads > elif vcpus != sockets * cores * threads: > raise InvalidParameter("KCHTMPL0025E")