From 6f7238135e2d0ebc78dbc3af357809c9438a1ac0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fran=C3=A7ois=20Rigault?= <frigo@amadeus.com>
Date: Tue, 23 Mar 2021 19:09:38 +0100
Subject: [PATCH] Return an integer value for Cores and Threads

Clients like Ironic Redfish inspector expect an integer value for
Cores and Threads. We do our best to return an integer value, we also
provide a default of "1" in case none is found.

Current logic expects the number of threads and cores to be present
in the domain, but this is not necessarily the case (test case
provided). We also can return a lot of information even when the
domain is not active, so we do just that.

Story: 2008743
Task: 42100

Change-Id: Id702ca2d7c5965217f348fc0931df0b0b1e57da8
---
 .../resources/systems/libvirtdriver.py        | 34 +++++++++++--------
 .../emulator/domain_processors_notopology.xml | 31 +++++++++++++++++
 .../resources/systems/test_libvirt.py         | 27 +++++++++++++++
 3 files changed, 77 insertions(+), 15 deletions(-)
 create mode 100644 sushy_tools/tests/unit/emulator/domain_processors_notopology.xml

diff --git a/sushy_tools/emulator/resources/systems/libvirtdriver.py b/sushy_tools/emulator/resources/systems/libvirtdriver.py
index 9b9f70c4..8a0369b1 100644
--- a/sushy_tools/emulator/resources/systems/libvirtdriver.py
+++ b/sushy_tools/emulator/resources/systems/libvirtdriver.py
@@ -776,28 +776,32 @@ class LibvirtDriver(AbstractSystemsDriver):
         domain = self._get_domain(identity, readonly=True)
         processors_count = self.get_total_cpus(identity)
 
-        # NOTE(rpittau) not a lot we can provide if the domain is not active
         processors = [{'id': 'CPU{0}'.format(x),
                        'socket': 'CPU {0}'.format(x)}
                       for x in range(processors_count)]
 
-        if domain.isActive():
-            tree = ET.fromstring(domain.XMLDesc())
-
+        tree = ET.fromstring(domain.XMLDesc())
+        try:
             model = tree.find('.//cpu/model').text
+        except AttributeError:
+            model = 'N/A'
+        try:
             vendor = tree.find('.//cpu/vendor').text
-            try:
-                cores = tree.find('.//cpu/topology').get('cores')
-                threads = tree.find('.//cpu/topology').get('threads')
-            except AttributeError:
-                cores = 'N/A'
-                threads = 'N/A'
+        except AttributeError:
+            vendor = 'N/A'
+        try:
+            cores = tree.find('.//cpu/topology').get('cores')
+            threads = tree.find('.//cpu/topology').get('threads')
+        except AttributeError:
+            # still return an integer as clients are expecting
+            cores = '1'
+            threads = '1'
 
-            for processor in processors:
-                processor['model'] = model
-                processor['vendor'] = vendor
-                processor['cores'] = cores
-                processor['threads'] = threads
+        for processor in processors:
+            processor['model'] = model
+            processor['vendor'] = vendor
+            processor['cores'] = cores
+            processor['threads'] = threads
 
         return processors
 
diff --git a/sushy_tools/tests/unit/emulator/domain_processors_notopology.xml b/sushy_tools/tests/unit/emulator/domain_processors_notopology.xml
new file mode 100644
index 00000000..6d1f1da1
--- /dev/null
+++ b/sushy_tools/tests/unit/emulator/domain_processors_notopology.xml
@@ -0,0 +1,31 @@
+<domain type='kvm'>
+  <name>test-node</name>
+  <uuid>5729db1e-3162-40e4-9fc4-817e4c0c121d</uuid>
+  <memory unit='KiB'>2097152</memory>
+  <currentMemory unit='KiB'>2097152</currentMemory>
+  <vcpu placement='static'>1</vcpu>
+  <os>
+    <type arch='x86_64' machine='pc-i440fx-bionic'>hvm</type>
+    <loader readonly='yes' type='pflash'>/usr/share/OVMF/OVMF_CODE.fd</loader>
+    <nvram>/var/lib/libvirt/qemu/nvram/test-node_VARS.fd</nvram>
+    <boot dev='hd'/>
+  </os>
+  <features>
+    <acpi/>
+    <apic/>
+    <vmport state='off'/>
+  </features>
+  <cpu mode='custom' match='exact' check='partial'/>
+  <clock offset='utc'>
+    <timer name='rtc' tickpolicy='catchup'/>
+    <timer name='pit' tickpolicy='delay'/>
+    <timer name='hpet' present='no'/>
+  </clock>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <pm>
+    <suspend-to-mem enabled='no'/>
+    <suspend-to-disk enabled='no'/>
+  </pm>
+</domain>
diff --git a/sushy_tools/tests/unit/emulator/resources/systems/test_libvirt.py b/sushy_tools/tests/unit/emulator/resources/systems/test_libvirt.py
index d5ee9ec1..95d44cd8 100644
--- a/sushy_tools/tests/unit/emulator/resources/systems/test_libvirt.py
+++ b/sushy_tools/tests/unit/emulator/resources/systems/test_libvirt.py
@@ -966,6 +966,33 @@ class LibvirtDriverTestCase(base.BaseTestCase):
                            'vendor': 'Intel'}],
                          sorted(processors, key=lambda k: k['id']))
 
+    @mock.patch('libvirt.openReadOnly', autospec=True)
+    def test_get_processors_notopology(self, libvirt_mock):
+        with open(
+                'sushy_tools/tests/unit/emulator/'
+                'domain_processors_notopology.xml') as f:
+            domain_xml = f.read()
+
+        conn_mock = libvirt_mock.return_value
+        domain_mock = conn_mock.lookupByUUID.return_value
+        domain_mock.XMLDesc.return_value = domain_xml
+        domain_mock.maxVcpus.return_value = 2
+
+        processors = self.test_driver.get_processors(self.uuid)
+        self.assertEqual([{'cores': '1',
+                           'id': 'CPU0',
+                           'model': 'N/A',
+                           'socket': 'CPU 0',
+                           'threads': '1',
+                           'vendor': 'N/A'},
+                          {'cores': '1',
+                           'id': 'CPU1',
+                           'model': 'N/A',
+                           'socket': 'CPU 1',
+                           'threads': '1',
+                           'vendor': 'N/A'}],
+                         sorted(processors, key=lambda k: k['id']))
+
     @mock.patch('libvirt.openReadOnly', autospec=True)
     def test_get_simple_storage_collection(self, libvirt_mock):
         with open('sushy_tools/tests/unit/emulator/'