Merge lp:~lool/linaro-image-tools/4-mib-align into lp:linaro-image-tools/11.11

[Loïc Minier]

This aligns partitions to 4 MiB

I've tested this successfully with:
* qemu 2G natty image, albeit no serial so couldn't check from Linux
* qemu 4G maverick image, this wouldn't boot at first, I had to replace u-boot.bin and MLO and then it booted -- not sure why, don't think this is a regression
* 1G overo image file dd-ed to a MMC, this booted and was aligned, but CHS was incorrect from Linux (didn't appear at 63/255 anymore)
* 1G to MMC for overo; this worked much like the image file case; fdisk output below
* 1G to MMC for beagle on beaglexm; fdisk output below

fdisk output on overo:
root@localhost:~# fdisk -l

Disk /dev/mmcblk0: 1967 MB, 1967128576 bytes
4 heads, 16 sectors/track, 60032 cylinders
Units = cylinders of 64 * 512 = 32768 bytes
Sector size (logical/physical): 512 bytes / 512 bytes
I/O size (minimum/optimal): 512 bytes / 512 bytes
Disk identifier: 0x0008ed19

        Device Boot Start End Blocks Id System
/dev/mmcblk0p1 * 257 2009 56068 c W95 FAT32 (LBA)
Partition 1 does not end on cylinder boundary.
/dev/mmcblk0p2 2049 60032 1855488 83 Linux
Partition 2 does not end on cylinder boundary.

fdisk output on beaglexm:

Disk /dev/mmcblk0: 1967 MB, 1967128576 bytes
4 heads, 16 sectors/track, 60032 cylinders
Units = cylinders of 64 * 512 = 32768 bytes
Sector size (logical/physical): 512 bytes / 512 bytes
I/O size (minimum/optimal): 512 bytes / 512 bytes
Disk identifier: 0x000bf37e

        Device Boot Start End Blocks Id System
/dev/mmcblk0p1 * 257 2009 56068 c W95 FAT32 (LBA)
Partition 1 does not end on cylinder boundary.
/dev/mmcblk0p2 2049 60032 1855488 83 Linux
Partition 2 does not end on cylinder boundary.

[Guilherme Salgado (salgado)]

On Thu, 2011-02-10 at 02:37 +0000, Loïc Minier wrote:
>
> This aligns partitions to 4 MiB
>
> I've tested this successfully with:
> * qemu 2G natty image, albeit no serial so couldn't check from Linux
> * qemu 4G maverick image, this wouldn't boot at first, I had to replace u-boot.bin and MLO and then it booted -- not sure why, don't think this is a regression

Can you test this using an image generated by trunk? I thought that
would work, but maybe I'm missing something?

> * 1G overo image file dd-ed to a MMC, this booted and was aligned, but CHS was incorrect from Linux (didn't appear at 63/255 anymore)

Is there any chance this wrong geometry would cause problems to the old
OMAP3 roms that rely on specific values for CHS? Any way we could test
it?

> * 1G to MMC for overo; this worked much like the image file case; fdisk output below
> * 1G to MMC for beagle on beaglexm; fdisk output below
>
> fdisk output on overo:
> root@localhost:~# fdisk -l
>
> Disk /dev/mmcblk0: 1967 MB, 1967128576 bytes
> 4 heads, 16 sectors/track, 60032 cylinders
> Units = cylinders of 64 * 512 = 32768 bytes
> Sector size (logical/physical): 512 bytes / 512 bytes
> I/O size (minimum/optimal): 512 bytes / 512 bytes
> Disk identifier: 0x0008ed19
>
> Device Boot Start End Blocks Id System
> /dev/mmcblk0p1 * 257 2009 56068 c W95 FAT32 (LBA)
> Partition 1 does not end on cylinder boundary.
> /dev/mmcblk0p2 2049 60032 1855488 83 Linux
> Partition 2 does not end on cylinder boundary.
>
> fdisk output on beaglexm:
>
> Disk /dev/mmcblk0: 1967 MB, 1967128576 bytes
> 4 heads, 16 sectors/track, 60032 cylinders
> Units = cylinders of 64 * 512 = 32768 bytes
> Sector size (logical/physical): 512 bytes / 512 bytes
> I/O size (minimum/optimal): 512 bytes / 512 bytes
> Disk identifier: 0x000bf37e
>
> Device Boot Start End Blocks Id System
> /dev/mmcblk0p1 * 257 2009 56068 c W95 FAT32 (LBA)
> Partition 1 does not end on cylinder boundary.
> /dev/mmcblk0p2 2049 60032 1855488 83 Linux
> Partition 2 does not end on cylinder boundary.
>
> differences between files attachment (review-diff.txt)
> === modified file 'linaro_media_create/boards.py'
> --- linaro_media_create/boards.py 2011-02-01 13:16:43 +0000
> +++ linaro_media_create/boards.py 2011-02-10 02:37:40 +0000
> @@ -32,6 +32,43 @@
>
> from linaro_media_create import cmd_runner
>
> +# Notes:
> +# * geometry is currently always 255 heads and 63 sectors due to limitations of
> +# older OMAP3 boot ROMs
> +# * we want partitions aligned on 4 MiB as to get the best performance and
> +# limit wear-leveling
> +# * partitions should preferably end on cylinder boundaries, at least to please
> +# sfdisk but also just to have them as big as possible
> +# * this assumes that root partition follows the boot partition which follows
> +# an optional bootloader partition
> +# * image_size is passed on the command-line and should preferably be a power
> +# of 2; it should be used as a "don't go over this size" information for a
> +# real device, and a "give me a fil...

[Loïc Minier (lool)]

On Thu, Feb 10, 2011, Guilherme Salgado wrote:
> > * qemu 4G maverick image, this wouldn't boot at first, I had to replace u-boot.bin and MLO and then it booted -- not sure why, don't think this is a regression
> Can you test this using an image generated by trunk? I thought that
> would work, but maybe I'm missing something?

 Ok; will try

> > * 1G overo image file dd-ed to a MMC, this booted and was aligned, but CHS was incorrect from Linux (didn't appear at 63/255 anymore)
>
> Is there any chance this wrong geometry would cause problems to the old
> OMAP3 roms that rely on specific values for CHS? Any way we could test
> it?

 There is; there is no good record of the actual limitation, so we're
 trying our best to do 63/255, but I don't know if it's enough as it is
 or not :-/

 I guess it might be that we need to have the boot part start at the
 beginning of a cylinder for x-loader.

 I have a C3 or C4 beagle I could test on; beagle xM worked so I'm not
 too worried -- worst case we use a different scheme for xm and for
 oldomap3?

> Neither relative nor absolute sizes can be specified for each of the
> partitions, right? Do we really care about that?

 Well, I think it would be helpful some day... but later :-)

> > - # This will create a partition of the given type, containing 9
> > - # cylinders (74027520 bytes, ~70 MiB), followed by a Linux-type
> > - # partition containing the rest of the free space.
> > - return ',9,%s,*\n,,,-' % partition_type
> > + # This will create a boot partition of type partition_type at offset
> > + # (in sectors) BOOT_PART_START_S and of size BOOT_PART_SIZE_S followed
> > + # by a root partition of the default type (Linux) at offset (in
> > + # sectors) ROOT_PART_START_S filling the remaining space
> > + # XXX we should honor the specified image size by specifying a
> > + # corresponding root partition size
>
> You mean we're no longer using the size as the upper limit for the image
> size or that the image size is not exactly what was requested? If the
> former, I think we need a bug to track it, but if it's the latter, do we
> really care about it?

 I don't think there's any regression, but I don't think we're enforcing
 the cylinder size properly; ideally, the sfdisk commands would include
 a length for the rootfs rather than "use all space".

> > - ',1,0xDA', 5, 63, '', tempfile, as_root=False,
> > + ',1,0xDA', 255, 63, '', tempfile, as_root=False,
>
> Why did you change the two tests above?

 I wanted to use 255/63 consistently

> > - sfdisk_commands, 5, 63, '', tempfile, as_root=False,
> > + sfdisk_commands, 255, 63, '', tempfile, as_root=False,
> I'd be interested in knowing what motivated this change as well.

 (same)

> > + # boot part at +8 MiB, root part at +16 MiB
> > + tempfile = self._create_qemu_img_with_partitions('16384,15746,0x0C,*\n32768,,,-')
> I've seen the above string hard-coded in way too many tests now so I
> think it'd make sense to make a constant out of it and use it here. That
> way you can even get rid of the comment above it.

 ack

--
Lo...

[Loïc Minier (lool)]

There is definitely a regression with the new branch and the maverick hwpach:
./linaro-media-create --image_size 4G --image_file beagleold.img --dev beagle --rootfs ext4 --hwpack ~/downloads/linaro/10.11/hwpack_linaro-omap3_20101109-1_armel_supported.tar.gz --binary ~/downloads/linaro/11.05/linaro-natty-headless-tar-20110203-1.tar.gz --hwpack-force-yes

with lp:linaro-image-tools:
* qemu-system-arm -M beaglexm -sd beagleold.img -serial stdio: kernel error on boot
* qemu-system-arm -M beagle -sd beagleold.img -serial stdio: boots

with this branch:
* qemu-system-arm -M beaglexm -sd beagleold.img -serial stdio: x-loader error
* qemu-system-arm -M beagle -sd beagleold.img -serial stdio: x-loader error

the x-loader error is:

Texas Instruments X-Loader 1.4.4ss (Sep 30 2010 - 14:44:32)
Beagle xM Rev A
Reading boot sector
Error: reading boot sector
u-boot.bin not found or blank nand contents - attempting serial boot . . .
## Ready for binary (kermit) download to 0x80008000 at 115200 bps...

So technically, we don't care too much about x-loader since it's on the SD card (except on IGEP, but then u-boot is not on SD card either!); in general, I think we would only care when we want to provide u-boot and not x-loader.

It does break our 10.11 images though, not sure what to do about this; what do you think?

[Guilherme Salgado (salgado)]

IIUC there's just no way we can use 4MiB aligned partitions on 10.11
images, but this whole thing is an optimization anyway, so maybe we can
keep the old alignment as default and add a new argument to l-m-c that
causes the 4MiB alignment to be used. That should be straightforward to
do, no?

[Loïc Minier (lool)]

I guess; it does seem like non-trivial work to have variable alignment though.

[Loïc Minier (lool)]

Pushed fixes which address the comments from the review; now need to rework to have variable alignment.

[lp:~lool/linaro-image-tools/4-mib-align]

290. By Loïc Minier on 2011-02-10

Add messages to asserts.

291. By Loïc Minier on 2011-02-10

Factorize the creation of a relatively common disk tempfile.

[Loïc Minier (lool)]

I've looked at our linaro-m final hwpacks:
* hwpack_linaro-bsp-omap4_20101109-1_armel_unsupported.tar.gz has pkgs/x-loader-omap4_L24.9git20100901-0ubuntu3_armel.deb
* hwpack_linaro-omap3_20101109-1_armel_supported.tar.gz has pkgs/x-loader-omap_1.4.4git20100713-1ubuntu2_armel.deb
* hwpack_linaro-omap3-x11-base_20101109-1_armel_supported.tar.gz has pkgs/x-loader-omap_1.4.4git20100713-1ubuntu2_armel.deb

vexpress, imx51 and bsp-ux500 don't need any x-loader; igep didn't have one.

Current linaro-m OMAP hwpacks use pkgs/x-loader-omap4-panda_1.4.4+git20110124+7d0e587-1ubuntu0_armel.deb, except IGEP which has no x-loader in the hwpack ATM.

[lp:~lool/linaro-image-tools/4-mib-align]

292. By Loïc Minier on 2011-02-11

Merge with lp:linaro-image-tools to pick up the partition rework changes.
- adjust test_partition_numbering() to create a third partition at +24 MiB
  and to expect the linux part at +16 MiB
- bump tempfile for sfdisk tests to 30M instead of 20M as to fit above test
- adjust linux size in tests per above change

293. By Loïc Minier on 2011-02-11

Rework sfdisk commands again; completely bypass CHS calculations as they are
irrelevant (see LP #626907 for discussion).
- enforce some sensible sizes of >= 1 MiB, >= 50 MiB, and >= 50 MiB for
  (optional) bootloader partition, boot partition and root partition
  respectively
- document partition layout in get_sfdisk_cmd()
- update testsuite for new values

294. By Loïc Minier on 2011-02-11

Add tests for align_up and align_partition.

[Loïc Minier (lool)]

So after realizing that we don't really care about CHS (!), it's indeed not as bad as I thought; I've pushed some revisions to have flexible alignment where we like.

Currently, there is a small duplication of code between mx51 and the default partition layout, but I think I prefer it like this. I think, on the long term, we should have a set of partition layout classes, one per partition layout, and they'd provide the relevant sfdisk commands. Board configs would just have a field saying which layout they want, and we would have no default layout.

The default is to create a bootloader partition at +63s; I think we want to make this omap specific, but that would make code duplication a bit worse (an additional copy for vexpress); I think that's ok, but not urgent anyway.

Finally, there's currently no flag to force 4 MiB alignment of the boot partition on OMAP, but the i.MX5 partitions will always be aligned. I could implement that now, and we would have complete 4 MiB support, but I wonder whether we should just declare that hardware pack v2 implies that your x-loader is good enough and that we can do 4 MiB alignment all the time. Depends how long it takes to get hwpack v2.

[lp:~lool/linaro-image-tools/4-mib-align]

295. By Loïc Minier on 2011-02-11

Add --align-boot-part flag to force (4 MiB) alignment of boot partition.

[Dave Martin (dave-martin-arm)]

> So after realizing that we don't really care about CHS (!), it's indeed not as
> bad as I thought; I've pushed some revisions to have flexible alignment where
> we like.
>
> Currently, there is a small duplication of code between mx51 and the default
> partition layout, but I think I prefer it like this. I think, on the long
> term, we should have a set of partition layout classes, one per partition
> layout, and they'd provide the relevant sfdisk commands. Board configs would
> just have a field saying which layout they want, and we would have no default
> layout.
>
> The default is to create a bootloader partition at +63s; I think we want to
> make this omap specific, but that would make code duplication a bit worse (an
> additional copy for vexpress); I think that's ok, but not urgent anyway.
>
> Finally, there's currently no flag to force 4 MiB alignment of the boot
> partition on OMAP, but the i.MX5 partitions will always be aligned. I could
> implement that now, and we would have complete 4 MiB support, but I wonder
> whether we should just declare that hardware pack v2 implies that your
> x-loader is good enough and that we can do 4 MiB alignment all the time.
> Depends how long it takes to get hwpack v2.

Doesn't X-loader come out of the FAT filesystem itself? I assume there must be some first-stage boot in ROM or flash which may also mave the restriction on the location of the boot partition.

This appears to be the case on my Beagle xM, because if the boot partition is misaligned I just get a cryptic error string like "60" printed to the UART-- X-loader does not appear to start at all.

That doesn't affect the validity of this patch -- we want need 4-MiB alignment capability regardless -- but we may have to keep the odd alignment for the FAT partition if the on-board boot can't cope with it being somewhere else.

Cheers
---Dave

[Dave Martin (dave-martin-arm)]

Does it work to use sfdisk -L instead of sfdisk --force?

--force might allow sfdisk to do bad stuff, whereas -L simply turns off sanity checks required for compatibility with silly OSes.

[Loïc Minier (lool)]

On Fri, Feb 11, 2011, Dave Martin wrote:
> Does it work to use sfdisk -L instead of sfdisk --force?

 I had tried that in earlier versions, but I didn't try with the latest
 one; see my comment in the code about --force for what I was seeing
 back then.

 In any case, it would be nice to add -L and drop -D if possible

--
Loïc Minier

[Dave Martin (dave-martin-arm)]

> On Fri, Feb 11, 2011, Dave Martin wrote:
> > Does it work to use sfdisk -L instead of sfdisk --force?
>
> I had tried that in earlier versions, but I didn't try with the latest
> one; see my comment in the code about --force for what I was seeing
> back then.
>
> In any case, it would be nice to add -L and drop -D if possible

For me, sfdisk prints warnings without --force but doesn't abort... if so, maybe it would be better not to have --force: using this just to suppress warnings may not be such a good idea.

Why do we have -D? IIUC, either we force the first partition to sector 63, or we force-align it to 4MiB. Either way, we don't trust sfdisk to place partitions for us, so -D probably doesn't do anything(?)

# sfdisk --version
sfdisk (util-linux-ng 2.17.2)

# echo '8192,73728,0xc,*'| sfdisk -S63 -H255 -uS -L /dev/sdg
Checking that no-one is using this disk right now ...
OK

Disk /dev/sdg: 15104 cylinders, 255 heads, 63 sectors/track
Old situation:
Warning: The partition table looks like it was made
  for C/H/S=*/16/32 (instead of 15104/255/63).
For this listing I'll assume that geometry.
Units = sectors of 512 bytes, counting from 0

   Device Boot Start End #sectors Id System
/dev/sdg1 * 8192 81919 73728 c W95 FAT32 (LBA)
/dev/sdg2 0 - 0 0 Empty
/dev/sdg3 0 - 0 0 Empty
/dev/sdg4 0 - 0 0 Empty
New situation:
Warning: The partition table looks like it was made
  for C/H/S=*/26/20 (instead of 15104/255/63).
For this listing I'll assume that geometry.
Units = sectors of 512 bytes, counting from 0

   Device Boot Start End #sectors Id System
/dev/sdg1 * 8192 81919 73728 c W95 FAT32 (LBA)
  start: (c,h,s) expected (15,19,13) found (0,130,3)
  end: (c,h,s) expected (157,13,20) found (5,25,20)
/dev/sdg2 0 - 0 0 Empty
/dev/sdg3 0 - 0 0 Empty
/dev/sdg4 0 - 0 0 Empty
Warning: partition 1 does not end at a cylinder boundary
Successfully wrote the new partition table

Re-reading the partition table ...

If you created or changed a DOS partition, /dev/foo7, say, then use dd(1)
to zero the first 512 bytes: dd if=/dev/zero of=/dev/foo7 bs=512 count=1
(See fdisk(8).)

[Loïc Minier (lool)]

So I did another series of test on top of r295 with commands like:
./linaro-media-create --image_file /tmp/beagle.img --dev beagle --rootfs ext4 --hwpack ~/downloads/linaro/11.05/hwpack_linaro-omap3_20110208-0_armel_supported.tar.gz --binary ~/downloads/linaro/11.05/linaro-natty-headless-tar-20110203-1.tar.gz --hwpack-force-yes --align-boot-part

* --dev beagle, --image_file, --hwpack from maverick: boots under QEMU (only beagle, not beaglexm), does not boot on beagle-xM (no output, I guess x-loader doesn't support it), /!\ boots on beagle but seems to use NAND x-loader
* --dev beagle, --image_file, --hwpack from natty: boots under QEMU (beagle and beaglexm), /!\ does not boot no beagle-xM (no output, no idea what's going on), /!\ boots on beagle but seems to use NAND x-loader
* --dev beagle, --image_file, --hwpack from maverick, --align-boot-part: does not boot under QEMU (whatever the machine)
* --dev beagle, --image_file, --hwpack from natty, --align-boot-part: boots under QEMU (whatever the machine, no serial on beaglexm), /!\ does NOT boot under beagle (pressing user button across resets, it still boots from NAND), nor beagle-xM (no output)

I am starting to suspect broken micro SD or something as it's not consistent at all.

[Loïc Minier (lool)]

Fail with different SD, different SD reader and with:
./linaro-media-create --mmc /dev/sdd --dev overo --rootfs btrfs --hwpack ~/downloads/linaro/11.05/hwpack_linaro-overo_20110211-1_armel_supported.tar.gz --binary ~/downloads/linaro/11.05/linaro-natty-headless-tar-20110203-1.tar.gz --hwpack-force-yes

no output on Overo

[Loïc Minier (lool)]

Dave had an interesting idea: maybe the odd lengths for the boot vfat break it; we indeed discovered that on both beaglexm and overo, creating partitions of even length at sector 63 works and of odd length doesn't! on xm we get some "6" characater on the console

I tested with this:
size=80256; sudo dd if=/dev/zero of=/dev/sdb count=10 bs=1M; sudo parted -s /dev/sdb mklabel msdos; echo "63,$size,0x0C,*" | LC_ALL=C sudo sfdisk -L -uS -H 255 -S 63 /dev/sdb; sudo mkfs.vfat -F32 /dev/sdb1; loop-mnt-do /dev/sdb1 sudo cp -v /home/lool/MLO .; sync

[lp:~lool/linaro-image-tools/4-mib-align]

296. By Loïc Minier on 2011-02-11

Make sure boot partition has an even length in sectors (partition length is an
entire number of KiBs) to please OMAP3 ROMs; these apparently don't care for
the start sector, but do care for the length of the partition... Update
comments as well.

[Loïc Minier (lool)]

With r296, which ensures that boot partition has an even number of sectors:

linaro-n, --mmc, beagle
sudo dd if=/dev/zero of=/dev/sdb bs=1M count=30
./linaro-media-create --mmc /dev/sdb --dev beagle --rootfs btrfs --hwpack ~/Bureau/hwpack_linaro-omap3_20110211-0_armel_supported.tar.gz --binary ~/downloads/linaro/11.05/linaro-natty-headless-tar-20110203-1.tar.gz --hwpack-force-yes

=> works on beagle and on beagle xM!

linaro-n, --mmc, overo
sudo dd if=/dev/zero of=/dev/sdb bs=1M count=30
./linaro-media-create --mmc /dev/sdb --dev overo --rootfs btrfs --hwpack ~/downloads/linaro/11.05/hwpack_linaro-overo_20110211-1_armel_supported.tar.gz --binary ~/Bureau/linaro-natty-headless-tar-20110211-3.tar.gz --hwpack-force-yes

=> works on overo (up to not finding fsck.btrfs :-)! also starts within qemu -M beagle and -M beaglexm, including Linux (and then crashes, since it can't emulate overo)

linaro-n, --mmc, beagle, +4MiB boot
sudo dd if=/dev/zero of=/dev/sdb bs=1M count=30
./linaro-media-create --mmc /dev/sdb --dev beagle --rootfs btrfs --hwpack ~/Bureau/hwpack_linaro-omap3_20110211-0_armel_supported.tar.gz --binary ~/downloads/linaro/11.05/linaro-natty-headless-tar-20110203-1.tar.gz --hwpack-force-yes --align-boot-part

=> works on beagle and on beagle xM!

linaro-n, --mmc, overo, +4MiB boot
sudo dd if=/dev/zero of=/dev/sdb bs=1M count=30
./linaro-media-create --mmc /dev/sdb --dev overo --rootfs btrfs --hwpack ~/downloads/linaro/11.05/hwpack_linaro-overo_20110211-1_armel_supported.tar.gz --binary ~/Bureau/linaro-natty-headless-tar-20110211-3.tar.gz --hwpack-force-yes --align-boot-part

=> works on overo! also starts within sudo qemu -M beagle and -M beaglexm, including Linux (and then crashes, since it can't emulate overo)

linaro-m, --mmc, beagle
sudo dd if=/dev/zero of=/dev/sdb bs=1M count=30
./linaro-media-create --mmc /dev/sdb --dev beagle --rootfs btrfs --hwpack ~/downloads/linaro/10.11/hwpack_linaro-omap3_20101109-1_armel_supported.tar.gz --binary ~/downloads/linaro/10.11/linaro-m-headless-tar-20101108-2.tar.gz --hwpack-force-yes

=> works on beagle and beagle xM! also starts within sudo qemu -M beagle and -M beaglexm, including Linux (and then crashes since linaro-m didn't support xM)

[Loïc Minier (lool)]

On Fri, Feb 11, 2011, Dave Martin wrote:
> For me, sfdisk prints warnings without --force but doesn't abort... if
> so, maybe it would be better not to have --force: using this just to
> suppress warnings may not be such a good idea.

 Problem is not the warnings, but the error code: if it's non-zero we
 abort l-m-c.

> Why do we have -D?

 Not sure; didn't add it, but I'd like to remove it too

--
Loïc Minier

[Guilherme Salgado (salgado)]

Hi Loïc,

Thanks for working on this and for the cleanup in tests. I see a few
things that could be improved but overall it looks good.

> === modified file 'linaro_media_create/boards.py'
> --- linaro_media_create/boards.py 2011-02-10 16:58:11 +0000
> +++ linaro_media_create/boards.py 2011-02-11 20:17:14 +0000
> @@ -31,6 +31,44 @@
> import tempfile
>
> from linaro_media_create import cmd_runner
> +from linaro_media_create.partitions import SECTOR_SIZE
> +
> +# Notes:
> +# * geometry is currently always 255 heads and 63 sectors due to limitations of
> +# older OMAP3 boot ROMs
> +# * apparently some OMAP3 ROMs don't tolerate vfat length of an odd number of
> +# sectors (only sizes rounded to 1 KiB seem to boot)
> +# * we want partitions aligned on 4 MiB as to get the best performance and
> +# limit wear-leveling
> +# * image_size is passed on the command-line and should preferably be a power
> +# of 2; it should be used as a "don't go over this size" information for a
> +# real device, and a "give me a file exactly this big" requirement for an
> +# image file. Having exactly a power of 2 helps with QEMU; there seem to be
> +# some truncating issues otherwise. XXX to be researched
> +
> +# align on 4 MiB
> +PART_ALIGN_S = 4 * 1024 * 1024 / SECTOR_SIZE
> +
> +align_up = lambda x, a: (x + a - 1) / a * a

I think it'd be nicer to write this as a regular function instead of
defining an anonymous function and then giving it a name

> +
> +# optional bootloader partition; at least 1 MiB; in theory, an i.MX5 bootloader
> +# partition could hold RedBoot, FIS table, RedBoot config, kernel, and initrd,
> +# but we typically use U-Boot which is about 167 KiB as of 2011/02/11 and
> +# currently doesn't even store its environment there, so this should be enough
> +LOADER_MIN_SIZE_S = align_up(1 * 1024 * 1024, SECTOR_SIZE) / SECTOR_SIZE
> +# boot partition; at least 50 MiB; XXX this shouldn't be hardcoded
> +BOOT_MIN_SIZE_S = align_up(50 * 1024 * 1024, SECTOR_SIZE) / SECTOR_SIZE
> +# root partition; at least 50 MiB; XXX this shouldn't be hardcoded
> +ROOT_MIN_SIZE_S = align_up(50 * 1024 * 1024, SECTOR_SIZE) / SECTOR_SIZE
> +
> +
> +def align_partition(min_start, min_length, start_alignment, end_alignment):

Care to add a docstring to this function?

> + start = align_up(min_start, start_alignment)
> + # end offset is inclusive, so substact one
> + end = align_up(start + min_length, end_alignment) - 1
> + # and add one to length
> + length = end - start + 1
> + return start, end, length
>
>
> class BoardConfig(object):
> @@ -53,17 +91,41 @@
> serial_tty = None
>
> @classmethod
> - def get_sfdisk_cmd(cls):
> - """Return the sfdisk command to partition the media."""
> + def get_sfdisk_cmd(cls, should_align_boot_part=False):
> + """Return the sfdisk command to partition the media.
> +
> + :param should_align_boot_part: Whether to align the boot partition too.
> +
> + This default implementation returns a boot vfat partition of type FAT16
> + or FAT32, followed by a root partition."""
> +
> if cls.fat_size == 32:
> partition_type = '0x0C'
> ...

[Guilherme Salgado (salgado)]

review: Approve

[lp:~lool/linaro-image-tools/4-mib-align]

297. By Loïc Minier on 2011-02-14

Convert align_up to a real function and add docstrings to align_partition()
and align_up().

298. By Loïc Minier on 2011-02-14

Closing triple double-quotes need to stand on their own line.

299. By Loïc Minier on 2011-02-14

Fix typos in comments.

=== modified file 'linaro-media-create'
--- linaro-media-create	2011-02-02 17:32:10 +0000
+++ linaro-media-create	2011-02-14 16:29:48 +0000
@@ -130,7 +130,7 @@
     boot_partition, root_partition = setup_partitions(
         board_config, media, args.image_size, args.boot_label, args.rfs_label,
         args.rootfs, args.should_create_partitions, args.should_format_bootfs,
-        args.should_format_rootfs)
+        args.should_format_rootfs, args.should_align_boot_part)
 
     rootfs_uuid = get_uuid(root_partition)
 

=== modified file 'linaro_media_create/__init__.py'
--- linaro_media_create/__init__.py	2011-02-10 16:58:11 +0000
+++ linaro_media_create/__init__.py	2011-02-14 16:29:48 +0000
@@ -105,4 +105,8 @@
     parser.add_argument(
         '--no-part', dest='should_create_partitions', action='store_false',
         help='Reuse existing partitions on the given media.')
+    parser.add_argument(
+        '--align-boot-part', dest='should_align_boot_part',
+        action='store_true',
+        help='Align boot partition too (might break older x-loaders).')
     return parser

=== modified file 'linaro_media_create/boards.py'
--- linaro_media_create/boards.py	2011-02-10 16:58:11 +0000
+++ linaro_media_create/boards.py	2011-02-14 16:29:48 +0000
@@ -31,6 +31,54 @@
 import tempfile
 
 from linaro_media_create import cmd_runner
+from linaro_media_create.partitions import SECTOR_SIZE
+
+# Notes:
+# * geometry is currently always 255 heads and 63 sectors due to limitations of
+#   older OMAP3 boot ROMs
+# * apparently some OMAP3 ROMs don't tolerate vfat length of an odd number of
+#   sectors (only sizes rounded to 1 KiB seem to boot)
+# * we want partitions aligned on 4 MiB as to get the best performance and
+#   limit wear-leveling
+# * image_size is passed on the command-line and should preferably be a power
+#   of 2; it should be used as a "don't go over this size" information for a
+#   real device, and a "give me a file exactly this big" requirement for an
+#   image file.  Having exactly a power of 2 helps with QEMU; there seem to be
+#   some truncating issues otherwise. XXX to be researched
+
+# align on 4 MiB
+PART_ALIGN_S = 4 * 1024 * 1024 / SECTOR_SIZE
+
+def align_up(value, align):
+    """Round value to the next multiple of align."""
+    return (value + align - 1) / align * align
+
+# optional bootloader partition; at least 1 MiB; in theory, an i.MX5 bootloader
+# partition could hold RedBoot, FIS table, RedBoot config, kernel, and initrd,
+# but we typically use U-Boot which is about 167 KiB as of 2011/02/11 and
+# currently doesn't even store its environment there, so this should be enough
+LOADER_MIN_SIZE_S = align_up(1 * 1024 * 1024, SECTOR_SIZE) / SECTOR_SIZE
+# boot partition; at least 50 MiB; XXX this shouldn't be hardcoded
+BOOT_MIN_SIZE_S = align_up(50 * 1024 * 1024, SECTOR_SIZE) / SECTOR_SIZE
+# root partition; at least 50 MiB; XXX this shouldn't be hardcoded
+ROOT_MIN_SIZE_S = align_up(50 * 1024 * 1024, SECTOR_SIZE) / SECTOR_SIZE
+
+
+def align_partition(min_start, min_length, start_alignment, end_alignment):
+    """Compute partition start and end offsets based on specified constraints.
+
+    :param min_start: Minimal start offset of partition
+    :param min_lengh: Minimal length of partition
+    :param start_alignment: Alignment of this partition
+    :param end_alignment: Alignment of the data following this partition
+    :return: start offset, end offset (inclusive), length
+    """
+    start = align_up(min_start, start_alignment)
+    # end offset is inclusive, so substact one
+    end = align_up(start + min_length, end_alignment) - 1
+    # and add one to length
+    length = end - start + 1
+    return start, end, length
 
 
 class BoardConfig(object):
@@ -53,17 +101,41 @@
     serial_tty = None
 
     @classmethod
-    def get_sfdisk_cmd(cls):
-        """Return the sfdisk command to partition the media."""
+    def get_sfdisk_cmd(cls, should_align_boot_part=False):
+        """Return the sfdisk command to partition the media.
+
+        :param should_align_boot_part: Whether to align the boot partition too.
+
+        This default implementation returns a boot vfat partition of type FAT16
+        or FAT32, followed by a root partition.
+        """
         if cls.fat_size == 32:
             partition_type = '0x0C'
         else:
             partition_type = '0x0E'
 
-        # This will create a partition of the given type, containing 9
-        # cylinders (74027520 bytes, ~70 MiB), followed by a Linux-type
-        # partition containing the rest of the free space.
-        return ',9,%s,*\n,,,-' % partition_type
+        # align on sector 63 for compatibility with broken versions of x-loader
+        # unless align_boot_part is set
+        boot_align = 63
+        if should_align_boot_part:
+            boot_align = PART_ALIGN_S
+
+        # can only start on sector 1 (sector 0 is MBR / partition table)
+        boot_start, boot_end, boot_len = align_partition(
+            1, BOOT_MIN_SIZE_S, boot_align, PART_ALIGN_S)
+        # apparently OMAP3 ROMs require the vfat length to be an even number
+        # of sectors (multiple of 1 KiB); decrease the length if it's odd,
+        # there should still be enough room
+        boot_len = boot_len - boot_len % 2
+        boot_end = boot_start + boot_len - 1
+        # we ignore _root_end / _root_len and return a sfdisk command to
+        # instruct the use of all remaining space; XXX if we had some root size
+        # config, we could do something more sensible
+        root_start, _root_end, _root_len = align_partition(
+            boot_end + 1, ROOT_MIN_SIZE_S, PART_ALIGN_S, PART_ALIGN_S)
+
+        return '%s,%s,%s,*\n%s,,,-' % (
+            boot_start, boot_len, partition_type, root_start)
 
     @classmethod
     def _get_boot_cmd(cls, is_live, is_lowmem, consoles, rootfs_uuid):
@@ -287,12 +359,32 @@
     mmc_option = '0:2'
 
     @classmethod
-    def get_sfdisk_cmd(cls):
-        # Create a one cylinder partition for fixed-offset bootloader data at
-        # the beginning of the image (size is one cylinder, so 8224768 bytes
-        # with the first sector for MBR).
-        sfdisk_cmd = super(Mx51evkConfig, cls).get_sfdisk_cmd()
-        return ',1,0xDA\n%s' % sfdisk_cmd
+    def get_sfdisk_cmd(cls, should_align_boot_part=None):
+        """Return the sfdisk command to partition the media.
+
+        :param should_align_boot_part: Ignored.
+
+        This i.MX5 implementation returns a non-FS data bootloader partition,
+        followed by a FAT32 boot partition, followed by a root partition.
+        """
+        # boot ROM expects bootloader at 0x400 which is sector 2 with the usual
+        # SECTOR_SIZE of 512; we could theoretically leave sector 1 unused, but
+        # older bootloaders like RedBoot might store the environment from 0x0
+        # onwards, so it's safer to just start at the first sector, sector 1
+        # (sector 0 is MBR / partition table)
+        loader_start, loader_end, loader_len = align_partition(
+            1, LOADER_MIN_SIZE_S, 1, PART_ALIGN_S)
+
+        boot_start, boot_end, boot_len = align_partition(
+            loader_end + 1, BOOT_MIN_SIZE_S, PART_ALIGN_S, PART_ALIGN_S)
+        # we ignore _root_end / _root_len and return a sfdisk command to
+        # instruct the use of all remaining space; XXX if we had some root size
+        # config, we could do something more sensible
+        root_start, _root_end, _root_len = align_partition(
+            boot_end + 1, ROOT_MIN_SIZE_S, PART_ALIGN_S, PART_ALIGN_S)
+
+        return '%s,%s,0xDA\n%s,%s,0x0C,*\n%s,,,-' % (
+            loader_start, loader_len, boot_start, boot_len, root_start)
 
     @classmethod
     def _make_boot_files(cls, uboot_parts_dir, boot_cmd, chroot_dir,

=== modified file 'linaro_media_create/partitions.py'
--- linaro_media_create/partitions.py	2011-02-10 17:16:48 +0000
+++ linaro_media_create/partitions.py	2011-02-14 16:29:48 +0000
@@ -47,15 +47,24 @@
 # might want to do it.
 def setup_partitions(board_config, media, image_size, bootfs_label,
                      rootfs_label, rootfs_type, should_create_partitions,
-                     should_format_bootfs, should_format_rootfs):
+                     should_format_bootfs, should_format_rootfs,
+                     should_align_boot_part=False):
     """Make sure the given device is partitioned to boot the given board.
 
     :param board_config: A BoardConfig class.
     :param media: The Media we should partition.
     :param image_size: The size of the image file, in case we're setting up a
         QEMU image.
+    :param bootfs_label: Label for the boot partition.
+    :param rootfs_label: Label for the root partition.
+    :param rootfs_type: Filesystem for the root partition.
     :param should_create_partitions: Whether or not we should erase existing
         partitions and create new ones.
+    :param should_format_bootfs: Whether to reuse the filesystem on the boot
+        partition.
+    :param should_format_rootfs: Whether to reuse the filesystem on the root
+        partition.
+    :param should_align_boot_part: Whether to align the boot partition too.
     """
     cylinders = None
     if not media.is_block_device:
@@ -69,7 +78,8 @@
 
     if should_create_partitions:
         create_partitions(
-            board_config, media, HEADS, SECTORS, cylinders)
+            board_config, media, HEADS, SECTORS, cylinders,
+            should_align_boot_part=should_align_boot_part)
 
     if media.is_block_device:
         bootfs, rootfs = get_boot_and_root_partitions_for_media(
@@ -264,17 +274,6 @@
         raise ValueError("Unknown size format: %s.  Use K[bytes], M[bytes] "
                          "or G[bytes]" % size)
 
-    # Round the size of the raw disk image up to a multiple of 256K so it is
-    # an exact number of SD card erase blocks in length.  Otherwise Linux
-    # under qemu cannot access the last part of the card and is likely to
-    # complain that the last partition on the disk has been truncated.  This
-    # doesn't appear to work in all cases, though, as can be seen on
-    # https://bugs.launchpad.net/linux-linaro/+bug/673335.
-    if real_size % (1024 * 256):
-        cylinders = real_size / CYLINDER_SIZE
-        real_size = cylinders * CYLINDER_SIZE
-        real_size = ((((real_size - 1) / (1024 * 256)) + 1) * (1024 * 256))
-
     return real_size
 
 
@@ -289,8 +288,13 @@
     :param commands: A string of sfdisk commands; each on a separate line.
     :return: A 2-tuple containing the subprocess' stdout and stderr.
     """
+    # --force is unfortunate, but a consequence of having partitions not
+    # starting on cylinder boundaries: sfdisk will abort with "Warning:
+    # partition 2 does not start at a cylinder boundary"
     args = ['sfdisk',
+            '--force',
             '-D',
+            '-uS',
             '-H', str(heads),
             '-S', str(sectors)]
     if cylinders is not None:
@@ -302,7 +306,8 @@
     return proc.communicate("%s\n" % commands)
 
 
-def create_partitions(board_config, media, heads, sectors, cylinders=None):
+def create_partitions(board_config, media, heads, sectors, cylinders=None,
+                      should_align_boot_part=False):
     """Partition the given media according to the board requirements.
 
     :param board_config: A BoardConfig class.
@@ -313,6 +318,7 @@
         partitions.
     :param cylinders: The number of cylinders to pass to sfdisk's -C argument.
         If None the -C argument is not passed.
+    :param should_align_boot_part: Whether to align the boot partition too.
     """
     if media.is_block_device:
         # Overwrite any existing partition tables with a fresh one.
@@ -320,7 +326,8 @@
             ['parted', '-s', media.path, 'mklabel', 'msdos'], as_root=True)
         proc.wait()
 
-    sfdisk_cmd = board_config.get_sfdisk_cmd()
+    sfdisk_cmd = board_config.get_sfdisk_cmd(
+        should_align_boot_part=should_align_boot_part)
     run_sfdisk_commands(sfdisk_cmd, heads, sectors, cylinders, media.path)
 
     # Sync and sleep to wait for the partition to settle.

=== modified file 'linaro_media_create/tests/test_media_create.py'
--- linaro_media_create/tests/test_media_create.py	2011-02-10 19:08:06 +0000
+++ linaro_media_create/tests/test_media_create.py	2011-02-14 16:29:48 +0000
@@ -43,6 +43,8 @@
     )
 import linaro_media_create
 from linaro_media_create.boards import (
+    align_up,
+    align_partition,
     BoardConfig,
     board_configs,
     make_boot_script,
@@ -283,6 +285,27 @@
         self.assertEqual(expected, self.funcs_calls)
 
 
+class TestAlignPartition(TestCase):
+    def test_align_up_none(self):
+        self.assertEqual(1024, align_up(1024, 1))
+
+    def test_align_up_no_rounding(self):
+        self.assertEqual(512, align_up(512, 512))
+
+    def test_align_up_rounding(self):
+        self.assertEqual(512, align_up(1, 512))
+
+    def test_align_partition_4_mib_4_mib(self):
+        expected = (4 * 1024 * 1024, 8 * 1024 * 1024 - 1, 4 * 1024 * 1024)
+        self.assertEqual(expected,
+            align_partition(1, 1, 4 * 1024 * 1024, 4 * 1024 * 1024))
+
+    def test_align_partition_none_4_mib(self):
+        expected = (1, 4 * 1024 * 1024 - 1, 4 * 1024 * 1024 - 1)
+        self.assertEqual(expected,
+            align_partition(1, 1, 1, 4 * 1024 * 1024))
+
+
 class TestFixForBug697824(TestCaseWithFixtures):
 
     def mock_set_appropriate_serial_tty(self, config):
@@ -331,12 +354,18 @@
 class TestGetSfdiskCmd(TestCase):
 
     def test_default(self):
-        self.assertEquals(
-            ',9,0x0C,*\n,,,-', boards.BoardConfig.get_sfdisk_cmd())
+        self.assertEqual(
+            '63,106432,0x0C,*\n106496,,,-',
+            boards.BoardConfig.get_sfdisk_cmd())
+
+    def test_default_aligned(self):
+        self.assertEqual(
+            '8192,106496,0x0C,*\n114688,,,-',
+            boards.BoardConfig.get_sfdisk_cmd(should_align_boot_part=True))
 
     def test_mx51evk(self):
-        self.assertEquals(
-            ',1,0xDA\n,9,0x0C,*\n,,,-',
+        self.assertEqual(
+            '1,8191,0xDA\n8192,106496,0x0C,*\n114688,,,-',
             board_configs['mx51evk'].get_sfdisk_cmd())
 
 
@@ -633,7 +662,7 @@
         # every time we run sfdisk it actually repartitions the device,
         # erasing any partitions created previously.
         self.assertEqual(
-            [(',1,0xDA\n,9,0x0C,*\n,,,-', 255, 63, '', self.media.path)],
+            [('1,8191,0xDA\n8192,106496,0x0C,*\n114688,,,-', 255, 63, '', self.media.path)],
             sfdisk_fixture.mock.calls)
 
     def test_create_partitions_for_beagle(self):
@@ -648,7 +677,7 @@
              ['sync']],
             popen_fixture.mock.calls)
         self.assertEqual(
-            [(',9,0x0C,*\n,,,-', 255, 63, '', self.media.path)],
+            [('63,106432,0x0C,*\n106496,,,-', 255, 63, '', self.media.path)],
             sfdisk_fixture.mock.calls)
 
     def test_create_partitions_with_img_file(self):
@@ -665,7 +694,7 @@
         self.assertEqual([['sync']], popen_fixture.mock.calls)
 
         self.assertEqual(
-            [(',9,0x0C,*\n,,,-', 255, 63, '', tempfile)],
+            [('63,106432,0x0C,*\n106496,,,-', 255, 63, '', tempfile)],
             sfdisk_fixture.mock.calls)
 
     def test_run_sfdisk_commands(self):
@@ -675,7 +704,7 @@
             stdout=subprocess.PIPE)
         proc.communicate()
         stdout, stderr = run_sfdisk_commands(
-            ',1,0xDA', 5, 63, '', tempfile, as_root=False,
+            '2,16063,0xDA', 255, 63, '', tempfile, as_root=False,
             stderr=subprocess.PIPE)
         self.assertIn('Successfully wrote the new partition table', stdout)
 
@@ -684,7 +713,7 @@
         self.assertRaises(
             cmd_runner.SubcommandNonZeroReturnValue,
             run_sfdisk_commands,
-            ',1,0xDA', 5, 63, '', tempfile, as_root=False,
+            ',1,0xDA', 255, 63, '', tempfile, as_root=False,
             stderr=subprocess.PIPE)
 
 
@@ -700,6 +729,11 @@
         super(TestPartitionSetup, self).tearDown()
         time.sleep = self.orig_sleep
 
+    def _create_tempfile(self):
+        # boot part at +8 MiB, root part at +16 MiB
+        return self._create_qemu_img_with_partitions(
+            '16384,15746,0x0C,*\n32768,,,-')
+
     def test_convert_size_in_kbytes_to_bytes(self):
         self.assertEqual(512 * 1024, convert_size_to_bytes('512K'))
 
@@ -709,28 +743,23 @@
     def test_convert_size_in_gbytes_to_bytes(self):
         self.assertEqual(12 * 1024**3, convert_size_to_bytes('12G'))
 
-    def test_convert_size_in_kbytes_to_bytes_rounds_to_256k_multiple(self):
-        # See comment in convert_size_to_bytes as to why we need to do this.
-        self.assertEqual(
-            3891 * (1024 * 256), convert_size_to_bytes('1000537K'))
-
     def test_calculate_partition_size_and_offset(self):
-        tempfile = self._create_qemu_img_with_partitions(',1,0x0C,*\n,,,-')
+        tempfile = self._create_tempfile()
         vfat_size, vfat_offset, linux_size, linux_offset = (
             calculate_partition_size_and_offset(tempfile))
         self.assertEqual(
-            [129024L, 32256L, 10321920L, 161280L],
+            [8061952L, 8388608L, 14680064L, 16777216L],
             [vfat_size, vfat_offset, linux_size, linux_offset])
 
     def test_partition_numbering(self):
-        # another Linux partition after the boot/root parts
+        # another Linux partition at +24 MiB after the boot/root parts
         tempfile = self._create_qemu_img_with_partitions(
-            ',1,0x0C,*\n,1,,-\n,,,-')
+            '16384,15746,0x0C,*\n32768,15427,,-\n49152,,,-')
         vfat_size, vfat_offset, linux_size, linux_offset = (
             calculate_partition_size_and_offset(tempfile))
-        # check that the linux partition offset starts on second cylinder so
-        # that it's the partition immediately following the vfat one
-        self.assertEqual(linux_offset, 5 * 63 * 512)
+        # check that the linux partition offset starts at +16 MiB so that it's
+        # the partition immediately following the vfat one
+        self.assertEqual(linux_offset, 32768 * 512)
 
     def test_get_boot_and_root_partitions_for_media_beagle(self):
         self.useFixture(MockSomethingFixture(
@@ -759,11 +788,11 @@
     def _create_qemu_img_with_partitions(self, sfdisk_commands):
         tempfile = self.createTempFileAsFixture()
         proc = cmd_runner.run(
-            ['qemu-img', 'create', '-f', 'raw', tempfile, '10M'],
+            ['qemu-img', 'create', '-f', 'raw', tempfile, '30M'],
             stdout=subprocess.PIPE)
         proc.communicate()
         stdout, stderr = run_sfdisk_commands(
-            sfdisk_commands, 5, 63, '', tempfile, as_root=False,
+            sfdisk_commands, 255, 63, '', tempfile, as_root=False,
             # Throw away stderr as sfdisk complains a lot when operating on a
             # qemu image.
             stderr=subprocess.PIPE)
@@ -786,7 +815,7 @@
         self.assertEqual(None, popen_fixture.mock.calls)
 
     def test_get_boot_and_root_loopback_devices(self):
-        tempfile = self._create_qemu_img_with_partitions(',1,0x0C,*\n,,,-')
+        tempfile = self._create_tempfile()
         atexit_fixture = self.useFixture(MockSomethingFixture(
             atexit, 'register', AtExitRegister()))
         popen_fixture = self.useFixture(MockCmdRunnerPopenFixture())
@@ -796,9 +825,9 @@
         get_boot_and_root_loopback_devices(tempfile)
         self.assertEqual(
             [['sudo', 'losetup', '-f', '--show', tempfile, '--offset',
-              '32256', '--sizelimit', '129024'],
+              '8388608', '--sizelimit', '8061952'],
              ['sudo', 'losetup', '-f', '--show', tempfile, '--offset',
-              '161280', '--sizelimit', '10321920']],
+              '16777216', '--sizelimit', '14680064']],
             popen_fixture.mock.calls)
 
         # get_boot_and_root_loopback_devices will also setup two exit handlers
@@ -818,7 +847,7 @@
         # but here we mock Popen() and thanks to that the image is not setup
         # (via qemu-img) inside setup_partitions.  That's why we pass an
         # already setup image file.
-        tempfile = self._create_qemu_img_with_partitions(',1,0x0C,*\n,,,-')
+        tempfile = self._create_tempfile()
         popen_fixture = self.useFixture(MockCmdRunnerPopenFixture())
         self.useFixture(MockSomethingFixture(
             sys, 'stdout', open('/dev/null', 'w')))
@@ -838,11 +867,11 @@
             board_configs['beagle'], Media(tempfile), '2G', 'boot',
             'root', 'ext3', True, True, True)
         self.assertEqual(
-             # This is the call that would create the image file.
+             # This is the call that would create a 2 GiB image file.
             [['qemu-img', 'create', '-f', 'raw', tempfile, '2147483648'],
              # This call would partition the image file.
-             ['sudo', 'sfdisk', '-D', '-H', '255', '-S', '63', '-C', '261',
-              tempfile],
+             ['sudo', 'sfdisk', '--force', '-D', '-uS', '-H', '255', '-S',
+              '63', '-C', '261', tempfile],
              # Make sure changes are written to disk.
              ['sync'],
              ['sudo', 'mkfs.vfat', '-F', '32', bootfs_dev, '-n', 'boot'],
@@ -855,7 +884,7 @@
         # Pretend the partitions are mounted.
         self.useFixture(MockSomethingFixture(
             partitions, 'is_partition_mounted', lambda part: True))
-        tempfile = self._create_qemu_img_with_partitions(',1,0x0C,*\n,,,-')
+        tempfile = self._create_tempfile()
         self.useFixture(MockSomethingFixture(
             partitions, '_get_device_file_for_partition_number',
             lambda dev, partition: '%s%d' % (tempfile, partition)))
@@ -868,7 +897,8 @@
             True, True, True)
         self.assertEqual(
             [['sudo', 'parted', '-s', tempfile, 'mklabel', 'msdos'],
-             ['sudo', 'sfdisk', '-D', '-H', '255', '-S', '63', tempfile],
+             ['sudo', 'sfdisk', '--force', '-D', '-uS', '-H', '255', '-S',
+              '63', tempfile],
              ['sync'],
              # Since the partitions are mounted, setup_partitions will umount
              # them before running mkfs.