From f76d09b19d5b7530b7e4e9fc255835f6aff5db6a Mon Sep 17 00:00:00 2001
From: Stephen Finucane <stephenfin@redhat.com>
Date: Thu, 21 May 2020 14:43:25 +0100
Subject: [PATCH] hardware: Raise useful error for invalid mempage size

The 'hw:mem_page_size' extra spec allows you to specify values in three
ways: via a relative size alias (large, small, any), via exact byte
values, and via an SI/IEC/mixed representation (e.g. 2MB, 1GB).
Currently, attempting to use an invalid variant of the latter, such as
'2M', will raise a rather confusing error message:

  Invalid memory page size '0'

Correct this so we instead see something like so:

  Invalid memory page size '2M'

Change-Id: I3af99f03514904da09f7b71f430b43bf75e8be67
Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
Closes-Bug: #1879964
---
 nova/tests/unit/virt/test_hardware.py | 12 ++++++++++--
 nova/virt/hardware.py                 | 20 +++++++-------------
 2 files changed, 17 insertions(+), 15 deletions(-)

diff --git a/nova/tests/unit/virt/test_hardware.py b/nova/tests/unit/virt/test_hardware.py
index 141ef98f11fe..7bbaa0fee29b 100644
--- a/nova/tests/unit/virt/test_hardware.py
+++ b/nova/tests/unit/virt/test_hardware.py
@@ -2431,15 +2431,23 @@ class VirtMemoryPagesTestCase(test.NoDBTestCase):
                 spec={"hw:mem_page_size": "2048"}))
 
     def test_get_requested_mempages_pagesize_from_flavor_invalid(self):
-        self.assertRaises(
+        ex = self.assertRaises(
             exception.MemoryPageSizeInvalid,
             self._test_get_requested_mempages_pagesize,
             {"hw:mem_page_size": "foo"})
+        self.assertIn("foo", str(ex))
 
-        self.assertRaises(
+        ex = self.assertRaises(
             exception.MemoryPageSizeInvalid,
             self._test_get_requested_mempages_pagesize,
             {"hw:mem_page_size": "-42"})
+        self.assertIn("-42", str(ex))
+
+        ex = self.assertRaises(
+            exception.MemoryPageSizeInvalid,
+            self._test_get_requested_mempages_pagesize,
+            {"hw:mem_page_size": "2M"})
+        self.assertIn("2M", str(ex))
 
     def test_get_requested_mempages_pagesizes_from_flavor_suffix_sweep(self):
         self.assertEqual(
diff --git a/nova/virt/hardware.py b/nova/virt/hardware.py
index 12a6a3f39bd6..0253dd992786 100644
--- a/nova/virt/hardware.py
+++ b/nova/virt/hardware.py
@@ -1343,20 +1343,14 @@ def _get_numa_pagesize_constraint(flavor, image_meta):
             return MEMPAGES_LARGE
         elif request == "small":
             return MEMPAGES_SMALL
-        else:
-            try:
-                request = int(request)
-            except ValueError:
-                try:
-                    request = strutils.string_to_bytes(
-                        request, return_int=True) / units.Ki
-                except ValueError:
-                    request = 0
+        elif request.isdigit():
+            return int(request)
 
-        if request <= 0:
-            raise exception.MemoryPageSizeInvalid(pagesize=request)
-
-        return request
+        try:
+            return strutils.string_to_bytes(
+                request, return_int=True) / units.Ki
+        except ValueError:
+            raise exception.MemoryPageSizeInvalid(pagesize=request) from None
 
     flavor_request, image_request = _get_flavor_image_meta(
         'mem_page_size', flavor, image_meta)