curtin-dev team mailing list archive
-
curtin-dev team
-
Mailing list archive
-
Message #02265
Re: [Merge] ~dbungert/curtin:resize into curtin:master
Review: Needs Fixing
This is really really close now. I think the only gap is that I don't think this handles resizing a partition that is to be reformatted quite correctly -- there is no need to resize the filesystem in this case, so if it's too full to resize or not a format we know how to resize, we should still go ahead.
Diff comments:
> diff --git a/curtin/commands/block_meta.py b/curtin/commands/block_meta.py
> index cdf30c5..c986e94 100644
> --- a/curtin/commands/block_meta.py
> +++ b/curtin/commands/block_meta.py
> @@ -923,12 +928,6 @@ def partition_handler(info, storage_config, handlers):
> # start sector is part of the sectors that define the partitions size
> # so length has to be "size in sectors - 1"
> length_sectors = int(length_bytes / logical_block_size_bytes) - 1
> - # logical partitions can't share their start sector with the extended
> - # partition and logical partitions can't go head-to-head, so we have to
> - # realign and for that increase size as required
> - if info.get('flag') == "extended":
> - logdisks = getnumberoflogicaldisks(device, storage_config)
> - length_sectors = length_sectors + (logdisks * alignment_offset)
I mean, I think this behaviour is crazy too but maybe its something for a different branch?
>
> # Handle preserve flag
> create_partition = True
> diff --git a/curtin/commands/block_meta_v2.py b/curtin/commands/block_meta_v2.py
> index 051649b..f882ed9 100644
> --- a/curtin/commands/block_meta_v2.py
> +++ b/curtin/commands/block_meta_v2.py
> @@ -214,6 +239,60 @@ def _wipe_for_action(action):
> return 'superblock'
>
>
> +def _prepare_resize(entry, part_info, table):
> + return {
> + 'start': table.sectors2bytes(part_info['size']),
> + 'end': table.sectors2bytes(entry.size),
> + }
> +
> +
> +def verify_format(devpath, storage_config, part_action):
> + actions = [v for v in storage_config.values()
> + if v['type'] == 'format' and v['volume'] == part_action['id']]
a) I think this only needs to look for _preserved_ formats, right? If we're reformatting the partition anyway we don't need to worry about resizing the filesystem...
b) can this at least yell into the log if it finds more than one action?
> + if not actions:
> + return
> + fstype = _get_volume_fstype(devpath)
> + target_fstype = 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, )
> + LOG.debug(msg)
> + if fstype not in resizers:
> + raise RuntimeError(msg + ' is unsupported')
> +
> +
> +def verify_offset(devpath, part_action, current_info, table):
> + if 'offset' not in part_action:
> + return
> + current_offset = table.sectors2bytes(current_info['start'])
> + action_offset = int(util.human2bytes(part_action['offset']))
> + msg = (
> + 'Verifying %s offset, expecting %s, found %s' % (
> + devpath, current_offset, action_offset))
> + LOG.debug(msg)
> + if current_offset != action_offset:
> + raise RuntimeError(msg)
> +
> +
> +def partition_verify_sfdisk_v2(part_action, label, sfdisk_part_info,
> + storage_config, table):
> + devpath = os.path.realpath(sfdisk_part_info['node'])
> + verify_format(devpath, storage_config, part_action)
> + if not part_action.get('resize', False):
> + verify_size(devpath, int(util.human2bytes(part_action['size'])),
> + sfdisk_part_info)
> + verify_offset(devpath, part_action, sfdisk_part_info, table)
> + expected_flag = part_action.get('flag')
> + if expected_flag:
> + verify_ptable_flag(devpath, expected_flag, label, sfdisk_part_info)
> +
> +
> def disk_handler_v2(info, storage_config, handlers):
> disk_handler_v1(info, storage_config, handlers)
>
> @@ -251,7 +331,10 @@ def disk_handler_v2(info, storage_config, handlers):
> # vmtest infrastructure unhappy.
> sfdisk_info = block.sfdisk_info(disk)
> part_info = _find_part_info(sfdisk_info, entry.start)
> - partition_verify_sfdisk(action, sfdisk_info['label'], part_info)
> + partition_verify_sfdisk_v2(action, sfdisk_info['label'], part_info,
> + storage_config, table)
> + if action.get('resize'):
> + resizes[entry.start] = _prepare_resize(entry, part_info, table)
Here too, we only want to do a resize if the format is preserved, right?
> preserved_offsets.add(entry.start)
> wipe = wipes[entry.start] = _wipe_for_action(action)
> if wipe is not None:
> diff --git a/tests/integration/test_block_meta.py b/tests/integration/test_block_meta.py
> index 0c74cd6..2b939d0 100644
> --- a/tests/integration/test_block_meta.py
> +++ b/tests/integration/test_block_meta.py
> @@ -34,6 +35,32 @@ def loop_dev(image, sector_size=512):
> PartData = namedtuple("PartData", ('number', 'offset', 'size'))
>
>
> +def _get_filesystem_size(dev, part_action, fstype='ext4'):
> + if fstype not in ('ext2', 'ext3', 'ext4'):
> + raise Exception(f'_get_filesystem_size: no support for {fstype}')
> + num = part_action['number']
> + cmd = ['dumpe2fs', '-h', f'{dev}p{num}']
> + out = util.subp(cmd, capture=True)[0]
> + for line in out.splitlines():
> + if line.startswith('Block count'):
> + block_count = line.split(':')[1].strip()
> + if line.startswith('Block size'):
> + block_size = line.split(':')[1].strip()
> + return int(block_count) * int(block_size)
Ooh code to put into probert at some point!
> +
> +
> +def _get_extended_partition_size(dev, num):
> + # sysfs reports extended partitions as having 1K size
> + # sfdisk seems to have a better idea
> + ptable_json = util.subp(['sfdisk', '-J', dev], capture=True)[0]
> + ptable = json.loads(ptable_json)
> +
> + nodename = f'{dev}p{num}'
> + partitions = ptable['partitiontable']['partitions']
> + partition = [part for part in partitions if part['node'] == nodename][0]
> + return partition['size'] * 512
> +
> +
> def summarize_partitions(dev):
> # We don't care about the kname
> return sorted(
> @@ -415,3 +468,306 @@ class TestBlockMeta(IntegrationTestCase):
> )
> finally:
> server.stop()
> +
> + def _do_test_resize(self, start, end, fstype):
> + start <<= 20
> + end <<= 20
> + img = self.tmp_path('image.img')
> + config = StorageConfigBuilder(version=2)
> + config.add_image(path=img, size='200M', ptable='gpt')
> + p1 = config.add_part(size=start, offset=1 << 20, number=1,
> + fstype=fstype)
> + self.run_bm(config.render())
> + with loop_dev(img) as dev:
> + self.create_data(dev, p1)
> + self.assertEqual(
> + summarize_partitions(dev), [
> + PartData(number=1, offset=1 << 20, size=start),
> + ])
> + fs_size = _get_filesystem_size(dev, p1, fstype)
> + self.assertEqual(start, fs_size)
> +
> + config.set_preserve()
> + p1['resize'] = True
> + p1['size'] = end
> + self.run_bm(config.render())
> + with loop_dev(img) as dev:
> + self.check_data(dev, p1)
> + self.assertEqual(
> + summarize_partitions(dev), [
> + PartData(number=1, offset=1 << 20, size=end),
> + ])
> + fs_size = _get_filesystem_size(dev, p1, fstype)
> + self.assertEqual(end, fs_size)
> +
> + def test_resize_up_ext2(self):
> + self._do_test_resize(40, 80, 'ext2')
> +
> + def test_resize_down_ext2(self):
> + self._do_test_resize(80, 40, 'ext2')
> +
> + def test_resize_up_ext3(self):
> + self._do_test_resize(40, 80, 'ext3')
> +
> + def test_resize_down_ext3(self):
> + self._do_test_resize(80, 40, 'ext3')
> +
> + def test_resize_up_ext4(self):
> + self._do_test_resize(40, 80, 'ext4')
> +
> + def test_resize_down_ext4(self):
> + self._do_test_resize(80, 40, 'ext4')
> +
> + def test_resize_logical(self):
> + img = self.tmp_path('image.img')
> + config = StorageConfigBuilder(version=2)
> + config.add_image(path=img, size='100M', ptable='msdos')
> + config.add_part(size='50M', number=1, flag='extended', offset=1 << 20)
> + config.add_part(size='10M', number=5, flag='logical', offset=2 << 20)
> + p6 = config.add_part(size='10M', number=6, flag='logical',
> + offset=13 << 20, fstype='ext4')
> + self.run_bm(config.render())
> +
> + with loop_dev(img) as dev:
> + self.create_data(dev, p6)
> + self.assertEqual(
> + summarize_partitions(dev), [
> + # extended partitions get a strange size in sysfs
> + PartData(number=1, offset=1 << 20, size=1 << 10),
> + PartData(number=5, offset=2 << 20, size=10 << 20),
> + # part 5 takes us to 12 MiB offset, curtin leaves a 1 MiB
> + # gap.
> + PartData(number=6, offset=13 << 20, size=10 << 20),
> + ])
> + self.assertEqual(50 << 20, _get_extended_partition_size(dev, 1))
> +
> + config.set_preserve()
> + p6['resize'] = True
> + p6['size'] = '20M'
> + self.run_bm(config.render())
> +
> + with loop_dev(img) as dev:
> + self.check_data(dev, p6)
> + self.assertEqual(
> + summarize_partitions(dev), [
> + PartData(number=1, offset=1 << 20, size=1 << 10),
> + PartData(number=5, offset=2 << 20, size=10 << 20),
> + PartData(number=6, offset=13 << 20, size=20 << 20),
> + ])
> + self.assertEqual(50 << 20, _get_extended_partition_size(dev, 1))
> +
> + def test_resize_extended(self):
> + img = self.tmp_path('image.img')
> + config = StorageConfigBuilder(version=2)
> + config.add_image(path=img, size='100M', ptable='msdos')
> + p1 = config.add_part(size='50M', number=1, flag='extended',
> + offset=1 << 20)
> + p5 = config.add_part(size='49M', number=5, flag='logical',
> + offset=2 << 20)
> + self.run_bm(config.render())
> +
> + with loop_dev(img) as dev:
> + self.assertEqual(
> + summarize_partitions(dev), [
> + # extended partitions get a strange size in sysfs
> + PartData(number=1, offset=1 << 20, size=1 << 10),
> + PartData(number=5, offset=2 << 20, size=49 << 20),
> + ])
> + self.assertEqual(50 << 20, _get_extended_partition_size(dev, 1))
> +
> + config.set_preserve()
> + p1['resize'] = True
> + p1['size'] = '99M'
> + p5['resize'] = True
> + p5['size'] = '98M'
> + self.run_bm(config.render())
> +
> + with loop_dev(img) as dev:
> + self.assertEqual(
> + summarize_partitions(dev), [
> + PartData(number=1, offset=1 << 20, size=1 << 10),
> + PartData(number=5, offset=2 << 20, size=98 << 20),
> + ])
> + self.assertEqual(99 << 20, _get_extended_partition_size(dev, 1))
> +
> + def test_split(self):
> + img = self.tmp_path('image.img')
> + config = StorageConfigBuilder(version=2)
> + config.add_image(path=img, size='200M', ptable='gpt')
> + config.add_part(size=9 << 20, offset=1 << 20, number=1)
> + p2 = config.add_part(size='180M', offset=10 << 20, number=2,
> + fstype='ext4')
> + self.run_bm(config.render())
> + with loop_dev(img) as dev:
> + self.create_data(dev, p2)
> + self.assertEqual(
> + summarize_partitions(dev), [
> + PartData(number=1, offset=1 << 20, size=9 << 20),
> + PartData(number=2, offset=10 << 20, size=180 << 20),
> + ])
> + self.assertEqual(180 << 20, _get_filesystem_size(dev, p2))
> +
> + config.set_preserve()
> + p2['resize'] = True
> + p2['size'] = '80M'
> + p3 = config.add_part(size='100M', offset=90 << 20, number=3,
> + fstype='ext4')
> + self.run_bm(config.render())
> + with loop_dev(img) as dev:
> + self.check_data(dev, p2)
> + self.assertEqual(
> + summarize_partitions(dev), [
> + PartData(number=1, offset=1 << 20, size=9 << 20),
> + PartData(number=2, offset=10 << 20, size=80 << 20),
> + PartData(number=3, offset=90 << 20, size=100 << 20),
> + ])
> + self.assertEqual(80 << 20, _get_filesystem_size(dev, p2))
> + self.assertEqual(100 << 20, _get_filesystem_size(dev, p3))
> +
> + def test_partition_unify(self):
> + img = self.tmp_path('image.img')
> + config = StorageConfigBuilder(version=2)
> + config.add_image(path=img, size='200M', ptable='gpt')
> + config.add_part(size=9 << 20, offset=1 << 20, number=1)
> + p2 = config.add_part(size='40M', offset=10 << 20, number=2,
> + fstype='ext4')
> + p3 = config.add_part(size='60M', offset=50 << 20, number=3,
> + fstype='ext4')
> + self.run_bm(config.render())
> + with loop_dev(img) as dev:
> + self.create_data(dev, p2)
> + self.assertEqual(
> + summarize_partitions(dev), [
> + PartData(number=1, offset=1 << 20, size=9 << 20),
> + PartData(number=2, offset=10 << 20, size=40 << 20),
> + PartData(number=3, offset=50 << 20, size=60 << 20),
> + ])
> + self.assertEqual(40 << 20, _get_filesystem_size(dev, p2))
> + self.assertEqual(60 << 20, _get_filesystem_size(dev, p3))
> +
> + config = StorageConfigBuilder(version=2)
> + config.add_image(path=img, size='200M', ptable='gpt')
> + config.add_part(size=9 << 20, offset=1 << 20, number=1)
> + p2 = config.add_part(size='100M', offset=10 << 20, number=2,
> + fstype='ext4', resize=True)
> + config.set_preserve()
> + self.run_bm(config.render())
> + with loop_dev(img) as dev:
> + self.check_data(dev, p2)
> + self.assertEqual(
> + summarize_partitions(dev), [
> + PartData(number=1, offset=1 << 20, size=9 << 20),
> + PartData(number=2, offset=10 << 20, size=100 << 20),
> + ])
> + self.assertEqual(100 << 20, _get_filesystem_size(dev, p2))
> +
> + def test_mix_of_operations_gpt(self):
> + # a test that keeps, creates, resizes, and deletes a partition
> + # 200 MiB disk, using full disk
> + # init size preserve final size
> + # p1 - 9 MiB yes 9MiB
> + # p2 - 90 MiB yes, resize 139MiB
> + # p3 - 99 MiB no 50MiB
> + img = self.tmp_path('image.img')
> + config = StorageConfigBuilder(version=2)
> + config.add_image(path=img, size='200M', ptable='gpt')
> + config.add_part(size=9 << 20, offset=1 << 20, number=1)
> + p2 = config.add_part(size='90M', offset=10 << 20, number=2,
> + fstype='ext4')
> + p3 = config.add_part(size='99M', offset=100 << 20, number=3,
> + fstype='ext4')
> + self.run_bm(config.render())
> + with loop_dev(img) as dev:
> + self.create_data(dev, p2)
> + self.assertEqual(
> + summarize_partitions(dev), [
> + PartData(number=1, offset=1 << 20, size=9 << 20),
> + PartData(number=2, offset=10 << 20, size=90 << 20),
> + PartData(number=3, offset=100 << 20, size=99 << 20),
> + ])
> + self.assertEqual(90 << 20, _get_filesystem_size(dev, p2))
> + self.assertEqual(99 << 20, _get_filesystem_size(dev, p3))
> +
> + config = StorageConfigBuilder(version=2)
> + config.add_image(path=img, size='200M', ptable='gpt')
> + config.add_part(size=9 << 20, offset=1 << 20, number=1)
> + p2 = config.add_part(size='139M', offset=10 << 20, number=2,
> + fstype='ext4', resize=True)
> + config.set_preserve()
> + p3 = config.add_part(size='50M', offset=149 << 20, number=3,
> + fstype='ext4')
> + self.run_bm(config.render())
> + with loop_dev(img) as dev:
> + self.check_data(dev, p2)
> + self.assertEqual(
> + summarize_partitions(dev), [
> + PartData(number=1, offset=1 << 20, size=9 << 20),
> + PartData(number=2, offset=10 << 20, size=139 << 20),
> + PartData(number=3, offset=149 << 20, size=50 << 20),
> + ])
> + self.assertEqual(139 << 20, _get_filesystem_size(dev, p2))
> + self.assertEqual(50 << 20, _get_filesystem_size(dev, p3))
> +
> + def test_mix_of_operations_msdos(self):
> + # a test that keeps, creates, resizes, and deletes a partition
> + # including handling of extended/logical
> + # 200 MiB disk, initially only using front 100MiB
> + # flag init size preserve final size
> + # p1 - primary 9MiB yes 9MiB
> + # p2 - extended 89MiB yes, resize 189MiB
> + # p3 - logical 37MiB yes, resize 137MiB
> + # p4 - logical 50MiB no 50MiB
> + img = self.tmp_path('image.img')
> + config = StorageConfigBuilder(version=2)
> + config.add_image(path=img, size='200M', ptable='msdos')
> + p1 = config.add_part(size='9M', offset=1 << 20, number=1,
> + fstype='ext4')
> + config.add_part(size='89M', offset=10 << 20, number=2, flag='extended')
> + p5 = config.add_part(size='36M', offset=11 << 20, number=5,
> + flag='logical', fstype='ext4')
> + p6 = config.add_part(size='50M', offset=49 << 20, number=6,
> + flag='logical', fstype='ext4')
> + self.run_bm(config.render())
> +
> + with loop_dev(img) as dev:
> + self.create_data(dev, p1)
> + self.create_data(dev, p5)
> + self.assertEqual(
> + summarize_partitions(dev), [
> + PartData(number=1, offset=1 << 20, size=9 << 20),
> + PartData(number=2, offset=10 << 20, size=1 << 10),
> + PartData(number=5, offset=11 << 20, size=36 << 20),
> + PartData(number=6, offset=49 << 20, size=50 << 20),
> + ])
> + self.assertEqual(89 << 20, _get_extended_partition_size(dev, 2))
> + self.assertEqual(9 << 20, _get_filesystem_size(dev, p1))
> + self.assertEqual(36 << 20, _get_filesystem_size(dev, p5))
> + self.assertEqual(50 << 20, _get_filesystem_size(dev, p6))
> +
> + config = StorageConfigBuilder(version=2)
> + config.add_image(path=img, size='200M', ptable='msdos')
> + p1 = config.add_part(size='9M', offset=1 << 20, number=1,
> + fstype='ext4')
> + config.add_part(size='189M', offset=10 << 20, number=2,
> + flag='extended', resize=True)
> + p5 = config.add_part(size='136M', offset=11 << 20, number=5,
> + flag='logical', fstype='ext4', resize=True)
> + config.set_preserve()
> + p6 = config.add_part(size='50M', offset=149 << 20, number=6,
> + flag='logical', fstype='ext4')
> + self.run_bm(config.render())
> +
> + with loop_dev(img) as dev:
> + self.check_data(dev, p1)
> + self.check_data(dev, p5)
> + self.assertEqual(
> + summarize_partitions(dev), [
> + PartData(number=1, offset=1 << 20, size=9 << 20),
> + PartData(number=2, offset=10 << 20, size=1 << 10),
> + PartData(number=5, offset=11 << 20, size=136 << 20),
> + PartData(number=6, offset=149 << 20, size=50 << 20),
> + ])
> + self.assertEqual(189 << 20, _get_extended_partition_size(dev, 2))
> + 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))
Thanks so much for this. I keep on wishing for a more compact way to describe this stuff!
--
https://code.launchpad.net/~dbungert/curtin/+git/curtin/+merge/417829
Your team curtin developers is subscribed to branch curtin:master.
References
