Add script to in-place modify a recovery image to ssd

Add script to in-place modify a recovery image to ssd

Change-Id: I6435a4b0f40a571f8e44830e6d32f42d2d3213ff

BUG=none
TEST=manually tested with a signed image and comparing the kernel, and rootfs partitions.

Committed: http://chrome-svn/viewvc/chromeos?view=rev&revision=e77bec9

[gauravsh, 2011-02-22 18:47:49]

ping?

On Wed, Feb 16, 2011 at 4:23 PM, <gauravsh@chromium.org> wrote:

> Reviewers: scottz,
>
> Description:
> Add script to in-place modify a recovery image to ssd
>
> Change-Id: I6435a4b0f40a571f8e44830e6d32f42d2d3213ff
>
> BUG=none
> TEST=manually tested with a signed image and comparing the kernel, and
> rootfs
> partitions.
>
> Please review this at http://codereview.chromium.org/6533015/
>
> SVN Base: ssh://git@gitrw.chromium.org:9222/vboot_reference.git@master
>
> Affected files:
>  A scripts/image_signing/convert_recovery_to_ssd.sh
>  M scripts/image_signing/sign_official_build.sh
>
>
> Index: scripts/image_signing/convert_recovery_to_ssd.sh
> diff --git a/scripts/image_signing/convert_recovery_to_ssd.sh
> b/scripts/image_signing/convert_recovery_to_ssd.sh
> new file mode 100755
> index
>
0000000000000000000000000000000000000000..b011321f8c0bed9ed973e745478c49ee1760fea1
> --- /dev/null
> +++ b/scripts/image_signing/convert_recovery_to_ssd.sh
> @@ -0,0 +1,67 @@
> +#!/bin/bash
> +
> +# Copyright (c) 2011 The Chromium OS Authors. All rights reserved.
> +# Use of this source code is governed by a BSD-style license that can be
> +# found in the LICENSE file.
> +
> +# Script to convert a recovery umage into an SSD image. Changes are made
> in place.
> +
> +# Load common constants and variables.
> +. "$(dirname "$0")/common_minimal.sh"
> +
> +usage() {
> +  cat <<EOF
> +Usage: $PROG <image> [--force]
> +
> +In-place converts recovery <image> into an SSD image. With --force, does
> not ask for
> +confirmation from the user.
> +
> +EOF
> +}
> +
> +if [ $# -gt 2 ]; then
> +  usage
> +  exit 1
> +fi
> +
> +type -P cgpt &>/dev/null ||
> +  { echo "cgpt tool must be in the path"; exit 1; }
> +
> +# Abort on errors.
> +set -e
> +
> +IMAGE=$1
> +IS_FORCE=$2
> +
> +if [ "${IS_FORCE}" != "--force" ]; then
> +  echo "This will modify ${IMAGE} in-place and convert it into an SSD
> image."
> +  read -p "Are you sure you want to continue (y/N)?" SURE
> +  SURE="${SURE:0:1}"
> +  [ "${SURE}" != "y" ] && exit 1
> +fi
> +
> +kerna_offset=$(partoffset ${IMAGE} 2)
> +kernb_offset=$(partoffset ${IMAGE} 4)
> +# Kernel partition sizes should be the same.
> +kern_size=$(partsize ${IMAGE} 2)
> +
> +# Move Kernel B to Kernel A.
> +kernb=$(make_temp_file)
> +echo "Replacing Kernel partition A with Kernel partition B"
> +extract_image_partition ${IMAGE} 4 ${kernb}
> +replace_image_partition ${IMAGE} 2 ${kernb}
> +
> +# Overwrite the vblock.
> +stateful_dir=$(make_temp_dir)
> +tmp_vblock=$(make_temp_file)
> +mount_image_partition_ro ${IMAGE} 1 ${stateful_dir}
> +sudo cp ${stateful_dir}/vmlinuz_hd.vblock ${tmp_vblock}
> +# Unmount before overwriting image to avoid sync issues.
> +sudo umount -d ${stateful_dir}
> +echo "Overwriting kernel partition A vblock with SSD vblock"
> +sudo dd if=${tmp_vblock} of=${IMAGE} seek=${kerna_offset} bs=512
> conv=notrunc
> +
> +# Zero out Kernel B partition.
> +echo "Zeroing out Kernel partition B"
> +sudo dd if=/dev/zero of=${IMAGE} seek=${kernb_offset} bs=512
> count=${kern_size} conv=notrunc
> +echo "${IMAGE} was converted to an SSD image."
> Index: scripts/image_signing/sign_official_build.sh
> diff --git a/scripts/image_signing/sign_official_build.sh
> b/scripts/image_signing/sign_official_build.sh
> index
>
3737d943c7d4643de08d6f6c287becfc4fe107cb..da9db0623fe36b2f71f2ad899cb9a86648a49797
> 100755
> --- a/scripts/image_signing/sign_official_build.sh
> +++ b/scripts/image_signing/sign_official_build.sh
> @@ -412,6 +412,8 @@ sign_for_recovery() {
>   # TODO(gauravsh): Remove this if we get rid of the need to overwrite
>   # the vblock during installs. Kern B could directly be signed by the
>   # SSD keys.
> +  # Note: This vblock is also needed for the ability to convert a recovery
> +  # image into the equivalent SSD image (convert_recovery_to_ssd.sh)
>   local stateful_dir=$(make_temp_dir)
>   mount_image_partition ${OUTPUT_IMAGE} 1 ${stateful_dir}
>   sudo cp ${temp_out_vb} ${stateful_dir}/vmlinuz_hd.vblock
>
>
>


-- 
-g

[scottz-goog, 2011-02-22 19:27:59]

It will happen today ;)

-Scott


On Tue, Feb 22, 2011 at 10:45, Gaurav Shah <gauravsh@chromium.org> wrote:
> ping?
>
> On Wed, Feb 16, 2011 at 4:23 PM, <gauravsh@chromium.org> wrote:
>>
>> Reviewers: scottz,
>>
>> Description:
>> Add script to in-place modify a recovery image to ssd
>>
>> Change-Id: I6435a4b0f40a571f8e44830e6d32f42d2d3213ff
>>
>> BUG=none
>> TEST=manually tested with a signed image and comparing the kernel, and
>> rootfs
>> partitions.
>>
>> Please review this at http://codereview.chromium.org/6533015/
>>
>> SVN Base: ssh://git@gitrw.chromium.org:9222/vboot_reference.git@master
>>
>> Affected files:
>>  A scripts/image_signing/convert_recovery_to_ssd.sh
>>  M scripts/image_signing/sign_official_build.sh
>>
>>
>> Index: scripts/image_signing/convert_recovery_to_ssd.sh
>> diff --git a/scripts/image_signing/convert_recovery_to_ssd.sh
>> b/scripts/image_signing/convert_recovery_to_ssd.sh
>> new file mode 100755
>> index
>>
0000000000000000000000000000000000000000..b011321f8c0bed9ed973e745478c49ee1760fea1
>> --- /dev/null
>> +++ b/scripts/image_signing/convert_recovery_to_ssd.sh
>> @@ -0,0 +1,67 @@
>> +#!/bin/bash
>> +
>> +# Copyright (c) 2011 The Chromium OS Authors. All rights reserved.
>> +# Use of this source code is governed by a BSD-style license that can be
>> +# found in the LICENSE file.
>> +
>> +# Script to convert a recovery umage into an SSD image. Changes are made
>> in place.
>> +
>> +# Load common constants and variables.
>> +. "$(dirname "$0")/common_minimal.sh"
>> +
>> +usage() {
>> +  cat <<EOF
>> +Usage: $PROG <image> [--force]
>> +
>> +In-place converts recovery <image> into an SSD image. With --force, does
>> not ask for
>> +confirmation from the user.
>> +
>> +EOF
>> +}
>> +
>> +if [ $# -gt 2 ]; then
>> +  usage
>> +  exit 1
>> +fi
>> +
>> +type -P cgpt &>/dev/null ||
>> +  { echo "cgpt tool must be in the path"; exit 1; }
>> +
>> +# Abort on errors.
>> +set -e
>> +
>> +IMAGE=$1
>> +IS_FORCE=$2
>> +
>> +if [ "${IS_FORCE}" != "--force" ]; then
>> +  echo "This will modify ${IMAGE} in-place and convert it into an SSD
>> image."
>> +  read -p "Are you sure you want to continue (y/N)?" SURE
>> +  SURE="${SURE:0:1}"
>> +  [ "${SURE}" != "y" ] && exit 1
>> +fi
>> +
>> +kerna_offset=$(partoffset ${IMAGE} 2)
>> +kernb_offset=$(partoffset ${IMAGE} 4)
>> +# Kernel partition sizes should be the same.
>> +kern_size=$(partsize ${IMAGE} 2)
>> +
>> +# Move Kernel B to Kernel A.
>> +kernb=$(make_temp_file)
>> +echo "Replacing Kernel partition A with Kernel partition B"
>> +extract_image_partition ${IMAGE} 4 ${kernb}
>> +replace_image_partition ${IMAGE} 2 ${kernb}
>> +
>> +# Overwrite the vblock.
>> +stateful_dir=$(make_temp_dir)
>> +tmp_vblock=$(make_temp_file)
>> +mount_image_partition_ro ${IMAGE} 1 ${stateful_dir}
>> +sudo cp ${stateful_dir}/vmlinuz_hd.vblock ${tmp_vblock}
>> +# Unmount before overwriting image to avoid sync issues.
>> +sudo umount -d ${stateful_dir}
>> +echo "Overwriting kernel partition A vblock with SSD vblock"
>> +sudo dd if=${tmp_vblock} of=${IMAGE} seek=${kerna_offset} bs=512
>> conv=notrunc
>> +
>> +# Zero out Kernel B partition.
>> +echo "Zeroing out Kernel partition B"
>> +sudo dd if=/dev/zero of=${IMAGE} seek=${kernb_offset} bs=512
>> count=${kern_size} conv=notrunc
>> +echo "${IMAGE} was converted to an SSD image."
>> Index: scripts/image_signing/sign_official_build.sh
>> diff --git a/scripts/image_signing/sign_official_build.sh
>> b/scripts/image_signing/sign_official_build.sh
>> index
>>
3737d943c7d4643de08d6f6c287becfc4fe107cb..da9db0623fe36b2f71f2ad899cb9a86648a49797
>> 100755
>> --- a/scripts/image_signing/sign_official_build.sh
>> +++ b/scripts/image_signing/sign_official_build.sh
>> @@ -412,6 +412,8 @@ sign_for_recovery() {
>>   # TODO(gauravsh): Remove this if we get rid of the need to overwrite
>>   # the vblock during installs. Kern B could directly be signed by the
>>   # SSD keys.
>> +  # Note: This vblock is also needed for the ability to convert a
>> recovery
>> +  # image into the equivalent SSD image (convert_recovery_to_ssd.sh)
>>   local stateful_dir=$(make_temp_dir)
>>   mount_image_partition ${OUTPUT_IMAGE} 1 ${stateful_dir}
>>   sudo cp ${temp_out_vb} ${stateful_dir}/vmlinuz_hd.vblock
>>
>>
>
>
>
> --
> -g
>

[scottz-goog, 2011-02-23 01:37:50]

I know you aren't going to like this but I can't help but notice how large these
"one off" image manipulation scripts have become. The readability is definitely
being impacted, what are the chances of us porting these to python sooner rather
than later?

http://codereview.chromium.org/6533015/diff/1/scripts/image_signing/convert_r...
File scripts/image_signing/convert_recovery_to_ssd.sh (right):

http://codereview.chromium.org/6533015/diff/1/scripts/image_signing/convert_r...
scripts/image_signing/convert_recovery_to_ssd.sh:7: # Script to convert a
recovery umage into an SSD image. Changes are made in place.
>80

http://codereview.chromium.org/6533015/diff/1/scripts/image_signing/convert_r...
scripts/image_signing/convert_recovery_to_ssd.sh:16: In-place converts recovery
<image> into an SSD image. With --force, does not ask for
>80

http://codereview.chromium.org/6533015/diff/1/scripts/image_signing/convert_r...
scripts/image_signing/convert_recovery_to_ssd.sh:27: type -P cgpt &>/dev/null ||
Any particular reason why you chose type over which?

http://codereview.chromium.org/6533015/diff/1/scripts/image_signing/convert_r...
scripts/image_signing/convert_recovery_to_ssd.sh:34: IS_FORCE=$2
You sort of assume that we are getting --force as the last argument, can we also
verify it is what you expect it is?

http://codereview.chromium.org/6533015/diff/1/scripts/image_signing/convert_r...
scripts/image_signing/convert_recovery_to_ssd.sh:43: kerna_offset=$(partoffset
${IMAGE} 2)
Are we assuming this is in our path or is this in a "bash library" somewhere :)

[gauravsh, 2011-02-23 02:07:00]

On Tue, Feb 22, 2011 at 5:37 PM, <scottz@google.com> wrote:

> I know you aren't going to like this but I can't help but notice how large
> these
> "one off" image manipulation scripts have become. The readability is
> definitely
> being impacted, what are the chances of us porting these to python sooner
> rather
> than later?
>

Don't assume you know how I am going to react. :)

I agree with you in general (sign_official_build needs a much needed
reboot), but I am not sure I agree that this particular script is large or
complex enough that it could be better re-written in python. (The flag
parsing is somewhat  unnecessary and I added --force only because I thought
in-place modification would be more convenient and faster for most users of
the script).

I do have plans to re-write the core image manipulation functions in
non-bash, but many of these scripts will unfortunately have to stay (they
run on the chrome os client machine which doesn't have python)


>
>
>
>
http://codereview.chromium.org/6533015/diff/1/scripts/image_signing/convert_r...
> File scripts/image_signing/convert_recovery_to_ssd.sh (right):
>
>
>
http://codereview.chromium.org/6533015/diff/1/scripts/image_signing/convert_r...
> scripts/image_signing/convert_recovery_to_ssd.sh:7: # Script to convert
>
> a recovery umage into an SSD image. Changes are made in place.
>
>> 80
>>
>
Done.


>
>
>
http://codereview.chromium.org/6533015/diff/1/scripts/image_signing/convert_r...
> scripts/image_signing/convert_recovery_to_ssd.sh:16: In-place converts
>
> recovery <image> into an SSD image. With --force, does not ask for
>
>> 80
>>
>
Done.


>
>
http://codereview.chromium.org/6533015/diff/1/scripts/image_signing/convert_r...
> scripts/image_signing/convert_recovery_to_ssd.sh:27: type -P cgpt
> &>/dev/null ||
> Any particular reason why you chose type over which?
>

type is a bash builtin. which is an external command.


>
>
>
http://codereview.chromium.org/6533015/diff/1/scripts/image_signing/convert_r...
> scripts/image_signing/convert_recovery_to_ssd.sh:34: IS_FORCE=$2
> You sort of assume that we are getting --force as the last argument, can
> we also verify it is what you expect it is?
>

Not sure I understand. I do compare the value of IS_FORCE.


>
>
>
http://codereview.chromium.org/6533015/diff/1/scripts/image_signing/convert_r...
> scripts/image_signing/convert_recovery_to_ssd.sh:43:
>
> kerna_offset=$(partoffset ${IMAGE} 2)
> Are we assuming this is in our path or is this in a "bash library"
> somewhere :)


the script uses common_minimal.sh, which contains these.


>
> http://codereview.chromium.org/6533015/
>



-- 
-g

[scottz, 2011-02-24 00:03:46]

LGTM.

I'm glad you liked what I said.