From 0103b5913816544c4ed9b75369a1fc2fe5e1d6ac Mon Sep 17 00:00:00 2001 From: David James Date: Wed, 6 Jun 2012 11:31:52 -0700 Subject: [PATCH] Clean up update_bootloaders.sh to avoid sleeping. Right now update_bootloaders.sh occasionally sleeps for 5 seconds if the developer machine has gvfs-gdu-volume-monitor installed. This sleep was only necessary because the script pointlessly mounted the device and then immediately unmounted it, thus triggering a race condition. In this commit, I've removed the pointless mount/unmount, thus avoiding this race. This removes the need for the sleep and for retrying the umount. I also cleaned up the error checking so that failures to cleanup fail the whole script now. BUG=none TEST=Run build_image.sh several times, verify it doesn't sleep for 5 seconds anymore. Also run remote trybot runs. Change-Id: Iaa715e9644292f97356a341d17b147be2a5178d9 Reviewed-on: https://gerrit.chromium.org/gerrit/24632 Commit-Ready: David James Reviewed-by: David James Tested-by: David James --- update_bootloaders.sh | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/update_bootloaders.sh b/update_bootloaders.sh index 1f6bbd9ec1..5aa9b9326b 100755 --- a/update_bootloaders.sh +++ b/update_bootloaders.sh @@ -1,6 +1,6 @@ #!/bin/bash -# Copyright (c) 2010 The Chromium OS Authors. All rights reserved. +# Copyright (c) 2012 The Chromium OS Authors. All rights reserved. # Use of this source code is governed by a BSD-style license that can be # found in the LICENSE file. @@ -161,19 +161,16 @@ fi ESP_FS_DIR=$(mktemp -d /tmp/esp.XXXXXX) cleanup() { - set +e - if ! sudo 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}" + local failed=() + if [[ -n "${ESP_FS_DIR}" ]]; then + sudo umount "${ESP_FS_DIR}" && rm -rf "${ESP_FS_DIR}" || failed+=( umount ) fi if [[ -n "${ESP_DEV}" && -z "${ESP_DEV//\/dev\/loop*}" ]]; then - sudo losetup -d "${ESP_DEV}" + sudo losetup -d "${ESP_DEV}" || failed+=( losetup ) + fi + if [[ -n "${failed[*]}" ]]; then + die "Cleanup failed in ${failed[*]}" fi - rm -rf "${ESP_FS_DIR}" } trap cleanup EXIT sudo mount "${ESP_DEV}" "${ESP_FS_DIR}" @@ -215,9 +212,9 @@ if [[ "${FLAGS_arch}" = "x86" || "${FLAGS_arch}" = "amd64" ]]; then # we cut over from rootfs booting (extlinux). if [[ ${FLAGS_install_syslinux} -eq ${FLAGS_TRUE} ]]; then sudo umount "${ESP_FS_DIR}" + rm -rf "${ESP_FS_DIR}" + 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}" fi elif [[ "${FLAGS_arch}" = "arm" ]]; then # Copy u-boot script to ESP partition