Merge lp:~paelzer/curtin/fix-bug-1513085-partition-alignment-v3 into lp:~curtin-dev/curtin/trunk

Proposed by Christian Ehrhardt 
Status: Merged
Merged at revision: 295
Proposed branch: lp:~paelzer/curtin/fix-bug-1513085-partition-alignment-v3
Merge into: lp:~curtin-dev/curtin/trunk
Diff against target: 99 lines (+52/-8)
2 files modified
curtin/commands/block_meta.py (+51/-7)
examples/tests/lvm.yaml (+1/-1)
To merge this branch: bzr merge lp:~paelzer/curtin/fix-bug-1513085-partition-alignment-v3
Reviewer Review Type Date Requested Status
curtin developers Pending
Review via email: mp+277117@code.launchpad.net

Commit message

Partition alignment and sizing fixes:
- end sector calculation so that partition size matches what was requested
- proper size of extended partition size calculation to be able to hold as much
  as it was specified in the yaml
- proper logical partition start calculation and realignment
- prepare curtin for the appearance of 4K logical sector size disk.
- by that implicitly fixing our alignment issues as long as sizes are aligned
  which they usually are as e.g. MAAS exposes only 1MB units
- extends one of the existing tests to cover 2G+3G logical in 5G extended

Description of the change

First partitions start at 2048 by default (which is parted default as well)
As I pointed out we have to understand that when currently creating a 1024
byte partition (small size makes no sense but good for the example) we do
2048 - 2048 + (1024/512)
2048 - 2050
That means we span 2048, 2049 2050 => that is 1536 byte

Also logical/extended partitions did wrong sector calculations by adding the
size of the extended (2 sectors) to the start - that only accidentially worked
most of the time.
Extended and Logical can't share the start sector and subsequent logical
partitions can't go head-to-head with former logical partitions.
While all that goes on a human would assume all that just works.
Also one will assume that e.g. a 5G extended partition can hold a 3G and a 2G
logical which so far it couldn't for all these reasons.

All that also broke alignment.

This patch now fixes:
- end sector calculation so that partition size matches what was requested
- proper size of extended partition size calculation to be able to hold as much
  as it was specified in the yaml
- proper logical partition start calculation and realignment
- prepare curtin for the appearance of 4K logical sector size disk.
- by that implicitly fixing our alignment issues as long as sizes are aligned
  which they usually are as e.g. MAAS exposes only 1MB units
- extends one of the existing tests to cover 2G+3G logical in 5G extended

To post a comment you must log in.
Revision history for this message
Scott Moser (smoser) wrote :

This looks good. Thank you Christian.
We really should start to write some doc to say things like "1MB gap" and such.
and also document what we leave at the end.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'curtin/commands/block_meta.py'
2--- curtin/commands/block_meta.py 2015-10-30 13:00:28 +0000
3+++ curtin/commands/block_meta.py 2015-11-10 12:47:04 +0000
4@@ -480,6 +480,14 @@
5 make_dname(info.get('id'), storage_config)
6
7
8+def getnumberoflogicaldisks(device, storage_config):
9+ logicaldisks = 0
10+ for key, item in storage_config.items():
11+ if item.get('device') == device and item.get('flag') == "logical":
12+ logicaldisks = logicaldisks + 1
13+ return logicaldisks
14+
15+
16 def partition_handler(info, storage_config):
17 device = info.get('device')
18 size = info.get('size')
19@@ -494,11 +502,18 @@
20 disk = get_path_to_storage_volume(device, storage_config)
21 partnumber = determine_partition_number(info.get('id'), storage_config)
22
23- # Offset is either 1 sector after last partition, or near the beginning if
24- # this is the first partition
25+ disk_kname = os.path.split(
26+ get_path_to_storage_volume(device, storage_config))[-1]
27+ # consider the disks logical sector size when calculating sectors
28+ try:
29+ prefix = "/sys/block/%s/queue/" % disk_kname
30+ with open(prefix + "logical_block_size", "r") as f:
31+ l = f.readline()
32+ logical_block_size_bytes = int(l)
33+ except:
34+ logical_block_size_bytes = 512
35+
36 if partnumber > 1:
37- disk_kname = os.path.split(
38- get_path_to_storage_volume(device, storage_config))[-1]
39 if partnumber == 5 and disk_ptable == "msdos":
40 for key, item in storage_config.items():
41 if item.get('type') == "partition" and \
42@@ -515,12 +530,41 @@
43 with open(os.path.join(previous_partition, "size"), "r") as fp:
44 previous_size = int(fp.read())
45 with open(os.path.join(previous_partition, "start"), "r") as fp:
46- offset_sectors = previous_size + int(fp.read()) + 1
47+ previous_start = int(fp.read())
48+
49+ # Align to 1M at the beginning of the disk and at logical partitions
50+ alignment_offset = (1 << 20) / logical_block_size_bytes
51+ if partnumber == 1:
52+ # start of disk
53+ offset_sectors = alignment_offset
54 else:
55- offset_sectors = 2048
56+ # further partitions
57+ if disk_ptable == "gpt" or flag != "logical":
58+ # msdos primary and any gpt part start after former partition end
59+ offset_sectors = previous_start + previous_size
60+ else:
61+ # msdos extended/logical partitions
62+ if flag == "logical":
63+ if partnumber == 5:
64+ # First logical partition
65+ # start at extended partition start + alignment_offset
66+ offset_sectors = previous_start + alignment_offset
67+ else:
68+ # Further logical partitions
69+ # start at former logical partition end + alignment_offset
70+ offset_sectors = (previous_start + previous_size
71+ + alignment_offset)
72
73 length_bytes = util.human2bytes(size)
74- length_sectors = int(length_bytes / 512)
75+ # start sector is part of the sectors that define the partitions size
76+ # so length has to be "size in sectors - 1"
77+ length_sectors = int(length_bytes / logical_block_size_bytes) - 1
78+ # logical partitions can't share their start sector with the extended
79+ # partition and logical partitions can't go head-to-head, so we have to
80+ # realign and for that increase size as required
81+ if info.get('flag') == "extended":
82+ logdisks = getnumberoflogicaldisks(device, storage_config)
83+ length_sectors = length_sectors + (logdisks * alignment_offset)
84
85 # Handle preserve flag
86 if info.get('preserve'):
87
88=== modified file 'examples/tests/lvm.yaml'
89--- examples/tests/lvm.yaml 2015-08-12 17:42:11 +0000
90+++ examples/tests/lvm.yaml 2015-11-10 12:47:04 +0000
91@@ -24,7 +24,7 @@
92 device: sda
93 - id: sda3
94 type: partition
95- size: 2G
96+ size: 3G
97 flag: logical
98 device: sda
99 - id: volgroup1

Subscribers

People subscribed via source and target branches