diff --git a/init.sh b/init.sh index 8bcb9d3..186befa 100755 --- a/init.sh +++ b/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) diff --git a/scripts/dev/lp-installer-disks-test b/scripts/dev/lp-installer-disks-test new file mode 100755 index 0000000..3dc79f3 --- /dev/null +++ b/scripts/dev/lp-installer-disks-test @@ -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."