Dan Bungert has proposed merging ~dbungert/curtin:resize-no-format-action into
curtin:master.
Commit message:
Fix two forms of resize data corruption
* if resizing downward, a wipe of a partition could be done too early
* if format was unspecified, the resize would not happen, and if the
partition needed a resize downward it would be corrupt
Requested reviews:
curtin developers (curtin-dev)
For more details, see:
https://code.launchpad.net/~dbungert/curtin/+git/curtin/+merge/419640
--
Your team curtin developers is requested to review the proposed merge of
~dbungert/curtin:resize-no-format-action into curtin:master.
diff --git a/curtin/commands/block_meta_v2.py b/curtin/commands/block_meta_v2.py
index c1e3630..fd87acc 100644
--- a/curtin/commands/block_meta_v2.py
+++ b/curtin/commands/block_meta_v2.py
@@ -60,13 +60,13 @@ def resize_ext(path, size):
util.subp(['resize2fs', path, f'{size_k}k'])
-def perform_resize(kname, size, direction):
+def perform_resize(kname, resize, direction):
path = block.kname_to_path(kname)
- fstype = _get_volume_fstype(path)
- if fstype:
- LOG.debug('Resizing %s of type %s %s to %s',
- path, fstype, direction, size)
- resizers[fstype](path, size)
+ fstype = resize['fstype']
+ size = resize['end']
+ LOG.debug('Resizing %s of type %s %s to %s',
+ path, fstype, direction, size)
+ resizers[fstype](path, size)
resizers = {
@@ -241,7 +241,10 @@ def _wipe_for_action(action):
def _prepare_resize(entry, part_info, table):
+ devpath = os.path.realpath(part_info['node'])
+ fstype = _get_volume_fstype(devpath)
return {
+ 'fstype': fstype,
'start': table.sectors2bytes(part_info['size']),
'end': table.sectors2bytes(entry.size),
}
@@ -253,26 +256,28 @@ def needs_resize(storage_config, part_action, sfdisk_part_info):
if not part_action.get('resize'):
return False
+ devpath = os.path.realpath(sfdisk_part_info['node'])
+ fstype = _get_volume_fstype(devpath)
+ if fstype == '':
+ return False
+
volume = part_action['id']
format_actions = select_configs(storage_config, type='format',
- volume=volume, preserve=True)
- if len(format_actions) < 1:
- return False
+ volume=volume)
if len(format_actions) > 1:
raise Exception(f'too many format actions for volume {volume}')
- if not format_actions[0].get('preserve'):
- return False
+ if len(format_actions) == 1:
+ if not format_actions[0].get('preserve'):
+ return False
- devpath = os.path.realpath(sfdisk_part_info['node'])
- fstype = _get_volume_fstype(devpath)
- target_fstype = format_actions[0]['fstype']
- msg = (
- 'Verifying %s format, expecting %s, found %s' % (
- devpath, fstype, target_fstype))
- LOG.debug(msg)
- if fstype != target_fstype:
- raise RuntimeError(msg)
+ target_fstype = format_actions[0]['fstype']
+ msg = (
+ 'Verifying %s format, expecting %s, found %s' % (
+ devpath, fstype, target_fstype))
+ LOG.debug(msg)
+ if fstype != target_fstype:
+ raise RuntimeError(msg)
if part_action.get('resize'):
msg = 'Resize requested for format %s' % (fstype, )
@@ -351,18 +356,7 @@ 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:
- # 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
- # subsystem and preventing the exclusive open done by the
- # 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)
+ wipes[entry.start] = _wipe_for_action(action)
for kname, nr, offset, size in block.sysfs_partition_data(disk):
offset_sectors = table.bytes2sectors(offset)
@@ -371,19 +365,28 @@ def disk_handler_v2(info, storage_config, handlers):
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')
+ perform_resize(kname, resize, 'down')
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:
+ # 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
+ # subsystem and preventing the exclusive open done by the
+ # wipe_volume call below. See
+ # https://bugs.launchpad.net/curtin/+bug/1718699 for all
+ # the gory details.
+ LOG.debug('Wiping 1M on %s at offset %s', disk, offset)
+ block.zero_file_at_offsets(disk, [offset], exclusive=False)
# 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')
+ perform_resize(kname, resize, 'up')
# Make the names if needed
if 'name' in info:
diff --git a/tests/unittests/test_commands_block_meta.py b/tests/unittests/test_commands_block_meta.py
index 3698d32..3ccddab 100644
--- a/tests/unittests/test_commands_block_meta.py
+++ b/tests/unittests/test_commands_block_meta.py
@@ -2702,6 +2702,34 @@ class TestPartitionNeedsResize(CiTestCase):
self.format['id']: self.format,
}
+ def test_partition_resize_happy_path(self):
+ self.partition['preserve'] = True
+ self.partition['resize'] = True
+ self.format['preserve'] = True
+ self.format['fstype'] = 'ext4'
+ self.m_get_volume_fstype.return_value = 'ext4'
+ self.assertTrue(
+ block_meta_v2.needs_resize(
+ self.storage_config, self.partition, self.sfdisk_part_info))
+
+ def test_partition_resize_no_format_action(self):
+ self.partition['preserve'] = True
+ self.partition['resize'] = True
+ self.storage_config = {self.partition['id']: self.partition}
+ self.m_get_volume_fstype.return_value = 'ext4'
+ self.assertTrue(
+ block_meta_v2.needs_resize(
+ self.storage_config, self.partition, self.sfdisk_part_info))
+
+ def test_partition_resize_unformatted(self):
+ self.partition['preserve'] = True
+ self.partition['resize'] = True
+ self.storage_config = {self.partition['id']: self.partition}
+ self.m_get_volume_fstype.return_value = ''
+ self.assertFalse(
+ block_meta_v2.needs_resize(
+ self.storage_config, self.partition, self.sfdisk_part_info))
+
def test_partition_resize_change_fs(self):
self.partition['preserve'] = True
self.partition['resize'] = True
@@ -2730,8 +2758,9 @@ class TestPartitionNeedsResize(CiTestCase):
self.format['preserve'] = False
self.format['fstype'] = 'reiserfs'
self.m_get_volume_fstype.return_value = 'reiserfs'
- block_meta_v2.needs_resize(
- self.storage_config, self.partition, self.sfdisk_part_info)
+ self.assertFalse(
+ block_meta_v2.needs_resize(
+ self.storage_config, self.partition, self.sfdisk_part_info))
def test_partition_resize_partition_preserve_false(self):
# not a resize - partition is recreated
@@ -2740,8 +2769,9 @@ class TestPartitionNeedsResize(CiTestCase):
self.format['preserve'] = False
self.format['fstype'] = 'reiserfs'
self.m_get_volume_fstype.return_value = 'reiserfs'
- block_meta_v2.needs_resize(
- self.storage_config, self.partition, self.sfdisk_part_info)
+ self.assertFalse(
+ block_meta_v2.needs_resize(
+ self.storage_config, self.partition, self.sfdisk_part_info))
class TestPartitionVerifyFdasd(CiTestCase):
--
Mailing list: https://launchpad.net/~curtin-dev
Post to : curtin-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~curtin-dev
More help : https://help.launchpad.net/ListHelp
