[Kimchi-devel] [PATCH v1] Parts to allow Kimchi to configure the cpu topology.
Aline Manera
alinefm at linux.vnet.ibm.com
Wed Nov 5 11:44:34 UTC 2014
On 11/03/2014 08:16 PM, Christy Perez wrote:
>
> 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.
On edit Template I plan to add a new combo box for SMT configuration
with the options: off, smt2, smt4, smt8
>
>> 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.).
Is it really needed? It seems a very complex information to display a
Kimchi user.
As I said before I was planning to allow the following options to user:
off, smt2, smt4, smt8
>
>>
>>> 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?
Yeap. Sorry, I inserted the comment in the wrong line
My point is, comparing output string is an easy point of failure.
Couldn't we rely only with rc code?
>> 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