From 273999c8b02e9ec2292525f208ee1b4fd3e3f5c8 Mon Sep 17 00:00:00 2001 From: Dan Bungert Date: Tue, 13 Jun 2023 15:30:44 -0600 Subject: [PATCH 1/3] filesystem: rename should_add_swapfile Will be used by the controller to make a swap partition decision. --- subiquity/models/filesystem.py | 4 ++-- subiquity/models/tests/test_filesystem.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/subiquity/models/filesystem.py b/subiquity/models/filesystem.py index 8ae05833..0d9220dc 100644 --- a/subiquity/models/filesystem.py +++ b/subiquity/models/filesystem.py @@ -1584,7 +1584,7 @@ class FilesystemModel(object): } if self.swap is not None: config['swap'] = self.swap - elif not self._should_add_swapfile(): + elif not self.should_add_swapfile(): config['swap'] = {'size': 0} if self.grub is not None: config['grub'] = self.grub @@ -1812,7 +1812,7 @@ class FilesystemModel(object): return (self.is_root_mounted() and not self.needs_bootloader_partition()) - def _should_add_swapfile(self): + def should_add_swapfile(self): mount = self._mount_for_path('/') if mount is not None and mount.device.fstype == 'btrfs': return False diff --git a/subiquity/models/tests/test_filesystem.py b/subiquity/models/tests/test_filesystem.py index 6d5b4253..1d300f84 100644 --- a/subiquity/models/tests/test_filesystem.py +++ b/subiquity/models/tests/test_filesystem.py @@ -1222,7 +1222,7 @@ class TestAlignmentData(unittest.TestCase): class TestSwap(unittest.TestCase): def test_basic(self): m = make_model() - with mock.patch.object(m, '_should_add_swapfile', return_value=False): + with mock.patch.object(m, 'should_add_swapfile', return_value=False): cfg = m.render() self.assertEqual({'size': 0}, cfg['swap']) From 3b0742e5fedcaaddbad54e4f03de2c22c0f4f213 Mon Sep 17 00:00:00 2001 From: Dan Bungert Date: Thu, 15 Jun 2023 11:55:32 -0600 Subject: [PATCH 2/3] filesystem: unittest for existing swap behavior --- subiquity/models/tests/test_filesystem.py | 28 +++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/subiquity/models/tests/test_filesystem.py b/subiquity/models/tests/test_filesystem.py index 1d300f84..3a990a05 100644 --- a/subiquity/models/tests/test_filesystem.py +++ b/subiquity/models/tests/test_filesystem.py @@ -1226,6 +1226,34 @@ class TestSwap(unittest.TestCase): cfg = m.render() self.assertEqual({'size': 0}, cfg['swap']) + @parameterized.expand([ + ['ext4'], + ['btrfs'], + ]) + def test_should_add_swapfile_nomount(self, fs): + m, d1 = make_model_and_disk(Bootloader.BIOS) + d1p1 = make_partition(m, d1) + m.add_filesystem(d1p1, fs) + self.assertTrue(m.should_add_swapfile()) + + @parameterized.expand([ + ['ext4', True], + ['btrfs', False], + ]) + def test_should_add_swapfile(self, fs, expected): + m, d1 = make_model_and_disk(Bootloader.BIOS) + d1p1 = make_partition(m, d1) + m.add_mount(m.add_filesystem(d1p1, fs), '/') + self.assertEqual(expected, m.should_add_swapfile()) + + def test_should_add_swapfile_has_swappart(self): + m, d1 = make_model_and_disk(Bootloader.BIOS) + d1p1 = make_partition(m, d1) + d1p2 = make_partition(m, d1) + m.add_mount(m.add_filesystem(d1p1, 'ext4'), '/') + m.add_mount(m.add_filesystem(d1p2, 'swap'), '') + self.assertFalse(m.should_add_swapfile()) + class TestPartition(unittest.TestCase): From 0afda56696cf27f298703feb39fe713cb84b027d Mon Sep 17 00:00:00 2001 From: Dan Bungert Date: Tue, 13 Jun 2023 15:32:44 -0600 Subject: [PATCH 3/3] filesystem: ask curtin about swapfiles Curtin and Subiquity both have swap file decision logic, let curtin be the source of truth. We can tweak the desired behavior in curtin. --- subiquity/models/filesystem.py | 6 ++++-- subiquity/models/tests/test_filesystem.py | 12 ++++++++---- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/subiquity/models/filesystem.py b/subiquity/models/filesystem.py index 0d9220dc..71cc5ee6 100644 --- a/subiquity/models/filesystem.py +++ b/subiquity/models/filesystem.py @@ -32,6 +32,7 @@ import more_itertools from curtin import storage_config from curtin.block import partition_kname +from curtin.swap import can_use_swapfile from curtin.util import human2bytes from probert.storage import StorageInfo @@ -1814,8 +1815,9 @@ class FilesystemModel(object): def should_add_swapfile(self): mount = self._mount_for_path('/') - if mount is not None and mount.device.fstype == 'btrfs': - return False + if mount is not None: + if not can_use_swapfile('/', mount.device.fstype): + return False for swap in self._all(type='format', fstype='swap'): if swap.mount(): return False diff --git a/subiquity/models/tests/test_filesystem.py b/subiquity/models/tests/test_filesystem.py index 3a990a05..7c563d55 100644 --- a/subiquity/models/tests/test_filesystem.py +++ b/subiquity/models/tests/test_filesystem.py @@ -1237,14 +1237,18 @@ class TestSwap(unittest.TestCase): self.assertTrue(m.should_add_swapfile()) @parameterized.expand([ - ['ext4', True], - ['btrfs', False], + ['ext4', None, True], + ['btrfs', 5, True], + ['btrfs', 4, False], + ['zfs', None, False], ]) - def test_should_add_swapfile(self, fs, expected): + def test_should_add_swapfile(self, fs, kern_maj_ver, expected): m, d1 = make_model_and_disk(Bootloader.BIOS) d1p1 = make_partition(m, d1) m.add_mount(m.add_filesystem(d1p1, fs), '/') - self.assertEqual(expected, m.should_add_swapfile()) + with mock.patch('curtin.swap.get_target_kernel_version', + return_value={'major': kern_maj_ver}): + self.assertEqual(expected, m.should_add_swapfile()) def test_should_add_swapfile_has_swappart(self): m, d1 = make_model_and_disk(Bootloader.BIOS)