curtin-dev team mailing list archive

[Ryan Harper <rharper@xxxxxxxxxxx>]

Ryan Harper has proposed merging ~raharper/curtin:fix/swap-files-on-btrfs into curtin:master.

Commit message:
swaps: handle swapfiles on btrfs

Special care and handling are needed for creating swap files on top
of btrfs filesystems.  Curtin will attempt to disable btrfs CoW
on the target file before attempting to fallocate/dd the file.

- Adjust TestBasic to add a swapfile to the /btrfs target
- Add the swap partition/file to TestBasic's fstab unittest
- Fix test_swaps_used to use fstab data instead of storage config
  since file-based swaps are created via curtin 'swap' config not
  storage-config.

LP: #1884161

Requested reviews:
  curtin developers (curtin-dev)
Related bugs:
  Bug #1884161 in curtin: "swap file unsuitable for btrfs"
  https://bugs.launchpad.net/curtin/+bug/1884161

For more details, see:
https://code.launchpad.net/~raharper/curtin/+git/curtin/+merge/386117
-- 
Your team curtin developers is requested to review the proposed merge of ~raharper/curtin:fix/swap-files-on-btrfs into curtin:master.

diff --git a/curtin/swap.py b/curtin/swap.py
index d3f29dc..2b6e74d 100644
--- a/curtin/swap.py
+++ b/curtin/swap.py
@@ -74,6 +74,8 @@ def setup_swapfile(target, fstab=None, swapfile=None, size=None, maxsize=None):
             util.subp(
                 ['sh', '-c',
                  ('rm -f "$1" && umask 0066 && '
+                  'truncate -s 0 "$1" && '
+                  '{ chattr +C "$1" || true; } && '
                   '{ fallocate -l "${2}M" "$1" || '
                   '  dd if=/dev/zero "of=$1" bs=1M "count=$2"; } && '
                   'mkswap "$1" || { r=$?; rm -f "$1"; exit $r; }'),
diff --git a/examples/tests/basic.yaml b/examples/tests/basic.yaml
index 71730c0..82f5ad1 100644
--- a/examples/tests/basic.yaml
+++ b/examples/tests/basic.yaml
@@ -1,4 +1,8 @@
 showtrace: true
+swap:
+    filename: /btrfs/btrfsswap.img
+    size: 1GB
+    maxsize: 1GB
 storage:
     version: 1
     config:
diff --git a/tests/vmtests/__init__.py b/tests/vmtests/__init__.py
index 8ffb7cb..2b21a1c 100644
--- a/tests/vmtests/__init__.py
+++ b/tests/vmtests/__init__.py
@@ -1676,8 +1676,8 @@ class VMBaseClass(TestCase):
                 if spec in line:
                     fstab_entry = line
                     self.assertIsNotNone(fstab_entry)
-                    self.assertEqual(mp, fstab_entry.split(' ')[1])
-                    self.assertEqual(fsopts, fstab_entry.split(' ')[3])
+                    self.assertEqual(mp, fstab_entry.split()[1])
+                    self.assertEqual(fsopts, fstab_entry.split()[3])
                     found.append((spec, mp, fsopts))
 
         self.assertEqual(sorted(expected), sorted(found))
@@ -1810,6 +1810,21 @@ class VMBaseClass(TestCase):
         self.assertEqual(len(uuid), 36)
         return uuid
 
+    def _byuuid_to_kname(self, devpath):
+        # lookup kname via /dev/disk/by-uuid symlink
+        # parsing ls -al output on /dev/disk/by-uuid:
+        # lrwxrwxrwx 1 root root   9 Dec  4 20:02
+        #  d591e9e9-825a-4f0a-b280-3bfaf470b83c -> ../../vdg
+        uuid = os.path.basename(devpath)
+        self.assertIsNotNone(uuid)
+        print(uuid)
+        ls_uuid = self.load_collect_file("ls_al_byuuid")
+        kname = [line.split()[-1] for line in ls_uuid.split('\n')
+                 if uuid in line.split()]
+        self.assertEqual(len(kname), 1)
+        kname = os.path.basename(kname.pop())
+        return kname
+
     def _bcache_to_byuuid(self, kname):
         # extract bcache uuid from /dev/bcache/by-uuid on /dev/<kname>
         # parsing ls -al output on /dev/bcache/by-uuid
@@ -1991,25 +2006,31 @@ class VMBaseClass(TestCase):
 
     @skip_if_flag('expected_failure')
     def test_swaps_used(self):
-        if not self.has_storage_config():
-            raise SkipTest("This test does not use storage config.")
 
-        stgcfg = self.get_storage_config()
-        swap_ids = [d["id"] for d in stgcfg if d.get("fstype") == "swap"]
-        swap_mounts = [d for d in stgcfg if d.get("device") in swap_ids]
-        self.assertEqual(len(swap_ids), len(swap_mounts),
-                         "number config swap fstypes != number swap mounts")
-
-        swaps_found = []
-        for line in self.load_collect_file("proc-swaps").splitlines():
-            fname, ttype, size, used, priority = line.split()
-            if ttype == "partition":
-                swaps_found.append(
-                    {"fname": fname, ttype: "ttype", "size": int(size),
-                     "used": int(used), "priority": int(priority)})
-        self.assertEqual(
-            len(swap_mounts), len(swaps_found),
-            "Number swaps configured != number used")
+        def find_fstab_swaps():
+            swaps = []
+            path = self.collect_path("fstab")
+            if not os.path.exists(path):
+                return swaps
+            for line in util.load_file(path).splitlines():
+                if line.startswith("#"):
+                    continue
+                (fs, mp, fstype, opts, dump, passno) = line.split()
+                if fstype == 'swap':
+                    if fs.startswith('/dev/disk/by-uuid'):
+                        swaps.append('/dev/' + self._byuuid_to_kname(fs))
+                    elif fs.startswith('/dev/disk/by-id'):
+                        kname = self._serial_to_kname(os.path.basename(fs))
+                        swaps.append('/dev/' + kname)
+                    else:
+                        swaps.append(fs)
+
+            return swaps
+
+        expected_swaps = find_fstab_swaps()
+        proc_swaps = self.load_collect_file("proc-swaps")
+        for swap in expected_swaps:
+            self.assertIn(swap, proc_swaps)
 
 
 class PsuedoVMBaseClass(VMBaseClass):
diff --git a/tests/vmtests/test_basic.py b/tests/vmtests/test_basic.py
index 88b9897..b9cee61 100644
--- a/tests/vmtests/test_basic.py
+++ b/tests/vmtests/test_basic.py
@@ -146,7 +146,9 @@ class TestBasicAbs(VMBaseClass):
         return [
             (self._kname_to_byuuid(rootdev + '1'), '/', 'defaults'),
             (self._kname_to_byuuid(rootdev + '2'), '/home', 'defaults'),
-            (self._kname_to_byuuid(btrfsdev), '/btrfs', 'defaults,noatime')
+            (self._kname_to_byuuid(btrfsdev), '/btrfs', 'defaults,noatime'),
+            (self._kname_to_byuuid(rootdev + '3'), 'none', 'sw'),
+            ('/btrfs/btrfsswap.img', 'none', 'sw'),
         ]
 
     def test_whole_disk_uuid(self):
@@ -307,6 +309,8 @@ class TestBasicScsiAbs(TestBasicAbs):
         home_kname = (
             self._serial_to_kname('0x39cc071e72c64cc4-part2'))
         btrfs_kname = self._serial_to_kname('0x22dc58dc023c7008')
+        swap_kname = (
+            self._serial_to_kname('0x39cc071e72c64cc4-part3'))
 
         map_func = self._kname_to_byuuid
         if self.arch == 's390x':
@@ -314,7 +318,9 @@ class TestBasicScsiAbs(TestBasicAbs):
 
         return [(map_func(root_kname), '/', 'defaults'),
                 (map_func(home_kname), '/home', 'defaults'),
-                (map_func(btrfs_kname), '/btrfs', 'defaults,noatime')]
+                (map_func(btrfs_kname), '/btrfs', 'defaults,noatime'),
+                (map_func(swap_kname), 'none', 'sw'),
+                ('/btrfs/btrfsswap.img', 'none', 'sw')]
 
     @skip_if_arch('s390x')
     def test_whole_disk_uuid(self):