[Kimchi-devel] [PATCH v1] Parts to allow Kimchi to configure the cpu topology.
Christy Perez
christy at linux.vnet.ibm.com
Mon Nov 3 22:16:44 UTC 2014
On 11/03/2014 01:08 PM, Aline Manera wrote:
>
> By default, Kimchi always sets the best Template configuration for user
> based on a single input: ISO file or Image file.
> So you don't need to create an API (auto-topology) for it. It will be
> the default behavior.
>
> The flow is:
>
> - Create a Template which will get the default CPU topology according to
> host OS and ISO OS.
>
> POST /templates {name: my-tmpl, cdrom: <my-iso>}
>
> - Update the CPU topology according to your needs:
>
> PUT /templates/my-tmpl {cpu_info: {...}}
>
> Ok?
I was assuming we only did that for things that are required to use a VM
and kimchi (cpus, memory, a console), etc., but if you think we should
set this without being asked, I am okay with that. As long as users
know, and can disable it if it doesn't fit their workloads.
Currently there's no way to remove the topology from a template once
it's added, so that will need to be done ASAP for this flow.
>
> See more comments inline below:
>
>
>
> On 11/03/2014 12:36 AM, 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 | 37 +++++++++
>> src/kimchi/i18n.py | 2 +
>> src/kimchi/model/cpuinfo.py | 176
>> ++++++++++++++++++++++++++++++++++++++++++
>> src/kimchi/model/host.py | 1 +
>> src/kimchi/model/templates.py | 13 +++-
>> 7 files changed, 236 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 9c06f85..4e449b3 100644
>> --- a/docs/API.md
>> +++ b/docs/API.md
>> @@ -226,6 +226,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..3a295ad
>> --- /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.uri_fmt = "/host/cpu_info"
>> +
>> + @property
>> + def data(self):
>> + return {'arch': self.info['arch'],
>> + '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']
>> + }
> ----
> All that will not needed.
I think it's nice to let users know how many cores, etc., are on their
system so they can plan accordingly. For example, if a Power system has
been changed to split-core mode, a user might want to know that they
only have 4 threads per core now instead of 8.
I also wanted to have this so that the UI would have something to call
so they'd know what to display (HT, SMTX, etc.).
>
>
>> diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py
>> index f789259..7669dbc 100644
>> --- a/src/kimchi/i18n.py
>> +++ b/src/kimchi/i18n.py
>> @@ -146,6 +146,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..12f5899
>> --- /dev/null
>> +++ b/src/kimchi/model/cpuinfo.py
>
> As this file will not needed, you can move the functions needed to
> identify the topology to
> a new module model/cputopology.py or something like it.
>
>> @@ -0,0 +1,176 @@
>> +# 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 libvirt
>> +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):
>> + """ 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
>> + """
>> + libvirt_topology = None
>
>> + try:
>> + connect = libvirt.open(None)
>
> Use LibvirtConnection to establish a connection to libvirt.
> One of the args on "kargs" is "conn" which is already an
> LibvirtConnection instance.
ACK
>
>> + 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')
>
> Only rc!=0 is not enough to check it?
I'm not sure what you mean here. Are you referring to the check for the
'out' message below?
>
> And isn't there a libvirt API to ppc64_cpu?
It doesn't work correctly right now. This is a well-known issue and
ppc64_cpu is a well-known workaround until its fixed.
>
>> + if rc or out is not 'SMT is off':
>> + # SMT has to be disabled for guest to use threads as
>> CPUs.
>> + self.guest_threads_enabled = False
>> + out, error, rc = run_command('ppc64_cpu', '--cores_present')
>> + if not rc:
>> + self.cores_present = out.split()[-1]
>> + out, error, rc = run_command('ppc64_cpu', '--cores_on')
>> + if not rc:
>> + self.cores_available = out.split()[-1]
>> + out, error, rc = run_command('ppc64_cpu',
>> '--threads_per_core')
>> + if not rc:
>> + self.threads_per_core = out.split()[-1]
>> + # This next line doesn't make sense as written, but takes
>> advantage
>> + # of the fact that libvirt for ppc reports all the cores
>> on the
>> + # system as the cores value in topology (e.g. sockets:
>> 1, cores:
>> + # 128, threads: 1)
>> + # @TODO: When libvirt on Power starts returning the
>> correct info,
>> + # we should be able to remove the seperate logic (or at
>> least put
>> + # in a check for the libvirt version, b/c that will
>> break this).
>> + self.cores_per_socket = \
>> + libvirt.topology.get('cores')/self.threads_per_core
>> + # Nothing actually uses 'sockets,' but set it anyway
>> + self.sockets = 1
>> + 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):
>> + """
>> + The UI can use this data to decide what to display:
>> + - SMT (arch = ppc) or HT (arch = x86)
>> + - If HT:
>> + 1. the UI can look at cores_present and
>> + warn the user if he/she creates a VM with more
>> + vCPUs than cores (bad performance).
>> + 2. The UI could include a suggestion of 'cores_present'
>> + or fewer vCPUs next to the CPU box.
>> + 3. There could be an actual limit on the number of vCPUs.
>> + A user can use this data to decide how to best manually
>> + create a VM or template.
>> + """
>> + return {
>> + 'arch': self.arch,
>> + '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.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
>> +
>> + if vcpus == 1:
>> + return {"sockets": sockets, "cores": cores, "threads":
>> threads}
>> +
>
>
>
>> + # creating a VM using REST APIs
>> + if self.arch is 'ppc' and smtx == 0:
>> + smtx = self.threads_per_core
>> +
>> + if smtx: # Power
>> + cores = vcpus/smtx
>> + threads = smtx
>> +
>> + else: # x86
>> + # The scheduler will move these around, but we'll split
>> + # it up anyway.
>> + cores = self.cores_available
>> + threads = vcpus/cores
>> +
>> + return {"sockets": sockets, "cores": cores, "threads": threads}
>> 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..e402260 100644
>> --- a/src/kimchi/model/templates.py
>> +++ b/src/kimchi/model/templates.py
>> @@ -25,6 +25,7 @@
>> 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.utils import pool_name_from_uri
>> from kimchi.utils import probe_file_permission_as_user
>> from kimchi.vmtemplate import VMTemplate
>> @@ -50,18 +51,28 @@ 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 = 1
>
> Why are you assuming it? Isn't this value same of params['cpus'] ?
> The default values come from osinfo.py and we should respect them.
Good point. I'll look at the value set as the default instead.
>
>> + topology = CPUInfoModel().get_rec_topology(vcpus)
>> + if topology:
>> + params['cpu_info']['topology'] = topology
>> + else:
>> + raise InvalidOperation("KCHTMPL0029E")
>> +
>> else:
>> params['cpu_info'] = dict()
>>
>
More information about the Kimchi-devel
mailing list