From ece65e0633abb258db037f47c6fa5eec0640f5c3 Mon Sep 17 00:00:00 2001 From: Brian Harring Date: Thu, 30 Aug 2012 13:42:21 -0700 Subject: [PATCH] Add a umount wrapper to suppress gvfsd/trashd breaking umount calls. Specifically, detect if the umount failed due to current access, if so, give it up to 9 more runs (w/ 1s pauses) continuing only if it's still failing due to currently open files. Via this, it should suppress the race of gvfs/trashd looking at quick mounted/umounted pathways. This CL is a two parter; this adds the script, and converts common.sh consumers over to using the override. The next CL will modify the chroot itself to ensure our script gets picked up/used. BUG=chromium-os:23443 TEST=trybot, manul validation. Change-Id: I92dedd91d6133c2063b1e5dbbc1a68366844801d Reviewed-on: https://gerrit.chromium.org/gerrit/32087 Commit-Ready: Brian Harring Reviewed-by: Brian Harring Tested-by: Brian Harring --- clean_loopback_devices | 4 ++-- common.sh | 14 ++++++++++++-- image_to_usb.sh | 2 +- image_to_vm.sh | 4 ++-- make_netboot.sh | 6 +++--- mod_image_for_recovery.sh | 8 ++++---- mount_gpt_image.sh | 10 +++++----- path-overrides/umount | 29 +++++++++++++++++++++++++++++ update_bootloaders.sh | 6 +++--- 9 files changed, 61 insertions(+), 22 deletions(-) create mode 100755 path-overrides/umount diff --git a/clean_loopback_devices b/clean_loopback_devices index f5eda73706..c32ca7f4fa 100755 --- a/clean_loopback_devices +++ b/clean_loopback_devices @@ -39,6 +39,6 @@ if [ "${SURE}" != "y" ]; then exit 1 fi -sudo umount "$OUTPUT_DIR"/*/* 2> /dev/null -sudo umount /tmp/esp.* 2> /dev/null +safe_umount "$OUTPUT_DIR"/*/* 2> /dev/null +safe_umount /tmp/esp.* 2> /dev/null sudo losetup -d /dev/loop* 2> /dev/null diff --git a/common.sh b/common.sh index 7904479470..839f421a5a 100644 --- a/common.sh +++ b/common.sh @@ -562,7 +562,7 @@ safe_umount_tree() { fi # First try to unmount in one shot to speed things up. - if sudo umount -d ${mounts}; then + if safe_umount -d ${mounts}; then return 0 fi @@ -570,12 +570,22 @@ safe_umount_tree() { mounts=$(sub_mounts "$1") warn "Failed to unmount ${mounts}" warn "Doing a lazy unmount" - if ! sudo umount -d -l ${mounts}; then + if ! safe_umount -d -l ${mounts}; then mounts=$(sub_mounts "$1") die "Failed to lazily unmount ${mounts}" fi } + +# Helper; all scripts should use this since it ensures our +# override of umount is used (inside the chroot, it's enforced +# via configuration; outside is the concern). +# Args are passed directly to umount; no sudo args are allowed. +safe_umount() { + sudo "${SCRIPT_ROOT}/path-overrides/umount" "$@" +} + + get_git_id() { git var GIT_COMMITTER_IDENT | sed -e 's/^.*<\(\S\+\)>.*$/\1/' } diff --git a/image_to_usb.sh b/image_to_usb.sh index f3547c3f1a..d2879ddb14 100755 --- a/image_to_usb.sh +++ b/image_to_usb.sh @@ -348,7 +348,7 @@ if [ -b "${FLAGS_to}" ]; then if [ -n "${mount_list}" ]; then echo "Attempting to unmount any mounts on the target device..." for i in ${mount_list}; do - if sudo umount "$i" 2>&1 >/dev/null | grep "not found"; then + if safe_umount "$i" 2>&1 >/dev/null | grep "not found"; then die_notrace "$i needs to be unmounted outside the chroot" fi done diff --git a/image_to_vm.sh b/image_to_vm.sh index 4b236ef6c8..69baea065b 100755 --- a/image_to_vm.sh +++ b/image_to_vm.sh @@ -146,8 +146,8 @@ dd if="${SRC_IMAGE}" of="${TEMP_PMBR}" bs=512 count=1 TEMP_MNT=$(mktemp -d) TEMP_ESP_MNT=$(mktemp -d) cleanup() { - sudo umount "${TEMP_MNT}" - sudo umount "${TEMP_ESP_MNT}" + safe_umount "${TEMP_MNT}" + safe_umount "${TEMP_ESP_MNT}" rmdir "${TEMP_MNT}" "${TEMP_ESP_MNT}" } trap cleanup INT TERM EXIT diff --git a/make_netboot.sh b/make_netboot.sh index 5a4d9ff5c0..d7a0fd15b0 100755 --- a/make_netboot.sh +++ b/make_netboot.sh @@ -68,8 +68,8 @@ fi # Prepare to mount rootfs. umount_loop() { - sudo umount r || true - sudo umount s || true + safe_umount r || true + safe_umount s || true false } @@ -141,7 +141,7 @@ sudo cp "s/dev_image/etc/lsb-factory" "r/${LSB_FACTORY_DIR}" # Clean up mounts. trap - EXIT -sudo umount r s +safe_umount r s sudo rm -rf r s # Generate an initrd fo u-boot to load. diff --git a/mod_image_for_recovery.sh b/mod_image_for_recovery.sh index 99424f9466..4dd2da968d 100755 --- a/mod_image_for_recovery.sh +++ b/mod_image_for_recovery.sh @@ -77,7 +77,7 @@ get_install_vblock() { sudo cp "$stateful_mnt/vmlinuz_hd.vblock" "$out" sudo chown $USER "$out" - sudo umount "$stateful_mnt" + safe_umount "$stateful_mnt" rmdir "$stateful_mnt" switch_to_strict_mode echo "$out" @@ -186,7 +186,7 @@ create_recovery_kernel_image() { # safe. sudo sed -i -e "s/cros_efi/cros_efi kern_b_hash=$kern_hash/g" \ "$efi_dir/efi/boot/grub.cfg" || true - sudo umount "$efi_dir" + safe_umount "$efi_dir" sudo losetup -a | sed 's/^/16651 /' sudo losetup -d "$efi_dev" rmdir "$efi_dir" @@ -243,7 +243,7 @@ install_recovery_kernel() { local esp_mnt=$(mktemp -d) sudo mount -o loop,offset=$((esp_offset * 512)) "$RECOVERY_IMAGE" "$esp_mnt" sudo cp "$vmlinuz" "$esp_mnt/syslinux/vmlinuz.A" || failed=1 - sudo umount "$esp_mnt" + safe_umount "$esp_mnt" rmdir "$esp_mnt" switch_to_strict_mode fi @@ -322,7 +322,7 @@ maybe_resize_stateful() { set +e sudo mount -o loop $small_stateful $new_stateful_mnt sudo cp "$INSTALL_VBLOCK" "$new_stateful_mnt/vmlinuz_hd.vblock" - sudo umount "$new_stateful_mnt" + safe_umount "$new_stateful_mnt" rmdir "$new_stateful_mnt" switch_to_strict_mode diff --git a/mount_gpt_image.sh b/mount_gpt_image.sh index acf62811b7..51eeef5bbb 100755 --- a/mount_gpt_image.sh +++ b/mount_gpt_image.sh @@ -85,13 +85,13 @@ unmount_image() { "${FLAGS_stateful_mountpt}" fix_broken_symlinks "${FLAGS_rootfs_mountpt}" fi - sudo umount "${FLAGS_rootfs_mountpt}/usr/local" - sudo umount "${FLAGS_rootfs_mountpt}/var" + safe_umount "${FLAGS_rootfs_mountpt}/usr/local" + safe_umount "${FLAGS_rootfs_mountpt}/var" if [[ -n "${FLAGS_esp_mountpt}" ]]; then - sudo umount "${FLAGS_esp_mountpt}" + safe_umount "${FLAGS_esp_mountpt}" fi - sudo umount "${FLAGS_stateful_mountpt}" - sudo umount "${FLAGS_rootfs_mountpt}" + safe_umount "${FLAGS_stateful_mountpt}" + safe_umount "${FLAGS_rootfs_mountpt}" switch_to_strict_mode } diff --git a/path-overrides/umount b/path-overrides/umount new file mode 100755 index 0000000000..0a9609d691 --- /dev/null +++ b/path-overrides/umount @@ -0,0 +1,29 @@ +#!/bin/bash + +# Work around a bug on precise where gvfs trash goes looking in mounts +# it shouldn't, resulting in the umount failing when it shouldn't. +# See crosbug.com/23443 for the sordid details. + +suppressed_dir=$(dirname "$(readlink -f "$0")") +cleaned_path="$(echo "$PATH" | sed -e 's+\(^\|:\)/usr/local/sbin\(:\|$\)++g')" +binary="$(PATH="${cleaned_path}" type -P umount)" +if [ $? -ne 0 ]; then + echo "umount: command not found" >&2 + exit 127 +fi + +for x in {1..10}; do + # umount doesn't give use a distinct exit code for device is busy; thus grep + # the output. + output=$(LC_ALL=C "${binary}" "$@" 2>&1) + ret=$? + if [ ${ret} -eq 0 ] || [[ "${output}" == *"device is busy"* ]]; then + # Nothing to do in these scenarios; either ran fine, or it failed in a non + # busy fashion. + break + fi + sleep 1 +done + +echo -n "${output}" >&2 +exit ${ret} diff --git a/update_bootloaders.sh b/update_bootloaders.sh index 6917432ef7..e77873ef33 100755 --- a/update_bootloaders.sh +++ b/update_bootloaders.sh @@ -159,13 +159,13 @@ fi ESP_FS_DIR=$(mktemp -d /tmp/esp.XXXXXX) cleanup() { set +e - if ! sudo umount "${ESP_FS_DIR}"; then + if ! safe_umount "${ESP_FS_DIR}"; then # There is a race condition possible on some ubuntu setups # with mounting and unmounting a device very quickly # Doing a quick sleep/retry as a temporary workaround warn "Initial unmount failed. Possibly crosbug.com/23443. Retrying" sleep 5 - sudo umount "${ESP_FS_DIR}" + safe_umount "${ESP_FS_DIR}" fi if [[ -n "${ESP_DEV}" && -z "${ESP_DEV//\/dev\/loop*}" ]]; then sudo losetup -d "${ESP_DEV}" @@ -211,7 +211,7 @@ if [[ "${FLAGS_arch}" = "x86" || "${FLAGS_arch}" = "amd64" ]]; then # Install the syslinux loader on the ESP image (part 12) so it is ready when # we cut over from rootfs booting (extlinux). if [[ ${FLAGS_install_syslinux} -eq ${FLAGS_TRUE} ]]; then - sudo umount "${ESP_FS_DIR}" + safe_umount "${ESP_FS_DIR}" sudo syslinux -d /syslinux "${ESP_DEV}" # mount again for cleanup to free resource gracefully sudo mount -o ro "${ESP_DEV}" "${ESP_FS_DIR}"