make_dev_ssd: more sanity checks

make_dev_ssd: more sanity checks

make_dev_ssd is a powerful command bug may confuse developers by its behavior.
Adding sanity checks can prevent developers throwing their system into
un-bootable ste.

BUG=chromium-os:14219
TEST=./make_dev_ssd.sh -i some_images; # no check, pass
     ./make_dev_ssd.sh # see alert for live partitions
     (with non-developer firmware) ./make_dev_ssd.sh --partitions 2 # seeing firmware warning
     (with developer firmware) ./make_dev_ssd.sh --partitions 2 # pass, no warning
     (with dev-signed normal firmware) ./make_dev_ssd.sh --partitions 2 # pass, no warning
     ./make_dev_ssd.sh -f # seeing 5 second condown alert screen and then continue

Change-Id: I7ae134c03899b2dc4a6d95f6d9091c38e6f8cf65

R=rspangler@chromium.org

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

[Hung-Te, 2011-04-19 01:56:05]

Hi, any comments for this? Thanks

[Randall Spangler, 2011-04-19 20:40:12]

http://codereview.chromium.org/6870026/diff/1/scripts/image_signing/make_dev_...
File scripts/image_signing/make_dev_ssd.sh (right):

http://codereview.chromium.org/6870026/diff/1/scripts/image_signing/make_dev_...
scripts/image_signing/make_dev_ssd.sh:8: # of a kernels on an disk image
(usually for SSD but also works for USB).
'a kernels' -> 'kernels'

http://codereview.chromium.org/6870026/diff/1/scripts/image_signing/make_dev_...
scripts/image_signing/make_dev_ssd.sh:305: if [ "$ORIGINAL_PARTITIONS" !=
"$DEFAULT_PARTITIONS" ]; then
What does this check do?

(and what if I explicitly specify --partitions='2 4'?  Wouldn't that skip this
check even though I did explicitly specify partitions?)

http://codereview.chromium.org/6870026/diff/1/scripts/image_signing/make_dev_...
scripts/image_signing/make_dev_ssd.sh:341: # 130 is the magic number for DEV key
Please add more explanation on how 130 was calculated.  Preferably using terms
which match how it's defined in vboot_struct.h, so that we have a better chance
of noticing that we need to update make_dev_ssd.sh if we refactor the key
format.

Or better, can you just compare the root key with the one in
/usr/share/vboot/devkeys?

[Hung-Te, 2011-04-20 01:30:20]

Thanks for the review. Please take a look again, thanks.

http://codereview.chromium.org/6870026/diff/1/scripts/image_signing/make_dev_...
File scripts/image_signing/make_dev_ssd.sh (right):

http://codereview.chromium.org/6870026/diff/1/scripts/image_signing/make_dev_...
scripts/image_signing/make_dev_ssd.sh:8: # of a kernels on an disk image
(usually for SSD but also works for USB).
On 2011/04/19 20:40:12, Randall Spangler wrote:
> 'a kernels' -> 'kernels'

Done.

http://codereview.chromium.org/6870026/diff/1/scripts/image_signing/make_dev_...
scripts/image_signing/make_dev_ssd.sh:305: if [ "$ORIGINAL_PARTITIONS" !=
"$DEFAULT_PARTITIONS" ]; then
On 2011/04/19 20:40:12, Randall Spangler wrote:
> What does this check do?
> (and what if I explicitly specify --partitions='2 4'?  
 Changed to compare with empty (new default value).

http://codereview.chromium.org/6870026/diff/1/scripts/image_signing/make_dev_...
scripts/image_signing/make_dev_ssd.sh:341: # 130 is the magic number for DEV key
On 2011/04/19 20:40:12, Randall Spangler wrote:
> Or better, can you just compare the root key with the one in
> /usr/share/vboot/devkeys?
  That's  a great idea. 
  However the file sizes are different now, so I'm marking it as TODO.

[Randall Spangler, 2011-04-20 19:31:57]

http://codereview.chromium.org/6870026/diff/1/scripts/image_signing/make_dev_...
File scripts/image_signing/make_dev_ssd.sh (right):

http://codereview.chromium.org/6870026/diff/1/scripts/image_signing/make_dev_...
scripts/image_signing/make_dev_ssd.sh:305: if [ "$ORIGINAL_PARTITIONS" !=
"$DEFAULT_PARTITIONS" ]; then
On 2011/04/20 01:30:20, Hung-Te wrote:
> On 2011/04/19 20:40:12, Randall Spangler wrote:
> > What does this check do?
> > (and what if I explicitly specify --partitions='2 4'?  
>  Changed to compare with empty (new default value).
>  

Isn't the default above still '2 4'?

[Hung-Te, 2011-04-21 01:58:35]

http://codereview.chromium.org/6870026/diff/1/scripts/image_signing/make_dev_...
File scripts/image_signing/make_dev_ssd.sh (right):

http://codereview.chromium.org/6870026/diff/1/scripts/image_signing/make_dev_...
scripts/image_signing/make_dev_ssd.sh:305: if [ "$ORIGINAL_PARTITIONS" !=
"$DEFAULT_PARTITIONS" ]; then
On 2011/04/20 19:31:57, Randall Spangler wrote:
> Isn't the default above still '2 4'?
The default partitions is '2 4', but the "default" parameter is now empty
(ORIGINAL_PARTITIONS).

And the new check code is if [ "$ORIGINAL_PARTITIONS" != "" ]

If user execute this command without giving --partitions, he will get
[ "" != "" ];

If user manually assigned --partitions "2 4", he will get
[ "2 4" != "" ];

[Randall Spangler, 2011-04-21 23:01:10]

LGTM