From 546747b982a096e61b3cdbbb4a6c25a55e25e46b Mon Sep 17 00:00:00 2001 From: David James Date: Tue, 23 Mar 2010 15:19:43 -0700 Subject: [PATCH] Update enter_chroot.sh to exit with error messages when mount/unmount fails. This is necessary for two reasons: 1) It's nice to get an error message when mount/unmount fails 2) set -e mode doesn't have any effect when you're in a subshell Note that these mount/unmount failures do happen regularly in development, so folks who depended on mount/umount failing silently will no longer be able to rely on this and will have to kill the mounts manually. Also fix subtle bugs in regexes for matching mount paths. (E.g. where the regex for $HOME/chroot also matches $HOME/chroot2). TEST=Tested mount/unmount with concurrently open shells. Tested mount/unmount when mount is being used by a process but the lock file does not reflect this. BUG=none Review URL: http://codereview.chromium.org/1211001 --- common.sh | 5 +++++ enter_chroot.sh | 37 +++++++++++++++++++------------------ 2 files changed, 24 insertions(+), 18 deletions(-) diff --git a/common.sh b/common.sh index f869b58e64..13888e37b3 100644 --- a/common.sh +++ b/common.sh @@ -288,3 +288,8 @@ function warn { function error { echo -e "${V_RED}Error....: $1${V_VIDOFF}" } + +function die { + error "$1" + exit 1 +} diff --git a/enter_chroot.sh b/enter_chroot.sh index 3c196a9313..33aad4bea3 100755 --- a/enter_chroot.sh +++ b/enter_chroot.sh @@ -78,25 +78,28 @@ function setup_env { # Mount only if not already mounted MOUNTED_PATH="$(readlink -f "$FLAGS_chroot/proc")" - if [ -z "$(mount | grep -F "on $MOUNTED_PATH")" ] + if [ -z "$(mount | grep -F "on $MOUNTED_PATH ")" ] then - sudo mount none -t proc "$MOUNTED_PATH" + sudo mount none -t proc "$MOUNTED_PATH" || \ + die "Could not mount $MOUNTED_PATH" fi MOUNTED_PATH="$(readlink -f "$FLAGS_chroot/dev/pts")" - if [ -z "$(mount | grep -F "on $MOUNTED_PATH")" ] + if [ -z "$(mount | grep -F "on $MOUNTED_PATH ")" ] then - sudo mount none -t devpts "$MOUNTED_PATH" + sudo mount none -t devpts "$MOUNTED_PATH" || \ + die "Could not mount $MOUNTED_PATH" fi MOUNTED_PATH="$(readlink -f "${FLAGS_chroot}$CHROOT_TRUNK_DIR")" - if [ -z "$(mount | grep -F "on $MOUNTED_PATH")" ] + if [ -z "$(mount | grep -F "on $MOUNTED_PATH ")" ] then - sudo mount --bind "$FLAGS_trunk" "$MOUNTED_PATH" + sudo mount --bind "$FLAGS_trunk" "$MOUNTED_PATH" || \ + die "Could not mount $MOUNTED_PATH" fi MOUNTED_PATH="$(readlink -f "${FLAGS_chroot}${INNER_CHROME_ROOT}")" - if [ -z "$(mount | grep -F "on $MOUNTED_PATH")" ] + if [ -z "$(mount | grep -F "on $MOUNTED_PATH ")" ] then ! CHROME_ROOT="$(readlink -f "$FLAGS_chrome_root")" if [ -z "$CHROME_ROOT" ]; then @@ -111,12 +114,13 @@ function setup_env { echo "$CHROME_ROOT" | \ sudo dd of="${FLAGS_chroot}${CHROME_ROOT_CONFIG}" mkdir -p "$MOUNTED_PATH" - sudo mount --bind "$CHROME_ROOT" "$MOUNTED_PATH" + sudo mount --bind "$CHROME_ROOT" "$MOUNTED_PATH" || \ + die "Could not mount $MOUNTED_PATH" fi fi MOUNTED_PATH="$(readlink -f "${FLAGS_chroot}${INNER_DEPOT_TOOLS_ROOT}")" - if [ -z "$(mount | grep -F "on $MOUNTED_PATH")" ] + if [ -z "$(mount | grep -F "on $MOUNTED_PATH ")" ] then if [ $(which gclient 2>/dev/null) ]; then echo "Mounting depot_tools" @@ -128,16 +132,11 @@ function setup_env { fi fi fi - ) 200>>"$LOCKFILE" + ) 200>>"$LOCKFILE" || die "setup_env failed" } function teardown_env { # Only teardown if we're the last enter_chroot to die - - # We should not return with an error if cleanup has an error. Cleanup is - # best effort only. - set +e - ( flock 200 @@ -165,11 +164,13 @@ function teardown_env { echo "At least one other pid is running in the chroot, so not" echo "tearing down env." else + MOUNTED_PATH=$(readlink -f "$FLAGS_chroot") echo "Unmounting chroot environment." - mount | grep "on $(readlink -f "$FLAGS_chroot")" | awk '{print $3}' \ - | xargs -r -L1 sudo umount + for i in $(mount | grep -F "on $MOUNTED_PATH/" | awk '{print $3}'); do + sudo umount "$i" || die "Failed to umount $i" + done fi - ) 200>>"$LOCKFILE" + ) 200>>"$LOCKFILE" || die "teardown_env failed" } if [ $FLAGS_mount -eq $FLAGS_TRUE ]