[PATCH v4] Auto Topology for Guest Templates

This is another rework of how this flow should work, after more thought and discussion with Aline. This new flow removes the 'auto-topology' element from the API and makes it so that users can enter varying information without being dinged for not doing exactly the right thing (e.g. vcpus != sockets * cores * threads). It also works in a way that doesn't require the UI to have access to the kimchi Model objects (as I misremembered for the previous patches). 1. UI calls GET host/cpuinfo curl -k -u user -H 'Content-Type: application/json' \ -H 'Accept: application/json' https://localhost:8001/host/cpuinfo { "cores":2, "threading_enabled":true, "sockets":1, "threads_per_core":2 } 2. UI displays appropriate title and checkboxes - If host arch is Power: - Display a menu labled SMT, with up to four check-boxes. The check-boxes will be labeled SMT1, SMT2, SMT4, and SMT8. The max SMT value displayed will be dependant upon threads_per_core from above. The default checkbox should be the greatest SMT value (e.g. SMT8). - Display the same box for CPUs as before. This value may change when the backend template creation is done, so if there's any way to pop up "Here are the values of your new template," kind of message, that would be nice. Otherwise, we'll just make it clear in the doc that the vCPUs value entered is not guaranteed. - Else: - Display a menu labeled Hyper-Threading, with only a checkbox labeled HT. Note: The same comments for vCPUS as above apply here. The number maybe need to change based on the CPU capabilities. 3. UI calls POST template with threads value set to '0' (x86) or SMTX (Power) Note that vcpus is still optional, so no change in how that's handled needs to be done. $ curl -k -u user -X POST -H 'Content-Type: application/json' -H 'Accept: application/json' https://localhost:8001/templates -d'{"name": "test_auto_topo", "cdrom": "/home/iso-pool/Fedora-Live-Desktop-x86_64-20-1.iso", "cpu_info": {"topology": {"threads":0}}}' { "cpus":1, "cpu_info":{ "topology":{ "cores":1, "threads":1, "sockets":1 } }, "graphics":{ "type":"vnc", "listen":"127.0.0.1" }, "cdrom":"/home/iso-pool/Fedora-Live-Desktop-x86_64-20-1.iso", "networks":[ "default" ], "icon":"images/icon-fedora.png", "os_distro":"fedora", "name":"test_auto_topo", "disks":[ { "index":0, "size":10 } ], "invalid":{}, "os_version":"20", "storagepool":"/storagepools/default", "memory":1024, "folder":[] } Regards, - Christy Christy Perez (1): Parts to allow Kimchi to configure the cpu topology. 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 -- 1.9.3

Signed-off-by: Christy Perez <christy@linux.vnet.ibm.com> --- 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 + 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") -- 1.9.3

On 2014年11月13日 07:00, Christy Perez wrote:
Signed-off-by: Christy Perez <christy@linux.vnet.ibm.com> --- 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")

On 11/13/2014 01:32 AM, Royce Lv wrote:
On 2014年11月13日 07:00, Christy Perez wrote:
Signed-off-by: Christy Perez <christy@linux.vnet.ibm.com> --- 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.
Sure. I just put this line at the top of cpuinfo.py: ARCH = 'ppc' if platform.machine().startswith('ppc') else 'x86' I tried putting that into host.py, but some circular dependencies prevented me from importing it, so, I just created the one local ARCH for cpuinfo. Thanks!
+ 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")

On 11/12/2014 09:00 PM, Christy Perez wrote:
This is another rework of how this flow should work, after more thought and discussion with Aline.
This new flow removes the 'auto-topology' element from the API and makes it so that users can enter varying information without being dinged for not doing exactly the right thing (e.g. vcpus != sockets * cores * threads).
It also works in a way that doesn't require the UI to have access to the kimchi Model objects (as I misremembered for the previous patches).
1. UI calls GET host/cpuinfo
curl -k -u user -H 'Content-Type: application/json' \ -H 'Accept: application/json' https://localhost:8001/host/cpuinfo
{ "cores":2, "threading_enabled":true, "sockets":1, "threads_per_core":2 }
2. UI displays appropriate title and checkboxes - If host arch is Power: - Display a menu labled SMT, with up to four check-boxes. The check-boxes will be labeled SMT1, SMT2, SMT4, and SMT8. The max SMT value displayed will be dependant upon threads_per_core from above. The default checkbox should be the greatest SMT value (e.g. SMT8).
- Display the same box for CPUs as before. This value may change when the backend template creation is done, so if there's any way to pop up "Here are the values of your new template," kind of message, that would be nice. Otherwise, we'll just make it clear in the doc that the vCPUs value entered is not guaranteed.
I don't like this approach. As an user I like to inform the CPU number and be sure it will be in use. Maybe we could change the way we display it to user. I checked on virt-manager and it allows user informs the 3 values: sockets, threads and cores. The values of threads are the SMTX values. So we can display them as a combo box according to /host/cpu_info response. The other 2 values could be collected as an input box. And the CPU will be themultiplication of those values. So on UI we could have a new tab on Edit Template named "Processor". In the first view we will have: CPU number: [______________] [ ] Manually inform CPU topology Once the "Manually inform CPU topology" is selected, the expand to show 3 new fields and make the "CPU number" field disabled. Sockets: [______________] Cores : [______________] Threads: [______________] => this is a combo box with the SMTX options (only the numbers) On values changes in those 3 fields the "CPU number" is updated to display sockets * cores * threads And with this approach we don't need to differ x86 from ppc.
- Else: - Display a menu labeled Hyper-Threading, with only a checkbox labeled HT. Note: The same comments for vCPUS as above apply here. The number maybe need to change based on the CPU capabilities.
3. UI calls POST template with threads value set to '0' (x86) or SMTX (Power) Note that vcpus is still optional, so no change in how that's handled needs to be done.
$ curl -k -u user -X POST -H 'Content-Type: application/json' -H 'Accept: application/json' https://localhost:8001/templates -d'{"name": "test_auto_topo", "cdrom": "/home/iso-pool/Fedora-Live-Desktop-x86_64-20-1.iso", "cpu_info": {"topology": {"threads":0}}}'
{ "cpus":1, "cpu_info":{ "topology":{ "cores":1, "threads":1, "sockets":1 } }, "graphics":{ "type":"vnc", "listen":"127.0.0.1" }, "cdrom":"/home/iso-pool/Fedora-Live-Desktop-x86_64-20-1.iso", "networks":[ "default" ], "icon":"images/icon-fedora.png", "os_distro":"fedora", "name":"test_auto_topo", "disks":[ { "index":0, "size":10 } ], "invalid":{}, "os_version":"20", "storagepool":"/storagepools/default", "memory":1024, "folder":[] }
Regards,
- Christy
Christy Perez (1): Parts to allow Kimchi to configure the cpu topology.
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

On 11/17/2014 12:58 PM, Aline Manera wrote:
On 11/12/2014 09:00 PM, Christy Perez wrote:
This is another rework of how this flow should work, after more thought and discussion with Aline.
This new flow removes the 'auto-topology' element from the API and makes it so that users can enter varying information without being dinged for not doing exactly the right thing (e.g. vcpus != sockets * cores * threads).
It also works in a way that doesn't require the UI to have access to the kimchi Model objects (as I misremembered for the previous patches).
1. UI calls GET host/cpuinfo
curl -k -u user -H 'Content-Type: application/json' \ -H 'Accept: application/json' https://localhost:8001/host/cpuinfo { "cores":2, "threading_enabled":true, "sockets":1, "threads_per_core":2 }
2. UI displays appropriate title and checkboxes - If host arch is Power: - Display a menu labled SMT, with up to four check-boxes. The check-boxes will be labeled SMT1, SMT2, SMT4, and SMT8. The max SMT value displayed will be dependant upon threads_per_core from above. The default checkbox should be the greatest SMT value (e.g. SMT8).
- Display the same box for CPUs as before. This value may change when the backend template creation is done, so if there's any way to pop up "Here are the values of your new template," kind of message, that would be nice. Otherwise, we'll just make it clear in the doc that the vCPUs value entered is not guaranteed.
I don't like this approach. As an user I like to inform the CPU number and be sure it will be in use.
I didn't really like it either, so I've redone things in the backend and this won't happen. Can we pick up this discussion on my next (v5) patchset? I'm about to test it, and then I'll send it out.
Maybe we could change the way we display it to user. I checked on virt-manager and it allows user informs the 3 values: sockets, threads and cores.
The values of threads are the SMTX values. So we can display them as a combo box according to /host/cpu_info response. The other 2 values could be collected as an input box. And the CPU will be themultiplication of those values.
So on UI we could have a new tab on Edit Template named "Processor".
In the first view we will have:
CPU number: [______________]
[ ] Manually inform CPU topology
("Manually configure CPU topology." sounds better) I like this option a lot. I thought that you were against it from the beginning, though, and that you only wanted users to be able to enter these values if there was some kind of "advanced" part of kimchi (to be added later). If the UI passes in those values, the backend can already handle it (since the topology bits have been in kimchi backend for a while now). But -- again -- let's pick this up on my next patchset. I have already been guilty of fragmenting these discussions over the UI patchsets and my different ones. :) I'll assume that the user has the ability to do either automatic, or manual when I write up the v5 summary.
Once the "Manually inform CPU topology" is selected, the expand to show 3 new fields and make the "CPU number" field disabled.
Sockets: [______________] Cores : [______________] Threads: [______________] => this is a combo box with the SMTX options (only the numbers)
On values changes in those 3 fields the "CPU number" is updated to display sockets * cores * threads
And with this approach we don't need to differ x86 from ppc.
- Else: - Display a menu labeled Hyper-Threading, with only a checkbox labeled HT. Note: The same comments for vCPUS as above apply here. The number maybe need to change based on the CPU capabilities.
3. UI calls POST template with threads value set to '0' (x86) or SMTX (Power) Note that vcpus is still optional, so no change in how that's handled needs to be done.
$ curl -k -u user -X POST -H 'Content-Type: application/json' -H 'Accept: application/json' https://localhost:8001/templates -d'{"name": "test_auto_topo", "cdrom": "/home/iso-pool/Fedora-Live-Desktop-x86_64-20-1.iso", "cpu_info": {"topology": {"threads":0}}}'
{ "cpus":1, "cpu_info":{ "topology":{ "cores":1, "threads":1, "sockets":1 } }, "graphics":{ "type":"vnc", "listen":"127.0.0.1" }, "cdrom":"/home/iso-pool/Fedora-Live-Desktop-x86_64-20-1.iso", "networks":[ "default" ], "icon":"images/icon-fedora.png", "os_distro":"fedora", "name":"test_auto_topo", "disks":[ { "index":0, "size":10 } ], "invalid":{}, "os_version":"20", "storagepool":"/storagepools/default", "memory":1024, "folder":[] }
Regards,
- Christy
Christy Perez (1): Parts to allow Kimchi to configure the cpu topology.
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
participants (3)
-
Aline Manera
-
Christy Perez
-
Royce Lv