diff --git a/aci_edge_sandboxes/README.md b/aci_edge_sandboxes/README.md index 33b39237f..dbc06c5c1 100644 --- a/aci_edge_sandboxes/README.md +++ b/aci_edge_sandboxes/README.md @@ -965,6 +965,10 @@ egress default of `allow` and no deny rule that covers the guest network; see - Deprovisioning a running sandbox fails with `already_started`; stop it first. After deprovision, every call fails with `stale_id` without writing any state, as do calls with IDs that were never provisioned. +- Deprovision removes the sandbox's files, its record last, and then its state + directory. Files in that directory that are not the backend's stay: the + deprovision fails with `backend_error`, naming them, and the ID is stale + from then on. - Lifecycle transitions of one sandbox are serialized by a lock file. - Executions run one at a time: a concurrent exec waits up to `control_timeout` for the running one to finish. diff --git a/aci_edge_sandboxes/src/openvmm/state.rs b/aci_edge_sandboxes/src/openvmm/state.rs index 6c2fd3431..9bd7d1a9c 100644 --- a/aci_edge_sandboxes/src/openvmm/state.rs +++ b/aci_edge_sandboxes/src/openvmm/state.rs @@ -20,11 +20,12 @@ use std::fs; use std::io; use std::path::{Path, PathBuf}; +use std::time::Instant; use nvx::host::filesystem::ExportPlan; use nvx::host::instance::{self, Instance, LockedInstance}; use nvx::host::platform; -use nvx::host::store::{FILE_NAMES, LOCK_NAME, LOG_NAME}; +use nvx::host::store::{CallerFiles, LOG_NAME, RemovalError}; use serde::{Deserialize, Serialize}; use crate::error::{Error, ErrorCode, Result}; @@ -73,31 +74,14 @@ pub(crate) struct StateStore { root: PathBuf, } -fn io_error(context: String) -> impl FnOnce(io::Error) -> Error { - move |error| Error::backend_error(context).with_source(error) -} - fn not_provisioned(sandbox_id: &SandboxId) -> Error { Error::stale_id(format!("sandbox {sandbox_id} is not provisioned")) } -/// Removes the directory `dir` of a sandbox whose configuration is gone, and returns whether it -/// is gone. -/// -/// The directory stays while it is not empty: a transition that waited for the sandbox's lock may -/// hold a lock file in it, and a file system without POSIX deletion keeps a removed lock file -/// there until its last handle closes. Each such transition finds no configuration, and removes -/// its lock file and the directory as it releases the lock. -fn remove_emptied(dir: &Path) -> Result { - match fs::remove_dir(dir) { - Ok(()) => Ok(true), - Err(error) if error.kind() == io::ErrorKind::NotFound => Ok(true), - Err(error) if error.kind() == io::ErrorKind::DirectoryNotEmpty => Ok(false), - Err(error) => Err(io_error(format!( - "cannot remove sandbox state {}", - dir.display() - ))(error)), - } +/// Returns the files that this crate keeps in a sandbox's directory beside the store of its +/// instance: the configuration, whose presence means that the sandbox is provisioned. +fn files() -> CallerFiles { + CallerFiles::new(RECORD_NAME).expect("the configuration has a plain file name") } impl StateStore { @@ -126,21 +110,21 @@ impl StateStore { /// /// An ID without configuration fails with [`ErrorCode::StaleId`], also when its state /// disappears while this call waits for the lock, as it does when a concurrent deprovision - /// removes it. A stale ID never becomes valid again, so the call removes the lock file that - /// it may have created, and the sandbox's directory, such as one that an interrupted removal - /// left, if that empties it: a stale ID leaves nothing behind. + /// removes it. A stale ID never becomes valid again, so the call removes what is left of the + /// sandbox's directory, such as one that an interrupted removal left, or the temporary file of + /// a configuration that a provision did not finish writing, as [`remove`](Self::remove) + /// removes a sandbox: a stale ID leaves nothing behind but entries that are not the sandbox's. pub(crate) fn lock_and_load( &self, sandbox_id: &SandboxId, ) -> Result<(LockedInstance, SandboxRecord)> { let dir = self.dir(sandbox_id); - let locked = match Instance::open(&dir, sandbox_id.to_string()) + let locked = match Instance::open(&dir, sandbox_id.to_string(), files()) .and_then(|instance| instance.lock(None)) { Ok(locked) => locked, - // The directory or its lock file went away, or never existed, with the sandbox. + // The directory went away, or never existed, with the sandbox. Err(_) if matches!(dir.join(RECORD_NAME).try_exists(), Ok(false)) => { - let _ = remove_emptied(&dir); return Err(not_provisioned(sandbox_id)); } Err(error) => return Err(error.into()), @@ -148,9 +132,8 @@ impl StateStore { match self.load(sandbox_id) { Ok(record) => Ok((locked, record)), Err(error) if error.code() == ErrorCode::StaleId => { - let _ = fs::remove_file(dir.join(LOCK_NAME)); - drop(locked); - let _ = remove_emptied(&dir); + let _ = instance::remove_interrupted_writes(&dir.join(RECORD_NAME)); + let _ = locked.remove(Some(Instant::now())); Err(error) } Err(error) => Err(error), @@ -168,7 +151,7 @@ impl StateStore { dir.display() ))); } - let created = Instance::create(&dir, sandbox_id.to_string()) + let created = Instance::create(&dir, sandbox_id.to_string(), files()) .and_then(|_| instance::write_json(&dir.join(RECORD_NAME), record)); if created.is_err() { let _ = fs::remove_dir_all(&dir); @@ -198,51 +181,29 @@ impl StateStore { }) } - /// Deletes the state of a stopped sandbox, whose lock `locked` holds, and releases the lock. + /// Deletes the state of a stopped sandbox, whose lock `locked` holds, through the store of its + /// instance, and releases the lock. /// - /// Unknown files abort the removal before anything is deleted. The configuration goes after - /// the instance's files, so an interrupted removal leaves a sandbox that can be deprovisioned - /// again, and the lock file goes last, while it is held, so that a transition that waits for - /// the lock finds no configuration. + /// The temporary files that interrupted writes of the configuration left go first, since the + /// store does not own them. The store then removes its files and the configuration, so an + /// interrupted removal leaves a sandbox that can be deprovisioned again. It then refuses + /// entries that are neither the store's nor this crate's, which it keeps with the directory, + /// and the ID is stale from then on. Otherwise it removes the lock file while it still holds + /// the lock, so that a transition that waits for the lock finds no sandbox, and then the + /// directory, waiting for the open files, such as the lock files of such transitions, that + /// keep it for a while, as it waits for the lock itself. pub(crate) fn remove(&self, sandbox_id: &SandboxId, locked: LockedInstance) -> Result<()> { let dir = self.dir(sandbox_id); - let entries = fs::read_dir(&dir).map_err(io_error(format!( - "cannot list sandbox state {}", - dir.display() - )))?; - let mut owned = Vec::new(); - for entry in entries { - let entry = entry.map_err(io_error(format!( - "cannot list sandbox state {}", - dir.display() - )))?; - let name = entry.file_name(); - let name = name.to_string_lossy(); - let temporary = name.starts_with('.') && name.ends_with(".tmp"); - if name == RECORD_NAME || name == LOCK_NAME { - continue; + instance::remove_interrupted_writes(&dir.join(RECORD_NAME))?; + locked.remove(None).map_err(|error| { + let message = format!("cannot remove sandbox state {}: {error}", dir.display()); + match error { + RemovalError::TimedOut(source) | RemovalError::Io { error: source, .. } => { + Error::backend_error(message).with_source(source) + } + _ => Error::backend_error(message), } - if FILE_NAMES.contains(&name.as_ref()) || temporary { - owned.push(entry.path()); - } else { - return Err(Error::backend_error(format!( - "sandbox state {} contains the unexpected entry {name:?}; remove it and retry", - dir.display() - ))); - } - } - for path in owned { - instance::remove_if_present(&path)?; - } - instance::remove_if_present(&dir.join(RECORD_NAME))?; - instance::remove_if_present(&dir.join(LOCK_NAME))?; - // The directory goes while the lock is held where it can, and once it is released where - // the removed lock file stays until its last handle closes. - if !remove_emptied(&dir)? { - drop(locked); - remove_emptied(&dir)?; - } - Ok(()) + }) } } @@ -253,6 +214,7 @@ mod tests { use std::time::{Duration, Instant}; use nvx::ErrorKind; + use nvx::host::store::LOCK_NAME; use super::*; use crate::ErrorCode; @@ -320,16 +282,30 @@ mod tests { } #[test] - fn removal_preserves_unknown_files_and_the_sandbox() { + fn removal_refuses_unknown_files_and_keeps_them() { let root = tempfile::tempdir().unwrap(); let store = StateStore::open(root.path()).unwrap(); let id = SandboxId::generate().unwrap(); store.create(&id, &record()).unwrap(); - fs::write(store.dir(&id).join("keep.txt"), b"user data").unwrap(); + let dir = store.dir(&id); + fs::write(dir.join("openvmm.log"), b"left").unwrap(); + fs::write(dir.join("keep.txt"), b"user data").unwrap(); let (locked, _) = store.lock_and_load(&id).unwrap(); - assert!(store.remove(&id, locked).is_err()); - assert!(store.dir(&id).join("keep.txt").exists()); - assert_eq!(store.load(&id).unwrap(), record()); + let error = store.remove(&id, locked).unwrap_err(); + assert_eq!(error.code(), ErrorCode::BackendError); + assert!(error.message().ends_with(": keep.txt"), "{error}"); + // The sandbox's own files went first, its configuration last, so the ID is stale, and the + // directory keeps the unknown file alone. + assert_eq!(entries(&dir), ["keep.txt"]); + assert_eq!(fs::read(dir.join("keep.txt")).unwrap(), b"user data"); + let error = store.lock_and_load(&id).unwrap_err(); + assert_eq!(error.code(), ErrorCode::StaleId, "{error}"); + assert_eq!(entries(&dir), ["keep.txt"]); + // Once it is gone, the stale ID leaves nothing behind. + fs::remove_file(dir.join("keep.txt")).unwrap(); + let error = store.lock_and_load(&id).unwrap_err(); + assert_eq!(error.code(), ErrorCode::StaleId, "{error}"); + assert!(entries(root.path()).is_empty()); } #[test] @@ -431,7 +407,7 @@ mod tests { let (locked, loaded) = store.lock_and_load(&id).unwrap(); assert_eq!(loaded, record()); // The lock is the lock of the instance's store. - let contender = Instance::open(store.dir(&id), id.to_string()).unwrap(); + let contender = Instance::open(store.dir(&id), id.to_string(), files()).unwrap(); let error = contender.lock(Some(Instant::now())).unwrap_err(); assert_eq!(error.kind(), ErrorKind::TimedOut, "{error}"); drop(locked); @@ -447,9 +423,9 @@ mod tests { // An interrupted removal can leave an empty directory, or one with only its lock file, // which are stale too. let emptied = SandboxId::generate().unwrap(); - fs::create_dir(store.dir(&emptied)).unwrap(); + Instance::create(store.dir(&emptied), emptied.to_string(), files()).unwrap(); let locked_only = SandboxId::generate().unwrap(); - Instance::create(store.dir(&locked_only), locked_only.to_string()).unwrap(); + Instance::create(store.dir(&locked_only), locked_only.to_string(), files()).unwrap(); fs::write(store.dir(&locked_only).join(LOCK_NAME), b"").unwrap(); for id in [&never_provisioned, &emptied, &locked_only] { let error = store.lock_and_load(id).unwrap_err(); @@ -463,6 +439,42 @@ mod tests { assert_eq!(entries(&store.dir(&id)), [RECORD_NAME, LOCK_NAME]); } + #[test] + fn interrupted_configuration_writes_go_with_the_sandbox() { + let root = tempfile::tempdir().unwrap(); + let store = StateStore::open(root.path()).unwrap(); + // A provision that died while it wrote the configuration left its temporary file, and no + // configuration, so the ID is stale, and leaves nothing behind. + let interrupted = SandboxId::generate().unwrap(); + Instance::create(store.dir(&interrupted), interrupted.to_string(), files()).unwrap(); + fs::write( + store.dir(&interrupted).join(".sandbox.json.12345.tmp"), + b"{", + ) + .unwrap(); + let error = store.lock_and_load(&interrupted).unwrap_err(); + assert_eq!(error.code(), ErrorCode::StaleId, "{error}"); + assert!(entries(root.path()).is_empty()); + // Deprovision removes such files beside the configuration too. + let id = SandboxId::generate().unwrap(); + store.create(&id, &record()).unwrap(); + let dir = store.dir(&id); + fs::write(dir.join(".sandbox.json.18446744073709551615.tmp"), b"{").unwrap(); + let (locked, _) = store.lock_and_load(&id).unwrap(); + store.remove(&id, locked).unwrap(); + assert!(entries(root.path()).is_empty()); + // A name that only resembles theirs is not the sandbox's. + store.create(&id, &record()).unwrap(); + fs::write(dir.join(".sandbox.json.012.tmp"), b"{").unwrap(); + let (locked, _) = store.lock_and_load(&id).unwrap(); + let error = store.remove(&id, locked).unwrap_err(); + assert!( + error.message().ends_with(": .sandbox.json.012.tmp"), + "{error}" + ); + assert_eq!(entries(&dir), [".sandbox.json.012.tmp"]); + } + #[test] fn state_removed_while_waiting_for_the_lock_leaves_nothing_behind() { let root = tempfile::tempdir().unwrap(); diff --git a/doc/run.md b/doc/run.md index d4572ba68..815f18d92 100644 --- a/doc/run.md +++ b/doc/run.md @@ -1059,22 +1059,32 @@ for any other in progress, and of two overlapping starts, exactly one launches OpenVMM while the other then fails because the sandbox is already running. `exec` holds the lock only while it finds the running OpenVMM, so a long workload does not delay `stop`. A provisioned state directory keeps -`sandbox.lock`, and `deprovision` removes it with NVX's other files. On NFS, -and on Windows file systems without POSIX deletion such as ReFS, a removed lock -file keeps the emptied directory until its last handle closes, so `deprovision` -then releases the lock before it removes the directory. A transition that -waited while `deprovision` removed the sandbox then fails as for a sandbox that -is not provisioned, except `provision`, which provisions the state directory -again. A `provision` that takes the lock after `deprovision` has removed the -lock file, but before it has removed the directory, provisions the directory -first, and `deprovision` then fails because the sandbox was provisioned again. +`sandbox.lock`, and `deprovision` removes it with NVX's other files. A +transition that leaves a state directory without `config.json`, such as one of +a sandbox that was never provisioned, removes `sandbox.lock` as it releases the +lock. These are the rules of the sandbox store, which other orchestrators of +sandboxes share through `nvx-rs`. On NFS, where a removed file that is still +open, such as a lock file, stays as `.nfs*` until its last handle closes, and +on Windows file systems without POSIX deletion such as ReFS, where a removed +lock file stays under its name, such files keep the emptied directory, so +`deprovision` then releases the lock before it removes the directory. A +transition that waited while `deprovision` removed the sandbox then fails as +for a sandbox that is not provisioned, except `provision`, which provisions the +state directory again. A `provision` that takes the lock after `deprovision` +has removed the lock file, but before it has removed the directory, provisions +the directory first, and `deprovision` then fails because the sandbox was +provisioned again. `--timeout` must be finite and greater than 0. If the guest exits while `start` waits for it to become ready, for example because it refuses the workload identity, OpenVMM closes the control endpoint and shuts down. `start` then waits up to `--timeout` seconds for OpenVMM to publish `outcome.json` and exit, and fails with the report's category and status, such as `guest-exit status 125`. It terminates OpenVMM only if OpenVMM outlives that -wait. Managed +wait. OpenVMM writes the report to a file of its own in the state directory, +`.tmp` and six ASCII letters or digits, before it publishes it: NVX removes +such a file, which an OpenVMM that is terminated while it writes the report +leaves, once no OpenVMM of the sandbox runs, and `deprovision` removes it with +NVX's other files. Managed workload arguments use the bounded control protocol rather than the kernel command line and may contain whitespace. Managed execution can select an absolute working directory and either repeated inline `KEY=VALUE` entries or a @@ -1136,10 +1146,10 @@ named. It checks that: - `provision` rejects a root `--workload-user` before it creates any state, and the guest refuses an identity that the Ubuntu image lacks before a one-shot workload starts, and in a managed `start`, which then fails with - `guest-exit status 125` from OpenVMM's outcome report and leaves only - `config.json`, `sandbox.lock`, `openvmm.log`, and that `outcome.json`, - without a runtime record, capability, control socket, report staging file, - or OpenVMM process; + `guest-exit status 125` from the outcome report that OpenVMM must publish + before it exits, and leaves only `config.json`, `sandbox.lock`, + `openvmm.log`, and that `outcome.json`, without a runtime record, + capability, control socket, report staging file, or OpenVMM process; - every operation fails on a state directory that was never provisioned, a repeated `provision` leaves the configuration unchanged, and `exec` and `stop` fail before `start`; diff --git a/nvx-rs/README.md b/nvx-rs/README.md index ec1500309..f3c7ed9ae 100644 --- a/nvx-rs/README.md +++ b/nvx-rs/README.md @@ -74,8 +74,8 @@ nvx-rs/ | `nvx::host::network` | `NetworkPolicy` and `openvmm_arguments`: the egress default and rules, ingress, host-loopback access and forwards as OpenVMM options. Egress rules are lowered exactly into OpenVMM's IPv4 and IPv6 rules by the lowering of egress policy files, with at most 256 rules for each action | | `nvx::host::network::policy` | Egress policy files: `compile_policy_file`, `compile_policy_bytes`, and `compile_policy` compile one to a `CompiledEgressPolicy` of native rules; see [Egress policy files](#egress-policy-files) | | `nvx::host::filesystem` | The `ExportPlan` of host paths through OpenVMM's aggregate virtio-fs device, the host-mapping table, the reserved guest targets, and `guest_path` and `resolve_guest_path` (also at `nvx::`) | -| `nvx::host::store` | `SandboxStore`: the runtime records of one sandbox in a private directory with one lock, which every host that supervises the sandbox shares. The `RuntimeRecord` names the running VM: the `ProcessRef` of its VMM, its control endpoint, and the `RuntimeIdentity` that it was launched with, the identities of its OpenVMM, kernel, and initramfs from install or staging metadata and its boot and control contract versions, which decide whether another build of the host may reattach to it. The `LaunchJournal` of a launch holds a marker written before the spawn, the VMM's `ProcessRef` right after it, and an inherited claim on the VMM's log, which proves that a launch that never published its endpoint has exited, so that recovery ends an interrupted launch and signals only a VMM that it holds through a verified `Process`. Records are JSON in one versioned format, and records in any other format fail rather than being migrated | -| `nvx::host::instance` | `Instance`: a VM in a sandbox store of its own, which `nvx::host::store`, `nvx::host::supervisor`, and `nvx::host::session` drive. Under the store's lock, `LockedInstance::find` reattaches to the VM, ends a start that was interrupted, and reports a VM of another runtime identity as `Mismatched` rather than driving it; `LockedInstance::start` launches OpenVMM through the supervisor with a fresh capability, under the caller's `RuntimeIdentity`, waits until the guest is ready and has mounted the mapped paths, and commits the launch; and `LockedInstance::stop` stops the VM with a forced fallback. `Instance::exec` runs workloads without the lock and pumps their output. `write_json` and `read_json` keep the configuration records that callers store beside an instance | +| `nvx::host::store` | `SandboxStore`: the runtime records of one sandbox in a private directory with one lock, which every host that supervises the sandbox shares. The `RuntimeRecord` names the running VM: the `ProcessRef` of its VMM, its control endpoint, and the `RuntimeIdentity` that it was launched with, the identities of its OpenVMM, kernel, and initramfs from install or staging metadata and its boot and control contract versions, which decide whether another build of the host may reattach to it. The `LaunchJournal` of a launch holds a marker written before the spawn, the VMM's `ProcessRef` right after it, and an inherited claim on the VMM's log, which proves that a launch that never published its endpoint has exited, so that recovery ends an interrupted launch and signals only a VMM that it holds through a verified `Process`. Records are JSON in one versioned format, and records in any other format fail rather than being migrated. The caller declares the files that it keeps in the directory, its configuration record, whose presence means that the sandbox is provisioned, and any others, as `CallerFiles`, and takes the lifecycle locks with them: `SandboxStore::lock_for_provision` creates the directory, again if a removal deletes it meanwhile, `SandboxStore::lock_existing` fails with `ErrorKind::NotFound` once it is gone, and a holder that leaves the directory without the configuration record removes its lock file as it releases the lock. `LockedStore::remove` removes a sandbox that no VMM can run in, for a caller that declares its configuration record: the store's files, the caller's, and the directory, unless it holds entries that are neither, which it refuses and keeps, and `Lingering` waits for open files, such as the lock files of other transitions, that keep the emptied directory; both remove the directory only while the path still names it. The store also owns the file in which the VMM stages its outcome report, which a VMM that is killed while it writes the report leaves, and removes it once no process of its launch runs | +| `nvx::host::instance` | `Instance`: a VM in a sandbox store of its own, which `nvx::host::store`, `nvx::host::supervisor`, and `nvx::host::session` drive, and in which the caller keeps the files that it declares as `CallerFiles`. Under the store's lifecycle lock of an existing sandbox, `LockedInstance::find` reattaches to the VM, ends a start that was interrupted, and reports a VM of another runtime identity as `Mismatched` rather than driving it; `LockedInstance::start` launches OpenVMM through the supervisor with a fresh capability, under the caller's `RuntimeIdentity`, waits until the guest is ready and has mounted the mapped paths, and commits the launch; `LockedInstance::stop` stops the VM with a forced fallback; and `LockedInstance::remove` removes a stopped instance, with the caller's files and its directory, through the store. `Instance::exec` runs workloads without the lock and pumps their output. `write_json` and `read_json` keep the configuration records that callers store beside an instance | | `nvx::host::nvxs` | `HostConnection`: the host's role of the NVXS session over a platform transport, on which `nvx::host::session` and other host applications run their protocols. It attaches with the launch capability, waiting while the broker asks it to, sends application bytes in as many DATA records as they take, and returns each DATA payload from the guest, with deadlines; a receive that times out keeps a partial record | | `nvx::host::process` | Verified process references. A `ProcessRef` names one process by its ID, its start time, and the `BootId` of the host boot during which it started. `Process::open` checks a reference and holds its process through a pidfd on Linux or a process handle on Windows, whether or not the caller started it, so that waits sleep until the process exits and termination never reaches a process that reuses its ID. A reference from an earlier boot names an exited process, and a process whose identity this user cannot check, such as another user's, makes `Process::open` fail rather than report an exit. Linux hosts without pidfds poll the reference instead, and cannot terminate the process | | `nvx::host::supervisor` | The supervisor of a sandbox's VMM, beneath the CLI, `Instance`, and external orchestrators, which keep the configuration, the authorization, and the application protocol. `launch` runs the store's launch steps under the sandbox's lock: it writes the journal's marker, spawns OpenVMM with the claimed log, records its `ProcessRef` right after, and returns a `Starting` launch that the caller commits once the VMM serves its control endpoint, or abandons. A launch that fails never leaves an unrecorded VMM running. `VmmCommand::inherit` lists the descriptors (Linux) or handles (Windows) that the VMM inherits under their numbers, besides its standard streams, and it inherits nothing else of the caller's: on Linux, they must be close-on-exec, a flag that only the VMM's own process clears; on Windows, they join the VMM's explicit handle list. `Vmm::stop` runs one graceful step of the caller's and waits for the VMM's exit event until a deadline, and takes a bounded forced step through the VMM's verified identity only on request. `reattach` recovers an interrupted launch, and holds the recorded VMM again after the orchestrator restarts within the same host boot: `Live`, `Mismatched` with another runtime identity, `Exited`, or `Indeterminate` | @@ -414,7 +414,14 @@ refuses to start a sandbox that `nvx.py` provisioned, which has no binding: `provision`, `start`, `stop`, and `deprovision` hold the store's lock for their whole transition, and `exec` only while it finds the running VM, so that a long -workload cannot delay a `stop`. Every transition but `provision` finds the +workload cannot delay a `stop`. The CLI takes the store's +[lifecycle locks](src/host/store/mod.rs) with `config.json` as its +configuration record: a transition that leaves the state directory without it, +such as one of a sandbox that was never provisioned, removes the lock file as +it releases the lock, and `deprovision` removes the sandbox through the store, +which removes the store's files and `config.json`, refuses files that are not +NVX's, and removes the lock file, while it still holds the lock, before the +directory. Every transition but `provision` finds the sandbox's VM with `nvx::host::supervisor::reattach`, as an orchestrator does after it restarts within the same boot of the host: it ends a start that was interrupted, holds an OpenVMM that still runs through its verified process @@ -572,6 +579,7 @@ use std::time::Duration; use nvx::host::instance::{Found, Instance}; use nvx::host::launch::LaunchSpec; +use nvx::host::store::CallerFiles; use nvx::protocol::{Workload, WorkloadEnvironment}; use nvx::{Artifacts, Hypervisor}; @@ -579,7 +587,9 @@ fn main() -> Result<(), Box> { let artifacts = Artifacts::discover()?; let runtime = artifacts.runtime_identity()?; let spec = LaunchSpec::new(artifacts, Hypervisor::from_env_or_default()?); - let instance = Instance::create(std::env::temp_dir().join("nvx-example"), "example")?; + // The example keeps no files of its own beside the instance's store. + let directory = std::env::temp_dir().join("nvx-example"); + let instance = Instance::create(directory, "example", CallerFiles::none())?; // Lifecycle transitions hold the instance's lock; executions need none. let mut locked = instance.lock(None)?; let Found::Stopped = locked.find(|| Ok(runtime.clone()))? else { @@ -624,6 +634,7 @@ as is, an `ErrorKind`, and an optional cause. | `Unsupported` | A well-formed request that NVX cannot represent or enforce, such as a policy beyond a limit of OpenVMM or the guest agent | | `Exited` | The VM's OpenVMM process exited | | `Io` | A host I/O failure, or state that NVX recorded on the host and cannot read | +| `NotFound` | State that an operation names and that does not exist on the host, such as the store of a sandbox that was never created or that a removal deleted | | `Protocol` | A control session that failed, or a request that the guest agent refused | | `TimedOut` | A deadline that passed | | `Other` | Any other failure | diff --git a/nvx-rs/src/error.rs b/nvx-rs/src/error.rs index db12019ff..e63479a0d 100644 --- a/nvx-rs/src/error.rs +++ b/nvx-rs/src/error.rs @@ -21,6 +21,9 @@ pub enum ErrorKind { /// A host I/O operation failed, or state that NVX recorded on the host is unreadable or /// malformed. Io, + /// The state that an operation names does not exist on the host, such as the store of a + /// sandbox that was never created or that a removal deleted. + NotFound, /// The control session failed: the peer violated the protocol, closed or reset the session, /// or refused a request. Protocol, @@ -38,6 +41,7 @@ impl ErrorKind { Self::Unsupported => "unsupported", Self::Exited => "exited", Self::Io => "io", + Self::NotFound => "not-found", Self::Protocol => "protocol", Self::TimedOut => "timed-out", Self::Other => "other", @@ -95,6 +99,11 @@ impl Error { Self::new(ErrorKind::Io, message) } + /// Creates an [`ErrorKind::NotFound`] error. + pub fn not_found(message: impl Into) -> Self { + Self::new(ErrorKind::NotFound, message) + } + /// Creates an [`ErrorKind::Protocol`] error. pub fn protocol(message: impl Into) -> Self { Self::new(ErrorKind::Protocol, message) diff --git a/nvx-rs/src/host/cli/sandbox/lifecycle.rs b/nvx-rs/src/host/cli/sandbox/lifecycle.rs index 1c80c9e0a..7a490f958 100644 --- a/nvx-rs/src/host/cli/sandbox/lifecycle.rs +++ b/nvx-rs/src/host/cli/sandbox/lifecycle.rs @@ -39,14 +39,11 @@ //! that does not, without ending it otherwise. use std::ffi::OsString; -use std::fs::{File, OpenOptions}; +use std::fs::OpenOptions; use std::io::Write; use std::time::{Duration, Instant}; -use super::lock::{ - LifecycleLock, OsErrorClass, directory_not_empty, directory_status, entries, lingers, - os_error_class, -}; +use super::lock::{LifecycleLock, OsErrorClass, os_error_class}; use super::state::{ CONFIG_FORMATS, RecordedDistro, config_int, config_text, deserialize_binding, deserialize_launch, hex, member, random_bytes, read_json, serialize_launch, temporary_path, @@ -66,7 +63,7 @@ use crate::host::python::value::py_str; use crate::host::sandbox::distro::DISTRO_POLICY_REVISION; use crate::host::sandbox::{SandboxLaunch, require_file}; use crate::host::store::{ - self, IdentityPart, LOCK_NAME, LockedStore, OUTCOME_NAME, RUNTIME_NAME, Recovery, SandboxStore, + IdentityPart, LockedStore, OUTCOME_NAME, RUNTIME_NAME, Recovery, SandboxStore, }; use crate::host::supervisor::{self, Launched, Reattached, StopOutcome, Vmm, VmmCommand}; use crate::protocol::{CAPABILITY_LEN, GuestFeatures}; @@ -74,7 +71,7 @@ use crate::protocol::{CAPABILITY_LEN, GuestFeatures}; /// The launch configuration, the CLI's own record beside those of the store. pub(crate) const CONFIG_NAME: &str = "config.json"; /// How often a caller retries a lifecycle lock that another caller holds, and the removal of a -/// state directory that a removed lock file still keeps. +/// state directory that open files still keep. pub(crate) const LOCK_RETRY_INTERVAL: Duration = RETRY_INTERVAL; /// The version of outcome reports. pub(crate) const OUTCOME_SCHEMA_VERSION: u64 = 1; @@ -546,8 +543,8 @@ fn start_locked( 0.0 }; let status = end_failed_start(starting.child(), grace, &command)?; - // OpenVMM has exited, so the launch ends: its capability, control socket, and journal go, - // and its log and outcome report stay. + // OpenVMM has exited, so the launch ends: its capability, control socket, report staging + // file, and journal go, and its log and outcome report stay. let unended = match starting.abandon() { Ok(Recovery::Indeterminate(reason)) => Some(reason), Ok(_) => None, @@ -650,8 +647,8 @@ fn extend_recorded_network_arguments(command: &mut Vec, config: &Documen /// Ends the OpenVMM process of a failed start, as `_end_failed_start` does: returns OpenVMM's /// status if it exits on its own within `grace` seconds, or terminates it and returns `None`. -/// Terminating OpenVMM while it publishes its outcome report would leave the report's staging -/// file in the state directory, which deprovision refuses to remove. +/// Terminating OpenVMM while it publishes its outcome report would lose the report, and leave +/// only its staging file, which the store removes as the launch ends. fn end_failed_start( launched: &mut Launched, grace: f64, @@ -806,19 +803,6 @@ fn wait_for_exit(vmm: &Vmm, deadline: Deadline) -> Result { } } -/// Returns whether `name` is a temporary file that the store leaves beside one of its files when a -/// replacement of the file is interrupted: `.NAME.NUMBER.tmp`. -fn is_store_temporary(name: &str) -> bool { - name.strip_prefix('.') - .and_then(|name| name.strip_suffix(".tmp")) - .and_then(|name| name.rsplit_once('.')) - .is_some_and(|(file, number)| { - store::FILE_NAMES.contains(&file) - && !number.is_empty() - && number.bytes().all(|byte| byte.is_ascii_digit()) - }) -} - /// Removes a stopped sandbox and its state directory, as `deprovision` does. pub(crate) fn deprovision(state_path: &str, artifacts: &Artifacts<'_>, timeout: f64) -> Result { let state_dir = prepare_state_directory(state_path, false)?; @@ -829,134 +813,9 @@ pub(crate) fn deprovision(state_path: &str, artifacts: &Artifacts<'_>, timeout: return Err(Exit::error("sandbox must be stopped before deprovision")); } } - // No OpenVMM of the sandbox runs, and a start that was interrupted is over, so every file of - // the store goes but its lock, which goes last. - for name in store::FILE_NAMES - .into_iter() - .filter(|&name| name != LOCK_NAME) - .chain([CONFIG_NAME]) - { - unlink_missing_ok(&state_dir.join(name))?; - } - let mut unknown: Vec = entries(&state_dir) - .map_err(|error| os_error(&error, &state_dir))? - .into_iter() - .filter(|name| name != LOCK_NAME) - .collect(); - for name in unknown.extract_if(.., |name| is_store_temporary(name)) { - unlink_missing_ok(&state_dir.join(&name))?; - } - if !unknown.is_empty() { - // `iterdir` lists the names in the order of the directory, which the message keeps. - return Err(Exit::error(format!( - "sandbox state directory contains files not owned by NVX: {}", - unknown.join(", ") - ))); - } - // Holding the lock until the directory is gone keeps a transition that waits for it from - // finding the sandbox halfway removed. - lock.remove()?; - let lingering = match std::fs::remove_dir(state_dir.as_path()) { - Ok(()) => false, - Err(error) if directory_not_empty(&error) => true, - Err(error) => return Err(os_error(&error, &state_dir)), - }; - if !lingering { - return Ok(()); - } - // A removed lock file that a handle still keeps stays in the directory, as do the lock files of - // transitions that wait meanwhile. Each of them finds no sandbox, and removes its lock file as - // it releases the lock, so the directory empties after this call releases its own. - let (removing, held) = hold_directory(&state_dir)?; - lock.release(); - let removed = remove_emptied_state_directory(&state_dir, removing, timeout); - drop(held); - removed -} - -/// Returns the identity of a directory, and a handle that keeps it, as `_hold_directory` does: on -/// POSIX, the open directory keeps a directory that replaces it from reusing its inode number; -/// NTFS and ReFS do not reuse file IDs. -fn hold_directory(path: &PurePath) -> Result<((u64, u128), Option)> { - #[cfg(unix)] - { - use std::os::unix::fs::{MetadataExt, OpenOptionsExt}; - - let file = OpenOptions::new() - .read(true) - .custom_flags((nix::fcntl::OFlag::O_DIRECTORY | nix::fcntl::OFlag::O_NOFOLLOW).bits()) - .open(path.as_path()) - .map_err(|error| os_error(&error, path))?; - // `os.fstat` gives the identity that `os.lstat` gives for the directory's path. - let metadata = file.metadata().map_err(Exit::from_os_error)?; - Ok(((metadata.dev(), u128::from(metadata.ino())), Some(file))) - } - #[cfg(not(unix))] - { - let status = fs::lstat(path.as_path()).map_err(|error| os_error(&error, path))?; - Ok((status.identity(), None)) - } -} - -/// Removes a state directory once no lingering lock file keeps it, as -/// `_remove_emptied_state_directory` does. A provision that waits for the removal may complete it -/// and provision the path again, which ends this attempt too. -fn remove_emptied_state_directory( - state_dir: &PurePath, - removing: (u64, u128), - timeout: f64, -) -> Result { - let deadline = Deadline::after(timeout); - let mut retried_at_once = false; - loop { - let error = match std::fs::remove_dir(state_dir.as_path()) { - Ok(()) => return Ok(()), - Err(error) if os_error_class(&error) == OsErrorClass::NotFound => return Ok(()), - Err(error) => error, - }; - if directory_status(state_dir) != Some(removing) { - return Ok(()); - } - let remaining = match entries(state_dir) { - Ok(remaining) => remaining, - Err(error) if os_error_class(&error) == OsErrorClass::NotFound => return Ok(()), - Err(error) => return Err(os_error(&error, state_dir)), - }; - let mut unexpected: Vec = remaining - .iter() - .filter(|name| !lingers(name)) - .cloned() - .collect(); - unexpected.sort(); - // A provision that starts after this deprovision removed the lock file, and before it - // removed the directory, provisions it again. - if unexpected.iter().any(|name| name == CONFIG_NAME) { - return Err(Exit::error( - "sandbox was provisioned again while deprovision removed it", - )); - } - if !unexpected.is_empty() { - return Err(Exit::error(format!( - "sandbox state directory contains files not owned by NVX: {}", - unexpected.join(", ") - ))); - } - if !directory_not_empty(&error) { - return Err(os_error(&error, state_dir)); - } - // Without remaining entries, the last lingering lock file went away after rmdir failed, - // so the next attempt follows at once. Another such failure right after it pauses first, - // so that a file system that does not report emptiness right away cannot spin. - if remaining.is_empty() && !retried_at_once { - retried_at_once = true; - if deadline.passed() { - return Err(os_error(&error, state_dir)); - } - continue; - } - retried_at_once = false; - if !deadline.pause(LOCK_RETRY_INTERVAL)? { - return Err(os_error(&error, state_dir)); - } - } + // No OpenVMM of the sandbox runs, and a start that was interrupted is over, so the store + // removes the sandbox: its files and the configuration, then refuses files that are not + // NVX's, and otherwise removes its lock file, while it still holds the lock, and the + // directory. + lock.remove(timeout) } diff --git a/nvx-rs/src/host/cli/sandbox/lock.rs b/nvx-rs/src/host/cli/sandbox/lock.rs index fb2dab88d..37c60aa6f 100644 --- a/nvx-rs/src/host/cli/sandbox/lock.rs +++ b/nvx-rs/src/host/cli/sandbox/lock.rs @@ -1,18 +1,21 @@ -//! The lifecycle lock of a state directory, which is the lock of the sandbox's -//! [`SandboxStore`]: its file `sandbox.lock`. +//! The lifecycle lock of a state directory: the store's +//! [lifecycle lock](crate::host::store#lifecycle-locks) of the sandbox, whose caller keeps its +//! configuration record, `config.json`, in the directory. //! //! - Provision, start, stop, and deprovision hold the lock for their whole transition, and exec //! while it finds the running VM. A caller that cannot take the lock retries every 25 ms //! within its timeout, and Ctrl-C ends the wait. -//! - The store takes the lock of the file that the path names when the lock is granted: a lock -//! file that its holder removed meanwhile locks nothing, so the store takes the lock of the -//! lock file at the path again, which it creates if needed. -//! - Deprovision removes the lock file before the directory. A transition that waited for it then -//! finds the directory gone, or takes the lock of a new lock file in the directory and finds no -//! sandbox there. Every transition that leaves the directory without a provisioned sandbox -//! removes its lock file as it releases the lock, so that the directory empties. +//! - The store takes the lock of the file that the path names when the lock is granted. A +//! provision whose directory deprovision removed meanwhile creates it again; any other +//! transition then fails, as for a path that names no directory. +//! - Deprovision [removes](crate::host::store#removal) the sandbox through the store, which +//! removes the lock file before the directory. A transition that waited for it then finds the +//! directory gone, or takes the lock of a new lock file in the directory and finds no sandbox +//! there. Every transition that leaves the directory without a provisioned sandbox removes its +//! lock file as it releases the lock, so that the directory empties. use std::io; +use std::path::Path; use std::time::Instant; use super::error_with_cause; @@ -20,9 +23,9 @@ use super::lifecycle::{CONFIG_NAME, LOCK_RETRY_INTERVAL, create_state_directory} use crate::ErrorKind; use crate::host::cli::deadline::Deadline; use crate::host::cli::{Exit, Result}; -use crate::host::python::fs::{self, os_error_message}; +use crate::host::python::fs::os_error_message; use crate::host::python::path::PurePath; -use crate::host::store::{LOCK_NAME, LockedStore, SandboxStore}; +use crate::host::store::{CallerFiles, LockedStore, RemovalError, SandboxStore}; /// How Python classifies an `OSError`, as far as the lifecycle tells the classes apart. #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -67,57 +70,20 @@ pub(crate) fn transition_in_progress() -> Exit { Exit::error("another lifecycle transition of the sandbox is still in progress") } -/// Returns `os.lstat(path)`'s identity, or `None` if the path cannot be examined, as -/// `_directory_status` does. -pub(crate) fn directory_status(path: &PurePath) -> Option<(u64, u128)> { - fs::lstat(path.as_path()) - .ok() - .map(|status| status.identity()) +/// Returns the files that the CLI keeps in a state directory beside the store's: its +/// configuration record, whose presence means that the sandbox is provisioned. +fn caller_files() -> CallerFiles { + CallerFiles::new(CONFIG_NAME).expect("the configuration record has a plain file name") } -/// Returns whether `name` is a removed lock file that a handle still keeps, as `_lingers` does: -/// NFS renames a removed file that is still open to `.nfs*`, and Windows file systems without -/// POSIX deletion keep it under its own name, as they keep the lock files that waiting -/// transitions create until they release them. -pub(crate) fn lingers(name: &str) -> bool { - name == LOCK_NAME || name.starts_with(".nfs") -} - -/// Returns whether `error` is how `rmdir` reports a directory that is not empty. -pub(crate) fn directory_not_empty(error: &io::Error) -> bool { - #[cfg(windows)] - let not_empty = matches!(error.raw_os_error(), Some(145 | 80 | 183)); - #[cfg(unix)] - let not_empty = matches!( - error.raw_os_error(), - Some(code) if code == nix::errno::Errno::ENOTEMPTY as i32 - || code == nix::errno::Errno::EEXIST as i32 - ); - #[cfg(not(any(windows, unix)))] - let not_empty = error.kind() == io::ErrorKind::DirectoryNotEmpty; - not_empty -} - -/// Returns the names of the entries of `directory`. -pub(crate) fn entries(directory: &PurePath) -> io::Result> { - std::fs::read_dir(directory.as_path())? - .map(|entry| { - let name = entry?.file_name(); - name.into_string() - .map_err(|_| crate::host::python::not_unicode("a file name")) - }) - .collect() -} - -/// The exclusive lock that serializes the lifecycle transitions of a state directory: the lock -/// of its sandbox's store, through which the transition changes the store's records. +/// The exclusive lock that serializes the lifecycle transitions of a state directory: the +/// lifecycle lock of its sandbox's store, through which the transition changes the store's +/// records. Dropping it releases the lock. pub(crate) struct LifecycleLock { /// The state directory. state_dir: PurePath, - /// The sandbox's locked store, until the lock is released. - locked: Option, - /// Whether the holder removed its lock file, after which the path may name another's. - removed: bool, + /// The sandbox's locked store. + locked: LockedStore, } impl LifecycleLock { @@ -133,20 +99,21 @@ impl LifecycleLock { )); } let deadline = Deadline::after(timeout); + let files = caller_files(); loop { - let opened = if provision { - SandboxStore::create(state_dir.as_path()) - } else { - SandboxStore::open(state_dir.as_path()) - }; // The store tries the lock once for a deadline that has passed, so that this wait // checks for Ctrl-C between tries. - let error = match opened.and_then(|store| store.lock(Some(Instant::now()))) { + let now = Some(Instant::now()); + let taken = if provision { + SandboxStore::lock_for_provision(state_dir.as_path(), &files, now) + } else { + SandboxStore::lock_existing(state_dir.as_path(), &files, now) + }; + let error = match taken { Ok(locked) => { return Ok(Self { state_dir: state_dir.clone(), - locked: Some(locked), - removed: false, + locked, }); } Err(error) => error, @@ -160,6 +127,7 @@ impl LifecycleLock { "sandbox state path is not a plain directory: {state_dir}" ))); } + // The store creates the state directory again itself, once its parent exists. (_, Some(OsErrorClass::NotFound)) => { if deadline.passed() { return Err(transition_in_progress()); @@ -180,34 +148,65 @@ impl LifecycleLock { /// Returns the sandbox's locked store. pub(crate) fn locked(&mut self) -> &mut LockedStore { - self.locked.as_mut().expect("the lock is held") - } - - /// Removes the held lock file, as deprovision does before it removes the directory. - pub(crate) fn remove(&mut self) -> Result { - let path = self.state_dir.join(LOCK_NAME); - std::fs::remove_file(path.as_path()) - .map_err(|error| Exit::error(os_error_message(&error, Some(path.as_str()))))?; - self.removed = true; - Ok(()) + &mut self.locked } - /// Releases the lock. The lock file of a directory that holds no provisioned sandbox goes - /// first, unless the holder removed it already, so that the directory can be removed. - pub(crate) fn release(&mut self) { - let Some(locked) = self.locked.take() else { - return; + /// Removes the sandbox and its state directory through the store, as deprovision does once no + /// OpenVMM of the sandbox can run, waiting up to `timeout` seconds for open files, such as the + /// lock files of other transitions, that keep the emptied directory, and releases the lock. + pub(crate) fn remove(self, timeout: f64) -> Result { + let Self { state_dir, locked } = self; + let dir = locked.dir().to_owned(); + let failed = |error| removal_error(error, &state_dir, &dir); + let Some(mut lingering) = locked.remove().map_err(failed)? else { + return Ok(()); }; - if !self.removed && !fs::lexists(self.state_dir.join(CONFIG_NAME).as_path()) { - let _ = std::fs::remove_file(self.state_dir.join(LOCK_NAME).as_path()); + // A removed file that a handle still keeps, such as a lock file, stays in the directory, + // as do the lock files of transitions that wait meanwhile. Each such transition finds no + // sandbox, and removes its lock file as it releases the lock, so the directory empties + // after the removal released its own, once the other handles close. The removal is tried + // once at a time, so that this wait checks for Ctrl-C between tries. + let deadline = Deadline::after(timeout); + loop { + match lingering.wait(Some(Instant::now())) { + Ok(()) => return Ok(()), + Err(RemovalError::TimedOut(error)) if !deadline.passed() => { + if !deadline.pause(LOCK_RETRY_INTERVAL)? { + return Err(failed(RemovalError::TimedOut(error))); + } + } + Err(error) => return Err(failed(error)), + } } - drop(locked); } } -impl Drop for LifecycleLock { - fn drop(&mut self) { - self.release(); +/// Returns the error of `nvx.py`'s deprovision for the store's removal of the sandbox in +/// `state_dir`, whose store names it `dir`, that `error` ended. +fn removal_error(error: RemovalError, state_dir: &PurePath, dir: &Path) -> Exit { + match error { + // The CLI always declares its configuration record. + error @ RemovalError::Undeclared => Exit::error(error.to_string()), + RemovalError::Launched => Exit::error("sandbox must be stopped before deprovision"), + RemovalError::Foreign(names) => Exit::error(format!( + "sandbox state directory contains files not owned by NVX: {}", + names.join(", ") + )), + RemovalError::ProvisionedAgain => { + Exit::error("sandbox was provisioned again while deprovision removed it") + } + RemovalError::TimedOut(error) => { + Exit::error(os_error_message(&error, Some(state_dir.as_str()))) + } + RemovalError::Io { path, error } => { + // The store names the directory, or one of its entries, by its own path. + let shown = match path.strip_prefix(dir).ok().map(Path::to_str) { + Some(Some("")) => state_dir.clone(), + Some(Some(name)) => state_dir.join(name), + _ => PurePath::native(&path.to_string_lossy()), + }; + Exit::error(os_error_message(&error, Some(shown.as_str()))) + } } } @@ -226,7 +225,7 @@ mod tests { let directory = tempfile::tempdir().unwrap(); let state = state_dir(&directory); std::fs::write(state.join(CONFIG_NAME).as_path(), "{}").unwrap(); - let mut held = LifecycleLock::acquire(&state, 10.0, false).unwrap(); + let held = LifecycleLock::acquire(&state, 10.0, false).unwrap(); let started = std::time::Instant::now(); let error = LifecycleLock::acquire(&state, 0.1, false).err().unwrap(); assert!(started.elapsed() >= std::time::Duration::from_millis(100)); @@ -234,42 +233,24 @@ mod tests { error.message(), Some("another lifecycle transition of the sandbox is still in progress") ); - held.release(); + drop(held); drop(LifecycleLock::acquire(&state, 10.0, false).unwrap()); - // A provisioned directory keeps its lock file. - assert!(state.join(LOCK_NAME).as_path().is_file()); } #[test] - fn released_locks_leave_no_file_in_unprovisioned_directories() { + fn missing_state_directories_are_created_by_provision_alone() { let directory = tempfile::tempdir().unwrap(); let state = state_dir(&directory); - drop(LifecycleLock::acquire(&state, 10.0, true).unwrap()); - assert!(!state.join(LOCK_NAME).as_path().exists()); let missing = state.join("missing"); let error = LifecycleLock::acquire(&missing, 1.0, false).err().unwrap(); assert_eq!( error.message().unwrap(), format!("sandbox state path is not a plain directory: {missing}") ); - // A provision creates the directory that deprovision removed meanwhile. - drop(LifecycleLock::acquire(&missing, 1.0, true).unwrap()); - assert!(missing.as_path().is_dir()); - } - - #[test] - fn a_removed_lock_file_is_not_removed_again_on_release() { - let directory = tempfile::tempdir().unwrap(); - let state = state_dir(&directory); - let mut lock = LifecycleLock::acquire(&state, 10.0, true).unwrap(); - lock.remove().unwrap(); - assert!(!state.join(LOCK_NAME).as_path().exists()); - // A transition that waited takes the lock of a new lock file, which stays its own. - let other = LifecycleLock::acquire(&state, 1.0, true).unwrap(); - lock.release(); - assert!(state.join(LOCK_NAME).as_path().is_file()); - drop(other); - assert!(!state.join(LOCK_NAME).as_path().exists()); + // A provision creates the directory that deprovision removed meanwhile, and its parents. + let nested = missing.join("nested"); + drop(LifecycleLock::acquire(&nested, 1.0, true).unwrap()); + assert!(nested.as_path().is_dir()); } #[cfg(unix)] @@ -286,4 +267,66 @@ mod tests { format!("the sandbox store {state} is accessible to other users (mode 755)") ); } + + #[test] + fn removals_keep_the_messages_of_deprovision() { + let directory = tempfile::tempdir().unwrap(); + let state = state_dir(&directory); + let dir = state.as_path().to_owned(); + let message = |error| { + removal_error(error, &state, &dir) + .message() + .unwrap() + .to_owned() + }; + assert_eq!( + message(RemovalError::Launched), + "sandbox must be stopped before deprovision" + ); + assert_eq!( + message(RemovalError::Foreign(vec![ + "b.txt".to_owned(), + "a.txt".to_owned() + ])), + "sandbox state directory contains files not owned by NVX: b.txt, a.txt" + ); + assert_eq!( + message(RemovalError::ProvisionedAgain), + "sandbox was provisioned again while deprovision removed it" + ); + let denied = || io::Error::from(io::ErrorKind::PermissionDenied); + let config = state.join(CONFIG_NAME); + for (path, shown) in [ + (dir.clone(), state.as_str()), + (dir.join(CONFIG_NAME), config.as_str()), + ] { + let expected = os_error_message(&denied(), Some(shown)); + assert_eq!( + message(RemovalError::Io { + path, + error: denied() + }), + expected + ); + } + assert_eq!( + message(RemovalError::TimedOut(denied())), + os_error_message(&denied(), Some(state.as_str())) + ); + } + + #[test] + fn removals_remove_the_state_directory() { + let directory = tempfile::tempdir().unwrap(); + let state = state_dir(&directory); + std::fs::write(state.join(CONFIG_NAME).as_path(), "{}").unwrap(); + let lock = LifecycleLock::acquire(&state, 10.0, false).unwrap(); + lock.remove(10.0).unwrap(); + assert!(!state.as_path().exists()); + let error = LifecycleLock::acquire(&state, 1.0, false).err().unwrap(); + assert_eq!( + error.message().unwrap(), + format!("sandbox state path is not a plain directory: {state}") + ); + } } diff --git a/nvx-rs/src/host/instance.rs b/nvx-rs/src/host/instance.rs index d5909e600..81e8e40f6 100644 --- a/nvx-rs/src/host/instance.rs +++ b/nvx-rs/src/host/instance.rs @@ -4,8 +4,9 @@ //! //! An instance lives in a [`SandboxStore`] of its own: a directory private to the current user, //! in which the store keeps the instance's lock, the journal of its launch, and the runtime -//! record of its VM, and in which the caller may keep configuration records of its own under -//! other names: +//! record of its VM, and in which the caller may keep files of its own under other names, which it +//! declares as [`CallerFiles`], such as the configuration record whose presence means that the +//! instance is provisioned: //! //! ```text //! / @@ -16,10 +17,13 @@ //! control.sock Linux control endpoint (running only) //! openvmm.log OpenVMM output of the latest start //! outcome.json OpenVMM outcome report of the latest run +//! ... the caller's files //! ``` //! //! Lifecycle transitions run under the instance's lock, through the [`LockedInstance`] that -//! [`Instance::lock`] returns, so that every operation can run in a different process: +//! [`Instance::lock`] returns, so that every operation can run in a different process. The lock is +//! the store's [lifecycle lock](super::store#lifecycle-locks) of an existing sandbox, which knows +//! the caller's files: //! //! - [`LockedInstance::find`] finds the instance's VM as an orchestrator does after it restarts //! within the same host boot, with [`supervisor::reattach`]: it ends a start whose caller died, @@ -36,6 +40,8 @@ //! cannot mount them all. //! - [`LockedInstance::stop`] asks the guest to shut down and falls back to terminating OpenVMM //! through its verified process identity. +//! - [`LockedInstance::remove`] removes a stopped instance, with the caller's files, and its +//! directory, through the store's [removal](super::store#removal). //! //! [`Instance::exec`] runs a workload through the control console of a [`Running`] VM without the //! lock, and [`Execution::pump`] streams its output live. The guest serializes executions: a @@ -59,8 +65,8 @@ use super::process::KILL_TIMEOUT; use super::report::{self, Report}; use super::session::{self, ControlSession, ExecEvent, SessionError}; use super::store::{ - IdentityPart, LockedStore, OUTCOME_NAME, Recovery, RuntimeIdentity, RuntimeRecord, SOCKET_NAME, - SandboxStore, + CallerFiles, IdentityPart, LockedStore, OUTCOME_NAME, Recovery, RemovalError, RuntimeIdentity, + RuntimeRecord, SOCKET_NAME, SandboxStore, }; use super::supervisor::{self, Reattached, Starting, StopOutcome, Vmm, VmmCommand}; use crate::error::{Error, ErrorKind, Result}; @@ -86,8 +92,18 @@ fn private_options() -> OpenOptions { options } +/// Returns the name of the temporary file through which [`write_private`] replaces the file `name` +/// for the random `number`: `.NAME.NUMBER.tmp`, which stays beside it if the replacement is +/// interrupted. +fn temporary_name(name: &str, number: u64) -> String { + format!(".{name}.{number}.tmp") +} + /// Atomically replaces `path` with `contents`, readable only by the current user, as callers do /// with the configuration records that they keep beside an instance's store. +/// +/// The contents go to a temporary file beside `path` first, which a replacement that is +/// interrupted leaves, and which [`remove_interrupted_writes`] removes. pub fn write_private(path: &Path, contents: &[u8]) -> Result<()> { let mut suffix = [0u8; 8]; getrandom::fill(&mut suffix) @@ -96,7 +112,7 @@ pub fn write_private(path: &Path, contents: &[u8]) -> Result<()> { .file_name() .map(|name| name.to_string_lossy().into_owned()) .unwrap_or_default(); - let temporary = path.with_file_name(format!(".{name}.{}.tmp", u64::from_le_bytes(suffix))); + let temporary = path.with_file_name(temporary_name(&name, u64::from_le_bytes(suffix))); let written = (|| -> io::Result<()> { let mut file = private_options().create_new(true).open(&temporary)?; file.write_all(contents)?; @@ -141,6 +157,50 @@ pub fn remove_if_present(path: &Path) -> Result<()> { } } +/// Removes the temporary files that interrupted replacements of `path` with [`write_private`] or +/// [`write_json`] left beside it, and keeps every other file. A directory that does not exist has +/// none. +/// +/// The store's removal refuses those files, which are neither the store's nor the files that its +/// caller declares, so a caller that writes its files with [`write_private`] removes them first, +/// under the instance's lock. +pub fn remove_interrupted_writes(path: &Path) -> Result<()> { + let Some(name) = path.file_name().map(|name| name.to_string_lossy()) else { + return Ok(()); + }; + let dir = match path.parent() { + Some(dir) if !dir.as_os_str().is_empty() => dir, + _ => Path::new("."), + }; + let unlisted = || io_error(format!("cannot list {}", dir.display())); + let entries = match fs::read_dir(dir) { + Ok(entries) => entries, + Err(error) if error.kind() == io::ErrorKind::NotFound => return Ok(()), + Err(error) => return Err(unlisted()(error)), + }; + for entry in entries { + let entry = entry.map_err(unlisted())?; + if entry + .file_name() + .to_str() + .is_some_and(|entry| is_temporary_of(entry, &name)) + { + remove_if_present(&entry.path())?; + } + } + Ok(()) +} + +/// Returns whether `entry` is the name of a temporary file through which [`write_private`] +/// replaces the file `name`. +fn is_temporary_of(entry: &str, name: &str) -> bool { + entry + .strip_suffix(".tmp") + .and_then(|rest| rest.rsplit_once('.')) + .and_then(|(_, number)| number.parse::().ok()) + .is_some_and(|number| temporary_name(name, number) == entry) +} + /// Returns a fresh launch capability, which is never all zero. pub fn generate_capability() -> Result<[u8; CAPABILITY_LEN]> { let mut capability = [0u8; CAPABILITY_LEN]; @@ -326,25 +386,39 @@ pub struct Instance { store: SandboxStore, /// Sandbox name used in diagnostics. name: String, + /// The files that the caller keeps in the instance's directory. + files: CallerFiles, } impl Instance { /// Creates the directory `dir` of a new instance, whose parent must exist, private to the /// current user, as [`SandboxStore::create`] does, and supervises the instance in it, which - /// messages call `sandbox NAME`. - pub fn create(dir: impl Into, name: impl Into) -> Result { + /// messages call `sandbox NAME`, and in which the caller keeps `files`. + pub fn create( + dir: impl Into, + name: impl Into, + files: CallerFiles, + ) -> Result { Ok(Self { store: SandboxStore::create(dir)?, name: name.into(), + files, }) } /// Supervises the instance in the existing directory `dir`, which must be private to the - /// current user, as [`SandboxStore::open`] requires, and which messages call `sandbox NAME`. - pub fn open(dir: impl Into, name: impl Into) -> Result { + /// current user, as [`SandboxStore::open`] requires, which messages call `sandbox NAME`, and + /// in which the caller keeps `files`. Fails with [`ErrorKind::NotFound`] if `dir` does not + /// exist. + pub fn open( + dir: impl Into, + name: impl Into, + files: CallerFiles, + ) -> Result { Ok(Self { store: SandboxStore::open(dir)?, name: name.into(), + files, }) } @@ -363,6 +437,11 @@ impl Instance { &self.name } + /// Returns the files that the caller keeps in the instance's directory. + pub fn files(&self) -> &CallerFiles { + &self.files + } + /// Returns the path of the OpenVMM log of the latest start. pub fn log_path(&self) -> PathBuf { self.store.log_path() @@ -374,10 +453,16 @@ impl Instance { } /// Takes the instance's lock, which serializes its lifecycle transitions, waiting for it - /// until `deadline`, or for as long as it takes without one, as [`SandboxStore::lock`] does. + /// until `deadline`, or for as long as it takes without one, as + /// [`SandboxStore::lock_existing`] does with the caller's files. + /// + /// Fails with [`ErrorKind::NotFound`] once the instance's directory is gone, as a removal + /// that this waited for leaves it, and with [`ErrorKind::TimedOut`] when the deadline passes + /// first. A caller that keeps a configuration record checks it under the lock: the lock of a + /// directory without it is released without a lock file. pub fn lock(&self, deadline: Option) -> Result { Ok(LockedInstance { - locked: self.store.lock(deadline)?, + locked: SandboxStore::lock_existing(self.dir(), &self.files, deadline)?, instance: self.clone(), }) } @@ -684,6 +769,26 @@ impl LockedInstance { report: report::read(&self.instance.report_path()), }) } + + /// Removes the stopped instance: its store's records and files, the caller's files that it + /// declared, the configuration record last, and its directory, through the store's + /// [`LockedStore::remove`], and releases the lock. Open files, such as the lock files of other + /// transitions, that keep the emptied directory are then waited for until `deadline`, or for + /// as long as it takes without one, as [`Lingering::wait`](super::store::Lingering::wait) + /// does. + /// + /// The caller has found the instance stopped with [`find`](Self::find): the removal refuses, + /// and removes nothing, while the store records a VM or a start, and for an instance whose + /// caller declared no configuration record. Entries in the directory that are neither the + /// store's nor the caller's are refused, and kept with the directory, once the rest is gone, + /// among them the temporary files that interrupted writes of the caller's files left, which + /// the caller removes first with [`remove_interrupted_writes`]. + pub fn remove(self, deadline: Option) -> std::result::Result<(), RemovalError> { + match self.locked.remove()? { + None => Ok(()), + Some(mut lingering) => lingering.wait(deadline), + } + } } /// Fails with [`ErrorKind::Unsupported`] when the duration that `name` describes is longer than @@ -860,6 +965,47 @@ impl Execution { mod tests { use super::*; + #[test] + fn interrupted_writes_leave_temporary_files_that_only_their_removal_takes() { + let directory = tempfile::tempdir().unwrap(); + let names = || { + let mut names: Vec = fs::read_dir(directory.path()) + .unwrap() + .map(|entry| entry.unwrap().file_name().into_string().unwrap()) + .collect(); + names.sort(); + names + }; + let path = directory.path().join("config.json"); + write_json(&path, &serde_json::json!({ "written": true })).unwrap(); + assert_eq!(names(), ["config.json"]); + // Replacements of the file that were interrupted left their temporary files, which other + // files only resemble. + let left = [0, 12345, u64::MAX].map(|number| temporary_name("config.json", number)); + let others = [ + ".config.json.tmp", + ".config.json..tmp", + ".config.json.012.tmp", + ".config.json.+12.tmp", + ".config.json.1a.tmp", + ".config.json.18446744073709551616.tmp", + ".config.json.1.tmp.tmp", + ".xconfig.json.1.tmp", + ".other.json.1.tmp", + "config.json.1.tmp", + ".tmpAbC123", + ]; + for name in left.iter().map(String::as_str).chain(others) { + fs::write(directory.path().join(name), b"{").unwrap(); + } + remove_interrupted_writes(&path).unwrap(); + let mut kept: Vec<&str> = others.into_iter().chain(["config.json"]).collect(); + kept.sort_unstable(); + assert_eq!(names(), kept); + // A directory that does not exist has none. + remove_interrupted_writes(&directory.path().join("gone").join("config.json")).unwrap(); + } + #[test] fn readiness_failures_explain_images_without_workload_accounts() { let hint = "predates workload account creation"; @@ -883,12 +1029,21 @@ mod tests { use super::super::*; use crate::host::launch::PROCESS_COMMAND_LINE_LIMIT; - use crate::host::store::{ArtifactIdentity, JOURNAL_NAME, RUNTIME_NAME}; + use crate::host::store::{ + ArtifactIdentity, JOURNAL_NAME, LOCK_NAME, LOG_NAME, RUNTIME_NAME, + }; + + /// The configuration record of these tests' caller. + const CONFIG: &str = "config.json"; fn instance() -> (tempfile::TempDir, Instance) { let directory = tempfile::tempdir().unwrap(); - let instance = - Instance::create(directory.path().join("vm"), "aci-edge-sandboxes:0123").unwrap(); + let instance = Instance::create( + directory.path().join("vm"), + "aci-edge-sandboxes:0123", + CallerFiles::new(CONFIG).unwrap(), + ) + .unwrap(); (directory, instance) } @@ -907,10 +1062,40 @@ mod tests { .unwrap(); assert!(matches!(found, Found::Stopped), "{found:?}"); drop(locked); - let reopened = Instance::open(instance.dir(), instance.name()).unwrap(); + // The instance's caller recorded no configuration, so its lock file went too. + assert!(!instance.dir().join(LOCK_NAME).exists()); + let reopened = + Instance::open(instance.dir(), instance.name(), instance.files().clone()).unwrap(); assert_eq!(reopened, instance); } + #[test] + fn stopped_instances_are_removed_with_the_callers_files() { + let (_directory, instance) = instance(); + for name in [CONFIG, LOG_NAME, RUNTIME_NAME] { + fs::write(instance.dir().join(name), b"{}").unwrap(); + } + // An instance whose store records a VM is not removed. + let locked = instance.lock(None).unwrap(); + assert!(matches!(locked.remove(None), Err(RemovalError::Launched))); + assert!(instance.dir().join(CONFIG).is_file()); + fs::remove_file(instance.dir().join(RUNTIME_NAME)).unwrap(); + // Nor is one whose caller declared no configuration record. + let undeclared = + Instance::open(instance.dir(), instance.name(), CallerFiles::none()).unwrap(); + let locked = undeclared.lock(None).unwrap(); + assert!(matches!(locked.remove(None), Err(RemovalError::Undeclared))); + assert!(instance.dir().join(CONFIG).is_file()); + instance.lock(None).unwrap().remove(None).unwrap(); + assert!(!instance.dir().exists()); + // A transition that comes later finds the instance gone. + let error = instance.lock(None).unwrap_err(); + assert_eq!(error.kind(), ErrorKind::NotFound, "{error}"); + let error = + Instance::open(instance.dir(), instance.name(), CallerFiles::none()).unwrap_err(); + assert_eq!(error.kind(), ErrorKind::NotFound, "{error}"); + } + #[test] fn records_of_the_earlier_format_fail_naming_it() { // Before the store, instances kept runtime records and launch markers of format 3. diff --git a/nvx-rs/src/host/python/fs.rs b/nvx-rs/src/host/python/fs.rs index 6a25c9769..2ea2f7143 100644 --- a/nvx-rs/src/host/python/fs.rs +++ b/nvx-rs/src/host/python/fs.rs @@ -79,7 +79,6 @@ pub(crate) fn stat(path: &Path) -> io::Result { } /// Returns `os.lstat(path)`, which does not follow a link that `path` names. -#[cfg(feature = "cli")] pub(crate) fn lstat(path: &Path) -> io::Result { imp::lstat(path) } @@ -160,7 +159,6 @@ mod imp { } /// See [`super::lstat`]. - #[cfg(feature = "cli")] pub(super) fn lstat(path: &Path) -> io::Result { status(fs::symlink_metadata(path)?) } @@ -317,7 +315,6 @@ mod imp { } /// See [`super::lstat`]. - #[cfg(feature = "cli")] pub(super) fn lstat(path: &Path) -> io::Result { let status = win32::stat(path, false)?; Ok(Stat { @@ -512,7 +509,6 @@ mod imp { Err(unsupported()) } - #[cfg(feature = "cli")] pub(super) fn lstat(_path: &Path) -> io::Result { Err(unsupported()) } diff --git a/nvx-rs/src/host/store/caller.rs b/nvx-rs/src/host/store/caller.rs new file mode 100644 index 000000000..6c2dcadf3 --- /dev/null +++ b/nvx-rs/src/host/store/caller.rs @@ -0,0 +1,428 @@ +//! The files that a store's caller keeps in the sandbox's directory beside the store's own, and the +//! names of the temporary files that the store owns there. + +use super::FILE_NAMES; +use crate::error::{Error, Result}; + +/// The files that the caller of a [`SandboxStore`](super::SandboxStore) keeps in the sandbox's +/// directory beside the store's own: the configuration record whose presence means that the +/// sandbox is provisioned, if the caller keeps one, and the caller's other files, such as the +/// socket of a boot console. +/// +/// The store's [lifecycle locks](super#lifecycle-locks) and its [removal](super#removal) take +/// the declaration: a holder that leaves the directory without the configuration record removes +/// its lock file as it releases the lock, and removal removes the caller's files with the store's, +/// the configuration record last. A caller that declares no configuration record, as +/// [`none`](Self::none) declares, keeps nothing that tells whether its sandbox is provisioned, +/// so the directory itself is the sandbox: its lock file stays when the lock is released, and the +/// store cannot be removed with the declaration. +/// +/// Each name is one plain file name, which [`new`](Self::new) and [`with`](Self::with) check, so +/// a declaration is valid before any store is created, opened, locked, or removed with it: +/// +/// - It is not empty, `.`, or `..`, and has no path separator, `/` or `\` on any host, so it +/// names no root, prefix, or path outside the directory. +/// - It has none of the characters that Windows refuses in a file name, a control character, +/// `"`, `*`, `:`, `<`, `>`, `?`, or `|`, no trailing dot or space, which Windows drops, and it is +/// not a name that Windows reserves for a device, such as `NUL` or `com1.json`, so it names the +/// same file on every host. +/// - It has only the portable file name characters, ASCII letters and digits, `.`, `_`, and `-`, +/// so that no file system takes it for another name: file systems ignore the case of other +/// letters by rules of their own, macOS's take a composed letter for the same letter followed +/// by a combining mark, and Windows also names files by short names, such as `RUNTIM~1.JSO` for +/// `runtime.json`. +/// - It is none of the store's [`FILE_NAMES`], the lock included, has neither form of a temporary +/// file that the store owns, and does not have the form of a removed file that a removal waits +/// for, `.nfs*`. +/// - It is declared once: the configuration record's name differs from the others. +/// +/// Names are compared without regard to ASCII case, as Windows and macOS file systems compare +/// them. +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub struct CallerFiles { + /// The configuration record, whose presence means that the sandbox is provisioned. + config: Option, + /// The caller's other files. + others: Vec, +} + +impl CallerFiles { + /// Declares no files: the caller keeps nothing in the sandbox's directory, which is itself the + /// sandbox. + pub fn none() -> Self { + Self::default() + } + + /// Declares `config`, the caller's configuration record, whose presence in the directory means + /// that the sandbox is provisioned. + /// + /// Fails with [`ErrorKind::Unsupported`](crate::ErrorKind::Unsupported) for a name that is not + /// one plain file name of the caller's, as the [type](Self) describes. + pub fn new(config: impl Into) -> Result { + let config = config.into(); + let mut files = Self::none(); + files.check(&config)?; + files.config = Some(config); + Ok(files) + } + + /// Adds `name`, another file of the caller's. + /// + /// Fails with [`ErrorKind::Unsupported`](crate::ErrorKind::Unsupported) for a name that is not + /// one plain file name of the caller's, as the [type](Self) describes, or that is declared + /// already. + pub fn with(mut self, name: impl Into) -> Result { + let name = name.into(); + self.check(&name)?; + self.others.push(name); + Ok(self) + } + + /// Returns the name of the caller's configuration record, if it keeps one. + pub fn config(&self) -> Option<&str> { + self.config.as_deref() + } + + /// Returns the names of the caller's other files. + pub fn others(&self) -> &[String] { + &self.others + } + + /// Returns every declared name, in the order in which removal removes them: the other files, + /// then the configuration record. + pub(super) fn names(&self) -> impl Iterator { + self.others + .iter() + .map(String::as_str) + .chain(self.config.as_deref()) + } + + /// Fails if `name` cannot be declared beside the names declared so far. + fn check(&self, name: &str) -> Result<()> { + match name_problem(name).or_else(|| self.collision(name)) { + Some(problem) => Err(Error::unsupported(format!( + "the caller's file name {name:?} {problem}" + ))), + None => Ok(()), + } + } + + /// Returns why `name` cannot be declared beside the store's files, its temporary files, and + /// the names declared so far, comparing names, which have only ASCII characters, without + /// regard to case. + fn collision(&self, name: &str) -> Option<&'static str> { + let folded = name.to_ascii_lowercase(); + if FILE_NAMES.contains(&folded.as_str()) { + Some("is the name of one of the store's files") + } else if is_store_temporary(&folded) || is_report_staging(&folded) { + Some("has the form of a temporary file that the store owns") + } else if folded.starts_with(".nfs") { + Some("has the form of a removed file that a removal waits for") + } else if self + .names() + .any(|declared| declared.eq_ignore_ascii_case(name)) + { + Some("is declared already") + } else { + None + } + } +} + +/// Returns why `name` is not one plain file name that names the same file on every host, if it is +/// not. +fn name_problem(name: &str) -> Option<&'static str> { + if name.is_empty() { + Some("is empty") + } else if name == "." || name == ".." { + Some("names a directory rather than a file in it") + } else if name.contains(['/', '\\']) { + Some("has a path separator") + } else if name + .chars() + .any(|c| c.is_control() || matches!(c, '"' | '*' | ':' | '<' | '>' | '?' | '|')) + { + Some("has a character that Windows refuses in a file name") + } else if name.ends_with(['.', ' ']) { + Some("ends with a dot or a space, which Windows drops") + } else if reserves_device(name) { + Some("is a name that Windows reserves for a device") + } else if !name + .bytes() + .all(|byte| byte.is_ascii_alphanumeric() || matches!(byte, b'.' | b'_' | b'-')) + { + Some("has a character other than ASCII letters and digits, `.`, `_`, and `-`") + } else { + None + } +} + +/// Returns whether Windows takes `name` for a device: a name whose part before its first dot, +/// without trailing spaces, is `CON`, `PRN`, `AUX`, `NUL`, `CONIN$`, `CONOUT$`, or `COM` or `LPT` +/// and one digit, which may be a superscript one, two, or three, in any case. +fn reserves_device(name: &str) -> bool { + let stem = name + .split('.') + .next() + .unwrap_or_default() + .trim_end_matches(' ') + .to_ascii_uppercase(); + let numbered = |prefix: &str| { + stem.strip_prefix(prefix).is_some_and(|rest| { + let mut digits = rest.chars(); + matches!(digits.next(), Some('0'..='9' | '¹' | '²' | '³')) && digits.next().is_none() + }) + }; + matches!( + stem.as_str(), + "CON" | "PRN" | "AUX" | "NUL" | "CONIN$" | "CONOUT$" + ) || numbered("COM") + || numbered("LPT") +} + +/// Returns whether `name` is a temporary file that the store leaves beside one of its files when a +/// replacement of the file is interrupted: `.NAME.NUMBER.tmp`. +pub(super) fn is_store_temporary(name: &str) -> bool { + name.strip_prefix('.') + .and_then(|name| name.strip_suffix(".tmp")) + .and_then(|name| name.rsplit_once('.')) + .is_some_and(|(file, number)| { + FILE_NAMES.contains(&file) + && !number.is_empty() + && number.bytes().all(|byte| byte.is_ascii_digit()) + }) +} + +/// Returns whether `name` is the file in which the VMM stages its outcome report before it +/// publishes it as `outcome.json`, which a VMM that is killed while it writes the report leaves: +/// `.tmp` and six ASCII letters or digits, the name that the `tempfile` crate gives it. +pub(super) fn is_report_staging(name: &str) -> bool { + name.strip_prefix(".tmp").is_some_and(|suffix| { + suffix.len() == 6 && suffix.bytes().all(|byte| byte.is_ascii_alphanumeric()) + }) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::ErrorKind; + + /// Returns the message with which declaring `config`, and then `others`, fails. + fn refusal(config: &str, others: &[&str]) -> String { + let declared = CallerFiles::new(config).and_then(|files| { + others + .iter() + .try_fold(files, |files, &name| files.with(name)) + }); + let error = declared.unwrap_err(); + assert_eq!(error.kind(), ErrorKind::Unsupported, "{error}"); + error.message().to_owned() + } + + #[test] + fn declarations_keep_their_names() { + let files = CallerFiles::new("sandbox.json") + .unwrap() + .with("boot.sock") + .unwrap() + .with(".config.json.1.tmp") + .unwrap(); + assert_eq!(files.config(), Some("sandbox.json")); + assert_eq!(files.others(), ["boot.sock", ".config.json.1.tmp"]); + assert_eq!( + files.names().collect::>(), + ["boot.sock", ".config.json.1.tmp", "sandbox.json"] + ); + let none = CallerFiles::none().with("boot.sock").unwrap(); + assert_eq!(none.config(), None); + assert_eq!(none.names().collect::>(), ["boot.sock"]); + assert_eq!(CallerFiles::none(), CallerFiles::default()); + } + + #[test] + fn names_that_leave_the_directory_are_refused_on_every_host() { + for (name, problem) in [ + ("", "is empty"), + (".", "names a directory rather than a file in it"), + ("..", "names a directory rather than a file in it"), + ("/etc/passwd", "has a path separator"), + ("../config.json", "has a path separator"), + ("state/config.json", "has a path separator"), + ("config.json/", "has a path separator"), + ("C:\\config.json", "has a path separator"), + ("..\\config.json", "has a path separator"), + ("\\\\?\\C:\\config.json", "has a path separator"), + ("\\", "has a path separator"), + ("C:", "has a character that Windows refuses in a file name"), + ( + "C:config.json", + "has a character that Windows refuses in a file name", + ), + ( + "config.json:stream", + "has a character that Windows refuses in a file name", + ), + ( + "config\0.json", + "has a character that Windows refuses in a file name", + ), + ( + "config\n.json", + "has a character that Windows refuses in a file name", + ), + ( + "config?.json", + "has a character that Windows refuses in a file name", + ), + ( + "config.json.", + "ends with a dot or a space, which Windows drops", + ), + ( + "config.json ", + "ends with a dot or a space, which Windows drops", + ), + ("NUL", "is a name that Windows reserves for a device"), + ("nul.json", "is a name that Windows reserves for a device"), + ("Com1.json", "is a name that Windows reserves for a device"), + ("lpt\u{b9}", "is a name that Windows reserves for a device"), + ("CON .json", "is a name that Windows reserves for a device"), + ("conout$", "is a name that Windows reserves for a device"), + ] { + let expected = format!("the caller's file name {name:?} {problem}"); + assert_eq!(refusal(name, &[]), expected); + assert_eq!(refusal("config.json", &[name]), expected); + } + // Names that only resemble those are plain file names. + for name in [ + "...json", + "com10", + "console.json", + "nul-file", + "lpt", + "a_b.json", + ] { + CallerFiles::new(name).unwrap(); + } + } + + #[test] + fn names_that_a_file_system_could_take_for_others_are_refused() { + let problem = "has a character other than ASCII letters and digits, `.`, `_`, and `-`"; + for name in [ + // The uppercase forms of `ı` and `ſ` are `I` and `S`, which `to_lowercase` does not + // tell, so a file system that compares the uppercase forms of names could take these + // for the store's files. + "runt\u{131}me.json", + "control.\u{17f}ock", + // macOS's file systems take a composed letter for the same letter followed by a + // combining mark. + "caf\u{e9}.json", + "cafe\u{301}.json", + // Windows names `runtime.json` by this short name too on volumes with short names. + "RUNTIM~1.JSO", + "a b.json", + ] { + let expected = format!("the caller's file name {name:?} {problem}"); + assert_eq!(refusal(name, &[]), expected); + assert_eq!(refusal("config.json", &[name]), expected); + } + } + + #[test] + fn names_that_collide_are_refused_whatever_their_case() { + for (name, problem) in [ + ("sandbox.lock", "is the name of one of the store's files"), + ("Runtime.JSON", "is the name of one of the store's files"), + ("outcome.json", "is the name of one of the store's files"), + ( + ".runtime.json.12345.tmp", + "has the form of a temporary file that the store owns", + ), + ( + ".SANDBOX.LOCK.1.TMP", + "has the form of a temporary file that the store owns", + ), + ( + ".tmpAbC123", + "has the form of a temporary file that the store owns", + ), + ( + ".TMP000000", + "has the form of a temporary file that the store owns", + ), + ( + ".nfs000000001", + "has the form of a removed file that a removal waits for", + ), + ( + ".NFS1", + "has the form of a removed file that a removal waits for", + ), + ] { + let expected = format!("the caller's file name {name:?} {problem}"); + assert_eq!(refusal(name, &[]), expected); + assert_eq!(refusal("config.json", &[name]), expected); + } + let twice = |name: &str| format!("the caller's file name {name:?} is declared already"); + assert_eq!( + refusal("config.json", &["config.json"]), + twice("config.json") + ); + assert_eq!( + refusal("config.json", &["CONFIG.json"]), + twice("CONFIG.json") + ); + assert_eq!( + refusal("config.json", &["boot.sock", "Boot.Sock"]), + twice("Boot.Sock") + ); + let none = CallerFiles::none().with("boot.sock").unwrap(); + assert_eq!( + none.with("BOOT.sock").unwrap_err().message(), + twice("BOOT.sock") + ); + } + + #[test] + fn store_temporaries_name_one_of_the_stores_files_and_a_number() { + for name in [ + ".runtime.json.0.tmp", + ".sandbox.lock.18446744073709551615.tmp", + ] { + assert!(is_store_temporary(name), "{name}"); + } + for name in [ + ".config.json.1.tmp", + ".runtime.json..tmp", + ".runtime.json.1a.tmp", + "runtime.json.1.tmp", + ".runtime.json.1.tmp.tmp", + ".tmpAbC123", + ".Runtime.json.1.tmp", + ] { + assert!(!is_store_temporary(name), "{name}"); + } + } + + #[test] + fn report_staging_files_have_the_names_that_tempfile_gives_them() { + for name in [".tmpAbC123", ".tmp000000", ".tmpzzzzzz"] { + assert!(is_report_staging(name), "{name}"); + } + // Other names are not the VMM's, whatever they resemble. + for name in [ + ".tmpAbC12", + ".tmpAbC1234", + ".tmpAb-123", + ".tmpAb.123", + ".tmpAbC12\u{e9}", + ".TMPAbC123", + "tmpAbC123", + ".tmp", + ".runtime.json.1.tmp", + ] { + assert!(!is_report_staging(name), "{name}"); + } + } +} diff --git a/nvx-rs/src/host/store/lifecycle.rs b/nvx-rs/src/host/store/lifecycle.rs new file mode 100644 index 000000000..7d2dbf938 --- /dev/null +++ b/nvx-rs/src/host/store/lifecycle.rs @@ -0,0 +1,611 @@ +//! The lifecycle of a sandbox's store beside its caller's files: the locks of a provision and of an +//! existing sandbox, the release of a lock in a directory that holds no provisioned sandbox, and +//! the removal of the store and its directory. The [module documentation](super#lifecycle-locks) +//! describes their rules. + +use std::fmt; +use std::fs::{self, File}; +use std::io; +use std::path::{Path, PathBuf}; +use std::time::Instant; + +use super::caller::{is_report_staging, is_store_temporary}; +use super::{ + CallerFiles, FILE_NAMES, JOURNAL_NAME, LOCK_NAME, LockedStore, RUNTIME_NAME, Races, + SandboxStore, Taken, absolute, pause, transition_in_progress, +}; +use crate::error::{Error, ErrorKind, Result}; +use crate::host::platform; +use crate::host::python::fs::lstat; +use crate::host::python::not_unicode; + +#[cfg(test)] +mod tests; + +impl SandboxStore { + /// Takes the lock of the sandbox in `dir` for its provision, by a caller that keeps `files` + /// in the directory, waiting for it until `deadline`, or for as long as it takes without one. + /// + /// The directory, whose parent must exist, is created if needed, private to the current user, + /// as [`create`](Self::create) creates it, and created again if a concurrent removal deletes + /// it before the lock is taken, also while it is created and checked. The caller checks under + /// the lock whether its configuration record exists already, and writes it. + /// + /// Fails with [`ErrorKind::TimedOut`] when the deadline passes first, also while Windows + /// refuses to open a removed lock file, as [`lock_existing`](Self::lock_existing) describes, + /// or while concurrent transitions keep deleting the directory or replacing the lock file, + /// with [`ErrorKind::NotFound`] if the parent does not exist, and as [`create`](Self::create) + /// fails for a directory that is not private. + pub fn lock_for_provision( + dir: impl Into, + files: &CallerFiles, + deadline: Option, + ) -> Result { + let dir = absolute(dir.into())?; + Self::provision_lock( + &dir, + files, + deadline, + |dir| Self::create(dir), + Self::take_lifecycle_lock, + ) + } + + /// Takes the lock as [`lock_for_provision`](Self::lock_for_provision) does, with `create` + /// creating or checking the absolute directory `dir`, as [`create`](Self::create) does, and + /// `take` taking the lock of its store, as [`take_lifecycle_lock`](Self::take_lifecycle_lock) + /// does. + fn provision_lock( + dir: &Path, + files: &CallerFiles, + deadline: Option, + mut create: impl FnMut(&Path) -> Result, + mut take: impl FnMut(&Self, Option) -> Result, + ) -> Result { + let mut races = Races::default(); + loop { + let error = match create(dir) { + Ok(store) => match take(&store, deadline) { + Ok(Taken::Locked(lock)) => { + return Ok(LockedStore::new(store, lock, files.clone())); + } + // A removal deleted the lock file while this waited for its lock, and maybe the + // directory, or a transition replaced it as it released its lock: the + // directory is created and locked again. + Ok(Taken::Replaced) => { + if !races.lost(deadline) { + return Err(transition_in_progress(dir)); + } + continue; + } + Err(error) => error, + }, + Err(error) => error, + }; + // A removal deleted the directory after it was created, before it was checked or its + // lock file was opened: unless its parent is gone, it is created again. + if error.kind() != ErrorKind::NotFound || !dir.parent().is_some_and(Path::is_dir) { + return Err(error); + } + if !races.lost(deadline) { + return Err(transition_in_progress(dir)); + } + } + } + + /// Takes the lock of the existing sandbox in `dir`, by a caller that keeps `files` in the + /// directory, waiting for it until `deadline`, or for as long as it takes without one. + /// + /// Fails with [`ErrorKind::NotFound`] if the directory does not exist, also once a concurrent + /// removal deletes it while this waits: a lock file that is removed or replaced meanwhile + /// locks nothing, so the lock of the lock file at the path is taken again, as + /// [`lock`](Self::lock) does, which fails once the directory is gone. A directory that exists + /// is locked even if it holds no provisioned sandbox, as one that was never provisioned, or + /// that an interrupted removal emptied, or whose removal open files still keep: the caller + /// checks its configuration record under the lock, and the release then removes the lock file + /// again. + /// + /// On Windows, a file system without POSIX deletion keeps a removed lock file under its name + /// until its last handle closes, and refuses to open it until then, so the open is tried + /// again every 25 ms. Fails with [`ErrorKind::TimedOut`] when the deadline passes first, as + /// [`lock`](Self::lock) does. A lock file that Windows refuses to open for good, such as a + /// read-only one, still opens without access rights, unlike a removed one, and fails the lock + /// with the denied access. + pub fn lock_existing( + dir: impl Into, + files: &CallerFiles, + deadline: Option, + ) -> Result { + Self::existing_lock(Self::open(dir)?, files, deadline, Self::take_lifecycle_lock) + } + + /// Takes the lock as [`lock_existing`](Self::lock_existing) does, of the opened `store`, with + /// `take` taking it, as [`take_lifecycle_lock`](Self::take_lifecycle_lock) does. + fn existing_lock( + store: Self, + files: &CallerFiles, + deadline: Option, + mut take: impl FnMut(&Self, Option) -> Result, + ) -> Result { + let mut races = Races::default(); + loop { + if let Taken::Locked(lock) = take(&store, deadline)? { + return Ok(LockedStore::new(store, lock, files.clone())); + } + if !races.lost(deadline) { + return Err(transition_in_progress(store.dir())); + } + } + } + + /// Takes the lock as [`take_lock`](Self::take_lock) does, and opens a lock file that Windows + /// refuses to open again every 25 ms until `deadline`, as a removed one that other handles + /// still hold, which a file system without POSIX deletion keeps until they close. A lock file + /// that Windows refuses to open although it opens without access rights is not a removed one, + /// and fails the lock once a second open at once is refused too. + fn take_lifecycle_lock(&self, deadline: Option) -> Result { + // Whether the latest attempt was refused a lock file that opened without access rights. + let mut refused_for_good = false; + loop { + match self.take_lock(deadline) { + Err(error) if cfg!(windows) && denied(&error) => { + if !pending_deletion(&self.path(LOCK_NAME)) { + // The lock file refuses access for good, such as a read-only one, unless + // the removed one went, and another replaced it, after the open failed. + if refused_for_good { + return Err(error); + } + refused_for_good = true; + continue; + } + refused_for_good = false; + if !pause(deadline) { + return Err(transition_in_progress(self.dir())); + } + } + taken => return taken, + } + } + } +} + +/// Returns whether the store's `error` is an I/O error whose cause is a denied access. +fn denied(error: &Error) -> bool { + error.kind() == ErrorKind::Io + && std::error::Error::source(error) + .and_then(|source| source.downcast_ref::()) + .is_some_and(|source| source.kind() == io::ErrorKind::PermissionDenied) +} + +/// Returns whether the file at `path`, which Windows refused to open, is a removed one that other +/// handles still hold: Windows refuses every open of such a file, also one without access rights, +/// which opens a file whose access it refuses otherwise. +fn pending_deletion(path: &Path) -> bool { + platform::file_identity(path) + .is_err_and(|error| error.kind() == io::ErrorKind::PermissionDenied) +} + +impl Drop for LockedStore { + /// Releases the lock. A holder that leaves the directory without its caller's configuration + /// record removes its own lock file first, unless it removed it already, so that a directory + /// that holds no provisioned sandbox keeps no lock file, and empties. + fn drop(&mut self) { + let Some(config) = self.files.config() else { + return; + }; + if self.lock_removed + || !matches!( + fs::symlink_metadata(self.path(config)), + Err(error) if error.kind() == io::ErrorKind::NotFound + ) + { + return; + } + let path = self.path(LOCK_NAME); + let held = platform::opened_file_identity(&self.lock); + if matches!((held, platform::file_identity(&path)), (Ok(held), Ok(current)) if held == current) + { + let _ = fs::remove_file(&path); + } + } +} + +impl LockedStore { + /// Removes the sandbox's store, its caller's files, and its directory, which only a sandbox + /// that no VMM can run in allows, and releases the lock. + /// + /// The removal needs the caller's configuration record: a lock that was taken without one, as + /// with [`CallerFiles::none`] or by [`SandboxStore::lock`], refuses with + /// [`RemovalError::Undeclared`], and removes nothing, since a transition that waits through + /// the removal could not tell that the sandbox is gone then, and would keep the lock file that + /// it creates. + /// + /// The removal refuses, and removes nothing, while the store records a VM or a launch: the + /// caller reattaches to the VM and stops it, or recovers the launch, first, as + /// [`supervisor::reattach`](crate::host::supervisor::reattach) does. It then removes the + /// store's files but its lock, the caller's files, its configuration record last, so that an + /// interrupted removal leaves a provisioned sandbox, which can be removed again, and the + /// temporary files that the store owns. It refuses the entries that are neither the store's + /// nor the caller's, which it names in the order in which the directory lists them, and keeps + /// them and the directory; the lock file goes as the lock is released, since the configuration + /// record is gone. Otherwise it removes its lock file while it still holds the lock, so that a + /// transition that waited for the lock finds the sandbox gone rather than halfway removed, and + /// then the directory, as long as the path still names it. + /// + /// Lock files of other transitions, and on NFS other removed files that handles still hold, + /// can keep the emptied directory, and are not refused: NFS renames a removed file that is + /// still open to `.nfs*`; a Windows file system without POSIX deletion keeps a removed lock + /// file under its own name until its last handle closes; and a transition that opens the lock + /// file between the removals of the lock file and of the directory creates a new one. Each of + /// those transitions finds no sandbox, and removes its lock file as it releases its lock, and + /// the handles of other removed files close, so the directory empties. The removal then holds + /// the directory's identity, releases the lock, and returns the [`Lingering`] removal, whose + /// [`wait`](Lingering::wait) removes the directory once it empties. It returns `None` once the + /// directory is gone, whoever removed it. + pub fn remove(self) -> std::result::Result, RemovalError> { + self.remove_contents()?; + self.remove_directory() + } + + /// Empties the directory but for lingering files, its own lock file included, as + /// [`remove`](Self::remove) does first: it refuses without the caller's configuration record + /// and while the store records a VM or a launch, removes the store's files and the caller's, + /// and the temporary files that the store owns, and refuses any other entry. + fn remove_contents(&self) -> std::result::Result<(), RemovalError> { + if self.files.config().is_none() { + return Err(RemovalError::Undeclared); + } + for name in [JOURNAL_NAME, RUNTIME_NAME] { + let path = self.path(name); + match fs::symlink_metadata(&path) { + Ok(_) => return Err(RemovalError::Launched), + Err(error) if error.kind() == io::ErrorKind::NotFound => {} + Err(error) => return Err(RemovalError::Io { path, error }), + } + } + let owned: Vec = FILE_NAMES + .into_iter() + .filter(|&name| name != LOCK_NAME) + .chain(self.files.names()) + .map(|name| self.path(name)) + .collect(); + for path in owned { + remove_entry(path)?; + } + let listed = entries(self.dir()).map_err(|error| RemovalError::Io { + path: self.dir().to_owned(), + error, + })?; + let mut foreign = Vec::new(); + for name in listed { + // Lock files, and removed files that NFS keeps while handles hold them, go once their + // holders release them, which the removal waits for, if they keep the directory. + if lingers(&name) { + continue; + } + if is_store_temporary(&name) || is_report_staging(&name) { + remove_entry(self.path(&name))?; + } else { + foreign.push(name); + } + } + if foreign.is_empty() { + Ok(()) + } else { + Err(RemovalError::Foreign(foreign)) + } + } + + /// Removes the lock file while it still holds the lock, and then the directory, as + /// [`remove`](Self::remove) does once the directory holds nothing else of the sandbox. + fn remove_directory(self) -> std::result::Result, RemovalError> { + self.remove_directory_after(|| {}) + } + + /// Removes the lock file and the directory as [`remove_directory`](Self::remove_directory) + /// does, and runs `unlinked` between the two, where the transitions that the removal races + /// can act. + fn remove_directory_after( + mut self, + unlinked: impl FnOnce(), + ) -> std::result::Result, RemovalError> { + let dir = self.dir().to_owned(); + // While the lock is held, its file keeps the directory, which the path then names. + let (identity, held) = hold_directory(&dir).map_err(|error| RemovalError::Io { + path: dir.clone(), + error, + })?; + let lock = self.path(LOCK_NAME); + fs::remove_file(&lock).map_err(|error| RemovalError::Io { path: lock, error })?; + self.lock_removed = true; + unlinked(); + match remove_held_directory(&dir, identity) { + // The directory is gone: this removal removed it, or another one did once it emptied, + // after which the path may name another directory, or none. + Ok(_) => return Ok(None), + Err(error) if error.kind() == io::ErrorKind::NotFound => return Ok(None), + Err(error) if directory_not_empty(&error) => {} + Err(error) => return Err(RemovalError::Io { path: dir, error }), + } + let config = self.files.config().map(str::to_owned); + drop(self); + Ok(Some(Lingering { + dir, + identity, + _held: held, + config, + retried_at_once: false, + })) + } +} + +/// The removal of a sandbox's directory that open files, such as the lock files of other +/// transitions, keep, which [`LockedStore::remove`] returns once it has removed everything else +/// and released the lock. +/// +/// It holds the identity of the directory that it removes: on Unix, its open handle keeps a +/// directory that replaces it from reusing its inode number, and NTFS and ReFS do not reuse file +/// IDs. The path may name another directory once this one is gone, such as one that a provision +/// created again, which is not this removal's to remove. +#[derive(Debug)] +pub struct Lingering { + /// The directory. + dir: PathBuf, + /// The identity of the directory, which `lstat` gives. + identity: (u64, u128), + /// The open directory on Unix, which keeps its identity. + _held: Option, + /// The caller's configuration record, which a provision of the path writes. + config: Option, + /// Whether the latest attempt was the one that followed at once a failure without remaining + /// entries. + retried_at_once: bool, +} + +impl Lingering { + /// Returns the directory that the removal waits for. + pub fn dir(&self) -> &Path { + &self.dir + } + + /// Removes the directory once open files no longer keep it, trying again every 25 ms until + /// `deadline`, or for as long as it takes without one. A deadline that has passed still tries + /// once, so a caller that waits on its own, as for an interruption, calls this again, and a + /// removal that timed out can go on. + /// + /// Returns once the directory is gone, whoever removed it, or once the path names another + /// directory, which it checks before each attempt to remove it. Fails, and keeps the + /// directory, with [`RemovalError::ProvisionedAgain`] once the caller's configuration record + /// appears in it, as a provision that took the lock after the removal released it writes it, + /// which it looks up by its path, so that a file system that ignores case finds it whatever + /// the case of the name that the provision gave it, with [`RemovalError::Foreign`] for other + /// entries than lock files and the removed files that NFS keeps, sorted, and with + /// [`RemovalError::TimedOut`] once the deadline passes. + pub fn wait(&mut self, deadline: Option) -> std::result::Result<(), RemovalError> { + loop { + let error = match remove_held_directory(&self.dir, self.identity) { + Ok(_) => return Ok(()), + Err(error) if error.kind() == io::ErrorKind::NotFound => return Ok(()), + Err(error) => error, + }; + if directory_status(&self.dir) != Some(self.identity) { + return Ok(()); + } + let remaining = match entries(&self.dir) { + Ok(remaining) => remaining, + Err(error) if error.kind() == io::ErrorKind::NotFound => return Ok(()), + Err(error) => { + return Err(RemovalError::Io { + path: self.dir.clone(), + error, + }); + } + }; + let mut unexpected: Vec = remaining + .iter() + .filter(|name| !lingers(name)) + .cloned() + .collect(); + unexpected.sort(); + if !unexpected.is_empty() { + // A provision that took the lock after the removal released it, before the + // directory went, provisions the directory again. Its record is looked up by its + // path, which a file system that ignores case resolves to the name that the + // provision gave it, and one that does not keeps apart from other names. + if let Some(config) = &self.config { + let path = self.dir.join(config); + match fs::symlink_metadata(&path) { + Ok(_) => return Err(RemovalError::ProvisionedAgain), + Err(error) if error.kind() == io::ErrorKind::NotFound => {} + Err(error) => return Err(RemovalError::Io { path, error }), + } + } + return Err(RemovalError::Foreign(unexpected)); + } + if !directory_not_empty(&error) { + return Err(RemovalError::Io { + path: self.dir.clone(), + error, + }); + } + // Without remaining entries, the last open file went away after the removal failed, + // so the next attempt follows at once. Another such failure right after it pauses + // first, so that a file system that does not report emptiness right away cannot spin. + if remaining.is_empty() && !self.retried_at_once { + self.retried_at_once = true; + continue; + } + self.retried_at_once = false; + if !pause(deadline) { + return Err(RemovalError::TimedOut(error)); + } + } + } +} + +/// Why [`LockedStore::remove`] or [`Lingering::wait`] kept a sandbox's directory. +#[derive(Debug)] +#[non_exhaustive] +pub enum RemovalError { + /// The lock was taken without the caller's configuration record, which the removal needs, and + /// nothing was removed: see [`LockedStore::remove`]. + Undeclared, + /// The store records a VM or a launch, so a VMM of the sandbox may still run, and nothing was + /// removed. + Launched, + /// The directory holds entries that are neither the store's nor its caller's, which the + /// removal keeps, with the directory: their names, in the order in which the directory lists + /// them, or sorted once the removal waited for open files. + Foreign(Vec), + /// The sandbox was provisioned again while the removal waited for open files: the caller's + /// configuration record appeared in the directory, which the removal keeps. + ProvisionedAgain, + /// Open files still kept the directory when the deadline passed: the error with which the + /// latest attempt to remove it failed. + TimedOut(io::Error), + /// An operation on one of the sandbox's files, or on its directory, failed. + Io { + /// The file or directory. + path: PathBuf, + /// The error of the operation. + error: io::Error, + }, +} + +impl fmt::Display for RemovalError { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::Undeclared => formatter.write_str( + "the removal of a sandbox needs its caller's configuration record, which its lock \ + was not taken with", + ), + Self::Launched => { + formatter.write_str("the sandbox's store records a VM or a launch, which may run") + } + Self::Foreign(names) => write!( + formatter, + "the sandbox's directory holds entries that are neither the store's nor its \ + caller's: {}", + names.join(", ") + ), + Self::ProvisionedAgain => formatter + .write_str("the sandbox was provisioned again while its directory was removed"), + Self::TimedOut(_) => { + formatter.write_str("files that are still open keep the sandbox's directory") + } + Self::Io { path, .. } => write!(formatter, "cannot remove {}", path.display()), + } + } +} + +impl std::error::Error for RemovalError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + match self { + Self::TimedOut(error) | Self::Io { error, .. } => Some(error), + _ => None, + } + } +} + +/// Keeps the message of a removal's error, and its I/O cause: a removal without the caller's +/// configuration record is of [`ErrorKind::Unsupported`], a timeout of [`ErrorKind::TimedOut`], an +/// I/O failure of [`ErrorKind::Io`], and every other refusal of [`ErrorKind::Other`]. +impl From for Error { + fn from(error: RemovalError) -> Self { + let message = error.to_string(); + match error { + RemovalError::Undeclared => Error::unsupported(message), + RemovalError::TimedOut(source) => Error::timed_out(message).with_source(source), + RemovalError::Io { error: source, .. } => Error::io(message).with_source(source), + _ => Error::other(message), + } + } +} + +/// Removes the file at `path` if it exists. +fn remove_entry(path: PathBuf) -> std::result::Result<(), RemovalError> { + match fs::remove_file(&path) { + Err(error) if error.kind() != io::ErrorKind::NotFound => { + Err(RemovalError::Io { path, error }) + } + _ => Ok(()), + } +} + +/// Returns whether `name` is a lock file, or a removed file that a handle still keeps, which goes +/// once its holder releases or closes it: NFS renames a removed file that is still open to +/// `.nfs*`, and Windows file systems without POSIX deletion keep a removed lock file under its +/// own name, as they keep the lock files that waiting transitions create until they release them. +fn lingers(name: &str) -> bool { + name == LOCK_NAME || name.starts_with(".nfs") +} + +/// Returns whether `error` is how the removal of a directory reports one that is not empty. +fn directory_not_empty(error: &io::Error) -> bool { + #[cfg(windows)] + let not_empty = matches!(error.raw_os_error(), Some(145 | 80 | 183)); + #[cfg(unix)] + let not_empty = matches!( + error.raw_os_error(), + Some(code) if code == nix::errno::Errno::ENOTEMPTY as i32 + || code == nix::errno::Errno::EEXIST as i32 + ); + #[cfg(not(any(windows, unix)))] + let not_empty = error.kind() == io::ErrorKind::DirectoryNotEmpty; + not_empty +} + +/// Returns the names of the entries of `directory`, in the order in which it lists them. +fn entries(directory: &Path) -> io::Result> { + fs::read_dir(directory)? + .map(|entry| { + entry? + .file_name() + .into_string() + .map_err(|_| not_unicode("a file name")) + }) + .collect() +} + +/// Returns the identity of the directory `path`, as `lstat` gives it, and a handle that keeps it: +/// on Unix, the open directory keeps a directory that replaces it from reusing its inode number, +/// and NTFS and ReFS do not reuse file IDs. +fn hold_directory(path: &Path) -> io::Result<((u64, u128), Option)> { + #[cfg(unix)] + { + use std::os::unix::fs::{MetadataExt, OpenOptionsExt}; + + let file = fs::OpenOptions::new() + .read(true) + .custom_flags((nix::fcntl::OFlag::O_DIRECTORY | nix::fcntl::OFlag::O_NOFOLLOW).bits()) + .open(path)?; + // The open directory's status gives the identity that `lstat` gives for its path. + let metadata = file.metadata()?; + Ok(((metadata.dev(), u128::from(metadata.ino())), Some(file))) + } + #[cfg(not(unix))] + { + Ok((lstat(path)?.identity(), None)) + } +} + +/// Returns the identity that `lstat` gives for `path`, or `None` if it cannot be examined. +fn directory_status(path: &Path) -> Option<(u64, u128)> { + lstat(path).ok().map(|status| status.identity()) +} + +/// Removes the directory `dir` if the path still names the one that the caller holds, whose +/// identity, which `lstat` gives, is `identity`, so that a directory that replaced it stays. +/// Returns whether the path named it, and fails as the check of the path or the removal fails. +/// +/// The check and the removal are two steps: a directory that replaces the held one between them, +/// which only a concurrent removal of the held one and a creation of another at the path both +/// allow, still goes while it is empty, and its creator then finds it gone, as a provision does, +/// which creates it again. +fn remove_held_directory(dir: &Path, identity: (u64, u128)) -> io::Result { + if lstat(dir)?.identity() != identity { + return Ok(false); + } + fs::remove_dir(dir)?; + Ok(true) +} diff --git a/nvx-rs/src/host/store/lifecycle/tests.rs b/nvx-rs/src/host/store/lifecycle/tests.rs new file mode 100644 index 000000000..f7b67f4d5 --- /dev/null +++ b/nvx-rs/src/host/store/lifecycle/tests.rs @@ -0,0 +1,991 @@ +//! Tests of the lifecycle locks, their release, and the removal of a store, by threads of one +//! process, and by transitions in processes of their own, which [`transition_role`] runs. + +use std::env; +use std::io::{BufRead, BufReader, Read}; +use std::process::{Child, ChildStdout, Command, Stdio}; +use std::sync::mpsc; +use std::thread; +use std::time::Duration; + +use super::*; +use crate::host::store::{CAPABILITY_NAME, CONSOLE_NAME, LOG_NAME, OUTCOME_NAME, SOCKET_NAME}; +use crate::host::test_support::released; + +/// The configuration record of these tests' caller. +const CONFIG: &str = "config.json"; +/// Another file of these tests' caller. +const BOOT: &str = "boot.sock"; +/// A name that NFS gives a removed lock file that a transition still holds open. +const RENAMED: &str = ".nfs00000000000000010000"; +/// The directory of the sandbox whose lock [`transition_role`] takes. +const ROLE_DIR: &str = "NVX_LIFECYCLE_TEST_DIR"; +/// The lock that [`transition_role`] takes: `provision` or `existing`. +const ROLE_LOCK: &str = "NVX_LIFECYCLE_TEST_LOCK"; +/// The line that [`transition_role`] prints before it waits for the lock. +const WAITING: &str = "nvx-lifecycle-test-waiting"; +/// The start of the line in which [`transition_role`] prints how its transition ended. +const OUTCOME: &str = "nvx-lifecycle-test-outcome "; + +/// Returns the files of these tests' caller. +fn files() -> CallerFiles { + CallerFiles::new(CONFIG).unwrap().with(BOOT).unwrap() +} + +/// Returns the deadline `seconds` from now. +fn within(seconds: u64) -> Option { + Some(Instant::now() + Duration::from_secs(seconds)) +} + +/// Returns the names of the entries of `dir`, sorted. +fn entries_of(dir: &Path) -> Vec { + let mut names = entries(dir).unwrap(); + names.sort(); + names +} + +/// Returns `names` as sorted strings. +fn sorted(names: &[&str]) -> Vec { + let mut names: Vec = names.iter().map(|&name| name.to_owned()).collect(); + names.sort(); + names +} + +/// Provisions a sandbox in a fresh temporary directory, and returns the directory of its store. +fn sandbox() -> (tempfile::TempDir, PathBuf) { + let directory = tempfile::tempdir().unwrap(); + let dir = directory.path().join("sandbox"); + let locked = SandboxStore::lock_for_provision(&dir, &files(), None).unwrap(); + assert!(!dir.join(CONFIG).exists()); + fs::write(dir.join(CONFIG), b"{}").unwrap(); + drop(locked); + (directory, dir) +} + +/// Takes the lock of the existing sandbox in `dir`, and starts its removal. +fn remove(dir: &Path) -> std::result::Result, RemovalError> { + SandboxStore::lock_existing(dir, &files(), None) + .unwrap() + .remove() +} + +/// Removes the sandbox in `dir` as [`LockedStore::remove`] does while the lock file of another +/// transition, which NFS renamed as the transition still held it, keeps the directory. +fn remove_lingering(dir: &Path) -> Lingering { + fs::write(dir.join(RENAMED), b"").unwrap(); + remove(dir) + .unwrap() + .expect("a lingering lock file keeps the directory") +} + +/// Takes the lock of the existing sandbox in `dir` until `deadline` and releases it, and returns +/// whether the sandbox was provisioned. +fn transition(dir: &Path, deadline: Option) -> Result { + let locked = SandboxStore::lock_existing(dir, &files(), deadline)?; + Ok(locked.dir().join(CONFIG).exists()) +} + +/// A transition in a process of its own, which [`transition_role`] runs. +struct Transition { + /// The process. + child: Child, + /// Its standard output. + output: BufReader, +} + +impl Transition { + /// Starts a transition that takes the lock `lock` of the sandbox in `dir`, and returns once + /// it is about to wait for the lock. + fn start(dir: &Path, lock: &str) -> Self { + let mut child = Command::new(env::current_exe().unwrap()) + .args([ + "--exact", + "host::store::lifecycle::tests::transition_role", + "--ignored", + "--nocapture", + "--test-threads=1", + ]) + .env(ROLE_DIR, dir) + .env(ROLE_LOCK, lock) + .stdin(Stdio::null()) + .stdout(Stdio::piped()) + .spawn() + .unwrap(); + let mut output = BufReader::new(child.stdout.take().unwrap()); + let mut line = String::new(); + while line.trim_end() != WAITING { + line.clear(); + let read = output.read_line(&mut line).unwrap(); + assert_ne!( + read, 0, + "the transition ended before it waited for the lock" + ); + } + Self { child, output } + } + + /// Returns whether the transition still runs. + fn runs(&mut self) -> bool { + self.child.try_wait().unwrap().is_none() + } + + /// Waits for the transition to end, and returns how it ended, as [`transition_role`] prints + /// it. + fn outcome(mut self) -> String { + let mut rest = String::new(); + self.output.read_to_string(&mut rest).unwrap(); + assert!(self.child.wait().unwrap().success(), "{rest}"); + rest.lines() + .find_map(|line| line.strip_prefix(OUTCOME)) + .expect("the transition printed its outcome") + .to_owned() + } +} + +impl Drop for Transition { + fn drop(&mut self) { + let _ = self.child.kill(); + let _ = self.child.wait(); + } +} + +/// A transition in a process of its own, which [`Transition::start`] runs: it takes the lock that +/// [`ROLE_LOCK`] names of the sandbox in [`ROLE_DIR`] within 30 s, and prints how it ended: +/// `provisioned` if it found the configuration record, `unprovisioned` if it did not, which a +/// provision then writes, and `gone` if the sandbox no longer exists. +#[test] +#[ignore = "a transition that the lifecycle tests run in a process of its own"] +fn transition_role() { + let (Some(dir), Ok(lock)) = (env::var_os(ROLE_DIR), env::var(ROLE_LOCK)) else { + return; + }; + // The test harness has printed the test's name without ending the line. + println!("\n{WAITING}"); + let locked = match lock.as_str() { + "provision" => SandboxStore::lock_for_provision(&dir, &files(), within(30)), + "existing" => SandboxStore::lock_existing(&dir, &files(), within(30)), + _ => panic!("unknown lock {lock}"), + }; + let outcome = match locked { + Ok(locked) if locked.dir().join(CONFIG).exists() => "provisioned", + Ok(locked) => { + if lock == "provision" { + fs::write(locked.dir().join(CONFIG), b"{}").unwrap(); + } + "unprovisioned" + } + Err(error) if error.kind() == ErrorKind::NotFound => "gone", + Err(error) => panic!("{error}"), + }; + println!("{OUTCOME}{outcome}"); +} + +#[test] +fn lifecycle_locks_exclude_other_holders_until_released() { + let (_directory, dir) = sandbox(); + let held = SandboxStore::lock_existing(&dir, &files(), None).unwrap(); + let started = Instant::now(); + let deadline = Some(Instant::now() + Duration::from_millis(100)); + let error = SandboxStore::lock_existing(&dir, &files(), deadline).unwrap_err(); + assert_eq!(error.kind(), ErrorKind::TimedOut, "{error}"); + assert!(started.elapsed() >= Duration::from_millis(100)); + let error = SandboxStore::lock_for_provision(&dir, &files(), Some(Instant::now())).unwrap_err(); + assert_eq!(error.kind(), ErrorKind::TimedOut, "{error}"); + // A waiter takes the lock once its holder releases it. + let (sender, receiver) = mpsc::channel(); + let waiter = thread::spawn({ + let dir = dir.clone(); + move || { + let locked = SandboxStore::lock_existing(&dir, &files(), within(30)).map(drop); + sender.send(()).unwrap(); + locked + } + }); + assert!(receiver.recv_timeout(Duration::from_millis(100)).is_err()); + drop(held); + waiter.join().unwrap().unwrap(); + assert!(released(|| { + SandboxStore::lock_existing(&dir, &files(), Some(Instant::now())).is_ok() + })); + // A provisioned directory keeps its lock file. + assert_eq!(entries_of(&dir), sorted(&[CONFIG, LOCK_NAME])); +} + +#[test] +fn released_locks_leave_no_lock_file_in_unprovisioned_directories() { + let directory = tempfile::tempdir().unwrap(); + let dir = directory.path().join("sandbox"); + // A provision that records nothing leaves the directory that it created empty, and so does + // a transition of a sandbox that was never provisioned. + drop(SandboxStore::lock_for_provision(&dir, &files(), None).unwrap()); + assert!(entries_of(&dir).is_empty()); + drop(SandboxStore::lock_existing(&dir, &files(), None).unwrap()); + assert!(entries_of(&dir).is_empty()); + // A directory that does not exist holds no sandbox: only a provision creates it, below a + // parent that exists. + let missing = directory.path().join("missing"); + let error = SandboxStore::lock_existing(&missing, &files(), None).unwrap_err(); + assert_eq!(error.kind(), ErrorKind::NotFound, "{error}"); + assert!(!missing.exists()); + drop(SandboxStore::lock_for_provision(&missing, &files(), None).unwrap()); + assert!(missing.is_dir()); + let orphan = directory.path().join("absent").join("sandbox"); + let error = SandboxStore::lock_for_provision(&orphan, &files(), None).unwrap_err(); + assert_eq!(error.kind(), ErrorKind::NotFound, "{error}"); + assert!(!directory.path().join("absent").exists()); + // A caller that keeps no configuration record keeps the lock file, as the store's own lock + // does. + drop(SandboxStore::lock_existing(&dir, &CallerFiles::none(), None).unwrap()); + assert_eq!(entries_of(&dir), [LOCK_NAME]); + drop(SandboxStore::open(&dir).unwrap().lock(None).unwrap()); + assert_eq!(entries_of(&dir), [LOCK_NAME]); +} + +#[test] +fn a_holder_removes_only_its_own_lock_file_on_release() { + let directory = tempfile::tempdir().unwrap(); + let dir = directory.path().join("sandbox"); + // A holder that removed its lock file, as a removal does while it holds the lock, leaves the + // new lock file of a transition that took the lock meanwhile. + let mut held = SandboxStore::lock_for_provision(&dir, &files(), None).unwrap(); + fs::remove_file(dir.join(LOCK_NAME)).unwrap(); + held.lock_removed = true; + let other = SandboxStore::lock_existing(&dir, &files(), within(10)).unwrap(); + drop(held); + assert_eq!(entries_of(&dir), [LOCK_NAME]); + drop(other); + assert!(entries_of(&dir).is_empty()); + // So does a holder whose lock file another replaced. + let held = SandboxStore::lock_existing(&dir, &files(), None).unwrap(); + fs::remove_file(dir.join(LOCK_NAME)).unwrap(); + fs::write(dir.join(LOCK_NAME), b"").unwrap(); + drop(held); + assert_eq!(entries_of(&dir), [LOCK_NAME]); +} + +#[test] +fn directories_that_others_may_access_are_refused() { + let (_directory, dir) = sandbox(); + #[cfg(unix)] + let problem = { + use std::os::unix::fs::PermissionsExt; + + fs::set_permissions(&dir, fs::Permissions::from_mode(0o755)).unwrap(); + "is accessible to other users (mode 755)" + }; + #[cfg(windows)] + let problem = { + platform::set_access(&dir, "D:(A;OICI;FA;;;WD)").unwrap(); + "allows other users" + }; + for error in [ + SandboxStore::lock_existing(&dir, &files(), None).unwrap_err(), + SandboxStore::lock_for_provision(&dir, &files(), None).unwrap_err(), + ] { + assert_eq!( + error.message(), + format!("the sandbox store {} {problem}", dir.display()) + ); + } +} + +#[test] +fn stale_directories_hold_no_sandbox_and_keep_no_lock_file() { + let directory = tempfile::tempdir().unwrap(); + // A directory that was never provisioned, and those that an interrupted removal left: empty, + // or with a lock file and the temporary file of an interrupted replacement. + let never = SandboxStore::create(directory.path().join("never")).unwrap(); + let emptied = SandboxStore::create(directory.path().join("emptied")).unwrap(); + let left = SandboxStore::create(directory.path().join("left")).unwrap(); + fs::write(left.dir().join(LOCK_NAME), b"").unwrap(); + fs::write(left.dir().join(".runtime.json.1.tmp"), b"{").unwrap(); + for store in [&never, &emptied, &left] { + let locked = SandboxStore::lock_existing(store.dir(), &files(), None).unwrap(); + assert!(!locked.dir().join(CONFIG).exists()); + drop(locked); + assert!(!store.dir().join(LOCK_NAME).exists()); + } + // A removal of such a directory finds nothing of a sandbox, and removes it. + assert!(remove(left.dir()).unwrap().is_none()); + assert!(!left.dir().exists()); +} + +#[test] +fn a_removal_removes_the_store_the_callers_files_and_the_directory() { + let (_directory, dir) = sandbox(); + for name in [ + LOG_NAME, + OUTCOME_NAME, + CONSOLE_NAME, + CAPABILITY_NAME, + SOCKET_NAME, + BOOT, + ".runtime.json.12345.tmp", + ".sandbox.lock.7.tmp", + ".tmpAbC123", + ] { + fs::write(dir.join(name), b"left").unwrap(); + } + assert!(remove(&dir).unwrap().is_none()); + assert!(!dir.exists()); + // A transition that comes later finds no sandbox, and creates nothing. + let error = SandboxStore::lock_existing(&dir, &files(), None).unwrap_err(); + assert_eq!(error.kind(), ErrorKind::NotFound, "{error}"); + assert!(!dir.exists()); +} + +#[test] +fn a_removal_refuses_foreign_entries_and_keeps_them() { + let (_directory, dir) = sandbox(); + fs::write(dir.join(LOG_NAME), b"log").unwrap(); + fs::write(dir.join(BOOT), b"").unwrap(); + // Entries that resemble the store's files and temporary files, or the caller's, are foreign, + // as are the caller's own temporary files, which it removes itself. + let foreign = [ + "notes.txt", + ".config.json.1.tmp", + ".runtime.json.1a.tmp", + "outcome.json.old", + ".tmpAbC1234", + "directory", + ]; + for name in &foreign[..5] { + fs::write(dir.join(name), b"kept").unwrap(); + } + fs::create_dir(dir.join("directory")).unwrap(); + // A removed file that NFS keeps while a handle holds it is not foreign: it goes as the + // handle closes. + fs::write(dir.join(RENAMED), b"").unwrap(); + let RemovalError::Foreign(mut names) = remove(&dir).unwrap_err() else { + panic!("foreign entries were not refused"); + }; + names.sort(); + assert_eq!(names, sorted(&foreign)); + // The store's files and the caller's went first, and the lock file as the lock was released, + // so the sandbox is gone and the foreign entries remain, in the directory. + let mut remaining = foreign.to_vec(); + remaining.push(RENAMED); + assert_eq!(entries_of(&dir), sorted(&remaining)); + assert_eq!(fs::read(dir.join("notes.txt")).unwrap(), b"kept"); + let error = Error::from(RemovalError::Foreign(vec!["a".to_owned(), "b".to_owned()])); + assert_eq!(error.kind(), ErrorKind::Other); + assert_eq!( + error.message(), + "the sandbox's directory holds entries that are neither the store's nor its caller's: a, b" + ); + // Once they are gone, the directory goes too. + for name in &foreign[..5] { + fs::remove_file(dir.join(name)).unwrap(); + } + fs::remove_dir(dir.join("directory")).unwrap(); + fs::remove_file(dir.join(RENAMED)).unwrap(); + assert!(remove(&dir).unwrap().is_none()); + assert!(!dir.exists()); +} + +#[test] +fn a_removal_is_refused_while_a_vm_or_a_launch_is_recorded() { + for name in [JOURNAL_NAME, RUNTIME_NAME] { + let (_directory, dir) = sandbox(); + fs::write(dir.join(name), b"{}").unwrap(); + fs::write(dir.join(LOG_NAME), b"log").unwrap(); + assert!( + matches!(remove(&dir), Err(RemovalError::Launched)), + "{name}" + ); + // Nothing was removed, and the provisioned sandbox keeps its lock file. + assert_eq!( + entries_of(&dir), + sorted(&[CONFIG, LOCK_NAME, LOG_NAME, name]) + ); + } +} + +#[test] +fn lingering_lock_files_keep_the_directory_until_they_go() { + let (_directory, dir) = sandbox(); + let mut lingering = remove_lingering(&dir); + assert_eq!(lingering.dir(), dir); + assert_eq!(entries_of(&dir), [RENAMED]); + // The removal released its lock, and the sandbox is gone. A transition takes the lock of a + // new lock file, which keeps the directory too, and finds no configuration record. + let waiting = SandboxStore::lock_existing(&dir, &files(), within(10)).unwrap(); + assert!(!dir.join(CONFIG).exists()); + let Err(RemovalError::TimedOut(error)) = lingering.wait(Some(Instant::now())) else { + panic!("the directory went while lock files kept it"); + }; + assert!(directory_not_empty(&error), "{error}"); + fs::remove_file(dir.join(RENAMED)).unwrap(); + assert!(matches!( + lingering.wait(Some(Instant::now())), + Err(RemovalError::TimedOut(_)) + )); + // The transition removes its lock file as it releases its lock, which empties the directory. + let removed = thread::spawn(move || lingering.wait(within(30))); + thread::sleep(Duration::from_millis(50)); + drop(waiting); + removed.join().unwrap().unwrap(); + assert!(!dir.exists()); +} + +#[test] +fn a_lingering_removal_reports_what_appears_in_the_directory() { + // A provision that takes the lock that the removal released provisions the directory again. + let (_directory, dir) = sandbox(); + let mut lingering = remove_lingering(&dir); + let provision = SandboxStore::lock_for_provision(&dir, &files(), within(10)).unwrap(); + fs::write(dir.join(CONFIG), b"{}").unwrap(); + drop(provision); + fs::remove_file(dir.join(RENAMED)).unwrap(); + assert!(matches!( + lingering.wait(within(10)), + Err(RemovalError::ProvisionedAgain) + )); + assert_eq!(entries_of(&dir), sorted(&[CONFIG, LOCK_NAME])); + // Other entries are foreign, sorted. + let mut lingering = remove_lingering(&dir); + for name in ["zebra.txt", "apple.txt"] { + fs::write(dir.join(name), b"").unwrap(); + } + let Err(RemovalError::Foreign(names)) = lingering.wait(within(10)) else { + panic!("foreign entries were not refused"); + }; + assert_eq!(names, ["apple.txt", "zebra.txt"]); + // The path may name another directory once the removal's is gone, which is not its to + // remove. + for name in [RENAMED, "zebra.txt", "apple.txt"] { + fs::remove_file(dir.join(name)).unwrap(); + } + fs::remove_dir(&dir).unwrap(); + fs::create_dir(&dir).unwrap(); + fs::write(dir.join(RENAMED), b"").unwrap(); + lingering.wait(Some(Instant::now())).unwrap(); + assert_eq!(entries_of(&dir), [RENAMED]); +} + +#[test] +fn a_lingering_removal_finds_the_configuration_record_by_its_path() { + // A provision by a caller that declares the configuration record in another case writes the + // removal's record on a file system that ignores case, as Windows' do, and another file on + // one that does not. + let (_directory, dir) = sandbox(); + let mut lingering = remove_lingering(&dir); + let upper = CONFIG.to_ascii_uppercase(); + let files = CallerFiles::new(upper.as_str()).unwrap(); + let provision = SandboxStore::lock_for_provision(&dir, &files, within(10)).unwrap(); + fs::write(dir.join(&upper), b"{}").unwrap(); + drop(provision); + fs::remove_file(dir.join(RENAMED)).unwrap(); + let ignores_case = dir.join(CONFIG).exists(); + match lingering.wait(within(10)) { + Err(RemovalError::ProvisionedAgain) => assert!(ignores_case), + Err(RemovalError::Foreign(names)) => { + assert!( + !ignores_case, + "a provision in another case was taken for a foreign file" + ); + assert_eq!(names, std::slice::from_ref(&upper)); + } + other => panic!("{other:?}"), + } + assert_eq!(entries_of(&dir), sorted(&[upper.as_str(), LOCK_NAME])); +} + +#[test] +fn a_removal_that_another_finishes_succeeds_and_keeps_what_replaced_the_directory() { + // Another removal, which takes the lock of a new lock file once this one has removed its own, + // finds the emptied directory, and removes it first. + let removed_by_another = |dir: &Path| { + let other = SandboxStore::lock_existing(dir, &files(), within(10)).unwrap(); + assert!(other.remove().unwrap().is_none()); + }; + let (_directory, dir) = sandbox(); + let locked = SandboxStore::lock_existing(&dir, &files(), None).unwrap(); + locked.remove_contents().unwrap(); + let lingering = locked + .remove_directory_after(|| removed_by_another(&dir)) + .unwrap(); + assert!(lingering.is_none()); + assert!(!dir.exists()); + // A directory that replaces it then stays, even an empty one. + let (_directory, dir) = sandbox(); + let locked = SandboxStore::lock_existing(&dir, &files(), None).unwrap(); + locked.remove_contents().unwrap(); + let lingering = locked + .remove_directory_after(|| { + removed_by_another(&dir); + fs::create_dir(&dir).unwrap(); + }) + .unwrap(); + assert!(lingering.is_none()); + assert!(entries_of(&dir).is_empty()); +} + +#[test] +fn a_lingering_removal_keeps_an_empty_directory_that_replaced_its_own() { + let (_directory, dir) = sandbox(); + let mut lingering = remove_lingering(&dir); + // Another removal finishes this one, and the path is created again, still empty. + fs::remove_file(dir.join(RENAMED)).unwrap(); + fs::remove_dir(&dir).unwrap(); + fs::create_dir(&dir).unwrap(); + lingering.wait(Some(Instant::now())).unwrap(); + assert!(dir.is_dir()); + assert!(entries_of(&dir).is_empty()); +} + +#[test] +fn only_the_directory_that_the_path_still_names_is_removed() { + let directory = tempfile::tempdir().unwrap(); + let dir = directory.path().join("sandbox"); + fs::create_dir(&dir).unwrap(); + let (identity, held) = hold_directory(&dir).unwrap(); + // Another removal removes the held directory, and another one replaces it, which stays. + fs::remove_dir(&dir).unwrap(); + fs::create_dir(&dir).unwrap(); + assert!(!remove_held_directory(&dir, identity).unwrap()); + assert!(dir.is_dir()); + drop(held); + let (identity, _held) = hold_directory(&dir).unwrap(); + assert!(remove_held_directory(&dir, identity).unwrap()); + assert!(!dir.exists()); + // A path that names nothing fails as its check does. + let error = remove_held_directory(&dir, identity).unwrap_err(); + assert_eq!(error.kind(), io::ErrorKind::NotFound, "{error}"); +} + +#[test] +fn a_removal_needs_the_configuration_record() { + let (_directory, dir) = sandbox(); + fs::write(dir.join(LOG_NAME), b"log").unwrap(); + // Without it, a transition that waits through the removal could not tell that the sandbox + // is gone, so neither a lifecycle lock without one nor the store's own lock removes it. + let undeclared = SandboxStore::lock_existing(&dir, &CallerFiles::none(), None).unwrap(); + assert!(matches!(undeclared.remove(), Err(RemovalError::Undeclared))); + let own = SandboxStore::open(&dir).unwrap().lock(None).unwrap(); + assert!(matches!(own.remove(), Err(RemovalError::Undeclared))); + assert_eq!(entries_of(&dir), sorted(&[CONFIG, LOCK_NAME, LOG_NAME])); + let error = Error::from(RemovalError::Undeclared); + assert_eq!(error.kind(), ErrorKind::Unsupported, "{error}"); +} + +#[test] +fn a_provision_creates_again_a_directory_that_a_removal_deletes_while_it_is_created() { + let directory = tempfile::tempdir().unwrap(); + let dir = directory.path().join("sandbox"); + // A concurrent removal deletes the directory after it is created, before it is checked. + let mut attempts = 0; + let locked = SandboxStore::provision_lock( + &dir, + &files(), + within(10), + |dir| { + attempts += 1; + if attempts == 1 { + SandboxStore::create(dir).unwrap(); + fs::remove_dir(dir).unwrap(); + return SandboxStore::open(dir); + } + SandboxStore::create(dir) + }, + SandboxStore::take_lifecycle_lock, + ) + .unwrap(); + assert_eq!(attempts, 2); + assert!(!locked.dir().join(CONFIG).exists()); + assert_eq!(entries_of(&dir), [LOCK_NAME]); + drop(locked); + // A missing parent fails at once. + let orphan = directory.path().join("missing").join("sandbox"); + let error = SandboxStore::lock_for_provision(&orphan, &files(), within(10)).unwrap_err(); + assert_eq!(error.kind(), ErrorKind::NotFound, "{error}"); + // A directory that keeps disappearing fails once the deadline passes. + let gone = directory.path().join("gone"); + let mut attempts = 0; + let deadline = Some(Instant::now() + Duration::from_millis(200)); + let error = SandboxStore::provision_lock( + &dir, + &files(), + deadline, + |_| { + attempts += 1; + SandboxStore::open(&gone) + }, + SandboxStore::take_lifecycle_lock, + ) + .unwrap_err(); + assert_eq!(error.kind(), ErrorKind::TimedOut, "{error}"); + assert!(attempts > 2, "{attempts}"); +} + +#[test] +fn locks_whose_lock_file_keeps_being_replaced_keep_their_deadline() { + // Transitions that keep removing their lock files as they release their locks replace the + // lock file each time that a waiter takes its lock. The waiter takes the lock again at once + // the first time, and after a pause when that repeats, until its deadline. + let (_directory, dir) = sandbox(); + for provision in [false, true] { + for (deadline, most) in [ + (Some(Instant::now()), 2), + (Some(Instant::now() + Duration::from_millis(200)), 20), + ] { + let mut taken = 0; + let replaced = |_: &SandboxStore, _: Option| { + taken += 1; + Ok(Taken::Replaced) + }; + let error = if provision { + SandboxStore::provision_lock( + &dir, + &files(), + deadline, + |dir| SandboxStore::create(dir), + replaced, + ) + } else { + let store = SandboxStore::open(&dir).unwrap(); + SandboxStore::existing_lock(store, &files(), deadline, replaced) + } + .unwrap_err(); + assert_eq!(error.kind(), ErrorKind::TimedOut, "{error}"); + assert!((2..=most).contains(&taken), "{provision} {most}: {taken}"); + } + } +} + +#[test] +fn a_provision_that_waits_through_a_removal_provisions_the_sandbox_again() { + let (_directory, dir) = sandbox(); + let locked = SandboxStore::lock_existing(&dir, &files(), None).unwrap(); + let (ready, waiting) = mpsc::channel(); + let provision = thread::spawn({ + let dir = dir.clone(); + move || { + ready.send(()).unwrap(); + let locked = SandboxStore::lock_for_provision(&dir, &files(), within(30)).unwrap(); + let provisioned = dir.join(CONFIG).exists(); + fs::write(dir.join(CONFIG), b"{}").unwrap(); + drop(locked); + provisioned + } + }); + waiting.recv().unwrap(); + thread::sleep(Duration::from_millis(100)); + // The removal completes before the provision takes the lock, which creates the directory + // again, or, should the provision open the lock file between the removals of the lock file + // and of the directory, reports the provision. Where a removed file stays while a handle + // holds it, as on NFS, the handle with which the provision waits for the removed lock file + // keeps the directory instead, which then goes once the provision closes it, before the + // provision creates the directory again. + match locked.remove().unwrap() { + None => {} + Some(mut lingering) => { + assert!(matches!( + lingering.wait(within(30)), + Ok(()) | Err(RemovalError::ProvisionedAgain) + )); + } + } + assert!(!provision.join().unwrap()); + assert_eq!(entries_of(&dir), sorted(&[CONFIG, LOCK_NAME])); +} + +#[test] +fn transitions_that_wait_through_a_removal_find_the_sandbox_gone() { + let (_directory, dir) = sandbox(); + let locked = SandboxStore::lock_existing(&dir, &files(), None).unwrap(); + let (ready, waiting) = mpsc::channel(); + let waiters: Vec<_> = [None, within(30)] + .into_iter() + .map(|deadline| { + let (dir, ready) = (dir.clone(), ready.clone()); + thread::spawn(move || { + ready.send(()).unwrap(); + transition(&dir, deadline) + }) + }) + .collect(); + for _ in &waiters { + waiting.recv().unwrap(); + } + thread::sleep(Duration::from_millis(100)); + let lingering = locked.remove().unwrap(); + let outcomes: Vec<_> = waiters + .into_iter() + .map(|waiter| waiter.join().unwrap()) + .collect(); + // The waiters wait for the lock of the removed lock file, and find the directory gone. A + // waiter that started late may open the lock file between the removals of the lock file and + // of the directory, which creates a new one, and find no sandbox: the directory then lingers + // until it releases that lock, unless it released it before the removal of the directory. + for outcome in outcomes { + match outcome { + Ok(provisioned) => assert!(!provisioned), + Err(error) => assert_eq!(error.kind(), ErrorKind::NotFound, "{error}"), + } + } + if let Some(mut lingering) = lingering { + lingering.wait(within(30)).unwrap(); + } + assert!(!dir.exists()); +} + +#[test] +fn a_transition_that_waits_for_a_holder_that_provisions_nothing_finds_no_sandbox() { + let directory = tempfile::tempdir().unwrap(); + let dir = directory.path().join("sandbox"); + let held = SandboxStore::lock_for_provision(&dir, &files(), None).unwrap(); + let waiter = thread::spawn({ + let dir = dir.clone(); + move || transition(&dir, within(30)) + }); + thread::sleep(Duration::from_millis(100)); + // The provision fails, and its release removes the lock file that the waiter waits for: the + // waiter takes the lock of a new one in the directory, which holds no sandbox. + drop(held); + assert!(!waiter.join().unwrap().unwrap()); + assert!(entries_of(&dir).is_empty()); +} + +#[test] +fn transitions_in_other_processes_that_wait_through_a_removal_find_the_sandbox_gone() { + let (_directory, dir) = sandbox(); + let locked = SandboxStore::lock_existing(&dir, &files(), None).unwrap(); + let mut transitions: Vec<_> = (0..2) + .map(|_| Transition::start(&dir, "existing")) + .collect(); + thread::sleep(Duration::from_millis(100)); + assert!(transitions.iter_mut().all(Transition::runs)); + let lingering = locked.remove().unwrap(); + let outcomes: Vec<_> = transitions.into_iter().map(Transition::outcome).collect(); + // As with threads, a transition that started late may find no sandbox rather than the + // directory gone, whether the directory lingers or not. + for outcome in &outcomes { + assert!( + ["gone", "unprovisioned"].contains(&outcome.as_str()), + "{outcome}" + ); + } + if let Some(mut lingering) = lingering { + lingering.wait(within(30)).unwrap(); + } + assert!(!dir.exists()); +} + +#[test] +fn provisions_in_other_processes_that_race_a_removal_provision_the_sandbox_again() { + // A provision that waits for the removal's lock creates the directory again once the removal + // completes. + let (_directory, dir) = sandbox(); + let locked = SandboxStore::lock_existing(&dir, &files(), None).unwrap(); + let mut provision = Transition::start(&dir, "provision"); + thread::sleep(Duration::from_millis(100)); + assert!(provision.runs()); + // A lingering removal reports the provision, or removes the directory that the provision's + // handle of the removed lock file kept, as in the threads' case. + if let Some(mut lingering) = locked.remove().unwrap() { + assert!(matches!( + lingering.wait(within(30)), + Ok(()) | Err(RemovalError::ProvisionedAgain) + )); + } + assert_eq!(provision.outcome(), "unprovisioned"); + assert_eq!(entries_of(&dir), sorted(&[CONFIG, LOCK_NAME])); + // One that takes the lock that the removal released while lingering lock files keep the + // directory provisions it again, which the removal reports. + let mut lingering = remove_lingering(&dir); + assert_eq!( + Transition::start(&dir, "provision").outcome(), + "unprovisioned" + ); + fs::remove_file(dir.join(RENAMED)).unwrap(); + assert!(matches!( + lingering.wait(within(10)), + Err(RemovalError::ProvisionedAgain) + )); + assert_eq!(entries_of(&dir), sorted(&[CONFIG, LOCK_NAME])); +} + +#[test] +fn refused_removals_keep_the_directory_for_transitions_in_other_processes() { + // A removal that is refused while the store records a VM removes nothing, and the waiting + // transition finds the sandbox. + let (_directory, dir) = sandbox(); + fs::write(dir.join(RUNTIME_NAME), b"{}").unwrap(); + let locked = SandboxStore::lock_existing(&dir, &files(), None).unwrap(); + let mut transition = Transition::start(&dir, "existing"); + thread::sleep(Duration::from_millis(100)); + assert!(transition.runs()); + assert!(matches!(locked.remove(), Err(RemovalError::Launched))); + assert_eq!(transition.outcome(), "provisioned"); + // One that is refused for foreign entries removed the sandbox's files: the waiting transition + // finds no sandbox, and the directory keeps the foreign entries alone. + fs::remove_file(dir.join(RUNTIME_NAME)).unwrap(); + fs::write(dir.join("notes.txt"), b"kept").unwrap(); + let locked = SandboxStore::lock_existing(&dir, &files(), None).unwrap(); + let mut transition = Transition::start(&dir, "existing"); + thread::sleep(Duration::from_millis(100)); + assert!(transition.runs()); + let Err(RemovalError::Foreign(names)) = locked.remove() else { + panic!("foreign entries were not refused"); + }; + assert_eq!(names, ["notes.txt"]); + assert_eq!(transition.outcome(), "unprovisioned"); + assert_eq!(entries_of(&dir), ["notes.txt"]); +} + +/// Tests of lock files that a Windows file system without POSIX deletion keeps under their name +/// until their last handle closes, which NTFS does for a file deleted without POSIX semantics. +#[cfg(windows)] +#[allow(unsafe_code)] +mod windows { + use std::os::windows::fs::OpenOptionsExt; + use std::os::windows::io::AsRawHandle; + + use windows_sys::Win32::Foundation::HANDLE; + use windows_sys::Win32::Storage::FileSystem::{ + DELETE, FILE_DISPOSITION_INFO, FILE_SHARE_DELETE, FILE_SHARE_READ, FILE_SHARE_WRITE, + FileDispositionInfo, SetFileInformationByHandle, + }; + + use super::*; + + /// Deletes the file `path` as a file system without POSIX deletion deletes a file that other + /// handles hold: its name stays, and opening it fails, until the last handle closes, of which + /// the returned one is the last unless others hold the file. + fn delete_pending(path: &Path) -> File { + let file = fs::OpenOptions::new() + .access_mode(DELETE) + .share_mode(FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE) + .open(path) + .unwrap(); + let disposition = FILE_DISPOSITION_INFO { DeleteFile: true }; + // SAFETY: the handle is open, and the information is a FILE_DISPOSITION_INFO of its size. + let set = unsafe { + SetFileInformationByHandle( + file.as_raw_handle() as HANDLE, + FileDispositionInfo, + (&raw const disposition).cast(), + size_of::() as u32, + ) + }; + assert_ne!(set, 0, "{}", io::Error::last_os_error()); + file + } + + #[test] + fn locks_wait_for_a_removed_lock_file_that_a_handle_keeps() { + let directory = tempfile::tempdir().unwrap(); + let dir = directory.path().join("sandbox"); + SandboxStore::create(&dir).unwrap(); + fs::write(dir.join(LOCK_NAME), b"").unwrap(); + let pending = delete_pending(&dir.join(LOCK_NAME)); + for error in [ + SandboxStore::lock_for_provision(&dir, &files(), Some(Instant::now())).unwrap_err(), + SandboxStore::lock_existing(&dir, &files(), Some(Instant::now())).unwrap_err(), + ] { + assert_eq!(error.kind(), ErrorKind::TimedOut, "{error}"); + } + let provision = thread::spawn({ + let dir = dir.clone(); + move || SandboxStore::lock_for_provision(&dir, &files(), within(30)).map(drop) + }); + thread::sleep(Duration::from_millis(100)); + drop(pending); + provision.join().unwrap().unwrap(); + assert!(entries_of(&dir).is_empty()); + } + + #[test] + fn locks_refuse_a_lock_file_that_refuses_access_for_good() { + // A read-only lock file, and a directory at its path, refuse an open for writing as a + // removed lock file does, but they open without access rights, so the locks fail at once, + // also without a deadline. + for read_only in [true, false] { + let directory = tempfile::tempdir().unwrap(); + let dir = directory.path().join("sandbox"); + SandboxStore::create(&dir).unwrap(); + let lock = dir.join(LOCK_NAME); + if read_only { + fs::write(&lock, b"").unwrap(); + set_read_only(&lock, true); + } else { + fs::create_dir(&lock).unwrap(); + } + let (sender, receiver) = mpsc::channel(); + thread::spawn({ + let dir = dir.clone(); + move || { + let provision = SandboxStore::lock_for_provision(&dir, &files(), None); + let existing = SandboxStore::lock_existing(&dir, &files(), None); + sender + .send([provision.map(drop), existing.map(drop)]) + .unwrap(); + } + }); + let taken = receiver + .recv_timeout(Duration::from_secs(10)) + .expect("a lock waited for a lock file that refuses access for good"); + for result in taken { + let error = result.unwrap_err(); + assert!(denied(&error), "{error}"); + } + if read_only { + set_read_only(&lock, false); + } + } + } + + /// Sets or clears the read-only attribute of the file `path`. + fn set_read_only(path: &Path, read_only: bool) { + let mut permissions = fs::metadata(path).unwrap().permissions(); + permissions.set_readonly(read_only); + fs::set_permissions(path, permissions).unwrap(); + } + + #[test] + fn a_transition_whose_lock_file_lingers_waits_until_it_goes() { + let (_directory, dir) = sandbox(); + let held = SandboxStore::lock_existing(&dir, &files(), None).unwrap(); + let (ready, waiting) = mpsc::channel(); + let waiter = thread::spawn({ + let dir = dir.clone(); + move || { + ready.send(()).unwrap(); + transition(&dir, within(30)) + } + }); + waiting.recv().unwrap(); + thread::sleep(Duration::from_millis(100)); + // The holder removes the sandbox, on a file system that keeps the lock file while a + // handle holds it. The waiter takes its lock, finds it removed, and cannot open the lock + // file at the path again until it goes. + fs::remove_file(dir.join(CONFIG)).unwrap(); + let pending = delete_pending(&dir.join(LOCK_NAME)); + drop(held); + thread::sleep(Duration::from_millis(100)); + assert!(!waiter.is_finished()); + drop(pending); + assert!(!waiter.join().unwrap().unwrap()); + assert!(entries_of(&dir).is_empty()); + } + + #[test] + fn transitions_in_other_processes_wait_for_a_lingering_lock_file() { + let (_directory, dir) = sandbox(); + fs::remove_file(dir.join(CONFIG)).unwrap(); + for (lock, left) in [ + ("existing", &[][..]), + ("provision", &[CONFIG, LOCK_NAME][..]), + ] { + fs::write(dir.join(LOCK_NAME), b"").unwrap(); + let pending = delete_pending(&dir.join(LOCK_NAME)); + let mut transition = Transition::start(&dir, lock); + thread::sleep(Duration::from_millis(100)); + assert!(transition.runs(), "{lock}"); + drop(pending); + assert_eq!(transition.outcome(), "unprovisioned", "{lock}"); + assert_eq!(entries_of(&dir), sorted(left), "{lock}"); + } + } +} diff --git a/nvx-rs/src/host/store/mod.rs b/nvx-rs/src/host/store/mod.rs index f7d051762..cc4f12703 100644 --- a/nvx-rs/src/host/store/mod.rs +++ b/nvx-rs/src/host/store/mod.rs @@ -12,6 +12,8 @@ //! openvmm.log the VMM's output, claimed by the launch that created it //! console.log the guest console of the latest launch, if the caller captures it //! outcome.json the VMM's outcome report of the latest launch +//! .tmpXXXXXX the outcome report while the VMM writes it, which it then publishes +//! ... the caller's configuration record and other files, which it declares //! ``` //! //! The directory is private to the current user, since its records decide which process @@ -32,7 +34,8 @@ //! names, which a launch may replace while the lock is held. //! Records are replaced atomically, so readers that hold no lock, such as an execution, see whole //! records. The store holds runtime records only: configuration belongs to its caller, which may -//! keep it in the same directory under a name of its own. +//! keep it in the same directory under a name of its own, and declares it, with its other files +//! there, as [`CallerFiles`]. //! //! # Records //! @@ -147,9 +150,69 @@ //! The runtime record of a committed launch names its running VM, and //! [`supervisor::reattach`](super::supervisor::reattach) holds that VM again after its //! orchestrator restarts. - +//! +//! # The report staging file +//! +//! The VMM writes its outcome report to a file of its own in the directory, `.tmp` and six ASCII +//! letters or digits, as the `tempfile` crate names it, and then publishes it as `outcome.json`. +//! A VMM that is killed while it writes the report leaves that file. The store owns files of +//! exactly that form, and removes them wherever it clears a launch's files once no process of +//! the launch runs: when [`LockedStore::recover`] or [`Launch::abandon`] ends a launch, unless +//! that is [`Recovery::Indeterminate`], in [`LockedStore::clear_runtime`], when +//! [`LockedStore::begin_launch`] begins the next launch, and in a [removal](#removal). It keeps +//! them while a process of the launch may still run. A file of any other name is not the +//! store's. +//! +//! # Lifecycle locks +//! +//! A caller that keeps a configuration record in the sandbox's directory, whose presence means +//! that the sandbox is provisioned, declares it, with any other files of its own there, as +//! [`CallerFiles`], and takes the lock of each lifecycle transition with them. Provisions, other +//! transitions, and removals of the sandbox then keep its directory consistent, from any process: +//! +//! - [`SandboxStore::lock_for_provision`] creates the directory if needed, and again if a +//! concurrent removal deletes it before the lock is taken, even while it is created. +//! - [`SandboxStore::lock_existing`] fails with [`ErrorKind::NotFound`] once the directory is gone, +//! also when a concurrent removal deletes it while the lock is awaited. The caller checks its +//! configuration record under the lock: a directory that was never provisioned, that an +//! interrupted removal emptied, or whose removal open files still keep, holds none. +//! - A holder that leaves the directory without the configuration record removes its own lock +//! file as it releases the lock, unless it removed it already, so that such a directory keeps +//! no lock file. +//! +//! Both locks wait for another holder until the caller's deadline, trying again every 25 ms, and +//! a deadline that has passed still tries once, so that a caller that waits on its own, as for an +//! interruption, can retry them itself. A lock file that another transition removes or replaces +//! after an attempt opened it is locked again at once, and after a pause, until the deadline, when +//! that happens again right after it, so that transitions that keep replacing the lock file keep +//! no caller past its deadline. +//! +//! # Removal +//! +//! [`LockedStore::remove`] removes a sandbox that no VMM can run in: it refuses while the store +//! records a VM or a launch, which the caller reattaches to, or recovers, first. It removes the +//! store's files and the caller's, the configuration record last, and the temporary files that the +//! store owns: those that an interrupted replacement of one of its files leaves, +//! `.NAME.NUMBER.tmp`, and the VMM's [report staging file](#the-report-staging-file). It then +//! refuses entries that are neither the store's nor the caller's, naming them, and keeps the +//! directory, or removes its lock file while it still holds the lock, and then the directory. A +//! removal needs the caller's configuration record, whose absence tells the transitions that wait +//! through it that the sandbox is gone: a lock taken without one refuses it. +//! +//! Files that are still open can keep the emptied directory for a while: NFS renames a removed +//! file that is still open, such as the lock file of a waiting transition, to `.nfs*`, Windows +//! file systems without POSIX deletion keep a removed lock file under its name until its last +//! handle closes, and a transition that opens the lock file after the removal removed it creates +//! a new one. Each such transition finds no sandbox, and removes its lock file as it releases its +//! lock. The removal leaves those files, then holds the directory's identity, releases its lock, +//! and returns a [`Lingering`] removal, which waits until they go, and reports a provision that +//! took the lock meanwhile as [`RemovalError::ProvisionedAgain`]. A removal removes the directory +//! only while the path still names it, which it checks before each attempt. + +mod caller; mod files; mod identity; +mod lifecycle; mod record; #[cfg(test)] mod tests; @@ -162,10 +225,13 @@ use std::path::{Path, PathBuf}; use std::thread; use std::time::{Duration, Instant}; +pub use self::caller::CallerFiles; +use self::caller::is_report_staging; use self::files::{io_error, remove_if_present}; pub use self::identity::{ ArtifactIdentity, IdentityCheck, IdentityPart, MAX_IDENTITY_LEN, RuntimeIdentity, }; +pub use self::lifecycle::{Lingering, RemovalError}; pub use self::record::{ LaunchId, LaunchJournal, LaunchState, LogClaim, MAX_ENDPOINT_LEN, RuntimeRecord, }; @@ -173,7 +239,7 @@ use super::filesystem::FileIdentity; use super::platform; use super::process::{BootId, Process, ProcessRef}; use super::supervisor::Launched; -use crate::error::{Error, Result}; +use crate::error::{Error, ErrorKind, Result}; use crate::protocol::CAPABILITY_LEN; /// Version of the store's records. @@ -199,7 +265,9 @@ pub const CONSOLE_NAME: &str = "console.log"; /// Name of the VMM's outcome report. pub const OUTCOME_NAME: &str = "outcome.json"; /// Every file that the store keeps in a sandbox's directory. Replacing a record leaves a -/// temporary file named `.NAME.NUMBER.tmp` beside it only if the replacement was interrupted. +/// temporary file named `.NAME.NUMBER.tmp` beside it only if the replacement was interrupted, and +/// a VMM that is killed while it writes its outcome report leaves the file in which it stages it; +/// see [the report staging file](self#the-report-staging-file). pub const FILE_NAMES: [&str; 8] = [ LOCK_NAME, JOURNAL_NAME, @@ -238,6 +306,72 @@ fn absolute(dir: PathBuf) -> Result { ))) } +/// Builds an error mapper that keeps an I/O source under `context`, as an error of +/// [`ErrorKind::NotFound`] if the source says that a path does not exist, and of +/// [`ErrorKind::Io`] otherwise. +fn store_error(context: String) -> impl FnOnce(io::Error) -> Error { + move |error| { + let kind = match error.kind() { + io::ErrorKind::NotFound => ErrorKind::NotFound, + _ => ErrorKind::Io, + }; + Error::new(kind, context).with_source(error) + } +} + +/// Returns the error of a wait for another lifecycle transition of the sandbox in `dir` that is +/// still in progress past its deadline. +fn transition_in_progress(dir: &Path) -> Error { + Error::timed_out(format!( + "another lifecycle transition of the sandbox in {} is still in progress", + dir.display() + )) +} + +/// Waits before the next try of a step that `deadline` bounds: returns `false` at once if the +/// deadline has passed, and otherwise sleeps for [`LOCK_RETRY_INTERVAL`] or the time left, +/// whichever is shorter, and returns `true`. +fn pause(deadline: Option) -> bool { + let left = deadline.map(|deadline| deadline.saturating_duration_since(Instant::now())); + if left.is_some_and(|left| left.is_zero()) { + return false; + } + thread::sleep(left.map_or(LOCK_RETRY_INTERVAL, |left| left.min(LOCK_RETRY_INTERVAL))); + true +} + +/// The races that the attempts to take a lock lose to other transitions, which remove or replace +/// the lock file, or the directory, after an attempt opens it. The attempt after a lost race +/// follows at once, and one after another race lost right after it pauses first, until the +/// deadline, so that transitions that keep replacing the lock file can neither keep a caller past +/// its deadline nor make it spin. +#[derive(Default)] +struct Races { + /// Whether the latest attempt followed at once one that lost a race. + retried_at_once: bool, +} + +impl Races { + /// Records that an attempt lost a race, and returns whether the lock may be tried again + /// before `deadline`, which it pauses for first if the attempt followed one that lost too. + fn lost(&mut self, deadline: Option) -> bool { + if self.retried_at_once && !pause(deadline) { + return false; + } + self.retried_at_once = !self.retried_at_once; + true + } +} + +/// How an attempt to take a store's lock ended, if it did not fail. +enum Taken { + /// The lock of the lock file that the path names. + Locked(File), + /// The lock of a lock file that was removed or replaced after it was opened, which locks + /// nothing. + Replaced, +} + /// The store of one sandbox: its private directory. #[derive(Debug, Clone, PartialEq, Eq)] pub struct SandboxStore { @@ -255,9 +389,10 @@ impl SandboxStore { /// /// A relative `dir` is resolved against the current directory, so that the store's paths, and /// the control endpoint that a launch records, name the same files from any working directory. + /// Fails with [`ErrorKind::NotFound`] if the parent does not exist. pub fn create(dir: impl Into) -> Result { let dir = absolute(dir.into())?; - let problem = platform::create_store_dir(&dir).map_err(io_error(format!( + let problem = platform::create_store_dir(&dir).map_err(store_error(format!( "cannot create the sandbox store {}", dir.display() )))?; @@ -266,10 +401,11 @@ impl SandboxStore { /// Opens the store in the existing directory `dir`, which a relative path names from the /// current directory, and which must be private to the current user, as - /// [`create`](Self::create) makes it. + /// [`create`](Self::create) makes it. Fails with [`ErrorKind::NotFound`] if `dir` does not + /// exist. pub fn open(dir: impl Into) -> Result { let dir = absolute(dir.into())?; - let problem = platform::check_store_dir(&dir).map_err(io_error(format!( + let problem = platform::check_store_dir(&dir).map_err(store_error(format!( "cannot open the sandbox store {}", dir.display() )))?; @@ -357,56 +493,93 @@ impl SandboxStore { /// Takes the sandbox's lock, waiting for it until `deadline`, or for as long as it takes /// without one. /// - /// Fails with [`ErrorKind::TimedOut`](crate::ErrorKind::TimedOut) when the deadline passes - /// first. A lock file that is removed or replaced while this waits locks nothing, so the - /// lock is then taken again. A lock file that is not private to the current user is refused, - /// as a record is, since others could hold its lock, and one that is a link, or not a regular - /// file, is refused at once. + /// Fails with [`ErrorKind::TimedOut`] when the deadline passes first, and with + /// [`ErrorKind::NotFound`] if the directory does not exist. A lock file that is removed or + /// replaced while this waits locks nothing, so the lock is then taken again, at once the first + /// time, and after a pause, until the deadline, when that happens again right after it. A lock + /// file that is not private to the current user is refused, as a record is, since others could + /// hold its lock, and one that is a link, or not a regular file, is refused at once. + /// + /// This lock knows nothing of its caller's files. A caller that keeps a configuration record + /// in the directory, or removes the store, takes the [lifecycle locks](self#lifecycle-locks) + /// instead. pub fn lock(&self, deadline: Option) -> Result { - let path = self.path(LOCK_NAME); - let failed = |error| io_error(format!("cannot lock {}", path.display()))(error); + let mut races = Races::default(); loop { - let file = files::open_lock(&path).map_err(failed)?; - if let Some(problem) = platform::private_file_problem(&file).map_err(failed)? { - return Err(Error::io(format!( - "the sandbox lock {} {problem}", - path.display() - ))); + if let Taken::Locked(file) = self.take_lock(deadline)? { + return Ok(LockedStore::new(self.clone(), file, CallerFiles::none())); + } + if !races.lost(deadline) { + return Err(transition_in_progress(&self.dir)); } - match deadline { - None => file.lock().map_err(failed)?, - Some(deadline) => loop { - match file.try_lock() { - Ok(()) => break, - Err(TryLockError::WouldBlock) => { - let left = deadline.saturating_duration_since(Instant::now()); - if left.is_zero() { - return Err(Error::timed_out(format!( - "another lifecycle transition of the sandbox in {} is still \ - in progress", - self.dir.display() - ))); - } - thread::sleep(left.min(LOCK_RETRY_INTERVAL)); + } + } + + /// Opens the lock file, which it creates if needed, takes its lock, waiting for it until + /// `deadline`, or for as long as it takes without one, and checks that the path still names + /// the locked file. + /// + /// Fails as [`lock`](Self::lock) does. On Windows, a file system without POSIX deletion keeps + /// a removed file under its name until its last handle closes, and refuses to open it until + /// then: the open then fails with an I/O error whose cause is the denied access, while a + /// removed lock file that the lock's own handle keeps is taken for a replaced one. + fn take_lock(&self, deadline: Option) -> Result { + let path = self.path(LOCK_NAME); + let failed = |error| store_error(format!("cannot lock {}", path.display()))(error); + let file = files::open_lock(&path).map_err(failed)?; + if let Some(problem) = platform::private_file_problem(&file).map_err(failed)? { + return Err(Error::io(format!( + "the sandbox lock {} {problem}", + path.display() + ))); + } + match deadline { + None => file.lock().map_err(failed)?, + Some(deadline) => loop { + match file.try_lock() { + Ok(()) => break, + Err(TryLockError::WouldBlock) => { + let left = deadline.saturating_duration_since(Instant::now()); + if left.is_zero() { + return Err(transition_in_progress(&self.dir)); } - Err(TryLockError::Error(error)) => return Err(failed(error)), + thread::sleep(left.min(LOCK_RETRY_INTERVAL)); } - }, - } - let locked = platform::opened_file_identity(&file).map_err(failed)?; - match platform::file_identity(&path) { - Ok(current) if current == locked => { - return Ok(LockedStore { - store: self.clone(), - _lock: file, - }); + Err(TryLockError::Error(error)) => return Err(failed(error)), } - Ok(_) => {} - Err(error) if error.kind() == io::ErrorKind::NotFound => {} - Err(error) => return Err(failed(error)), + }, + } + let locked = platform::opened_file_identity(&file).map_err(failed)?; + match platform::file_identity(&path) { + Ok(current) if current == locked => Ok(Taken::Locked(file)), + Ok(_) => Ok(Taken::Replaced), + Err(error) if error.kind() == io::ErrorKind::NotFound => Ok(Taken::Replaced), + Err(error) if cfg!(windows) && error.kind() == io::ErrorKind::PermissionDenied => { + Ok(Taken::Replaced) } + Err(error) => Err(failed(error)), } } + + /// Removes the files in which a VMM of the sandbox staged an outcome report that it never + /// published, which only a VMM that was killed while it wrote the report leaves. The caller + /// knows that no process of the sandbox's launches runs. Fails with [`ErrorKind::NotFound`] + /// if the directory is gone, as [`open`](Self::open) does. + fn remove_report_staging(&self) -> Result<()> { + let failed = |error| { + store_error(format!( + "cannot list the sandbox store {}", + self.dir.display() + ))(error) + }; + for entry in std::fs::read_dir(&self.dir).map_err(failed)? { + let name = entry.map_err(failed)?.file_name(); + if name.to_str().is_some_and(is_report_staging) { + remove_if_present(&self.dir.join(name))?; + } + } + Ok(()) + } } /// The store of a sandbox whose lock this holds, through which its lifecycle transitions change @@ -414,13 +587,19 @@ impl SandboxStore { /// /// Every transition borrows it exclusively, and a [`Launch`] keeps it borrowed until the launch /// commits or is abandoned, so transitions never overlap, not even through one guard that -/// threads share. +/// threads share. A lock that a [lifecycle lock](self#lifecycle-locks) took also knows its +/// caller's files: as it is released, it removes its lock file if the caller's configuration +/// record is missing, and it can [remove](Self::remove) the store. #[derive(Debug)] pub struct LockedStore { /// The sandbox's store. store: SandboxStore, /// The locked file, whose lock lasts until it closes. - _lock: File, + lock: File, + /// The caller's files in the directory, which a store's own lock does not know. + files: CallerFiles, + /// Whether this holder removed its lock file, after which the path may name another's. + lock_removed: bool, } impl Deref for LockedStore { @@ -432,15 +611,33 @@ impl Deref for LockedStore { } impl LockedStore { + /// Returns the held lock of `store`, on its locked lock file `lock`, whose caller keeps + /// `files`. + fn new(store: SandboxStore, lock: File, files: CallerFiles) -> Self { + Self { + store, + lock, + files, + lock_removed: false, + } + } + + /// Returns the caller's files that the lock knows: those that a [lifecycle + /// lock](self#lifecycle-locks) was taken with, and none for [`SandboxStore::lock`]. + pub fn files(&self) -> &CallerFiles { + &self.files + } + /// Begins a launch of the runtime `runtime`, whose VMM receives `capability`. /// /// It stores the capability, replaces the log of the previous launch with a fresh one, which /// it claims, removes the outcome report, console log, and control socket of the previous - /// launch, chooses a control endpoint, and writes the journal's marker. Nothing is spawned - /// yet: the caller spawns the VMM with [`Launch::take_log`]. + /// launch, and any outcome report that its VMM staged without publishing it, chooses a control + /// endpoint, and writes the journal's marker. Nothing is spawned yet: the caller spawns the + /// VMM with [`Launch::take_log`]. /// /// Fails if the sandbox's VM runs, or a launch was interrupted and is not recovered yet, and - /// with [`ErrorKind::Unsupported`](crate::ErrorKind::Unsupported) for an all-zero capability + /// with [`ErrorKind::Unsupported`] for an all-zero capability /// or an invalid runtime identity. pub fn begin_launch( &mut self, @@ -469,6 +666,7 @@ impl LockedStore { for path in [&socket, &self.outcome_path(), &self.console_path()] { remove_if_present(path)?; } + self.remove_report_staging()?; let endpoint = platform::control_endpoint(&socket) .map_err(|error| Error::io("cannot choose a control endpoint").with_source(error))?; let (log, claim) = self.create_log()?; @@ -505,9 +703,9 @@ impl LockedStore { /// Recovery signals only a VMM that it holds through a verified [`Process`]: the one that the /// journal's [`ProcessRef`] names, or the one that serves the journal's control endpoint, as /// the [module documentation](self#recovery) describes. Once nothing of the launch runs, it - /// removes the launch's capability, control socket, and journal, and keeps its log. A - /// recovery that cannot prove that is [`Recovery::Indeterminate`] and keeps them all for a - /// later recovery. + /// removes the launch's capability, control socket, outcome report that its VMM staged + /// without publishing it, and journal, and keeps its log. A recovery that cannot prove that is + /// [`Recovery::Indeterminate`] and keeps them all for a later recovery. /// /// A journal that a committed runtime record of the same launch supersedes is removed. A /// runtime record of another launch fails as inconsistent state. @@ -538,11 +736,13 @@ impl LockedStore { None => self.end_unidentified(journal), }; if !matches!(recovery, Recovery::Indeterminate(_)) { - // The journal goes last, so that an interrupted removal leaves it for a later - // recovery to finish. - for name in [CAPABILITY_NAME, SOCKET_NAME, JOURNAL_NAME] { + for name in [CAPABILITY_NAME, SOCKET_NAME] { remove_if_present(&self.path(name))?; } + self.remove_report_staging()?; + // The journal goes last, so that an interrupted removal leaves it for a later + // recovery to finish. + remove_if_present(&self.path(JOURNAL_NAME))?; } Ok(recovery) } @@ -646,12 +846,14 @@ impl LockedStore { } /// Removes the runtime record, the launch capability, and the control socket of a VM that has - /// exited, the runtime record last. The log, console log, and outcome report outlive the run. + /// exited, and any outcome report that its VMM staged without publishing it, the runtime + /// record last. The log, console log, and outcome report outlive the run. pub fn clear_runtime(&mut self) -> Result<()> { - for name in [CAPABILITY_NAME, SOCKET_NAME, RUNTIME_NAME] { + for name in [CAPABILITY_NAME, SOCKET_NAME] { remove_if_present(&self.path(name))?; } - Ok(()) + self.remove_report_staging()?; + remove_if_present(&self.path(RUNTIME_NAME)) } } diff --git a/nvx-rs/src/host/store/tests.rs b/nvx-rs/src/host/store/tests.rs index 4ce0fb309..9459c2343 100644 --- a/nvx-rs/src/host/store/tests.rs +++ b/nvx-rs/src/host/store/tests.rs @@ -324,7 +324,7 @@ fn a_lock_file_replaced_while_waiting_locks_nothing() { let locked = store.lock(None).unwrap(); // The waiter holds the lock of the file that the path names now. let current = platform::file_identity(&locked.path(LOCK_NAME)).unwrap(); - let held = platform::opened_file_identity(&locked._lock).unwrap(); + let held = platform::opened_file_identity(&locked.lock).unwrap(); assert_eq!(current, held); } }); @@ -634,7 +634,7 @@ fn stores_open_only_existing_directories() { let (directory, store) = store(); assert_eq!(SandboxStore::open(store.dir()).unwrap(), store); let missing = SandboxStore::open(directory.path().join("missing")).unwrap_err(); - assert_eq!(missing.kind(), ErrorKind::Io); + assert_eq!(missing.kind(), ErrorKind::NotFound); let file = directory.path().join("file"); fs::write(&file, b"").unwrap(); assert!( @@ -828,6 +828,67 @@ fn a_journal_that_its_runtime_record_supersedes_is_removed() { drop(locked.begin_launch(&runtime(), &CAPABILITY).unwrap()); } +#[test] +fn report_staging_files_go_once_nothing_of_their_launch_runs() { + let (_directory, store) = store(); + // The file in which a VMM that was killed staged its outcome report, and other names, which + // are not the store's. + let staging = store.path(".tmpAbC123"); + let others = [".tmpAbC1234", ".tmpAb-123", ".tmp"]; + for name in others { + fs::write(store.path(name), b"{").unwrap(); + } + fs::write(&staging, b"{").unwrap(); + let mut locked = store.lock(None).unwrap(); + // A launch begins without the staging file of an earlier one. + let mut launch = locked.begin_launch(&runtime(), &CAPABILITY).unwrap(); + assert!(!staging.exists()); + // While a process of the launch may still hold its log, as its VMM does, the file stays. + let log = launch.take_log().unwrap(); + drop(launch); + fs::write(&staging, b"{").unwrap(); + let recovery = locked.recover().unwrap(); + assert!( + matches!(recovery, Recovery::Indeterminate(_)), + "{recovery:?}" + ); + assert!(staging.exists()); + // Once nothing holds the log, recovery ends the launch and removes the file. + drop(log); + assert!(released(|| locked.recover().unwrap() == Recovery::Exited)); + assert!(!staging.exists()); + // An abandoned launch removes it, and so does clearing the runtime of a VM that exited. + let launch = locked.begin_launch(&runtime(), &CAPABILITY).unwrap(); + fs::write(&staging, b"{").unwrap(); + match launch.abandon().unwrap() { + Recovery::Exited => {} + // A process that a concurrent test spawned may still hold the log that the launch closed, + // until it executes its program, and the next recovery then ends the launch. + Recovery::Indeterminate(_) => { + assert!(staging.exists()); + assert!(released(|| locked.recover().unwrap() == Recovery::Exited)); + } + other => panic!("{other:?}"), + } + assert!(!staging.exists()); + fs::write(&staging, b"{").unwrap(); + locked.clear_runtime().unwrap(); + assert!(!staging.exists()); + for name in others { + assert!(store.path(name).exists(), "{name}"); + } +} + +#[test] +fn a_store_whose_directory_went_reports_it_missing_as_it_clears_a_runtime() { + let (_directory, store) = store(); + let mut locked = store.lock(None).unwrap(); + // Something other than a removal deletes the directory while the lock is held. + fs::remove_dir_all(store.dir()).unwrap(); + let error = locked.clear_runtime().unwrap_err(); + assert_eq!(error.kind(), ErrorKind::NotFound, "{error}"); +} + #[test] fn launch_identities_are_fresh_hexadecimal() { let first = LaunchId::generate().unwrap(); diff --git a/nvx-rs/src/lib.rs b/nvx-rs/src/lib.rs index a71154ebd..ff820dcfd 100644 --- a/nvx-rs/src/lib.rs +++ b/nvx-rs/src/lib.rs @@ -49,6 +49,7 @@ //! //! use nvx::host::instance::{Found, Instance}; //! use nvx::host::launch::LaunchSpec; +//! use nvx::host::store::CallerFiles; //! use nvx::protocol::{Workload, WorkloadEnvironment}; //! use nvx::{Artifacts, Hypervisor}; //! @@ -57,7 +58,9 @@ //! let artifacts = Artifacts::discover()?; //! let runtime = artifacts.runtime_identity()?; //! let spec = LaunchSpec::new(artifacts, Hypervisor::from_env_or_default()?); -//! let instance = Instance::create(std::env::temp_dir().join("nvx-example"), "example")?; +//! // The example keeps no files of its own beside the instance's store. +//! let directory = std::env::temp_dir().join("nvx-example"); +//! let instance = Instance::create(directory, "example", CallerFiles::none())?; //! // Lifecycle transitions hold the instance's lock; executions need none. //! let mut locked = instance.lock(None)?; //! let Found::Stopped = locked.find(|| Ok(runtime.clone()))? else { diff --git a/nvx-rs/tests/fake_openvmm.rs b/nvx-rs/tests/fake_openvmm.rs index 365539324..b19dcb575 100644 --- a/nvx-rs/tests/fake_openvmm.rs +++ b/nvx-rs/tests/fake_openvmm.rs @@ -20,7 +20,7 @@ use std::time::Duration; use nvx::host::instance::{Found, Instance, Running, Started, Stopped, WorkloadOutcome}; use nvx::host::launch::{LaunchSpec, PROCESS_COMMAND_LINE_LIMIT}; -use nvx::host::store::{ArtifactIdentity, IdentityPart, RuntimeIdentity}; +use nvx::host::store::{ArtifactIdentity, CallerFiles, IdentityPart, RuntimeIdentity}; use nvx::protocol::{Workload, WorkloadEnvironment}; use nvx::{Artifacts, ErrorKind, Hypervisor}; @@ -56,7 +56,7 @@ fn spec(directory: &Path) -> LaunchSpec { /// Creates the fake guest artifacts and an instance directory below `directory`. fn setup(directory: &Path) -> (LaunchSpec, Instance) { - let instance = Instance::create(directory.join("vm"), "test").unwrap(); + let instance = Instance::create(directory.join("vm"), "test", CallerFiles::none()).unwrap(); (spec(directory), instance) } @@ -360,7 +360,7 @@ fn orchestrator_role() { return; }; let spec = spec(&env::current_dir().unwrap()); - let instance = Instance::create(PathBuf::from(dir), "test").unwrap(); + let instance = Instance::create(PathBuf::from(dir), "test", CallerFiles::none()).unwrap(); start(&instance, &spec).unwrap(); } @@ -385,7 +385,7 @@ fn a_restarted_orchestrator_reattaches_to_its_vm() { // The orchestrator restarts within the same host boot: it finds the VM that its earlier run // started, through the records of the store, and drives it. - let instance = Instance::open(&dir, "test").unwrap(); + let instance = Instance::open(&dir, "test", CallerFiles::none()).unwrap(); let idle = AtomicBool::new(false); let output = run(&instance, &workload(&["/bin/echo", "reattached"]), &idle); assert_eq!(output.outcome, WorkloadOutcome::Exited(0)); diff --git a/scripts/nvx_tools/sandbox_lifecycle_tests.py b/scripts/nvx_tools/sandbox_lifecycle_tests.py index 08c8be37f..e276aec16 100644 --- a/scripts/nvx_tools/sandbox_lifecycle_tests.py +++ b/scripts/nvx_tools/sandbox_lifecycle_tests.py @@ -505,6 +505,10 @@ def check_identity_admission(self) -> None: ) self.copy_evidence(self.identity_state / LOG, "identity-openvmm.log") self.copy_evidence(self.identity_state / OUTCOME, "identity-outcome.json") + # OpenVMM publishes its report before it exits, rather than being ended + # while it writes it, which left only the report's staging file (#438). + if not (self.identity_state / OUTCOME).is_file(): + raise RuntimeError("the refused managed start published no outcome report") self.require_entries( self.identity_state, STOPPED_ENTRIES, "after a refused managed start" ) diff --git a/scripts/test_microvm_tests.py b/scripts/test_microvm_tests.py index c3dfe0fac..a600162a4 100644 --- a/scripts/test_microvm_tests.py +++ b/scripts/test_microvm_tests.py @@ -1261,6 +1261,10 @@ def _deprovision( "sandbox.lock", ): (state / name).unlink(missing_ok=True) + # The store owns the file in which OpenVMM stages its outcome report. + for path in state.iterdir(): + if re.fullmatch(r"\.tmp[A-Za-z0-9]{6}", path.name): + path.unlink(missing_ok=True) unknown = sorted(path.name for path in state.iterdir()) if unknown and "deprovision-removes-foreign" not in self.faults: return self._error( @@ -1454,6 +1458,11 @@ def test_acceptance_detects_lifecycle_regressions(self): "start returned 1, expected 1 with 'error: OpenVMM exited during " "startup: guest-exit status 125'" ), + # OpenVMM ended while it published its report (#438), which left + # only the report's staging file. + "failed-start-kills-reporter": ( + "the refused managed start published no outcome report" + ), "duplicate-start": "start returned 0, expected 1", "exec-loses-state": "the state-reading request returned status 1", "probe-fails": "exec returned 1, expected 0", @@ -1495,28 +1504,6 @@ def test_acceptance_detects_lifecycle_regressions(self): (root / "results" / "sandbox-lifecycle-checks.json").is_file() ) - def test_acceptance_detects_a_leaked_report_staging_file(self): - # A start that ends OpenVMM while it publishes its outcome report leaves - # the report's staging file, which deprovision refuses to remove, so the - # acceptance preserves its fixture, as on the WHP runners in issue #438. - with tempfile.TemporaryDirectory() as temporary: - root = Path(temporary) - fake = _FakeSandboxCli("failed-start-kills-reporter") - with self.assertRaises(RuntimeError) as raised: - self._run(root, fake) - fixtures = list(root.glob("nvx-public-lifecycle-*")) - - message = str(raised.exception) - for fragment in ( - "after a refused managed start: the state directory holds " - "['.tmpAbC123', 'config.json', 'openvmm.log', 'sandbox.lock']", - "sandbox state directory contains files not owned by NVX: .tmpAbC123", - "fixture preserved for recovery", - ): - self.assertIn(fragment, message) - self.assertEqual(len(fixtures), 1) - self.assertFalse(any(fake.running.values())) - def test_evidence_keeps_the_end_of_long_logs(self): with tempfile.TemporaryDirectory() as temporary: output_dir = self._run(Path(temporary), _FakeSandboxCli("noisy-log"))