From 3b99747b42e460567d52cc6d748396cb349d56b8 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Tue, 9 Jun 2020 18:38:10 +0100 Subject: [PATCH] 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 Closes-Bug: #1882821 --- doc/source/admin/file-backed-memory.rst | 16 ++++-- nova/tests/unit/virt/libvirt/test_driver.py | 33 ++++++++++++ nova/virt/libvirt/driver.py | 51 +++++++++++++------ ...ry-reserved-conflict-3ad4c04ab993ebf8.yaml | 15 ++++++ 4 files changed, 96 insertions(+), 19 deletions(-) create mode 100644 releasenotes/notes/bug-1882821-file-backed-memory-reserved-conflict-3ad4c04ab993ebf8.yaml diff --git a/doc/source/admin/file-backed-memory.rst b/doc/source/admin/file-backed-memory.rst index 601e2c51f2fa..dffb3de3833b 100644 --- a/doc/source/admin/file-backed-memory.rst +++ b/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 ` - 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 + ` 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 diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index 8203f2a8f787..815a603e4fc9 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/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"], diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py index bc832749abd6..aef1aac6c78d 100644 --- a/nova/virt/libvirt/driver.py +++ b/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() diff --git a/releasenotes/notes/bug-1882821-file-backed-memory-reserved-conflict-3ad4c04ab993ebf8.yaml b/releasenotes/notes/bug-1882821-file-backed-memory-reserved-conflict-3ad4c04ab993ebf8.yaml new file mode 100644 index 000000000000..b66408d70d8a --- /dev/null +++ b/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``.