diff --git a/CHANGELOG.md b/CHANGELOG.md index 859a99c..e46733d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -123,6 +123,40 @@ - `patch/__pycache__/create_task.cpython-314.pyc` was committed to the repository; it is now untracked and `__pycache__/` is gitignored. +### Fixed (post-merge audit) + +- **`create_task.py verify` failed on a default install.** `hook_status.json` + emitted a per-file entry for the nested module with `ok: false` whenever the + feature was switched off — which is the default — so `verify` printed `[FAIL]` + and exited 1 right after the README told users to run it. Status is now + reported per *module* with an `active` flag, and `verify` renders an inactive + module as `[SKIP]` rather than a failure. +- **A partial apply suppressed the middlewared restart.** The exit code + conflated "nothing applied" with "one module applied, one failed", so a failing + providers patch would prevent the restart that a freshly-applied nested patch + needs — leaving it on disk and never loaded. Exit 2 now means partial, and the + restart still fires. +- **The native-nested probe could never fire.** It scanned `crud.py` for the + guard message, but our own injected block *quotes* that message, so once + applied the probe would always conclude the guard was still present. It now + reads only the stock portion of the file. +- `recover.sh` did not unmount staging trees, so an emergency recovery left bind + mounts pinning ZFS snapshots that could then never be destroyed. +- `uninstall.sh` deleted sidecar files without reading them. A sidecar is the + only record that an interrupted run's snapshot tree is still on disk; both + scripts now name the snapshot (`zfs destroy -r ...`) before clearing it. + +### Refactored + +- Staging teardown had been copy-pasted into `uninstall.sh` and `recover.sh` — + two untested shell copies of the fiddly depth-ordering and lazy-umount logic. + Both now call `python3 patch/truecloud_nested.py cleanup`, so there is one + implementation and it is the one under test. +- Dropped the in-memory `ACTIVE` dict. The sidecar file was already the source of + truth; a second in-process record could only desync — and it is the + middlewared-restart case (which empties it) that must not orphan a snapshot + tree. One record, on disk, or none. + ### Known issues - Stock `restic_backup()` deletes the ZFS snapshot in its own `finally`, which diff --git a/README.md b/README.md index 56f11b2..270dd98 100644 --- a/README.md +++ b/README.md @@ -248,7 +248,7 @@ mount | grep truecloud-nested # expect no output | Backup fails: `dataset '…' has no snapshot '…'; refusing to back up an incomplete tree` | Working as designed — a descendant dataset was not covered by the snapshot. The backup is refused rather than silently omitting that data. | | Backup fails: `snapshot '…' cannot be read (Permission denied)` | The snapshot exists but is unreadable. Middleware runs as root, so this indicates a real permissions problem, not a missing snapshot. | | `cloud_backup-*` snapshots accumulating | The sweep is not running. Check `apply.log` for the nested patch applying, and confirm `sync.py` carries the `TRUECLOUD_PATCH` block. | -| Stale mounts under `/run/truecloud-nested` | A crashed run. The next run tears them down; `uninstall.sh` also cleans them. | +| Stale mounts under `/run/truecloud-nested` | A crashed run. The next backup tears them down. To clear them now: `python3 patch/truecloud_nested.py cleanup` (also run by `uninstall.sh` and `recover.sh`). It names any ZFS snapshot an interrupted run left pinned. | ## Supported providers after patching @@ -539,19 +539,28 @@ patch re-runs at next reboot, or you run python3 /mnt/tank/truenas-truecloud-patch/patch/create_task.py verify ``` -If one or more entries show `[FAIL]`: +`verify` reports one line per module: + +| Label | Meaning | +|---|---| +| `[OK ]` | Module is active and applied. | +| `[SKIP]` | Module is inactive — either TrueNAS now does it natively, or it is opt-in and switched off. **Not a failure.** `nested_snapshots` shows SKIP on a default install. | +| `[FAIL]` | Module is needed but did not apply. | + +If a module shows `[FAIL]`: 1. **Check the apply log** for errors during the last boot: ```bash - cat /mnt/tank/truenas-truecloud-patch/apply.log | tail -40 + tail -40 /mnt/tank/truenas-truecloud-patch/apply.log ``` 2. **Check middlewared's own log** for Python tracebacks: ```bash grep -i "truecloud\|traceback\|error" /var/log/middlewared.log 2>/dev/null | tail -30 journalctl -u middlewared -n 50 ``` -3. **A FAIL is non-fatal.** middlewared runs normally; the affected provider - falls back to Storj-only. Your existing backups are not at risk. +3. **A FAIL is non-fatal.** middlewared runs normally and the other module is + unaffected; the failed one is simply inactive. Existing backups are not at + risk. 4. **If the detail says the module doesn't exist**, a TrueNAS update renamed or restructured the internal API. [Open an issue](https://github.com/sudolulo/truenas-truecloud-patch/issues) diff --git a/patch/apply.sh b/patch/apply.sh index 59a408c..412a5b1 100755 --- a/patch/apply.sh +++ b/patch/apply.sh @@ -153,15 +153,22 @@ except Exception: pass # nested: stock gates nested datasets with a validation in plugins/cloud/crud.py. -# We only ever append to that file, so the stock text survives our patch -- if the -# guard is gone, iX removed it, which means they implemented the traversal. +# If that guard is gone, iX removed it, which means they implemented the traversal. +# +# Only look at the STOCK part of the file. Our own CRUD_BLOCK quotes the guard +# message (it filters on it), so scanning the whole file would find the string in +# our own patch and conclude the guard is still there. That happens to be +# harmless today because detection runs before patching, but it makes the probe +# silently order-dependent -- so cut our block off explicitly. +# # If the file cannot be read we assume 'no' and keep patching: worst case the # patch declines to apply and the option simply stays unavailable. try: crud = os.path.join(result['mw_dir'], 'plugins', 'cloud', 'crud.py') with open(crud, encoding='utf-8', errors='replace') as fh: - if 'no further nesting' not in fh.read(): - result['native_nested'] = 'yes' + stock_src = fh.read().split('\n# TRUECLOUD_PATCH', 1)[0] + if 'no further nesting' not in stock_src: + result['native_nested'] = 'yes' except Exception: pass @@ -532,29 +539,21 @@ else: print('WARNING: Stock nesting guard remains; snapshot option stays unavailable') print('WARNING: for nested datasets. Existing backups are unaffected.') +# One entry per MODULE, not per file. `ok` means "nothing is wrong", so a module +# that is inactive (superseded, or opt-in and off) is ok -- reporting a disabled +# opt-in feature as FAIL would make `create_task.py verify` fail on a default +# install. `active` says whether the module is doing anything. patches = { - 'module.providers': { - 'ok': bool(b2_ok and restic_ok) or not providers_needed, + 'providers': { + 'ok': (not providers_needed) or bool(b2_ok and restic_ok), 'active': providers_needed, 'detail': providers_detail, }, - 'module.nested_snapshots': { - 'ok': nested_ok or not nested_needed, + 'nested_snapshots': { + 'ok': (not nested_needed) or nested_ok, 'active': nested_needed, 'detail': nested_detail, }, - 'middlewared.rclone.remote.b2': { - 'ok': b2_ok, - 'detail': 'patched on disk in overlay at boot' if b2_ok else providers_detail, - }, - 'middlewared.plugins.cloud_backup.restic': { - 'ok': restic_ok, - 'detail': 'patched on disk in overlay at boot' if restic_ok else providers_detail, - }, - 'middlewared.plugins.cloud.nested_snapshot': { - 'ok': nested_ok, - 'detail': nested_detail, - }, } payload = {'patched_at': time.strftime('%Y-%m-%dT%H:%M:%SZ', time.gmtime()), 'patches': patches} tmp = status_path + '.tmp' @@ -566,18 +565,34 @@ try: except OSError as e: print(f'WARNING: Could not write hook_status.json: {e}') -# Exit 0 only if every module that is still NEEDED applied cleanly. A module that -# was skipped (superseded or opt-out) is not a failure. -_providers_done = (not providers_needed) or (b2_ok and restic_ok) +# Exit code tells the caller whether a middlewared restart is still worth doing: +# +# 0 every module that was needed applied cleanly +# 2 PARTIAL -- one module failed but another landed, so there IS something new +# on disk waiting to be loaded +# 1 nothing landed; a restart would accomplish nothing +# +# Collapsing 2 into 1 would mean a failing providers patch suppresses the restart +# that a freshly-applied nested patch needs, leaving it on disk and never loaded. +_providers_done = (not providers_needed) or bool(b2_ok and restic_ok) _nested_done = (not nested_needed) or nested_ok -sys.exit(0 if (_providers_done and _nested_done) else 1) +_landed = (providers_needed and b2_ok and restic_ok) or (nested_needed and nested_ok) + +if _providers_done and _nested_done: + sys.exit(0) +sys.exit(2 if _landed else 1) PYEOF then _backend_ok=1 else - # Individual results already printed above; exit code 1 means at least - # one module that was still needed failed to apply. - _backend_ok=0 + _rc=$? + if [ "$_rc" = "2" ]; then + # One module failed, but another was applied and still needs loading. + _backend_ok=1 + echo "WARNING: a module failed to apply; the other landed and will be loaded." + else + _backend_ok=0 + fi fi fi @@ -625,15 +640,17 @@ fi echo "--- deferred restart ---" -# Restart when ANY still-needed backend module landed. Keying this off the -# providers module alone would skip the restart on a box where B2 has gone native -# but the nested module was freshly patched — leaving it on disk and never loaded. +# Restart when ANY still-needed backend module landed (_backend_ok, incl. the +# partial case). Keying this off the providers module alone would skip the restart +# on a box where B2 has gone native but the nested module was freshly patched — +# leaving it on disk and never loaded. +# +# "No module active at all" cannot reach here: that is the kill-switch branch +# above, which exits. if ! grep -aq middlewared "/proc/$PPID/cmdline" 2>/dev/null; then echo "Manual run (parent is not middlewared) — no restart scheduled." -elif [ "$_providers_needed" = "0" ] && [ "$_nested_needed" = "0" ]; then - echo "No backend module active — no restart scheduled (nothing new to load)." -elif [ "${_backend_ok:-0}" != "1" ]; then - echo "Backend patch incomplete — no restart scheduled (nothing new to load)." +elif [ "$_backend_ok" != "1" ]; then + echo "Nothing landed on disk — no restart scheduled (nothing new to load)." else # A failed unit from an earlier attempt this boot would block systemd-run. systemctl reset-failed truecloud-mw-restart.service 2>/dev/null diff --git a/patch/create_task.py b/patch/create_task.py index 65943ff..c0ad384 100755 --- a/patch/create_task.py +++ b/patch/create_task.py @@ -127,13 +127,21 @@ def cmd_verify(): print(f"Hook status (recorded at {status.get('patched_at', 'unknown')})") print() all_ok = True + any_active = False for module, info in status.get("patches", {}).items(): ok = info.get("ok", False) + # A module can be inactive because TrueNAS now does it natively, or + # because it is opt-in and switched off. Neither is a failure. + active = info.get("active", True) label = "OK " if ok else "FAIL" + if ok and not active: + label = "SKIP" detail = f" — {info['detail']}" if info.get("detail") else "" print(f" [{label}] {module}{detail}") if not ok: all_ok = False + if active: + any_active = True # The disk status alone can false-positive: at boot the files are patched # while middlewared is already running with the stock modules imported. @@ -146,7 +154,11 @@ def cmd_verify(): mw_start = _middlewared_start_epoch() proc_stale = False - if patched_epoch is None or mw_start is None: + if not any_active: + # Nothing is patched into middlewared, so whether it restarted since is + # irrelevant -- there is nothing for it to have loaded. + print(" [-- ] running middlewared process — no active module; nothing to load") + elif patched_epoch is None or mw_start is None: print(" [?? ] running middlewared process — could not compare start time;") print(" the results above reflect the on-disk state only") elif mw_start + 2 < patched_epoch: diff --git a/patch/truecloud_nested.py b/patch/truecloud_nested.py index df02c6a..8995a81 100644 --- a/patch/truecloud_nested.py +++ b/patch/truecloud_nested.py @@ -60,10 +60,10 @@ import stat import subprocess __all__ = [ - "ACTIVE", "STAGING_BASE", "StagingError", "apply_plan", + "cleanup_all", "cleanup_task", "current_mounts_under", "delete_snapshot_tree", @@ -79,9 +79,10 @@ __all__ = [ #: Where staging trees are assembled. tmpfs; bind mounts consume no space. STAGING_BASE = "/run/truecloud-nested" -#: staging_root -> zfs snapshot name. A cache; the sidecar file is the source of -#: truth, so that a middlewared restart cannot orphan a snapshot. -ACTIVE: dict[str, str] = {} +# Which snapshot a staging tree pins is recorded ONLY in the sidecar file, never +# also in memory. An in-process dict would be a second source of truth that a +# middlewared restart silently empties -- and it is exactly the restart case that +# must not orphan a 250-snapshot tree. One record, on disk, or none. class StagingError(Exception): @@ -459,8 +460,6 @@ async def stage_nested(middleware, path, snapshot, base_dataset, base_mountpoint await middleware.run_in_thread(_remove_sidecar, staging_root) raise - ACTIVE[staging_root] = snapshot - if logger: logger.info( "truecloud-patch: staged %d dataset(s) from %s at %s", @@ -475,12 +474,7 @@ async def cleanup_task(middleware, task_name, logger=None): Safe to call unconditionally: a no-op when the task was never staged. """ staging_root = staging_root_for(task_name) - sidecar = sidecar_for(staging_root) - - snapshot = ACTIVE.pop(staging_root, None) - if snapshot is None: - # Sidecar survives a middlewared restart; ACTIVE does not. - snapshot = _read_sidecar(staging_root) + snapshot = _read_sidecar(staging_root) if snapshot is None and not os.path.isdir(staging_root): return # never staged; nothing to do @@ -493,5 +487,65 @@ async def cleanup_task(middleware, task_name, logger=None): if snapshot is not None: await delete_snapshot_tree(middleware, snapshot, logger=logger) - with contextlib.suppress(OSError): - os.unlink(sidecar) + _remove_sidecar(staging_root) + + +# ── offline cleanup (uninstall.sh / recover.sh) ─────────────────────────────── + + +def cleanup_all(base=None, runner=_run, mounts_file="/proc/self/mounts", + glob_fn=None, read_sidecar=_read_sidecar): + """Tear down every staging tree. Used by uninstall.sh and recover.sh. + + Those scripts must work when middlewared is dead, so they cannot go through + the async path -- but they must not reimplement the teardown either: the + depth-ordering and lazy-umount fallback are fiddly, and a second copy in + shell would be the untested one. This is the same tested code. + + Returns ``(lines, errors)``: report lines to print, and unmount errors. + """ + import glob as _glob + + base = base or STAGING_BASE + glob_fn = glob_fn or _glob.glob + lines = [] + + # Report orphaned snapshots BEFORE removing the sidecars that name them -- + # a sidecar is the only record that an interrupted run's snapshot tree (one + # snapshot per descendant dataset) is still on disk. + for sc in sorted(glob_fn(os.path.join(base, "*.snapshot"))): + snap = read_sidecar(sc[: -len(".snapshot")]) + if snap: + lines.append(f" NOTE: an interrupted backup left snapshot '{snap}' behind.") + lines.append(f" Remove it and its children: zfs destroy -r '{snap}'") + + mounts = current_mounts_under(base, mounts_file=mounts_file) + if not mounts: + lines.append(" None active.") + for mp in mounts: + lines.append(f" Unmounting: {mp}") + + errors = teardown(base, runner=runner, mounts_file=mounts_file) + for err in errors: + lines.append(f" WARNING: could not unmount {err}") + + if not errors: + for sc in glob_fn(os.path.join(base, "*.snapshot")): + with contextlib.suppress(OSError): + os.unlink(sc) + with contextlib.suppress(OSError): + os.rmdir(base) + + return lines, errors + + +if __name__ == "__main__": + import sys + + if len(sys.argv) > 1 and sys.argv[1] == "cleanup": + _lines, _errors = cleanup_all() + for _line in _lines: + print(_line) + sys.exit(1 if _errors else 0) + print("usage: truecloud_nested.py cleanup", file=sys.stderr) + sys.exit(2) diff --git a/recover.sh b/recover.sh index cf54b64..a6a6a0f 100755 --- a/recover.sh +++ b/recover.sh @@ -21,6 +21,7 @@ VERSION="0.3.0" PATCH_DIR="$(cd "$(dirname "$0")" && pwd)" + echo "=== TrueNAS TrueCloud Provider Patch v${VERSION} — Recover ===" echo "" @@ -52,6 +53,14 @@ for _tag in mw ui; do done [ "$_any" -eq 0 ] && echo " No overlays active." +# Nested-snapshot staging trees are bind mounts that PIN their ZFS snapshots, so +# leaving them mounted blocks those snapshots from ever being destroyed. The +# overlays above are volatile, but these are not self-healing without a reboot, +# and recover.sh is expected to work without one. +echo "Unmounting nested-snapshot staging trees ..." +# Best-effort: never block recovery. Same tested implementation as uninstall.sh. +python3 "$PATCH_DIR/patch/truecloud_nested.py" cleanup || true + # Cancel a deferred boot restart if one is still queued — we restart ourselves. systemctl stop truecloud-mw-restart.service 2>/dev/null systemctl reset-failed truecloud-mw-restart.service 2>/dev/null diff --git a/tests/test_apply_blocks.py b/tests/test_apply_blocks.py index c59e652..4dbac85 100644 --- a/tests/test_apply_blocks.py +++ b/tests/test_apply_blocks.py @@ -144,6 +144,24 @@ class TestIndependentModules: i = sh.index("--- UI patch ---") assert '[ "$_providers_needed" = "0" ]' in sh[i:i + 400] + def test_status_reports_an_inactive_module_as_ok(self): + # `create_task.py verify` fails if any patches[*].ok is false. An opt-in + # module that is switched off (the DEFAULT) must not report FAIL, or a + # stock install fails verification out of the box. + src = heredoc_source() + assert "'ok': (not nested_needed) or nested_ok" in src + assert "'ok': (not providers_needed) or bool(b2_ok and restic_ok)" in src + assert "'active': nested_needed" in src + + def test_nested_native_probe_ignores_our_own_block(self): + # CRUD_BLOCK quotes the guard message, so scanning the whole file would + # find the string in our own patch and never detect native support. + sh = self._sh() + assert "split('\\n# TRUECLOUD_PATCH', 1)[0]" in sh + assert "no further nesting" in extract_blocks()["CRUD_BLOCK"], ( + "if this ever stops being true, the probe comment is stale" + ) + def test_restart_fires_when_any_needed_module_landed(self): # Keying the restart off providers alone would leave a freshly-patched # nested module on disk and never loaded on a native-B2 box. @@ -153,6 +171,18 @@ class TestIndependentModules: assert '_backend_ok' in tail assert '"$_b2_ok"' not in tail + def test_partial_failure_still_schedules_the_restart(self): + # If providers fails but nested landed (or vice versa), something new IS + # on disk. Collapsing that into "nothing to do" would leave the module + # that succeeded permanently unloaded. + src = heredoc_source() + assert "sys.exit(2 if _landed else 1)" in src + assert "_landed = (providers_needed and b2_ok and restic_ok) or (nested_needed and nested_ok)" in src + + sh = self._sh() + assert '_rc=$?' in sh + assert '[ "$_rc" = "2" ]' in sh + class TestOptIn: """Nested-snapshot support must be opt-in and must never self-enable.""" diff --git a/tests/test_truecloud_nested.py b/tests/test_truecloud_nested.py index 217aa6f..1e2f0f6 100644 --- a/tests/test_truecloud_nested.py +++ b/tests/test_truecloud_nested.py @@ -21,9 +21,9 @@ import pytest sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..", "patch")) from truecloud_nested import ( # noqa: E402 - ACTIVE, StagingError, apply_plan, + cleanup_all, cleanup_task, current_mounts_under, delete_snapshot_tree, @@ -255,9 +255,6 @@ class TestDeleteSnapshotTree: class TestStageNestedOrdering: - def setup_method(self): - ACTIVE.clear() - def test_sidecar_is_written_before_anything_is_mounted(self, tmp_path, monkeypatch): # middlewared can die at any moment. If the snapshot were recorded only # after apply_plan, a crash in that window would orphan a 160-snapshot @@ -344,13 +341,10 @@ class TestStageNestedOrdering: class TestCleanupTask: - def setup_method(self): - ACTIVE.clear() - def test_recovers_snapshot_from_sidecar_after_middlewared_restart(self, tmp_path, monkeypatch): - # ACTIVE is in-process; a restart wipes it. The sidecar is the source of - # truth, otherwise the snapshot tree is orphaned forever. + # The sidecar is the ONLY record of the pinned snapshot, precisely so a + # middlewared restart cannot orphan the tree. import truecloud_nested as tn monkeypatch.setattr(tn, "STAGING_BASE", str(tmp_path)) @@ -359,7 +353,6 @@ class TestCleanupTask: with open(sidecar_for(root), "w", encoding="utf-8") as fh: fh.write("Tap@snap") - ACTIVE.clear() # simulate the restart mw = FakeMiddleware(["Tap@snap", "Tap/apps@snap"]) monkeypatch.setattr(tn, "teardown", lambda *_a, **_k: []) @@ -489,6 +482,86 @@ class TestTeardown: assert ["umount", "-l", ROOT] in runner.calls +class TestCleanupAll: + """uninstall.sh and recover.sh call this instead of reimplementing teardown.""" + + def test_reports_orphan_snapshots_before_deleting_their_sidecars(self, tmp_path): + # The sidecar is the only record that an interrupted run's snapshot tree + # is still on disk. Deleting it without naming the snapshot orphans the + # whole tree silently. + base = tmp_path / "stage" + base.mkdir() + (base / "cloud_backup-5.snapshot").write_text("Tap@interrupted") + + mounts_file = tmp_path / "mounts" + mounts_file.write_text("") + + lines, errors = cleanup_all( + base=str(base), runner=FakeRunner(), mounts_file=str(mounts_file) + ) + assert errors == [] + assert any("Tap@interrupted" in ln for ln in lines) + assert any("zfs destroy -r" in ln for ln in lines) + # Sidecar cleared only after being reported. + assert not (base / "cloud_backup-5.snapshot").exists() + + def test_unmounts_everything_under_the_base_deepest_first(self, tmp_path): + base = tmp_path / "stage" + base.mkdir() + mounts_file = tmp_path / "mounts" + mounts_file.write_text( + f"tmpfs {base} tmpfs rw 0 0\n" + f"tmpfs {base}/cloud_backup-5 tmpfs rw 0 0\n" + f"tmpfs {base}/cloud_backup-5/apps tmpfs rw 0 0\n" + ) + runner = FakeRunner() + _lines, errors = cleanup_all( + base=str(base), runner=runner, mounts_file=str(mounts_file) + ) + assert errors == [] + order = [c[-1] for c in runner.calls if c[0] == "umount"] + assert order == [ + f"{base}/cloud_backup-5/apps", + f"{base}/cloud_backup-5", + str(base), + ] + + def test_keeps_sidecars_when_an_unmount_failed(self, tmp_path): + # If a mount is stuck, the snapshot is still pinned — so the record of it + # must survive for the next run (or the operator) to act on. + base = tmp_path / "stage" + base.mkdir() + (base / "cloud_backup-5.snapshot").write_text("Tap@stuck") + mounts_file = tmp_path / "mounts" + mounts_file.write_text(f"tmpfs {base}/cloud_backup-5 tmpfs rw 0 0\n") + + class Stuck(FakeRunner): + def __call__(self, cmd): + self.calls.append(cmd) + + class R: + returncode = 32 + stderr = "target is busy" + + return R + + _lines, errors = cleanup_all( + base=str(base), runner=Stuck(), mounts_file=str(mounts_file) + ) + assert errors, "a stuck unmount must be reported" + assert (base / "cloud_backup-5.snapshot").exists() + + def test_is_a_noop_on_a_clean_system(self, tmp_path): + mounts_file = tmp_path / "mounts" + mounts_file.write_text("") + lines, errors = cleanup_all( + base=str(tmp_path / "absent"), runner=FakeRunner(), + mounts_file=str(mounts_file), + ) + assert errors == [] + assert lines == [" None active."] + + class TestCurrentMountsUnder: def test_matches_only_the_staging_subtree(self, tmp_path): mounts_file = tmp_path / "mounts" diff --git a/uninstall.sh b/uninstall.sh index 9c1e866..3c314d2 100755 --- a/uninstall.sh +++ b/uninstall.sh @@ -95,38 +95,13 @@ echo "" # These bind mounts pin their ZFS snapshots, so they must go before anything # tries to destroy those snapshots. Deepest first. +# Delegated to the patch module rather than reimplemented here: the depth +# ordering and lazy-umount fallback are fiddly, and a shell copy would be the +# untested one. echo "Unmounting nested-snapshot staging trees (if any) ..." -_stage_found=0 -_stage_failed=0 -# Deepest FIRST, by path depth (slash count) — not string length, which would -# let a long shallow path jump ahead of a short deep one and leave a child -# mounted (and its ZFS snapshot pinned). -while IFS= read -r _mp; do - [ -n "$_mp" ] || continue - if umount "$_mp" 2>/dev/null || umount -l "$_mp" 2>/dev/null; then - echo " Unmounted: $_mp" - else - echo " WARNING: Could not unmount $_mp" - _stage_failed=1 - fi - _stage_found=1 -done < <(awk '$2 == "/run/truecloud-nested" || index($2, "/run/truecloud-nested/") == 1 { - n = gsub(/\//, "/", $2); print n, $2 - }' /proc/self/mounts 2>/dev/null | sort -rn | cut -d' ' -f2-) - -if [ "$_stage_found" -eq 0 ]; then - echo " None active." -fi - -# NEVER `rm -rf` here: if an unmount failed, that would recurse *through* a live -# bind mount into the ZFS snapshot behind it. Remove empty directories only. -if [ "$_stage_failed" -eq 0 ]; then - find /run/truecloud-nested -depth -type d -exec rmdir {} + 2>/dev/null || true - rm -f /run/truecloud-nested/*.snapshot 2>/dev/null || true - rmdir /run/truecloud-nested 2>/dev/null || true -else - echo " WARNING: staging mounts remain; leaving /run/truecloud-nested in place." - echo " Unmount them manually, then remove the directory." +if ! python3 "$PATCH_DIR/patch/truecloud_nested.py" cleanup; then + echo " WARNING: staging mounts remain. Unmount them manually; until you do," + echo " the ZFS snapshots they pin cannot be destroyed." fi # The opt-in marker lives in the repo dir; remove it so a later re-install