Merge lp:~salgado/linaro-image-tools/refactor-lmc into lp:linaro-image-tools/11.11

Proposed by Guilherme Salgado
Status: Merged
Merged at revision: 96
Proposed branch: lp:~salgado/linaro-image-tools/refactor-lmc
Merge into: lp:linaro-image-tools/11.11
Diff against target: 163 lines (+29/-29)
1 file modified
linaro-media-create (+29/-29)
To merge this branch: bzr merge lp:~salgado/linaro-image-tools/refactor-lmc
Reviewer Review Type Date Requested Status
James Westby (community) Approve
Review via email: mp+36466@code.launchpad.net

Description of the change

This branch removes some unneeded code which was getting in the way of the refactoring I want to do. It also uses some constants to avoid repetition and make some things more obvious

To post a comment you must log in.
Revision history for this message
Guilherme Salgado (salgado) wrote :

- ln -sf binary/${parts_dir}/initrd.img-* .
- ln -sf binary/${parts_dir}/vmlinuz-* .

This is the unneeded code; those symlinks were not used by anything.

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

Looks good to me.

Thanks,

James

review: Approve
Revision history for this message
Peter Maydell (pmaydell) wrote :

- sudo dd if=/dev/zero of=${DIR}/disk/SWAP.swap bs=1M count=$SWAP_SIZE
- sudo mkswap ${DIR}/disk/SWAP.swap
- echo "/SWAP.swap none swap sw 0 0" | sudo tee -a ${DIR}/disk/etc/fstab
+ sudo dd if=/dev/zero of=${ROOT_DIR}/SWAP.swap bs=1M count=$SWAP_SIZE
+ sudo mkswap ${ROOT_DIR}/SWAP.swap
+ echo "/SWAP.swap none swap sw 0 0" | sudo tee -a ${ROOT_DIR}/etc/fstab

Shouldn't these (and other) uses of ROOT_DIR be ROOT_DISK instead? I can't see anywhere initialising ROOT_DIR...

+ mkdir -p ${ROOT_DISK} || true

I know they were in the previous version, but I don't think the "|| true" on "mkdir -p" or "rm -rf" is right. mkdir -p/rm -rf already exit 0 for the expected cases of "dir already exists" and "dir already deleted", and silently ignoring unexpected errors seems likely to just make us fail more obscurely later.

Revision history for this message
Guilherme Salgado (salgado) wrote :

On Thu, 2010-09-23 at 16:31 +0000, Peter Maydell wrote:
> - sudo dd if=/dev/zero of=${DIR}/disk/SWAP.swap bs=1M count=$SWAP_SIZE
> - sudo mkswap ${DIR}/disk/SWAP.swap
> - echo "/SWAP.swap none swap sw 0 0" | sudo tee -a ${DIR}/disk/etc/fstab
> + sudo dd if=/dev/zero of=${ROOT_DIR}/SWAP.swap bs=1M count=$SWAP_SIZE
> + sudo mkswap ${ROOT_DIR}/SWAP.swap
> + echo "/SWAP.swap none swap sw 0 0" | sudo tee -a ${ROOT_DIR}/etc/fstab
>
> Shouldn't these (and other) uses of ROOT_DIR be ROOT_DISK instead? I can't see anywhere initialising ROOT_DIR...
>

Good catch, that should've been ROOT_DISK.

>
> + mkdir -p ${ROOT_DISK} || true
>
> I know they were in the previous version, but I don't think the "|| true" on "mkdir -p" or "rm -rf" is right. mkdir -p/rm -rf already exit 0 for the expected cases of "dir already exists" and "dir already deleted", and silently ignoring unexpected errors seems likely to just make us fail more obscurely later.

That makes total sense; I'll remove the "|| true".

Thanks!

--
Guilherme Salgado <https://launchpad.net/~salgado>

97. By Guilherme Salgado

Fix some places in l-m-c that were using ROOT_DIR when they meant ROOT_DISC and some changes suggested by Peter Maydell

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'linaro-media-create'
2--- linaro-media-create 2010-09-20 15:52:54 +0000
3+++ linaro-media-create 2010-09-23 18:09:43 +0000
4@@ -35,6 +35,8 @@
5 IMAGE_SIZE=2G
6
7 DIR=$PWD
8+BOOT_DISK=${DIR}/boot-disc
9+ROOT_DISK=${DIR}/root-disc
10
11 ensure_command() {
12 # ensure_command foo foo-package
13@@ -297,6 +299,7 @@
14 install_hwpack_into_tarball() {
15 ensure_command qemu-arm-static qemu-arm-static
16
17+
18 tmp_dir=$(mktemp -d)
19 chroot=${tmp_dir}/binary
20 # Make sure the tmp_dir where we unpack the tarball is removed when this
21@@ -346,9 +349,8 @@
22 boot_snippet='root=UUID='${RFS_UUID}
23 fi
24
25- rm -rf ${DIR}/binary/ || true
26- rm -rf binary/initrd.img-* || true
27- rm -rf binary/vmlinuz-* || true
28+ # Remove the binary/ directory so that previous runs don't interfere here.
29+ rm -rf binary/
30
31 extras=
32 if [ "$DEVIMAGE" = beagle ]; then
33@@ -356,9 +358,6 @@
34 fi
35 tar -xf $BINARY_TARBALL binary/${parts_dir} $extras
36
37- ln -sf binary/${parts_dir}/initrd.img-* .
38- ln -sf binary/${parts_dir}/vmlinuz-* .
39-
40 if [ "${IMAGE_FILE}" ]; then
41 cat > binary/boot.cmd << BOOTCMD
42 setenv bootcmd 'mmc init; fatload mmc 0:1 0x80000000 uImage; bootm 0x80000000'
43@@ -452,14 +451,14 @@
44 parts_dir=boot
45 fi
46
47- mkdir -p ${DIR}/disk || true
48- sudo mount ${MMC1} ${DIR}/disk
49+ mkdir -p ${BOOT_DISK} || true
50+ sudo mount ${MMC1} ${BOOT_DISK}
51 case "$DEVIMAGE" in
52 beagle|igep)
53 if [ "$DEVIMAGE" = "beagle" ]; then
54 if [ -e binary/${MLO_FILE} ] && [ -e binary/${UBOOT_FILE} ]; then
55- sudo cp -v binary/${MLO_FILE} ${DIR}/disk/MLO
56- sudo cp -v binary/${UBOOT_FILE} ${DIR}/disk/u-boot.bin
57+ sudo cp -v binary/${MLO_FILE} ${BOOT_DISK}/MLO
58+ sudo cp -v binary/${UBOOT_FILE} ${BOOT_DISK}/u-boot.bin
59 fi
60 fi
61 sync
62@@ -468,25 +467,25 @@
63
64 sudo mkimage -A arm -O linux -T kernel -C none -a 0x80008000 \
65 -e 0x80008000 -n Linux -d "${DIR}/binary/${parts_dir}"/vmlinuz-*omap \
66- "${DIR}/disk/uImage"
67+ "${BOOT_DISK}/uImage"
68
69 sudo mkimage -A arm -O linux -T ramdisk -C none -a 0 \
70 -e 0 -n initramfs -d "${DIR}/binary/${parts_dir}"/initrd.img-*-omap \
71- "${DIR}/disk/uInitrd"
72+ "${BOOT_DISK}/uInitrd"
73
74 sudo mkimage -A arm -O linux -T script -C none -a 0 \
75 -e 0 -n "$CODENAME 10.05" -d "${DIR}/binary/boot.cmd" \
76- "${DIR}/disk/boot.scr"
77- sudo cp -v ${DIR}/disk/boot.scr ${DIR}/disk/boot.ini
78+ "${BOOT_DISK}/boot.scr"
79+ sudo cp -v ${BOOT_DISK}/boot.scr ${BOOT_DISK}/boot.ini
80 ;;
81 vexpress)
82- sudo tar --strip-components=5 -C ${DIR}/disk/ -xf $BINARY_TARBALL binary/usr/lib/u-boot/ca9x4_ct_vxp/u-boot.bin
83+ sudo tar --strip-components=5 -C ${BOOT_DISK}/ -xf $BINARY_TARBALL binary/usr/lib/u-boot/ca9x4_ct_vxp/u-boot.bin
84 sudo mkimage -A arm -O linux -T kernel -C none -a 0x60008000 -e 0x60008000 \
85- -n "Linux" -d ${DIR}/binary/${parts_dir}/vmlinuz*-vexpress ${DIR}/disk/uImage
86+ -n "Linux" -d ${DIR}/binary/${parts_dir}/vmlinuz*-vexpress ${BOOT_DISK}/uImage
87 sudo mkimage -A arm -O linux -T ramdisk -C none -a 0x81000000 \
88 -e 0x81000000 -n "initramfs" -d \
89 ${DIR}/binary/${parts_dir}/initrd.img-*-linaro-vexpress \
90- "${DIR}"/disk/uInitrd
91+ "${BOOT_DISK}"/uInitrd
92 ;;
93 *)
94 echo "Set --dev parameter: --dev <beagle|igep|vexpress>" 1>&2
95@@ -495,12 +494,12 @@
96 ;;
97 esac
98
99- cd ${DIR}/disk/
100+ cd ${BOOT_DISK}/
101 sync
102 sync
103 cd ${DIR}/
104
105- sudo umount ${DIR}/disk || true
106+ sudo umount ${BOOT_DISK} || true
107 }
108
109 populate_rootfs() {
110@@ -508,12 +507,13 @@
111 echo "Populating rootfs Partition"
112 echo "Be patient, this may take a few minutes"
113 echo ""
114- sudo mount ${MMC2} ${DIR}/disk
115+ mkdir -p ${ROOT_DISK}
116+ sudo mount ${MMC2} ${ROOT_DISK}
117
118- sudo tar -xf $BINARY_TARBALL --strip-components=1 -C disk/
119+ sudo tar -xf $BINARY_TARBALL --strip-components=1 -C $ROOT_DISK
120
121 ## add fstab entry for rootfs and boot
122- echo "UUID=${RFS_UUID} / ${RFS} errors=remount-ro 0 1 " | sudo tee -a ${DIR}/disk/etc/fstab
123+ echo "UUID=${RFS_UUID} / ${RFS} errors=remount-ro 0 1 " | sudo tee -a ${ROOT_DISK}/etc/fstab
124
125 if [ "$CREATE_SWAP" ] ; then
126
127@@ -521,14 +521,14 @@
128 echo "Creating SWAP File"
129 echo ""
130
131- SPACE_LEFT=$(df ${DIR}/disk/ | grep ${MMC2} | awk '{print $4}')
132+ SPACE_LEFT=$(df ${ROOT_DISK} | grep ${MMC2} | awk '{print $4}')
133
134 let SIZE=$SWAP_SIZE*1024
135
136 if [ $SPACE_LEFT -ge $SIZE ] ; then
137- sudo dd if=/dev/zero of=${DIR}/disk/SWAP.swap bs=1M count=$SWAP_SIZE
138- sudo mkswap ${DIR}/disk/SWAP.swap
139- echo "/SWAP.swap none swap sw 0 0" | sudo tee -a ${DIR}/disk/etc/fstab
140+ sudo dd if=/dev/zero of=${ROOT_DISK}/SWAP.swap bs=1M count=$SWAP_SIZE
141+ sudo mkswap ${ROOT_DISK}/SWAP.swap
142+ echo "/SWAP.swap none swap sw 0 0" | sudo tee -a ${ROOT_DISK}/etc/fstab
143 else
144 echo "SWAP file bigger then whats left on partition"
145 fi
146@@ -538,14 +538,14 @@
147 echo "Creating /etc/flash-kernel.conf"
148 echo ""
149
150- echo "UBOOT_PART=/dev/mmcblk0p1" | sudo tee ${DIR}/disk/etc/flash-kernel.conf >/dev/null
151+ echo "UBOOT_PART=/dev/mmcblk0p1" | sudo tee ${ROOT_DISK}/etc/flash-kernel.conf >/dev/null
152
153- cd ${DIR}/disk/
154+ cd ${ROOT_DISK}
155 sync
156 sync
157 cd ${DIR}/
158
159- sudo umount ${DIR}/disk || true
160+ sudo umount ${ROOT_DISK} || true
161 }
162
163 calculatesize() {

Subscribers

People subscribed via source and target branches