curtin-dev team mailing list archive

[Dan Bungert <mp+444944@xxxxxxxxxxxxxxxxxx>]

Dan Bungert has proposed merging ~dbungert/curtin:ask-about-swap into curtin:master.

Commit message:
swap: API improvement, fixes, and tests for can_use_swapfile

Requested reviews:
  curtin developers (curtin-dev)

For more details, see:
https://code.launchpad.net/~dbungert/curtin/+git/curtin/+merge/444944
-- 
Your team curtin developers is requested to review the proposed merge of ~dbungert/curtin:ask-about-swap into curtin:master.

diff --git a/curtin/swap.py b/curtin/swap.py
index 5aea0b8..7acc69e 100644
--- a/curtin/swap.py
+++ b/curtin/swap.py
@@ -93,18 +93,21 @@ def get_target_kernel_version(target):
 
 def can_use_swapfile(target, fstype):
     if fstype is None:
-        raise RuntimeError(
-            'Unknown target filesystem type, may not support swapfiles')
-    if fstype in ['btrfs', 'xfs']:
+        LOG.debug('Unknown target filesystem type, may not support swapfiles')
+        return False
+    elif fstype == 'btrfs':
         # check kernel version
         pkg_ver = get_target_kernel_version(target)
         if not pkg_ver:
-            raise RuntimeError('Failed to read target kernel version')
-        if fstype == 'btrfs' and pkg_ver['major'] < 5:
-            raise RuntimeError(
-                'btrfs requiers kernel version 5.0+ to use swapfiles')
-    elif fstype in ['zfs']:
-        raise RuntimeError('ZFS cannot use swapfiles')
+            LOG.debug('Failed to read target kernel version')
+            return False
+        if pkg_ver['major'] < 5:
+            LOG.debug('btrfs requires kernel version 5.0+ to use swapfiles')
+            return False
+    elif fstype == 'zfs':
+        LOG.debug('ZFS cannot use swapfiles')
+        return False
+    return True
 
 
 def setup_swapfile(target, fstab=None, swapfile=None, size=None, maxsize=None,
@@ -124,13 +127,11 @@ def setup_swapfile(target, fstab=None, swapfile=None, size=None, maxsize=None,
 
     # query the directory in which swapfile will reside
     fstype = get_fstype(target, os.path.dirname(swapfile))
-    try:
-        can_use_swapfile(target, fstype)
-    except RuntimeError as err:
+    if not can_use_swapfile(target, fstype):
         if force:
-            LOG.warning('swapfile may not work: %s', err)
+            LOG.warning('swapfile may not work')
         else:
-            LOG.debug('Not creating swapfile: %s', err)
+            LOG.debug('Not creating swapfile')
             return
 
     allocate_cmd = 'fallocate -l "${2}M" "$1"'
diff --git a/tests/unittests/test_swap.py b/tests/unittests/test_swap.py
index fd6c527..b41cd39 100644
--- a/tests/unittests/test_swap.py
+++ b/tests/unittests/test_swap.py
@@ -51,3 +51,22 @@ class TestSwap(CiTestCase):
         blob = b'\x00\x00c\x05\x00\x00\x11\x19'
         util.write_file(path, int(pagesize * 2 / len(blob)) * blob, omode="wb")
         self.assertFalse(swap.is_swap_device(path))
+
+    def test_swapfile_nonefs(self):
+        self.assertFalse(swap.can_use_swapfile(None, None))
+
+    def test_swapfile_ext4(self):
+        self.assertTrue(swap.can_use_swapfile(None, 'ext4'))
+
+    def test_swapfile_zfs(self):
+        self.assertFalse(swap.can_use_swapfile(None, 'zfs'))
+
+    @mock.patch('curtin.swap.get_target_kernel_version')
+    def test_swapfile_btrfs_oldkernel(self, mock_gtkv):
+        mock_gtkv.return_value = dict(major=4)
+        self.assertFalse(swap.can_use_swapfile(None, 'btrfs'))
+
+    @mock.patch('curtin.swap.get_target_kernel_version')
+    def test_swapfile_btrfs_ok(self, mock_gtkv):
+        mock_gtkv.return_value = dict(major=5)
+        self.assertTrue(swap.can_use_swapfile(None, 'btrfs'))