From 2877fc53d409e950a685a4bd31ac37b4865981a9 Mon Sep 17 00:00:00 2001
From: Mike Turek <mjturek@linux.vnet.ibm.com>
Date: Thu, 30 Nov 2017 16:30:42 +0000
Subject: [PATCH] Use lshw in place of dmidecode for the default hardware
 manager

Currently the generic hardware manager uses dmidecode to get the
total physical memory and system details. This patch switches the
generic hardware manager to use lshw, as it is capable of reading
more than DMI [0]. This enables systems that do not support DMI
to use the generic hardware manager, such as IBM Power systems.

[0] https://github.com/lyonel/lshw/blob/master/README.md

Closes-Bug: #1715790
Change-Id: Ie370331df6bb5ef131c5cb60f458877e2a7ad71a
Depends-On: Idaf05b8efce28cd0cbf339cf693db4f55a693d9b
---
 imagebuild/tinyipa/build-tinyipa.sh           |   8 +
 imagebuild/tinyipa/finalise-tinyipa.sh        |   3 +
 ironic_python_agent/hardware.py               |  88 ++++-----
 .../tests/unit/test_hardware.py               | 173 +++++++++++++-----
 ...mory-and-system-info-35c69da067c72b36.yaml |  15 ++
 5 files changed, 195 insertions(+), 92 deletions(-)
 create mode 100644 releasenotes/notes/lshw-for-memory-and-system-info-35c69da067c72b36.yaml

diff --git a/imagebuild/tinyipa/build-tinyipa.sh b/imagebuild/tinyipa/build-tinyipa.sh
index 40e26bb7a..1112e1ede 100755
--- a/imagebuild/tinyipa/build-tinyipa.sh
+++ b/imagebuild/tinyipa/build-tinyipa.sh
@@ -62,6 +62,7 @@ sudo sh -c "echo $TINYCORE_MIRROR_URL > $BUILDDIR/opt/tcemirror"
 # Download TGT, Qemu-utils, Biosdevname and IPMItool source
 clone_and_checkout "https://github.com/fujita/tgt.git" "${BUILDDIR}/tmp/tgt" "v1.0.62"
 clone_and_checkout "https://github.com/qemu/qemu.git" "${BUILDDIR}/tmp/qemu" "v2.5.0"
+clone_and_checkout "https://github.com/lyonel/lshw.git" "${BUILDDIR}/tmp/lshw" "B.02.18"
 if $TINYIPA_REQUIRE_BIOSDEVNAME; then
     wget -N -O - https://linux.dell.com/biosdevname/biosdevname-0.7.2/biosdevname-0.7.2.tar.gz | tar -xz -C "${BUILDDIR}/tmp" -f -
 fi
@@ -137,6 +138,13 @@ cd $WORKDIR/build_files && mksquashfs $BUILDDIR/tmp/qemu-utils qemu-utils.tcz &&
 # Create qemu-utils.tcz.dep
 echo "glib2.tcz" > qemu-utils.tcz.dep
 
+# Build lshw
+rm -rf $WORKDIR/build_files/lshw.tcz
+# NOTE(mjturek): We touch src/lshw.1 and clear src/po/Makefile to avoid building the man pages, as they aren't used and require large dependencies to build.
+$CHROOT_CMD /bin/sh -c "cd /tmp/lshw && touch src/lshw.1 && echo install: > src/po/Makefile && make && make install DESTDIR=/tmp/lshw-installed"
+find $BUILDDIR/tmp/lshw-installed/ -type f -executable | xargs file | awk -F ':' '/ELF/ {print $1}' | sudo xargs strip
+cd $WORKDIR/build_files && mksquashfs $BUILDDIR/tmp/lshw-installed lshw.tcz && md5sum lshw.tcz > lshw.tcz.md5.txt
+
 # Build biosdevname
 if $TINYIPA_REQUIRE_BIOSDEVNAME; then
     rm -rf $WORKDIR/build_files/biosdevname.tcz
diff --git a/imagebuild/tinyipa/finalise-tinyipa.sh b/imagebuild/tinyipa/finalise-tinyipa.sh
index 2ff2d6f57..972681e11 100755
--- a/imagebuild/tinyipa/finalise-tinyipa.sh
+++ b/imagebuild/tinyipa/finalise-tinyipa.sh
@@ -76,6 +76,8 @@ cp -Rp "$BUILDDIR/tmp/wheels" "$FINALDIR/tmp/wheelhouse"
 
 cp $WORKDIR/build_files/tgt.* $FINALDIR/tmp/builtin/optional
 cp $WORKDIR/build_files/qemu-utils.* $FINALDIR/tmp/builtin/optional
+cp $WORKDIR/build_files/lshw.* $FINALDIR/tmp/builtin/optional
+
 if $TINYIPA_REQUIRE_BIOSDEVNAME; then
     cp $WORKDIR/build_files/biosdevname.* $FINALDIR/tmp/builtin/optional
 fi
@@ -118,6 +120,7 @@ fi
 
 $TC_CHROOT_CMD tce-load -ic /tmp/builtin/optional/tgt.tcz
 $TC_CHROOT_CMD tce-load -ic /tmp/builtin/optional/qemu-utils.tcz
+$TC_CHROOT_CMD tce-load -ic /tmp/builtin/optional/lshw.tcz
 if $TINYIPA_REQUIRE_BIOSDEVNAME; then
     $TC_CHROOT_CMD tce-load -ic /tmp/builtin/optional/biosdevname.tcz
 fi
diff --git a/ironic_python_agent/hardware.py b/ironic_python_agent/hardware.py
index 5972a21c0..bd12a209c 100644
--- a/ironic_python_agent/hardware.py
+++ b/ironic_python_agent/hardware.py
@@ -1,4 +1,4 @@
-# Copyright 2013 Rackspace, Inc.
+#  Copyright 2013 Rackspace, Inc.
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
@@ -15,6 +15,7 @@
 import abc
 import binascii
 import functools
+import json
 import os
 import shlex
 import time
@@ -43,8 +44,8 @@ CONF = cfg.CONF
 WARN_BIOSDEVNAME_NOT_FOUND = False
 
 UNIT_CONVERTER = pint.UnitRegistry(filename=None)
-UNIT_CONVERTER.define('MB = []')
-UNIT_CONVERTER.define('GB = 1024 MB')
+UNIT_CONVERTER.define('bytes = []')
+UNIT_CONVERTER.define('MB = 1048576 bytes')
 
 NODE = None
 
@@ -62,6 +63,18 @@ def _get_device_info(dev, devclass, field):
                 field, dev, devclass))
 
 
+def _get_system_lshw_dict():
+    """Get a dict representation of the system from lshw
+
+    Retrieves a json representation of the system from lshw and converts
+    it to a python dict
+
+    :return: A python dict from the lshw json output
+    """
+    out, _e = utils.execute('lshw', '-quiet', '-json')
+    return json.loads(out)
+
+
 def _udev_settle():
     """Wait for the udev event queue to settle.
 
@@ -670,38 +683,25 @@ class GenericHardwareManager(HardwareManager):
             total = None
             LOG.exception(("Cannot fetch total memory size using psutil "
                            "version %s"), psutil.version_info[0])
-
+        sys_dict = None
         try:
-            out, _e = utils.execute("dmidecode --type 17 | grep Size",
-                                    shell=True)
-        except (processutils.ProcessExecutionError, OSError) as e:
-            LOG.warning("Cannot get real physical memory size: %s", e)
+            sys_dict = _get_system_lshw_dict()
+        except (processutils.ProcessExecutionError, OSError, ValueError) as e:
+            LOG.warning('Could not get real physical RAM from lshw: %s', e)
             physical = None
         else:
             physical = 0
-            for line in out.strip().split('\n'):
-                line = line.strip()
-                if not line:
-                    continue
-
-                if 'Size:' not in line:
-                    continue
-
-                value = None
-                try:
-                    value = line.split('Size: ', 1)[1]
-                    physical += int(UNIT_CONVERTER(value).to_base_units())
-                except Exception as exc:
-                    if (value == "No Module Installed" or
-                            value == "Not Installed"):
-                        LOG.debug('One memory slot is empty')
-                    else:
-                        LOG.error('Cannot parse size expression %s: %s',
-                                  line, exc)
-
+            # locate memory information in system_dict
+            for sys_child in sys_dict['children']:
+                if sys_child['id'] == 'core':
+                    for core_child in sys_child['children']:
+                        if core_child['id'] == 'memory':
+                            if core_child.get('size'):
+                                value = "%(size)s %(units)s" % core_child
+                                physical += int(UNIT_CONVERTER(value).to(
+                                    'MB').magnitude)
             if not physical:
-                LOG.warning('failed to get real physical RAM, dmidecode '
-                            'returned %s', out)
+                LOG.warning('Did not find any physical RAM')
 
         return Memory(total=total, physical_mb=physical)
 
@@ -748,28 +748,14 @@ class GenericHardwareManager(HardwareManager):
         return dev_name
 
     def get_system_vendor_info(self):
-        product_name = None
-        serial_number = None
-        manufacturer = None
         try:
-            out, _e = utils.execute("dmidecode --type system",
-                                    shell=True)
-        except (processutils.ProcessExecutionError, OSError) as e:
-            LOG.warning("Cannot get system vendor information: %s", e)
-        else:
-            for line in out.split('\n'):
-                line_arr = line.split(':', 1)
-                if len(line_arr) != 2:
-                    continue
-                if line_arr[0].strip() == 'Product Name':
-                    product_name = line_arr[1].strip()
-                elif line_arr[0].strip() == 'Serial Number':
-                    serial_number = line_arr[1].strip()
-                elif line_arr[0].strip() == 'Manufacturer':
-                    manufacturer = line_arr[1].strip()
-        return SystemVendorInfo(product_name=product_name,
-                                serial_number=serial_number,
-                                manufacturer=manufacturer)
+            sys_dict = _get_system_lshw_dict()
+        except (processutils.ProcessExecutionError, OSError, ValueError) as e:
+            LOG.warning('Could not retrieve vendor info from lshw: %e', e)
+            sys_dict = {}
+        return SystemVendorInfo(product_name=sys_dict.get('product', ''),
+                                serial_number=sys_dict.get('serial', ''),
+                                manufacturer=sys_dict.get('vendor', ''))
 
     def get_boot_info(self):
         boot_mode = 'uefi' if os.path.isdir('/sys/firmware/efi') else 'bios'
diff --git a/ironic_python_agent/tests/unit/test_hardware.py b/ironic_python_agent/tests/unit/test_hardware.py
index 48096e90a..da7f20268 100644
--- a/ironic_python_agent/tests/unit/test_hardware.py
+++ b/ironic_python_agent/tests/unit/test_hardware.py
@@ -239,13 +239,113 @@ CPUINFO_FLAGS_OUTPUT = """
 flags           : fpu vme de pse
 """
 
-DMIDECODE_MEMORY_OUTPUT = ("""
-Foo
-Size: 2048 MB
-Size: 2 GB
-Installed Size: Not Installed
-Enabled Size: Not Installed
-Size: No Module Installed
+LSHW_JSON_OUTPUT = ("""
+{
+  "id": "fuzzypickles",
+  "product": "ABC123 (GENERIC_SERVER)",
+  "vendor": "GENERIC",
+  "serial": "1234567",
+  "width": 64,
+  "capabilities": {
+    "smbios-2.7": "SMBIOS version 2.7",
+    "dmi-2.7": "DMI version 2.7",
+    "vsyscall32": "32-bit processes"
+  },
+  "children": [
+    {
+      "id": "core",
+      "description": "Motherboard",
+      "product": "ABC123",
+      "vendor": "GENERIC",
+      "serial": "ABCDEFGHIJK",
+      "children": [
+        {
+          "id": "memory",
+          "class": "memory",
+          "description": "System Memory",
+          "units": "bytes",
+          "size": 4294967296,
+          "children": [
+            {
+              "id": "bank:0",
+              "class": "memory",
+              "physid": "0",
+              "units": "bytes",
+              "size": 2147483648,
+              "width": 64,
+              "clock": 1600000000
+            },
+            {
+              "id": "bank:1",
+              "class": "memory",
+              "physid": "1"
+            },
+            {
+              "id": "bank:2",
+              "class": "memory",
+              "physid": "2",
+              "units": "bytes",
+              "size": 1073741824,
+              "width": 64,
+              "clock": 1600000000
+            },
+            {
+              "id": "bank:3",
+              "class": "memory",
+              "physid": "3",
+              "units": "bytes",
+              "size": 1073741824,
+              "width": 64,
+              "clock": 1600000000
+            }
+          ]
+        },
+        {
+          "id": "cpu:0",
+          "class": "processor",
+          "claimed": true,
+          "product": "Intel Xeon E312xx (Sandy Bridge)",
+          "vendor": "Intel Corp.",
+          "physid": "1",
+          "businfo": "cpu@0",
+          "width": 64,
+          "capabilities": {
+            "fpu": "mathematical co-processor",
+            "fpu_exception": "FPU exceptions reporting",
+            "wp": true,
+            "mmx": "multimedia extensions (MMX)"
+          }
+        }
+      ]
+    },
+    {
+      "id": "network:0",
+      "class": "network",
+      "claimed": true,
+      "description": "Ethernet interface",
+      "physid": "1",
+      "logicalname": "ovs-tap",
+      "serial": "1c:90:c0:f9:4e:a1",
+      "units": "bit/s",
+      "size": 10000000000,
+      "configuration": {
+        "autonegotiation": "off",
+        "broadcast": "yes",
+        "driver": "veth",
+        "driverversion": "1.0",
+        "duplex": "full",
+        "link": "yes",
+        "multicast": "yes",
+        "port": "twisted pair",
+        "speed": "10Gbit/s"
+      },
+      "capabilities": {
+        "ethernet": true,
+        "physical": "Physical interface"
+      }
+    }
+  ]
+}
 """, "")
 
 
@@ -861,7 +961,7 @@ class TestGenericHardwareManager(base.IronicAgentTest):
     @mock.patch.object(utils, 'execute', autospec=True)
     def test_get_memory_psutil(self, mocked_execute, mocked_psutil):
         mocked_psutil.return_value.total = 3952 * 1024 * 1024
-        mocked_execute.return_value = DMIDECODE_MEMORY_OUTPUT
+        mocked_execute.return_value = LSHW_JSON_OUTPUT
         mem = self.hardware.get_memory()
 
         self.assertEqual(3952 * 1024 * 1024, mem.total)
@@ -870,13 +970,23 @@ class TestGenericHardwareManager(base.IronicAgentTest):
     @mock.patch('psutil.virtual_memory', autospec=True)
     @mock.patch.object(utils, 'execute', autospec=True)
     def test_get_memory_psutil_exception(self, mocked_execute, mocked_psutil):
-        mocked_execute.return_value = DMIDECODE_MEMORY_OUTPUT
+        mocked_execute.return_value = LSHW_JSON_OUTPUT
         mocked_psutil.side_effect = AttributeError()
         mem = self.hardware.get_memory()
 
         self.assertIsNone(mem.total)
         self.assertEqual(4096, mem.physical_mb)
 
+    @mock.patch('psutil.virtual_memory', autospec=True)
+    @mock.patch.object(utils, 'execute', autospec=True)
+    def test_get_memory_lshw_exception(self, mocked_execute, mocked_psutil):
+        mocked_execute.side_effect = OSError()
+        mocked_psutil.return_value.total = 3952 * 1024 * 1024
+        mem = self.hardware.get_memory()
+
+        self.assertEqual(3952 * 1024 * 1024, mem.total)
+        self.assertIsNone(mem.physical_mb)
+
     def test_list_hardware_info(self):
         self.hardware.list_network_interfaces = mock.Mock()
         self.hardware.list_network_interfaces.return_value = [
@@ -1625,38 +1735,19 @@ class TestGenericHardwareManager(base.IronicAgentTest):
 
     @mock.patch.object(utils, 'execute', autospec=True)
     def test_get_system_vendor_info(self, mocked_execute):
-        mocked_execute.return_value = (
-            '# dmidecode 2.12\n'
-            'SMBIOS 2.6 present.\n'
-            '\n'
-            'Handle 0x0001, DMI type 1, 27 bytes\n'
-            'System Information\n'
-            '\tManufacturer: NEC\n'
-            '\tProduct Name: Express5800/R120b-2 [N8100-1653]\n'
-            '\tVersion: FR1.3\n'
-            '\tSerial Number: 0800113\n'
-            '\tUUID: 00433468-26A5-DF11-8001-406186F5A681\n'
-            '\tWake-up Type: Power Switch\n'
-            '\tSKU Number: Not Specified\n'
-            '\tFamily: Not Specified\n'
-            '\n'
-            'Handle 0x002E, DMI type 12, 5 bytes\n'
-            'System Configuration Options\n'
-            '\tOption 1: CLR_CMOS: Close to clear CMOS\n'
-            '\tOption 2: BMC_FRB3: Close to stop FRB3 Timer\n'
-            '\tOption 3: BIOS_RECOVERY: Close to run BIOS Recovery\n'
-            '\tOption 4: PASS_DIS: Close to clear Password\n'
-            '\n'
-            'Handle 0x0059, DMI type 32, 11 bytes\n'
-            'System Boot Information\n'
-            '\tStatus: No errors detected\n'
-        ), ''
-        self.assertEqual('Express5800/R120b-2 [N8100-1653]',
-                         self.hardware.get_system_vendor_info().product_name)
-        self.assertEqual('0800113',
-                         self.hardware.get_system_vendor_info().serial_number)
-        self.assertEqual('NEC',
-                         self.hardware.get_system_vendor_info().manufacturer)
+        mocked_execute.return_value = LSHW_JSON_OUTPUT
+        vendor_info = self.hardware.get_system_vendor_info()
+        self.assertEqual('ABC123 (GENERIC_SERVER)', vendor_info.product_name)
+        self.assertEqual('1234567', vendor_info.serial_number)
+        self.assertEqual('GENERIC', vendor_info.manufacturer)
+
+    @mock.patch.object(utils, 'execute', autospec=True)
+    def test_get_system_vendor_info_failure(self, mocked_execute):
+        mocked_execute.side_effect = processutils.ProcessExecutionError()
+        vendor_info = self.hardware.get_system_vendor_info()
+        self.assertEqual('', vendor_info.product_name)
+        self.assertEqual('', vendor_info.serial_number)
+        self.assertEqual('', vendor_info.manufacturer)
 
     @mock.patch.object(hardware.GenericHardwareManager,
                        'get_os_install_device', autospec=True)
diff --git a/releasenotes/notes/lshw-for-memory-and-system-info-35c69da067c72b36.yaml b/releasenotes/notes/lshw-for-memory-and-system-info-35c69da067c72b36.yaml
new file mode 100644
index 000000000..1649cfdc5
--- /dev/null
+++ b/releasenotes/notes/lshw-for-memory-and-system-info-35c69da067c72b36.yaml
@@ -0,0 +1,15 @@
+---
+features:
+  - |
+    Switched to ``lshw`` for memory configuration and system information collection
+    when using the default hardware manager. This information can now be retrieved
+    on both DMI capable and OpenFirmware capable systems. ``dmidecode`` is no longer
+    used by the default hardware manager.
+fixes:
+  - |
+    The default hardware manager is now capable of collecting memory configuration
+    and system information on OpenFirmware (PowerPC) capable systems, in addition
+    to the already supported DMI (x86 and ARM) capable systems.
+upgrade:
+  - |
+    ``lshw`` is now a dependency of the default hardware manager.