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

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
+   :oslo.config:option:`ram_allocation_ratio` must be set to ``1.0`` in
-   host must not be added to a :doc:`host aggregate </admin/aggregates>`
+   ``nova.conf``, and the host must not be added to a :doc:`host aggregate
-   with ``ram_allocation_ratio`` set to anything but ``1.0``.
+   </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:
+        if not CONF.libvirt.file_backed_memory:
-            # file_backed_memory is only compatible with qemu/kvm virts
+            return
-            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.
+        # file_backed_memory is only compatible with qemu/kvm virts
-            # Block service startup if file-backed memory is enabled and
+        if CONF.libvirt.virt_type not in ("qemu", "kvm"):
-            # ram_allocation_ratio is not 1.0
+            raise exception.InternalError(
-            if CONF.ram_allocation_ratio != 1.0:
+                _('Running Nova with file_backed_memory and virt_type '
-                raise exception.InternalError(
+                  '%(type)s is not supported. file_backed_memory is only '
-                    'Running Nova with file_backed_memory requires '
+                  'supported with qemu and kvm types.') %
-                    'ram_allocation_ratio configured to 1.0')
+                {'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``.