installer: say something when a spare drive is attached but not mounted
initPickRoots can only offer MOUNTED filesystems, so on a box whose second drive
is new — unmounted, often unformatted — it returned in silence and the "where
should LibrePortal keep things?" prompt never appeared. The users most likely to
want a separate disk were the ones told nothing.
Add a notice listing what is attached and how to use it, deliberately only a
notice: mounting or formatting someone's disk is not something an installer
should do unasked, and `storage add` does it properly later with the
empty-directory admission rule and the fitness checks behind it.
Two things it has to get right, and both bit during development:
* "has no mount point" is not "is free" — the disk holding root has no mount
point of its own, its partition does, so the naive check offered the user
the disk they booted from. Walk the parent chain and mark holders in use.
* lsblk -r renders an empty mount point as a run of spaces, which `read`
collapses, shifting every later column left; an LVM member then parsed as a
mount point and was offered as free space. Use -P.
scripts/dev/lp-installer-disks-test runs the real function against a stubbed
lsblk that honours the output flag it is passed — without that the stub answers
-P to everything and cannot tell the two parsers apart, which is the bug being
guarded against. Verified both regressions fail it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
librelad
parent
b3aff2833c
commit
d13138398c
83
init.sh
83
init.sh
@ -213,6 +213,78 @@ libreportalDerivePaths
|
||||
# passed, or when the scan finds nothing else — a prompt with one possible
|
||||
# answer is not a question. Self-contained (findmnt only): scripts/ is not
|
||||
# necessarily loadable this early.
|
||||
# Say something when another drive is attached but not usable yet. Deliberately
|
||||
# only a notice: mounting or formatting someone's disk is not a thing an
|
||||
# installer should do without being asked, and `storage add` does it properly
|
||||
# later with the empty-directory admission rule and the fitness checks.
|
||||
_initMentionUnmounted()
|
||||
{
|
||||
command -v lsblk >/dev/null 2>&1 || return 0
|
||||
|
||||
# "Has no mount point" is not the same as "is free". The disk holding the
|
||||
# root filesystem has no mount point of its own — its PARTITION does — so a
|
||||
# naive check offers the user the disk they booted from. Mark anything
|
||||
# mounted or claimed as in use, then propagate that up the parent chain.
|
||||
declare -A _used=() _child=() _type=() _size=() _fs=() _parent=()
|
||||
local line kv val name type size mnt fstype pkname
|
||||
# -P, not -r: an unmounted device has an EMPTY mount point, and in raw output
|
||||
# that is just a run of spaces, so `read` collapses it and every column after
|
||||
# it shifts left. An LVM member then reads as a mount point and is offered as
|
||||
# free space. Key="value" pairs cannot slide.
|
||||
while IFS= read -r line; do
|
||||
[[ -z "$line" ]] && continue
|
||||
name=""; type=""; size=""; mnt=""; fstype=""; pkname=""
|
||||
for kv in NAME TYPE SIZE MOUNTPOINT FSTYPE PKNAME; do
|
||||
val="${line#*${kv}=\"}"
|
||||
[[ "$val" == "$line" ]] && continue
|
||||
val="${val%%\"*}"
|
||||
case "$kv" in
|
||||
NAME) name="$val" ;; TYPE) type="$val" ;; SIZE) size="$val" ;;
|
||||
MOUNTPOINT) mnt="$val" ;; FSTYPE) fstype="$val" ;; PKNAME) pkname="$val" ;;
|
||||
esac
|
||||
done
|
||||
[[ -z "$name" ]] && continue
|
||||
_type[$name]="$type"; _size[$name]="$size"; _fs[$name]="$fstype"
|
||||
_parent[$name]="$pkname"
|
||||
[[ -n "$pkname" ]] && _child[$pkname]=1
|
||||
[[ -n "$mnt" ]] && _used[$name]=1
|
||||
case "$fstype" in swap|LVM2_member|crypto_LUKS|linux_raid_member) _used[$name]=1 ;; esac
|
||||
done < <(lsblk -Pno NAME,TYPE,SIZE,MOUNTPOINT,FSTYPE,PKNAME 2>/dev/null)
|
||||
|
||||
# Anything holding something in use is itself in use, all the way up.
|
||||
local n p
|
||||
for n in "${!_used[@]}"; do
|
||||
p="${_parent[$n]:-}"
|
||||
while [[ -n "$p" ]]; do _used[$p]=1; p="${_parent[$p]:-}"; done
|
||||
done
|
||||
|
||||
local -a spare=()
|
||||
for n in "${!_type[@]}"; do
|
||||
[[ -n "${_used[$n]:-}" ]] && continue
|
||||
case "$n" in loop*|sr*|zram*|ram*|fd*) continue ;; esac
|
||||
# A disk with partitions is described by those partitions; offering both
|
||||
# would list the same space twice.
|
||||
[[ "${_type[$n]}" == "disk" && -n "${_child[$n]:-}" ]] && continue
|
||||
[[ "${_type[$n]}" == "disk" || "${_type[$n]}" == "part" ]] || continue
|
||||
# Skip the small ones — EFI and boot stubs are not app-data drives.
|
||||
case "${_size[$n]}" in *G|*T) ;; *) continue ;; esac
|
||||
spare+=("/dev/$n ${_size[$n]}${_fs[$n]:+ ${_fs[$n]}}${_fs[$n]:+}")
|
||||
[[ -z "${_fs[$n]}" ]] && spare[-1]="${spare[-1]} (no filesystem yet)"
|
||||
done
|
||||
|
||||
(( ${#spare[@]} )) || return 0
|
||||
|
||||
echo ""
|
||||
isNotice "Another drive is attached but not mounted, so it cannot be used yet:"
|
||||
local s
|
||||
for s in "${spare[@]}"; do printf ' %s\n' "$s"; done
|
||||
echo ""
|
||||
echo " Mount it and run this installer again to install onto it, or add it"
|
||||
echo " afterwards with: libreportal storage add /path/to/mountpoint"
|
||||
echo ""
|
||||
return 0
|
||||
}
|
||||
|
||||
initPickRoots()
|
||||
{
|
||||
[[ "$init_unattended_mode" == true ]] && return 0
|
||||
@ -250,7 +322,16 @@ initPickRoots()
|
||||
labels+=("$(printf '%-24s %-8s %s free' "$target" "${size:-?}" "${avail:-?}")")
|
||||
done < <(findmnt -Pno TARGET,SOURCE,FSTYPE,SIZE,AVAIL 2>/dev/null)
|
||||
|
||||
(( ${#paths[@]} )) || return 0
|
||||
# Nothing mounted to offer. That is not the same as "no other disk": a
|
||||
# brand-new drive is usually unmounted, sometimes unformatted, and this
|
||||
# prompt is the only place the installer ever mentions that app data can
|
||||
# live somewhere else. Returning in silence taught those users the feature
|
||||
# does not exist.
|
||||
if (( ${#paths[@]} == 0 )); then
|
||||
_initMentionUnmounted
|
||||
return 0
|
||||
fi
|
||||
|
||||
|
||||
local root_size root_avail
|
||||
root_size=$(findmnt -no SIZE --target / 2>/dev/null | tail -1)
|
||||
|
||||
113
scripts/dev/lp-installer-disks-test
Executable file
113
scripts/dev/lp-installer-disks-test
Executable file
@ -0,0 +1,113 @@
|
||||
#!/bin/bash
|
||||
# Does the installer mention a spare drive — and only a genuinely spare one?
|
||||
#
|
||||
# scripts/dev/lp-installer-disks-test
|
||||
#
|
||||
# initPickRoots can only offer MOUNTED filesystems, so on a box whose second
|
||||
# drive is new (unmounted, often unformatted) it returned in silence and the
|
||||
# prompt never appeared at all. That taught exactly the users who have a spare
|
||||
# disk that the feature does not exist.
|
||||
#
|
||||
# The notice that fills that gap has to be careful in two specific ways, and
|
||||
# both are cases below:
|
||||
#
|
||||
# 1. "Has no mount point" is not "is free". The disk holding the root
|
||||
# filesystem has no mount point of its own — its partition does — so the
|
||||
# obvious check offers the user the disk they booted from.
|
||||
# 2. lsblk's raw output renders an empty mount point as a run of spaces, which
|
||||
# `read` collapses, shifting every later column left. An LVM member then
|
||||
# parses as a mount point and gets offered as free space. Hence -P.
|
||||
#
|
||||
# Runs the REAL _initMentionUnmounted extracted from init.sh against a stubbed
|
||||
# lsblk, so editing the heuristic is what makes this fail.
|
||||
|
||||
REPO="$(cd "$(dirname "$0")/../.." && pwd)"
|
||||
fail=0
|
||||
chk(){ if [[ "$2" == "$3" ]]; then echo " ok $1"; else echo " FAIL $1: got '$2' want '$3'"; fail=1; fi; }
|
||||
|
||||
FN=$(awk '/^_initMentionUnmounted\(\)/,/^\}/' "$REPO/init.sh")
|
||||
[[ -n "$FN" ]] || { echo " FAIL could not extract _initMentionUnmounted from init.sh"; exit 1; }
|
||||
|
||||
# Fixtures are written as lsblk -P rows. The stub HONOURS the output flag it is
|
||||
# called with: asked for -r it re-renders them raw, collapsing the empty fields
|
||||
# the way lsblk really does. Without that the stub answers -P to everything and
|
||||
# the test cannot tell the two parsers apart — which is the bug being guarded
|
||||
# against, so a stub that ignores the flag guards nothing.
|
||||
STUB_SRC=$(mktemp)
|
||||
{
|
||||
echo 'lsblk() {'
|
||||
echo ' case " $* " in'
|
||||
echo ' *" -P"*) cat "$ROWS_FILE"; return 0 ;;'
|
||||
echo ' esac'
|
||||
echo ' sed -E '"'"'s/[A-Z]+="([^"]*)"/\1/g; s/ +/ /g; s/^ +| +$//g'"'"' "$ROWS_FILE"'
|
||||
echo '}'
|
||||
} > "$STUB_SRC"
|
||||
trap 'rm -f "$STUB_SRC"' EXIT
|
||||
|
||||
runrows() {
|
||||
local tmp rf out
|
||||
tmp=$(mktemp); rf=$(mktemp)
|
||||
cat > "$rf"
|
||||
{
|
||||
echo 'isNotice(){ echo "NOTICE: $*"; }'
|
||||
cat "$STUB_SRC"
|
||||
echo "$FN"
|
||||
echo '_initMentionUnmounted'
|
||||
} > "$tmp"
|
||||
out=$(ROWS_FILE="$rf" bash "$tmp" 2>/dev/null)
|
||||
rm -f "$tmp" "$rf"
|
||||
printf '%s\n' "$out"
|
||||
}
|
||||
|
||||
ROOT_ROWS='NAME="nvme0n1" TYPE="disk" SIZE="931.5G" MOUNTPOINT="" FSTYPE="" PKNAME=""
|
||||
NAME="nvme0n1p1" TYPE="part" SIZE="1G" MOUNTPOINT="/boot/efi" FSTYPE="vfat" PKNAME="nvme0n1"
|
||||
NAME="nvme0n1p2" TYPE="part" SIZE="930G" MOUNTPOINT="" FSTYPE="crypto_LUKS" PKNAME="nvme0n1"
|
||||
NAME="dm-0" TYPE="crypt" SIZE="930G" MOUNTPOINT="/" FSTYPE="ext4" PKNAME="nvme0n1p2"'
|
||||
|
||||
echo "--- a normal single-disk box (root on LUKS over nvme0n1p2) ---"
|
||||
out=$(runrows <<< "$ROOT_ROWS")
|
||||
chk "offers nothing" "$(grep -c NOTICE <<< "$out")" "0"
|
||||
|
||||
echo "--- a spare unformatted disk ---"
|
||||
out=$(runrows <<< "$ROOT_ROWS
|
||||
NAME=\"sdb\" TYPE=\"disk\" SIZE=\"3.6T\" MOUNTPOINT=\"\" FSTYPE=\"\" PKNAME=\"\"")
|
||||
chk "mentions it" "$(grep -c NOTICE <<< "$out")" "1"
|
||||
chk "names the device" "$(grep -c '/dev/sdb' <<< "$out")" "1"
|
||||
chk "says it is blank" "$(grep -c 'no filesystem yet' <<< "$out")" "1"
|
||||
|
||||
echo "--- a spare formatted partition ---"
|
||||
out=$(runrows <<< "$ROOT_ROWS
|
||||
NAME=\"sdb\" TYPE=\"disk\" SIZE=\"3.6T\" MOUNTPOINT=\"\" FSTYPE=\"\" PKNAME=\"\"
|
||||
NAME=\"sdb1\" TYPE=\"part\" SIZE=\"3.6T\" MOUNTPOINT=\"\" FSTYPE=\"ext4\" PKNAME=\"sdb\"")
|
||||
chk "names the partition" "$(grep -c '/dev/sdb1' <<< "$out")" "1"
|
||||
chk "not the whole disk as well" "$(grep -cE '/dev/sdb ' <<< "$out")" "0"
|
||||
|
||||
echo "--- an LVM member is not free space (the column-shift case) ---"
|
||||
out=$(runrows <<< "$ROOT_ROWS
|
||||
NAME=\"sdb\" TYPE=\"disk\" SIZE=\"2T\" MOUNTPOINT=\"\" FSTYPE=\"\" PKNAME=\"\"
|
||||
NAME=\"sdb1\" TYPE=\"part\" SIZE=\"2T\" MOUNTPOINT=\"\" FSTYPE=\"LVM2_member\" PKNAME=\"sdb\"")
|
||||
chk "offers nothing" "$(grep -c NOTICE <<< "$out")" "0"
|
||||
|
||||
echo "--- a mounted spare is already usable, so the picker handles it ---"
|
||||
out=$(runrows <<< "$ROOT_ROWS
|
||||
NAME=\"sdb\" TYPE=\"disk\" SIZE=\"2T\" MOUNTPOINT=\"\" FSTYPE=\"\" PKNAME=\"\"
|
||||
NAME=\"sdb1\" TYPE=\"part\" SIZE=\"2T\" MOUNTPOINT=\"/mnt/data\" FSTYPE=\"ext4\" PKNAME=\"sdb\"")
|
||||
chk "offers nothing" "$(grep -c NOTICE <<< "$out")" "0"
|
||||
|
||||
echo "--- a partition whose CHILD is mounted (nothing on the part itself) ---"
|
||||
# sdb1 has no mount point and no member fstype of its own; only dm-1 below it is
|
||||
# mounted. Without walking the parent chain, sdb1 looks like free space.
|
||||
out=$(runrows <<< "$ROOT_ROWS
|
||||
NAME=\"sdb\" TYPE=\"disk\" SIZE=\"2T\" MOUNTPOINT=\"\" FSTYPE=\"\" PKNAME=\"\"
|
||||
NAME=\"sdb1\" TYPE=\"part\" SIZE=\"2T\" MOUNTPOINT=\"\" FSTYPE=\"\" PKNAME=\"sdb\"
|
||||
NAME=\"dm-1\" TYPE=\"crypt\" SIZE=\"2T\" MOUNTPOINT=\"/data\" FSTYPE=\"ext4\" PKNAME=\"sdb1\"")
|
||||
chk "offers nothing" "$(grep -c NOTICE <<< "$out")" "0"
|
||||
|
||||
echo "--- small stubs are not app-data drives ---"
|
||||
out=$(runrows <<< "$ROOT_ROWS
|
||||
NAME=\"sdb\" TYPE=\"disk\" SIZE=\"512M\" MOUNTPOINT=\"\" FSTYPE=\"\" PKNAME=\"\"")
|
||||
chk "offers nothing" "$(grep -c NOTICE <<< "$out")" "0"
|
||||
|
||||
echo ""
|
||||
if (( fail )); then echo "FAILED"; exit 1; fi
|
||||
echo "All installer disk-notice checks passed."
|
||||
Loading…
x
Reference in New Issue
Block a user