Merge lp:~salgado/linaro-image-tools/bug-701678 into lp:linaro-image-tools/11.11

Proposed by Guilherme Salgado
Status: Merged
Merged at revision: 264
Proposed branch: lp:~salgado/linaro-image-tools/bug-701678
Merge into: lp:linaro-image-tools/11.11
Diff against target: 72 lines (+16/-9)
2 files modified
linaro_media_create/partitions.py (+6/-6)
linaro_media_create/tests/test_media_create.py (+10/-3)
To merge this branch: bzr merge lp:~salgado/linaro-image-tools/bug-701678
Reviewer Review Type Date Requested Status
Loïc Minier (community) Approve
James Westby (community) Approve
Review via email: mp+47688@code.launchpad.net

Description of the change

Only ensure a partition is not mounted before running mkfs when we're burning an SD card.

That check is not necessary when we're writing an image file because there's nothing that would automount a loopback device that has just been setup.

Oh, and this will also get rid of the last remaining place using UDisks when creating an image file, hence fixing bug 705571.

To post a comment you must log in.
265. By Guilherme Salgado

Add a test to make sure ensure_partition_is_not_mounted is not called when generating image files

Revision history for this message
James Westby (james-w) wrote :

I like this one too. The error message will be very useful if someone
ever trips this, thanks.

James

review: Approve
Revision history for this message
Loïc Minier (lool) wrote :

(Sorry for reviewing this earlier; I got confused by the diff; now that I see it in context I understand the change)

This seems all fine; great to kill two bugs with this

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'linaro_media_create/partitions.py'
--- linaro_media_create/partitions.py 2011-01-26 18:03:50 +0000
+++ linaro_media_create/partitions.py 2011-01-27 17:22:24 +0000
@@ -54,15 +54,16 @@
54 if media.is_block_device:54 if media.is_block_device:
55 bootfs, rootfs = get_boot_and_root_partitions_for_media(55 bootfs, rootfs = get_boot_and_root_partitions_for_media(
56 media, board_config)56 media, board_config)
57 else:
58 bootfs, rootfs = get_boot_and_root_loopback_devices(media.path)
59
60 if should_format_bootfs:
61 print "\nFormating boot partition\n"
62 # It looks like KDE somehow automounts the partitions after you57 # It looks like KDE somehow automounts the partitions after you
63 # repartition a disk so we need to unmount them here to create the58 # repartition a disk so we need to unmount them here to create the
64 # filesystem.59 # filesystem.
65 ensure_partition_is_not_mounted(bootfs)60 ensure_partition_is_not_mounted(bootfs)
61 ensure_partition_is_not_mounted(rootfs)
62 else:
63 bootfs, rootfs = get_boot_and_root_loopback_devices(media.path)
64
65 if should_format_bootfs:
66 print "\nFormating boot partition\n"
66 proc = cmd_runner.run(67 proc = cmd_runner.run(
67 ['mkfs.vfat', '-F', str(board_config.fat_size), bootfs, '-n',68 ['mkfs.vfat', '-F', str(board_config.fat_size), bootfs, '-n',
68 bootfs_label],69 bootfs_label],
@@ -72,7 +73,6 @@
72 if should_format_rootfs:73 if should_format_rootfs:
73 print "\nFormating root partition\n"74 print "\nFormating root partition\n"
74 mkfs = 'mkfs.%s' % rootfs_type75 mkfs = 'mkfs.%s' % rootfs_type
75 ensure_partition_is_not_mounted(rootfs)
76 proc = cmd_runner.run(76 proc = cmd_runner.run(
77 [mkfs, rootfs, '-L', rootfs_label],77 [mkfs, rootfs, '-L', rootfs_label],
78 as_root=True)78 as_root=True)
7979
=== modified file 'linaro_media_create/tests/test_media_create.py'
--- linaro_media_create/tests/test_media_create.py 2011-01-26 18:03:50 +0000
+++ linaro_media_create/tests/test_media_create.py 2011-01-27 17:22:24 +0000
@@ -161,7 +161,7 @@
161 self.mock_all_boards_funcs()161 self.mock_all_boards_funcs()
162162
163 def mock_all_boards_funcs(self):163 def mock_all_boards_funcs(self):
164 """Mock all functions of linaro_media_create.boards with a call tracer."""164 """Mock functions of linaro_media_create.boards with a call tracer."""
165165
166 def mock_func_creator(name):166 def mock_func_creator(name):
167 return lambda *args, **kwargs: self.funcs_calls.append(name)167 return lambda *args, **kwargs: self.funcs_calls.append(name)
@@ -680,8 +680,15 @@
680 popen_fixture = self.useFixture(MockCmdRunnerPopenFixture())680 popen_fixture = self.useFixture(MockCmdRunnerPopenFixture())
681 self.useFixture(MockSomethingFixture(681 self.useFixture(MockSomethingFixture(
682 sys, 'stdout', open('/dev/null', 'w')))682 sys, 'stdout', open('/dev/null', 'w')))
683 def ensure_partition_not_mounted(part):
684 raise AssertionError(
685 "ensure_partition_is_not_mounted must not be called when "
686 "generating image files. It makes no sense to do that and "
687 "it depends on UDisks, thus making it hard to run on a "
688 "chroot")
683 self.useFixture(MockSomethingFixture(689 self.useFixture(MockSomethingFixture(
684 partitions, 'is_partition_mounted', lambda part: False))690 partitions,
691 'ensure_partition_is_not_mounted', ensure_partition_not_mounted))
685 self.useFixture(MockSomethingFixture(692 self.useFixture(MockSomethingFixture(
686 partitions, 'get_boot_and_root_loopback_devices',693 partitions, 'get_boot_and_root_loopback_devices',
687 lambda image: ('/dev/loop99', '/dev/loop98')))694 lambda image: ('/dev/loop99', '/dev/loop98')))
@@ -724,8 +731,8 @@
724 # Since the partitions are mounted, setup_partitions will umount731 # Since the partitions are mounted, setup_partitions will umount
725 # them before running mkfs.732 # them before running mkfs.
726 ['sudo', 'umount', bootfs_dev],733 ['sudo', 'umount', bootfs_dev],
727 ['sudo', 'mkfs.vfat', '-F', '32', bootfs_dev, '-n', 'boot'],
728 ['sudo', 'umount', rootfs_dev],734 ['sudo', 'umount', rootfs_dev],
735 ['sudo', 'mkfs.vfat', '-F', '32', bootfs_dev, '-n', 'boot'],
729 ['sudo', 'mkfs.ext3', rootfs_dev, '-L', 'root']],736 ['sudo', 'mkfs.ext3', rootfs_dev, '-L', 'root']],
730 popen_fixture.mock.calls)737 popen_fixture.mock.calls)
731738

Subscribers

People subscribed via source and target branches