libvirt: Don't allow "reserving" file-backed memory

When file-backed memory is configured, it is the only "memory" reported
by nova and used by instances, with RAM used in caching capacity. We
should be warning users of this and insisting they explicitly configure
the '[DEFAULT] reserved_host_memory_mb' config option to 0. However,
doing so now would be breaking change. Instead, start logging a warning
instead, failing only for the truly broken combination of reserving more
file-backed memory than we have allocated.

Change-Id: I9619338ad0f60253b628d96543f8ce3ac86242e3
Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
Closes-Bug: #1882821

doc/source/admin/file-backed-memory.rst

@@ -46,14 +46,22 @@ Libvirt
    capability requires libvirt version 4.4.0 or newer.
 
 Qemu
-   File-backed memory requires qemu version 2.6.0 or newer.Discard capability
+   File-backed memory requires qemu version 2.6.0 or newer. Discard capability
    requires qemu version 2.10.0 or newer.
 
 Memory overcommit
    File-backed memory is not compatible with memory overcommit.
-   ``ram_allocation_ratio`` must be set to ``1.0`` in ``nova.conf``, and the
-   host must not be added to a :doc:`host aggregate </admin/aggregates>`
-   with ``ram_allocation_ratio`` set to anything but ``1.0``.
+   :oslo.config:option:`ram_allocation_ratio` must be set to ``1.0`` in
+   ``nova.conf``, and the host must not be added to a :doc:`host aggregate
+   </admin/aggregates>` with ``ram_allocation_ratio`` set to anything but
+   ``1.0``.
+
+Reserved memory
+   When configured, file-backed memory is reported as total system memory to
+   placement, with RAM used as cache. Reserved memory corresponds to disk
+   space not set aside for file-backed memory.
+   :oslo.config:option:`reserved_host_memory_mb` should be set to ``0`` in
+   ``nova.conf``.
 
 Huge pages
    File-backed memory is not compatible with huge pages. Instances with huge

nova/tests/unit/virt/libvirt/test_driver.py

@@ -1341,6 +1341,39 @@ class LibvirtConnTestCase(test.NoDBTestCase,
         self.assertRaises(exception.InternalError,
                           drvr._check_file_backed_memory_support)
 
+    def test__check_file_backed_memory_support__total_lt_reserved(self):
+        """Ensure an error is raised if total memory < reserved.
+
+        Placement won't allow $resource.total < $resource.reserved, so we need
+        to catch this early.
+        """
+        self.flags(file_backed_memory=1024, group='libvirt')
+        self.flags(ram_allocation_ratio=1.0, reserved_host_memory_mb=4096)
+        drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True)
+        self.assertRaises(
+            exception.InternalError, drvr._check_file_backed_memory_support,
+        )
+
+    @mock.patch.object(libvirt_driver.LOG, 'warning')
+    def test__check_file_backed_memory_support__has_reserved(self, mock_log):
+        """Ensure a warning is issued if memory is reserved.
+
+        It doesn't make sense to "reserve" memory when file-backed memory is in
+        use. We should report things so as to avoid confusion.
+        """
+        self.flags(file_backed_memory=8192, group='libvirt')
+        self.flags(ram_allocation_ratio=1.0)
+        # we don't need to configure '[DEFAULT] reserved_host_memory_mb' since
+        # it defaults to 512 (MB)
+        drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True)
+        drvr._check_file_backed_memory_support()
+        mock_log.assert_called_once()
+        self.assertIn(
+            "Reserving memory via '[DEFAULT] reserved_host_memory_mb' is not "
+            "compatible",
+            six.text_type(mock_log.call_args[0]),
+        )
+
     def test__check_cpu_compatibility_start_ok(self):
         self.flags(cpu_mode="custom",
                    cpu_models=["Penryn"],

nova/virt/libvirt/driver.py

@@ -819,22 +819,43 @@ class LibvirtDriver(driver.ComputeDriver):
                 self._create_new_mediated_device(parent, uuid=mdev_uuid)
 
     def _check_file_backed_memory_support(self):
-        if CONF.libvirt.file_backed_memory:
-            # file_backed_memory is only compatible with qemu/kvm virts
-            if CONF.libvirt.virt_type not in ("qemu", "kvm"):
-                raise exception.InternalError(
-                    _('Running Nova with file_backed_memory and virt_type '
-                      '%(type)s is not supported. file_backed_memory is only '
-                      'supported with qemu and kvm types.') %
-                    {'type': CONF.libvirt.virt_type})
+        if not CONF.libvirt.file_backed_memory:
+            return
 
-            # file-backed memory doesn't work with memory overcommit.
-            # Block service startup if file-backed memory is enabled and
-            # ram_allocation_ratio is not 1.0
-            if CONF.ram_allocation_ratio != 1.0:
-                raise exception.InternalError(
-                    'Running Nova with file_backed_memory requires '
-                    'ram_allocation_ratio configured to 1.0')
+        # file_backed_memory is only compatible with qemu/kvm virts
+        if CONF.libvirt.virt_type not in ("qemu", "kvm"):
+            raise exception.InternalError(
+                _('Running Nova with file_backed_memory and virt_type '
+                  '%(type)s is not supported. file_backed_memory is only '
+                  'supported with qemu and kvm types.') %
+                {'type': CONF.libvirt.virt_type})
+
+        # file-backed memory doesn't work with memory overcommit.
+        # Block service startup if file-backed memory is enabled and
+        # ram_allocation_ratio is not 1.0
+        if CONF.ram_allocation_ratio != 1.0:
+            raise exception.InternalError(
+                'Running Nova with file_backed_memory requires '
+                'ram_allocation_ratio configured to 1.0')
+
+        if CONF.reserved_host_memory_mb:
+            # this is a hard failure as placement won't allow total < reserved
+            if CONF.reserved_host_memory_mb >= CONF.libvirt.file_backed_memory:
+                msg = _(
+                    "'[libvirt] file_backed_memory', which represents total "
+                    "memory reported to placement, must be greater than "
+                    "reserved memory configured via '[DEFAULT] "
+                    "reserved_host_memory_mb'"
+                )
+                raise exception.InternalError(msg)
+
+            # TODO(stephenfin): Change this to an exception in W or later
+            LOG.warning(
+                "Reserving memory via '[DEFAULT] reserved_host_memory_mb' "
+                "is not compatible with file-backed memory. Consider "
+                "setting '[DEFAULT] reserved_host_memory_mb' to 0. This will "
+                "be an error in a future release."
+            )
 
     def _check_my_ip(self):
         ips = compute_utils.get_machine_ips()

releasenotes/notes/bug-1882821-file-backed-memory-reserved-conflict-3ad4c04ab993ebf8.yaml

@@ -0,0 +1,15 @@
+---
+upgrade:
+  - |
+    When using file-backed memory, the ``nova-compute`` service will now fail
+    to start if the amount of reserved memory configured using ``[DEFAULT]
+    reserved_host_memory_mb`` is equal to or greater than the total amount of
+    memory configured using ``[libvirt] file_backed_memory``. Where reserved
+    memory is less than the total amount of memory configured, a warning will
+    be raised. This warning will become an error in a future release.
+
+    The former combination is invalid as it would suggest reserved memory is
+    greater than total memory available, while the latter is considered
+    incorrect behavior as reserving of file-backed memory can and should be
+    achieved by reducing the filespace allocated as memory by modifying
+    ``[libvirt] file_backed_memory``.