From b9257ee2a88503f3f32aeaacfee1531b1c81035b Mon Sep 17 00:00:00 2001 From: Michael Marineau Date: Thu, 18 Jul 2013 15:27:02 -0700 Subject: [PATCH 1/4] fix(common): Read mounts from /proc/self/mounts Just in case the filesystem view is slightly different. --- common.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common.sh b/common.sh index b5b743d4ac..cbffa10c7c 100644 --- a/common.sh +++ b/common.sh @@ -643,7 +643,7 @@ sub_mounts() { # will). As such, we have to unmount in reverse order to cleanly # unmount submounts (think /dev/pts and /dev). awk -v path=$1 -v len="${#1}" \ - '(substr($2, 1, len) == path) { print $2 }' /proc/mounts | \ + '(substr($2, 1, len) == path) { print $2 }' /proc/self/mounts | \ tac | \ sed -e 's/\\040(deleted)$//' # Hack(zbehan): If a bind mount's source is mysteriously removed, From 67cea270705071801b7b0241d8b02861562ebb08 Mon Sep 17 00:00:00 2001 From: Michael Marineau Date: Thu, 18 Jul 2013 15:52:47 -0700 Subject: [PATCH 2/4] fix(common): Fix okboat and failboat because boat. --- common.sh | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/common.sh b/common.sh index cbffa10c7c..1cd6cbacd7 100644 --- a/common.sh +++ b/common.sh @@ -1162,9 +1162,9 @@ okboat() { . o .. o . o o.o ...oo_ - _[__\___ - __|_o_o_o_o\__ - OK \' ' ' ' ' ' / + _[__\\___ + __|_o_o_o_o\\__ + OK \\' ' ' ' ' ' / ^^^^^^^^^^^^^^^^^^^^ BOAT echo -e "${V_VIDOFF}" @@ -1176,10 +1176,10 @@ failboat() { ' ' ) ) ( - ( .') __/\ - (. /o/` \ - __/o/` \ - FAIL / /o/` / + ( .') __/\\ + (. /o/\` \\ + __/o/\` \\ + FAIL / /o/\` / ^^^^^^^^^^^^^^^^^^^^ BOAT echo -e "${V_VIDOFF}" From e99f088f19fbbe5aa2131ce2144cba4326d2a540 Mon Sep 17 00:00:00 2001 From: Michael Marineau Date: Thu, 18 Jul 2013 16:59:13 -0700 Subject: [PATCH 3/4] 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() { From 99089076af8509d1e8f42d58e96655db09777b40 Mon Sep 17 00:00:00 2001 From: Michael Marineau Date: Fri, 19 Jul 2013 02:41:52 -0400 Subject: [PATCH 4/4] fix(build_image): Don't unmount when the rootfs isn't mounted This function is never called when rootfs is mounted but leaving in a check for it as a just in case sort of thing. --- build_library/disk_layout_util.sh | 4 +++- common.sh | 9 +++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/build_library/disk_layout_util.sh b/build_library/disk_layout_util.sh index 04248d6bdf..d19d1c0a5d 100644 --- a/build_library/disk_layout_util.sh +++ b/build_library/disk_layout_util.sh @@ -61,7 +61,9 @@ run_partition_script() { ;; esac - safe_umount "${root_fs_dir}" + if is_mounted "${root_fs_dir}"; then + safe_umount "${root_fs_dir}" + fi sudo mount -o loop "${root_fs_img}" "${root_fs_dir}" . "${root_fs_dir}/${PARTITION_SCRIPT_PATH}" write_partition_table "${outdev}" "${pmbr_img}" diff --git a/common.sh b/common.sh index a84bd215f4..1a6230b2cd 100644 --- a/common.sh +++ b/common.sh @@ -690,6 +690,15 @@ safe_umount() { fi } +# Check if a single path is mounted. +is_mounted() { + if grep -q "$(readlink -f "$1")" /proc/self/mounts; then + return 0 + else + return 1 + fi +} + get_git_id() { git var GIT_COMMITTER_IDENT | sed -e 's/^.*<\(\S\+\)>.*$/\1/' }