hive-c0re: polish subvol-upgrade error paths (post-merge review follow-up)

Two non-blocking points from the subvol-upgrade review:

- The start request was `?`-propagated before the migration result was
  surfaced, so a restart-side failure (incl. the IPC call itself
  erroring) could shadow whether the migration succeeded or failed.
  Capture the start result instead and surface the migration outcome
  first; the restart-failure messages now point at `hivectl start
  --agent <name>` for manual recovery.

- The crash-window case (host dies between the two swap renames, leaving
  the agent root missing but the original data under `.<name>.old`) now
  detects the leftover and tells the operator to `mv` it back, instead
  of a bare "no state dir to upgrade — nothing to do".
This commit is contained in:
atlas 2026-06-21 21:10:00 +02:00 committed by mara
commit cac4a4d65a
2 changed files with 37 additions and 7 deletions

View file

@ -1339,23 +1339,38 @@ async fn subvol_upgrade(socket: &Path, name: &str, yes: bool) -> Result<()> {
println!("migrating {name} state dir to a btrfs subvolume…");
let upgrade = hive_c0re::priv_client::upgrade_agent_subvolume(name).await;
// Always restart, even if the migration failed — don't leave the agent down.
// Always attempt the restart, even if the migration failed — don't leave
// the agent down. Capture the result rather than `?`-ing it so a
// start-side failure (incl. the IPC call itself erroring) can't mask the
// migration outcome below.
println!("starting {name}");
let start_resp = hive_c0re::client::request(
let start_result = hive_c0re::client::request(
socket,
hive_sh4re::HostRequest::Start {
scope: single_agent_scope(name),
},
)
.await
.with_context(|| format!("connect to daemon socket {}", socket.display()))?;
.await;
// Surface the migration error first — it's the meaningful one — but only
// after the restart attempt above.
// Surface the migration outcome FIRST — it's the meaningful result and
// must not be shadowed by a restart-side failure. On migration failure the
// original state dir is untouched (the priv op rolls back before the swap).
upgrade.with_context(|| format!("upgrade {name} state subvolume"))?;
// Migration succeeded; now surface any restart problem — either the IPC
// call erroring, or the daemon reporting a failed start. The migration is
// done regardless, so point at the manual recovery.
let start_resp = start_result.with_context(|| {
format!(
"{name} migrated to a btrfs subvolume, but the restart request to the daemon \
socket {} failed run `hivectl start --agent {name}` to bring it back up",
socket.display()
)
})?;
if !start_resp.ok {
bail!(
"migration succeeded but restarting {name} failed: {}",
"{name} migrated to a btrfs subvolume, but restarting it failed: {} — run \
`hivectl start --agent {name}` to retry",
start_resp.error.as_deref().unwrap_or("unknown error")
);
}

View file

@ -797,6 +797,21 @@ async fn upgrade_agent_subvolume(agent_name: &str) -> Result<(String, String)> {
let agent_root = root.join(agent_name);
if !agent_root.exists() {
// A crash between the two swap renames (original → `.<name>.old`
// succeeded, `.<name>.migrating` → agent_root did not) leaves the
// agent root missing but the original data intact under `.<name>.old`.
// Point at the recovery rather than a bare "nothing to do" so the
// operator isn't left guessing where the data went.
let old = root.join(format!(".{agent_name}.old"));
if old.exists() {
bail!(
"no state dir at {agent} — but {old} holds the original data from an \
interrupted upgrade (host crashed mid-swap). Restore it with \
`mv {old} {agent}`, then re-run the upgrade.",
agent = agent_root.display(),
old = old.display(),
);
}
bail!(
"no state dir to upgrade at {} — nothing to do",
agent_root.display()