Merge lp:~wesley-wiedenmeier/curtin/curtin-1543263 into lp:~curtin-dev/curtin/trunk

Proposed by Wesley Wiedenmeier
Status: Merged
Merged at revision: 351
Proposed branch: lp:~wesley-wiedenmeier/curtin/curtin-1543263
Merge into: lp:~curtin-dev/curtin/trunk
Diff against target: 95 lines (+49/-2)
3 files modified
curtin/commands/block_meta.py (+17/-1)
examples/tests/basic.yaml (+16/-0)
tests/vmtests/test_basic.py (+16/-1)
To merge this branch: bzr merge lp:~wesley-wiedenmeier/curtin/curtin-1543263
Reviewer Review Type Date Requested Status
Server Team CI bot continuous-integration Needs Fixing
Scott Moser (community) Approve
Review via email: mp+285825@code.launchpad.net

Commit message

partitioning: limited support for odd ordering of partition numbers

This fixes a bug in the partitioning code where it assumes that the
previous partition is numbered N-1. That is not always the case.
The change here is very focused on:
 a.) fixing bug 1543263 and allowing maas to send configuration
     for power that "hides" the PReP partition as number 15
     so they don't have to really show it to the user.
 b.) Being non-invasive.

Ultimately, we want to re-write the partitioning code to walk
through the storage config and correctly fill in number, start
and end for each entry. That is a more invasive and time-consuming
change though.

To post a comment you must log in.
Revision history for this message
Server Team CI bot (server-team-bot) wrote :

FAILED: Continuous integration, rev:350
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~wesley-wiedenmeier/curtin/curtin-1543263/+merge/285825/+edit-commit-message

https://server-team-jenkins.canonical.com/job/curtin-ci/159/
Executed test runs:
    None: https://server-team-jenkins.canonical.com/job/generic-update-mp/156/console

Click here to trigger a rebuild:
https://server-team-jenkins.canonical.com/job/curtin-ci/159/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Scott Moser (smoser) wrote :

nitpicks in line, but need fixing.

review: Needs Fixing
351. By Wesley Wiedenmeier

Dropped print statements and added a debug message for partition number

Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
352. By Wesley Wiedenmeier

Use better style string formatting for syslog

Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Scott Moser (smoser) wrote :

I'm running a test with your suggested vmtest change at http://paste.ubuntu.com/15021746/
I approve of this for nwo.

review: Approve
353. By Wesley Wiedenmeier

Added test case for partition numbering

Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)

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 2016-02-11 18:52:29 +0000
3+++ curtin/commands/block_meta.py 2016-02-12 04:14:05 +0000
4@@ -503,6 +503,19 @@
5 return logicaldisks
6
7
8+def find_previous_partition(disk_id, part_id, storage_config):
9+ last_partnum = 0
10+ for item_id, command in storage_config.items():
11+ if item_id == part_id:
12+ return last_partnum
13+ if command['type'] == 'partition' and command['device'] == disk_id:
14+ if 'number' in item_id:
15+ last_partnum = item_id['number']
16+ else:
17+ last_partnum += 1
18+ return None
19+
20+
21 def partition_handler(info, storage_config):
22 device = info.get('device')
23 size = info.get('size')
24@@ -540,8 +553,11 @@
25 previous_partition = "/sys/block/%s/%s%s/" % \
26 (disk_kname, disk_kname, extended_part_no)
27 else:
28+ pnum = find_previous_partition(device, info['id'], storage_config)
29+ LOG.debug("previous partition number for '%s' found to be '%s'",
30+ info.get('id'), pnum)
31 previous_partition = "/sys/block/%s/%s%s/" % \
32- (disk_kname, disk_kname, partnumber - 1)
33+ (disk_kname, disk_kname, pnum)
34 with open(os.path.join(previous_partition, "size"), "r") as fp:
35 previous_size = int(fp.read())
36 with open(os.path.join(previous_partition, "start"), "r") as fp:
37
38=== modified file 'examples/tests/basic.yaml'
39--- examples/tests/basic.yaml 2015-12-07 15:14:55 +0000
40+++ examples/tests/basic.yaml 2016-02-12 04:14:05 +0000
41@@ -54,3 +54,19 @@
42 type: mount
43 path: /btrfs
44 device: btrfs_disk_fmt_id
45+ - id: pnum_disk
46+ type: disk
47+ path: /dev/vde
48+ name: sparedisk
49+ wipe: superblock
50+ ptable: gpt
51+ - id: pnum_disk_p1
52+ type: partition
53+ number: 1
54+ size: 1GB
55+ device: pnum_disk
56+ - id: pnum_disk_p2
57+ type: partition
58+ number: 10
59+ size: 1GB
60+ device: pnum_disk
61
62=== modified file 'tests/vmtests/test_basic.py'
63--- tests/vmtests/test_basic.py 2015-12-17 22:07:00 +0000
64+++ tests/vmtests/test_basic.py 2016-02-12 04:14:05 +0000
65@@ -13,7 +13,7 @@
66 conf_file = "examples/tests/basic.yaml"
67 install_timeout = 600
68 boot_timeout = 120
69- extra_disks = ['128G', '128G']
70+ extra_disks = ['128G', '128G', '4G']
71 disk_to_check = {'main_disk': 1, 'main_disk': 2}
72 collect_scripts = [textwrap.dedent("""
73 cd OUTPUT_COLLECT_D
74@@ -43,6 +43,21 @@
75 blkid_info = self.get_blkid_data("blkid_output_vda")
76 self.assertEquals(blkid_info["PTTYPE"], "dos")
77
78+ def test_partition_numbers(self):
79+ # vde should have partitions 1 and 10
80+ disk = "vde"
81+ proc_partitions_path = os.path.join(self.td.collect,
82+ 'proc_partitions')
83+ self.assertTrue(os.path.exists(proc_partitions_path))
84+ found = []
85+ with open(proc_partitions_path, 'r') as fp:
86+ for line in fp.readlines():
87+ if disk in line:
88+ found.append(line.split()[3])
89+ # /proc/partitions should have 3 lines with 'vde' in them.
90+ expected = [disk + s for s in ["", "1", "10"]]
91+ self.assertEqual(found, expected)
92+
93 def test_partitions(self):
94 with open(os.path.join(self.td.collect, "fstab")) as fp:
95 fstab_lines = fp.readlines()

Subscribers

People subscribed via source and target branches