Merge lp:~mwaddel/linaro-image-tools/split-deploy-v2 into lp:linaro-image-tools/11.11

[Matt Waddel]

Added the --no-rootfs, --no-bootfs and --no-part options.

Rearranged the code so these options could be generically added through a DEPLOY_STEPS="functions" mechanism.

These changes should not affect existing scripts or instructions. I tested on the vexpress and on a beagle qemu image deploy.

Incorporated previous code reviews.

[Loïc Minier (lool)]

r146:
- could you rename MMC1/MMC2, and get_mmcs_by_id() as well? you can do that in an additional commit on top
- "Storge" -- but that's ok, was actually fixed in r148
- you could have renamed --mmc, but I guess you didn't mean to break the external interface; ok with me

r147: good catch!

r148:
- testing for BOOTFS_STEP in create_boot_cmd() feels a bit awkard, it seems this code shouldn't be called in the first place!; I think I'd either:
  * test at the top
  * or merge this as a BOOTFS_STEP, that is, define BOOTFS_STEP="create_boot_cmd populate_boot", but I didn't actually check whether that works
  * or arrange for create_boot_cmd to be called from a bootfs step, e.g. from populate_boot
- the "parted -s ${DEVICE} mklabel msdos" bit should have been moved in a separate commit, it's unrelated and hard to read there :-/
- same for "waiting for partitioning to settle" snippet
- replace "if test -z "$MMC1"; then" with "if [ -z "$MMC1" ]; then"; why is this reached at all?
- instead of "for funcs in $DEPLOY_STEPS; do $funcs; done" it would be nicer to name your loop var singular instead of plural ($func)
- could you add a comment in the code to explain why this test is needed? echo "Do not use --no-rootfs or --no-bootfs with --image_file option"; people who ask for nothing get nothing ;-)

r149:
- "!" is a bit hard to read (inverted non-empty test); -z and -n are more common: if [ -z "${ROOTFS_STEP}" ] || [ -z "${BOOTFS_STEP}" ]; then

r150: cute!

[Matt Waddel (mwaddel)]

This branch has been superseded with all the corrections below integrated into:

https://code.launchpad.net/~mwaddel/linaro-image-tools/split-deploy-v3

On 10/18/2010 03:17 PM, Loïc Minier wrote:
> r146:
> - could you rename MMC1/MMC2, and get_mmcs_by_id() as well? you can do that in an additional commit on top

Fixed.

> - "Storge" -- but that's ok, was actually fixed in r148
> - you could have renamed --mmc, but I guess you didn't mean to break the external interface; ok with me

Yep, even though it's not exactly correct I thought it was important to not
break existing instructions.

>
> r147: good catch!
>
> r148:
> - testing for BOOTFS_STEP in create_boot_cmd() feels a bit awkard, it seems this code shouldn't be called in the first place!; I think I'd either:
> * test at the top
> * or merge this as a BOOTFS_STEP, that is, define BOOTFS_STEP="create_boot_cmd populate_boot", but I didn't actually check whether that works

+1, I liked this one the best since it retains the modularity of the script.

> * or arrange for create_boot_cmd to be called from a bootfs step, e.g. from populate_boot
> - the "parted -s ${DEVICE} mklabel msdos" bit should have been moved in a separate commit, it's unrelated and hard to read there :-/
> - same for "waiting for partitioning to settle" snippet

Got carried away in the this commit. There wasn't a nice place to
break up the commit...

> - replace "if test -z "$MMC1"; then" with "if [ -z "$MMC1" ]; then"; why is this reached at all?

I looked at this for a long time and decided this could fail if the
partitioning steps failed. It's unlikely, but possible.

> - instead of "for funcs in $DEPLOY_STEPS; do $funcs; done" it would be nicer to name your loop var singular instead of plural ($func)

Done.

> - could you add a comment in the code to explain why this test is needed? echo "Do not use --no-rootfs or --no-bootfs with --image_file option"; people who ask for nothing get nothing ;-)

Done.

>
> r149:
> - "!" is a bit hard to read (inverted non-empty test); -z and -n are more common: if [ -z "${ROOTFS_STEP}" ] || [ -z "${BOOTFS_STEP}" ]; then

Done.

>
> r150: cute!

[Loïc Minier (lool)]

On Tue, Oct 19, 2010, Matt Waddel wrote:
> > - replace "if test -z "$MMC1"; then" with "if [ -z "$MMC1" ]; then"; why is this reached at all?
>
> I looked at this for a long time and decided this could fail if the
> partitioning steps failed. It's unlikely, but possible.

 The partitioning step should cause the script to fail in this case
 though? (script is set -e)

 Checked your new branch; looks good!

--
Loïc Minier

[Loïc Minier (lool)]

Superseded with:
https://code.launchpad.net/~mwaddel/linaro-image-tools/split-deploy-v3/+merge/38799

Unmerged revisions

=== modified file 'linaro-media-create'
--- linaro-media-create	2010-10-13 21:57:24 +0000
+++ linaro-media-create	2010-10-18 19:21:26 +0000
@@ -19,8 +19,8 @@
 
 set -e
 
-unset MMC MMC1 MMC2 MMC3 IMAGE_FILE HWPACK_FILE UBOOT_FLAVOR KERNEL_ADDR
-unset INITRD_ADDR CREATE_SWAP SWAP_SIZE
+unset DEVICE MMC1 MMC2 IMAGE_FILE HWPACK_FILE UBOOT_FLAVOR KERNEL_ADDR
+unset INITRD_ADDR CREATE_SWAP SWAP_SIZE DEPLOY_STEPS
 
 #Defaults
 RFS=ext3
@@ -31,6 +31,9 @@
 FAT_SIZE=32
 IMAGE_SIZE=2G
 MMC_PART_OFFSET=0
+BOOTFS_STEP="populate_boot"
+ROOTFS_STEP="populate_rootfs"
+PARTITION_STEP="create_partitions"
 
 DIR=$PWD
 TMP_DIR=$(mktemp -d)
@@ -55,7 +58,7 @@
 
 required options:
 --mmc </dev/sdX>
-    Unformated MMC Card
+    Unformatted Storage Device
 <or>
 --image_file <xxx>
     specify name of image file
@@ -108,14 +111,23 @@
 
 --binary <filename>
     specify file used to create the bootable system. Default binary-tar.tar.gz
+
+--no-rootfs
+    do not deploy the root filesystem
+
+--no-bootfs
+    do not deploy the boot filesystem
+
+--no-part
+    reuse existing partitions on the deploy media
 EOF
   exit
 }
 
-check_mmc() {
-  FDISK=$(sudo LC_ALL=C sfdisk -l | grep "[Disk] ${MMC}" | awk '{print $2}')
+check_device() {
+  FDISK=$(sudo LC_ALL=C sfdisk -l | grep "[Disk] ${DEVICE}" | awk '{print $2}')
 
-  if test "-$FDISK-" = "-$MMC:-"
+  if test "-$FDISK-" = "-$DEVICE:-"
   then
     echo ""
     echo "I see..."
@@ -125,12 +137,12 @@
     echo "mount:"
     LC_ALL=C mount | grep -v none | grep "/dev/" --color=never
     echo ""
-    read -p "Are you 100% sure, on selecting [${MMC}] (y/n)? "
+    read -p "Are you 100% sure, on selecting [${DEVICE}] (y/n)? "
     [ "$REPLY" == "y" ] || exit
     echo ""
   else
     echo ""
-    echo "Are you sure? I Don't see [${MMC}], here is what I do see..."
+    echo "Are you sure? I Don't see [${DEVICE}], here is what I do see..."
     echo ""
     echo "sudo sfdisk -l:"
     sudo LC_ALL=C sfdisk -l | grep "[Disk] /dev/" --color=never
@@ -176,8 +188,8 @@
       ;;
     --mmc)
       checkparm "$2"
-      MMC="$2"
-      check_mmc
+      DEVICE="$2"
+      check_device
       ;;
     --image_file)
       checkparm "$2"
@@ -224,10 +236,21 @@
       checkparm "$2"
       BINARY_TARBALL="$2"
       ;;
+    --no-rootfs)
+      ROOTFS_STEP=""
+      ;;
+    --no-bootfs)
+      BOOTFS_STEP=""
+      ;;
+    --no-part)
+      PARTITION_STEP=""
+      ;;
   esac
   shift
 done
 
+DEPLOY_STEPS="$PARTITION_STEP prepare_partitions $BOOTFS_STEP $ROOTFS_STEP"
+
 ensure_command mkimage uboot-mkimage
 ensure_command uuidgen uuid-runtime
 ensure_command parted parted
@@ -257,10 +280,10 @@
 get_mmcs_by_id() {
   if [ ! "${IMAGE_FILE}" ]; then
     for device in /dev/disk/by-id/*; do
-      if [ `realpath $device` = $MMC ]; then
+      if [ `realpath $device` = $DEVICE ]; then
         case "$device" in
           *-part[0-9]*)
-            echo "device $MMC must not be a partition part ($device)" >&2
+            echo "device $DEVICE must not be a partition part ($device)" >&2
             exit 1
           ;;
         esac
@@ -273,8 +296,6 @@
             MMC1=$part
           elif test "$part_no" = $((2 + $MMC_PART_OFFSET)) ; then
             MMC2=$part
-          elif test "$part_no" = $((3 + $MMC_PART_OFFSET)); then
-            MMC3=$part
           fi
         done
         break
@@ -335,6 +356,10 @@
 }
 
 create_boot_cmd() {
+  if [ ! "$BOOTFS_STEP" ]; then
+    return
+  fi
+
   if [ "$IS_LIVE" ]; then
     boot_snippet='boot=casper'
     [ "$IS_LOWMEM" ] && lowmem_opt=only-ubiquity
@@ -373,16 +398,17 @@
   if test -n "$MMC2"; then
     sudo umount ${MMC2} &> /dev/null || true
   fi
-  if [ "${MMC}" ]; then
-    sudo parted -s ${MMC} mklabel msdos
-  fi
 }
 
 create_partitions() {
+  if [ "${DEVICE}" ]; then
+    sudo parted -s ${DEVICE} mklabel msdos
+  fi
+
   if [ "${IMAGE_FILE}" ]; then
     partdev=${IMAGE_FILE}
   else
-    partdev=${MMC}
+    partdev=${DEVICE}
   fi
 
   if [ "$FAT_SIZE" = "32" ]; then
@@ -426,16 +452,36 @@
 
 
 prepare_partitions() {
+
+echo -n "waiting for partitioning to settle ..."
+sync
+sleep 3
+echo "done."
+get_mmcs_by_id
+
+if [ "$BOOTFS_STEP" ]; then
   echo ""
   echo "Formating Boot Partition"
   echo ""
-
-  sudo mkfs.vfat -F ${FAT_SIZE} ${MMC1} -n ${BOOT_LABEL}
-
+  if test -z "$MMC1"; then
+    echo "Partition $MMC1 does not exist"
+    exit 2
+  else
+    sudo mkfs.vfat -F ${FAT_SIZE} ${MMC1} -n ${BOOT_LABEL}
+  fi
+fi
+
+if [ "$ROOTFS_STEP" ]; then
   echo ""
   echo "Formating ${RFS} Partition"
   echo ""
-  sudo mkfs.${RFS} -U "$RFS_UUID" ${MMC2} -L ${RFS_LABEL}
+  if test -z "$MMC2"; then
+    echo "Partition $MMC2 does not exist"
+    exit 2
+  else
+    sudo mkfs.${RFS} -U "$RFS_UUID" ${MMC2} -L ${RFS_LABEL}
+  fi
+fi
 }
 
 populate_boot() {
@@ -675,10 +721,17 @@
 TARGET_BOOT_DEV=/dev/mmcblk0p$(( 1 + $MMC_PART_OFFSET ))
 TARGET_ROOT_DEV=/dev/mmcblk0p$(( 2 + $MMC_PART_OFFSET ))
 
-if [ ! "${MMC}" -a ! "${IMAGE_FILE}" ]; then
+if [ ! "${DEVICE}" -a ! "${IMAGE_FILE}" ]; then
   usage
 fi
 
+if [ "${IMAGE_FILE}" ]; then
+  if [ ! "${ROOTFS_STEP}" ] || [ ! "${BOOTFS_STEP}" ]; then
+    echo "Do not use --no-rootfs or --no-bootfs with --image_file option"
+    exit
+  fi
+fi
+
 unpack_binary_tarball
 if [ "$HWPACK_FILE" ]; then
   install_hwpack
@@ -688,18 +741,8 @@
 create_boot_cmd
 get_mmcs_by_id
 cleanup_sd
-create_partitions
-echo -n "waiting for partitioning to settle ..."
-sync
-sleep 3
-echo "done."
-get_mmcs_by_id
-if test -z "$MMC1" -o -z "$MMC2"; then
-  echo "MMC1: $MMC1 nor MMC2: $MMC2 must be empty"
-  exit 2
-fi
-prepare_partitions
-populate_boot
-populate_rootfs
+for funcs in $DEPLOY_STEPS; do
+  $funcs
+done
 remove_binary_dir