Code review comment for lp:~salgado/linaro-image-tools/refactor-lmc

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>