fix for issue 2610: add script to populate payload partition of recovery image

fix for issue 2610
  - add shell script to populate payload partition of a recovery image
    (i.e. copying partition A of a pristine Chrome OS image to partition B
     of a recovery image)

[Tan Gao, 2010-06-04 00:20:52]

Hi Andrew,

Here's the shell script to populate payload partition (B) of recovery image by
copying from a pristine image (partition A)

Tan

[adlr, 2010-06-07 16:53:55]

http://codereview.chromium.org/2568003/diff/1/2
File src/scripts/populate_recovery_payload.sh (right):

http://codereview.chromium.org/2568003/diff/1/2#newcode78
src/scripts/populate_recovery_payload.sh:78: echo "About to overwrite payload
partition..."
you can move this down a couple lines so that it runs if --yes is specified, or
if the user types 'y' into the term

http://codereview.chromium.org/2568003/diff/1/2#newcode141
src/scripts/populate_recovery_payload.sh:141: sudo dd if=/dev/zero
of=$FLAGS_dst_image conv=notrunc bs=512 \
i think you can skip this...

http://codereview.chromium.org/2568003/diff/1/2#newcode145
src/scripts/populate_recovery_payload.sh:145: sudo dd if=/dev/zero
of=$FLAGS_dst_image conv=notrunc bs=512 \
and this

http://codereview.chromium.org/2568003/diff/1/2#newcode150
src/scripts/populate_recovery_payload.sh:150: sudo dd if=$FLAGS_src_image
of=$FLAGS_dst_image conv=notrunc bs=512 \
can you check if it's possible to do it with bs=2M? I think we set up the images
so that you can do that.

You can use bs=2M if all of the following hold:
- $(( $SRC_ROOTFS_OFFSET % (2 * 1024 * 1024 / 512) )) = "0"
- $(( $SRC_ROOTFS_SECTORS % (2 * 1024 * 1024 / 512) )) = "0"
(repeat checks for DST and for KERN partitions)

if you use bs=2M, it'll just run faster, which people really like.

[Tan Gao, 2010-06-07 20:36:18]

Hi Andrew,

I just added check for using 2MB block size (faster write!). PTAL

Thanks,
Tan

http://codereview.chromium.org/2568003/diff/1/2
File src/scripts/populate_recovery_payload.sh (right):

http://codereview.chromium.org/2568003/diff/1/2#newcode78
src/scripts/populate_recovery_payload.sh:78: echo "About to overwrite payload
partition..."
On 2010/06/07 16:53:55, adlr wrote:
> you can move this down a couple lines so that it runs if --yes is specified,
or
> if the user types 'y' into the term

Done.

http://codereview.chromium.org/2568003/diff/1/2#newcode141
src/scripts/populate_recovery_payload.sh:141: sudo dd if=/dev/zero
of=$FLAGS_dst_image conv=notrunc bs=512 \
On 2010/06/07 16:53:55, adlr wrote:
> i think you can skip this...

removed

http://codereview.chromium.org/2568003/diff/1/2#newcode145
src/scripts/populate_recovery_payload.sh:145: sudo dd if=/dev/zero
of=$FLAGS_dst_image conv=notrunc bs=512 \
On 2010/06/07 16:53:55, adlr wrote:
> and this

removed

http://codereview.chromium.org/2568003/diff/1/2#newcode150
src/scripts/populate_recovery_payload.sh:150: sudo dd if=$FLAGS_src_image
of=$FLAGS_dst_image conv=notrunc bs=512 \
indeed! with 2MB, it writes in 43 sec!

[adlr, 2010-06-07 21:38:18]

just a couple small things...

http://codereview.chromium.org/2568003/diff/8001/9001
File src/scripts/populate_recovery_payload.sh (right):

http://codereview.chromium.org/2568003/diff/8001/9001#newcode140
src/scripts/populate_recovery_payload.sh:140: ROOTFS_BS=512                    
# default sector size = 512 bytes
rename to SECTOR_SIZE

http://codereview.chromium.org/2568003/diff/8001/9001#newcode144
src/scripts/populate_recovery_payload.sh:144: if [ $(( $SRC_ROOTFS_OFFSET % (2 *
1024 * 1024 / 512) )) = "0" ] && \
s/512/$SECTOR_SIZE/

http://codereview.chromium.org/2568003/diff/8001/9001#newcode148
src/scripts/populate_recovery_payload.sh:148: ROOTFS_BS=2097152  # increase
sector size to 2MB
ROOTFS_BS=$((2 * 1024 * 1024))

this makes it easier to see what this number is

[Tan Gao, 2010-06-07 22:06:29]

Hi Andrew,

cleaned up numbers with appropriate constants to make the lines a bit more
readable.

PTAL,
Tan

http://codereview.chromium.org/2568003/diff/8001/9001
File src/scripts/populate_recovery_payload.sh (right):

http://codereview.chromium.org/2568003/diff/8001/9001#newcode140
src/scripts/populate_recovery_payload.sh:140: ROOTFS_BS=512                    
# default sector size = 512 bytes
On 2010/06/07 21:38:20, adlr wrote:
> rename to SECTOR_SIZE

added SECTOR_SIZE

(in case 2MB boundary check fails, ROOTFS_BS still needs a value to be used in
sudo dd below)

http://codereview.chromium.org/2568003/diff/8001/9001#newcode144
src/scripts/populate_recovery_payload.sh:144: if [ $(( $SRC_ROOTFS_OFFSET % (2 *
1024 * 1024 / 512) )) = "0" ] && \
On 2010/06/07 21:38:20, adlr wrote:
> s/512/$SECTOR_SIZE/

Done.

http://codereview.chromium.org/2568003/diff/8001/9001#newcode148
src/scripts/populate_recovery_payload.sh:148: ROOTFS_BS=2097152  # increase
sector size to 2MB
On 2010/06/07 21:38:20, adlr wrote:
> ROOTFS_BS=$((2 * 1024 * 1024))
> 
> this makes it easier to see what this number is

Done. and s/2 * 1024 * 1024/$LARGE_SECTOR_SIZE/

[adlr, 2010-06-07 22:18:00]

LGTM w/ one more rename request.

Thanks for making these changes!

http://codereview.chromium.org/2568003/diff/13001/3002
File src/scripts/populate_recovery_payload.sh (right):

http://codereview.chromium.org/2568003/diff/13001/3002#newcode141
src/scripts/populate_recovery_payload.sh:141: LARGE_SECTOR_SIZE=$((2 * 1024 *
1024))  # large sector size in bytes
I would actually prefer to call this something like IDEAL_BLOCK_SIZE. The reason
is that sectors are a property of disks; they are the smallest addressable unit
on the disk, and they are 512 bytes in length. So, the term "sector" implies
that it's a property of the disk.

Here, it's not a bigger sector size, it's just that we'll do writes that are
bigger. block is a more generic term than sector, so it's fine to call it a
block size.

[Tan Gao, 2010-06-07 22:35:48]

darn .. tree is closed .. will push after it's reopened :-(

thanks again for the review!

http://codereview.chromium.org/2568003/diff/13001/3002
File src/scripts/populate_recovery_payload.sh (right):

http://codereview.chromium.org/2568003/diff/13001/3002#newcode141
src/scripts/populate_recovery_payload.sh:141: LARGE_SECTOR_SIZE=$((2 * 1024 *
1024))  # large sector size in bytes
On 2010/06/07 22:18:00, adlr wrote:
> I would actually prefer to call this something like IDEAL_BLOCK_SIZE. The
reason
> is that sectors are a property of disks; they are the smallest addressable
unit
> on the disk, and they are 512 bytes in length. So, the term "sector" implies
> that it's a property of the disk.
> 
> Here, it's not a bigger sector size, it's just that we'll do writes that are
> bigger. block is a more generic term than sector, so it's fine to call it a
> block size.

Done.