From ecca97805300de2ab8baef1c849923fefdd50e3a Mon Sep 17 00:00:00 2001 From: Michael Marineau Date: Wed, 16 Oct 2013 18:05:54 -0700 Subject: [PATCH 1/3] fix(cgpt.py): Fix GPT reserved space calculations. The existing code arbitrarily multiplies START_SECTOR by 512 converting from blocks/sectors to bytes, but blocks was the correct unit to begin with. Also the secondary GPT area is not considered but that was OK because the bogus unit conversion oversized our disks by almost 16MB. Instead of relying on bugs properly reserve 34 sectors at each end of the disk. (Well, we could get away with only 33 at the end since it doesn't have a MBR but meh.) --- build_library/cgpt.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/build_library/cgpt.py b/build_library/cgpt.py index 2176cf258e..2d6d54d3f4 100755 --- a/build_library/cgpt.py +++ b/build_library/cgpt.py @@ -12,7 +12,7 @@ import uuid from optparse import OptionParser # First sector we can use. -START_SECTOR = 64 +GPT_RESERVED_SECTORS = 34 class ConfigNotFound(Exception): pass @@ -222,14 +222,16 @@ def WritePartitionTable(options, image_type, layout_filename, disk_filename): config = LoadPartitionConfig(layout_filename) partitions = GetPartitionTable(options, config, image_type) - disk_block_count = START_SECTOR * config['metadata']['block_size'] + disk_block_count = GPT_RESERVED_SECTORS for partition in partitions: disk_block_count += partition['blocks'] - sector = START_SECTOR + disk_block_count += GPT_RESERVED_SECTORS + Cgpt('create', '-c', '-s', disk_block_count, disk_filename) + sector = GPT_RESERVED_SECTORS for partition in partitions: if partition['type'] != 'blank': Cgpt('add', '-i', partition['num'], From 03754b2d62e7f74166622764cb55fa74c302534b Mon Sep 17 00:00:00 2001 From: Michael Marineau Date: Wed, 16 Oct 2013 18:33:20 -0700 Subject: [PATCH 2/3] fix(cgpt.py): Add support for aligning partitions When using anything other than classic spinning disks with 512 sectors it is generally best to maintain some alignment with the underlying physical sector or erase block size. The default alignment most partitioning tools use these days is 1MB (2048 sectors). Also sometimes qemu-img requires disk sizes to be aligned to 64KB. --- build_library/cgpt.py | 21 +++++++++++++++++++-- build_library/legacy_disk_layout.json | 1 + 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/build_library/cgpt.py b/build_library/cgpt.py index 2d6d54d3f4..0299730204 100755 --- a/build_library/cgpt.py +++ b/build_library/cgpt.py @@ -36,7 +36,7 @@ def LoadPartitionConfig(filename): valid_keys = set(('_comment', 'metadata', 'layouts')) valid_layout_keys = set(( '_comment', 'type', 'num', 'label', 'blocks', 'block_size', 'fs_blocks', - 'fs_block_size', 'features', 'uuid')) + 'fs_block_size', 'features', 'uuid', 'alignment')) if not os.path.exists(filename): raise ConfigNotFound('Partition config %s was not found!' % filename) @@ -45,9 +45,14 @@ def LoadPartitionConfig(filename): try: metadata = config['metadata'] - for key in ('block_size', 'fs_block_size'): + for key in ('alignment', 'block_size', 'fs_block_size'): metadata[key] = int(metadata[key]) + # Sometimes qemu-img expects disks sizes aligned to 64k + align_bytes = metadata['alignment'] * metadata['block_size'] + if align_bytes < 65536 or align_bytes % 65536 != 0: + raise InvalidLayout('Invalid alignment, 64KB or better required') + unknown_keys = set(config.keys()) - valid_keys if unknown_keys: raise InvalidLayout('Unknown items: %r' % unknown_keys) @@ -67,6 +72,7 @@ def LoadPartitionConfig(filename): if not s in part: raise InvalidLayout('Layout "%s" missing "%s"' % (layout_name, s)) + part['alignment'] = int(part.get('alignment', metadata['alignment'])) part['blocks'] = int(part['blocks']) part['bytes'] = part['blocks'] * metadata['block_size'] @@ -220,19 +226,30 @@ def WritePartitionTable(options, image_type, layout_filename, disk_filename): def Cgpt(*args): subprocess.check_call(['cgpt'] + [str(a) for a in args]) + def Align(count, alignment): + offset = count % alignment + if offset: + count += alignment - offset + return count + config = LoadPartitionConfig(layout_filename) + metadata = config['metadata'] partitions = GetPartitionTable(options, config, image_type) disk_block_count = GPT_RESERVED_SECTORS for partition in partitions: + disk_block_count = Align(disk_block_count, partition['alignment']) disk_block_count += partition['blocks'] disk_block_count += GPT_RESERVED_SECTORS + # Sometimes qemu-img expects disks sizes aligned to 64k + disk_block_count = Align(disk_block_count, config['metadata']['alignment']) Cgpt('create', '-c', '-s', disk_block_count, disk_filename) sector = GPT_RESERVED_SECTORS for partition in partitions: + sector = Align(sector, partition['alignment']) if partition['type'] != 'blank': Cgpt('add', '-i', partition['num'], '-b', sector, diff --git a/build_library/legacy_disk_layout.json b/build_library/legacy_disk_layout.json index fcdcf0ec0c..2beaef3297 100644 --- a/build_library/legacy_disk_layout.json +++ b/build_library/legacy_disk_layout.json @@ -1,6 +1,7 @@ { "_comment": "See http://www.chromium.org/chromium-os/building-chromium-os/disk-layout-format", "metadata":{ + "alignment": 2048, "block_size": 512, "fs_block_size": 4096 }, From f6341fc31dd38777cca1cf263bcafa6b57bdd986 Mon Sep 17 00:00:00 2001 From: Michael Marineau Date: Wed, 16 Oct 2013 19:44:49 -0700 Subject: [PATCH 3/3] fix(legacy_disk_layout.json): Omit unused partitions. We don't need to reserve space on disk just to reserve partition numbers. And now that partitions are aligned these blanks spots grew from 512 bytes to 1MB which is not much but still silly. --- build_library/legacy_disk_layout.json | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/build_library/legacy_disk_layout.json b/build_library/legacy_disk_layout.json index 2beaef3297..183de0332f 100644 --- a/build_library/legacy_disk_layout.json +++ b/build_library/legacy_disk_layout.json @@ -39,8 +39,8 @@ "num": 5, "label":"ROOT-C", "uuid":"d82521b4-07ac-4f1c-8840-ddefedc332f3", - "type":"coreos-rootfs", - "blocks":"1" + "type":"blank", + "blocks":"0" }, { "num": 6, @@ -50,15 +50,15 @@ }, { "num": 7, - "type":"coreos-reserved", + "type":"blank", "label":"coreos-reserved", - "blocks":"1" + "blocks":"0" }, { "num": 8, - "type":"coreos-reserved", + "type":"blank", "label":"coreos-reserved", - "blocks":"1" + "blocks":"0" }, { "num": 9,