diff mbox

[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
State Accepted
Headers show

Commit Message

Georgy Redkozubov Dec. 26, 2011, 12:07 p.m. UTC
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 <georgy.redkozubov@linaro.org>
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
diff mbox

Patch

=== 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',