Skip to content

pool: volume holds of a VM whose removal is pending are lost across a sandboxd restart #348

Closed

Description

@CMGS

Problem

A teardown keeps a claim's volume holds until cocoon confirms the VM is gone: release, reap and quarantine hand the holds to removeOrRetry, and a failed vm rm parks them in pendingRemovals until a later reap tick removes the VM (pool/remove.go). That queue lives only in memory.

If sandboxd restarts while a removal is still pending (or between the claims commit that drops the claim and the vm rm), the holds are gone. On startup sweepStaleVMs retries the removal of the unowned VM on every tick, but it reserves nothing for the volumes that VM may still have attached. A new claim of the same name is admitted in that window:

  • a writable claim of a volume the old VM held, or
  • a reader of a volume the old VM held writable.

Why nothing breaks today

Cloud Hypervisor locks a hot-plugged image itself (a write OFD lock for rw, a read lock for ro), and sandboxd attaches every volume by hotplug. The conflicting cocoon vm disk attach fails, so the new claim fails instead of corrupting data. A writable image also keeps its dirty marker until a clean unmount, which refuses read-only claims.

What is wrong is the outcome: the caller gets an attach failure (a 500 on the claim, a destroyed warm VM) instead of a clean 409 "volume is held by another claim", and the safety rests on the hypervisor's lock rather than on sandboxd's admission. The gap predates live volumes and covers claim-time rw volumes too; live additions are read-only and do not widen it.

Options

  1. On Reconcile, before sweeping an unowned sbx- VM, reserve holds for the catalog volumes it still has attached (from cocoon's VM record), and release them when the sweep confirms removal. No journal change, and it covers both the failed-removal and the crash-between-commit-and-rm cases.
  2. Persist pending removals with their holds. This was tried in feat(sandboxd): reconcile live read-only volume attachments #339 as a v2 claims journal and dropped: it broke rolling sandboxd back, added work to every claims commit, and needed a node-wide fence for unknown VMs.

Option 1 looks sufficient. Acceptance: on hardware, fail vm rm for a released rw claim, restart sandboxd, and show that a new rw claim of that volume gets 409 until the old VM is removed, then 200.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions