Allow disk image target for copy machine.

Allow disk image target for copy machine.

BUG=10531
TEST=build and imaged a mario 

Change-Id: I60790f178159d707f610ba1668c29179a6c32e61

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

[Nick Sanders, 2010-12-23 04:05:54]

Thoughts? Seem reasonable?

http://codereview.chromium.org/6050006/diff/1/make_factory_package.sh
File make_factory_package.sh (right):

http://codereview.chromium.org/6050006/diff/1/make_factory_package.sh#newcode93
make_factory_package.sh:93: if ! [ -e gptmbr.bin ]; then
Where might be a good location for this file? Should we just always pull it from
the release image?

http://codereview.chromium.org/6050006/diff/1/make_factory_package.sh#newcode99
make_factory_package.sh:99: install_gpt "${outdev}" 0 0 "${pmbrcode}" 0
"${force_full}"
Probably only works in the chroot, not sure that there's any way around this.

http://codereview.chromium.org/6050006/diff/1/make_factory_package.sh#newcode228
make_factory_package.sh:228: # Clean up stale config and data files.
Everythign below this line is unchanged except for indentation, reitveld just is
confused.

[Nick Sanders, 2010-12-23 04:07:43]

http://codereview.chromium.org/6050006/diff/4001/make_factory_package.sh
File make_factory_package.sh (right):

http://codereview.chromium.org/6050006/diff/4001/make_factory_package.sh#newc...
make_factory_package.sh:188: dd of="${dst}" bs=512 seek="${dstoffset}"
conv=notrunc
seems like perf is OK here despite bs=512

[Hung-Te, 2010-12-23 07:02:20]

The two procedures (omaha/disk image) are totally different. I'm wondering if a
new generic script that can do "merge images" are more reasonable.

For example,
./cro_merge_image --ref chromium_image.bin --a_part factory_image.bin --b_part
chromium_image.bin [--c_part xxx]

which will use --ref to create the image, and replace a/b/c parts by params

http://codereview.chromium.org/6050006/diff/1/make_factory_package.sh
File make_factory_package.sh (right):

http://codereview.chromium.org/6050006/diff/1/make_factory_package.sh#newcode39
make_factory_package.sh:39: DEFINE_integer sectors 31277232  "Size of image in
sectors"
I'd prefer to calculate this instead of manually assigning.

http://codereview.chromium.org/6050006/diff/1/make_factory_package.sh#newcode93
make_factory_package.sh:93: if ! [ -e gptmbr.bin ]; then
PMBR from release images should be good enough.
build_image always creates legacy compatible stubs so there's no need to get
local gptmbr.

[Nick Sanders, 2010-12-28 22:52:21]

On 2010/12/23 07:02:20, Hung-Te wrote:
> The two procedures (omaha/disk image) are totally different. I'm wondering if
a
> new generic script that can do "merge images" are more reasonable.
> 

It's likely we will need to hardcode in copying the chromeos-firmwareupdate from
the release into autotest. We'd need to create the initial image at some point
as well, so we should have the make_factory_package interface as a consistant
way to set this up. Underlying, a copy partition, or merge partition script or
library call would be a good idea. I'll move the copy partition into the image
library you've created.

[Nick Sanders, 2010-12-28 22:52:31]

http://codereview.chromium.org/6050006/diff/1/make_factory_package.sh
File make_factory_package.sh (right):

http://codereview.chromium.org/6050006/diff/1/make_factory_package.sh#newcode39
make_factory_package.sh:39: DEFINE_integer sectors 31277232  "Size of image in
sectors"
As we are creating the disk image, we don't know what size it ought to be,
unless passed in. There's nothing to calculate.

http://codereview.chromium.org/6050006/diff/1/make_factory_package.sh#newcode93
make_factory_package.sh:93: if ! [ -e gptmbr.bin ]; then
makes sense. done.

[Tom Wai-Hong Tam, 2010-12-29 05:33:24]

I went through this change. LGTM.

[Hung-Te, 2010-12-29 07:48:19]

http://codereview.chromium.org/6050006/diff/1/make_factory_package.sh
File make_factory_package.sh (right):

http://codereview.chromium.org/6050006/diff/1/make_factory_package.sh#newcode39
make_factory_package.sh:39: DEFINE_integer sectors 31277232  "Size of image in
sectors"
We are creating the disk image from existing partitions, so we can definitely
know the size of each partition; and that can be used to derive a new disk
image.

For example, merge image A by partition 3/5 from image B: new image size =
(image A - partition 3/5 of image A + partition 3/5 of image B).

Anyway that's just an idea - I'm ok with current CL. We can create that kind of
tool (which can tweak images) later.

[Nick Sanders, 2010-12-29 09:28:04]

On Tue, Dec 28, 2010 at 11:48 PM, <hungte@chromium.org> wrote:

>
> http://codereview.chromium.org/6050006/diff/1/make_factory_package.sh
> File make_factory_package.sh (right):
>
>
>
http://codereview.chromium.org/6050006/diff/1/make_factory_package.sh#newcode39
> make_factory_package.sh:39: DEFINE_integer sectors 31277232  "Size of
> image in sectors"
> We are creating the disk image from existing partitions, so we can
> definitely know the size of each partition; and that can be used to
> derive a new disk image.
>

No, the disk image must be laid out as chromeos-install would lay it out,
and be compatible with larger filesystems autoupdated into those partitions.
This sector size is the size of the physical SSD, so we can determine
partition layout. As we're writing into a file, we don't know, other than by
this argument, how big the SSD is.


>
> For example, merge image A by partition 3/5 from image B: new image size
> = (image A - partition 3/5 of image A + partition 3/5 of image B).
>
> Anyway that's just an idea - I'm ok with current CL. We can create that
> kind of tool (which can tweak images) later.


That's not the kind of image we are creating. The expected use is (roughly)
that the binfile will be dd'd verbatim onto the SSD before assembly, and
then once the machine is assembled with the preimaged drive, it will boot
directly into the factory image. Therefore the partition set in this image
will be the final shipping partition layout. The output file size must be
16G, exactly the number of sectors on the physical SSD. Each rootfs/kernel
partition must be the size expected by AU, such that it will be compatible
with future AUs.


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

[Hung-Te, 2010-12-29 09:37:24]

On 2010/12/29 09:28:04, Nick Sanders wrote:
> That's not the kind of image we are creating. The expected use is (roughly)
  Ah, you're right - now I understand how this image will be used.