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

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.

« Back to merge proposal