Linaro Image Tools

Merge lp:~dave-martin-arm/linaro-image-tools/miscellaneous-fixes into lp:linaro-image-tools/11.11

  1. miscellaneous-fixes
  2. Merge into trunk
Proposed by Dave Martin
Status: Merged
Merged at revision: 136
Proposed branch: lp:~dave-martin-arm/linaro-image-tools/miscellaneous-fixes
Merge into: lp:linaro-image-tools/11.11
Diff against target: 175 lines (+31/-28)
2 files modified
linaro-hwpack-install (+1/-1)
linaro-media-create (+30/-27)
To merge this branch: bzr merge lp:~dave-martin-arm/linaro-image-tools/miscellaneous-fixes
Reviewer Review Type Date Requested Status
James Westby (community) quoting Approve
Linaro Maintainers Pending
Review via email: mp+37950@code.launchpad.net

Description of the change

Minor fixes:
 * make argument quoting a bit more robust so that if the argument to --image_file, --hwpack or --binary contains whitespace or metacharacters, things still work
 * add missing ensure_command dependency for qemu-img.
 * fix an fdisk -l output parsing issue for partitions with odd sector counts
 * fix an off-by-one-sector partition sizing error

To post a comment you must log in.
lp:~dave-martin-arm/linaro-image-tools/miscellaneous-fixes updated
137. By Dave Martin

Fixed misparse of fdisk output.

When a partition has an odd number of sectors, the size field printed
by fdisk is "<size in KiB>+", not "<size in KiB>".

This causes a syntax error when the extracted field is pasted into
an expression to evaluate.

The fix is to subtract the start sector index from end end sector index
(as is already done for the linux partition).

138. By Dave Martin

Fix off-by-one-sector error determining partition sizes from fdisk output.

fdisk lists start sector and end sector indices: the first sector
not in a given partition is <end sector>+1. Thus the correct sector count
for the partition is <end sector>+1-<start sector>.

Revision history for this message
James Westby (james-w) wrote :

106 + sudo cp "$LINARO_HWPACK_INSTALL" ${chroot}/usr/bin

Might as well quote the second argument too.

121 + sudo rm -f ${chroot}/"$(basename "$HWPACK_FILE")"

I would prefer "${chroot}/$(basename "$HWPACK_FILE")"

I haven't reviewed the functional changes.

Thanks,

James

review: Approve (quoting)
Revision history for this message
Matt Waddel (mwaddel) wrote :

Added James' 2 suggestions
Tested on a qemu image deploy and an mmc card deploy with a file that included a space (ie. HW vexpress.tar.gz)
Merged with trunk

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'linaro-hwpack-install'
--- linaro-hwpack-install 2010-09-20 16:09:22 +0000
+++ linaro-hwpack-install 2010-10-08 14:40:14 +0000
@@ -40,7 +40,7 @@
40elif [ $# -eq 1 ]; then40elif [ $# -eq 1 ]; then
41 HWPACK_TARBALL=$141 HWPACK_TARBALL=$1
42elif [ $# -eq 2 ]; then42elif [ $# -eq 2 ]; then
43 if [ $1 != "--install-latest" ]; then43 if [ "$1" != "--install-latest" ]; then
44 die "Unknown argument: $1 \n$usage_msg"44 die "Unknown argument: $1 \n$usage_msg"
45 fi45 fi
46 INSTALL_LATEST="yes"46 INSTALL_LATEST="yes"
4747
=== modified file 'linaro-media-create'
--- linaro-media-create 2010-10-07 15:41:49 +0000
+++ linaro-media-create 2010-10-08 14:40:14 +0000
@@ -164,42 +164,42 @@
164164
165# parse commandline options165# parse commandline options
166while [ ! -z "$1" ]; do166while [ ! -z "$1" ]; do
167 case $1 in167 case "$1" in
168 -h|--help)168 -h|--help)
169 usage169 usage
170 ;;170 ;;
171 --hwpack)171 --hwpack)
172 checkparm $2172 checkparm "$2"
173 HWPACK_FILE="$2"173 HWPACK_FILE="$2"
174 ;;174 ;;
175 --mmc)175 --mmc)
176 checkparm $2176 checkparm "$2"
177 MMC="$2"177 MMC="$2"
178 check_mmc178 check_mmc
179 ;;179 ;;
180 --image_file)180 --image_file)
181 checkparm $2181 checkparm "$2"
182 IMAGE_FILE="$2"182 IMAGE_FILE="$2"
183 ;;183 ;;
184 --image_size)184 --image_size)
185 checkparm $2185 checkparm "$2"
186 IMAGE_SIZE=$2186 IMAGE_SIZE=$2
187 ;;187 ;;
188 --rootfs)188 --rootfs)
189 checkparm $2189 checkparm "$2"
190 RFS="$2"190 RFS="$2"
191 check_fs_type191 check_fs_type
192 ;;192 ;;
193 --boot_label)193 --boot_label)
194 checkparm $2194 checkparm "$2"
195 BOOT_LABEL="$2"195 BOOT_LABEL="$2"
196 ;;196 ;;
197 --rfs_label)197 --rfs_label)
198 checkparm $2198 checkparm "$2"
199 RFS_LABEL="$2"199 RFS_LABEL="$2"
200 ;;200 ;;
201 --swap_file)201 --swap_file)
202 checkparm $2202 checkparm "$2"
203 SWAP_SIZE="$2"203 SWAP_SIZE="$2"
204 CREATE_SWAP=1204 CREATE_SWAP=1
205 ;;205 ;;
@@ -211,15 +211,15 @@
211 IS_LOWMEM=1211 IS_LOWMEM=1
212 ;;212 ;;
213 --console)213 --console)
214 checkparm $2214 checkparm "$2"
215 consoles="$consoles $2"215 consoles="$consoles $2"
216 ;;216 ;;
217 --dev)217 --dev)
218 checkparm $2218 checkparm "$2"
219 DEVIMAGE=$2219 DEVIMAGE=$2
220 ;;220 ;;
221 --binary)221 --binary)
222 checkparm $2222 checkparm "$2"
223 BINARY_TARBALL="$2"223 BINARY_TARBALL="$2"
224 ;;224 ;;
225 esac225 esac
@@ -253,7 +253,7 @@
253RFS_UUID=`uuidgen -r`253RFS_UUID=`uuidgen -r`
254254
255get_mmcs_by_id() {255get_mmcs_by_id() {
256 if [ ! ${IMAGE_FILE} ]; then256 if [ ! "${IMAGE_FILE}" ]; then
257 for device in /dev/disk/by-id/*; do257 for device in /dev/disk/by-id/*; do
258 if [ `realpath $device` = $MMC ]; then258 if [ `realpath $device` = $MMC ]; then
259 case "$device" in259 case "$device" in
@@ -304,15 +304,16 @@
304 ;;304 ;;
305 *)305 *)
306 ensure_command qemu-arm-static qemu-arm-static306 ensure_command qemu-arm-static qemu-arm-static
307 ensure_command qemu-img qemu-kvm
307 sudo cp /usr/bin/qemu-arm-static ${chroot}/usr/bin308 sudo cp /usr/bin/qemu-arm-static ${chroot}/usr/bin
308 ;;309 ;;
309 esac310 esac
310 sudo cp $LINARO_HWPACK_INSTALL ${chroot}/usr/bin311 sudo cp "$LINARO_HWPACK_INSTALL" ${chroot}/usr/bin
311 sudo cp "$HWPACK_FILE" "$chroot"312 sudo cp "$HWPACK_FILE" "$chroot"
312313
313 # Actually install the hwpack.314 # Actually install the hwpack.
314 sudo mount proc ${chroot}/proc -t proc315 sudo mount proc ${chroot}/proc -t proc
315 sudo LC_ALL=C chroot "$chroot" linaro-hwpack-install /$(basename "$HWPACK_FILE")316 sudo LC_ALL=C chroot "$chroot" linaro-hwpack-install /"$(basename "$HWPACK_FILE")"
316317
317 # Revert some changes we did to the rootfs as we don't want them in the318 # Revert some changes we did to the rootfs as we don't want them in the
318 # image.319 # image.
@@ -323,14 +324,14 @@
323 sudo rm -f ${chroot}/usr/bin/qemu-arm-static324 sudo rm -f ${chroot}/usr/bin/qemu-arm-static
324 fi325 fi
325 sudo rm -f ${chroot}/usr/bin/linaro-hwpack-install326 sudo rm -f ${chroot}/usr/bin/linaro-hwpack-install
326 sudo rm -f ${chroot}/$(basename "$HWPACK_FILE")327 sudo rm -f ${chroot}/"$(basename "$HWPACK_FILE")"
327}328}
328329
329unpack_binary_tarball() {330unpack_binary_tarball() {
330 # Remove the binary/ directory so that previous runs don't interfere here.331 # Remove the binary/ directory so that previous runs don't interfere here.
331 remove_binary_dir332 remove_binary_dir
332333
333 sudo tar -xf $BINARY_TARBALL334 sudo tar -xf "$BINARY_TARBALL"
334}335}
335336
336create_boot_cmd() {337create_boot_cmd() {
@@ -418,14 +419,16 @@
418 ) | sudo sfdisk -D -H $HEADS -S $SECTORS $CYLINDER_ARG $partdev419 ) | sudo sfdisk -D -H $HEADS -S $SECTORS $CYLINDER_ARG $partdev
419420
420 if [ "${IMAGE_FILE}" ]; then421 if [ "${IMAGE_FILE}" ]; then
421 VFATOFFSET=$(($(LC_ALL=C fdisk -l -u $IMAGE_FILE | grep FAT | awk '{print $3}')*512))422 VFATOFFSET=$(($(LC_ALL=C fdisk -l -u "$IMAGE_FILE" | grep FAT | awk '{print $3}')*512))
422 VFATSIZE=$(($(LC_ALL=C fdisk -l -u $IMAGE_FILE | grep FAT | awk '{print $5}')*1024))423 VFATSIZE2=$(($(LC_ALL=C fdisk -l -u "$IMAGE_FILE" | grep FAT | awk '{print $4}')))
423 ROOTOFFSET=$(($(LC_ALL=C fdisk -l -u $IMAGE_FILE | grep Linux | awk '{print $2}')*512))424 VFATSIZE1=$(($(LC_ALL=C fdisk -l -u "$IMAGE_FILE" | grep FAT | awk '{print $3}')))
424 ROOTSIZE2=$(($(LC_ALL=C fdisk -l -u $IMAGE_FILE | grep Linux | awk '{print $3}')))425 VFATSIZE=$((((VFATSIZE2+1-VFATSIZE1)/2)*1024))
425 ROOTSIZE1=$(($(LC_ALL=C fdisk -l -u $IMAGE_FILE | grep Linux | awk '{print $2}')))426 ROOTOFFSET=$(($(LC_ALL=C fdisk -l -u "$IMAGE_FILE" | grep Linux | awk '{print $2}')*512))
426 ROOTSIZE=$((((ROOTSIZE2-ROOTSIZE1)/2)*1024))427 ROOTSIZE2=$(($(LC_ALL=C fdisk -l -u "$IMAGE_FILE" | grep Linux | awk '{print $3}')))
427 MMC1=$(sudo losetup -f --show $IMAGE_FILE --offset $VFATOFFSET --sizelimit $VFATSIZE)428 ROOTSIZE1=$(($(LC_ALL=C fdisk -l -u "$IMAGE_FILE" | grep Linux | awk '{print $2}')))
428 MMC2=$(sudo losetup -f --show $IMAGE_FILE --offset $ROOTOFFSET --sizelimit $ROOTSIZE)429 ROOTSIZE=$((((ROOTSIZE2+1-ROOTSIZE1)/2)*1024))
430 MMC1=$(sudo losetup -f --show "$IMAGE_FILE" --offset $VFATOFFSET --sizelimit $VFATSIZE)
431 MMC2=$(sudo losetup -f --show "$IMAGE_FILE" --offset $ROOTOFFSET --sizelimit $ROOTSIZE)
429 fi432 fi
430}433}
431434
@@ -591,7 +594,7 @@
591}594}
592595
593setup_image() {596setup_image() {
594 sudo qemu-img create -f raw $IMAGE_FILE $IMAGE_SIZE597 sudo qemu-img create -f raw "$IMAGE_FILE" $IMAGE_SIZE
595}598}
596599
597remove_binary_dir() {600remove_binary_dir() {
@@ -713,7 +716,7 @@
713716
714if [ "${IMAGE_FILE}" ]; then717if [ "${IMAGE_FILE}" ]; then
715 echo "Create ${IMAGE_FILE}.gz"718 echo "Create ${IMAGE_FILE}.gz"
716 gzip -f ${IMAGE_FILE} > ${IMAGE_FILE}.gz719 gzip -f "${IMAGE_FILE}" > "${IMAGE_FILE}".gz
717fi720fi
718721
719remove_binary_dir722remove_binary_dir

Subscribers

People subscribed via source and target branches