Skip to content

Commit 1d55d6d

Browse files
AkihiroSudaAkihiroSuda
authored
Merge pull request #4831 from jandubois/fix-data-volume-race
Fix race condition in 04-persistent-data-volume.sh
2 parents 7ba5237 + 34a1f47 commit 1d55d6d

1 file changed

Lines changed: 59 additions & 33 deletions

File tree

‎pkg/cidata/cidata.TEMPLATE.d/boot.Linux/04-persistent-data-volume.sh‎

Lines changed: 59 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -51,20 +51,37 @@ done
5151
chmod +x /mnt.sh
5252

5353
mkdir -p /mnt/data
54-
if [ -e /dev/disk/by-label/data-volume ]; then
55-
# Find which disk is data volume on
56-
DATA_DISK=$(blkid | grep "data-volume" | awk '{split($0,s,":"); sub(/\d$/, "", s[1]); print s[1]};')
54+
55+
# Resolve the data volume device via blkid, not the udev symlink.
56+
# The /dev/disk/by-label/ symlink depends on udev having probed the device.
57+
# A race between growpart (which triggers a partition table re-read and thus
58+
# a udev re-probe) and e2fsck (which modifies the superblock) can cause the
59+
# re-probe to see an inconsistent ext4 checksum and delete the symlink.
60+
# BusyBox blkid doesn't support --label, so we parse the output instead.
61+
# The leading whitespace anchor on LABEL= avoids matching PARTLABEL= on
62+
# util-linux blkid output; quitting after the first match prevents a
63+
# multi-match from producing a newline-separated device list.
64+
DATA_VOLUME=$(blkid | sed -n '/[[:space:]]LABEL="data-volume"/{s/^\([^:]*\):.*/\1/p;q;}')
65+
66+
if [ -n "${DATA_VOLUME}" ]; then
67+
DATA_DISK="/dev/$(lsblk --noheadings --output pkname "${DATA_VOLUME}")"
5768
# growpart command may be missing in older VMs
5869
if command -v growpart >/dev/null 2>&1 && command -v resize2fs >/dev/null 2>&1; then
5970
# Automatically expand the data volume filesystem
6071
growpart "$DATA_DISK" 1 || true
72+
# growpart triggers a partition table re-read; settle udev before
73+
# touching the device to avoid racing with the re-probe. The
74+
# settle is defense-in-depth (blkid above eliminates the core
75+
# udev dependency), so tolerate its failure rather than crashing
76+
# the boot under set -e if udevadm is missing or stuck.
77+
udevadm settle || true
6178
# Only resize when filesystem is in a healthy state
62-
if e2fsck -f -p /dev/disk/by-label/data-volume; then
63-
resize2fs /dev/disk/by-label/data-volume || true
79+
if e2fsck -f -p "${DATA_VOLUME}"; then
80+
resize2fs "${DATA_VOLUME}" || true
6481
fi
6582
fi
6683
# Mount data volume
67-
mount -t ext4 /dev/disk/by-label/data-volume /mnt/data
84+
mount -t ext4 "${DATA_VOLUME}" /mnt/data
6885
# Update /etc files that might have changed during this boot
6986
cp /etc/network/interfaces /mnt/data/etc/network/
7087
cp /etc/resolv.conf /mnt/data/etc/
@@ -85,34 +102,43 @@ else
85102
# Find an unpartitioned disk and create data-volume
86103
DISKS=$(lsblk --list --noheadings --output name,type | awk '$2 == "disk" {print $1}')
87104
for DISK in ${DISKS}; do
88-
IN_USE=false
89-
# Looking for a disk that is not mounted or partitioned
90-
# shellcheck disable=SC2013
91-
for PART in $(awk '/^\/dev\// {gsub("/dev/", ""); print $1}' /proc/mounts); do
92-
if [ "${DISK}" == "${PART}" ] || [ -e /sys/block/"${DISK}"/"${PART}" ]; then
93-
IN_USE=true
94-
break
95-
fi
96-
done
97-
if [ "${IN_USE}" == "false" ]; then
98-
echo 'type=83' | sfdisk --label dos /dev/"${DISK}"
99-
PART=$(lsblk --list /dev/"${DISK}" --noheadings --output name,type | awk '$2 == "part" {print $1}')
100-
mkfs.ext4 -L data-volume /dev/"${PART}"
101-
mount -t ext4 /dev/disk/by-label/data-volume /mnt/data
102-
# setup apk package cache
103-
mkdir -p /mnt/data/apk/cache
104-
mkdir -p /etc/apk
105-
ln -s /mnt/data/apk/cache /etc/apk/cache
106-
# Move all persisted directories to the data volume
107-
for DIR in ${DATADIRS}; do
108-
DEST="/mnt/data$(dirname "${DIR}")"
109-
mkdir -p "${DIR}" "${DEST}"
110-
mv "${DIR}" "${DEST}"
111-
done
112-
# Make sure all data moved to the persistent volume has been committed to disk
113-
sync
114-
break
105+
# A disk is in use if it has any partitions, carries a filesystem
106+
# signature directly, or is mounted. Check lsblk for partitions
107+
# and filesystem types, not just /proc/mounts; an unmounted but
108+
# partitioned or raw-formatted disk (e.g. the data volume after
109+
# a failed boot) must not be reformatted.
110+
if lsblk --list --noheadings --output fstype /dev/"${DISK}" | grep --quiet "[^[:space:]]"; then
111+
continue
115112
fi
113+
if lsblk --list --noheadings --output type /dev/"${DISK}" | grep --quiet "part"; then
114+
continue
115+
fi
116+
if awk '/^\/dev\// {sub("^/dev/", "", $1); print $1}' /proc/mounts | grep --quiet "^${DISK}$"; then
117+
continue
118+
fi
119+
echo 'type=83' | sfdisk --label dos /dev/"${DISK}"
120+
PART=$(lsblk --list /dev/"${DISK}" --noheadings --output name,type | awk '$2 == "part" {print $1}')
121+
mkfs.ext4 -L data-volume /dev/"${PART}"
122+
# Let udev process the new filesystem before continuing; mount
123+
# uses the device path directly, but later boot scripts or
124+
# services may depend on the /dev/disk/by-label/ symlink.
125+
# Tolerate failure so a missing or stuck udevadm does not crash
126+
# the boot under set -e.
127+
udevadm settle || true
128+
mount -t ext4 /dev/"${PART}" /mnt/data
129+
# setup apk package cache
130+
mkdir -p /mnt/data/apk/cache
131+
mkdir -p /etc/apk
132+
ln -s /mnt/data/apk/cache /etc/apk/cache
133+
# Move all persisted directories to the data volume
134+
for DIR in ${DATADIRS}; do
135+
DEST="/mnt/data$(dirname "${DIR}")"
136+
mkdir -p "${DIR}" "${DEST}"
137+
mv "${DIR}" "${DEST}"
138+
done
139+
# Make sure all data moved to the persistent volume has been committed to disk
140+
sync
141+
break
116142
done
117143
fi
118144
for DIR in ${DATADIRS}; do

0 commit comments

Comments
 (0)