Repository navigation
os/v3/puppy: the boot's log capture waits for /data instead of testing it once - #1001
Conversation
…g it once 4 Nov (TOS3-NOTE-c21-logs): of five boots that reached userspace, one kept a log and four kept none - on the 1BIT-DATA partition and anywhere else. Two one-shot tests are why, and each fails silently: - /etc/init.d/06onboard-logs ran 7 s into the boot and returned 0 unless " /data " was already in /proc/mounts at that instant. Nothing re-ran it (measured: /data mounted 3.7 min into a boot), so the rest of that boot logged nowhere. - 1bit-logs' dir() only asked "can I mkdir", and $DATA is a directory even when the partition never mounted (05onboard-data mkdirs it first). So a boot with no mount wrote its "persisted" log to the RAM-backed root and reported success - the same class, one layer down. Now: dir() requires $DATA to be a mount point (/proc/mounts), and the daemon waits for it (5 s poll, one line per state change) instead of exiting, so a boot where /data ever mounts has its evidence. The service starts the daemon unconditionally and says which state it started in. `1bit-logs snap` on an unmounted $DATA is now rc=1 and creates nothing, where it used to return 0 after writing to the root filesystem. Gates: c22-logs-repro.sh reproduces the boot ordering from the two real files (before = origin/main, after = this tree) in an unprivileged mount namespace: before = service gives up, no log after the late mount, snap rc=0 with 4 files created on the root fs; after = daemon waits, BOOT appears after the mount, snap rc=1 and nothing created. vm-check: `U3 boot logs` now requires the mount point AND a live daemon, and a new `U3 logs wait` row mounts a tmpfs over the daemon's target inside the guest and requires the log to appear. container-build.sh refuses a packed image carrying either old spelling.
…, the daemon's own output
… empty word 1bit-logs dir now refuses on an unmounted $DATA (the fix in the previous commit), and 1bit-desktop used its output directly as a path: an empty word turns "$d/xwin-console.log" into "/xwin-console.log" on the root filesystem. It falls back to /var/log/1bit-logs, which is what the function already did when the tool was missing. Three arms measured with a stub: prints a dir -> that dir; exits 1 -> the fallback; missing tool -> the fallback.
PR Reviewer Guide 🔍(Review updated until commit d5360a9)Here are some key observations to aid the review process:
|
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT: READY — I reproduced both silent failures and the fix on this box, without a VM and without a mount.
Reviewed at 7fc902e84 (5 files, +69/-10).
The A/B, on this host. 1bit-logs takes ONEBIT_LOG_DATA, so the whole mechanism runs here. Same directory that
is not a mount point, main vs this head:
$ ONEBIT_LOG_DATA=/tmp/tprs-logs-X 1bit-logs dir # at main:
/tmp/tprs-logs-X/logs/20261104T130323-4c8f0841 # rc 0, and 3 files created in the RAM directory
# -> the "persisted" log written to RAM, reported OK
$ ONEBIT_LOG_DATA=/tmp/tprs-logs-Y 1bit-logs dir # at this head:
# rc 1, no output, 0 files created
$ ONEBIT_LOG_DATA=/tmp/tprs-logs-Y 1bit-logs snap
1bit-logs: nothing persists (/tmp/tprs-logs-Y is not a mounted partition) # rc 1, 0 files
$ timeout 6 ONEBIT_LOG_DATA=/tmp/tprs-logs-Y 1bit-logs daemon
1bit-logs: /tmp/tprs-logs-Y is not mounted yet - waiting for it (nothing is persisted until then)
# rc 124 = still waiting when I killed it, and 0 files written
So both halves are real: dir() no longer treats "can I mkdir" as "is it mounted", and daemon() waits instead of
exiting (and says the waiting line exactly once). The message also names the directory, which is what the c21 note had
to reconstruct from dumpe2fs mount counts.
The rest of the checks. bash -n clean on all five changed files; shellcheck -S warning silent on the three
runtime scripts (1bit-logs, 06onboard-logs, 1bit-desktop). No boot-hang risk from the new loop: 06onboard-logs
starts the daemon with &, so the wait never blocks rc.services. And I looked for the third caller of 1bit-logs dir
rather than trusting the two the PR fixes: 1bit-console-menu:572 already guards with
[ -n "$bootlogs" ] && [ -d "$bootlogs" ], so the empty-output class of bug that 1bit-desktop's logdir() had (/xwin-console.log)
does not exist anywhere else.
Two build/VM checks I could not run and am relying on the PR for: the container-build.sh refusals (they need an
unsquashfs of the packed SFS) and the new U3 logs wait vm-check row (it mounts a tmpfs in the guest — I did not mount
anything on this box, per the lane rules). The guest row is the right instrument for the "mount arrives late" half, and
the shape of it matches what I reproduced here: pre=0, then a mount, then a BOOT file appearing with the daemon still
alive.
One note, not a gate: the wait has no bound. A box that never mounts /data (the desktop image without a data
partition) now keeps a sleeper forever instead of exiting, which is a deliberate reading of "a boot where it never does
is said once" — but it does change the process's lifetime, and stop() is now the only thing that ends it. If that
matters, a cap (say 30 min, then the old one-line exit) keeps the diagnostic and the old semantics.
… symlink in this image Measured in the guest (c22-guest-probe.sh): /tmp -> /initrd/mnt/tmpfs/tmp, so a tmpfs mounted at /tmp/logs-late appears in /proc/mounts as /initrd/mnt/tmpfs/tmp/logs-late; findmnt showed the mount and the literal " $DATA " compare found nothing, so the daemon waited with the partition mounted. The path is resolved with readlink -f first now; the U3 logs wait row resolves it too when it reports mount-lines. Guest proof on the rebuilt test image: the daemon says it waits, then writes /tmp/logs-late2/logs/<bootid>/BOOT after the tmpfs lands.
|
Persistent review updated to latest commit 0718669 |
|
#1001 ready for review at 0718669 — the boot's log capture waits for /data instead of testing it once (closes #1000). Reproducer before/after in the body, and |
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT: READY
Re-verdicted at 0718669d5 (one new commit after the 13:03 verdict at 7fc902e84): mounted() now canonicalises the
data path, and the U3 logs wait guest row does the same for its $d.
The new commit fixes a real defect in the previous one. 1bit-logs compared $DATA literally against
/proc/mounts, but /proc/mounts records the mount point the kernel was given, so a tmpfs mounted at a path that
reaches through the /tmp symlink is recorded under the resolved path — the daemon would wait for ever on a directory
that is mounted. I reproduced both halves of that with a symlinked directory and a mount fixture (the same strings,
without touching /proc):
old mounted(): says NOT mounted <- the defect, on a mounted directory
new mounted(): says mounted <- the fix
/…/targetdir (no symlink): old mounted, new mounted (no regression)
/…/absent (not mounted): old not mounted, new not mounted
So it is not just a fix for the case in the comment: it is the difference between "unmounted" and "mounted through a
symlink", and the fail-closed direction is preserved (readlink -f failing or empty -> return 1 -> still not
mounted). readlink -f already runs inside this image in vm-check.sh rows 542/629/632, so the tool is present in
the guest.
bash -n clean on both changed files, shellcheck silent on 1bit-logs (0 findings).
Not re-run by me: the two guest-only rows still need the VM (the U3 logs wait row mounts a tmpfs, and I mount
nothing on this box), and container-build.sh's refusal arms need the packed SFS. Everything from the 13:03 review
stands unchanged: at f17cc61 1bit-logs dir wrote 3 files into a non-mount and reported OK, and this head exits 1
with no output and no files; snap exits 1 naming the reason; daemon waits and writes nothing.
The unbounded-wait note still stands as a design question, not a gate: a box that never mounts /data keeps a
sleeper forever instead of exiting.
|
Persistent review updated to latest commit 7ca1776 |
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT: READY at 9eef254a2 — merge-only update, own content byte-identical.
Checked with a hash of the PR's own diff at both heads rather than by eyeballing the file list: git diff <merge-base> <head> | sha256sum is the same at 0718669d5 and at 9eef254a2, so nothing of this PR's moved. The branch is MERGEABLE/BLOCKED; the whole delta is main advancing (#1007, #1002, and the console-TUI/prefix-cache work that landed with them).
Evidence from the review at 0718669d5 stands: mounted() canonicalises the data path — tested against a symlinked directory and a mount fixture (old: NOT mounted on a mounted dir; new: mounted), fail-closed on both.
CI at this head: red/pending: build, pr_agent, Analyze (rust) — pr_agent failures on engine PRs are superseded-run cancellations ("Canceling since a higher priority waiting request exists"), not findings.
|
Persistent review updated to latest commit d5360a9 |
closes #1000
4 Nov,
desktopgateon the frozen c20 image (e066f9eaaf0a…): of five boots that reached userspace, one kept a logand four kept none —
/data/logs/<boot-id>/absent, and nothing written to1BIT-DATAat all (dumpe2fs:Mount count: 1, last mount ~3.7 min into the boot). Two one-shot tests are why, and each fails silently.Root cause
/etc/init.d/06onboard-logsran ~7 s into the boot andreturn 0ed unless" /data "was already in/proc/mountsat that instant. Nothing re-ran it, so a boot whose partition mounts later logs nowhere for therest of its life — and the only trace was a line on tty1 that nobody is looking at.
1bit-logs'dir()only asked "can I mkdir", and$DATAis a directory even when nothing is mounted on it(
05onboard-datamkdir -ps it before mounting). So a boot with no mount wrote the "persisted" log onto theRAM-backed root and returned 0 — the same class, one layer down. That is also why the missing logs left no error
anywhere.
A third defect fell out of gating it (below): the mount-point test compared
$DATAliterally, and/tmpis asymlink to
/initrd/mnt/tmpfs/tmpin this image, so a mount reached through it was invisible to the test.The change (5 files, +79/−13)
1bit-logs:mounted()requires$DATAto be a mount point — resolved withreadlink -f— anddir()refuseswithout it;
daemon()waits for the mount (5 s poll, one line per state change) instead of exiting;snapon anunmounted
$DATAis rc=1 and creates nothing (it used to return 0 after writing to the root fs).06onboard-logs: no one-shot test — the daemon is started unconditionally and the service says which state itstarted in (
writing /data/logs/…vs/data is not mounted yet, 1bit-logs waits for it).1bit-desktop:logdir()falls back to/var/log/1bit-logsinstead of returning an empty word (dircan nowrefuse), which would otherwise have turned
"$d/xwin-console.log"into/xwin-console.logon the root.container-build.sh: refuses a packed image carrying either old spelling (this boot's logs stay in RAM, or a1bit-logswith nomounted()line).vm-check.sh:U3 boot logsnow requires the mount point AND a live daemon ANDBOOTwith the cmdline; a newU3 logs waitrow mounts a tmpfs over the daemon's target inside the guest — the same ordering as/dataarrivinglate — and requires the log to appear without anything restarting the daemon. It prints
mount-rc,mount-lines,daemon liveness and the daemon's own two lines, so this class is never diagnosed by guesswork again.
Reproducer, before and after, same command
~/wt/os3i/c22-logs-repro.sh <repo> <before|after>runs the real two files in an unprivileged mount namespace(only the service's
LOGS/PIDFILE/log-sink paths are substituted), reproducing one boot's ordering: the servicestarts with nothing mounted, then the partition arrives.
Run twice per arm; same verdict both times.
Gates
sh -n/bash -nclean on every touched script;tools/copyright.py --checkrc=0; no shellcheck warning on anyof them (0 before, 0 after); the row table's embedded python
compile()s.vm-check.shon an artifact carrying the fix — 51 PASS / 0 FAIL / 3 SKIP (/tmp/c22-vm3.log; the SKIPs are thetwo board rows and
U7 ssh, which needs a private key). The artifact is the c21 image with only these three filesre-packed into its puppy SFS (
mksquashfs→mkimage.sh, sha25691a4024919379d76b91790bffcd8946109152bd2c7d9251f74c5f7d82417530a) —a patched test image, not a cycle artifact; the next image from main carries it.
~/wt/os3i/c22-guest-probe.sh: boots theshipped image under OVMF + Secure Boot, selects the serial-shell entry, drives commands):
ls -ld /tmp→/tmp -> /initrd/mnt/tmpfs/tmp; aftermount -t tmpfs none /tmp/logs-late,findmntprints/initrd/mnt/tmpfs/tmp/logs-late none tmpfs rw,relatime,inode64while a literal" /tmp/logs-late "match in/proc/mountsfinds nothing. With the canonical-path fix the same boot logs… is mounted after waiting, this boot's logs go to /tmp/logs-late2/logs/<boot-id>and theBOOTfile is there.rc.shutdownstops/etc/init.d/*in reverse order(
sort -r, rc.shutdown:80), so06onboard-logs stop's final snap runs before05onboard-data stopunmounts/data— the last 15 s are still captured.Not in this PR / not claimed
main.
u2-desktop-session/u5-modproberows that need a boot log are pi-release's files; this PR makes their inputexist on every boot that mounts
/dataat all.:18097untouched.