Delete the dead ISO-eject step from provision-vm-from-host #19

New issue

Closed

opened 2026-07-28 19:14:13 +01:00 by allod-agent · 0 comments

allod-agent commented

2026-07-28 19:14:13 +01:00

Member

Delete a two-line ISO-eject step in the host-side VM provisioning script that cannot affect the definition virsh start reads, and correct a stale comment above the nixos-anywhere call that documents a flag the invocation does not pass.

Primary goals:

Delete the dead eject — remove scripts/provision-vm-from-host:113-114, which mutates only the live domain that the very next line destroys, so it can never reach the persistent XML that the subsequent virsh start reads.
Remove a double-swallowed failure — the step hides its own failure twice, via 2>/dev/null and via an [[ ... ]] && cmd || true chain, which is the anti-pattern memory/shell.md names and a violation of architecture.md principle 11 (fail loud, never fall back silently).
Disarm a wrong-column match — awk '/cdrom/ {print $3}' matches the whole line rather than the Device column, so a source path containing the substring cdrom yields a disk row's target instead.
Fix a stale comment — scripts/provision-vm-from-host:101 describes a --no-reboot flag that the invocation immediately below it does not pass.
Keep the mechanism that works — the destroy / sleep 1 / start sequence at scripts/provision-vm-from-host:115-117 stays untouched; it is what actually boots the VM off the installed disk.

The eject cannot reach the definition that matters

The two lines under the # Eject ISO and hard restart so UEFI boots from installed disk comment at scripts/provision-vm-from-host:111 are:

CDROM=$("$VIRSH" domblklist "${VM_NAME}" --details 2>/dev/null | awk '/cdrom/ {print $3}')
[[ -n "$CDROM" ]] && "$VIRSH" change-media "${VM_NAME}" "$CDROM" --eject --force 2>/dev/null || true

change-media is passed neither --config nor --live, so against a running QEMU domain it affects the live domain only. That live domain is destroyed on the very next line, scripts/provision-vm-from-host:115. The eject therefore never touches the persistent definition, and the persistent definition is exactly what virsh start at scripts/provision-vm-from-host:117 boots from. What actually makes the VM boot off disk is that virt-install --cdrom at scripts/new-vm:125 attaches the ISO for the install boot but leaves the media path empty in the persistent definition, so the destroy/start pair brings the domain back with no ISO in the drive. This is confirmed empirically on the host: virsh dumpxml --inactive for a provisioned dev VM shows the cdrom disk with no <source> element at all.

The failure is swallowed twice

2>/dev/null discards any diagnostic, and the [[ -n "$CDROM" ]] && ... || true chain guarantees exit 0 regardless of what change-media does. A step that has never worked has also never been able to say so. memory/shell.md records that in a && b only the final command's failure aborts under set -e, which is what makes this chain a permanent pass.

The awk pattern matches the wrong column

domblklist --details prints the columns Type Device Target Source. The test fixture at tests/provision-vm-from-host.sh:102-103 stubs exactly that shape, emitting file cdrom sda /tmp/installer.iso. The pattern /cdrom/ is unanchored across the whole record, not restricted to the Device field, so a disk whose Source path happens to contain cdrom — an ISO library directory, a pool named for it — would match and hand back that disk's Target. For change-media --eject this is harmless in practice, but it is a live foot-gun the moment anyone converts the step into something destructive such as detach-disk. Deleting the step removes the foot-gun along with the dead code.

Stale `--no-reboot` comment

scripts/provision-vm-from-host:101 reads # --no-reboot: we eject the ISO before rebooting; otherwise the VM boots back into installer, but the nixos-anywhere invocation directly beneath it passes no --no-reboot. It relies on --phases disko,install at scripts/provision-vm-from-host:105 simply omitting the reboot phase. The comment should describe the phase selection that is actually in force, and should stop referring to an eject that will no longer exist.

Scope

In scope: deleting scripts/provision-vm-from-host:113-114, adjusting the surrounding comment and echo at scripts/provision-vm-from-host:111-112 so they describe the destroy/start restart rather than an eject, and rewriting the stale comment at scripts/provision-vm-from-host:101. The destroy / sleep 1 / start sequence is deliberately left alone.

Explicitly out of scope, and not to be re-added later: a virsh detach-disk to remove the leftover CD-ROM device from the persistent definition. The device that remains is an empty drive with no media path — it is cosmetic, it carries no boot risk because there is nothing to boot, and it is not a host-to-guest data path. Adding a detach would arm the wrong-column targeting problem described above against a genuinely destructive command in order to solve a non-problem. architecture.md principle 14 says gates that cannot verify quality get deleted rather than elaborated, and principle 15 puts minimalism first; both point at deletion rather than a replacement step.

Also out of scope: any change to the test suite's behavior contract. tests/provision-vm-from-host.sh asserts only grep -q 'start alpha-dev' against the virsh log at tests/provision-vm-from-host.sh:355; nothing asserts the eject, so the suite should pass unchanged. The domblklist case in the virsh stub becomes unreachable and may be left or trimmed with the script change.

Residual risk: R1. The change is confined to one host-side provisioning script and deletes only code that provably has no effect on the persistent domain definition; it touches no key material, no secrets, no inventory, and no guest-facing state. Blast radius is a single host command sequence run by a human at the terminal, and any error surfaces immediately and loudly at the next virsh start during a provisioning run, which is itself a destructive-and-repeatable operation on a disposable VM. Rollback is a one-commit revert.

Delete a two-line ISO-eject step in the host-side VM provisioning script that cannot affect the definition `virsh start` reads, and correct a stale comment above the `nixos-anywhere` call that documents a flag the invocation does not pass. Primary goals: - **Delete the dead eject** — remove `scripts/provision-vm-from-host:113-114`, which mutates only the live domain that the very next line destroys, so it can never reach the persistent XML that the subsequent `virsh start` reads. - **Remove a double-swallowed failure** — the step hides its own failure twice, via `2>/dev/null` and via an `[[ ... ]] && cmd || true` chain, which is the anti-pattern `memory/shell.md` names and a violation of `architecture.md` principle 11 (fail loud, never fall back silently). - **Disarm a wrong-column match** — `awk '/cdrom/ {print $3}'` matches the whole line rather than the Device column, so a source path containing the substring `cdrom` yields a disk row's target instead. - **Fix a stale comment** — `scripts/provision-vm-from-host:101` describes a `--no-reboot` flag that the invocation immediately below it does not pass. - **Keep the mechanism that works** — the `destroy` / `sleep 1` / `start` sequence at `scripts/provision-vm-from-host:115-117` stays untouched; it is what actually boots the VM off the installed disk. ### The eject cannot reach the definition that matters The two lines under the `# Eject ISO and hard restart so UEFI boots from installed disk` comment at `scripts/provision-vm-from-host:111` are: ```bash CDROM=$("$VIRSH" domblklist "${VM_NAME}" --details 2>/dev/null | awk '/cdrom/ {print $3}') [[ -n "$CDROM" ]] && "$VIRSH" change-media "${VM_NAME}" "$CDROM" --eject --force 2>/dev/null || true ``` `change-media` is passed neither `--config` nor `--live`, so against a running QEMU domain it affects the live domain only. That live domain is destroyed on the very next line, `scripts/provision-vm-from-host:115`. The eject therefore never touches the persistent definition, and the persistent definition is exactly what `virsh start` at `scripts/provision-vm-from-host:117` boots from. What actually makes the VM boot off disk is that `virt-install --cdrom` at `scripts/new-vm:125` attaches the ISO for the install boot but leaves the media path empty in the persistent definition, so the destroy/start pair brings the domain back with no ISO in the drive. This is confirmed empirically on the host: `virsh dumpxml --inactive` for a provisioned dev VM shows the cdrom disk with no `<source>` element at all. ### The failure is swallowed twice `2>/dev/null` discards any diagnostic, and the `[[ -n "$CDROM" ]] && ... || true` chain guarantees exit 0 regardless of what `change-media` does. A step that has never worked has also never been able to say so. `memory/shell.md` records that in `a && b` only the final command's failure aborts under `set -e`, which is what makes this chain a permanent pass. ### The awk pattern matches the wrong column `domblklist --details` prints the columns `Type Device Target Source`. The test fixture at `tests/provision-vm-from-host.sh:102-103` stubs exactly that shape, emitting `file cdrom sda /tmp/installer.iso`. The pattern `/cdrom/` is unanchored across the whole record, not restricted to the Device field, so a disk whose Source path happens to contain `cdrom` — an ISO library directory, a pool named for it — would match and hand back that disk's Target. For `change-media --eject` this is harmless in practice, but it is a live foot-gun the moment anyone converts the step into something destructive such as `detach-disk`. Deleting the step removes the foot-gun along with the dead code. ### Stale `--no-reboot` comment `scripts/provision-vm-from-host:101` reads `# --no-reboot: we eject the ISO before rebooting; otherwise the VM boots back into installer`, but the `nixos-anywhere` invocation directly beneath it passes no `--no-reboot`. It relies on `--phases disko,install` at `scripts/provision-vm-from-host:105` simply omitting the reboot phase. The comment should describe the phase selection that is actually in force, and should stop referring to an eject that will no longer exist. ### Scope In scope: deleting `scripts/provision-vm-from-host:113-114`, adjusting the surrounding comment and echo at `scripts/provision-vm-from-host:111-112` so they describe the destroy/start restart rather than an eject, and rewriting the stale comment at `scripts/provision-vm-from-host:101`. The `destroy` / `sleep 1` / `start` sequence is deliberately left alone. Explicitly out of scope, and not to be re-added later: a `virsh detach-disk` to remove the leftover CD-ROM device from the persistent definition. The device that remains is an empty drive with no media path — it is cosmetic, it carries no boot risk because there is nothing to boot, and it is not a host-to-guest data path. Adding a detach would arm the wrong-column targeting problem described above against a genuinely destructive command in order to solve a non-problem. `architecture.md` principle 14 says gates that cannot verify quality get deleted rather than elaborated, and principle 15 puts minimalism first; both point at deletion rather than a replacement step. Also out of scope: any change to the test suite's behavior contract. `tests/provision-vm-from-host.sh` asserts only `grep -q 'start alpha-dev'` against the virsh log at `tests/provision-vm-from-host.sh:355`; nothing asserts the eject, so the suite should pass unchanged. The `domblklist` case in the virsh stub becomes unreachable and may be left or trimmed with the script change. Residual risk: R1. The change is confined to one host-side provisioning script and deletes only code that provably has no effect on the persistent domain definition; it touches no key material, no secrets, no inventory, and no guest-facing state. Blast radius is a single host command sequence run by a human at the terminal, and any error surfaces immediately and loudly at the next `virsh start` during a provisioning run, which is itself a destructive-and-repeatable operation on a disposable VM. Rollback is a one-commit revert.

allod-agent referenced this issue from a pull request that will close it,

2026-07-28 19:17:25 +01:00

Delete the dead ISO-eject step from provision-vm-from-host #20