From patchwork Mon Dec 26 12:07:15 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Georgy Redkozubov X-Patchwork-Id: 5974 Return-Path: X-Original-To: patchwork@peony.canonical.com Delivered-To: patchwork@peony.canonical.com Received: from fiordland.canonical.com (fiordland.canonical.com [91.189.94.145]) by peony.canonical.com (Postfix) with ESMTP id 5B13623E14 for ; Mon, 26 Dec 2011 12:07:18 +0000 (UTC) Received: from mail-ey0-f180.google.com (mail-ey0-f180.google.com [209.85.215.180]) by fiordland.canonical.com (Postfix) with ESMTP id 47248A1835C for ; Mon, 26 Dec 2011 12:07:18 +0000 (UTC) Received: by eaac11 with SMTP id c11so8981957eaa.11 for ; Mon, 26 Dec 2011 04:07:18 -0800 (PST) Received: by 10.205.132.148 with SMTP id hu20mr3826971bkc.96.1324901237941; Mon, 26 Dec 2011 04:07:17 -0800 (PST) X-Forwarded-To: linaro-patchwork@canonical.com X-Forwarded-For: patch@linaro.org linaro-patchwork@canonical.com Delivered-To: patches@linaro.org Received: by 10.205.82.144 with SMTP id ac16cs146752bkc; Mon, 26 Dec 2011 04:07:17 -0800 (PST) Received: by 10.216.135.75 with SMTP id t53mr13507481wei.2.1324901236285; Mon, 26 Dec 2011 04:07:16 -0800 (PST) Received: from indium.canonical.com (indium.canonical.com. [91.189.90.7]) by mx.google.com with ESMTPS id e20si10345279wed.61.2011.12.26.04.07.16 (version=TLSv1/SSLv3 cipher=OTHER); Mon, 26 Dec 2011 04:07:16 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of bounces@canonical.com designates 91.189.90.7 as permitted sender) client-ip=91.189.90.7; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of bounces@canonical.com designates 91.189.90.7 as permitted sender) smtp.mail=bounces@canonical.com Received: from ackee.canonical.com ([91.189.89.26]) by indium.canonical.com with esmtp (Exim 4.71 #1 (Debian)) id 1Rf9Kl-0001xg-Vk for ; Mon, 26 Dec 2011 12:07:15 +0000 Received: from ackee.canonical.com (localhost [127.0.0.1]) by ackee.canonical.com (Postfix) with ESMTP id E41AFE0023 for ; Mon, 26 Dec 2011 12:07:15 +0000 (UTC) MIME-Version: 1.0 X-Launchpad-Project: linaro-image-tools X-Launchpad-Branch: ~linaro-image-tools/linaro-image-tools/trunk X-Launchpad-Message-Rationale: Subscriber X-Launchpad-Branch-Revision-Number: 481 X-Launchpad-Notification-Type: branch-revision To: Linaro Patch Tracker From: noreply@launchpad.net Subject: [Branch ~linaro-image-tools/linaro-image-tools/trunk] Rev 481: [r=mabac] Run sfdisk -l in loop to wait for partition to settle after parted in function create_p... Message-Id: <20111226120715.10232.84046.launchpad@ackee.canonical.com> Date: Mon, 26 Dec 2011 12:07:15 -0000 Reply-To: noreply@launchpad.net Sender: bounces@canonical.com Errors-To: bounces@canonical.com Precedence: bulk X-Generated-By: Launchpad (canonical.com); Revision="14487"; Instance="launchpad-lazr.conf" X-Launchpad-Hash: a03ac3681a996b53b370ebbfbea8f3515f8cd9d6 Merge authors: Georgy Redkozubov (georgy-redkozubov) Related merge proposals: https://code.launchpad.net/~georgy-redkozubov/linaro-image-tools/718665/+merge/85479 proposed by: Georgy Redkozubov (georgy-redkozubov) review: Needs Fixing - Mattias Backman (mabac) ------------------------------------------------------------ revno: 481 [merge] committer: Georgy Redkozubov branch nick: lit-trunk timestamp: Mon 2011-12-26 16:05:41 +0400 message: [r=mabac] Run sfdisk -l in loop to wait for partition to settle after parted in function create_partitions. modified: linaro_image_tools/media_create/partitions.py linaro_image_tools/media_create/tests/test_media_create.py --- lp:linaro-image-tools https://code.launchpad.net/~linaro-image-tools/linaro-image-tools/trunk You are subscribed to branch lp:linaro-image-tools. To unsubscribe from this branch go to https://code.launchpad.net/~linaro-image-tools/linaro-image-tools/trunk/+edit-subscription === modified file 'linaro_image_tools/media_create/partitions.py' --- linaro_image_tools/media_create/partitions.py 2011-12-22 08:30:57 +0000 +++ linaro_image_tools/media_create/partitions.py 2011-12-26 12:02:11 +0000 @@ -38,7 +38,7 @@ HEADS = 128 SECTORS = 32 -SECTOR_SIZE = 512 # bytes +SECTOR_SIZE = 512 # bytes CYLINDER_SIZE = HEADS * SECTORS * SECTOR_SIZE DBUS_PROPERTIES = 'org.freedesktop.DBus.Properties' UDISKS = "org.freedesktop.UDisks" @@ -66,7 +66,7 @@ if media.is_block_device: bootfs, system, cache, data, sdcard = \ - get_android_partitions_for_media (media, board_config) + get_android_partitions_for_media(media, board_config) ensure_partition_is_not_mounted(bootfs) ensure_partition_is_not_mounted(system) ensure_partition_is_not_mounted(cache) @@ -380,7 +380,7 @@ media.path, 3 + board_config.mmc_part_offset) else: # In the current setup, partition 4 is always the - # extended partition container, so we need to skip 4 + # extended partition container, so we need to skip 4 cache_partition = _get_device_file_for_partition_number( media.path, 5) data_partition = _get_device_file_for_partition_number( @@ -547,6 +547,8 @@ ['parted', '-s', media.path, 'mklabel', 'msdos'], as_root=True) proc.wait() + wait_partition_to_settle(media) + sfdisk_cmd = board_config.get_sfdisk_cmd( should_align_boot_part=should_align_boot_part) @@ -554,11 +556,33 @@ # Sync and sleep to wait for the partition to settle. cmd_runner.run(['sync']).wait() - # Sleeping just 1 second seems to be enough here, but if we start getting - # errors because the disk is not partitioned then we should revisit this. - # XXX: This sleep can probably die now; need to do more tests before doing - # so, though. - time.sleep(1) + wait_partition_to_settle(media) + + +def wait_partition_to_settle(media): + """Sleep in a loop to wait partition to settle + + :param media: A setup_partitions.Media object to partition. + """ + logger = logging.getLogger("linaro_image_tools") + tts = 1 + while (tts > 0) and (tts <= MAX_TTS): + try: + logger.info("Sleeping for %s second(s) to wait " + "for the partition to settle" % tts) + time.sleep(tts) + proc = cmd_runner.run( + ['sfdisk', '-l', media.path], + as_root=True, stdout=open('/dev/null', 'w')) + proc.wait() + return 0 + except cmd_runner.SubcommandNonZeroReturnValue: + logger.info("Partition table is not available " + "for device %s" % media.path) + tts += 1 + logger.error("Couldn't read partition table " + "for a reasonable time for device %s" % media.path) + raise class Media(object): === modified file 'linaro_image_tools/media_create/tests/test_media_create.py' --- linaro_image_tools/media_create/tests/test_media_create.py 2011-12-22 14:36:11 +0000 +++ linaro_image_tools/media_create/tests/test_media_create.py 2011-12-23 10:35:27 +0000 @@ -93,6 +93,7 @@ setup_partitions, get_uuid, _parse_blkid_output, + wait_partition_to_settle, _get_device_file_for_partition_number, ) from linaro_image_tools.media_create.rootfs import ( @@ -258,7 +259,7 @@ with hp: test_file = hp.get_file(metadata_file) self.assertEquals(data, open(test_file, 'r').read()) - + class TestSetMetadata(TestCaseWithFixtures): @@ -1186,6 +1187,7 @@ '1318912,-,E\n1318912,524288,L\n1843200,1048576,L\n2891776,,,-', android_boards.AndroidMx53LoCoConfig.get_sfdisk_cmd()) + class TestGetSfdiskCmdV2(TestCase): def test_mx5(self): @@ -1328,7 +1330,6 @@ 'bootm 0x00100000 0x08000000'} self.assertEqual(expected, boot_commands) - def test_panda(self): # XXX: To fix bug 697824 we have to change class attributes of our # OMAP board configs, and some tests do that so to make sure they @@ -1826,7 +1827,9 @@ self.assertEqual( ['%s parted -s %s mklabel msdos' % (sudo_args, self.media.path), - 'sync'], + '%s sfdisk -l %s' % (sudo_args, self.media.path), + 'sync', + '%s sfdisk -l %s' % (sudo_args, self.media.path)], popen_fixture.mock.commands_executed) # Notice that we create all partitions in a single sfdisk run because # every time we run sfdisk it actually repartitions the device, @@ -1846,7 +1849,9 @@ self.assertEqual( ['%s parted -s %s mklabel msdos' % (sudo_args, self.media.path), - 'sync'], + '%s sfdisk -l %s' % (sudo_args, self.media.path), + 'sync', + '%s sfdisk -l %s' % (sudo_args, self.media.path)], popen_fixture.mock.commands_executed) # Notice that we create all partitions in a single sfdisk run because # every time we run sfdisk it actually repartitions the device, @@ -1865,7 +1870,9 @@ self.assertEqual( ['%s parted -s %s mklabel msdos' % (sudo_args, self.media.path), - 'sync'], + '%s sfdisk -l %s' % (sudo_args, self.media.path), + 'sync', + '%s sfdisk -l %s' % (sudo_args, self.media.path)], popen_fixture.mock.commands_executed) # Notice that we create all partitions in a single sfdisk run because # every time we run sfdisk it actually repartitions the device, @@ -1883,7 +1890,9 @@ self.assertEqual( ['%s parted -s %s mklabel msdos' % (sudo_args, self.media.path), - 'sync'], + '%s sfdisk -l %s' % (sudo_args, self.media.path), + 'sync', + '%s sfdisk -l %s' % (sudo_args, self.media.path)], popen_fixture.mock.commands_executed) self.assertEqual( [('63,106432,0x0C,*\n106496,,,-', HEADS, SECTORS, '', @@ -1901,7 +1910,11 @@ # Unlike the test for partitioning of a regular block device, in this # case parted was not called as there's no existing partition table # for us to overwrite on the image file. - self.assertEqual(['sync'], popen_fixture.mock.commands_executed) + self.assertEqual( + ['%s sfdisk -l %s' % (sudo_args, tmpfile), + 'sync', + '%s sfdisk -l %s' % (sudo_args, tmpfile)], + popen_fixture.mock.commands_executed) self.assertEqual( [('63,106432,0x0C,*\n106496,,,-', HEADS, SECTORS, '', tmpfile)], @@ -1926,6 +1939,54 @@ ',1,0xDA', HEADS, SECTORS, '', tmpfile, as_root=False, stderr=subprocess.PIPE) + def test_wait_partitions_to_settle(self): + class Namespace: + pass + + ns = Namespace() + ns.count = 0 + + class MockCmdRunnerPopen(object): + def __call__(self, cmd, *args, **kwargs): + ns.count += 1 + self.returncode = 0 + if ns.count < 5: + raise cmd_runner.SubcommandNonZeroReturnValue(args, 1) + else: + return self + + def communicate(self, input=None): + self.wait() + return '', '' + + def wait(self): + return self.returncode + + fixture = self.useFixture(MockCmdRunnerPopenFixture()) + + tmpfile = self.createTempFileAsFixture() + media = Media(tmpfile) + media.is_block_device = True + + self.assertEqual(0, wait_partition_to_settle(media)) + + def test_wait_partitions_to_settle_raises_SubcommandNonZeroReturnValue(self): + def mock_run(args, as_root=False, chroot=None, stdin=None, stdout=None, + stderr=None, cwd=None): + raise cmd_runner.SubcommandNonZeroReturnValue(args, 1) + + self.useFixture(MockSomethingFixture( + cmd_runner, 'run', + mock_run)) + + tmpfile = self.createTempFileAsFixture() + media = Media(tmpfile) + media.is_block_device = True + + self.assertRaises(cmd_runner.SubcommandNonZeroReturnValue, + wait_partition_to_settle, + media) + class TestPartitionSetup(TestCaseWithFixtures): @@ -1937,7 +1998,7 @@ self.linux_image_size = 30 * 1024**2 self.linux_offsets_and_sizes = [ (16384 * SECTOR_SIZE, 15746 * SECTOR_SIZE), - (32768 * SECTOR_SIZE, (self.linux_image_size - + (32768 * SECTOR_SIZE, (self.linux_image_size - 32768 * SECTOR_SIZE)) ] self.android_image_size = 256 * 1024**2 @@ -1949,17 +2010,17 @@ (98367 * SECTOR_SIZE, 65536 * SECTOR_SIZE), ((294975 + ext_part_size) * SECTOR_SIZE, (131072 - ext_part_size) * SECTOR_SIZE), - ((426047 + ext_part_size) * SECTOR_SIZE, + ((426047 + ext_part_size) * SECTOR_SIZE, self.android_image_size - (426047 + ext_part_size) * SECTOR_SIZE) ] - + self.android_snowball_offsets_and_sizes = [ (8192 * SECTOR_SIZE, 24639 * SECTOR_SIZE), (32831 * SECTOR_SIZE, 65536 * SECTOR_SIZE), - ((98367 + ext_part_size)* SECTOR_SIZE, + ((98367 + ext_part_size)* SECTOR_SIZE, (65536 - ext_part_size) * SECTOR_SIZE), (294975 * SECTOR_SIZE, 131072 * SECTOR_SIZE), - ((426047 + ext_part_size) * SECTOR_SIZE, + ((426047 + ext_part_size) * SECTOR_SIZE, self.android_image_size - (426047 + ext_part_size) * SECTOR_SIZE) ] @@ -2098,7 +2159,7 @@ get_boot_and_root_loopback_devices(tmpfile) self.assertEqual( ['%s losetup -f --show %s --offset %s --sizelimit %s' - % (sudo_args, tmpfile, offset, size) for (offset, size) in + % (sudo_args, tmpfile, offset, size) for (offset, size) in self.linux_offsets_and_sizes], popen_fixture.mock.commands_executed) @@ -2126,7 +2187,7 @@ get_android_loopback_devices(tmpfile) self.assertEqual( ['%s losetup -f --show %s --offset %s --sizelimit %s' - % (sudo_args, tmpfile, offset, size) for (offset, size) in + % (sudo_args, tmpfile, offset, size) for (offset, size) in self.android_offsets_and_sizes], popen_fixture.mock.commands_executed) @@ -2173,11 +2234,13 @@ self.assertEqual( # This is the call that would create a 2 GiB image file. ['dd of=%s bs=1 seek=2147483648 count=0' % tmpfile, + '%s sfdisk -l %s' % (sudo_args, tmpfile), # This call would partition the image file. '%s sfdisk --force -D -uS -H %s -S %s -C 1024 %s' % ( sudo_args, HEADS, SECTORS, tmpfile), # Make sure changes are written to disk. 'sync', + '%s sfdisk -l %s' % (sudo_args, tmpfile), '%s mkfs.vfat -F 32 %s -n boot' % (sudo_args, bootfs_dev), '%s mkfs.ext3 %s -L root' % (sudo_args, rootfs_dev)], popen_fixture.mock.commands_executed) @@ -2201,9 +2264,11 @@ True, True, True) self.assertEqual( ['%s parted -s %s mklabel msdos' % (sudo_args, tmpfile), + '%s sfdisk -l %s' % (sudo_args, tmpfile), '%s sfdisk --force -D -uS -H %s -S %s %s' % ( sudo_args, HEADS, SECTORS, tmpfile), 'sync', + '%s sfdisk -l %s' % (sudo_args, tmpfile), # Since the partitions are mounted, setup_partitions will umount # them before running mkfs. '%s umount %s' % (sudo_args, bootfs_dev), @@ -2213,15 +2278,15 @@ popen_fixture.mock.commands_executed) def test_get_device_file_for_partition_number_raises_DBusException(self): - def mock_get_udisks_device_path(d): - raise dbus.exceptions.DBusException + def mock_get_udisks_device_path(d): + raise dbus.exceptions.DBusException self.useFixture(MockSomethingFixture( partitions, '_get_udisks_device_path', mock_get_udisks_device_path)) tmpfile = self.createTempFileAsFixture() - partition = board_configs['beagle'].mmc_part_offset + partition = board_configs['beagle'].mmc_part_offset self.useFixture(MockSomethingFixture( glob, 'glob', @@ -2233,26 +2298,27 @@ media = Media(tmpfile) media.is_block_device = True self.assertRaises(dbus.exceptions.DBusException, - _get_device_file_for_partition_number, + _get_device_file_for_partition_number, media.path, partition) def test_get_device_file_for_partition_number(self): - class Namespace: pass - ns = Namespace() - ns.count = 0 - - def mock_get_udisks_device_path(dev): - ns.count += 1 - if ns.count < 5: - raise dbus.exceptions.DBusException - else: - return '/abc/123' - - def mock_get_udisks_device_file(dev, part): - if ns.count < 5: - raise dbus.exceptions.DBusException - else: - return '/abc/123' + class Namespace: + pass + ns = Namespace() + ns.count = 0 + + def mock_get_udisks_device_path(dev): + ns.count += 1 + if ns.count < 5: + raise dbus.exceptions.DBusException + else: + return '/abc/123' + + def mock_get_udisks_device_file(dev, part): + if ns.count < 5: + raise dbus.exceptions.DBusException + else: + return '/abc/123' self.useFixture(MockSomethingFixture( partitions, '_get_udisks_device_path', @@ -2263,7 +2329,7 @@ mock_get_udisks_device_file)) tmpfile = self.createTempFileAsFixture() - partition = board_configs['beagle'].mmc_part_offset + partition = board_configs['beagle'].mmc_part_offset self.useFixture(MockSomethingFixture( glob, 'glob',