From 17f972b6fb172fe19d6e115a20664eefdbd3838d Mon Sep 17 00:00:00 2001 From: Eduardo Otubo Date: Mon, 24 Aug 2020 15:25:38 +0200 Subject: [PATCH 3/3] Detect kernel version before swap file creation (#428) RH-Author: Eduardo Otubo Message-id: <20200820092042.5418-4-otubo@redhat.com> Patchwork-id: 98191 O-Subject: [RHEL-8.3.0 cloud-init PATCH 3/3] Detect kernel version before swap file creation (#428) Bugzilla: 1794664 RH-Acked-by: Miroslav Rezanina RH-Acked-by: Mohammed Gamal commit b749548a9eb43b34cce64f8688107645411abc8c Author: Eduardo Otubo Date: Tue Aug 18 23:12:02 2020 +0200 Detect kernel version before swap file creation (#428) According to man page `man 8 swapon', "Preallocated swap files are supported on XFS since Linux 4.18". This patch checks for kernel version before attepting to create swapfile, using dd for XFS only on kernel versions <= 4.18 or btrfs. Add new func util.kernel_version which returns a tuple of ints (major, minor) Signed-off-by: Eduardo Otubo otubo@redhat.com Signed-off-by: Eduardo Otubo otubo@redhat.com Signed-off-by: Miroslav Rezanina --- cloudinit/config/cc_mounts.py | 8 +- cloudinit/util.py | 4 + .../unittests/test_handler/test_handler_mounts.py | 107 +++++++++++++++++++++ tests/unittests/test_util.py | 15 +++ 4 files changed, 131 insertions(+), 3 deletions(-) diff --git a/cloudinit/config/cc_mounts.py b/cloudinit/config/cc_mounts.py index 0573026..e1c43e3 100644 --- a/cloudinit/config/cc_mounts.py +++ b/cloudinit/config/cc_mounts.py @@ -65,7 +65,7 @@ swap file is created. from string import whitespace import logging -import os.path +import os import re from cloudinit import type_utils @@ -249,7 +249,8 @@ def create_swapfile(fname, size): fstype = util.get_mount_info(swap_dir)[1] - if fstype in ("xfs", "btrfs"): + if (fstype == "xfs" and + util.kernel_version() < (4, 18)) or fstype == "btrfs": create_swap(fname, size, "dd") else: try: @@ -259,7 +260,8 @@ def create_swapfile(fname, size): LOG.warning("Will attempt with dd.") create_swap(fname, size, "dd") - util.chmod(fname, 0o600) + if os.path.exists(fname): + util.chmod(fname, 0o600) try: util.subp(['mkswap', fname]) except util.ProcessExecutionError: diff --git a/cloudinit/util.py b/cloudinit/util.py index 5d51ba8..ad89376 100644 --- a/cloudinit/util.py +++ b/cloudinit/util.py @@ -79,6 +79,10 @@ CONTAINER_TESTS = (['systemd-detect-virt', '--quiet', '--container'], ['lxc-is-container']) +def kernel_version(): + return tuple(map(int, os.uname().release.split('.')[:2])) + + @lru_cache() def get_architecture(target=None): out, _ = subp(['dpkg', '--print-architecture'], capture=True, diff --git a/tests/unittests/test_handler/test_handler_mounts.py b/tests/unittests/test_handler/test_handler_mounts.py index 7bcefa0..27bcc6f 100644 --- a/tests/unittests/test_handler/test_handler_mounts.py +++ b/tests/unittests/test_handler/test_handler_mounts.py @@ -132,6 +132,113 @@ class TestSanitizeDevname(test_helpers.FilesystemMockingTestCase): 'ephemeral0.1', lambda x: disk_path, mock.Mock())) +class TestSwapFileCreation(test_helpers.FilesystemMockingTestCase): + + def setUp(self): + super(TestSwapFileCreation, self).setUp() + self.new_root = self.tmp_dir() + self.patchOS(self.new_root) + + self.fstab_path = os.path.join(self.new_root, 'etc/fstab') + self.swap_path = os.path.join(self.new_root, 'swap.img') + self._makedirs('/etc') + + self.add_patch('cloudinit.config.cc_mounts.FSTAB_PATH', + 'mock_fstab_path', + self.fstab_path, + autospec=False) + + self.add_patch('cloudinit.config.cc_mounts.subp.subp', + 'm_subp_subp') + + self.add_patch('cloudinit.config.cc_mounts.util.mounts', + 'mock_util_mounts', + return_value={ + '/dev/sda1': {'fstype': 'ext4', + 'mountpoint': '/', + 'opts': 'rw,relatime,discard' + }}) + + self.mock_cloud = mock.Mock() + self.mock_log = mock.Mock() + self.mock_cloud.device_name_to_device = self.device_name_to_device + + self.cc = { + 'swap': { + 'filename': self.swap_path, + 'size': '512', + 'maxsize': '512'}} + + def _makedirs(self, directory): + directory = os.path.join(self.new_root, directory.lstrip('/')) + if not os.path.exists(directory): + os.makedirs(directory) + + def device_name_to_device(self, path): + if path == 'swap': + return self.swap_path + else: + dev = None + + return dev + + @mock.patch('cloudinit.util.get_mount_info') + @mock.patch('cloudinit.util.kernel_version') + def test_swap_creation_method_fallocate_on_xfs(self, m_kernel_version, + m_get_mount_info): + m_kernel_version.return_value = (4, 20) + m_get_mount_info.return_value = ["", "xfs"] + + cc_mounts.handle(None, self.cc, self.mock_cloud, self.mock_log, []) + self.m_subp_subp.assert_has_calls([ + mock.call(['fallocate', '-l', '0M', self.swap_path], capture=True), + mock.call(['mkswap', self.swap_path]), + mock.call(['swapon', '-a'])]) + + @mock.patch('cloudinit.util.get_mount_info') + @mock.patch('cloudinit.util.kernel_version') + def test_swap_creation_method_xfs(self, m_kernel_version, + m_get_mount_info): + m_kernel_version.return_value = (3, 18) + m_get_mount_info.return_value = ["", "xfs"] + + cc_mounts.handle(None, self.cc, self.mock_cloud, self.mock_log, []) + self.m_subp_subp.assert_has_calls([ + mock.call(['dd', 'if=/dev/zero', + 'of=' + self.swap_path, + 'bs=1M', 'count=0'], capture=True), + mock.call(['mkswap', self.swap_path]), + mock.call(['swapon', '-a'])]) + + @mock.patch('cloudinit.util.get_mount_info') + @mock.patch('cloudinit.util.kernel_version') + def test_swap_creation_method_btrfs(self, m_kernel_version, + m_get_mount_info): + m_kernel_version.return_value = (4, 20) + m_get_mount_info.return_value = ["", "btrfs"] + + cc_mounts.handle(None, self.cc, self.mock_cloud, self.mock_log, []) + self.m_subp_subp.assert_has_calls([ + mock.call(['dd', 'if=/dev/zero', + 'of=' + self.swap_path, + 'bs=1M', 'count=0'], capture=True), + mock.call(['mkswap', self.swap_path]), + mock.call(['swapon', '-a'])]) + + @mock.patch('cloudinit.util.get_mount_info') + @mock.patch('cloudinit.util.kernel_version') + def test_swap_creation_method_ext4(self, m_kernel_version, + m_get_mount_info): + m_kernel_version.return_value = (5, 14) + m_get_mount_info.return_value = ["", "ext4"] + + cc_mounts.handle(None, self.cc, self.mock_cloud, self.mock_log, []) + self.m_subp_subp.assert_has_calls([ + mock.call(['fallocate', '-l', '0M', self.swap_path], capture=True), + mock.call(['mkswap', self.swap_path]), + mock.call(['swapon', '-a'])]) + + class TestFstabHandling(test_helpers.FilesystemMockingTestCase): swap_path = '/dev/sdb1' diff --git a/tests/unittests/test_util.py b/tests/unittests/test_util.py index 0e71db8..87dc8dd 100644 --- a/tests/unittests/test_util.py +++ b/tests/unittests/test_util.py @@ -1177,4 +1177,19 @@ class TestGetProcEnv(helpers.TestCase): my_ppid = os.getppid() self.assertEqual(my_ppid, util.get_proc_ppid(my_pid)) + +class TestKernelVersion(): + """test kernel version function""" + + params = [ + ('5.6.19-300.fc32.x86_64', (5, 6)), + ('4.15.0-101-generic', (4, 15)), + ('3.10.0-1062.12.1.vz7.131.10', (3, 10)), + ('4.18.0-144.el8.x86_64', (4, 18))] + + @mock.patch('os.uname') + @pytest.mark.parametrize("uname_release,expected", params) + def test_kernel_version(self, m_uname, uname_release, expected): + m_uname.return_value.release = uname_release + assert expected == util.kernel_version() # vi: ts=4 expandtab -- 1.8.3.1