Skip to content

Commit ece65e0

Browse files
Brian HarringGerrit
Brian Harring
authored and
Gerrit
committed
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 <[email protected]> Reviewed-by: Brian Harring <[email protected]> Tested-by: Brian Harring <[email protected]>
1 parent fd8a8ee commit ece65e0

9 files changed

+61
-22
lines changed

clean_loopback_devices

+2-2
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,6 @@ if [ "${SURE}" != "y" ]; then
3939
exit 1
4040
fi
4141

42-
sudo umount "$OUTPUT_DIR"/*/* 2> /dev/null
43-
sudo umount /tmp/esp.* 2> /dev/null
42+
safe_umount "$OUTPUT_DIR"/*/* 2> /dev/null
43+
safe_umount /tmp/esp.* 2> /dev/null
4444
sudo losetup -d /dev/loop* 2> /dev/null

common.sh

+12-2
Original file line numberDiff line numberDiff line change
@@ -562,20 +562,30 @@ safe_umount_tree() {
562562
fi
563563

564564
# First try to unmount in one shot to speed things up.
565-
if sudo umount -d ${mounts}; then
565+
if safe_umount -d ${mounts}; then
566566
return 0
567567
fi
568568

569569
# Well that didn't work, so lazy unmount remaining ones.
570570
mounts=$(sub_mounts "$1")
571571
warn "Failed to unmount ${mounts}"
572572
warn "Doing a lazy unmount"
573-
if ! sudo umount -d -l ${mounts}; then
573+
if ! safe_umount -d -l ${mounts}; then
574574
mounts=$(sub_mounts "$1")
575575
die "Failed to lazily unmount ${mounts}"
576576
fi
577577
}
578578

579+
580+
# Helper; all scripts should use this since it ensures our
581+
# override of umount is used (inside the chroot, it's enforced
582+
# via configuration; outside is the concern).
583+
# Args are passed directly to umount; no sudo args are allowed.
584+
safe_umount() {
585+
sudo "${SCRIPT_ROOT}/path-overrides/umount" "$@"
586+
}
587+
588+
579589
get_git_id() {
580590
git var GIT_COMMITTER_IDENT | sed -e 's/^.*<\(\S\+\)>.*$/\1/'
581591
}

image_to_usb.sh

+1-1
Original file line numberDiff line numberDiff line change
@@ -348,7 +348,7 @@ if [ -b "${FLAGS_to}" ]; then
348348
if [ -n "${mount_list}" ]; then
349349
echo "Attempting to unmount any mounts on the target device..."
350350
for i in ${mount_list}; do
351-
if sudo umount "$i" 2>&1 >/dev/null | grep "not found"; then
351+
if safe_umount "$i" 2>&1 >/dev/null | grep "not found"; then
352352
die_notrace "$i needs to be unmounted outside the chroot"
353353
fi
354354
done

image_to_vm.sh

+2-2
Original file line numberDiff line numberDiff line change
@@ -146,8 +146,8 @@ dd if="${SRC_IMAGE}" of="${TEMP_PMBR}" bs=512 count=1
146146
TEMP_MNT=$(mktemp -d)
147147
TEMP_ESP_MNT=$(mktemp -d)
148148
cleanup() {
149-
sudo umount "${TEMP_MNT}"
150-
sudo umount "${TEMP_ESP_MNT}"
149+
safe_umount "${TEMP_MNT}"
150+
safe_umount "${TEMP_ESP_MNT}"
151151
rmdir "${TEMP_MNT}" "${TEMP_ESP_MNT}"
152152
}
153153
trap cleanup INT TERM EXIT

make_netboot.sh

+3-3
Original file line numberDiff line numberDiff line change
@@ -68,8 +68,8 @@ fi
6868

6969
# Prepare to mount rootfs.
7070
umount_loop() {
71-
sudo umount r || true
72-
sudo umount s || true
71+
safe_umount r || true
72+
safe_umount s || true
7373
false
7474
}
7575

@@ -141,7 +141,7 @@ sudo cp "s/dev_image/etc/lsb-factory" "r/${LSB_FACTORY_DIR}"
141141

142142
# Clean up mounts.
143143
trap - EXIT
144-
sudo umount r s
144+
safe_umount r s
145145
sudo rm -rf r s
146146

147147
# Generate an initrd fo u-boot to load.

mod_image_for_recovery.sh

+4-4
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ get_install_vblock() {
7777
sudo cp "$stateful_mnt/vmlinuz_hd.vblock" "$out"
7878
sudo chown $USER "$out"
7979

80-
sudo umount "$stateful_mnt"
80+
safe_umount "$stateful_mnt"
8181
rmdir "$stateful_mnt"
8282
switch_to_strict_mode
8383
echo "$out"
@@ -186,7 +186,7 @@ create_recovery_kernel_image() {
186186
# safe.
187187
sudo sed -i -e "s/cros_efi/cros_efi kern_b_hash=$kern_hash/g" \
188188
"$efi_dir/efi/boot/grub.cfg" || true
189-
sudo umount "$efi_dir"
189+
safe_umount "$efi_dir"
190190
sudo losetup -a | sed 's/^/16651 /'
191191
sudo losetup -d "$efi_dev"
192192
rmdir "$efi_dir"
@@ -243,7 +243,7 @@ install_recovery_kernel() {
243243
local esp_mnt=$(mktemp -d)
244244
sudo mount -o loop,offset=$((esp_offset * 512)) "$RECOVERY_IMAGE" "$esp_mnt"
245245
sudo cp "$vmlinuz" "$esp_mnt/syslinux/vmlinuz.A" || failed=1
246-
sudo umount "$esp_mnt"
246+
safe_umount "$esp_mnt"
247247
rmdir "$esp_mnt"
248248
switch_to_strict_mode
249249
fi
@@ -322,7 +322,7 @@ maybe_resize_stateful() {
322322
set +e
323323
sudo mount -o loop $small_stateful $new_stateful_mnt
324324
sudo cp "$INSTALL_VBLOCK" "$new_stateful_mnt/vmlinuz_hd.vblock"
325-
sudo umount "$new_stateful_mnt"
325+
safe_umount "$new_stateful_mnt"
326326
rmdir "$new_stateful_mnt"
327327
switch_to_strict_mode
328328

mount_gpt_image.sh

+5-5
Original file line numberDiff line numberDiff line change
@@ -85,13 +85,13 @@ unmount_image() {
8585
"${FLAGS_stateful_mountpt}"
8686
fix_broken_symlinks "${FLAGS_rootfs_mountpt}"
8787
fi
88-
sudo umount "${FLAGS_rootfs_mountpt}/usr/local"
89-
sudo umount "${FLAGS_rootfs_mountpt}/var"
88+
safe_umount "${FLAGS_rootfs_mountpt}/usr/local"
89+
safe_umount "${FLAGS_rootfs_mountpt}/var"
9090
if [[ -n "${FLAGS_esp_mountpt}" ]]; then
91-
sudo umount "${FLAGS_esp_mountpt}"
91+
safe_umount "${FLAGS_esp_mountpt}"
9292
fi
93-
sudo umount "${FLAGS_stateful_mountpt}"
94-
sudo umount "${FLAGS_rootfs_mountpt}"
93+
safe_umount "${FLAGS_stateful_mountpt}"
94+
safe_umount "${FLAGS_rootfs_mountpt}"
9595
switch_to_strict_mode
9696
}
9797

path-overrides/umount

+29
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
#!/bin/bash
2+
3+
# Work around a bug on precise where gvfs trash goes looking in mounts
4+
# it shouldn't, resulting in the umount failing when it shouldn't.
5+
# See crosbug.com/23443 for the sordid details.
6+
7+
suppressed_dir=$(dirname "$(readlink -f "$0")")
8+
cleaned_path="$(echo "$PATH" | sed -e 's+\(^\|:\)/usr/local/sbin\(:\|$\)++g')"
9+
binary="$(PATH="${cleaned_path}" type -P umount)"
10+
if [ $? -ne 0 ]; then
11+
echo "umount: command not found" >&2
12+
exit 127
13+
fi
14+
15+
for x in {1..10}; do
16+
# umount doesn't give use a distinct exit code for device is busy; thus grep
17+
# the output.
18+
output=$(LC_ALL=C "${binary}" "$@" 2>&1)
19+
ret=$?
20+
if [ ${ret} -eq 0 ] || [[ "${output}" == *"device is busy"* ]]; then
21+
# Nothing to do in these scenarios; either ran fine, or it failed in a non
22+
# busy fashion.
23+
break
24+
fi
25+
sleep 1
26+
done
27+
28+
echo -n "${output}" >&2
29+
exit ${ret}

update_bootloaders.sh

+3-3
Original file line numberDiff line numberDiff line change
@@ -159,13 +159,13 @@ fi
159159
ESP_FS_DIR=$(mktemp -d /tmp/esp.XXXXXX)
160160
cleanup() {
161161
set +e
162-
if ! sudo umount "${ESP_FS_DIR}"; then
162+
if ! safe_umount "${ESP_FS_DIR}"; then
163163
# There is a race condition possible on some ubuntu setups
164164
# with mounting and unmounting a device very quickly
165165
# Doing a quick sleep/retry as a temporary workaround
166166
warn "Initial unmount failed. Possibly crosbug.com/23443. Retrying"
167167
sleep 5
168-
sudo umount "${ESP_FS_DIR}"
168+
safe_umount "${ESP_FS_DIR}"
169169
fi
170170
if [[ -n "${ESP_DEV}" && -z "${ESP_DEV//\/dev\/loop*}" ]]; then
171171
sudo losetup -d "${ESP_DEV}"
@@ -211,7 +211,7 @@ if [[ "${FLAGS_arch}" = "x86" || "${FLAGS_arch}" = "amd64" ]]; then
211211
# Install the syslinux loader on the ESP image (part 12) so it is ready when
212212
# we cut over from rootfs booting (extlinux).
213213
if [[ ${FLAGS_install_syslinux} -eq ${FLAGS_TRUE} ]]; then
214-
sudo umount "${ESP_FS_DIR}"
214+
safe_umount "${ESP_FS_DIR}"
215215
sudo syslinux -d /syslinux "${ESP_DEV}"
216216
# mount again for cleanup to free resource gracefully
217217
sudo mount -o ro "${ESP_DEV}" "${ESP_FS_DIR}"

0 commit comments

Comments
 (0)