scripts: More forcefully remove support for offset=
BUG=chromium:954188
TEST=tryjob succeeds
Change-Id: I2b4e25a849469604068ecee811340d796e2716a4
Reviewed-on: https://chromium-review.googlesource.com/1585878
Commit-Ready: LaMont Jones <lamontjones@chromium.org>
Tested-by: LaMont Jones <lamontjones@chromium.org>
Reviewed-by: Jason Clinton <jclinton@chromium.org>
diff --git a/bin/cros_make_image_bootable b/bin/cros_make_image_bootable
index 10eb1e6..26c0609 100755
--- a/bin/cros_make_image_bootable
+++ b/bin/cros_make_image_bootable
@@ -388,6 +388,10 @@
kernel_part=""
fi
+ # Force all of the file writes to complete, in case it's necessary for
+ # crbug.com/938958
+ sync
+
if [[ ${esp_size} -gt 0 ]]; then
# Update EFI partition
${SCRIPTS_DIR}/update_bootloaders.sh \
diff --git a/build_library/disk_layout_util.sh b/build_library/disk_layout_util.sh
index f5e8aeb..9c97c8d 100644
--- a/build_library/disk_layout_util.sh
+++ b/build_library/disk_layout_util.sh
@@ -257,7 +257,8 @@
if [[ "${x}" != "${umount}" ]]; then
cat >>"${x}" <<\EOF
-# If losetup is new enough to create the partition table, use that instead.
+# Losetup has support for partitions, and offset= has issues.
+# See crbug.com/954188
LOOPDEV=''
cleanup() {
if [[ -n "${LOOPDEV}" ]]; then
@@ -265,9 +266,7 @@
fi
}
trap cleanup EXIT
-if sudo losetup --help |& grep -q -e--partscan; then
- LOOPDEV=$(sudo losetup -P -f --show "${TARGET}") || exit 1
-fi
+LOOPDEV=$(sudo losetup -P -f --show "${TARGET}") || exit 1
EOF
fi
@@ -293,31 +292,18 @@
done
cat <<EOF >> "${unpack}"
-if [[ -n "\${LOOPDEV}" ]]; then
- sudo dd if="\${LOOPDEV}p${part}" of="${file}"
-else
- dd if=${target} of="${file}" ${dd_args} skip=${start}
-fi
+sudo dd if="\${LOOPDEV}p${part}" of="${file}"
ln -sfT ${file} "${file}_${label}"
EOF
cat <<EOF >> "${pack}"
-if [[ -n "\${LOOPDEV}" ]]; then
- sudo dd if="${file}" of="\${LOOPDEV}p${part}"
-else
- dd if="${file}" of=${target} ${dd_args} seek=${start} conv=notrunc
-fi
+sudo dd if="${file}" of="\${LOOPDEV}p${part}"
EOF
if [[ ${size} -gt 1 ]]; then
cat <<-EOF >>"${mount}"
(
mkdir -p "${dir}"
-m=( sudo mount )
-if [[ -n "\${LOOPDEV}" ]]; then
- m+=( "\${LOOPDEV}p${part}" "${dir}" )
-else
- m+=( -o loop,offset=${start_b},sizelimit=${size_b} "${target}" "${dir}" )
-fi
+m=( sudo mount "\${LOOPDEV}p${part}" "${dir}" )
if ! "\${m[@]}"; then
if ! "\${m[@]}" -o ro; then
rmdir ${dir}
diff --git a/build_library/filesystem_util.sh b/build_library/filesystem_util.sh
index d9cdd33..3a9f11f 100644
--- a/build_library/filesystem_util.sh
+++ b/build_library/filesystem_util.sh
@@ -11,17 +11,27 @@
#
# Args:
# mount_options: Options that could be passed to the "mount" command, for
-# example "loop,ro,offset=1234".
+# example "loop,ro".
# option_key: The key you are looking for.
# default_value: An optional default value used if the option key is not
# found.
fs_parse_option() {
- # If we are mounting a partition inside a device by passing an offset
- # in the mount options, we need to tell unsquashfs to read from there.
local mount_options="$1"
local option_key="$2"
local default_value="${3:-}"
+ # offset= interacts with dirty pages in the file in a very poor manner. See
+ # crbug.com/954188. Use device partitions on the loop device instead.
+ case "${option_key}" in
+ offset|size)
+ local msg="Support for ${option_key} dropped from fs_parse_option."
+ msg="${msg} See crbug.com/954188."
+ die "${msg}"
+ # unittests cause die to return to us, so make sure we return the default.
+ option_key='$'
+ ;;
+ esac
+
local option_value
if option_value=$(echo "${mount_options}" | tr , '\n' | \
grep -E "^${option_key}"'(=|$)'); then
@@ -57,10 +67,9 @@
die "ro_rw must be \"ro\" or \"rw\", not \"${ro_rw}\"."
fi
- local offset=$(fs_parse_option "${mount_options}" offset UNDEF)
- if [[ "X${offset}" != XUNDEF ]]; then
- echo "${mount_options}" # XXXLJ
- die "Attempt to use offset=${offset}. (See crbug/954118)"
+ # Explicitly deny offset= in options.
+ if echo ${mount_options} | grep -qE '^(.*,)?offset='; then
+ die "Support for offset= dropped from fs_mount. See crbug.com/954188."
fi
local all_options="${ro_rw}"
@@ -83,23 +92,19 @@
sudo mount "${part_dev}" "${mount_point}" -o "${all_options}" \
-t "${fs_format}"
else
- local offset=$(fs_parse_option "${mount_options}" offset 0)
local sizelimit=$(fs_parse_option "${mount_options}" sizelimit)
- local sizelimit_arg=""
- if [[ "${offset}" != "0" || -n "${sizelimit}" ]]; then
- local losetup_opts=( --show --read-only --offset "${offset}" )
- if [[ -n "${sizelimit}" ]]; then
- losetup_opts+=( --sizelimit "${sizelimit}" )
- fi
+ if [[ -n "${sizelimit}" ]]; then
+ local losetup_opts=( --show --read-only --sizelimit "${sizelimit}" )
part_dev=$(sudo losetup "${losetup_opts[@]}" -f "${part_dev}")
fi
sudo unsquashfs -dest "${mount_point}" -no-progress -force "${part_dev}"
- if [[ "${offset}" != "0" ]]; then
+ if [[ -n "${sizelimit}" ]]; then
# Cleanup the loop device used to unsquash the filesystem.
sudo losetup -d "${part_dev}"
fi
+ sudo unsquashfs -dest "${mount_point}" -no-progress -force "${part_dev}"
fi
;;
*)
@@ -162,10 +167,9 @@
"size of your filesystem or remove some files from it."
fi
- local offset=$(fs_parse_option "${mount_options}" offset 0)
# mksquashfs pads the filesystem up to 4kB, but we can use a bigger block
# size to improve speed.
- sudo dd if="${squash_file}" of="${part_dev}" bs=8M seek="${offset}" \
+ sudo dd if="${squash_file}" of="${part_dev}" bs=8M \
oflag=seek_bytes conv=notrunc status=none
sudo rm -f "${squash_file}"
;;
diff --git a/build_library/filesystem_util_unittest.sh b/build_library/filesystem_util_unittest.sh
index 46d9ba4..9ee394a 100755
--- a/build_library/filesystem_util_unittest.sh
+++ b/build_library/filesystem_util_unittest.sh
@@ -8,6 +8,11 @@
set -e -u
+# Make die() non-fatal for testing.
+exit() {
+ echo exit "$@"
+}
+
output_test() {
local expected_output="$1"
echo "Testing ${*:2}"
@@ -32,7 +37,10 @@
output_test 42 fs_parse_option "loop=,rostuff,offset=1234" "ro" 42
output_test 42 fs_parse_option "loop=,ro,offset=1234" "laap" 42
-output_test 1234 fs_parse_option "loop=,ro,offset=1234" "offset" 42
+# offset= interacts with dirty pages in the file in a very poor manner.
+# See crbug.com/954188
+output_test 'exit 1' fs_parse_option "loop=,ro,offset=1234" "offset" ""
+
output_test 42 fs_parse_option "loop=,ro,offset=1234" "laap" 42
echo "All tests passed."
diff --git a/common.sh b/common.sh
index d87318c..df5e47e 100644
--- a/common.sh
+++ b/common.sh
@@ -908,6 +908,9 @@
printf '\377' |
sudo dd of="${rootfs}" seek=$((offset + ro_compat_offset)) \
conv=notrunc count=1 bs=1 status=none
+ # Force all of the file writes to complete, in case it's necessary for
+ # crbug.com/954188
+ sync
}
enable_rw_mount() {
@@ -921,6 +924,9 @@
printf '\000' |
sudo dd of="${rootfs}" seek=$((offset + ro_compat_offset)) \
conv=notrunc count=1 bs=1 status=none
+ # Force all of the file writes to complete, in case it's necessary for
+ # crbug.com/954188
+ sync
}
is_ext2_rw_mount_enabled() {
diff --git a/mod_image_for_recovery.sh b/mod_image_for_recovery.sh
index 1d07461..2d3c53e 100755
--- a/mod_image_for_recovery.sh
+++ b/mod_image_for_recovery.sh
@@ -139,6 +139,9 @@
fi
local kern_hash=$(sha1sum "$kern_tmp" | cut -f1 -d' ')
rm "$kern_tmp"
+ # Force all of the file writes to complete, in case it's necessary for
+ # crbug.com/954188
+ sync
# TODO(wad) add FLAGS_boot_args support too.
${SCRIPTS_DIR}/build_kernel_image.sh \
@@ -225,6 +228,9 @@
seek=$kern_a_offset \
count=$kern_a_size \
conv=notrunc
+ # Force all of the file writes to complete, in case it's necessary for
+ # crbug.com/954188
+ sync
# Set the 'Success' flag to 1 (to prevent the firmware from updating
# the 'Tries' flag).
@@ -318,6 +324,9 @@
# This is the real vblock and not the recovery vblock.
new_stateful_mnt=$(mktemp -d)
+ # Force all of the file writes to complete, in case it's necessary for
+ # crbug.com/954188
+ sync
sudo mount -o loop $small_stateful $new_stateful_mnt
# Create the directories that are going to be needed below. With correct
diff --git a/update_bootloaders.sh b/update_bootloaders.sh
index bc4b599..df4bf22 100755
--- a/update_bootloaders.sh
+++ b/update_bootloaders.sh
@@ -31,10 +31,12 @@
"Path the legacy bootloader templates are copied from. (Default /tmp/boot)"
DEFINE_string to "/tmp/esp.img" \
"Path to esp image (Default: /tmp/esp.img)"
-DEFINE_integer to_offset 0 \
- "Offset in bytes into 'to' if it is a file (Default: 0)"
-DEFINE_integer to_size -1 \
- "Size in bytes of 'to' to use if it is a file. -1 is ignored. (Default: -1)"
+# offset= interacts with dirty pages in the file in a very poor manner. See
+# crbug.com/954188. Use device partitions on the loop device instead.
+#DEFINE_integer to_offset 0 \
+# "Offset in bytes into 'to' if it is a file (Default: 0)"
+#DEFINE_integer to_size -1 \
+# "Size in bytes of 'to' to use if it is a file. -1 is ignored. (Default: -1)"
DEFINE_integer to_partition -1 \
"Partition number to use on block device. -1 is ignored. (Default: -1)"
DEFINE_string vmlinuz "/tmp/vmlinuz" \
@@ -143,17 +145,6 @@
fi
else
if [[ -f "${FLAGS_to}" ]]; then
- esp_offset="--offset ${FLAGS_to_offset}"
- esp_size="--sizelimit ${FLAGS_to_size}"
- if [ ${FLAGS_to_size} -lt 0 ]; then
- esp_size=
- fi
- if [ ${FLAGS_to_offset} -gt 0]; then
- die "Attempt to use --to-offset. (See crbug/954118)"
- fi
- if [ ${FLAGS_to_size} -ge 0]; then
- die "Attempt to use --to-size. (See crbug/954118)"
- fi
ESP_DEV=$(sudo losetup --show -f "${FLAGS_to}")
ESP_DEV_OURS=y
if [ -z "${ESP_DEV}" ]; then
