Overall code looks great to me with some comments/queries. Comments inline!!

...

From: Suresh Babu Angadi <sureshab@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@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

On Mon, 2015-11-23 at 17:33 +0530, sureshab@linux.vnet.ibm.com wrote: 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

On 11/23/2015 11:24 PM, Abhiram wrote: 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

On 11/24/2015 12:39 AM, Aline Manera wrote:

On 23/11/2015 10:03, sureshab@linux.vnet.ibm.com wrote:

...

From: Suresh Babu Angadi <sureshab@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@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

We should not change the data object across platforms. So use a dict for all them as I explained in the other email.

memory: The host memory information * online: ... * offline: ...

Agreed. Will make the changes in V2

...

* 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 + 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

Only 'shared' and 'dedicated' are exclusive for s390x

cpus: The host cpus information * online: ... * offline: ... * shared (s390x only): ... * dedicated (s390x only): ...

Accepted

...

+ 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 + * 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" 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 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'] + 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()

I understand why you separated the functions per arch, but that way it is too hard to identify which code is common for all platforms and which code is platform-specific.

My suggestion is to keep the common code on lookup() and call external functions for the specific matters.

Example:

def lookup(self, *name): # all the common code here <...>

# specific code here if ARCH.startwith('s390x'): do else: do

I suspect while changing the memory and cpus to be a dict across platforms, the code will be simplified. As x86 and ppc code will be the same. The unique different would be related to how collect shared and dedicated memory information for s390x.

I agree, but individual methods have pydocs about what they do, so it shouldn't be tough for code maintenance, but nevertheless will try to change lookup as per the above suggestion. Yes, changing cpus & memory to dict would make the code look simpler.

...

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