From c6d94353165e8a5cc1d1570f0ba2c47b503607fc Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Tue, 13 Oct 2026 19:36:01 +0200 Subject: [PATCH] Host power-off: re-check jobs before each step; cancel if one is still running (R-998) Security review of f38b03c: a controller update, drive/NAS job, escrow ceremony or Proxmox vzdump could start between acceptance and the power-off (the heavy gate does not cover them). runHostPowerOff now asks jobBlocker again before the guest shutdown and before the host power-off, waits up to 15 min, and CANCELS (gate released, box stays on) if still busy. Red-proofed: no wait, and a gate kept after a cancel, each convicted. --- internal/localapi/hostpoweroff.go | 46 ++++++++++++++++++++++++ internal/localapi/hostpoweroff_test.go | 48 ++++++++++++++++++++++++++ 2 files changed, 94 insertions(+) diff --git a/internal/localapi/hostpoweroff.go b/internal/localapi/hostpoweroff.go index c28a5c4..30bde6f 100644 --- a/internal/localapi/hostpoweroff.go +++ b/internal/localapi/hostpoweroff.go @@ -50,6 +50,8 @@ type HostPowerOffConfig struct { MinUptime time.Duration // default 10 min GuestStop time.Duration // pct shutdown --timeout; default 120 s ReportMax time.Duration // longest wait for the hub; default 60 s + JobWait time.Duration // longest wait for a job that started after acceptance; default 15 min + JobPoll time.Duration // how often that wait looks again; default 10 s Now func() time.Time } @@ -82,6 +84,12 @@ func (s *Server) SetHostPowerOff(c HostPowerOffConfig) { if c.Now == nil { c.Now = time.Now } + if c.JobWait == 0 { + c.JobWait = 15 * time.Minute + } + if c.JobPoll == 0 { + c.JobPoll = 10 * time.Second + } if c.Running == nil { c.Running = func() ([]int, error) { return runningGuestsFromProc("/proc") } } @@ -155,6 +163,10 @@ func (s *Server) runHostPowerOff(base context.Context, vmid int, release func()) cancel() s.logger.Warn("host-poweroff: step 1/3 — hub told", "vmid", vmid, "hub_accepted", ok) } + if !s.waitJobsClear(base, vmid, "before the guest shutdown") { + release() + return + } start := time.Now() ctx, cancel := context.WithTimeout(base, c.GuestStop+60*time.Second) err := c.Power.ShutdownGuest(ctx, vmid, c.GuestStop) @@ -165,6 +177,10 @@ func (s *Server) runHostPowerOff(base context.Context, vmid int, release func()) } else { s.logger.Warn("host-poweroff: step 2/3 — guest stopped", "vmid", vmid, "took", time.Since(start).Round(time.Second)) } + if !s.waitJobsClear(base, vmid, "before the host power-off") { + release() + return + } s.logger.Warn("host-poweroff: step 3/3 — powering the host off") ctx, cancel = context.WithTimeout(base, 60*time.Second) defer cancel() @@ -218,6 +234,13 @@ func (s *Server) powerOffBlocker(ctx context.Context, vmid int) (string, string) return PowerOffBusy, busy } } + return s.jobBlocker(ctx, vmid) +} + +// jobBlocker is the part of the checks that can change after acceptance — a backup job, a controller update, a drive +// or NAS job, an escrow ceremony, a Proxmox vzdump. runHostPowerOff asks it again before each step (security review +// 2026-10-13: the checks ran only once). The heavy-operation gate is not here: the power-off holds it. +func (s *Server) jobBlocker(ctx context.Context, vmid int) (string, string) { s.jobsMu.Lock() for k, j := range s.jobs { if j != nil && backupInFlight(j.Phase) { @@ -300,3 +323,26 @@ func runningGuestsFromProc(procDir string) ([]int, error) { } return out, nil } + +// waitJobsClear waits until jobBlocker finds nothing, at most JobWait. false = still busy (or unreadable) at the end: +// the power-off is CANCELLED — the box stays on, the gate is released, the log says why. Better a box that did not +// switch off than a backup, update or format cut in the middle. +func (s *Server) waitJobsClear(base context.Context, vmid int, when string) bool { + c := s.powerOff + ctx, cancel := context.WithTimeout(base, c.JobWait) + defer cancel() + for { + code, detail := s.jobBlocker(ctx, vmid) + if code == "" { + return true + } + s.logger.Warn("host-poweroff: waiting — a job started after the request", "when", when, "reason", code, "detail", detail) + select { + case <-ctx.Done(): + s.logger.Error("host-poweroff: CANCELLED — still busy after the wait; the box stays on", "when", when, + "reason", code, "detail", detail, "waited", c.JobWait) + return false + case <-time.After(c.JobPoll): + } + } +} diff --git a/internal/localapi/hostpoweroff_test.go b/internal/localapi/hostpoweroff_test.go index f2c8f93..cae5b99 100644 --- a/internal/localapi/hostpoweroff_test.go +++ b/internal/localapi/hostpoweroff_test.go @@ -10,6 +10,7 @@ import ( "sort" "strings" "sync" + "sync/atomic" "testing" "time" @@ -280,3 +281,50 @@ func TestRunningGuestsFromProc(t *testing.T) { t.Fatal("an unreadable /proc must be an error") } } + +// switchVzdump is a vzdump reader the test can flip while the power-off runs. +type switchVzdump struct { + fakeVzdump + on *atomic.Bool +} + +func (f switchVzdump) BackupRunning(context.Context, int) (bool, error) { return f.on.Load(), nil } + +// A job that starts AFTER acceptance (security review 2026-10-13): the power-off waits; if the job is still running +// when the wait ends, the power-off is cancelled — no guest shutdown, no host power-off, the gate given back. +func TestHostPowerOff_JobStartedAfterAcceptance(t *testing.T) { + // (a) it clears during the wait → the power-off goes ahead after it + r := newPowerRig(t, time.Hour) + var on atomic.Bool + r.srv.staleLock = switchVzdump{on: &on} + r.srv.powerOff.JobWait, r.srv.powerOff.JobPoll = 2*time.Second, 2*time.Millisecond + r.srv.powerOff.ReportNow = func(context.Context) bool { on.Store(true); return true } // a vzdump starts now + if w := do(t, r.h, "POST", "/host/poweroff", "A", ""); w.Code != http.StatusAccepted { + t.Fatalf("accept: %d %s", w.Code, w.Body.String()) + } + time.Sleep(30 * time.Millisecond) + if s := r.power.got(); len(s) != 0 { + t.Fatalf("a step ran while the vzdump ran: %v", s) + } + on.Store(false) + if s := r.waitSteps(t, 2); len(s) != 2 { + t.Fatalf("after the vzdump ended the steps = %v, want shutdown + poweroff", s) + } + + // (b) it never clears → cancelled, nothing switched off, the heavy gate free again + r2 := newPowerRig(t, time.Hour) + var on2 atomic.Bool + r2.srv.staleLock = switchVzdump{on: &on2} + r2.srv.powerOff.JobWait, r2.srv.powerOff.JobPoll = 40*time.Millisecond, 2*time.Millisecond + r2.srv.powerOff.ReportNow = func(context.Context) bool { on2.Store(true); return true } + if w := do(t, r2.h, "POST", "/host/poweroff", "A", ""); w.Code != http.StatusAccepted { + t.Fatalf("accept: %d", w.Code) + } + time.Sleep(150 * time.Millisecond) + if s := r2.power.got(); len(s) != 0 { + t.Fatalf("cancelled power-off still ran: %v", s) + } + if busy := r2.srv.inFlight.Busy(); busy != "" { + t.Fatalf("the heavy gate is still held after a cancel: %q", busy) + } +}