[Kimchi-devel] [PATCH] [Ginger-Base 1/1] Issue 728 - Processor Info in s390 architecture
Suresh Babu Angadi
sureshab at linux.vnet.ibm.com
Tue Nov 24 06:58:35 UTC 2015
On 11/23/2015 11:24 PM, Abhiram wrote:
> Overall code looks great to me with some comments/queries. Comments
> inline!!
>
> On Mon, 2015-11-23 at 17:33 +0530, sureshab at linux.vnet.ibm.com wrote:
>> From: Suresh Babu Angadi <sureshab at in.ibm.com>
>>
>> Fix for issue 728: processor info displays
>> blank for system z this patch set also adds additional capability: retrieving
>> architecture and host name (for all architecture) split CPUs to show online
>> and offline cpus(for x86 and s390x) split memory to show online and offline
>> memory(for s390x) additional virtualization details(for s390x):
>> virtualization will have hypervisor details and lpar details
>>
>> Signed-off-by: Suresh Babu Angadi <sureshab at in.ibm.com>
>> ---
>> src/wok/plugins/gingerbase/docs/API.md | 22 ++-
>> src/wok/plugins/gingerbase/i18n.py | 1 +
>> src/wok/plugins/gingerbase/lscpu.py | 59 ++++++
>> src/wok/plugins/gingerbase/model/host.py | 256 +++++++++++++++++++++++---
>> src/wok/plugins/gingerbase/tests/test_host.py | 12 +-
>> 5 files changed, 320 insertions(+), 30 deletions(-)
>>
>> diff --git a/src/wok/plugins/gingerbase/docs/API.md b/src/wok/plugins/gingerbase/docs/API.md
>> index 8f37ddc..e1b380e 100644
>> --- a/src/wok/plugins/gingerbase/docs/API.md
>> +++ b/src/wok/plugins/gingerbase/docs/API.md
>> @@ -96,12 +96,30 @@ Contains information of host.
>>
>> * **GET**: Retrieve host static information
>> * memory: Total size of host physical memory
>> - The unit is Bytes
>> + For s390x architecture this information is a json with keys
>> + * online: Total online memory
>> + * offline: Total offline memory
>> + The unit is Bytes
>> * cpu_model: The model name of host CPU
>> - * cpus: The number of online CPUs available on host
>> + * cpus: The number of online CPUs available on host for power pc
> Here below you are mentioning cpus for different architecture, but for
> power pc is it in main headline. Doc is confusing. Do you want to have
> separate section for power pc architecture?
Accepted.
>> + For s390x architecture this information is a json with keys
>> + * online: Number of online CPUs
>> + * offline: Number of offline CPUs
>> + * shared: Number of shared CPUs
>> + * dedicated: Number of dedicated CPUs
>> + For other architectures this information is a json with keys
>> + * online: Number of online CPUs
>> + * offline: Number of offline CPUs
>> * os_distro: The OS distribution that runs on host
>> * os_version: The version of OS distribution
>> * os_codename: The code name of OS distribution
>> + * host: Host name of the network node
>> + * architecture: Architecture of the host
>> + * virtualization *(for s390x)*: This is a json with following keys
> Everything in API.md defined is key:value json structure right? Do you
> want to mention separately "is a json" ?
Agreed.
>> + * hypervisor: Hypervisor name of the host
>> + * hypervisor_vendor: Hypervisor Vendor name
>> + * lpar_number: LPAR Number of host
>> + * lpar_name: Name of host LPAR
>>
>> * **POST**: *See Host Actions*
>>
>> diff --git a/src/wok/plugins/gingerbase/i18n.py b/src/wok/plugins/gingerbase/i18n.py
>> index af75c70..a0a8c36 100644
>> --- a/src/wok/plugins/gingerbase/i18n.py
>> +++ b/src/wok/plugins/gingerbase/i18n.py
>> @@ -88,5 +88,6 @@ messages = {
>> "GGBCPUINF0005E": _("This host (or current configuration) does not provide Socket(s) information."),
>> "GGBCPUINF0006E": _("This host (or current configuration) does not provide Core(s) per socket information."),
>> "GGBCPUINF0007E": _("This host (or current configuration) does not provide Thread(s) per core information."),
>> + "GGBCPUINF0008E": _("This host (or current configuration) does not provide CPU(s) information."),
>>
>> }
>> diff --git a/src/wok/plugins/gingerbase/lscpu.py b/src/wok/plugins/gingerbase/lscpu.py
>> index 0fc1e72..7ad6470 100644
>> --- a/src/wok/plugins/gingerbase/lscpu.py
>> +++ b/src/wok/plugins/gingerbase/lscpu.py
>> @@ -64,6 +64,28 @@ class LsCpu(object):
>> # L2 cache: 256K
>> # L3 cache: 3072K
>> # NUMA node0 CPU(s): 0-3
>> + #
>> + # Output of lscpu in s390x is expected to be
>> + # Architecture: s390x
>> + # CPU op-mode(s): 32-bit, 64-bit
>> + # Byte Order: Big Endian
>> + # CPU(s): 4
>> + # On-line CPU(s) list: 0,1
>> + # Off-line CPU(s) list: 2,3
>> + # Thread(s) per core: 1
>> + # Core(s) per socket: 6
>> + # Socket(s) per book: 6
>> + # Book(s): 4
>> + # Vendor ID: IBM/S390
>> + # BogoMIPS: 18115.00
>> + # Hypervisor: PR/SM
>> + # Hypervisor vendor: IBM
>> + # Virtualization type: full
>> + # Dispatching mode: horizontal
>> + # L1d cache: 96K
>> + # L1i cache: 64K
>> + # L2d cache: 1024K
>> + # L2i cache: 1024K
>>
>> if not rc and (not out.isspace()):
>> lscpuout = out.split('\n')
>> @@ -85,8 +107,12 @@ class LsCpu(object):
>> """
>> try:
>> sockets = "Socket(s)"
>> + sockets_per_book = "Socket(s) per book"
> rather then initializing both variables is it better to initialize based
> on arch one socket variable? i am assuming "Socket" is default and for
> s390 "Socket(s) per book"
> if s390:
> sockets = "Socket(s) per book"
> else:
> sockets = "Socket(s)"
With current implementation of KVM on Z, lscpu will have only sockets =
"Socket(s) per book". Will change in V2.
>
>> if len(self.lsCpuInfo) > 0 and sockets in self.lsCpuInfo.keys():
>> return int(self.lsCpuInfo[sockets])
>> + elif len(self.lsCpuInfo) > 0 and sockets_per_book\
>> + in self.lsCpuInfo.keys():
>> + return int(self.lsCpuInfo[sockets_per_book])
>> else:
>> raise NotFoundError("GGBCPUINF0005E")
>> except IndexError, e:
>> @@ -124,3 +150,36 @@ class LsCpu(object):
>> except IndexError, e:
>> self.log_error(e)
>> raise NotFoundError("GGBCPUINF0007E")
>> +
>> + def get_total_cpus(self):
>> + """
>> +
>> + :return:
>> + """
>> + total_cpus = 'CPU(s)'
>> + if len(self.lsCpuInfo) > 0 and total_cpus in self.lsCpuInfo.keys():
>> + return int(self.lsCpuInfo[total_cpus])
>> + else:
>> + self.log_error("Failed to fetch total cpus count in lscpu output")
>> + raise NotFoundError("GGBCPUINF0008E")
>> +
>> + def get_hypervisor(self):
>> + """
>> + method to get hypervisor name if present in lscpu o/p
>> + :return: Hypervisor Name
>> + """
>> + hypervisor = 'Hypervisor'
>> + if len(self.lsCpuInfo) > 0 and hypervisor in self.lsCpuInfo.keys():
>> + return self.lsCpuInfo[hypervisor]
>> + return None
>> +
>> + def get_hypervisor_vendor(self):
>> + """
>> + method to get hypervisor vendor if present in lscpu o/p
>> + :return: Hypervisor Vendor
>> + """
>> + hypervisor_vendor = 'Hypervisor vendor'
>> + if len(self.lsCpuInfo) > 0 and hypervisor_vendor in \
>> + self.lsCpuInfo.keys():
>> + return self.lsCpuInfo[hypervisor_vendor]
>> + return None
> In above 3 methods the key check code is duplicated, we can have one
> common method to check key?
In some methods, the check raises NotFoundError. So I would like to keep
the current implementation as is.
>> diff --git a/src/wok/plugins/gingerbase/model/host.py b/src/wok/plugins/gingerbase/model/host.py
>> index a58bada..42db7eb 100644
>> --- a/src/wok/plugins/gingerbase/model/host.py
>> +++ b/src/wok/plugins/gingerbase/model/host.py
>> @@ -22,6 +22,7 @@
>> import os
>> import platform
>> import psutil
>> +import re
>> import time
>> from cherrypy.process.plugins import BackgroundTask
>> from collections import defaultdict
>> @@ -31,9 +32,10 @@ from wok.basemodel import Singleton
>> from wok.config import config as kconfig
>> from wok.exception import InvalidOperation
>> from wok.exception import OperationFailed
>> -from wok.utils import add_task, wok_log
>> +from wok.utils import add_task, run_command, wok_log
>> from wok.model.tasks import TaskModel
>>
>> +from wok.plugins.gingerbase.lscpu import LsCpu
>> from wok.plugins.gingerbase.model.debugreports import DebugReportsModel
>> from wok.plugins.gingerbase.repositories import Repositories
>> from wok.plugins.gingerbase.swupdate import SoftwareUpdate
>> @@ -48,17 +50,26 @@ DOM_STATE_MAP = {0: 'nostate',
>> 6: 'crashed',
>> 7: 'pmsuspended'}
>>
>> +ARCH = platform.machine()
>> +PROC_CPUINFO = '/proc/cpuinfo'
>> +PROC_SYSINFO = '/proc/sysinfo'
>> +LSMEM = 'lsmem'
>> +CPUS_DEDICATED = 'cpus_dedicated'
>> +CPUS_SHARED = 'cpus_shared'
>> +LPAR_NAME = 'lpar_name'
>> +LPAR_NUMBER = 'lpar_number'
>> +
>>
>> class HostModel(object):
>> def __init__(self, **kargs):
>> # self.conn = kargs['conn']
>> self.objstore = kargs['objstore']
>> self.task = TaskModel(**kargs)
>> - self.host_info = self._get_host_info()
>> + self.lscpu = LsCpu()
>>
>> def _get_ppc_cpu_info(self):
>> res = {}
>> - with open('/proc/cpuinfo') as f:
>> + with open(PROC_CPUINFO) as f:
>> for line in f.xreadlines():
>> # Parse CPU, CPU's revision and CPU's clock information
>> for key in ['cpu', 'revision', 'clock']:
>> @@ -81,32 +92,212 @@ class HostModel(object):
>>
>> return ""
>>
>> - def _get_host_info(self):
>> - res = {}
>> - if platform.machine().startswith('ppc'):
>> - res['cpu_model'] = self._get_ppc_cpu_info()
>> - else:
>> - with open('/proc/cpuinfo') as f:
>> + def _get_ppc_host_info(self):
>> + """
>> + method to get host information for power pc
>> + :return: dictionary
>> + """
>> + host_info = {}
>> + host_info.update(self._get_base_info())
>> + host_info['memory'] = self._get_total_memory()
>> + host_info['cpus'] = self._get_online_cpus_()
>> + host_info['cpu_model'] = self._get_ppc_cpu_info()
>> + return host_info
>> +
>> + def _get_x86_host_info(self):
>> + """
>> + method to get host information for x86 architecture
>> + :return: dictionary
>> + """
>> + host_info = {}
>> + host_info.update(self._get_base_info())
>> + host_info['memory'] = self._get_total_memory()
>> + host_info['cpus'] = self._get_cpus()
>> + host_info['cpu_model'] = ""
>> + try:
>> + with open(PROC_CPUINFO) as f:
>> for line in f.xreadlines():
>> if "model name" in line:
>> - res['cpu_model'] = line.split(':')[1].strip()
>> + host_info['cpu_model'] = line.split(':')[1].strip()
>> break
>> -
>> - res['cpus'] = 0
>> - res['memory'] = 0L
>> -
>> + except Exception as e:
>> + wok_log.error("Failed to retrive cpu_model for "
>> + "%s. Error: %s", ARCH, e.__str__())
>> + return host_info
>> +
>> + def _get_s390x_host_info(self):
>> + """
>> + method to get host information for s390x architecture
>> + :return: dictionary
>> + """
>> + host_info = {}
>> + host_info.update(self._get_base_info())
>> + host_info['memory'] = self._get_s390x_memory()
>> + host_info['cpus'] = self._get_cpus()
>> + host_info['cpus']['dedicated'] = 0
>> + host_info['cpus']['shared'] = 0
>> + host_info['cpu_model'] = ""
>> + host_info['virtualization'] = {}
>> + s390x_sysinfo = self._get_s390x_sysinfo()
>> + if 'manufacturer' in s390x_sysinfo.keys():
>> + host_info['cpu_model'] = s390x_sysinfo['manufacturer']
>> + if 'type' in s390x_sysinfo.keys():
>> + host_info['cpu_model'] = \
>> + host_info['cpu_model'] + "/" + s390x_sysinfo['type']
>> + if 'model' in s390x_sysinfo.keys():
>> + host_info['cpu_model'] = \
>> + host_info['cpu_model'] + "/" + s390x_sysinfo['model']
> Query:--- Here you are constructing cpu_model as manufacturer/type/model
> if i am right. If so always the output should be
> s390x_sysinfo['manufacturer']+"/" + s390x_sysinfo['type']+"/" +
> s390x_sysinfo['model']
> If for some reasons you are suspecting the keys will not be there is it
> ok to just append the info available?
Since this being basic information, I would prefer to keep the available
information than ignoring complete information.
>> + if CPUS_DEDICATED in s390x_sysinfo.keys():
>> + host_info['cpus']['dedicated'] = s390x_sysinfo[CPUS_DEDICATED]
>> + if CPUS_SHARED in s390x_sysinfo.keys():
>> + host_info['cpus']['shared'] = s390x_sysinfo[CPUS_SHARED]
>> + host_info['virtualization']['hypervisor'] = \
>> + self.lscpu.get_hypervisor()
>> + host_info['virtualization']['hypervisor_vendor'] = \
>> + self.lscpu.get_hypervisor_vendor()
>> + host_info['virtualization'][LPAR_NAME] = ''
>> + host_info['virtualization'][LPAR_NUMBER] = ''
>> + if LPAR_NAME in s390x_sysinfo.keys():
>> + host_info['virtualization'][LPAR_NAME] = s390x_sysinfo[LPAR_NAME]
>> + if LPAR_NUMBER in s390x_sysinfo.keys():
>> + host_info['virtualization'][LPAR_NUMBER] = \
>> + s390x_sysinfo[LPAR_NUMBER]
>> +
>> + return host_info
>> +
>> + def _get_cpus(self):
>> + """
>> + method to retrieve online cpus count and offline cpus
>> + count for x86 and s390x architecture
>> + :return: dictionary with keys "online" and "offline"
>> + """
>> + cpus = {}
>> + total_cpus = int(self.lscpu.get_total_cpus())
>> + online_cpus = int(self._get_online_cpus_())
>> + offline_cpus = 0
>> + if total_cpus >= online_cpus:
>> + offline_cpus = total_cpus - online_cpus
>> + cpus['online'] = online_cpus
>> + cpus['offline'] = offline_cpus
>> + return cpus
>> +
>> + def _get_s390x_memory(self):
>> + """
>> + method to retrieve memory information for s390x
>> + :return: dictionary with keys "online" and "offline"
>> + """
>> + memory = {}
>> + online_memory = 0
>> + offline_memory = 0
>> + online_mem_pat = r'^Total online memory :\s+(\d+)\s+MB$'
>> + offline_mem_pat = r'^Total offline memory:\s+(\d+)\s+MB$'
>> + out, err, rc = run_command(LSMEM)
>> + # output of lsmem in s390x architecture is expected to be
>> + # Address Range Size (MB) State\
>> + # Removable Device
>> + # ========================================================\
>> + # =======================
>> + # 0x0000000000000000-0x000000000fffffff 256 online\
>> + # no 0
>> + # 0x0000000010000000-0x000000002fffffff 512 online\
>> + # yes 1-2
>> + # 0x0000000030000000-0x000000007fffffff 1280 online\
>> + # no 3-7
>> + # 0x0000000080000000-0x00000000ffffffff 2048 offline\
>> + # - 8-15
>> + #
>> + # Memory device size : 256 MB
>> + # Memory block size : 256 MB
>> + # Total online memory : 2048 MB
>> + # Total offline memory: 2048 MB
>> + if not rc:
>> + online_mem = re.search(online_mem_pat, out.strip(), re.M | re.I)
>> + offline_mem = re.search(offline_mem_pat, out.strip(), re.M | re.I)
>> + if online_mem and len(online_mem.groups()) == 1:
>> + online_memory = int(online_mem.group(1)) * 1024 * 1024
>> + # converting MB to bytes
>> + # lsmem always returns memory in MB
>> + if offline_mem and len(offline_mem.groups()) == 1:
>> + offline_memory = int(offline_mem.group(1)) * 1024 * 1024
>> + else:
>> + wok_log.error('Failed to retrieve memory information with command'
>> + '%s. Error: %s' % (LSMEM, err))
>> + memory['online'] = online_memory
>> + memory['offline'] = offline_memory
>> + return memory
>> +
>> + def _get_s390x_sysinfo(self):
>> + """
>> + This method retrieves following system information
>> + for s390 architecture
>> + * manufacturer: Manufacturer of host machine
>> + * type: Type of the host machine
>> + * model:Model of host machine
>> + * LPAR_NUMBER: LPAR Number of host
>> + * LPAR_NAME: Name of host LPAR
>> + * CPUS_DEDICATED: LPAR CPUs Dedicated
>> + * CPUS_SHARED: LPAR CPUs Shared
>> +
>> + :param self: object of the class self
>> + :return: dictionary with following keys -
>> + 'manufacturer', 'type', 'model', CPUS_SHARED,
>> + CPUS_DEDICATED, LPAR_NUMBER, LPAR_NAME
>> + """
>> + s390x_sysinfo = {}
>> + try:
>> + with open(PROC_SYSINFO) as f:
>> + for line in f.xreadlines():
>> + if ":" in line and (len(line.split(':')) == 2):
>> + info = line.split(':')
>> + if info[0] == 'Model' and (len(info[1].split()) == 2):
>> + s390x_sysinfo['model'] = \
>> + info[1].split()[0].strip() +\
>> + " "+info[1].split()[1].strip()
>> + elif info[0] == 'Manufacturer':
>> + s390x_sysinfo['manufacturer'] = info[1].strip()
>> + elif info[0] == 'Type':
>> + s390x_sysinfo['type'] = info[1].strip()
>> + elif info[0] == 'LPAR Number':
>> + s390x_sysinfo[LPAR_NUMBER] = int(info[1].strip())
>> + elif info[0] == 'LPAR Name':
>> + s390x_sysinfo[LPAR_NAME] = info[1].strip()
>> + elif info[0] == 'LPAR CPUs Dedicated':
>> + s390x_sysinfo[CPUS_DEDICATED] =\
>> + int(info[1].strip())
>> + elif info[0] == 'LPAR CPUs Shared':
>> + s390x_sysinfo[CPUS_SHARED] = int(info[1].strip())
>> + except Exception as e:
>> + wok_log.error("Failed to retrieve information from %s file. "
>> + "Error: %s", PROC_SYSINFO, e.__str__())
>> +
>> + return s390x_sysinfo
>> +
>> + def _get_base_info(self):
>> + """
>> + method to retrieve common host information for all architectures
>> + :return: dictionary with keys 'os_distro', 'os_version', 'os_codename'
>> + 'architecture', 'host'
>> + """
>> + common_info = {}
>> # Include IBM PowerKVM name to supported distro names
>> _sup_distros = platform._supported_dists + ('ibm_powerkvm',)
>> # 'fedora' '17' 'Beefy Miracle'
>> distro, version, codename = platform.linux_distribution(
>> supported_dists=_sup_distros)
>> - res['os_distro'] = distro
>> - res['os_version'] = version
>> - res['os_codename'] = unicode(codename, "utf-8")
>> -
>> - return res
>> -
>> - def lookup(self, *name):
>> + common_info['os_distro'] = distro
>> + common_info['os_version'] = version
>> + common_info['os_codename'] = unicode(codename, "utf-8")
>> + common_info['architecture'] = ARCH
>> + common_info['host'] = platform.node()
>> +
>> + return common_info
>> +
>> + def _get_online_cpus_(self):
>> + """
>> + method to retrieve online cpus count for
>> + all architectures using psutil
>> + :return: online cpus count
>> + """
>> cpus = psutil.NUM_CPUS
>>
>> # psutil is unstable on how to get the number of
>> @@ -124,13 +315,28 @@ class HostModel(object):
>> if method is not None:
>> cpus = method()
>> break
>> + return cpus
>>
>> - self.host_info['cpus'] = cpus
>> + def _get_total_memory(self):
>> + """
>> + method to retrieve total memory for all architectures using psutil
>> + :return: total memory
>> + """
>> if hasattr(psutil, 'phymem_usage'):
>> - self.host_info['memory'] = psutil.phymem_usage().total
>> + return psutil.phymem_usage().total
>> elif hasattr(psutil, 'virtual_memory'):
>> - self.host_info['memory'] = psutil.virtual_memory().total
>> - return self.host_info
>> + return psutil.virtual_memory().total
>> +
>> + def lookup(self, *name):
>> + """
>> + method to get basic information for host
>> + """
>> + if ARCH.startswith('ppc'):
>> + return self._get_ppc_host_info()
>> + elif ARCH.startswith('s390x'):
>> + return self._get_s390x_host_info()
>> + else:
>> + return self._get_x86_host_info()
>>
>> def swupdate(self, *name):
>> try:
>> diff --git a/src/wok/plugins/gingerbase/tests/test_host.py b/src/wok/plugins/gingerbase/tests/test_host.py
>> index 4f3e848..ec0d5a3 100644
>> --- a/src/wok/plugins/gingerbase/tests/test_host.py
>> +++ b/src/wok/plugins/gingerbase/tests/test_host.py
>> @@ -66,15 +66,21 @@ class HostTests(unittest.TestCase):
>> def test_hostinfo(self):
>> resp = self.request('/plugins/gingerbase/host').read()
>> info = json.loads(resp)
>> - keys = ['os_distro', 'os_version', 'os_codename', 'cpu_model',
>> - 'memory', 'cpus']
>> + if platform.machine().startswith('s390x'):
>> + keys = ['os_distro', 'os_version', 'os_codename', 'cpu_model',
>> + 'memory', 'cpus', 'architecture', 'host', 'virtualization']
>> + else:
>> + keys = ['os_distro', 'os_version', 'os_codename', 'cpu_model',
>> + 'memory', 'cpus', 'architecture', 'host']
>> + self.assertEquals(psutil.TOTAL_PHYMEM, info['memory'])
>> self.assertEquals(sorted(keys), sorted(info.keys()))
>>
>> distro, version, codename = platform.linux_distribution()
>> self.assertEquals(distro, info['os_distro'])
>> self.assertEquals(version, info['os_version'])
>> self.assertEquals(unicode(codename, "utf-8"), info['os_codename'])
>> - self.assertEquals(psutil.TOTAL_PHYMEM, info['memory'])
>> + self.assertEqual(platform.node(), info['host'])
>> + self.assertEqual(platform.machine(), info['architecture'])
>>
>> def test_hoststats(self):
>> time.sleep(1)
>
--
Regards,
Suresh Babu Angadi
More information about the Kimchi-devel
mailing list