curtin-dev team mailing list archive

[Dan Bungert <mp+420511@xxxxxxxxxxxxxxxxxx>]

Dan Bungert has proposed merging ~dbungert/curtin:resize-vs-wipe into curtin:master.

Commit message:
block/v2: resize-friendly ordering of wipe

Requested reviews:
  curtin developers (curtin-dev)

For more details, see:
https://code.launchpad.net/~dbungert/curtin/+git/curtin/+merge/420511

Split from https://code.launchpad.net/~dbungert/curtin/+git/curtin/+merge/419640 and is expected to have resolved the comments there.
-- 
Your team curtin developers is requested to review the proposed merge of ~dbungert/curtin:resize-vs-wipe into curtin:master.

diff --git a/curtin/commands/block_meta_v2.py b/curtin/commands/block_meta_v2.py
index c1e3630..420e719 100644
--- a/curtin/commands/block_meta_v2.py
+++ b/curtin/commands/block_meta_v2.py
@@ -351,8 +351,20 @@ def disk_handler_v2(info, storage_config, handlers):
             if needs_resize(storage_config, action, part_info):
                 resizes[entry.start] = _prepare_resize(entry, part_info, table)
             preserved_offsets.add(entry.start)
-        wipe = wipes[entry.start] = _wipe_for_action(action)
-        if wipe is not None:
+        wipes[entry.start] = _wipe_for_action(action)
+
+    for kname, nr, offset, size in block.sysfs_partition_data(disk):
+        offset_sectors = table.bytes2sectors(offset)
+        resize = resizes.get(offset_sectors)
+        if resize and size > resize['end']:
+            perform_resize(kname, resize['end'], 'down')
+
+    for kname, nr, offset, size in block.sysfs_partition_data(disk):
+        offset_sectors = table.bytes2sectors(offset)
+        if offset_sectors not in preserved_offsets:
+            # Do a superblock wipe of any partitions that are being deleted.
+            block.wipe_volume(block.kname_to_path(kname), 'superblock')
+        elif wipes.get(offset_sectors) is not None:
             # We do a quick wipe of where any new partitions will be,
             # because if there is bcache or other metadata there, this
             # can cause the partition to be used by a storage
@@ -360,27 +372,17 @@ def disk_handler_v2(info, storage_config, handlers):
             # wipe_volume call below. See
             # https://bugs.launchpad.net/curtin/+bug/1718699 for all
             # the gory details.
-            wipe_offset = table.sectors2bytes(entry.start)
-            LOG.debug('Wiping 1M on %s at offset %s', disk, wipe_offset)
-            block.zero_file_at_offsets(disk, [wipe_offset], exclusive=False)
-
-    for kname, nr, offset, size in block.sysfs_partition_data(disk):
-        offset_sectors = table.bytes2sectors(offset)
-        if offset_sectors not in preserved_offsets:
-            # Do a superblock wipe of any partitions that are being deleted.
-            block.wipe_volume(block.kname_to_path(kname), 'superblock')
-        resize = resizes.get(offset_sectors)
-        if resize and size > resize['end']:
-            perform_resize(kname, resize['end'], 'down')
+            LOG.debug('Wiping 1M on %s at offset %s', disk, offset)
+            block.zero_file_at_offsets(disk, [offset], exclusive=False)
 
     table.apply(disk)
 
     for kname, number, offset, size in block.sysfs_partition_data(disk):
         offset_sectors = table.bytes2sectors(offset)
-        mode = wipes[offset_sectors]
-        if mode is not None:
+        wipe = wipes[offset_sectors]
+        if wipe is not None:
             # Wipe the new partitions as needed.
-            block.wipe_volume(block.kname_to_path(kname), mode)
+            block.wipe_volume(block.kname_to_path(kname), wipe)
         resize = resizes.get(offset_sectors)
         if resize and resize['start'] < size:
             perform_resize(kname, resize['end'], 'up')
diff --git a/tests/integration/test_block_meta.py b/tests/integration/test_block_meta.py
index be69bc0..13795d7 100644
--- a/tests/integration/test_block_meta.py
+++ b/tests/integration/test_block_meta.py
@@ -773,3 +773,45 @@ class TestBlockMeta(IntegrationTestCase):
             self.assertEqual(9 << 20, _get_filesystem_size(dev, p1))
             self.assertEqual(136 << 20, _get_filesystem_size(dev, p5))
             self.assertEqual(50 << 20, _get_filesystem_size(dev, p6))
+
+    def test_split_and_wiping(self):
+        # regression test for a bug where a partition wipe would happen before
+        # a resize was performed, resulting in data loss.
+        img = self.tmp_path('image.img')
+        config = StorageConfigBuilder(version=2)
+        config.add_image(path=img, size='100M', ptable='gpt')
+        p1 = config.add_part(size=98 << 20, offset=1 << 20, number=1,
+                             fstype='ext4')
+        self.run_bm(config.render())
+        with loop_dev(img) as dev:
+            self.assertEqual(
+                summarize_partitions(dev), [
+                    PartData(number=1, offset=1 << 20, size=98 << 20),
+                ])
+            with self.mount(dev, p1) as mnt_point:
+                # Attempt to create files across the partition with gaps
+                for i in range(1, 41):
+                    with open(f'{mnt_point}/{str(i)}', 'wb') as fp:
+                        fp.write(bytes([i]) * (2 << 20))
+                for i in range(1, 41):
+                    if i % 5 != 0:
+                        os.remove(f'{mnt_point}/{str(i)}')
+
+        config = StorageConfigBuilder(version=2)
+        config.add_image(path=img, size='100M', ptable='gpt')
+        p1 = config.add_part(size=49 << 20, offset=1 << 20, number=1,
+                             fstype='ext4', resize=True)
+        config.set_preserve()
+        config.add_part(size=49 << 20, offset=50 << 20, number=2,
+                        fstype='ext4')
+        self.run_bm(config.render())
+        with loop_dev(img) as dev:
+            self.assertEqual(
+                summarize_partitions(dev), [
+                    PartData(number=1, offset=1 << 20, size=49 << 20),
+                    PartData(number=2, offset=50 << 20, size=49 << 20),
+                ])
+            with self.mount(dev, p1) as mnt_point:
+                for i in range(5, 41, 5):
+                    with open(f'{mnt_point}/{str(i)}', 'rb') as fp:
+                        self.assertEqual(bytes([i]) * (2 << 20), fp.read())