From e99f088f19fbbe5aa2131ce2144cba4326d2a540 Mon Sep 17 00:00:00 2001 From: Michael Marineau Date: Thu, 18 Jul 2013 16:59:13 -0700 Subject: [PATCH] fix(common): Simplify unmount code, die loud and die hard. As-is safe_umount is extremely dangerous. When passed multiple mount points and any one of them fail with a "not mounted" or "doesn't exist" error then any others that fail with a more serious error will be silently ignored. This can cause untold sadness when running deleting a chroot with cros_sdk if /mnt/host/source is left mounted, all your code will be gone. To avoid this situation remove *ALL* this extra logic and die very loudly when umount fails. Due to the way bind mounts interact with this code "not mounted" so when unmounting a full tree we need to still need to gracefully retry when the first umount fails. --- common.sh | 46 ++++++++++------------------------------------ 1 file changed, 10 insertions(+), 36 deletions(-) diff --git a/common.sh b/common.sh index 1cd6cbacd7..a84bd215f4 100644 --- a/common.sh +++ b/common.sh @@ -663,9 +663,9 @@ safe_umount_tree() { return 0 fi - # First try to unmount in one shot to speed things up. - if safe_umount -d ${mounts}; then - return 0 + # First try to unmount, this might fail because of nested binds. + if sudo umount -d ${mounts}; then + return 0; fi # Check whether our mounts were successfully unmounted. @@ -675,45 +675,19 @@ safe_umount_tree() { return 0 fi - # Well that didn't work, so lazy unmount remaining ones. + # Try one more time, this one will die hard if it fails. warn "Failed to unmount ${mounts}" - warn "Doing a lazy unmount" - if ! safe_umount -d -l ${mounts}; then - mounts=$(sub_mounts "$1") - die "Failed to lazily unmount ${mounts}" - fi + safe_umount -d ${mounts} } # Run umount as root. safe_umount() { - local ret=0 - local out="" - set +e - for i in $(seq 1 4); do - out=`$([[ ${UID:-$(id -u)} != 0 ]] && echo sudo) umount "$@" 2>&1` - ret=$? - if [[ $ret -eq 0 ]]; then - set -e - return 0 - fi - # Mount is not found - if [[ `expr index "${out}" "not found"` -ne "0" ]]; then - set -e - return 0 - fi - # Mount is not mounted. - if [[ `expr index "${out}" "not mounted"` -ne "0" ]]; then - set -e - return 0 - fi - sleep 1 - # Mount is actually busy. - if [[ `expr index "${out}" "busy"` -ne "0" ]]; then - continue - fi - done - return $ret + if sudo umount "$@"; then + return 0; + else + failboat safe_umount + fi } get_git_id() {