Merge lp:~lool/linaro-image-tools/samsung-v310-v3 into lp:~angus-akkea/linaro-image-tools/Samsung-SMDKV310
- samsung-v310-v3
- Merge into Samsung-SMDKV310
Proposed by
Loïc Minier
Status: | Merged | ||||
---|---|---|---|---|---|
Merged at revision: | 309 | ||||
Proposed branch: | lp:~lool/linaro-image-tools/samsung-v310-v3 | ||||
Merge into: | lp:~angus-akkea/linaro-image-tools/Samsung-SMDKV310 | ||||
Diff against target: |
420 lines (+112/-70) 2 files modified
linaro_media_create/boards.py (+46/-28) linaro_media_create/tests/test_media_create.py (+66/-42) |
||||
To merge this branch: | bzr merge lp:~lool/linaro-image-tools/samsung-v310-v3 | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Angus Ainslie | Pending | ||
Review via email: mp+52274@code.launchpad.net |
Commit message
Description of the change
This addresses some other comments from Guilherme and adds tests.
To post a comment you must log in.
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | === modified file 'linaro_media_create/boards.py' |
2 | --- linaro_media_create/boards.py 2011-03-02 19:57:43 +0000 |
3 | +++ linaro_media_create/boards.py 2011-03-05 00:53:54 +0000 |
4 | @@ -95,7 +95,7 @@ |
5 | SAMSUNG_V310_UINITRD_START = ( |
6 | SAMSUNG_V310_UIMAGE_START + SAMSUNG_V310_UIMAGE_LEN) |
7 | SAMSUNG_V310_UINITRD_RESERVED_LEN = 204800 |
8 | -SAMSUNG_V310_UINITRD_COPY_LEN = 32768 |
9 | +SAMSUNG_V310_UINITRD_COPY_LEN = 32768 |
10 | assert SAMSUNG_V310_UINITRD_START == 9281, ( |
11 | "BL2 (u-boot) expects uInitrd at +9281s") |
12 | assert SAMSUNG_V310_UINITRD_RESERVED_LEN * SECTOR_SIZE == 100 * 1024 * 1024, ( |
13 | @@ -120,6 +120,14 @@ |
14 | return start, end, length |
15 | |
16 | |
17 | +class classproperty(object): |
18 | + """A descriptor that provides @property behavior on class methods.""" |
19 | + def __init__(self, getter): |
20 | + self.getter = getter |
21 | + def __get__(self, instance, cls): |
22 | + return self.getter(cls) |
23 | + |
24 | + |
25 | class BoardConfig(object): |
26 | """The configuration used when building an image for a board.""" |
27 | # These attributes may not need to be redefined on some subclasses. |
28 | @@ -178,8 +186,8 @@ |
29 | return '%s,%s,%s,*\n%s,,,-' % ( |
30 | boot_start, boot_len, partition_type, root_start) |
31 | |
32 | - @classmethod |
33 | - def _get_bootcmd(cls): |
34 | + @classproperty |
35 | + def bootcmd(cls): |
36 | """Get the bootcmd for this board. |
37 | |
38 | In general subclasses should not have to override this. |
39 | @@ -232,7 +240,7 @@ |
40 | boot_env = {} |
41 | boot_env["bootargs"] = cls._get_bootargs( |
42 | is_live, is_lowmem, consoles, rootfs_uuid) |
43 | - boot_env["bootcmd"] = cls._get_bootcmd() |
44 | + boot_env["bootcmd"] = cls.bootcmd |
45 | return boot_env |
46 | |
47 | @classmethod |
48 | @@ -255,14 +263,6 @@ |
49 | raise NotImplementedError() |
50 | |
51 | |
52 | -class classproperty(object): |
53 | - """A descriptor that provides @property behavior on class methods.""" |
54 | - def __init__(self, getter): |
55 | - self.getter = getter |
56 | - def __get__(self, instance, cls): |
57 | - return self.getter(cls) |
58 | - |
59 | - |
60 | class OmapConfig(BoardConfig): |
61 | uboot_in_boot_part = True |
62 | |
63 | @@ -588,10 +588,7 @@ |
64 | |
65 | def _dd(input_file, output_file, block_size=SECTOR_SIZE, count=None, seek=None, |
66 | skip=None): |
67 | - """a generic dd function used to insert blobs into files or devices |
68 | - |
69 | - uses the OS dd function |
70 | - """ |
71 | + """Wrapper around the dd command""" |
72 | cmd = [ |
73 | "dd", "if=%s" % input_file, "of=%s" % output_file, |
74 | "bs=%s" % block_size, "conv=notrunc"] |
75 | @@ -675,11 +672,16 @@ |
76 | |
77 | def make_flashable_env(boot_env, env_size): |
78 | env_strings = ["%s=%s" % (k, v) for k, v in boot_env.items()] |
79 | + env_strings.sort() |
80 | env = struct.pack('B', 0).join(env_strings) |
81 | |
82 | - # pad the rest of the env for the CRC calc |
83 | - while len(env) < (env_size - 4): |
84 | - env += struct.pack('B', 0) |
85 | + # we still need to zero-terminate the last string, and 4 bytes for crc |
86 | + assert len(env) + 1 + 4 <= env_size, ( |
87 | + "environment doesn't fit in %s bytes" % env_size) |
88 | + |
89 | + # pad the rest of the env for the CRC calc; the "s" format will write zero |
90 | + # bytes to pad the (empty) string to repeat count |
91 | + env += struct.pack('%ss' % (env_size - len(env) - 4), '') |
92 | |
93 | crc = crc32(env) |
94 | env = struct.pack('<i', crc) + env |
95 | @@ -693,8 +695,14 @@ |
96 | |
97 | |
98 | def install_mx5_boot_loader(imx_file, boot_device_or_file): |
99 | - # XXX need to check that the length of imx_file is smaller than |
100 | - # LOADER_MIN_SIZE_S |
101 | + # bootloader partition starts at +1s but we write the file at +2s, so we |
102 | + # need to check that the bootloader partition minus 1s is at least as large |
103 | + # as the u-boot binary; note that the real bootloader partition might be |
104 | + # larger than LOADER_MIN_SIZE_S, but if u-boot is larger it's a sign we |
105 | + # need to bump LOADER_MIN_SIZE_S |
106 | + max_size = (LOADER_MIN_SIZE_S - 1) * SECTOR_SIZE |
107 | + assert os.path.getsize(imx_file) <= max_size, ( |
108 | + "%s is larger than guaranteed bootloader partition size" % imx_file) |
109 | _dd(imx_file, boot_device_or_file, seek=2) |
110 | |
111 | |
112 | @@ -735,23 +743,32 @@ |
113 | |
114 | |
115 | def install_smdkv310_uImage(uImage_file, boot_device_or_file): |
116 | - # XXX need to check that the length of uImage_file is smaller than |
117 | - # SAMSUNG_V310_UIMAGE_LEN |
118 | + # the layout keeps SAMSUNG_V310_UIMAGE_LEN sectors for uImage; make sure |
119 | + # uImage isn't actually larger or it would be truncated |
120 | + max_size = SAMSUNG_V310_UIMAGE_LEN * SECTOR_SIZE |
121 | + assert os.path.getsize(uImage_file) <= max_size, ( |
122 | + "%s is larger than the allocated v310 uImage length" % uImage_file) |
123 | _dd(uImage_file, boot_device_or_file, count=SAMSUNG_V310_UIMAGE_LEN, |
124 | seek=SAMSUNG_V310_UIMAGE_START) |
125 | |
126 | |
127 | def install_smdkv310_initrd(initrd_file, boot_device_or_file): |
128 | - # XXX need to check that the length of initrd_file is smaller than |
129 | - # SAMSUNG_V310_UINITRD_LEN |
130 | - _dd(initrd_file, boot_device_or_file, |
131 | + # the layout keeps SAMSUNG_V310_UINITRD_RESERVED_LEN sectors for uInitrd |
132 | + # but only SAMSUNG_V310_UINITRD_COPY_LEN sectors are loaded into memory; |
133 | + # make sure uInitrd isn't actually larger or it would be truncated in |
134 | + # memory |
135 | + max_size = SAMSUNG_V310_UINITRD_COPY_LEN * SECTOR_SIZE |
136 | + assert os.path.getsize(initrd_file) <= max_size, ( |
137 | + "%s is larger than the v310 uInitrd length as used by u-boot" |
138 | + % initrd_file) |
139 | + _dd(initrd_file, boot_device_or_file, |
140 | count=SAMSUNG_V310_UINITRD_COPY_LEN, |
141 | seek=SAMSUNG_V310_UINITRD_START) |
142 | |
143 | |
144 | def install_smdkv310_boot_env(env_file, boot_device_or_file): |
145 | - # XXX need to check that the length of env_file is smaller than |
146 | - # SAMSUNG_V310_ENV_LEN |
147 | + # the environment file is exactly SAMSUNG_V310_ENV_LEN as created by |
148 | + # make_flashable_env(), so we don't need to check the size of env_file |
149 | _dd(env_file, boot_device_or_file, count=SAMSUNG_V310_ENV_LEN, |
150 | seek=SAMSUNG_V310_ENV_START) |
151 | |
152 | @@ -773,3 +790,4 @@ |
153 | # SAMSUNG_V310_BL2_LEN |
154 | _dd(v310_file, boot_device_or_file, seek=SAMSUNG_V310_BL2_START, |
155 | skip=(SAMSUNG_V310_BL2_START - SAMSUNG_V310_BL1_START)) |
156 | + |
157 | |
158 | === modified file 'linaro_media_create/tests/test_media_create.py' |
159 | --- linaro_media_create/tests/test_media_create.py 2011-03-02 14:51:14 +0000 |
160 | +++ linaro_media_create/tests/test_media_create.py 2011-03-05 00:53:54 +0000 |
161 | @@ -43,9 +43,12 @@ |
162 | ) |
163 | import linaro_media_create |
164 | from linaro_media_create.boards import ( |
165 | + LOADER_MIN_SIZE_S, |
166 | + SECTOR_SIZE, |
167 | align_up, |
168 | align_partition, |
169 | board_configs, |
170 | + make_flashable_env, |
171 | install_mx5_boot_loader, |
172 | install_omap_boot_loader, |
173 | make_boot_script, |
174 | @@ -621,14 +624,35 @@ |
175 | '-d', 'parts_dir/initrd.img-*-sub_arch', 'boot_disk/uInitrd'] |
176 | self.assertEqual([expected], fixture.mock.calls) |
177 | |
178 | + def test_make_flashable_env_too_small_env(self): |
179 | + env = {'verylong': 'evenlonger'} |
180 | + self.assertRaises(AssertionError, make_flashable_env, env, 8) |
181 | + |
182 | + def test_make_flashable_env(self): |
183 | + env_file = self.createTempFileAsFixture() |
184 | + self.useFixture(MockSomethingFixture( |
185 | + tempfile, "mkstemp", lambda: (None, env_file))) |
186 | + env = {'a': 'b', 'x': 'y'} |
187 | + make_flashable_env(env, 12) |
188 | + with open(env_file, "r") as fd: |
189 | + self.assertEqual("\x80\x29\x2E\x89a=b\x00x=y\x00", fd.read()) |
190 | + |
191 | def test_install_mx5_boot_loader(self): |
192 | fixture = self._mock_Popen() |
193 | - install_mx5_boot_loader("imx_file", "boot_device_or_file") |
194 | + imx_file = self.createTempFileAsFixture() |
195 | + install_mx5_boot_loader(imx_file, "boot_device_or_file") |
196 | expected = [ |
197 | - 'sudo', 'dd', 'if=imx_file', 'of=boot_device_or_file', 'bs=512', |
198 | - 'conv=notrunc', 'seek=2'] |
199 | + 'sudo', 'dd', 'if=%s' % imx_file, 'of=boot_device_or_file', |
200 | + 'bs=512', 'conv=notrunc', 'seek=2'] |
201 | self.assertEqual([expected], fixture.mock.calls) |
202 | |
203 | + def test_install_mx5_boot_loader_too_large(self): |
204 | + self.useFixture(MockSomethingFixture( |
205 | + os.path, "getsize", |
206 | + lambda s: (LOADER_MIN_SIZE_S - 1) * SECTOR_SIZE + 1)) |
207 | + self.assertRaises(AssertionError, |
208 | + install_mx5_boot_loader, "imx_file", "boot_device_or_file") |
209 | + |
210 | def test_install_omap_boot_loader(self): |
211 | fixture = self._mock_Popen() |
212 | self.useFixture(MockSomethingFixture( |
213 | @@ -763,9 +787,9 @@ |
214 | popen_fixture = self.useFixture(MockCmdRunnerPopenFixture()) |
215 | sfdisk_fixture = self.useFixture(MockRunSfdiskCommandsFixture()) |
216 | |
217 | - tempfile = self.createTempFileAsFixture() |
218 | + tmpfile = self.createTempFileAsFixture() |
219 | create_partitions( |
220 | - board_configs['beagle'], Media(tempfile), 255, 63, '') |
221 | + board_configs['beagle'], Media(tmpfile), 255, 63, '') |
222 | |
223 | # Unlike the test for partitioning of a regular block device, in this |
224 | # case parted was not called as there's no existing partition table |
225 | @@ -773,26 +797,26 @@ |
226 | self.assertEqual([['sync']], popen_fixture.mock.calls) |
227 | |
228 | self.assertEqual( |
229 | - [('63,106432,0x0C,*\n106496,,,-', 255, 63, '', tempfile)], |
230 | + [('63,106432,0x0C,*\n106496,,,-', 255, 63, '', tmpfile)], |
231 | sfdisk_fixture.mock.calls) |
232 | |
233 | def test_run_sfdisk_commands(self): |
234 | - tempfile = self.createTempFileAsFixture() |
235 | + tmpfile = self.createTempFileAsFixture() |
236 | proc = cmd_runner.run( |
237 | - ['qemu-img', 'create', '-f', 'raw', tempfile, '10M'], |
238 | + ['qemu-img', 'create', '-f', 'raw', tmpfile, '10M'], |
239 | stdout=subprocess.PIPE) |
240 | proc.communicate() |
241 | stdout, stderr = run_sfdisk_commands( |
242 | - '2,16063,0xDA', 255, 63, '', tempfile, as_root=False, |
243 | + '2,16063,0xDA', 255, 63, '', tmpfile, as_root=False, |
244 | stderr=subprocess.PIPE) |
245 | self.assertIn('Successfully wrote the new partition table', stdout) |
246 | |
247 | def test_run_sfdisk_commands_raises_on_non_zero_returncode(self): |
248 | - tempfile = self.createTempFileAsFixture() |
249 | + tmpfile = self.createTempFileAsFixture() |
250 | self.assertRaises( |
251 | cmd_runner.SubcommandNonZeroReturnValue, |
252 | run_sfdisk_commands, |
253 | - ',1,0xDA', 255, 63, '', tempfile, as_root=False, |
254 | + ',1,0xDA', 255, 63, '', tmpfile, as_root=False, |
255 | stderr=subprocess.PIPE) |
256 | |
257 | |
258 | @@ -808,7 +832,7 @@ |
259 | super(TestPartitionSetup, self).tearDown() |
260 | time.sleep = self.orig_sleep |
261 | |
262 | - def _create_tempfile(self): |
263 | + def _create_tmpfile(self): |
264 | # boot part at +8 MiB, root part at +16 MiB |
265 | return self._create_qemu_img_with_partitions( |
266 | '16384,15746,0x0C,*\n32768,,,-') |
267 | @@ -823,19 +847,19 @@ |
268 | self.assertEqual(12 * 1024**3, convert_size_to_bytes('12G')) |
269 | |
270 | def test_calculate_partition_size_and_offset(self): |
271 | - tempfile = self._create_tempfile() |
272 | + tmpfile = self._create_tmpfile() |
273 | vfat_size, vfat_offset, linux_size, linux_offset = ( |
274 | - calculate_partition_size_and_offset(tempfile)) |
275 | + calculate_partition_size_and_offset(tmpfile)) |
276 | self.assertEqual( |
277 | [8061952L, 8388608L, 14680064L, 16777216L], |
278 | [vfat_size, vfat_offset, linux_size, linux_offset]) |
279 | |
280 | def test_partition_numbering(self): |
281 | # another Linux partition at +24 MiB after the boot/root parts |
282 | - tempfile = self._create_qemu_img_with_partitions( |
283 | + tmpfile = self._create_qemu_img_with_partitions( |
284 | '16384,15746,0x0C,*\n32768,15427,,-\n49152,,,-') |
285 | vfat_size, vfat_offset, linux_size, linux_offset = ( |
286 | - calculate_partition_size_and_offset(tempfile)) |
287 | + calculate_partition_size_and_offset(tmpfile)) |
288 | # check that the linux partition offset starts at +16 MiB so that it's |
289 | # the partition immediately following the vfat one |
290 | self.assertEqual(linux_offset, 32768 * 512) |
291 | @@ -843,39 +867,39 @@ |
292 | def test_get_boot_and_root_partitions_for_media_beagle(self): |
293 | self.useFixture(MockSomethingFixture( |
294 | partitions, '_get_device_file_for_partition_number', |
295 | - lambda dev, partition: '%s%d' % (tempfile, partition))) |
296 | - tempfile = self.createTempFileAsFixture() |
297 | - media = Media(tempfile) |
298 | + lambda dev, partition: '%s%d' % (tmpfile, partition))) |
299 | + tmpfile = self.createTempFileAsFixture() |
300 | + media = Media(tmpfile) |
301 | media.is_block_device = True |
302 | self.assertEqual( |
303 | - ("%s%d" % (tempfile, 1), "%s%d" % (tempfile, 2)), |
304 | + ("%s%d" % (tmpfile, 1), "%s%d" % (tmpfile, 2)), |
305 | get_boot_and_root_partitions_for_media( |
306 | media, board_configs['beagle'])) |
307 | |
308 | def test_get_boot_and_root_partitions_for_media_mx5(self): |
309 | self.useFixture(MockSomethingFixture( |
310 | partitions, '_get_device_file_for_partition_number', |
311 | - lambda dev, partition: '%s%d' % (tempfile, partition))) |
312 | - tempfile = self.createTempFileAsFixture() |
313 | - media = Media(tempfile) |
314 | + lambda dev, partition: '%s%d' % (tmpfile, partition))) |
315 | + tmpfile = self.createTempFileAsFixture() |
316 | + media = Media(tmpfile) |
317 | media.is_block_device = True |
318 | self.assertEqual( |
319 | - ("%s%d" % (tempfile, 2), "%s%d" % (tempfile, 3)), |
320 | + ("%s%d" % (tmpfile, 2), "%s%d" % (tmpfile, 3)), |
321 | get_boot_and_root_partitions_for_media(media, boards.Mx5Config)) |
322 | |
323 | def _create_qemu_img_with_partitions(self, sfdisk_commands): |
324 | - tempfile = self.createTempFileAsFixture() |
325 | + tmpfile = self.createTempFileAsFixture() |
326 | proc = cmd_runner.run( |
327 | - ['qemu-img', 'create', '-f', 'raw', tempfile, '30M'], |
328 | + ['qemu-img', 'create', '-f', 'raw', tmpfile, '30M'], |
329 | stdout=subprocess.PIPE) |
330 | proc.communicate() |
331 | stdout, stderr = run_sfdisk_commands( |
332 | - sfdisk_commands, 255, 63, '', tempfile, as_root=False, |
333 | + sfdisk_commands, 255, 63, '', tmpfile, as_root=False, |
334 | # Throw away stderr as sfdisk complains a lot when operating on a |
335 | # qemu image. |
336 | stderr=subprocess.PIPE) |
337 | self.assertIn('Successfully wrote the new partition table', stdout) |
338 | - return tempfile |
339 | + return tmpfile |
340 | |
341 | def test_ensure_partition_is_not_mounted_for_mounted_partition(self): |
342 | self.useFixture(MockSomethingFixture( |
343 | @@ -893,18 +917,18 @@ |
344 | self.assertEqual(None, popen_fixture.mock.calls) |
345 | |
346 | def test_get_boot_and_root_loopback_devices(self): |
347 | - tempfile = self._create_tempfile() |
348 | + tmpfile = self._create_tmpfile() |
349 | atexit_fixture = self.useFixture(MockSomethingFixture( |
350 | atexit, 'register', AtExitRegister())) |
351 | popen_fixture = self.useFixture(MockCmdRunnerPopenFixture()) |
352 | # We can't test the return value of get_boot_and_root_loopback_devices |
353 | # because it'd require running losetup as root, so we just make sure |
354 | # it calls losetup correctly. |
355 | - get_boot_and_root_loopback_devices(tempfile) |
356 | + get_boot_and_root_loopback_devices(tmpfile) |
357 | self.assertEqual( |
358 | - [['sudo', 'losetup', '-f', '--show', tempfile, '--offset', |
359 | + [['sudo', 'losetup', '-f', '--show', tmpfile, '--offset', |
360 | '8388608', '--sizelimit', '8061952'], |
361 | - ['sudo', 'losetup', '-f', '--show', tempfile, '--offset', |
362 | + ['sudo', 'losetup', '-f', '--show', tmpfile, '--offset', |
363 | '16777216', '--sizelimit', '14680064']], |
364 | popen_fixture.mock.calls) |
365 | |
366 | @@ -925,7 +949,7 @@ |
367 | # but here we mock Popen() and thanks to that the image is not setup |
368 | # (via qemu-img) inside setup_partitions. That's why we pass an |
369 | # already setup image file. |
370 | - tempfile = self._create_tempfile() |
371 | + tmpfile = self._create_tmpfile() |
372 | popen_fixture = self.useFixture(MockCmdRunnerPopenFixture()) |
373 | self.useFixture(MockSomethingFixture( |
374 | sys, 'stdout', open('/dev/null', 'w'))) |
375 | @@ -942,14 +966,14 @@ |
376 | partitions, 'get_boot_and_root_loopback_devices', |
377 | lambda image: ('/dev/loop99', '/dev/loop98'))) |
378 | bootfs_dev, rootfs_dev = setup_partitions( |
379 | - board_configs['beagle'], Media(tempfile), '2G', 'boot', |
380 | + board_configs['beagle'], Media(tmpfile), '2G', 'boot', |
381 | 'root', 'ext3', True, True, True) |
382 | self.assertEqual( |
383 | # This is the call that would create a 2 GiB image file. |
384 | - [['qemu-img', 'create', '-f', 'raw', tempfile, '2147483648'], |
385 | + [['qemu-img', 'create', '-f', 'raw', tmpfile, '2147483648'], |
386 | # This call would partition the image file. |
387 | ['sudo', 'sfdisk', '--force', '-D', '-uS', '-H', '255', '-S', |
388 | - '63', '-C', '261', tempfile], |
389 | + '63', '-C', '261', tmpfile], |
390 | # Make sure changes are written to disk. |
391 | ['sync'], |
392 | ['sudo', 'mkfs.vfat', '-F', '32', bootfs_dev, '-n', 'boot'], |
393 | @@ -962,21 +986,21 @@ |
394 | # Pretend the partitions are mounted. |
395 | self.useFixture(MockSomethingFixture( |
396 | partitions, 'is_partition_mounted', lambda part: True)) |
397 | - tempfile = self._create_tempfile() |
398 | + tmpfile = self._create_tmpfile() |
399 | self.useFixture(MockSomethingFixture( |
400 | partitions, '_get_device_file_for_partition_number', |
401 | - lambda dev, partition: '%s%d' % (tempfile, partition))) |
402 | - media = Media(tempfile) |
403 | - # Pretend our tempfile is a block device. |
404 | + lambda dev, partition: '%s%d' % (tmpfile, partition))) |
405 | + media = Media(tmpfile) |
406 | + # Pretend our tmpfile is a block device. |
407 | media.is_block_device = True |
408 | popen_fixture = self.useFixture(MockCmdRunnerPopenFixture()) |
409 | bootfs_dev, rootfs_dev = setup_partitions( |
410 | board_configs['beagle'], media, '2G', 'boot', 'root', 'ext3', |
411 | True, True, True) |
412 | self.assertEqual( |
413 | - [['sudo', 'parted', '-s', tempfile, 'mklabel', 'msdos'], |
414 | + [['sudo', 'parted', '-s', tmpfile, 'mklabel', 'msdos'], |
415 | ['sudo', 'sfdisk', '--force', '-D', '-uS', '-H', '255', '-S', |
416 | - '63', tempfile], |
417 | + '63', tmpfile], |
418 | ['sync'], |
419 | # Since the partitions are mounted, setup_partitions will umount |
420 | # them before running mkfs. |