From 9ad7257333e4291241dc1753c7ef585276d9d97d Mon Sep 17 00:00:00 2001 From: Pedro Henrique Penna Date: Thu, 12 Nov 2026 05:08:40 -0800 Subject: [PATCH 1/2] host: speak NVXC v2, and stream execs in the CLI NVX's host spoke NVXC v1, which buffers an execution's input and caps its output, and asked the guest for FEATURES. SL4.4c (#1128) moves every host path onto v2, which SL4.4a's codec and SL4.4b's servers provide. The host's control session speaks v2 alone: every READY must carry the protocol version, and no host path sends FEATURES. PING, MAPS, capture and the restore driver's activate exchange use v2's frames, with their payloads and order unchanged. The driver ends the restored workload's standard input after its Start, and returns the live v2 session. The session streams an execution: send_stdin keeps at most the 64 KiB window unacknowledged, close_stdin ends the input, the output arrives without the old total cap, and a STOP ends an execution that runs, skipping its frames until STOPPED. `sandbox exec` and `sandbox run` stream through the session: the workload reads the command's standard input until its end, and its output reaches the command's as it arrives. Ctrl-C sends CANCEL, so that the workload does not outlive the session, and a guest's diagnostic of a refused request now precedes the CLI's error. The CLI delegates PING, MAPS and STOP to the host session, with Python's messages, and a refusal keeps the session open, so that a refused mapping still lets the start stop the guest. SL6.12's dead-code expectations on ControlSession::{ping, start_exec, cancel_exec, stop} go with their seams, since the CLI calls them. Version 1's frames, which only the servers use now, lose the cli gate. The host's FEATURES request and REQUIRED_FEATURES go with their only caller, while GuestFeatures and ExitCategory::parse stay, marked dead, for SL4.4d to delete with the rest of version 1. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../4.4-host-guest-control-interface.md | 29 +- doc/run.md | 16 +- doc/usage.md | 11 +- nvx-rs/README.md | 73 ++- .../managed_agent/tests/loopback/tests.rs | 134 +++- nvx-rs/src/host/cli/sandbox/lifecycle/mod.rs | 5 - .../src/host/cli/sandbox/lifecycle/start.rs | 47 +- .../sandbox/lifecycle/start/tests/restore.rs | 100 +-- nvx-rs/src/host/cli/sandbox/mod.rs | 12 +- nvx-rs/src/host/cli/session.rs | 394 +++++------ nvx-rs/src/host/cli/session/tests.rs | 415 ++++++++---- .../src/host/cli/snapshots/capture/tests.rs | 22 +- nvx-rs/src/host/restore/tests.rs | 9 +- nvx-rs/src/host/restore_driver.rs | 13 +- nvx-rs/src/host/restore_driver/tests.rs | 20 +- nvx-rs/src/host/session.rs | 361 +++++------ nvx-rs/src/host/session/tests.rs | 610 ++++++++++++------ .../host/session/tests/managed_lifecycle.rs | 66 +- nvx-rs/src/protocol/app.rs | 43 +- nvx-rs/src/protocol/exec.rs | 4 + nvx-rs/src/protocol/features.rs | 27 +- nvx-rs/src/protocol/mod.rs | 25 +- nvx-rs/src/protocol/tests.rs | 12 +- nvx-rs/tests/cli.rs | 133 ++-- nvx-rs/tests/cli/harness.rs | 30 + nvx-rs/tests/cli/lifecycle.rs | 73 ++- nvx-rs/tests/cli/sandbox_run.rs | 71 +- scripts/test-layout-allowlist.json | 4 +- 28 files changed, 1747 insertions(+), 1012 deletions(-) diff --git a/doc/design/4.4-host-guest-control-interface.md b/doc/design/4.4-host-guest-control-interface.md index 2fab67599..1e64cb18b 100644 --- a/doc/design/4.4-host-guest-control-interface.md +++ b/doc/design/4.4-host-guest-control-interface.md @@ -40,8 +40,12 @@ and optionally an absolute working directory of at most 4,096 bytes and up to 256 `KEY=VALUE` environment entries of at most 4,096 bytes each, with distinct, non-empty keys. The environment either replaces the default environment or is layered over it, each entry replacing the default variable -of the same name. Output returns as separate standard output and standard -error streams within the credit window, at most 1 MiB of them combined for +of the same name. In version 2 of the protocol, which NVX's host speaks, the +host streams an execution's standard input, which the guest agents +acknowledge as they write it, with at most 64 KiB unacknowledged, until the +host ends it, and output returns as separate standard output and standard +error streams within the credit window, without a total limit. Version 1 +carries no standard input, and forwards at most 1 MiB of output combined for each execution. As Table 4.3 shows, an execution that runs ends with an exit report, which carries an outcome category and a status. @@ -53,7 +57,7 @@ report, which carries an outcome category and a status. | `signal` | A signal ended the workload; the status is 128 plus the signal number | | `timeout` | The workload exceeded its timeout and was killed; the status is 124 | | `cancelled` | A cancellation killed the workload; the status is 137 | -| `output-limit` | The workload's combined output exceeded 1 MiB, and the guest agents killed it; the status is 125 | +| `output-limit` | In version 1, the workload's combined output exceeded 1 MiB, and the guest agents killed it; the status is 125 | | `failed` | The guest agents could not supervise the workload; the status is 125 | Otherwise, the guest agents reply with an error, which carries one of the @@ -77,9 +81,15 @@ account for. ## 4.4.2 Feature Negotiation +Version 2 of the protocol negotiates nothing: every frame names its version, +the guest's `READY` names the version that its agents speak, and NVX's host +requires version 2, whose guest agents provide every behavior that the host +depends on. Version 1 negotiates features instead. + A successful readiness probe does not prove that an image enforces the -behaviors that a host depends on. As Table 4.5 shows, a `FEATURES` request -therefore returns a four-byte little-endian bit mask of control features. +behaviors that a host depends on. As Table 4.5 shows, a `FEATURES` request of +version 1 therefore returns a four-byte little-endian bit mask of control +features. **Table 4.5.** Control feature bits. @@ -110,10 +120,11 @@ Requests are typed, length-bounded, ordered, and associated with one VM instance and epoch. Credit limits bound queued data in both directions. Malformed, unauthenticated, stale, duplicated, or out-of-order records fail closed. Only one execution runs at a time; any request other than a -cancellation that arrives during an execution is refused as `busy`. Shutdown -is therefore graceful only between executions: a host cancels a running -execution before it requests shutdown, or else terminates the VM, which fails -the execution. +cancellation, or in version 2 the execution's standard input or a shutdown, +that arrives during an execution is refused as `busy`. In version 2, a +shutdown ends the running execution first. In version 1, shutdown is therefore +graceful only between executions: a host cancels a running execution before +it requests shutdown, or else terminates the VM, which fails the execution. The control session is process-local even when guest parser state is snapshotted. Restore establishes a fresh host capability, peer identity, diff --git a/doc/run.md b/doc/run.md index 0ba9e25f3..c7647d94e 100644 --- a/doc/run.md +++ b/doc/run.md @@ -908,11 +908,12 @@ VM directory, and prints its OpenVMM command as `>> COMMAND`. Once nvx-init is ready and has mapped the shares, the run executes its workload once over the control session, as [`sandbox exec`](#managed-lifecycle) does, with the same `--cwd`, `--environment`, `--environment-file`, -`--inherit-default-environment`, and `--exec-timeout-ms` options. It then asks +`--inherit-default-environment`, and `--exec-timeout-ms` options, and streams +its standard input, output and error as `sandbox exec` does. It then asks nvx-init to stop the VM, terminates OpenVMM if it does not exit within `--timeout` seconds, warning that it did, and removes the VM directory, and -finally prints the workload's standard output and standard error and exits -with its status. A VM that exits before nvx-init is ready, as when the guest +finally exits with the workload's status. A VM that exits before nvx-init is +ready, as when the guest refuses the workload identity, fails the run as it fails a `start`, with the category and status of the outcome report that OpenVMM published, such as `guest-exit status 125`. `--outcome-report` forwards that report, of the VM's @@ -1738,10 +1739,11 @@ rejects `--state-dir`. Environment files are limited to 1 MiB of UTF-8 JSON. Environments contain at most 256 unique, nonempty names. Each `KEY=VALUE` entry and working-directory path is limited to 4096 UTF-8 bytes; the combined execution request must also -fit the existing 64 KiB control-payload bound. A managed `exec` forwards at -most 1 MiB of combined standard output and standard error; the guest agent -kills a workload that writes more, and `exec` exits with status 125 and reports -the `output-limit` category. +fit the existing 64 KiB control-payload bound. A managed `exec` streams its +standard input to the workload, which sees it end when the command's own +input ends, and the workload's standard output and standard error back as +they arrive, without a limit on their size. Ctrl-C cancels the workload, and +`exec` exits with status 130. The `managed-exec-config` scenario of `test-microvm --backend BACKEND` is the authoritative acceptance for these public options. It invokes diff --git a/doc/usage.md b/doc/usage.md index 331feefe0..a5e0a6ca7 100644 --- a/doc/usage.md +++ b/doc/usage.md @@ -333,10 +333,13 @@ launches or is recorded; see [Sandbox networking](run.md#sandbox-networking). given. It exits with status 1 for an error, 2 for a usage error, and 130 after Ctrl-C, and otherwise with the status that the workload or OpenVMM returns. Its errors and usage errors carry their [error codes](#error-codes), as -`error[CODE]: MESSAGE` and `nvx-rs sandbox: error[CODE]: MESSAGE`; an `exec` -prints its workload's standard output and error before any error of its own, -which starts a new line. Options take their values as those of [`run`](#run) -do, and their names cannot be abbreviated either. +`error[CODE]: MESSAGE` and `nvx-rs sandbox: error[CODE]: MESSAGE`. An `exec`, +and the workload of a `run`, stream the command's standard input to the +workload until it ends, and the workload's standard output and error back as +they arrive, without a limit; a command whose input is the null device gives +the workload none. An error of the command's own follows the workload's output +on a new line, and Ctrl-C cancels the workload. Options take their values as +those of [`run`](#run) do, and their names cannot be abbreviated either. | Option | Default | Description | | --- | --- | --- | diff --git a/nvx-rs/README.md b/nvx-rs/README.md index 981d6f9f4..6d64115fe 100644 --- a/nvx-rs/README.md +++ b/nvx-rs/README.md @@ -151,9 +151,10 @@ frames, the `EXEC` payload that the host encodes and the guest agent decodes, and the control features that a guest image advertises. Beside version 1's frames, `protocol::app::v2` holds the codec of version 2's, which carry every control path, with version 1's payloads, and stream an execution's standard -input, output and error. The guest loop that nvx-init and the managed agent -share, and the fake OpenVMM, serve both versions, so `v2` follows their -features, `guest` and `testing`, until the host speaks it too. +input, output and error. NVX's host speaks version 2 alone, and the guest loop +that nvx-init and the managed agent share, and the fake OpenVMM, serve both +versions, so `v2` follows the features `cli`, `guest` and `testing`, and +version 1's frames those of their servers, `guest` and `testing`. The [conformance vectors](tests/vectors/README.md) of version 1, `tests/vectors/nvxc-v1.txt`, and OpenVMM's vectors and cases of the `NVXS` @@ -1050,7 +1051,11 @@ OpenVMM's outcome report. The Linux and Windows primitives are private to the crate, in `host::platform`, and so is the authenticated control session, which carries NVX's own control protocol, `NVXC`, one frame per DATA record of a `HostConnection`: the CLI and its restore driver use it, with the `cli` -feature. +feature. The session speaks version 2 alone, and requires the guest's `READY` +to name that version. It streams an execution's standard input, with an +explicit end, and keeps at most 64 KiB of it that the guest has not +acknowledged writing, and receives the execution's output without a total +limit; a `STOP` ends an execution that runs. `report::read` returns `Ok(None)` for a missing report, and an `Io` error for an unreadable or malformed report, retaining the underlying error. It keeps today's @@ -1905,16 +1910,17 @@ before them. as `nvx.py` quoted it with `subprocess.list2cmdline` and `shlex.join`. Unless `--dry-run` is given, it then executes the workload once through the control session, as `exec` does, with its working directory and environment - options, asks the guest to shut the VM down, waits up to `--timeout` seconds - for OpenVMM to exit before it terminates it, removes the VM directory, and - prints the workload's standard output and then its standard error, and exits - with its status. `--outcome-report` forwards OpenVMM's outcome report of the - VM's stop. + options, streaming its standard input, output and error, asks the guest to + shut the VM down, waits up to `--timeout` seconds for OpenVMM to exit before + it terminates it, removes the VM directory, and exits with the workload's + status. `--outcome-report` forwards OpenVMM's outcome report of the VM's + stop. - `provision` records the sandbox in its state directory. `start` launches OpenVMM detached with the managed lifecycle, passes it a fresh capability through its standard input, and waits until the guest answers. `exec` runs a - workload, prints its standard output and then its standard error, and exits - with its status; `--outcome-report` writes + workload, streams its own standard input to it, until that ends, and the + workload's standard output and error back as they arrive, and exits with the + workload's status; Ctrl-C cancels the workload. `--outcome-report` writes `{"operation_id": HEX, "outcome": {"category": CATEGORY, "operation": "exec", "status_code": STATUS}, "schema_version": 1}`. `stop` shuts the VM down and prints nothing, but for a warning when it terminates a VM of another @@ -2015,24 +2021,21 @@ OpenVMM that a start of such a build left behind when it was interrupted has no record, and that build asked for it to be ended by hand: end it before the switch too, since this build finds no VM in that sandbox. -#### Guest features - -After the guest answers, `start`, and `sandbox run`, ask it for its control -features and require `GuestFeatures::LAYERED_SANDBOX`, the features that their -launches use: per-execution environments (bit 4), since `exec` and -`sandbox run` send a working directory and an environment, and host path -mapping tables (bit 6), since they then send the mapping table of the -sandbox's shares, which the guest agent mounts before -it runs any workload; a mapping that it refuses fails the start, after the -guest shuts down as `stop` has it do. It needs neither cancellation (bit 0), -since the CLI never cancels a workload, and Ctrl-C ends the session, nor -workload account creation (bit 2), since a layered sandbox runs its workloads -as an account of the image. A guest image that lacks a required feature, or -that predates the request, is stopped as a failed start is, and the start -fails with -`the guest image lacks control features that layered sandboxes need (...)`, -which names the missing features. `nvx.py` does not ask; this, the checks of -the [distro binding](#distro-bindings) and of the +#### Guest protocol + +After the guest answers, `start`, and `sandbox run`, speak version 2 of `NVXC` +to it, whose `READY` names the version that the guest agent speaks, and require +that version: a guest image whose agent answers in another version, as one +that predates version 2 does, is stopped as a failed start is, and the start +fails. They ask for no control features, which only version 1's `FEATURES` +request reports, and which `start` required before: per-execution environments +and host path mapping tables, which every guest agent of version 2 provides. +`exec` and `sandbox run` send a working directory and an environment, and the +start then sends the mapping table of the sandbox's shares, which the guest +agent mounts before it runs any workload; a mapping that it refuses fails the +start, after the guest shuts down as `stop` has it do. `nvx.py` does not check +the version; this, the streams of `exec` and `sandbox run`, the checks of the +[distro binding](#distro-bindings) and of the [scratch binding](#images-by-path), and the claim of the scratch image are the intended changes of behavior. @@ -2110,8 +2113,11 @@ intended changes of behavior. launched exits, as `nvx::host::transport::connect_verified` does, and fails at once with `exited`, where `nvx.py` waited until `--timeout` passed before it reported the exit. -- `exec` refuses more than 1 MiB of workload output, which the guest agent - never forwards. +- `exec` and `sandbox run` stream the workload's standard input, output and + error while it runs, where `nvx.py` gave it no input and printed its output + once it ended. So a guest's diagnostic of a request that it refuses, such as + a working directory that the workload cannot enter, precedes the error. + Ctrl-C cancels the workload, where `nvx.py` closed the session. - `provision`, and `start` again, refuse a share whose resolved host path equals, contains, or lies inside the sandbox's state directory, image, or scratch image, or the OpenVMM, kernel, or initramfs of the artifact root, as @@ -2641,8 +2647,9 @@ tests, only the test hooks that The tests need no hypervisor. The fake-OpenVMM test runs `nvx-fake-openvmm` directly, as `nvx-rs run` runs OpenVMM on the terminal. The CLI test runs the -[scenario tests](#scenario-tests) of `nvx-rs`, and checks the guest-feature -requirement and, on Linux, Ctrl-C. The +[scenario tests](#scenario-tests) of `nvx-rs`, and checks that a start asks +the guest for no features, that `exec` and `sandbox run` stream, and, on +Linux, Ctrl-C, which also cancels an `exec`. The [conformance vectors](tests/vectors/README.md) of the wire formats cover both sides of every record, frame, and payload: the codec decodes each vector of `tests/vectors/nvxc-v1.txt` to its values and encodes them to its bytes, it diff --git a/nvx-rs/src/guest/managed_agent/tests/loopback/tests.rs b/nvx-rs/src/guest/managed_agent/tests/loopback/tests.rs index 28570ecbc..9667d2b28 100644 --- a/nvx-rs/src/guest/managed_agent/tests/loopback/tests.rs +++ b/nvx-rs/src/guest/managed_agent/tests/loopback/tests.rs @@ -2,8 +2,10 @@ use std::thread; use std::time::Instant; use crate::host::session::{ControlSession, ExecEvent, SessionError}; +use crate::protocol::v2::Outcome; use crate::protocol::{ - CAPABILITY_LEN, ExitCategory, GuestFeatures, HostMap, Workload, WorkloadEnvironment, + APP_EXEC, APP_EXIT, APP_STDERR, APP_STDOUT, CAPABILITY_LEN, HostMap, Workload, + WorkloadEnvironment, decode_app, encode_app, encode_exec_payload, }; use super::super::AgentConfig; @@ -16,24 +18,67 @@ fn deadline() -> Instant { Instant::now() + Duration::from_secs(10) } -fn run_exec( +/// Runs `workload` as the execution of `id`, with `input` as its standard input, and returns its +/// output and how it ended. +fn run_exec_with( session: &mut ControlSession, + id: u64, workload: &Workload<'_>, + input: &[u8], ) -> (Vec, Vec, ExecEvent) { - let id = session.start_exec(workload, deadline()).unwrap(); - let mut stdout = Vec::new(); - let mut stderr = Vec::new(); + session + .start_exec(id, &encode_exec_payload(workload).unwrap(), deadline()) + .unwrap(); + let (mut stdout, mut stderr, mut rest, mut open) = (Vec::new(), Vec::new(), input, true); loop { + // The window takes more of the input as the agent acknowledges what it wrote. + let sent = session.send_stdin(id, rest, deadline()).unwrap(); + rest = &rest[sent..]; + if rest.is_empty() && open { + session.close_stdin(id, deadline()).unwrap(); + open = false; + } match session.next_exec_event(id, Some(deadline())).unwrap() { ExecEvent::Stdout(bytes) => stdout.extend(bytes), ExecEvent::Stderr(bytes) => stderr.extend(bytes), - event @ ExecEvent::Exit { .. } | event @ ExecEvent::Rejected { .. } => { + ExecEvent::Acknowledged(_) => {} + event @ (ExecEvent::Exit { .. } | ExecEvent::Rejected { .. }) => { return (stdout, stderr, event); } } } } +fn run_exec( + session: &mut ControlSession, + workload: &Workload<'_>, +) -> (Vec, Vec, ExecEvent) { + run_exec_with(session, 1, workload, &[]) +} + +/// Runs `workload` over version 1's frames, which the agent still serves, and returns its output, +/// its exit category and its status. +fn run_exec_v1( + session: &mut ControlSession, + workload: &Workload<'_>, +) -> (Vec, Vec, (Vec, i32)) { + let payload = encode_exec_payload(workload).unwrap(); + let request = encode_app(APP_EXEC, 1, 0, &payload); + session.connection.send(&request, Some(deadline())).unwrap(); + let mut stdout = Vec::new(); + let mut stderr = Vec::new(); + loop { + let reply = session.connection.receive(Some(deadline())).unwrap(); + let frame = decode_app(&reply).unwrap(); + match frame.kind { + APP_STDOUT => stdout.extend(frame.payload), + APP_STDERR => stderr.extend(frame.payload), + APP_EXIT => return (stdout, stderr, (frame.payload.to_vec(), frame.status)), + kind => panic!("unexpected frame of kind {kind}"), + } + } +} + #[test] fn control_session_broker_and_real_agent_loop_back() { if super::super::isolate_process_test( @@ -71,10 +116,6 @@ fn control_session_broker_and_real_agent_loop_back() { let mut session = ControlSession::attach(broker.transport(), &capability, deadline()).unwrap(); session.ping(deadline()).unwrap(); - assert_eq!( - session.features(deadline()).unwrap(), - GuestFeatures::DIRECT_AGENT - ); session .map_host_paths( &[HostMap { @@ -100,7 +141,7 @@ fn control_session_broker_and_real_agent_loop_back() { assert_eq!( exit, ExecEvent::Exit { - category: ExitCategory::Exit, + category: Outcome::Exit, status: 7 } ); @@ -109,7 +150,7 @@ fn control_session_broker_and_real_agent_loop_back() { assert_eq!( signal, ExecEvent::Exit { - category: ExitCategory::Signal, + category: Outcome::Signal, status: 128 + sys::Signal::SIGTERM as i32 } ); @@ -117,7 +158,7 @@ fn control_session_broker_and_real_agent_loop_back() { assert_eq!( realtime_signal, ExecEvent::Exit { - category: ExitCategory::Signal, + category: Outcome::Signal, status: 128 + 40 } ); @@ -125,21 +166,40 @@ fn control_session_broker_and_real_agent_loop_back() { assert_eq!( timeout, ExecEvent::Exit { - category: ExitCategory::Timeout, + category: Outcome::Timeout, status: 124 } ); - let (_, _, limited) = run_exec(&mut session, &workload("head -c 1048577 /dev/zero", 0)); + // Version 2 caps no output: the old cap of 1 MiB passes whole. + let (stdout, _, uncapped) = run_exec(&mut session, &workload("head -c 1048577 /dev/zero", 0)); + assert_eq!(stdout, vec![0; 1024 * 1024 + 1]); + assert_eq!( + uncapped, + ExecEvent::Exit { + category: Outcome::Exit, + status: 0 + } + ); + // The workload reads the standard input that the host streams, more than the window holds, + // until its end. + let input: Vec = (0..200 * 1024).map(|index| (index % 251) as u8).collect(); + let (stdout, _, echoed) = run_exec_with(&mut session, 2, &workload("cat", 0), &input); + assert_eq!(stdout, input); assert_eq!( - limited, + echoed, ExecEvent::Exit { - category: ExitCategory::OutputLimit, - status: 125 + category: Outcome::Exit, + status: 0 } ); - let cancel_id = session - .start_exec(&workload("sleep 10", 0), deadline()) + let cancel_id = 3; + session + .start_exec( + cancel_id, + &encode_exec_payload(&workload("sleep 10", 0)).unwrap(), + deadline(), + ) .unwrap(); session.cancel_exec(cancel_id, deadline()).unwrap(); assert_eq!( @@ -147,13 +207,18 @@ fn control_session_broker_and_real_agent_loop_back() { .next_exec_event(cancel_id, Some(deadline())) .unwrap(), ExecEvent::Exit { - category: ExitCategory::Cancelled, + category: Outcome::Cancelled, status: 128 + sys::Signal::SIGKILL as i32 } ); - let reset_id = session - .start_exec(&workload("sleep 10", 0), deadline()) + let reset_id = 4; + session + .start_exec( + reset_id, + &encode_exec_payload(&workload("sleep 10", 0)).unwrap(), + deadline(), + ) .unwrap(); broker.reset(); assert!(matches!( @@ -163,6 +228,14 @@ fn control_session_broker_and_real_agent_loop_back() { drop(session); let mut session = ControlSession::attach(broker.transport(), &capability, deadline()).unwrap(); session.ping(deadline()).unwrap(); + // A stop during an execution ends it first. + session + .start_exec( + 5, + &encode_exec_payload(&workload("sleep 10", 0)).unwrap(), + deadline(), + ) + .unwrap(); session.stop(deadline()).unwrap(); drop(session); @@ -171,8 +244,8 @@ fn control_session_broker_and_real_agent_loop_back() { assert!(system.diagnostics.is_empty()); } -/// Once the output cap is hit, the agent forwards nothing more: each stream the client -/// receives stays a prefix of what the workload wrote, with no silent gap where a later +/// Once the output cap of NVXC version 1 is hit, the agent forwards nothing more: each stream the +/// client receives stays a prefix of what the workload wrote, with no silent gap where a later /// chunk fit the leftover room. #[test] fn output_limit_forwards_nothing_after_the_cap() { @@ -239,15 +312,10 @@ fn output_limit_forwards_nothing_after_the_cap() { cwd: None, environment: WorkloadEnvironment::Default, }; - let (stdout, stderr, exit) = run_exec(&mut session, &workload); + // Version 1, which caps the output, until SL4.4d deletes it. + let (stdout, stderr, exit) = run_exec_v1(&mut session, &workload); let count = |byte: u8| stdout.iter().filter(|&&each| each == byte).count(); - assert_eq!( - exit, - ExecEvent::Exit { - category: ExitCategory::OutputLimit, - status: 125 - } - ); + assert_eq!(exit, (b"output-limit".to_vec(), 125)); assert!(stderr.is_empty()); assert_eq!(count(b'a'), crate::protocol::MAX_OUTPUT_BYTES - room); assert!(count(b'b') <= room); diff --git a/nvx-rs/src/host/cli/sandbox/lifecycle/mod.rs b/nvx-rs/src/host/cli/sandbox/lifecycle/mod.rs index 65f8319d7..1b11bc969 100644 --- a/nvx-rs/src/host/cli/sandbox/lifecycle/mod.rs +++ b/nvx-rs/src/host/cli/sandbox/lifecycle/mod.rs @@ -44,7 +44,6 @@ use crate::host::runtime::{IdentityPart, LaunchOptions, Runtime, RuntimeIdentity use crate::host::sandbox::layer::SandboxLayer; use crate::host::sandbox::require_file; use crate::host::vm::SavedVm; -use crate::protocol::GuestFeatures; pub(crate) use self::directory::{create_state_directory, prepare_state_directory}; pub(crate) use self::outcome::{ @@ -81,10 +80,6 @@ pub(crate) struct ProvisionConfig<'a> { pub(crate) network: Option<&'a NetworkRecord>, } -/// The control features that the layered sandbox root must provide for the requests of -/// `nvx-rs sandbox`. -pub(crate) const REQUIRED_FEATURES: GuestFeatures = GuestFeatures::LAYERED_SANDBOX; - /// Records the launch of the distro layer `distro`, at the path at which it was verified, with /// the layer's binding, over `scratch`, the scratch image of an upper on scratch, with the options /// of `config`, in a new state directory, as `provision` does, with the binding of its scratch diff --git a/nvx-rs/src/host/cli/sandbox/lifecycle/start.rs b/nvx-rs/src/host/cli/sandbox/lifecycle/start.rs index 0ef21b4fe..e2e48b2cd 100644 --- a/nvx-rs/src/host/cli/sandbox/lifecycle/start.rs +++ b/nvx-rs/src/host/cli/sandbox/lifecycle/start.rs @@ -1,6 +1,7 @@ //! Starting a managed sandbox and executing workloads in its running VM, and running one workload //! in a VM of its own, without a state directory. +use std::cell::Cell; use std::fs; use std::io; use std::path::Path; @@ -14,8 +15,8 @@ use serde_json::Value; use super::directory::prepare_state_directory; use super::outcome::{copy_openvmm_outcome, validate_outcome_destination}; use super::{ - CONFIG_NAME, Found, ProvisionConfig, REQUIRED_FEATURES, connect, find_vm, mismatched, - require_config, running_vm, runtime_for, + CONFIG_NAME, Found, ProvisionConfig, connect, find_vm, mismatched, require_config, running_vm, + runtime_for, }; use crate::activation::{ActivationRecord, TransactionId}; use crate::host::cli::artifacts::locate; @@ -29,7 +30,7 @@ use crate::host::cli::sandbox::state::{ use crate::host::cli::sandbox::{ Artifacts, Options, error_with_cause, export_tokens, path_string, refuse_own_files, }; -use crate::host::cli::session::{ExecRequest, ExecResult, Session}; +use crate::host::cli::session::{ExecRequest, ExecResult, Session, closes}; use crate::host::cli::shell::format_command; use crate::host::cli::snapshots::Store; use crate::host::cli::store::StateFile; @@ -48,8 +49,9 @@ use crate::host::snapshot::OPENVMM_DIR; use crate::host::vm::{self, Launched, Prepared, SavedVm, VmDir}; use crate::protocol::cmdline::{self, Parameter}; use crate::protocol::root::RootLayout; +use crate::protocol::v2::Outcome; use crate::protocol::workload::WorkloadIdentity; -use crate::protocol::{self, ExitCategory, HostMap, WorkloadEnvironment}; +use crate::protocol::{self, HostMap, WorkloadEnvironment}; use crate::{ErrorCode, ErrorKind}; /// Starts a provisioned sandbox, as `start` does. A workload of a restored sandbox that does not @@ -414,24 +416,12 @@ fn binding_error(error: crate::Error) -> Exit { } /// Waits until the guest of a VM that starts answers on `endpoint`, which `launched`, the OpenVMM -/// process of the start, serves, or until OpenVMM exits, checks that the guest provides the -/// features of [`REQUIRED_FEATURES`], and returns the session, on which the caller sends the -/// guest what it needs before the launch commits. +/// process of the start, serves, or until OpenVMM exits, and returns the session, on which the +/// caller sends the guest what it needs before the launch commits. The guest's Ready names the +/// version of NVXC that it speaks, which the session requires. fn check_ready(saved: &SavedVm, runtime: &Runtime, timeout: f64) -> Result { let mut session = Session::reattach(saved, runtime, timeout)?; session.ping(timeout)?; - let features = session.features(timeout)?; - let missing = features.missing(REQUIRED_FEATURES); - if !missing.is_empty() { - return Err(Exit::error( - ErrorCode::GuestFeaturesMissing, - format!( - "the guest image lacks control features that layered sandboxes need ({}); use \ - an NVX release or build that includes them", - missing.join(", ") - ), - )); - } Ok(session) } @@ -495,8 +485,9 @@ fn restore_guest( Ok(ExecEvent::Stderr(data)) => { output::write_stderr(&data).map_err(Exit::from_os_error)?; } + Ok(ExecEvent::Acknowledged(_)) => {} Ok(ExecEvent::Exit { - category: ExitCategory::Exit, + category: Outcome::Exit, status: 0, }) => return Ok(()), Ok(ExecEvent::Exit { status, .. }) if restore.forward_status => { @@ -533,8 +524,10 @@ fn restore_guest( ControlRestoreError::Maps(error) | ControlRestoreError::Start(error) => match error { SessionError::TimedOut => ErrorCode::TimedOut, // As for a cold start, a closed endpoint proves no exit, and gives OpenVMM its grace - // to exit by itself before the start ends it. + // to exit by itself before the start ends it. An endpoint that closes before it read + // the end of the workload's input, which follows its Start, resets the stream. SessionError::ProcessExited | SessionError::Closed => return Err(Exit::EndpointClosed), + SessionError::Io(error) if closes(error) => return Err(Exit::EndpointClosed), SessionError::Io(_) => ErrorCode::Io, _ => ErrorCode::Protocol, }, @@ -683,6 +676,10 @@ pub(crate) fn run( let dir = VmDir::open(&path).map_err(error_with_cause)?; let timeout = options.timeout; let mut cleanup = true; + // A diagnostic or an error of the run's own starts a line of its own after standard error + // that the workload's output left open. + let open_line = Cell::new(None); + let end_line = || output::end_stderr_line(open_line.take().as_slice()); let result = (|| { let mut prepared = vm::create(&runtime, &vm_config, None, &dir) .map_err(error_with_cause)? @@ -735,6 +732,7 @@ pub(crate) fn run( .expect("a ready VM has a session") .exec(request, timeout) }); + open_line.set(executed.as_ref().ok().and_then(|result| result.stderr_end)); // A VM whose control endpoint closed is shutting down already. let requested = match session.take() { Some(mut session) @@ -769,12 +767,14 @@ pub(crate) fn run( if let Some(report) = &options.outcome_report && let Err(error) = copy_openvmm_outcome(&launched.outputs.outcome, report) { + end_line(); error.report(); } // Ctrl-C during the request ends the run as interrupted, which is no failed shutdown. if let Some(Err(error)) = &requested && !matches!(error, Exit::Interrupted) { + end_line(); output::print_stderr(&format!( "warning: managed guest shutdown failed: {error:?}" )); @@ -785,16 +785,21 @@ pub(crate) fn run( return Err(exited_during_startup(report)); } } else if executed.is_ok() && stopped.forced { + end_line(); output::print_stderr("warning: OpenVMM was forcibly terminated after the run"); } executed.map(Some) })(); if cleanup && let Err(error) = remove_vm_directory(&dir) { + end_line(); output::print_stderr(&format!( "warning: cannot remove the temporary VM directory {}: {error:?}", dir.path().display() )); } + if result.is_err() { + end_line(); + } result } diff --git a/nvx-rs/src/host/cli/sandbox/lifecycle/start/tests/restore.rs b/nvx-rs/src/host/cli/sandbox/lifecycle/start/tests/restore.rs index e6f939913..f974b9867 100644 --- a/nvx-rs/src/host/cli/sandbox/lifecycle/start/tests/restore.rs +++ b/nvx-rs/src/host/cli/sandbox/lifecycle/start/tests/restore.rs @@ -31,13 +31,23 @@ use crate::host::sandbox::{DEFAULT_HOSTNAME, DEFAULT_WORKLOAD_IDENTITY}; use crate::host::snapshot::tests::RestoreFixture; use crate::host::snapshot::{DESCRIPTION_FILE, KEY_FILE, OPENVMM_DIR}; use crate::protocol::root::RootImage; +use crate::protocol::v2::{self, Frame, Outcome}; use crate::protocol::{ - APP_ACTIVATE, APP_EXEC, APP_EXIT, APP_HEADER_LEN, APP_MAPS, APP_READY, APP_STDOUT, OUTER_DATA, - OUTER_HEADER_LEN, OUTER_HOST_ATTACH, OUTER_MAX_PAYLOAD, OUTER_READY, decode_app, - decode_outer_header, encode_app, encode_exec_payload, encode_maps_payloads, encode_outer, - status, + OUTER_DATA, OUTER_HEADER_LEN, OUTER_HOST_ATTACH, OUTER_READY, decode_outer_header, + encode_exec_payload, encode_maps_payloads, encode_outer, status, }; +/// The codes of the requests of NVXC that the fake guest of the fixture's restores serves: the +/// activate message, the mapping table and the workload's Start, and the end of its standard +/// input. +const ACTIVATE: u8 = 0x08; +/// See [`ACTIVATE`]. +const MAPS: u8 = 0x06; +/// See [`ACTIVATE`]. +const EXEC: u8 = 0x02; +/// See [`ACTIVATE`]. +const STDIN_END: u8 = 0x0a; + /// The fake VMM and guest that the fixture's restores spawn in place of OpenVMM. const HELPER: &str = "host::cli::sandbox::lifecycle::start::tests::restore::restore_helper"; /// OpenVMM's restore-ready event. @@ -286,7 +296,7 @@ fn named_snapshots_restore_each_image_format_and_upper_with_their_own_policy_and fixture.start("state", config, "ready", 30.0).unwrap(); let requests = fixture.requests("state"); let kinds: Vec = requests.iter().map(|(kind, _)| *kind).collect(); - assert_eq!(kinds, [APP_ACTIVATE, APP_MAPS, APP_EXEC]); + assert_eq!(kinds, [ACTIVATE, MAPS, EXEC, STDIN_END]); let (record, identity) = activation(&requests[0].1); assert_eq!(identity, (1000, 1001, 0)); let configuration = record.configuration(); @@ -306,7 +316,7 @@ fn named_snapshots_restore_each_image_format_and_upper_with_their_own_policy_and assert_eq!(configuration.resources.cpus, 2); assert_eq!( requests[1].1, - encode_maps_payloads(&maps, OUTER_MAX_PAYLOAD - APP_HEADER_LEN).unwrap()[0] + encode_maps_payloads(&maps, v2::MAX_PAYLOAD).unwrap()[0] ); let workload = protocol::Workload { argv: WORKLOAD.map(str::to_owned).to_vec(), @@ -390,7 +400,7 @@ fn a_one_off_restore_installs_the_network_options_of_run_as_its_own_policy() { // The guest learns the DNS server that the policy lets it reach, and announces its listeners, // which the policy publishes on host loopback. let requests = fixture.requests("state"); - assert_eq!(requests[0].0, APP_ACTIVATE); + assert_eq!(requests[0].0, ACTIVATE); assert_eq!( activation(&requests[0].1).0.configuration().network, Some(Network { @@ -506,7 +516,7 @@ fn concurrent_restores_of_one_snapshot_wait_for_its_lock_until_the_first_vm_has_ assert!(spawned[1].2); assert!(fixture.exited(0)); assert!(fixture.requests("first").is_empty()); - assert_eq!(fixture.requests("second").len(), 2); + assert_eq!(fixture.requests("second").len(), 3); assert!(fixture.snapshot_free()); } @@ -655,14 +665,14 @@ fn a_later_start_restores_again_with_an_upper_in_ram_but_never_with_a_scratch_im if ram { again.unwrap(); assert_eq!(fixture.spawned().len(), 2); - assert_eq!(fixture.requests("state").len(), 4); + assert_eq!(fixture.requests("state").len(), 6); } else { // The guest of the first start may have written the image, which a restore's guest // refuses: nothing launches, and the view stays for deprovision. let error = again.unwrap_err(); assert_eq!(error.code(), Some(ErrorCode::Unsupported), "{error:?}"); assert_eq!(fixture.spawned().len(), 1); - assert_eq!(fixture.requests("state").len(), 2); + assert_eq!(fixture.requests("state").len(), 3); } assert_eq!(fixture.view("state"), 3, "{ram}"); assert!(fixture.snapshot_free()); @@ -957,7 +967,7 @@ fn read_record(stream: &mut impl Read) -> Option<(u8, Vec)> { } /// Serves one session of the host as the guest of the recipe's behavior does, and traces its -/// requests. +/// requests, and the end of the standard input of the workload that the Start runs. fn serve(stream: &mut (impl Read + Write), capability: &[u8], epoch: u64, recipe: &Value) { let Some((kind, payload)) = read_record(stream) else { return; @@ -968,26 +978,39 @@ fn serve(stream: &mut (impl Read + Write), capability: &[u8], epoch: u64, recipe return; } let trace = recipe["trace"].as_str().unwrap(); - let mut sequence = 1; - while let Some((kind, payload)) = read_record(stream) { - assert_eq!(kind, OUTER_DATA); - let request = decode_app(&payload).unwrap(); - let line = json!({"kind": request.kind, "payload": hex(request.payload)}); + // Traces the frame of a DATA record, and returns its kind, its request ID and its payload. + let request = |frame: &[u8]| { + let (request_id, _) = Frame::decode(frame).unwrap(); + let (kind, payload) = (frame[5], &frame[24..]); + let line = json!({"kind": kind, "payload": hex(payload)}); let mut file = OpenOptions::new() .create(true) .append(true) .open(trace) .unwrap(); writeln!(file, "{line}").unwrap(); + (kind, request_id, payload.to_vec()) + }; + let mut sequence = 1; + while let Some((kind, frame)) = read_record(stream) { + assert_eq!(kind, OUTER_DATA); + let (kind, request_id, payload) = request(&frame); // A guest that hangs up does once it has the workload's Start, which it never answers. - if request.kind == APP_EXEC && recipe["behavior"] == "hangup" { + if kind == EXEC && recipe["behavior"] == "hangup" { return; } - let replies = match request.kind { - APP_ACTIVATE if recipe["behavior"] == "malformed" => vec![(APP_READY, 0, vec![0])], + let ready = |activation: &[u8]| { + let frame = Frame::Ready { + version: v2::VERSION, + activation, + }; + frame.encode(request_id).unwrap() + }; + let replies = match kind { + ACTIVATE if recipe["behavior"] == "malformed" => vec![ready(&[0])], // The guest cannot assemble its root, and powers off with the status of the failure's // class, which OpenVMM reports, and then exits with, or reports unreadably. - APP_ACTIVATE + ACTIVATE if matches!( recipe["behavior"].as_str(), Some("unusable-root" | "unreadable-report") @@ -1006,24 +1029,29 @@ fn serve(stream: &mut (impl Read + Write), capability: &[u8], epoch: u64, recipe fs::write(recipe["outcome"].as_str().unwrap(), report).unwrap(); std::process::exit(code.into()); } - APP_ACTIVATE => { - let ready = ReadyActivation::of(&activation(request.payload).0); - vec![(APP_READY, 0, ready.encode())] + ACTIVATE => vec![ready( + &ReadyActivation::of(&activation(&payload).0).encode(), + )], + MAPS => vec![ready(&[])], + // The workload reads its standard input until its end, which follows the Start, + // writes its output, then exits, with status 3 if it fails, or SIGTERM ends it. + _ => { + let (_, end) = read_record(stream).unwrap(); + assert_eq!(request(&end), (STDIN_END, request_id, Vec::new())); + let (status, outcome) = match recipe["behavior"].as_str() { + Some("failing") => (3, Outcome::Exit), + Some("signaled") => (143, Outcome::Signal), + _ => (0, Outcome::Exit), + }; + [ + Frame::Stdout(b"restored\n"), + Frame::Exit { status, outcome }, + ] + .map(|frame| frame.encode(request_id).unwrap()) + .to_vec() } - APP_MAPS => vec![(APP_READY, 0, Vec::new())], - // The workload writes its output, then exits, with status 3 if it fails, or SIGTERM - // ends it. - _ => vec![ - (APP_STDOUT, 0, b"restored\n".to_vec()), - match recipe["behavior"].as_str() { - Some("failing") => (APP_EXIT, 3, b"exit".to_vec()), - Some("signaled") => (APP_EXIT, 143, b"signal".to_vec()), - _ => (APP_EXIT, 0, b"exit".to_vec()), - }, - ], }; - for (kind, status, payload) in replies { - let reply = encode_app(kind, request.request_id, status, &payload); + for reply in replies { let record = encode_outer(OUTER_DATA, &[7; 16], epoch, sequence, &reply).unwrap(); if stream.write_all(&record).is_err() { return; diff --git a/nvx-rs/src/host/cli/sandbox/mod.rs b/nvx-rs/src/host/cli/sandbox/mod.rs index c67b49bc5..52dffcc65 100644 --- a/nvx-rs/src/host/cli/sandbox/mod.rs +++ b/nvx-rs/src/host/cli/sandbox/mod.rs @@ -5,8 +5,8 @@ //! - `run`, the default, runs one workload in a sandbox of its own, without a state directory: it //! launches the VM that a `provision` and a `start` of the same options would launch, in a //! temporary VM directory, prints its OpenVMM command as `>> COMMAND`, executes the workload -//! once over NVXC, as `exec` would, with `exec`'s options, stops the VM, and exits with the -//! workload's status, after its output. A dry run prints the command and launches nothing. See +//! once over NVXC, as `exec` would, with `exec`'s options and its streams, stops the VM, and +//! exits with the workload's status. A dry run prints the command and launches nothing. See //! [`lifecycle::run`]. //! - `provision`, `start`, `exec`, `stop`, and `deprovision` manage a persistent sandbox in its //! `--state-dir`; see [`lifecycle`]. @@ -380,15 +380,12 @@ fn run_from(root: Option<&str>, options: Options, source: Option<&Source>) -> Re options.timeout, options.wait, )?; - output::write_stdout(&result.stdout) - .and_then(|()| output::write_stderr(&result.stderr)) - .map_err(Exit::from_os_error)?; if let Some(report) = &options.outcome_report { // An error of the report follows the workload's output, and starts a line of its // own even after standard error that the workload left without a line end, so // that callers find it as they find any other. lifecycle::write_exec_outcome(report, &result) - .inspect_err(|_| output::end_stderr_line(&result.stderr))?; + .inspect_err(|_| output::end_stderr_line(result.stderr_end.as_slice()))?; } Err(Exit::Status(result.returncode)) } @@ -420,9 +417,6 @@ fn run_from(root: Option<&str>, options: Options, source: Option<&Source>) -> Re else { return Ok(()); }; - output::write_stdout(&result.stdout) - .and_then(|()| output::write_stderr(&result.stderr)) - .map_err(Exit::from_os_error)?; Err(Exit::Status(result.returncode)) } (SandboxOperation::Run | SandboxOperation::Provision, None) => { diff --git a/nvx-rs/src/host/cli/session.rs b/nvx-rs/src/host/cli/session.rs index 31cd61672..da3e7395a 100644 --- a/nvx-rs/src/host/cli/session.rs +++ b/nvx-rs/src/host/cli/session.rs @@ -1,6 +1,6 @@ //! The control session of `nvx-rs sandbox`, a port of the former -//! `scripts/nvx_tools/control_session.py` on the framing, sequencing, attachment, and transports -//! of [`crate::host::session`]. +//! `scripts/nvx_tools/control_session.py` on the requests, framing, sequencing, attachment, and +//! transports of [`crate::host::session`], which speaks version 2 of NVXC. //! //! It keeps the observable behavior of the Python session: //! @@ -14,9 +14,9 @@ //! - An endpoint that closes, as OpenVMM closes it when it shuts down, ends the operation with //! [`Exit::EndpointClosed`], while a session that OpenVMM resets fails with `managed control //! session was reset`. -//! - `exec` refuses invalid requests before it sends anything, with Python's messages, buffers -//! the workload's output, and accepts the exit categories of `MANAGED_EXIT_CATEGORIES`; a -//! refusal keeps its status and its category, which the guest sends in ASCII. +//! - `exec` refuses invalid requests before it sends anything, with Python's messages, and +//! accepts the exit categories of `MANAGED_EXIT_CATEGORIES`; a refusal keeps its status and its +//! category. //! - A session whose transfer failed is closed, and refuses further requests. //! //! Unlike the Python session, it also hands the guest agent a host mapping table, as `start` @@ -24,37 +24,41 @@ //! [`Session::reattach`] verifies a saved VM and observes its transport before authentication; //! [`Session::connect_launched_cancellable`], with which a cold run waits for the endpoint of the //! OpenVMM that it launched, also stops waiting once that OpenVMM exits, where Python waited until -//! its timeout. +//! its timeout. `exec` streams, where Python buffered the workload's output: the workload reads +//! the command's standard input, until its end, and its output reaches the command's standard +//! output and error as it comes, without a total cap; and Ctrl-C cancels it, which would +//! otherwise outlive the session. //! //! The transports also wait in slices of at most 250 ms, as Python's do, so that Ctrl-C ends a -//! wait within that time, except while a request is being sent, which its timeout bounds. +//! wait within that time, except while a request is being sent, which its timeout bounds, and +//! while a workload streams, whose loop checks for Ctrl-C itself. -use std::io; +use std::io::{self, Read}; use std::sync::Arc; use std::sync::atomic::{AtomicBool, AtomicU8, Ordering}; -use std::time::Duration; +use std::sync::mpsc; +use std::thread; +use std::time::{Duration, Instant}; use super::deadline::{Deadline, RETRY_INTERVAL, sleep}; -use super::{Exit, Result, check_interrupt}; +use super::{Exit, Result, check_interrupt, output}; use crate::host::platform::{self, Transport}; use crate::host::process::Process; use crate::host::runtime::Runtime; use crate::host::sandbox::options::validate_exec_environment; -use crate::host::session::{ControlSession, SessionError}; +use crate::host::session::{ControlSession, ExecEvent, SessionError}; use crate::host::transport; use crate::host::vm::{self, SavedVm}; +use crate::protocol::v2::{self, Outcome}; use crate::protocol::{ - APP_ERROR, APP_EXEC, APP_EXIT, APP_FEATURES, APP_HEADER_LEN, APP_MAPS, APP_PING, APP_READY, - APP_STDERR, APP_STDOUT, APP_STOP, APP_STOPPED, CAPABILITY_LEN, GuestFeatures, HostMap, - MAPPING_FAILED, MAX_OUTPUT_BYTES, OUTER_MAX_PAYLOAD, UNSUPPORTED_OPERATION, Workload, - WorkloadEnvironment, encode_exec_payload, encode_maps_payloads, + CAPABILITY_LEN, HostMap, Workload, WorkloadEnvironment, encode_exec_payload, }; use crate::{ErrorCode, ErrorKind}; -/// The exit categories that `exec` accepts, `MANAGED_EXIT_CATEGORIES`. A cancellation, which this -/// session never requests, is not among them. -pub(crate) const EXIT_CATEGORIES: [&str; 5] = - ["exit", "timeout", "output-limit", "signal", "failed"]; +/// The exit categories that `exec` accepts, `MANAGED_EXIT_CATEGORIES`, but the output limit, +/// which version 2 of NVXC does not have. A cancellation, whose end this session never waits +/// for, is not among them. +pub(crate) const EXIT_CATEGORIES: [&str; 4] = ["exit", "timeout", "signal", "failed"]; /// The most arguments of a workload, `APP_MAX_ARGUMENTS`. const MAX_ARGUMENTS: usize = 64; /// The longest argument and working directory, in UTF-8 bytes. @@ -157,6 +161,9 @@ struct Probe { closed: AtomicBool, /// Whether Ctrl-C ended a wait. interrupted: AtomicBool, + /// Whether a workload streams, whose loop, rather than a wait of the transport, ends at + /// Ctrl-C, so that it can cancel the workload first. + streaming: AtomicBool, /// The foreground owner's cancellation, absent for ordinary sessions. cancellation: Option, } @@ -171,7 +178,7 @@ struct ProbedTransport { /// Returns whether `error` tells that the endpoint closed the stream, as Python's /// `_PEER_CLOSED` and `_PIPE_CLOSED_ERRORS` do. -fn closes(error: &io::Error) -> bool { +pub(super) fn closes(error: &io::Error) -> bool { matches!( error.kind(), io::ErrorKind::BrokenPipe @@ -195,7 +202,7 @@ impl Transport for ProbedTransport { self.probe.direction.store(RECEIVING, Ordering::SeqCst); let deadline = timeout.map(|timeout| std::time::Instant::now() + timeout); loop { - if platform::interrupted() { + if platform::interrupted() && !self.probe.streaming.load(Ordering::SeqCst) { return Err(self.interrupted()); } if let Some(cancellation) = &self.probe.cancellation { @@ -264,17 +271,16 @@ pub(crate) struct ExecRequest { pub(crate) inherit_default_environment: bool, } -/// How a workload ended: `ManagedExecResult`. +/// How a workload ended: `ManagedExecResult`, whose output already went to the standard streams. #[derive(Debug, Clone, PartialEq, Eq)] pub(crate) struct ExecResult { /// The workload's status. pub(crate) returncode: i32, /// One of [`EXIT_CATEGORIES`]. pub(crate) category: &'static str, - /// The workload's standard output. - pub(crate) stdout: Vec, - /// The workload's standard error. - pub(crate) stderr: Vec, + /// The last byte that the workload wrote to standard error, if any, which tells whether it + /// left a line open there. + pub(crate) stderr_end: Option, } /// An authenticated control session: `ControlSession` of the former `control_session.py`. @@ -328,9 +334,7 @@ impl Session { } })?; Ok(Self { - inner: Some(ControlSession { - connection: attached.connection, - }), + inner: Some(attached.connection.into()), probe, }) } @@ -451,54 +455,23 @@ impl Session { .ok_or_else(|| Exit::error(ErrorCode::Protocol, "managed control session is closed")) } - /// Sends a request and closes the session if the transfer fails. - fn send(&mut self, kind: u8, request_id: u64, payload: &[u8], deadline: Deadline) -> Result { + /// Runs `operation` on the open session, and closes the session if a transfer fails. + fn call( + &mut self, + operation: impl FnOnce(&mut ControlSession) -> std::result::Result, + ) -> Result { self.probe.direction.store(SENDING, Ordering::SeqCst); - let sent = self - .session()? - .send_request(kind, request_id, payload, deadline.instant()); - sent.map_err(|error| self.fail(error)) - } - - /// Reads the next response and closes the session if the transfer fails. - fn receive(&mut self, deadline: Deadline) -> Result<(u8, u64, i32, Vec)> { - self.probe.direction.store(RECEIVING, Ordering::SeqCst); - let received = self.session()?.read_response(Some(deadline.instant())); - received.map_err(|error| self.fail(error)) - } - - /// Reads the next response, which must answer `request_id`, and returns its kind, status, - /// and payload. - fn reply(&mut self, request_id: u64, deadline: Deadline) -> Result<(u8, i32, Vec)> { - let (kind, response_id, status, payload) = self.receive(deadline)?; - if response_id != request_id { - return Err(Exit::error( - ErrorCode::Protocol, - "managed guest returned a mismatched request ID", - )); - } - Ok((kind, status, payload)) - } - - /// Sends a request of `kind` without a payload and waits up to `timeout` seconds until the - /// guest agent acknowledges `what` with an empty `reply` of status 0. - fn acknowledged(&mut self, kind: u8, reply: u8, what: &str, timeout: f64) -> Result { - let request_id = request_id()?; - let deadline = Deadline::after(timeout); - self.send(kind, request_id, &[], deadline)?; - let (kind, response_id, status, payload) = self.receive(deadline)?; - if kind != reply || response_id != request_id || status != 0 || !payload.is_empty() { - return Err(Exit::error( - ErrorCode::Protocol, - format!("managed guest did not acknowledge {what}"), - )); - } - Ok(()) + let result = operation(self.session()?); + result.map_err(|error| self.fail(error)) } - /// Closes the session after a failed transfer and returns the failure. + /// Closes the session after a failed transfer, and returns the failure. A refusal of the guest + /// agent, or an answer that the protocol does not allow, fails no transfer: the session stays, + /// and its connection ends at a record that the protocol does not allow. fn fail(&mut self, error: SessionError) -> Exit { - self.inner = None; + if !matches!(error, SessionError::Protocol(_)) { + self.inner = None; + } self.failure(error) } @@ -541,27 +514,8 @@ impl Session { /// Waits until the guest agent acknowledges readiness, as `ControlSession.ping` does. pub(crate) fn ping(&mut self, timeout: f64) -> Result { - self.acknowledged(APP_PING, APP_READY, "readiness", timeout) - } - - /// Asks the guest agent which control features its image provides. An agent that predates - /// the request refuses it as an unsupported operation, so its image provides none. - pub(crate) fn features(&mut self, timeout: f64) -> Result { - let request_id = request_id()?; - let deadline = Deadline::after(timeout); - self.send(APP_FEATURES, request_id, &[], deadline)?; - let (kind, status, payload) = self.reply(request_id, deadline)?; - let invalid = || { - Exit::error( - ErrorCode::Protocol, - "managed guest returned an invalid features response", - ) - }; - match kind { - APP_READY if status == 0 => GuestFeatures::decode(&payload).map_err(|_| invalid()), - APP_ERROR if payload == UNSUPPORTED_OPERATION => Ok(GuestFeatures::NONE), - _ => Err(invalid()), - } + let deadline = Deadline::after(timeout).instant(); + self.call(|session| session.ping(deadline)) } /// Hands the guest agent the host mapping table `maps`, in order and in as many `MAPS` @@ -569,41 +523,8 @@ impl Session { /// `timeout` seconds. The agent refuses workloads until then, and refuses the rest of the /// table after an entry that it could not mount. pub(crate) fn map_host_paths(&mut self, maps: &[HostMap], timeout: f64) -> Result { - let payloads = encode_maps_payloads(maps, OUTER_MAX_PAYLOAD - APP_HEADER_LEN) - .map_err(|error| Exit::error(ErrorCode::Protocol, error.message()))?; - let deadline = Deadline::after(timeout); - for payload in payloads { - let request_id = request_id()?; - self.send(APP_MAPS, request_id, &payload, deadline)?; - let (kind, status, payload) = self.reply(request_id, deadline)?; - match kind { - APP_READY if status == 0 && payload.is_empty() => {} - APP_ERROR if payload == MAPPING_FAILED.as_bytes() => { - return Err(Exit::error( - ErrorCode::Protocol, - format!( - "managed guest could not mount a mapped host path (error {status})" - ), - )); - } - APP_ERROR => { - return Err(Exit::error( - ErrorCode::Protocol, - format!( - "managed guest did not mount the host paths: {} (status {status})", - String::from_utf8_lossy(&payload) - ), - )); - } - _ => { - return Err(Exit::error( - ErrorCode::Protocol, - "managed guest returned an invalid host mapping response", - )); - } - } - } - Ok(()) + let deadline = Deadline::after(timeout).instant(); + self.call(|session| session.map_host_paths(maps, deadline)) } /// Checks the workload of `request` and returns the payload of its exec request, as @@ -658,11 +579,48 @@ impl Session { } /// Runs a workload and returns how it ended, as `ControlSession.exec` does: the request and - /// every response must arrive within `response_timeout` seconds of each other's start. + /// every response must arrive within `response_timeout` seconds of its start. The workload + /// reads this process's standard input until its end, and its standard output and error + /// reach this process's as they come. Ctrl-C cancels it. An error that follows its output + /// starts a line of its own, as callers find errors. pub(crate) fn exec( &mut self, request: &ExecRequest, response_timeout: f64, + ) -> Result { + let mut end = None; + let write = |stderr, data: &[u8]| { + if stderr { + end = data.last().copied(); + output::write_stderr(data) + } else { + output::write_stdout(data) + } + }; + let result = self.exec_with( + request, + response_timeout, + read_stdin, + platform::interrupted, + write, + ); + if result.is_err() { + output::end_stderr_line(end.as_slice()); + } + result + } + + /// Runs a workload as [`exec`](Self::exec) does, with the chunks that `input` returns the + /// receiver of, until it disconnects, as its standard input, or a read of it fails, `write` + /// for its standard output, or its standard error if its first argument holds, and + /// `interrupted` for Ctrl-C. + fn exec_with( + &mut self, + request: &ExecRequest, + response_timeout: f64, + input: impl FnOnce() -> mpsc::Receiver>>, + interrupted: impl Fn() -> bool, + mut write: impl FnMut(bool, &[u8]) -> io::Result<()>, ) -> Result { if !(response_timeout > 0.0 && response_timeout < f64::INFINITY) { return Err(Exit::error( @@ -673,57 +631,72 @@ impl Session { let payload = Self::exec_payload(request)?; let request_id = request_id()?; let deadline = Deadline::after(response_timeout); - self.send(APP_EXEC, request_id, &payload, deadline)?; - let mut stdout = Vec::new(); - let mut stderr = Vec::new(); + self.call(|session| session.start_exec(request_id, &payload, deadline.instant()))?; + self.probe.streaming.store(true, Ordering::SeqCst); + let result = self.stream(request_id, deadline, &input(), &interrupted, &mut write); + self.probe.streaming.store(false, Ordering::SeqCst); + result + } + + /// Streams the execution of `request_id` until it ends or `deadline` passes, as + /// [`exec_with`](Self::exec_with) does, with no more than the window of its standard input + /// unacknowledged, which [`ControlSession::send_stdin`] keeps. + fn stream( + &mut self, + request_id: u64, + deadline: Deadline, + input: &mpsc::Receiver>>, + interrupted: &impl Fn() -> bool, + write: &mut impl FnMut(bool, &[u8]) -> io::Result<()>, + ) -> Result { + let (mut pending, mut open, mut stderr_end) = (Vec::new(), true, None); loop { - let (kind, status, response) = self.reply(request_id, deadline)?; - let violation = |message: &str| Exit::error(ErrorCode::Protocol, message); - match kind { - APP_STDOUT | APP_STDERR => { - // The guest agent forwards at most this much output, so more breaks the - // protocol. - if stdout.len() + stderr.len() + response.len() > MAX_OUTPUT_BYTES { - return Err(violation(&format!( - "managed guest exceeded the {MAX_OUTPUT_BYTES}-byte output limit" - ))); + if interrupted() { + self.cancel(request_id); + return Err(Exit::Interrupted); + } + if open && pending.is_empty() { + match input.try_recv() { + Ok(Ok(chunk)) => pending = chunk, + // An end of the input would hand the workload what it read as all of it. + Ok(Err(error)) => { + self.cancel(request_id); + return Err(Exit::error( + ErrorCode::Io, + format!("cannot read standard input: {error}"), + )); } - let output = if kind == APP_STDOUT { - &mut stdout - } else { - &mut stderr - }; - output.extend_from_slice(&response); - } - APP_EXIT => { - if !response.is_ascii() { - return Err(violation("managed guest returned an invalid exit category")); + Err(mpsc::TryRecvError::Empty) => {} + Err(mpsc::TryRecvError::Disconnected) => { + open = false; + self.call(|session| session.close_stdin(request_id, deadline.instant()))?; } - let category = EXIT_CATEGORIES - .iter() - .find(|category| category.as_bytes() == response) - .ok_or_else(|| { - violation("managed guest returned an unsupported exit category") - })?; + } + } + if !pending.is_empty() { + let sent = self + .call(|session| session.send_stdin(request_id, &pending, deadline.instant()))?; + pending.drain(..sent); + } + self.probe.direction.store(RECEIVING, Ordering::SeqCst); + let slice = deadline.instant().min(Instant::now() + RETRY_INTERVAL); + let written = match self.session()?.next_exec_event(request_id, Some(slice)) { + Err(SessionError::TimedOut) if !deadline.passed() => Ok(()), + Err(error) => return Err(self.fail(error)), + Ok(ExecEvent::Stdout(data)) => write(false, &data), + Ok(ExecEvent::Stderr(data)) => { + stderr_end = data.last().copied(); + write(true, &data) + } + Ok(ExecEvent::Acknowledged(_)) => Ok(()), + Ok(ExecEvent::Exit { category, status }) => { return Ok(ExecResult { returncode: status, - category, - stdout, - stderr, + category: exit_category(category)?, + stderr_end, }); } - APP_ERROR => { - // `bytes.decode("ascii", "replace")`. - let category: String = response - .iter() - .map(|&byte| { - if byte.is_ascii() { - char::from(byte) - } else { - '\u{fffd}' - } - }) - .collect(); + Ok(ExecEvent::Rejected { status, category }) => { return Err(Exit::error( ErrorCode::ExecRefused, format!( @@ -731,23 +704,81 @@ impl Session { ), )); } - _ => { - return Err(violation("managed guest returned an invalid exec response")); - } + }; + // Ctrl-C ends a write, after which the next turn cancels the workload. + if let Err(error) = written + && !interrupted() + { + return Err(Exit::from_os_error(error)); } } } + /// Asks the guest agent, within one wait slice, to end the execution of `request_id`, which + /// would otherwise outlive the session, which ends with the command. + fn cancel(&mut self, request_id: u64) { + if let Some(session) = &mut self.inner { + let _ = session.cancel_exec(request_id, Instant::now() + POLL_SLICE); + } + } + /// Asks the guest agent to shut the VM down, as `ControlSession.stop` does. pub(crate) fn stop(&mut self, timeout: f64) -> Result { - self.acknowledged(APP_STOP, APP_STOPPED, "stop", timeout) + let deadline = Deadline::after(timeout).instant(); + self.call(|session| session.stop(deadline)) } } +/// Returns the category of [`EXIT_CATEGORIES`] of a workload that ended with `outcome`, or fails +/// for a cancellation, whose end no execution of this session reports. +fn exit_category(outcome: Outcome) -> Result<&'static str> { + Ok(match outcome { + Outcome::Exit => "exit", + Outcome::Timeout => "timeout", + Outcome::Signal => "signal", + Outcome::Failed => "failed", + Outcome::Cancelled => { + return Err(Exit::error( + ErrorCode::Protocol, + "managed guest returned an unsupported exit category", + )); + } + }) +} + +/// Returns the receiver of the chunks of this process's standard input, which a thread of its own +/// reads until the input ends, or a read fails, whose error ends them, or no one receives them +/// any longer. +fn read_stdin() -> mpsc::Receiver>> { + let (sender, receiver) = mpsc::sync_channel(1); + thread::spawn(move || { + let mut input = io::stdin().lock(); + loop { + let mut chunk = vec![0; v2::MAX_PAYLOAD]; + match input.read(&mut chunk) { + Err(error) if error.kind() == io::ErrorKind::Interrupted => {} + Ok(0) => return, + Ok(count) => { + chunk.truncate(count); + if sender.send(Ok(chunk)).is_err() { + return; + } + } + Err(error) => { + let _ = sender.send(Err(error)); + return; + } + } + } + }); + receiver +} + /// Returns Python's message for a violation of the protocol that `nvx::host::session` reports. /// /// The two sessions report the same violations of records that have one fault, and their -/// messages differ only where Python folds a case into a broader one. +/// messages differ only where Python folds a case into a broader one, and where Python named the +/// guest agent `managed guest`. fn protocol_message(message: String) -> String { match message.as_str() { "control attach response carried a payload" => { @@ -760,7 +791,10 @@ fn protocol_message(message: String) -> String { "control endpoint returned a truncated application frame" => { "control endpoint returned an invalid data record".to_owned() } - _ => message, + _ => match message.strip_prefix("the guest agent ") { + Some(rest) => format!("managed guest {rest}"), + None => message, + }, } } diff --git a/nvx-rs/src/host/cli/session/tests.rs b/nvx-rs/src/host/cli/session/tests.rs index 5699bbc3a..6deb02a4d 100644 --- a/nvx-rs/src/host/cli/session/tests.rs +++ b/nvx-rs/src/host/cli/session/tests.rs @@ -12,18 +12,33 @@ use crate::host::transport::HostConnection; use crate::protocol::exec::{ EXEC_CWD_PRESENT, EXEC_ENVIRONMENT_PRESENT, EXEC_EXTENDED, EXEC_INHERIT_DEFAULT_ENV, }; +use crate::protocol::v2::{Frame, STDIN_WINDOW, VERSION}; use crate::protocol::{ - OUTER_DATA, OUTER_HEADER_LEN, OUTER_HOST_ATTACH, OUTER_READY, OUTER_RESET, decode_app, - decode_outer_header, encode_app, encode_outer, + OUTER_DATA, OUTER_HEADER_LEN, OUTER_HOST_ATTACH, OUTER_READY, OUTER_RESET, decode_outer_header, + encode_outer, }; const CAPABILITY: [u8; CAPABILITY_LEN] = [7; CAPABILITY_LEN]; const INSTANCE: [u8; 16] = [0x11; 16]; +/// The codes of the kinds of requests that version 2 shares with version 1, as its header holds +/// them, and those of an execution's standard input. +const PING: u8 = 0x01; +const EXEC: u8 = 0x02; +const CANCEL: u8 = 0x04; +const STDIN: u8 = 0x09; +const STDIN_END: u8 = 0x0a; + +/// A `READY` of the protocol version, which acknowledges a request. +const READY: Frame<'static> = Frame::Ready { + version: VERSION, + activation: &[], +}; + impl Session { fn from_connection(connection: HostConnection) -> Self { Self { - inner: Some(ControlSession { connection }), + inner: Some(connection.into()), probe: Arc::new(Probe::default()), } } @@ -36,6 +51,46 @@ impl Session { ) -> Result { Self::connect_unless_exited(endpoint, pid, capability, timeout, || false, None) } + + /// Runs `request` as `exec` does, with a standard input that stays open, as a terminal's does, + /// without Ctrl-C, and discards its output. + fn run(&mut self, request: &ExecRequest, response_timeout: f64) -> Result { + let (_input, receiver) = mpsc::channel(); + self.exec_with( + request, + response_timeout, + || receiver, + || false, + |_, _| Ok(()), + ) + } + + /// Runs `request` as `exec` does, with the standard input that `chunks` hold, until its end, + /// and without Ctrl-C, and returns how it ended and its standard output and error. + fn run_with( + &mut self, + request: &ExecRequest, + response_timeout: f64, + chunks: &[&[u8]], + ) -> (Result, Vec, Vec) { + let (input, receiver) = mpsc::channel(); + for chunk in chunks { + input.send(Ok(chunk.to_vec())).unwrap(); + } + drop(input); + let (mut stdout, mut stderr) = (Vec::new(), Vec::new()); + let result = self.exec_with( + request, + response_timeout, + || receiver, + || false, + |error, data| { + (if error { &mut stderr } else { &mut stdout }).extend_from_slice(data); + Ok(()) + }, + ); + (result, stdout, stderr) + } } /// What a [`Recording`] transport records: the direction and timeout of each transfer. @@ -163,14 +218,19 @@ impl Guest { fn request(&mut self) -> (u8, u64, Vec) { let (record_type, frame) = self.record().unwrap(); assert_eq!(record_type, OUTER_DATA); - let frame = decode_app(&frame).unwrap(); - (frame.kind, frame.request_id, frame.payload.to_vec()) + let (request_id, _) = Frame::decode(&frame).unwrap(); + (frame[5], request_id, frame[24..].to_vec()) + } + + /// Sends `frame` with `request_id`. + fn send(&mut self, request_id: u64, frame: Frame<'_>) { + self.send_raw(&frame.encode(request_id).unwrap()); } - fn send(&mut self, kind: u8, request_id: u64, status: i32, payload: &[u8]) { + /// Sends the bytes of a frame, which may break the protocol. + fn send_raw(&mut self, frame: &[u8]) { self.sequence += 1; - let frame = encode_app(kind, request_id, status, payload); - let record = encode_outer(OUTER_DATA, &INSTANCE, 1, self.sequence, &frame).unwrap(); + let record = encode_outer(OUTER_DATA, &INSTANCE, 1, self.sequence, frame).unwrap(); self.stream.write_all(&record, None).unwrap(); } } @@ -197,21 +257,21 @@ fn an_owned_connection_keeps_its_authenticated_framing_without_another_attach() guest.accept(); for expected in [17, 0] { let (kind, request_id, payload) = guest.request(); - assert_eq!(kind, APP_PING); + assert_eq!(kind, PING); assert!(payload.is_empty()); if expected != 0 { assert_eq!(request_id, expected); } - guest.send(APP_READY, request_id, 0, &[]); + guest.send(request_id, READY); } }); let until = Instant::now() + Duration::from_secs(5); let mut connection = HostConnection::attach(Box::new(host), &CAPABILITY, until).unwrap(); connection - .send(&encode_app(APP_PING, 17, 0, &[]), Some(until)) + .send(&Frame::Ping.encode(17).unwrap(), Some(until)) .unwrap(); let reply = connection.receive(Some(until)).unwrap(); - assert_eq!(decode_app(&reply).unwrap().request_id, 17); + assert_eq!(Frame::decode(&reply).unwrap(), (17, READY)); let mut session = Session::from_connection(connection); session.ping(5.0).unwrap(); worker.join().unwrap(); @@ -256,15 +316,35 @@ fn exec_payload(workload: ExecRequest) -> Vec { let (sender, receiver) = mpsc::channel(); let (mut session, worker) = session_with(move |guest| { let (kind, request_id, payload) = guest.request(); - assert_eq!(kind, APP_EXEC); + assert_eq!(kind, EXEC); sender.send(payload).unwrap(); - guest.send(APP_EXIT, request_id, 0, b"exit"); + guest.send(request_id, exit(0, Outcome::Exit)); }); - session.exec(&workload, 5.0).unwrap(); + session.run(&workload, 5.0).unwrap(); worker.join().unwrap(); receiver.recv().unwrap() } +/// Returns the `EXIT` of an execution that ended with `outcome` and `status`. +fn exit(status: i32, outcome: Outcome) -> Frame<'static> { + Frame::Exit { status, outcome } +} + +/// Returns the bytes of an `EXIT` of `request_id` and `status` whose outcome is `text`, which +/// version 2 may not know. +fn exit_text(request_id: u64, status: i32, text: &[u8]) -> Vec { + let length = u32::try_from(text.len()).unwrap(); + [ + b"NVXC".as_slice(), + &[VERSION, 0x84, 0, 0], + &request_id.to_le_bytes(), + &status.to_le_bytes(), + &length.to_le_bytes(), + text, + ] + .concat() +} + fn u16_at(payload: &[u8], offset: usize) -> u16 { u16::from_le_bytes(payload[offset..offset + 2].try_into().unwrap()) } @@ -288,7 +368,7 @@ fn values(payload: &[u8], mut offset: usize, count: usize) -> (Vec>, usi fn exec_streams_output_and_returns_bounded_status() { let (mut session, worker) = session_with(|guest| { let (kind, request_id, payload) = guest.request(); - assert_eq!(kind, APP_EXEC); + assert_eq!(kind, EXEC); // The legacy request: the timeout, the argument count, and no extension. assert_eq!( ( @@ -298,39 +378,145 @@ fn exec_streams_output_and_returns_bounded_status() { ), (5000, 3, 0) ); - guest.send(APP_STDOUT, request_id, 0, b"hello"); - guest.send(APP_STDERR, request_id, 0, b"warning"); - guest.send(APP_EXIT, request_id, 7, b"exit"); + // The command's standard input is empty, so the workload's ends at once. + assert_eq!(guest.request(), (STDIN_END, request_id, Vec::new())); + guest.send(request_id, Frame::Stdout(b"hello")); + guest.send(request_id, Frame::Stderr(b"warning")); + guest.send(request_id, exit(7, Outcome::Exit)); }); let mut workload = request(&["/bin/sh", "-c", "echo hello"]); workload.timeout_ms = 5000; - let result = session.exec(&workload, 5.0).unwrap(); + let (result, stdout, stderr) = session.run_with(&workload, 5.0, &[]); worker.join().unwrap(); assert_eq!( - result, + result.unwrap(), ExecResult { returncode: 7, category: "exit", - stdout: b"hello".to_vec(), - stderr: b"warning".to_vec(), + stderr_end: Some(b'g'), } ); + assert_eq!((stdout, stderr), (b"hello".to_vec(), b"warning".to_vec())); +} + +#[test] +fn exec_streams_output_past_the_old_cap() { + // Version 1 forwarded at most 1 MiB of output; version 2 streams it whole. + let (mut session, worker) = session_with(|guest| { + let (_, request_id, _) = guest.request(); + assert_eq!(guest.request(), (STDIN_END, request_id, Vec::new())); + let chunk = vec![b'x'; 60_000]; + for _ in 0..18 { + guest.send(request_id, Frame::Stdout(&chunk)); + } + guest.send(request_id, exit(0, Outcome::Exit)); + }); + let (result, stdout, stderr) = session.run_with(&request(&["/bin/true"]), 5.0, &[]); + worker.join().unwrap(); + assert_eq!(result.unwrap().returncode, 0); + assert_eq!(stdout, vec![b'x'; 18 * 60_000]); + assert!(stderr.is_empty()); +} + +#[test] +fn exec_streams_its_standard_input_under_the_window_until_its_end() { + let input: Vec = (0..120 * 1024).map(|index| (index % 251) as u8).collect(); + let (mut session, worker) = session_with(|guest| { + let (_, request_id, _) = guest.request(); + // The guest acknowledges nothing until the window is full, or the input ended. + let (mut received, mut outstanding) = (Vec::new(), 0); + loop { + match guest.request() { + (STDIN, id, bytes) if id == request_id => { + outstanding += bytes.len(); + assert!(outstanding <= STDIN_WINDOW as usize, "{outstanding}"); + received.extend(bytes); + } + (STDIN_END, id, bytes) if id == request_id && bytes.is_empty() => break, + request => panic!("unexpected request {request:?}"), + } + if outstanding == STDIN_WINDOW as usize { + let count = u32::try_from(outstanding).unwrap(); + guest.send(request_id, Frame::StdinAck(count)); + outstanding = 0; + } + } + if outstanding > 0 { + let count = u32::try_from(outstanding).unwrap(); + guest.send(request_id, Frame::StdinAck(count)); + } + for chunk in received.chunks(v2::MAX_PAYLOAD) { + guest.send(request_id, Frame::Stdout(chunk)); + } + guest.send(request_id, exit(0, Outcome::Exit)); + }); + let chunks: Vec<&[u8]> = input.chunks(40 * 1024).collect(); + let (result, stdout, _) = session.run_with(&request(&["/bin/cat"]), 5.0, &chunks); + worker.join().unwrap(); + assert_eq!(result.unwrap().returncode, 0); + assert_eq!(stdout, input); +} + +#[test] +fn ctrl_c_cancels_the_running_workload() { + let (mut session, worker) = session_with(|guest| { + let (_, request_id, _) = guest.request(); + guest.send(request_id, Frame::Stdout(b"started")); + // Ctrl-C after the output: the guest ends the workload, which would outlive the session. + assert_eq!(guest.request(), (CANCEL, request_id, Vec::new())); + }); + let started = AtomicBool::new(false); + let (_input, receiver) = mpsc::channel(); + let result = session.exec_with( + &request(&["/bin/sleep", "60"]), + 5.0, + || receiver, + || started.load(Ordering::SeqCst), + |_, _| { + started.store(true, Ordering::SeqCst); + Ok(()) + }, + ); + assert!(matches!(result, Err(Exit::Interrupted)), "{result:?}"); + worker.join().unwrap(); +} + +#[test] +fn a_failed_read_of_the_standard_input_cancels_the_workload() { + let (mut session, worker) = session_with(|guest| { + let (_, request_id, _) = guest.request(); + assert_eq!(guest.request(), (STDIN, request_id, b"partial".to_vec())); + // No end of the input follows, which would let the workload take what it read as all. + assert_eq!(guest.request(), (CANCEL, request_id, Vec::new())); + }); + let (input, receiver) = mpsc::channel(); + input.send(Ok(b"partial".to_vec())).unwrap(); + input.send(Err(io::Error::other("read failed"))).unwrap(); + let result = session.exec_with( + &request(&["/bin/cat"]), + 5.0, + || receiver, + || false, + |_, _| Ok(()), + ); + assert_eq!(message(result), "cannot read standard input: read failed"); + worker.join().unwrap(); } #[test] fn exec_rejects_unbounded_or_relative_arguments() { let (mut session, worker) = session_with(|_| {}); assert_eq!( - message(session.exec(&request(&["relative"]), 1.0)), + message(session.run(&request(&["relative"]), 1.0)), "managed exec entrypoint must be absolute" ); assert_eq!( - message(session.exec(&request(&["/bin/true"; 65]), 1.0)), + message(session.run(&request(&["/bin/true"; 65]), 1.0)), "managed exec requires 1 through 64 arguments" ); for argument in ["", "a\0b", &"a".repeat(4097)] { assert_eq!( - message(session.exec(&request(&["/bin/true", argument]), 1.0)), + message(session.run(&request(&["/bin/true", argument]), 1.0)), "managed exec argument is empty or exceeds 4096 bytes" ); } @@ -455,7 +641,7 @@ fn exec_rejects_invalid_execution_fields_before_sending() { let mut workload = request(&["/bin/true"]); workload.cwd = Some(cwd.to_owned()); assert_eq!( - failure(session.exec(&workload, 1.0)), + failure(session.run(&workload, 1.0)), ( ErrorCode::Unsupported, "managed exec working directory must be an absolute path of at most 4096 bytes" @@ -480,7 +666,7 @@ fn exec_rejects_invalid_execution_fields_before_sending() { .map(|entry| (*entry).to_owned()) .collect(), ); - assert_eq!(message(session.exec(&workload, 1.0)), expected); + assert_eq!(message(session.run(&workload, 1.0)), expected); } // Nothing reached the guest, which reads the end of the stream. drop(session); @@ -493,7 +679,7 @@ fn exec_rejects_invalid_response_timeout_before_sending() { let (mut session, worker, received) = silent_session(); for timeout in [0.0, -1.0, f64::NAN, f64::INFINITY] { assert_eq!( - message(session.exec(&request(&["/bin/true"]), timeout)), + message(session.run(&request(&["/bin/true"]), timeout)), "managed exec response timeout must be positive and finite" ); } @@ -504,59 +690,67 @@ fn exec_rejects_invalid_response_timeout_before_sending() { #[test] fn exec_rejects_unknown_exit_category() { - // A cancellation, which this session never requests, is not among the categories. - for category in [&b"guest-provided-detail"[..], b"cancelled"] { + // A cancellation, whose end this session never waits for, is not among the categories. + let (mut session, worker) = session_with(|guest| { + let (_, request_id, _) = guest.request(); + guest.send(request_id, exit(137, Outcome::Cancelled)); + }); + assert_eq!( + message(session.run(&request(&["/bin/true"]), 5.0)), + "managed guest returned an unsupported exit category" + ); + worker.join().unwrap(); + // Version 2 knows no other outcome. + for category in [&b"guest-provided-detail"[..], b"\xffexit", b"output-limit"] { let (mut session, worker) = session_with(move |guest| { let (_, request_id, _) = guest.request(); - guest.send(APP_EXIT, request_id, 125, category); + guest.send_raw(&exit_text(request_id, 125, category)); }); assert_eq!( - message(session.exec(&request(&["/bin/true"]), 5.0)), - "managed guest returned an unsupported exit category" + message(session.run(&request(&["/bin/true"]), 5.0)), + "the NVXC frame of version 2 is malformed" ); worker.join().unwrap(); } - let (mut session, worker) = session_with(|guest| { - let (_, request_id, _) = guest.request(); - guest.send(APP_EXIT, request_id, 0, b"\xffexit"); - }); - assert_eq!( - message(session.exec(&request(&["/bin/true"]), 5.0)), - "managed guest returned an invalid exit category" - ); - worker.join().unwrap(); } #[test] fn exec_refusal_keeps_status_category_and_diagnostic() { + let diagnostic = + b"nvx-managed-agent: cannot enter working directory /missing: No such file or directory\n"; let (mut session, worker) = session_with(|guest| { let (_, request_id, _) = guest.request(); - guest.send( - APP_STDERR, - request_id, - 0, - b"nvx-managed-agent: cannot enter working directory /missing: No such file or \ - directory\n", - ); - guest.send(APP_ERROR, request_id, 2, b"cwd-failed"); + assert_eq!(guest.request(), (STDIN_END, request_id, Vec::new())); + guest.send(request_id, Frame::Stderr(diagnostic)); + let refusal = Frame::Error { + status: 2, + category: b"cwd-failed", + }; + guest.send(request_id, refusal); }); let mut workload = request(&["/bin/true"]); workload.cwd = Some("/missing".to_owned()); + let (result, stdout, stderr) = session.run_with(&workload, 5.0, &[]); assert_eq!( - failure(session.exec(&workload, 5.0)), + failure(result), ( ErrorCode::ExecRefused, "managed guest rejected exec (status=2, category=cwd-failed)".to_owned() ) ); + assert_eq!((stdout, stderr), (Vec::new(), diagnostic.to_vec())); worker.join().unwrap(); - // A category that is not ASCII is decoded with replacement characters. + // A category that is not UTF-8 is decoded with replacement characters. let (mut session, worker) = session_with(|guest| { let (_, request_id, _) = guest.request(); - guest.send(APP_ERROR, request_id, 5, b"bad\xff"); + let refusal = Frame::Error { + status: 5, + category: b"bad\xff", + }; + guest.send(request_id, refusal); }); assert_eq!( - message(session.exec(&request(&["/bin/true"]), 5.0)), + message(session.run(&request(&["/bin/true"]), 5.0)), "managed guest rejected exec (status=5, category=bad\u{fffd})" ); worker.join().unwrap(); @@ -639,14 +833,15 @@ impl Transport for Recording { let response = if header.record_type == OUTER_HOST_ATTACH { encode_outer(OUTER_READY, &INSTANCE, 1, 0, &[]).unwrap() } else { - let request = decode_app(&data[OUTER_HEADER_LEN..]).unwrap(); - let (kind, payload) = match request.kind { - APP_PING => (APP_READY, &b""[..]), - APP_EXEC => (APP_EXIT, &b"exit"[..]), - _ => (APP_STOPPED, &b""[..]), + let request = &data[OUTER_HEADER_LEN..]; + let (request_id, _) = Frame::decode(request).unwrap(); + let response = match request[5] { + PING => READY, + EXEC => exit(0, Outcome::Exit), + _ => Frame::Stopped, }; self.sequence += 1; - let frame = encode_app(kind, request.request_id, 0, payload); + let frame = response.encode(request_id).unwrap(); encode_outer(OUTER_DATA, &INSTANCE, 1, self.sequence, &frame).unwrap() }; self.responses.extend(response); @@ -664,7 +859,7 @@ fn operations_bound_their_requests_by_their_deadlines() { let mut operations = vec![(7.0, transfers.lock().unwrap().split_off(0))]; session.ping(5.0).unwrap(); operations.push((5.0, transfers.lock().unwrap().split_off(0))); - session.exec(&request(&["/bin/true"]), 3.0).unwrap(); + session.run(&request(&["/bin/true"]), 3.0).unwrap(); operations.push((3.0, transfers.lock().unwrap().split_off(0))); session.stop(2.0).unwrap(); operations.push((2.0, transfers.lock().unwrap().split_off(0))); @@ -702,7 +897,7 @@ fn failed_transfer_closes_the_session() { ); let count = transfers.lock().unwrap().len(); assert_eq!( - self::failure(session.exec(&request(&["/bin/true"]), 5.0)), + self::failure(session.run(&request(&["/bin/true"]), 5.0)), ( ErrorCode::Protocol, "managed control session is closed".to_owned() @@ -731,7 +926,7 @@ fn request_to_a_stalled_endpoint_times_out_and_closes_the_session() { let mut session = Session::attach(Box::new(host), &CAPABILITY, 5.0, None).unwrap(); let started = Instant::now(); assert_eq!( - message(session.exec(&request(&["/bin/true"]), 0.25)), + message(session.run(&request(&["/bin/true"]), 0.25)), "managed control request timed out" ); let elapsed = started.elapsed(); @@ -1041,64 +1236,11 @@ fn a_closed_endpoint_ends_the_operation_with_its_own_error() { assert!(matches!(failure, Exit::EndpointClosed), "{failure:?}"); } -#[test] -fn output_beyond_the_guest_limit_breaks_the_protocol() { - // The guest agent forwards at most 1 MiB of output, so a guest that sends more misbehaves. - let (mut session, worker) = session_with(|guest| { - let (_, request_id, _) = guest.request(); - let chunk = vec![b'x'; 60_000]; - for _ in 0..18 { - guest.send(APP_STDOUT, request_id, 0, &chunk); - } - guest.send(APP_EXIT, request_id, 0, b"exit"); - }); - assert_eq!( - failure(session.exec(&request(&["/bin/true"]), 5.0)), - ( - ErrorCode::Protocol, - format!("managed guest exceeded the {MAX_OUTPUT_BYTES}-byte output limit") - ) - ); - // The guest may still be sending its exit record, which the host never reads, and a send - // fails once the session is gone, so the session ends after the guest. - worker.join().unwrap(); -} - -#[cfg(feature = "testing")] -#[test] -fn features_of_a_guest_that_predates_them_are_none() { - let (mut session, worker) = session_with(|guest| { - let (kind, request_id, _) = guest.request(); - assert_eq!(kind, APP_FEATURES); - guest.send(APP_ERROR, request_id, 95, UNSUPPORTED_OPERATION); - let (_, request_id, _) = guest.request(); - guest.send( - APP_READY, - request_id, - 0, - &GuestFeatures::SANDBOX_AGENT.encode(), - ); - let (_, request_id, _) = guest.request(); - guest.send(APP_ERROR, request_id, 22, b"invalid-request"); - }); - assert_eq!(session.features(5.0).unwrap(), GuestFeatures::NONE); - assert_eq!(session.features(5.0).unwrap(), GuestFeatures::SANDBOX_AGENT); - assert_eq!( - message(session.features(5.0)), - "managed guest returned an invalid features response" - ); - worker.join().unwrap(); -} - -#[cfg(any( - feature = "testing", - all(feature = "guest", target_os = "linux", target_arch = "x86_64") -))] #[test] fn protocol_violations_keep_the_messages_of_python() { let (mut session, worker) = session_with(|guest| { let (_, request_id, _) = guest.request(); - guest.send(APP_READY, request_id ^ 1, 0, &[]); + guest.send(request_id ^ 1, READY); }); assert_eq!( message(session.ping(5.0)), @@ -1107,43 +1249,36 @@ fn protocol_violations_keep_the_messages_of_python() { worker.join().unwrap(); let (mut session, worker) = session_with(|guest| { let (_, request_id, _) = guest.request(); - guest.send(APP_STDOUT, request_id ^ 1, 0, b"x"); + guest.send(request_id ^ 1, Frame::Stdout(b"x")); }); assert_eq!( - message(session.exec(&request(&["/bin/true"]), 5.0)), + message(session.run(&request(&["/bin/true"]), 5.0)), "managed guest returned a mismatched request ID" ); worker.join().unwrap(); let (mut session, worker) = session_with(|guest| { let (_, request_id, _) = guest.request(); - guest.send(APP_READY, request_id ^ 1, 0, &GuestFeatures::NONE.encode()); + guest.send(request_id, READY); }); assert_eq!( - message(session.features(5.0)), - "managed guest returned a mismatched request ID" + message(session.stop(5.0)), + "managed guest did not acknowledge stop" ); worker.join().unwrap(); + // An acknowledgement has no payload but the protocol version. let (mut session, worker) = session_with(|guest| { let (_, request_id, _) = guest.request(); - guest.send(APP_READY, request_id, 0, b""); + let ready = Frame::Ready { + version: VERSION, + activation: b"x", + }; + guest.send(request_id, ready); }); assert_eq!( - message(session.stop(5.0)), - "managed guest did not acknowledge stop" + message(session.ping(5.0)), + "managed guest did not acknowledge readiness" ); worker.join().unwrap(); - // An acknowledgement has status 0 and no payload. - for (status, payload) in [(1, &b""[..]), (0, b"x")] { - let (mut session, worker) = session_with(move |guest| { - let (_, request_id, _) = guest.request(); - guest.send(APP_READY, request_id, status, payload); - }); - assert_eq!( - message(session.ping(5.0)), - "managed guest did not acknowledge readiness" - ); - worker.join().unwrap(); - } } /// The endpoints of POSIX hosts: Unix-domain sockets that this process serves. @@ -1207,7 +1342,7 @@ mod endpoints { workload .arguments .extend(std::iter::repeat_n("a".repeat(4000), 10)); - let failure = session.exec(&workload, 5.0).unwrap_err(); + let failure = session.run(&workload, 5.0).unwrap_err(); worker.join().unwrap(); assert!(matches!(failure, Exit::EndpointClosed), "{failure:?}"); let (mut session, worker, _directory) = connected(|_| {}); @@ -1490,7 +1625,7 @@ mod endpoints { fn read_reports_a_closed_endpoint_after_its_data() { let (mut session, worker) = connected(|guest| { let (_, request_id, _) = guest.request(); - guest.send(APP_READY, request_id, 0, &[]); + guest.send(request_id, READY); let _ = guest.request(); }); session.ping(5.0).unwrap(); diff --git a/nvx-rs/src/host/cli/snapshots/capture/tests.rs b/nvx-rs/src/host/cli/snapshots/capture/tests.rs index 94c82360f..067f0673b 100644 --- a/nvx-rs/src/host/cli/snapshots/capture/tests.rs +++ b/nvx-rs/src/host/cli/snapshots/capture/tests.rs @@ -16,9 +16,10 @@ use crate::host::process::{BootId, Opened, ProcessRef}; use crate::host::snapshot::tests::{ManifestSpec, SnapshotFixture, context}; use crate::host::snapshot::{MANIFEST_NAME, Snapshot}; use crate::host::spawn::{self, VmmCommand}; +use crate::protocol::v2::{self, Frame}; use crate::protocol::{ - APP_CAPTURE, APP_ERROR, APP_READY, CAPABILITY_LEN, OUTER_DATA, OUTER_HEADER_LEN, - OUTER_HOST_ATTACH, OUTER_READY, decode_app, decode_outer_header, encode_app, encode_outer, + CAPABILITY_LEN, OUTER_DATA, OUTER_HEADER_LEN, OUTER_HOST_ATTACH, OUTER_READY, + decode_outer_header, encode_outer, }; const HELPER: &str = "host::cli::snapshots::capture::tests::capture_helper"; @@ -834,14 +835,21 @@ fn respond(stream: &mut (impl Read + Write), capability: &[u8], refused: bool) { .unwrap(); let (kind, payload) = read_record(stream); assert_eq!(kind, OUTER_DATA); - let request = decode_app(&payload).unwrap(); - assert_eq!(request.kind, APP_CAPTURE); - assert_eq!(request.payload, [0]); + // The capture request of NVXC version 2, with its flag that keeps the executions. + let (request_id, frame) = Frame::decode(&payload).unwrap(); + assert_eq!(frame, Frame::Capture(&[0])); let reply = if refused { - encode_app(APP_ERROR, request.request_id, 22, b"capture-failed") + Frame::Error { + status: 22, + category: b"capture-failed", + } } else { - encode_app(APP_READY, request.request_id, 0, &[]) + Frame::Ready { + version: v2::VERSION, + activation: &[], + } }; + let reply = reply.encode(request_id).unwrap(); stream .write_all(&encode_outer(OUTER_DATA, &[7; 16], 1, 1, &reply).unwrap()) .unwrap(); diff --git a/nvx-rs/src/host/restore/tests.rs b/nvx-rs/src/host/restore/tests.rs index 3e0b9d40f..14b20f739 100644 --- a/nvx-rs/src/host/restore/tests.rs +++ b/nvx-rs/src/host/restore/tests.rs @@ -1,7 +1,14 @@ #![cfg(any(target_os = "linux", windows))] #![cfg_attr(windows, allow(unsafe_code))] -#[cfg(feature = "cli")] +// It speaks version 1 of NVXC, which only the guest agents and the fake OpenVMM build. +#[cfg(all( + feature = "cli", + any( + feature = "testing", + all(feature = "guest", target_os = "linux", target_arch = "x86_64") + ) +))] mod live_init; use std::io::{BufRead, BufReader, Write}; diff --git a/nvx-rs/src/host/restore_driver.rs b/nvx-rs/src/host/restore_driver.rs index e35650aa0..4e1b5c97d 100644 --- a/nvx-rs/src/host/restore_driver.rs +++ b/nvx-rs/src/host/restore_driver.rs @@ -2,9 +2,10 @@ //! //! [`restore`] drives a [`RestoreMachine`] through a launched VM's restore-ready event and //! control sessions until the guest's Ready, then sends the VM's mapping table and starts its -//! workload, and returns the live [`ControlSession`]. The restore-ready client and the control -//! server must be the VM's held process. Each session carries the same activate message: the -//! activation record and the workload's identity. A lost session is replaced, in at most +//! workload, whose standard input it ends at once, and returns the live [`ControlSession`], of +//! NVXC version 2, in which the workload's events arrive. The restore-ready client and the +//! control server must be the VM's held process. Each session carries the same activate message: +//! the activation record and the workload's identity. A lost session is replaced, in at most //! [`MAX_SESSIONS`] sessions in all. //! //! Every pending accept, read and write races the step's deadline, the caller's [`Cancel`] and @@ -38,7 +39,7 @@ use super::transport::Transport; use super::vm::{Capability, Launched, VmOutputs}; use crate::ErrorKind; use crate::activation::ActivationRecord; -use crate::protocol::{self, APP_EXEC, HostMap, workload::WorkloadIdentity}; +use crate::protocol::{self, HostMap, workload::WorkloadIdentity}; /// How long a restore may take, from the driver's start to the guest's Ready. const READY_BUDGET: Duration = Duration::from_secs(60); @@ -240,9 +241,11 @@ fn start( } let payload = protocol::encode_exec_payload(&workload.exec) .map_err(|error| ControlRestoreError::Start(error.into()))?; + // The workload reads no standard input, as the null device gives none. watch .check() - .and_then(|()| session.send_request(APP_EXEC, workload.request_id, &payload, deadline)) + .and_then(|()| session.start_exec(workload.request_id, &payload, deadline)) + .and_then(|()| session.close_stdin(workload.request_id, deadline)) .map_err(|error| ControlRestoreError::Start(watch.cause(error, deadline)))?; Ok(session) } diff --git a/nvx-rs/src/host/restore_driver/tests.rs b/nvx-rs/src/host/restore_driver/tests.rs index 718a7c2be..5abe88072 100644 --- a/nvx-rs/src/host/restore_driver/tests.rs +++ b/nvx-rs/src/host/restore_driver/tests.rs @@ -11,12 +11,13 @@ use crate::activation::{ }; use crate::host::process::Opened; use crate::host::session::ExecEvent; -use crate::protocol::{ExitCategory, WorkloadEnvironment}; +use crate::protocol::WorkloadEnvironment; +use crate::protocol::v2::Outcome; /// A stand-in for OpenVMM and its guest, which serves the control endpoint, sends the /// restore-ready event, attaches the host with the capability of its standard input, and answers -/// NVXC requests as `case` asks, with `activation` in its Ready. It records each request in -/// `trace`, and the step at which it stalls in `stage`. +/// NVXC requests of version 2 as `case` asks, with `activation` in its Ready. It records each +/// request in `trace`, and the step at which it stalls in `stage`. const PEER: &str = r#" import json, os, socket, struct, sys, time case, endpoint, ready, trace, stage, activation = sys.argv[1:] @@ -78,7 +79,7 @@ def record(stream, kind, epoch, sequence, payload=b''): stream.write(struct.pack('<4sHBB16sQQI', b'NVXS', 1, kind, 0, bytes([7]) * 16, epoch, sequence, len(payload)) + payload) def reply(stream, kind, rid, status, payload, epoch, sequence): - frame = struct.pack('<4sBBHQiI', b'NVXC', 1, kind, 0, rid, status, len(payload)) + payload + frame = struct.pack('<4sBBHQiI', b'NVXC', 2, kind, 0, rid, status, len(payload)) + payload record(stream, 5, epoch, sequence, frame) server = None if case == 'connect' else bind() if case == 'stall': @@ -103,7 +104,7 @@ for session in range(1, 4): while True: header = exact(stream, 44) frame = exact(stream, struct.unpack('), /// A chunk of the workload's standard error. Stderr(Vec), + /// The guest agent wrote this many more bytes of the standard input into the workload's, + /// which leave the window. + Acknowledged(u32), /// The workload ended. Exit { /// How it ended. - category: ExitCategory, - /// Its exit status, 128 plus the signal number for [`ExitCategory::Signal`]. + category: Outcome, + /// Its exit status, 128 plus the signal number for [`Outcome::Signal`]. status: i32, }, /// The guest agent refused the workload, and nothing of it runs any longer. Rejected { /// The guest's error number. status: i32, - /// The error category, such as [`LAUNCH_FAILED`](crate::protocol::LAUNCH_FAILED), - /// truncated to 64 characters. + /// The error category, such as `launch-failed`, truncated to 64 characters. category: String, }, } -/// Authenticated control session: `NVXC` frames, each in one DATA record of the host's NVXS -/// connection to OpenVMM's broker. +/// Authenticated control session: `NVXC` frames of version 2, each in one DATA record of the +/// host's NVXS connection to OpenVMM's broker. /// /// Received bytes are buffered until a whole record is available, so a read that times out /// loses nothing and callers can poll with short deadlines. A failure that leaves the stream @@ -146,6 +149,18 @@ pub(crate) enum ExecEvent { pub(crate) struct ControlSession { /// The host's NVXS connection to OpenVMM's control endpoint. pub(crate) connection: HostConnection, + /// The bytes of the running execution's standard input that the host sent and the guest agent + /// has not acknowledged, at most [`STDIN_WINDOW`]. + unacknowledged: u32, +} + +impl From for ControlSession { + fn from(connection: HostConnection) -> Self { + Self { + connection, + unacknowledged: 0, + } + } } impl ControlSession { @@ -158,19 +173,12 @@ impl ControlSession { ) -> Result { let connection = HostConnection::attach(transport, capability, deadline) .map_err(|error| failure(error, Stage::Attach))?; - Ok(Self { connection }) + Ok(connection.into()) } /// Waits until the guest agent acknowledges readiness. - #[cfg_attr( - not(test), - expect( - dead_code, - reason = "SL4.4c's host paths ping the guest through the session" - ) - )] pub(crate) fn ping(&mut self, deadline: Instant) -> Result<(), SessionError> { - self.acknowledged(APP_PING, &[], APP_READY, "readiness", deadline) + self.acknowledged(Frame::Ping, "readiness", deadline) } /// Asks nvx-init to capture the VM, killing its executions first when requested. @@ -179,13 +187,8 @@ impl ControlSession { kill_executions: bool, deadline: Instant, ) -> Result<(), SessionError> { - self.acknowledged( - APP_CAPTURE, - &protocol::encode_capture_payload(kill_executions), - APP_READY, - "capture", - deadline, - ) + let payload = protocol::encode_capture_payload(kill_executions); + self.acknowledged(Frame::Capture(&payload), "capture", deadline) } /// Sends nvx-init's activate message, whose `payload` carries a restore's activation record @@ -196,41 +199,14 @@ impl ControlSession { deadline: Instant, ) -> Result { let request_id = request_id()?; - self.send_app(protocol::APP_ACTIVATE, request_id, payload, Some(deadline))?; - match self.read_reply(request_id, Some(deadline))? { - (APP_READY, 0, ready) => { - crate::activation::ReadyActivation::decode(&ready).map_err(|error| { - SessionError::Protocol(format!("the guest's Ready is invalid: {error}")) - }) - } - _ => Err(SessionError::Protocol( - "the guest agent did not activate the restore".to_owned(), - )), - } - } - - /// Asks the guest agent which control features its image provides. - /// - /// An agent that predates the request refuses it as an unsupported operation, which means - /// that the image provides none of them. - #[cfg_attr( - not(test), - expect( - dead_code, - reason = "no host path sends FEATURES here, and SL4.4d deletes it" - ) - )] - pub(crate) fn features(&mut self, deadline: Instant) -> Result { - let request_id = request_id()?; - self.send_app(APP_FEATURES, request_id, &[], Some(deadline))?; - let (kind, status, payload) = self.read_reply(request_id, Some(deadline))?; - match kind { - APP_READY if status == 0 => Ok(GuestFeatures::decode(&payload)?), - APP_ERROR if payload == UNSUPPORTED_OPERATION => Ok(GuestFeatures::NONE), - _ => Err(SessionError::Protocol( - "the guest agent returned an invalid features response".to_owned(), - )), - } + self.send(request_id, Frame::Activate(payload), Some(deadline))?; + let reply = self.reply(request_id, Some(deadline))?; + let ready = ready(Frame::decode(&reply)?.1)?.ok_or_else(|| { + SessionError::Protocol("the guest agent did not activate the restore".to_owned()) + })?; + crate::activation::ReadyActivation::decode(ready).map_err(|error| { + SessionError::Protocol(format!("the guest's Ready is invalid: {error}")) + }) } /// Hands the guest agent the host mapping table, in as many requests as it takes, and waits @@ -240,25 +216,26 @@ impl ControlSession { maps: &[HostMap], deadline: Instant, ) -> Result<(), SessionError> { - let payloads = protocol::encode_maps_payloads(maps, OUTER_MAX_PAYLOAD - APP_HEADER_LEN)?; - for payload in payloads { + for payload in protocol::encode_maps_payloads(maps, v2::MAX_PAYLOAD)? { let request_id = request_id()?; - self.send_app(APP_MAPS, request_id, &payload, Some(deadline))?; - let (kind, status, payload) = self.read_reply(request_id, Some(deadline))?; - match kind { - APP_READY if status == 0 && payload.is_empty() => {} - APP_ERROR if payload == protocol::MAPPING_FAILED.as_bytes() => { + self.send(request_id, Frame::Maps(&payload), Some(deadline))?; + let reply = self.reply(request_id, Some(deadline))?; + match Frame::decode(&reply)?.1 { + Frame::Error { status, category } + if category == protocol::MAPPING_FAILED.as_bytes() => + { return Err(SessionError::Protocol(format!( "the guest agent could not mount a mapped host path (error {status})" ))); } - APP_ERROR => { - let category = error_category(&payload); + Frame::Error { status, category } => { + let category = error_category(category); return Err(SessionError::Protocol(format!( "the guest agent did not mount the host paths: {category} (status \ {status})" ))); } + frame if ready(frame)?.is_some_and(<[u8]>::is_empty) => {} _ => { return Err(SessionError::Protocol( "the guest agent returned an invalid host mapping response".to_owned(), @@ -269,23 +246,43 @@ impl ControlSession { Ok(()) } - /// Starts a workload and returns the request ID that identifies its events. - #[cfg_attr( - not(test), - expect( - dead_code, - reason = "SL4.4c's sandbox exec and run start workloads through it" - ) - )] + /// Starts the workload that the `EXEC` payload `payload` describes as the execution of + /// `request_id`, which its events repeat, with none of its standard input in the window. pub(crate) fn start_exec( &mut self, - workload: &Workload<'_>, + request_id: u64, + payload: &[u8], deadline: Instant, - ) -> Result { - let payload = protocol::encode_exec_payload(workload)?; - let request_id = request_id()?; - self.send_app(APP_EXEC, request_id, &payload, Some(deadline))?; - Ok(request_id) + ) -> Result<(), SessionError> { + self.unacknowledged = 0; + self.send(request_id, Frame::Exec(payload), Some(deadline)) + } + + /// Sends the next bytes of `data` that the window takes, at most what one frame holds, as + /// standard input of the execution of `request_id`, and returns how many it sent: none + /// while the guest agent has not acknowledged [`STDIN_WINDOW`] bytes. + pub(crate) fn send_stdin( + &mut self, + request_id: u64, + data: &[u8], + deadline: Instant, + ) -> Result { + let room = (STDIN_WINDOW - self.unacknowledged) as usize; + let chunk = &data[..data.len().min(room).min(v2::MAX_PAYLOAD)]; + if !chunk.is_empty() { + self.send(request_id, Frame::Stdin(chunk), Some(deadline))?; + self.unacknowledged += chunk.len() as u32; + } + Ok(chunk.len()) + } + + /// Ends the standard input of the execution of `request_id` after the bytes that it sent. + pub(crate) fn close_stdin( + &mut self, + request_id: u64, + deadline: Instant, + ) -> Result<(), SessionError> { + self.send(request_id, Frame::StdinEnd, Some(deadline)) } /// Reads the next event of the workload started with `request_id`. @@ -294,22 +291,33 @@ impl ControlSession { request_id: u64, deadline: Option, ) -> Result { - let (kind, status, payload) = self.read_reply(request_id, deadline)?; - match kind { - APP_STDOUT => Ok(ExecEvent::Stdout(payload)), - APP_STDERR => Ok(ExecEvent::Stderr(payload)), - APP_EXIT => Ok(ExecEvent::Exit { - category: ExitCategory::parse(&payload)?, + let reply = self.reply(request_id, deadline)?; + Ok(match Frame::decode(&reply)?.1 { + Frame::Stdout(data) => ExecEvent::Stdout(data.to_vec()), + Frame::Stderr(data) => ExecEvent::Stderr(data.to_vec()), + Frame::StdinAck(count) => { + self.unacknowledged = self.unacknowledged.checked_sub(count).ok_or_else(|| { + SessionError::Protocol( + "the guest agent acknowledged standard input that it did not receive" + .to_owned(), + ) + })?; + ExecEvent::Acknowledged(count) + } + Frame::Exit { status, outcome } => ExecEvent::Exit { + category: outcome, status, - }), - APP_ERROR => Ok(ExecEvent::Rejected { + }, + Frame::Error { status, category } => ExecEvent::Rejected { status, - category: error_category(&payload), - }), - _ => Err(SessionError::Protocol( - "the guest agent returned an invalid exec response".to_owned(), - )), - } + category: error_category(category), + }, + _ => { + return Err(SessionError::Protocol( + "the guest agent returned an invalid exec response".to_owned(), + )); + } + }) } /// Asks the guest agent to terminate the workload started with `request_id`. @@ -317,128 +325,111 @@ impl ControlSession { /// The guest reports the workload's outcome as a regular exec event, with the cancelled /// category if the request arrived before the workload exited. A request for a workload that /// already finished is ignored. - #[cfg_attr( - not(test), - expect( - dead_code, - reason = "SL4.4c's sandbox exec and run cancel through it on Ctrl-C" - ) - )] pub(crate) fn cancel_exec( &mut self, request_id: u64, deadline: Instant, ) -> Result<(), SessionError> { - self.send_app(APP_CANCEL, request_id, &[], Some(deadline)) + self.send(request_id, Frame::Cancel, Some(deadline)) } - /// Asks the guest agent to shut the VM down. - #[cfg_attr( - not(test), - expect( - dead_code, - reason = "SL4.4c's sandbox exec and run stop the guest through it" - ) - )] + /// Asks the guest agent to shut the VM down, which first ends an execution that runs: the + /// frames of that execution, which come before the acknowledgement, are skipped. pub(crate) fn stop(&mut self, deadline: Instant) -> Result<(), SessionError> { - self.acknowledged(APP_STOP, &[], APP_STOPPED, "stop", deadline) - } - - /// Sends an application request of `kind` with `request_id` and `payload` in the next - /// sequenced outer record, for a caller that checks the responses itself, as the `nvx-rs` - /// CLI does with the messages of `nvx.py`. - pub(crate) fn send_request( - &mut self, - kind: u8, - request_id: u64, - payload: &[u8], - deadline: Instant, - ) -> Result<(), SessionError> { - self.send_app(kind, request_id, payload, Some(deadline)) - } - - /// Reads the next application frame, checked against the session's identity and sequence, - /// and returns its kind, request ID, status, and payload. - pub(crate) fn read_response( - &mut self, - deadline: Option, - ) -> Result<(u8, u64, i32, Vec), SessionError> { - self.read_app(deadline) + let request_id = request_id()?; + self.send(request_id, Frame::Stop, Some(deadline))?; + loop { + let record = self.receive(Some(deadline))?; + match Frame::decode(&record)? { + (id, Frame::Stopped) if id == request_id => return Ok(()), + ( + id, + Frame::Stdout(_) + | Frame::Stderr(_) + | Frame::StdinAck(_) + | Frame::Exit { .. } + | Frame::Error { .. }, + ) if id != request_id => {} + _ => { + return Err(SessionError::Protocol( + "the guest agent did not acknowledge stop".to_owned(), + )); + } + } + } } - /// Encodes and sends one application frame in the next DATA record. - fn send_app( + /// Encodes `frame` with `request_id`, and sends it in the next DATA record, which the guest + /// agent takes as one frame. + fn send( &mut self, - kind: u8, request_id: u64, - payload: &[u8], + frame: Frame<'_>, deadline: Option, ) -> Result<(), SessionError> { - let frame = protocol::encode_app(kind, request_id, 0, payload); - // The guest agent takes each DATA record as one frame, so a frame must fit one. - if frame.len() > OUTER_MAX_PAYLOAD { - return Err(SessionError::Protocol( - "control payload exceeds the outer protocol limit".to_owned(), - )); - } + let frame = frame.encode(request_id)?; self.connection .send(&frame, deadline) .map_err(|error| failure(error, Stage::Data)) } - /// Reads and decodes the next application frame, the payload of the next DATA record. - fn read_app( - &mut self, - deadline: Option, - ) -> Result<(u8, u64, i32, Vec), SessionError> { - let payload = self - .connection + /// Returns the payload of the next DATA record: the bytes of the guest agent's next frame. + fn receive(&mut self, deadline: Option) -> Result, SessionError> { + self.connection .receive(deadline) - .map_err(|error| failure(error, Stage::Data))?; - let frame = protocol::decode_app(&payload)?; - Ok(( - frame.kind, - frame.request_id, - frame.status, - frame.payload.to_vec(), - )) + .map_err(|error| failure(error, Stage::Data)) } - /// Reads the next application frame, which must answer `request_id`, and returns its kind, - /// status, and payload. - fn read_reply( + /// Returns the bytes of the next frame, which must answer `request_id`. + fn reply( &mut self, request_id: u64, deadline: Option, - ) -> Result<(u8, i32, Vec), SessionError> { - let (kind, response_id, status, payload) = self.read_app(deadline)?; - if response_id != request_id { + ) -> Result, SessionError> { + let record = self.receive(deadline)?; + if Frame::decode(&record)?.0 != request_id { return Err(SessionError::Protocol( "the guest agent returned a mismatched request ID".to_owned(), )); } - Ok((kind, status, payload)) + Ok(record) } - /// Sends a request of `kind` and waits until the guest agent acknowledges - /// `what` with an empty `reply` of status 0. + /// Sends `request` and waits until the guest agent acknowledges `what` with a `READY` of its + /// request ID that carries nothing but the protocol version. fn acknowledged( &mut self, - kind: u8, - payload: &[u8], - reply: u8, + request: Frame<'_>, what: &str, deadline: Instant, ) -> Result<(), SessionError> { let request_id = request_id()?; - self.send_app(kind, request_id, payload, Some(deadline))?; - let (kind, response_id, status, payload) = self.read_app(Some(deadline))?; - if kind != reply || response_id != request_id || status != 0 || !payload.is_empty() { - return Err(SessionError::Protocol(format!( + self.send(request_id, request, Some(deadline))?; + let reply = self.receive(Some(deadline))?; + match Frame::decode(&reply)? { + (id, frame) if id == request_id && ready(frame)?.is_some_and(<[u8]>::is_empty) => { + Ok(()) + } + _ => Err(SessionError::Protocol(format!( "the guest agent did not acknowledge {what}" - ))); + ))), } - Ok(()) + } +} + +/// Returns what a `READY` reports about an activation, once it checked that the `READY` carries +/// the protocol version, or `None` for another frame. +fn ready(frame: Frame<'_>) -> Result, SessionError> { + match frame { + Frame::Ready { + version: v2::VERSION, + activation, + } => Ok(Some(activation)), + Frame::Ready { version, .. } => Err(SessionError::Protocol(format!( + "the guest agent speaks NVXC version {version}, not version {}", + v2::VERSION + ))), + _ => Ok(None), } } diff --git a/nvx-rs/src/host/session/tests.rs b/nvx-rs/src/host/session/tests.rs index 74f63c441..a1960ff3a 100644 --- a/nvx-rs/src/host/session/tests.rs +++ b/nvx-rs/src/host/session/tests.rs @@ -6,7 +6,7 @@ use super::*; use crate::protocol::nvxs::{ErrorCode, Identity}; use crate::protocol::{ OUTER_DATA, OUTER_ERROR, OUTER_HEADER_LEN, OUTER_HOST_ATTACH, OUTER_READY, OUTER_RESET, - OUTER_WAIT, WorkloadEnvironment, decode_app, decode_outer_header, encode_app, encode_outer, + OUTER_WAIT, Workload, WorkloadEnvironment, decode_outer_header, encode_outer, }; #[cfg(any(target_os = "linux", windows))] @@ -14,6 +14,51 @@ mod managed_lifecycle; const INSTANCE: [u8; 16] = [7; 16]; +/// The codes of the kinds of requests that version 2 shares with version 1, as its header holds +/// them. +const PING: u8 = 0x01; +const EXEC: u8 = 0x02; +const STOP: u8 = 0x03; +const CANCEL: u8 = 0x04; +const MAPS: u8 = 0x06; +const CAPTURE: u8 = 0x07; +const ACTIVATE: u8 = 0x08; +const STDIN: u8 = 0x09; +const STDIN_END: u8 = 0x0a; + +/// Returns the bytes of `frame` with `request_id`. +fn encode(request_id: u64, frame: Frame<'_>) -> Vec { + frame.encode(request_id).unwrap() +} + +/// Returns a `READY` of the protocol version for `request_id`, which reports `activation`. +fn ready_frame(request_id: u64, activation: &[u8]) -> Vec { + encode( + request_id, + Frame::Ready { + version: v2::VERSION, + activation, + }, + ) +} + +/// Returns the kind, request ID and payload of a request that the host sent. +fn request(frame: &[u8]) -> (u8, u64, Vec) { + let (request_id, _) = Frame::decode(frame).unwrap(); + (frame[5], request_id, frame[24..].to_vec()) +} + +/// Returns a workload of `argv`, without a timeout, in the default environment. +fn workload(argv: &[&str]) -> Vec { + protocol::encode_exec_payload(&Workload { + argv: argv.iter().map(|&argument| argument.to_owned()).collect(), + timeout_ms: 0, + cwd: None, + environment: WorkloadEnvironment::Default, + }) + .unwrap() +} + #[test] fn the_legacy_connect_wrapper_keeps_indeterminate_process_failures_as_io() { let error = Error::io("cannot verify the VM process").with_source(io::Error::new( @@ -118,12 +163,18 @@ fn ready() -> (ControlSession, Scripted) { /// Kind and payload of each application request that a guest received. type Requests = Arc)>>>; -/// Transport of a guest that answers each application request with `answer(kind, request_id)` -/// and records the kind and payload of each request. +/// The frames with which a guest answers a request, from its kind and request ID. +type Answer = Box Vec> + Send>; + +/// The frame with which a guest answers a request, from its kind and request ID. +type Reply = fn(u8, u64) -> Vec; + +/// Transport of a guest that answers each application request with the frames that `answer` +/// returns for it, each in a record, and records the kind and payload of each request. struct Responder { input: VecDeque, next_sequence: u64, - answer: fn(u8, u64) -> Vec, + answer: Answer, requests: Requests, } @@ -142,15 +193,13 @@ impl Transport for Responder { fn write_all(&mut self, data: &[u8], _timeout: Option) -> io::Result<()> { let header = decode_outer_header(data[..OUTER_HEADER_LEN].try_into().unwrap()).unwrap(); if header.record_type == OUTER_DATA { - let request = decode_app(&data[OUTER_HEADER_LEN..]).unwrap(); - self.requests - .lock() - .unwrap() - .push((request.kind, request.payload.to_vec())); - let reply = (self.answer)(request.kind, request.request_id); - self.input - .extend(outer(OUTER_DATA, 1, self.next_sequence, &reply)); - self.next_sequence += 1; + let (kind, request_id, payload) = request(&data[OUTER_HEADER_LEN..]); + self.requests.lock().unwrap().push((kind, payload)); + for reply in (self.answer)(kind, request_id) { + self.input + .extend(outer(OUTER_DATA, 1, self.next_sequence, &reply)); + self.next_sequence += 1; + } } Ok(()) } @@ -164,6 +213,14 @@ fn attached(answer: fn(u8, u64) -> Vec) -> ControlSession { /// Attaches to a guest that answers each application request with `answer`, and returns the /// kind and payload of each request that the guest receives. fn attached_recording(answer: fn(u8, u64) -> Vec) -> (ControlSession, Requests) { + responding(Box::new(move |kind, request_id| { + vec![answer(kind, request_id)] + })) +} + +/// Attaches to a guest that answers each application request with the frames that `answer` +/// returns, and returns the kind and payload of each request that the guest receives. +fn responding(answer: Answer) -> (ControlSession, Requests) { let requests = Arc::new(Mutex::new(Vec::new())); let guest = Responder { input: outer(OUTER_READY, 1, 0, &[]).into(), @@ -299,55 +356,67 @@ fn attach_responses_that_the_protocol_does_not_allow_fail_closed() { #[test] fn exec_streams_events_in_order() { - // The request ID is random, so the scripted guest answers the frames the host sent. + // The scripted guest answers the frames of the execution that the host started, whose output + // passes the old cap of version 1, 1 MiB, in frames that each fill a record. let (mut session, wire) = ready(); - let workload = Workload { - argv: vec!["/bin/sh".to_owned(), "-c".to_owned(), "echo hi".to_owned()], - timeout_ms: 1000, - cwd: None, - environment: WorkloadEnvironment::Default, - }; - let request_id = session.start_exec(&workload, deadline()).unwrap(); + let payload = workload(&["/bin/sh", "-c", "echo hi"]); + session.start_exec(9, &payload, deadline()).unwrap(); let records = host_records(&wire.output()); let (record_type, sequence, frame) = &records[1]; assert_eq!((*record_type, *sequence), (OUTER_DATA, 0)); - let frame = decode_app(frame).unwrap(); - assert_eq!((frame.kind, frame.request_id), (APP_EXEC, request_id)); + assert_eq!(request(frame), (EXEC, 9, payload)); - wire.supply(&[ - outer( - OUTER_DATA, - 1, - 1, - &encode_app(APP_STDOUT, request_id, 0, b"hi\n"), - ), + let chunk = vec![b'x'; v2::MAX_PAYLOAD]; + let chunks = 1024 * 1024 / v2::MAX_PAYLOAD + 1; + let mut records: Vec> = (1..=chunks) + .map(|sequence| { + outer( + OUTER_DATA, + 1, + sequence as u64, + &encode(9, Frame::Stdout(&chunk)), + ) + }) + .collect(); + let sequence = chunks as u64; + records.extend([ outer( OUTER_DATA, 1, - 2, - &encode_app(APP_STDERR, request_id, 0, b"warn"), + sequence + 1, + &encode(9, Frame::Stderr(b"warn")), ), outer( OUTER_DATA, 1, - 3, - &encode_app(APP_EXIT, request_id, 3, b"exit"), + sequence + 2, + &encode( + 9, + Frame::Exit { + status: 3, + outcome: Outcome::Exit, + }, + ), ), ]); - let events: Vec = (0..3) - .map(|_| { - session - .next_exec_event(request_id, Some(deadline())) - .unwrap() - }) - .collect(); + wire.supply(&records); + let mut stdout = 0; + let events = loop { + match session.next_exec_event(9, Some(deadline())).unwrap() { + ExecEvent::Stdout(data) => stdout += data.len(), + event => { + break [event, session.next_exec_event(9, Some(deadline())).unwrap()]; + } + } + }; + assert_eq!(stdout, chunks * v2::MAX_PAYLOAD); + assert!(stdout > 1024 * 1024); assert_eq!( events, [ - ExecEvent::Stdout(b"hi\n".to_vec()), ExecEvent::Stderr(b"warn".to_vec()), ExecEvent::Exit { - category: ExitCategory::Exit, + category: Outcome::Exit, status: 3 }, ] @@ -361,42 +430,35 @@ fn exec_streams_events_in_order() { #[test] fn exec_sends_the_working_directory_and_reports_its_refusal() { let (mut session, wire) = ready(); - let workload = Workload { + let payload = protocol::encode_exec_payload(&Workload { argv: vec!["/bin/pwd".to_owned()], timeout_ms: 0, cwd: Some("/work"), environment: WorkloadEnvironment::Default, - }; - let request_id = session.start_exec(&workload, deadline()).unwrap(); + }) + .unwrap(); + session.start_exec(5, &payload, deadline()).unwrap(); let records = host_records(&wire.output()); - let frame = decode_app(&records[1].2).unwrap(); - assert_eq!((frame.kind, frame.request_id), (APP_EXEC, request_id)); - assert_eq!( - frame.payload, - protocol::encode_exec_payload(&workload).unwrap() - ); + assert_eq!(request(&records[1].2), (EXEC, 5, payload)); let diagnostic = b"nvx-managed-agent: cannot enter working directory /work: No such file\n"; wire.supply(&[ - outer( - OUTER_DATA, - 1, - 1, - &encode_app(APP_STDERR, request_id, 0, diagnostic), - ), + outer(OUTER_DATA, 1, 1, &encode(5, Frame::Stderr(diagnostic))), outer( OUTER_DATA, 1, 2, - &encode_app(APP_ERROR, request_id, 2, protocol::CWD_FAILED.as_bytes()), + &encode( + 5, + Frame::Error { + status: 2, + category: protocol::CWD_FAILED.as_bytes(), + }, + ), ), ]); let events: Vec = (0..2) - .map(|_| { - session - .next_exec_event(request_id, Some(deadline())) - .unwrap() - }) + .map(|_| session.next_exec_event(5, Some(deadline())).unwrap()) .collect(); assert_eq!( events, @@ -410,13 +472,63 @@ fn exec_sends_the_working_directory_and_reports_its_refusal() { ); } +#[test] +fn standard_input_keeps_at_most_the_window_unacknowledged() { + let (mut session, wire) = ready(); + session + .start_exec(7, &workload(&["/bin/cat"]), deadline()) + .unwrap(); + // A frame holds what a record does, and the window the rest of its 64 KiB. + let data = vec![b'i'; 100 * 1024]; + let first = session.send_stdin(7, &data, deadline()).unwrap(); + assert_eq!(first, v2::MAX_PAYLOAD); + let second = session.send_stdin(7, &data[first..], deadline()).unwrap(); + assert_eq!(first + second, STDIN_WINDOW as usize); + assert_eq!(session.send_stdin(7, &data, deadline()).unwrap(), 0); + // An acknowledgement returns its bytes to the window, and no more than it. + wire.supply(&[outer(OUTER_DATA, 1, 1, &encode(7, Frame::StdinAck(1000)))]); + assert_eq!( + session.next_exec_event(7, Some(deadline())).unwrap(), + ExecEvent::Acknowledged(1000) + ); + assert_eq!(session.send_stdin(7, &data, deadline()).unwrap(), 1000); + assert_eq!(session.send_stdin(7, &data, deadline()).unwrap(), 0); + session.close_stdin(7, deadline()).unwrap(); + let frames: Vec<_> = host_records(&wire.output())[2..] + .iter() + .map(|(_, _, frame)| request(frame)) + .collect(); + assert_eq!( + frames, + [ + (STDIN, 7, data[..first].to_vec()), + (STDIN, 7, data[..second].to_vec()), + (STDIN, 7, data[..1000].to_vec()), + (STDIN_END, 7, Vec::new()), + ] + ); + // A new execution starts with its whole window, and the guest agent acknowledges no bytes + // that the host did not send. + session + .start_exec(8, &workload(&["/bin/cat"]), deadline()) + .unwrap(); + assert_eq!(session.send_stdin(8, &data[..10], deadline()).unwrap(), 10); + wire.supply(&[outer(OUTER_DATA, 1, 2, &encode(8, Frame::StdinAck(11)))]); + assert_violation( + &session.next_exec_event(8, Some(deadline())).unwrap_err(), + "the guest agent acknowledged standard input that it did not receive", + "an acknowledgement beyond the bytes sent", + ); +} + #[test] fn records_that_the_protocol_does_not_allow_fail_closed() { - let frame = encode_app(APP_STDOUT, 1, 0, b"x"); + let frame = encode(1, Frame::Stdout(b"x")); let data = outer(OUTER_DATA, 1, 1, &frame); let oversized = u32::try_from(OUTER_MAX_PAYLOAD + 1).unwrap().to_le_bytes(); let invalid = "control endpoint returned an invalid data record"; let truncated = "control endpoint returned a truncated application frame"; + let malformed = "the NVXC frame of version 2 is malformed"; let cases = [ ( "DATA out of sequence", @@ -452,8 +564,13 @@ fn records_that_the_protocol_does_not_allow_fail_closed() { ), ( "a truncated frame", - outer(OUTER_DATA, 1, 1, &frame[..APP_HEADER_LEN - 1]), - truncated, + outer(OUTER_DATA, 1, 1, &frame[..23]), + malformed, + ), + ( + "a frame of version 1", + outer(OUTER_DATA, 1, 1, &patched(frame.clone(), 4, &[1])), + "the control peer sent a frame of another protocol than NVXC version 2", ), ( "nonzero flags", @@ -507,7 +624,11 @@ fn records_that_the_protocol_does_not_allow_fail_closed() { #[test] fn timed_out_polls_keep_partial_records() { let (mut session, wire) = ready(); - let record = outer(OUTER_DATA, 1, 1, &encode_app(APP_EXIT, 9, 0, b"cancelled")); + let exit = Frame::Exit { + status: 0, + outcome: Outcome::Cancelled, + }; + let record = outer(OUTER_DATA, 1, 1, &encode(9, exit)); let (first, second) = record.split_at(30); wire.supply(&[first.to_vec()]); assert!(matches!( @@ -519,107 +640,241 @@ fn timed_out_polls_keep_partial_records() { assert_eq!( session.next_exec_event(9, Some(deadline())).unwrap(), ExecEvent::Exit { - category: ExitCategory::Cancelled, + category: Outcome::Cancelled, status: 0 } ); let records = host_records(&wire.output()); - let frame = decode_app(&records.last().unwrap().2).unwrap(); - assert_eq!((frame.kind, frame.request_id), (APP_CANCEL, 9)); + assert_eq!(request(&records.last().unwrap().2), (CANCEL, 9, Vec::new())); } #[test] fn frames_travel_in_one_record_each() { let (mut session, wire) = ready(); - let longest = vec![0x5a; OUTER_MAX_PAYLOAD - APP_HEADER_LEN]; - session - .send_request(APP_EXEC, 1, &longest, deadline()) - .unwrap(); + let longest = vec![0x5a; v2::MAX_PAYLOAD]; + session.start_exec(1, &longest, deadline()).unwrap(); let error = session - .send_request( - APP_EXEC, - 2, - &[longest.as_slice(), &[0]].concat(), - deadline(), - ) + .start_exec(2, &[longest.as_slice(), &[0]].concat(), deadline()) .unwrap_err(); - let message = "control payload exceeds the outer protocol limit"; + let message = "an NVXC frame must fit one outer data record"; assert_violation(&error, message, "a frame longer than a record"); let records = host_records(&wire.output()); assert_eq!(records.len(), 2); let (record_type, sequence, frame) = &records[1]; assert_eq!((*record_type, *sequence), (OUTER_DATA, 0)); - let frame = decode_app(frame).unwrap(); - assert_eq!((frame.kind, frame.request_id), (APP_EXEC, 1)); - assert_eq!(frame.payload, longest); + assert_eq!(request(frame), (EXEC, 1, longest)); // The refused frame changed nothing. session.cancel_exec(1, deadline()).unwrap(); let records = host_records(&wire.output()); assert_eq!((records[2].0, records[2].1), (OUTER_DATA, 1)); } -/// The response kind that acknowledges a ping or a stop request. -fn acknowledgement(kind: u8) -> u8 { - if kind == APP_PING { - APP_READY +/// Returns the frame that acknowledges a ping or a stop request of `request_id`. +fn acknowledgement(kind: u8, request_id: u64) -> Vec { + if kind == PING { + ready_frame(request_id, &[]) } else { - APP_STOPPED + encode(request_id, Frame::Stopped) } } #[test] fn pings_and_stops_need_an_empty_acknowledgement() { - let (mut session, requests) = - attached_recording(|kind, id| encode_app(acknowledgement(kind), id, 0, &[])); + let (mut session, requests) = attached_recording(acknowledgement); session.ping(deadline()).unwrap(); session.stop(deadline()).unwrap(); assert_eq!( *requests.lock().unwrap(), - [(APP_PING, Vec::new()), (APP_STOP, Vec::new())] + [(PING, Vec::new()), (STOP, Vec::new())] ); - let answers: [fn(u8, u64) -> Vec; 4] = [ - |_, id| encode_app(APP_ERROR, id, 0, &[]), - |kind, id| encode_app(acknowledgement(kind), id ^ 1, 0, &[]), - |kind, id| encode_app(acknowledgement(kind), id, 1, &[]), - |kind, id| encode_app(acknowledgement(kind), id, 0, b"x"), + let readiness = "the guest agent did not acknowledge readiness"; + let stop = "the guest agent did not acknowledge stop"; + let malformed = "the NVXC frame of version 2 is malformed"; + // A refusal, another request's acknowledgement, one with a status, and one of the other + // request or with a payload. + let answers: [(Reply, &str, &str); 4] = [ + (|_, id| encode(id, Frame::Busy), readiness, stop), + (|kind, id| acknowledgement(kind, id ^ 1), readiness, stop), + ( + |kind, id| patched(acknowledgement(kind, id), 16, &[1]), + malformed, + malformed, + ), + ( + |kind, id| { + if kind == PING { + ready_frame(id, b"x") + } else { + ready_frame(id, &[]) + } + }, + readiness, + stop, + ), ]; - for answer in answers { + for (answer, pinged, stopped) in answers { let error = attached(answer).ping(deadline()).unwrap_err(); - assert_eq!( - error.to_string(), - "the guest agent did not acknowledge readiness" - ); + assert_violation(&error, pinged, "a ping"); let error = attached(answer).stop(deadline()).unwrap_err(); - assert_eq!( - error.to_string(), - "the guest agent did not acknowledge stop" - ); + assert_violation(&error, stopped, "a stop"); + } +} + +#[test] +fn ready_must_carry_the_protocol_version() { + let maps = [HostMap { + source: "0".to_owned(), + target: "/work".to_owned(), + read_only: true, + }]; + let answers: [(Reply, u8); 2] = [ + ( + |_, id| { + let frame = Frame::Ready { + version: 1, + activation: &[], + }; + encode(id, frame) + }, + 1, + ), + ( + |_, id| { + let frame = Frame::Ready { + version: 3, + activation: &[], + }; + encode(id, frame) + }, + 3, + ), + ]; + for (answer, version) in answers { + let message = format!("the guest agent speaks NVXC version {version}, not version 2"); + let error = attached(answer).ping(deadline()).unwrap_err(); + assert_violation(&error, &message, "a ping"); + let error = attached(answer).capture(false, deadline()).unwrap_err(); + assert_violation(&error, &message, "a capture"); + let error = attached(answer) + .map_host_paths(&maps, deadline()) + .unwrap_err(); + assert_violation(&error, &message, "a mapping table"); + let error = attached(answer) + .activate(b"activation", deadline()) + .unwrap_err(); + assert_violation(&error, &message, "an activation"); } + // A guest agent that predates version 2 answers in version 1's frames. + let error = attached(|_, id| { + [ + b"NVXC".as_slice(), + &[1, 0x81, 0, 0], + &id.to_le_bytes(), + &[0; 8], + ] + .concat() + }) + .ping(deadline()) + .unwrap_err(); + let message = "the control peer sent a frame of another protocol than NVXC version 2"; + assert_violation(&error, message, "a READY of version 1"); +} + +#[test] +fn a_stop_ends_the_execution_that_runs_and_skips_its_frames() { + // The guest agent ends the execution, whose output and end come before the acknowledgement. + let (mut session, requests) = responding(Box::new(|kind, id| match kind { + STOP => vec![ + encode(4, Frame::Stdout(b"partial")), + encode(4, Frame::StdinAck(1)), + encode( + 4, + Frame::Exit { + status: 137, + outcome: Outcome::Cancelled, + }, + ), + encode(id, Frame::Stopped), + ], + _ => Vec::new(), + })); + session + .start_exec(4, &workload(&["/bin/cat"]), deadline()) + .unwrap(); + assert_eq!(session.send_stdin(4, b"i", deadline()).unwrap(), 1); + session.stop(deadline()).unwrap(); + let kinds: Vec = requests + .lock() + .unwrap() + .iter() + .map(|(kind, _)| *kind) + .collect(); + assert_eq!(kinds, [EXEC, STDIN, STOP]); + // An execution that the guest agent refused ends with its error, which comes before the + // acknowledgement too. + let (mut session, _) = responding(Box::new(|kind, id| match kind { + STOP => vec![ + encode(5, Frame::Stderr(b"cannot enter /missing")), + encode( + 5, + Frame::Error { + status: 2, + category: b"cwd-failed", + }, + ), + encode(id, Frame::Stopped), + ], + _ => Vec::new(), + })); + session + .start_exec(5, &workload(&["/bin/pwd"]), deadline()) + .unwrap(); + session.stop(deadline()).unwrap(); + // A frame of the stop's own ID other than its acknowledgement is refused, its refusal too. + let (mut session, _) = responding(Box::new(|_, id| vec![encode(id, Frame::Stdout(b"x"))])); + assert_violation( + &session.stop(deadline()).unwrap_err(), + "the guest agent did not acknowledge stop", + "output of the stop's own ID", + ); + let (mut session, _) = responding(Box::new(|_, id| { + vec![encode( + id, + Frame::Error { + status: 1, + category: b"invalid-request", + }, + )] + })); + assert_violation( + &session.stop(deadline()).unwrap_err(), + "the guest agent did not acknowledge stop", + "a refusal of the stop", + ); } #[test] fn capture_sends_both_kill_flags_and_waits_for_acknowledgement() { let (mut session, requests) = attached_recording(|kind, id| { - assert_eq!(kind, APP_CAPTURE); + assert_eq!(kind, CAPTURE); assert_ne!(id, 0); - encode_app(APP_READY, id, 0, &[]) + ready_frame(id, &[]) }); session.capture(false, deadline()).unwrap(); session.capture(true, deadline()).unwrap(); assert_eq!( *requests.lock().unwrap(), - [(APP_CAPTURE, vec![0]), (APP_CAPTURE, vec![1])] + [(CAPTURE, vec![0]), (CAPTURE, vec![1])] ); } #[test] fn capture_refuses_invalid_acknowledgements() { - let answers: [fn(u8, u64) -> Vec; 5] = [ - |_, id| encode_app(APP_ERROR, id, 16, b"busy"), - |_, id| encode_app(APP_STOPPED, id, 0, &[]), - |_, id| encode_app(APP_READY, id ^ 1, 0, &[]), - |_, id| encode_app(APP_READY, id, 1, &[]), - |_, id| encode_app(APP_READY, id, 0, b"x"), + let answers: [fn(u8, u64) -> Vec; 4] = [ + |_, id| encode(id, Frame::Busy), + |_, id| encode(id, Frame::Stopped), + |_, id| ready_frame(id ^ 1, &[]), + |_, id| ready_frame(id, b"x"), ]; for answer in answers { let error = attached(answer).capture(false, deadline()).unwrap_err(); @@ -629,14 +884,23 @@ fn capture_refuses_invalid_acknowledgements() { "an invalid capture acknowledgement", ); } + // Version 2 has no status in a READY. + let error = attached(|_, id| patched(ready_frame(id, &[]), 16, &[1])) + .capture(false, deadline()) + .unwrap_err(); + assert_violation( + &error, + "the NVXC frame of version 2 is malformed", + "a READY with a status", + ); } #[cfg(feature = "cli")] #[test] fn activate_sends_its_payload_and_returns_the_activation_that_ready_reports() { let (mut session, requests) = attached_recording(|kind, id| { - assert_eq!(kind, protocol::APP_ACTIVATE); - encode_app(APP_READY, id, 0, &[[1].as_slice(), &[9; 16]].concat()) + assert_eq!(kind, ACTIVATE); + ready_frame(id, &[[1].as_slice(), &[9; 16]].concat()) }); let ready = session.activate(b"activation", deadline()).unwrap(); assert_eq!( @@ -645,7 +909,7 @@ fn activate_sends_its_payload_and_returns_the_activation_that_ready_reports() { ); assert_eq!( *requests.lock().unwrap(), - [(protocol::APP_ACTIVATE, b"activation".to_vec())] + [(ACTIVATE, b"activation".to_vec())] ); } @@ -653,10 +917,16 @@ fn activate_sends_its_payload_and_returns_the_activation_that_ready_reports() { #[test] fn activate_refuses_a_reply_other_than_a_valid_ready() { let answers: [fn(u8, u64) -> Vec; 4] = [ - |_, id| encode_app(APP_ERROR, id, 22, b"invalid-request"), - |_, id| encode_app(APP_READY, id ^ 1, 0, &[0]), - |_, id| encode_app(APP_READY, id, 1, &[0]), - |_, id| encode_app(APP_READY, id, 0, &[2]), + |_, id| { + let frame = Frame::Error { + status: 22, + category: b"invalid-request", + }; + encode(id, frame) + }, + |_, id| ready_frame(id ^ 1, &[0]), + |_, id| patched(ready_frame(id, &[0]), 16, &[1]), + |_, id| ready_frame(id, &[2]), ]; for answer in answers { let error = attached(answer) @@ -666,49 +936,11 @@ fn activate_refuses_a_reply_other_than_a_valid_ready() { } } -#[cfg(all(feature = "guest", target_os = "linux", target_arch = "x86_64"))] -#[test] -fn features_report_what_the_guest_provides() { - let mut session = attached(|kind, id| { - assert_eq!(kind, APP_FEATURES); - encode_app(APP_READY, id, 0, &GuestFeatures::REQUIRED.encode()) - }); - assert_eq!( - session.features(deadline()).unwrap(), - GuestFeatures::REQUIRED - ); -} - -#[test] -fn guests_that_predate_the_features_request_provide_none() { - // This is how an agent without the request refuses an unknown request kind. - let mut session = attached(|_, id| encode_app(APP_ERROR, id, 95, b"unsupported-operation")); - assert_eq!(session.features(deadline()).unwrap(), GuestFeatures::NONE); -} - -#[test] -fn malformed_features_responses_fail_closed() { - let answers: [fn(u8, u64) -> Vec; 5] = [ - |_, id| encode_app(APP_READY, id, 0, &[1, 0]), - |_, id| encode_app(APP_ERROR, id, 16, b"busy"), - |_, id| encode_app(APP_READY, id, 1, &[0; 4]), - |_, id| encode_app(APP_READY, id ^ 1, 0, &[0; 4]), - |_, id| encode_app(APP_STDOUT, id, 0, &[0; 4]), - ]; - for answer in answers { - let mut session = attached(answer); - assert!(matches!( - session.features(deadline()), - Err(SessionError::Protocol(_)) - )); - } -} - #[test] fn host_mappings_travel_in_records_that_the_guest_acknowledges() { let (mut session, requests) = attached_recording(|kind, id| { - assert_eq!(kind, APP_MAPS); - encode_app(APP_READY, id, 0, &[]) + assert_eq!(kind, MAPS); + ready_frame(id, &[]) }); let target = format!("/{}", "t".repeat(4000)); let maps: Vec = (0..40) @@ -740,7 +972,13 @@ fn refused_host_mappings_fail_the_session() { target: "/work".to_owned(), read_only: false, }]; - let mut session = attached(|_, id| encode_app(APP_ERROR, id, 2, b"mapping-failed")); + let mut session = attached(|_, id| { + let frame = Frame::Error { + status: 2, + category: b"mapping-failed", + }; + encode(id, frame) + }); let error = session.map_host_paths(&maps, deadline()).unwrap_err(); assert!( error @@ -748,11 +986,24 @@ fn refused_host_mappings_fail_the_session() { .contains("could not mount a mapped host path (error 2)"), "{error}" ); - let answers: [fn(u8, u64) -> Vec; 4] = [ - |_, id| encode_app(APP_ERROR, id, 22, b"invalid-request"), - |_, id| encode_app(APP_ERROR, id, 95, b"unsupported-operation"), - |_, id| encode_app(APP_READY, id, 0, b"x"), - |_, id| encode_app(APP_READY, id ^ 1, 0, &[]), + let answers: [fn(u8, u64) -> Vec; 5] = [ + |_, id| { + let frame = Frame::Error { + status: 22, + category: b"invalid-request", + }; + encode(id, frame) + }, + |_, id| { + let frame = Frame::Error { + status: 95, + category: b"unsupported-operation", + }; + encode(id, frame) + }, + |_, id| encode(id, Frame::Busy), + |_, id| ready_frame(id, b"x"), + |_, id| ready_frame(id ^ 1, &[]), ]; for answer in answers { let mut session = attached(answer); @@ -766,29 +1017,20 @@ fn refused_host_mappings_fail_the_session() { #[test] fn responses_to_another_request_fail_closed() { let mismatched = "the guest agent returned a mismatched request ID"; - let answer: fn(u8, u64) -> Vec = |_, id| encode_app(APP_READY, id ^ 1, 0, &[]); - let error = attached(answer).features(deadline()).unwrap_err(); - assert_eq!(error.to_string(), mismatched); let maps = [HostMap { source: "0".to_owned(), target: "/work".to_owned(), read_only: true, }]; - let error = attached(answer) + let error = attached(|_, id| ready_frame(id ^ 1, &[])) .map_host_paths(&maps, deadline()) .unwrap_err(); assert_eq!(error.to_string(), mismatched); - let mut session = attached(|_, id| encode_app(APP_STDOUT, id ^ 1, 0, b"x")); - let workload = Workload { - argv: vec!["/bin/true".to_owned()], - timeout_ms: 0, - cwd: None, - environment: WorkloadEnvironment::Default, - }; - let request_id = session.start_exec(&workload, deadline()).unwrap(); - let error = session - .next_exec_event(request_id, Some(deadline())) - .unwrap_err(); + let mut session = attached(|_, id| encode(id ^ 1, Frame::Stdout(b"x"))); + session + .start_exec(6, &workload(&["/bin/true"]), deadline()) + .unwrap(); + let error = session.next_exec_event(6, Some(deadline())).unwrap_err(); assert_eq!(error.to_string(), mismatched); } diff --git a/nvx-rs/src/host/session/tests/managed_lifecycle.rs b/nvx-rs/src/host/session/tests/managed_lifecycle.rs index c0f0c67a4..c52b1b557 100644 --- a/nvx-rs/src/host/session/tests/managed_lifecycle.rs +++ b/nvx-rs/src/host/session/tests/managed_lifecycle.rs @@ -30,16 +30,15 @@ use std::io::{self, Write}; use std::net::{SocketAddr, TcpStream}; use std::path::{Path, PathBuf}; use std::process::{Child, Command, Stdio}; +use std::sync::atomic::{AtomicU64, Ordering}; use std::sync::mpsc; use std::thread; use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH}; use crate::host::platform::process_start_time; use crate::host::session::{self, ControlSession, ExecEvent}; -use crate::protocol::{ - self, APP_ERROR, APP_MAPS, APP_READY, CAPABILITY_LEN, ExitCategory, HostMap, Workload, - WorkloadEnvironment, -}; +use crate::protocol::v2::{self, Frame}; +use crate::protocol::{self, CAPABILITY_LEN, HostMap, Workload, WorkloadEnvironment}; /// The categories with which the guest refuses a working directory that the workload cannot /// enter, a malformed request, and a workload before it applied every host mapping: NVXC's wire @@ -91,7 +90,7 @@ impl Drop for OpenVmm { #[derive(Debug, Default)] struct Outcome { /// How it exited, and its status, if it ran. - exit: Option<(ExitCategory, i32)>, + exit: Option<(v2::Outcome, i32)>, /// The guest's error number and category, if the guest refused it. refusal: Option<(i32, String)>, /// Its standard output. @@ -103,12 +102,18 @@ struct Outcome { impl Outcome { /// Returns whether the workload exited with status 0 and printed exactly `stdout`. fn printed(&self, stdout: &str) -> bool { - self.exit == Some((ExitCategory::Exit, 0)) + self.exit == Some((v2::Outcome::Exit, 0)) && self.stdout == stdout.as_bytes() && self.stderr.is_empty() } } +/// Returns a request ID that no earlier request of the test took. +fn request_id() -> u64 { + static NEXT: AtomicU64 = AtomicU64::new(1); + NEXT.fetch_add(1, Ordering::Relaxed) +} + /// The control session of the guest that OpenVMM `pid` runs. struct Guest { /// The control endpoint. @@ -133,7 +138,7 @@ impl Guest { .expect("the control endpoint accepts the capability") } - /// Runs `argv` in `session` and returns how it ended. + /// Runs `argv` in `session`, which ends its standard input at once, and returns how it ended. fn exec( &self, session: &mut ControlSession, @@ -150,8 +155,11 @@ impl Guest { }; // The longest wait bounds the whole exchange, so the workloads must end well before it. let deadline = Instant::now() + self.timeout; - let request = session - .start_exec(&workload, deadline) + let request = request_id(); + let payload = protocol::encode_exec_payload(&workload).expect("the workload fits a frame"); + session + .start_exec(request, &payload, deadline) + .and_then(|()| session.close_stdin(request, deadline)) .expect("the guest accepts the request"); let mut outcome = Outcome::default(); loop { @@ -161,6 +169,7 @@ impl Guest { { ExecEvent::Stdout(chunk) => outcome.stdout.extend(chunk), ExecEvent::Stderr(chunk) => outcome.stderr.extend(chunk), + ExecEvent::Acknowledged(count) => panic!("{argv:?} acknowledged {count} bytes"), ExecEvent::Exit { category, status } => { outcome.exit = Some((category, status)); return outcome; @@ -395,14 +404,21 @@ fn layered_mapping_session( let send = |session: &mut ControlSession, payload: &[u8], accepted| { let deadline = Instant::now() + guest.timeout; session - .send_request(APP_MAPS, 900, payload, deadline) + .send(900, Frame::Maps(payload), Some(deadline)) .unwrap(); + let reply = session.receive(Some(deadline)).unwrap(); let expected = if accepted { - (APP_READY, 900, 0, Vec::new()) + Frame::Ready { + version: v2::VERSION, + activation: &[], + } } else { - (APP_ERROR, 900, 22, INVALID_REQUEST.as_bytes().to_vec()) + Frame::Error { + status: 22, + category: INVALID_REQUEST.as_bytes(), + } }; - assert_eq!(session.read_response(Some(deadline)).unwrap(), expected); + assert_eq!(Frame::decode(&reply).unwrap(), (900, expected)); }; if count == 0 { send(&mut session, &parts[0], false); @@ -461,16 +477,16 @@ fn layered_mapping_session( ); assert!(mapped.printed("sl76b-real-export\n"), "{path}: {mapped:?}"); } - let request = session - .start_exec( - &Workload { - argv: vec!["/bin/sleep".to_owned(), "30".to_owned()], - timeout_ms: 0, - cwd: None, - environment: WorkloadEnvironment::Default, - }, - Instant::now() + guest.timeout, - ) + let request = request_id(); + let payload = protocol::encode_exec_payload(&Workload { + argv: vec!["/bin/sleep".to_owned(), "30".to_owned()], + timeout_ms: 0, + cwd: None, + environment: WorkloadEnvironment::Default, + }) + .unwrap(); + session + .start_exec(request, &payload, Instant::now() + guest.timeout) .unwrap(); session .cancel_exec(request, Instant::now() + guest.timeout) @@ -480,7 +496,7 @@ fn layered_mapping_session( .next_exec_event(request, Some(Instant::now() + guest.timeout)) .unwrap(), ExecEvent::Exit { - category: ExitCategory::Cancelled, + category: v2::Outcome::Cancelled, status: 137 } )); @@ -647,7 +663,7 @@ fn a_managed_guest_runs_request_scoped_workloads_until_it_stops() { ); assert_eq!( timed_out.exit, - Some((ExitCategory::Timeout, 124)), + Some((v2::Outcome::Timeout, 124)), "{timed_out:?}" ); let after_timeout = guest.exec( diff --git a/nvx-rs/src/protocol/app.rs b/nvx-rs/src/protocol/app.rs index 6ca904452..3c267741a 100644 --- a/nvx-rs/src/protocol/app.rs +++ b/nvx-rs/src/protocol/app.rs @@ -6,12 +6,17 @@ //! of their request. This module holds version 1, and `v2` version 2, which carries every control //! path and streams an execution's standard input, output and error. +#[cfg(any( + feature = "testing", + all(feature = "guest", target_os = "linux", target_arch = "x86_64") +))] +use super::array; #[cfg(any( feature = "cli", feature = "testing", all(feature = "guest", target_os = "linux", target_arch = "x86_64") ))] -use super::{ProtocolError, array, violation}; +use super::{ProtocolError, violation}; #[cfg(any( feature = "cli", all(feature = "guest", target_os = "linux", target_arch = "x86_64") @@ -44,6 +49,7 @@ pub(crate) mod workload { } #[cfg(any( + feature = "cli", feature = "testing", all(feature = "guest", target_os = "linux", target_arch = "x86_64") ))] @@ -51,14 +57,12 @@ pub(crate) mod v2; /// Magic of every application frame. #[cfg(any( - feature = "cli", feature = "testing", all(feature = "guest", target_os = "linux", target_arch = "x86_64") ))] pub(crate) const APP_MAGIC: [u8; 4] = *b"NVXC"; /// Version of the application frame format. #[cfg(any( - feature = "cli", feature = "testing", all(feature = "guest", target_os = "linux", target_arch = "x86_64") ))] @@ -73,28 +77,24 @@ pub(crate) const APP_HEADER_LEN: usize = 24; /// Request that the guest agent acknowledge readiness with [`APP_READY`]. #[cfg(any( - feature = "cli", feature = "testing", all(feature = "guest", target_os = "linux", target_arch = "x86_64") ))] pub(crate) const APP_PING: u8 = 1; /// Request that the guest agent run a workload; the payload is an `EXEC` payload. #[cfg(any( - feature = "cli", feature = "testing", all(feature = "guest", target_os = "linux", target_arch = "x86_64") ))] pub(crate) const APP_EXEC: u8 = 2; /// Request that the guest agent shut the VM down, which it acknowledges with [`APP_STOPPED`]. #[cfg(any( - feature = "cli", feature = "testing", all(feature = "guest", target_os = "linux", target_arch = "x86_64") ))] pub(crate) const APP_STOP: u8 = 3; /// Request that the guest agent terminate the workload of the request with the frame's ID. #[cfg(any( - feature = "cli", feature = "testing", all(feature = "guest", target_os = "linux", target_arch = "x86_64") ))] @@ -102,7 +102,6 @@ pub(crate) const APP_CANCEL: u8 = 4; /// Request for the guest's control features, which the guest agent answers with [`APP_READY`] /// and a [`GuestFeatures`](super::GuestFeatures) payload. #[cfg(any( - feature = "cli", feature = "testing", all(feature = "guest", target_os = "linux", target_arch = "x86_64") ))] @@ -110,40 +109,30 @@ pub(crate) const APP_FEATURES: u8 = 5; /// Request that the guest agent mount entries of the host mapping table; the payload is a `MAPS` /// payload. #[cfg(any( - feature = "cli", feature = "testing", all(feature = "guest", target_os = "linux", target_arch = "x86_64") ))] pub(crate) const APP_MAPS: u8 = 6; /// Request that nvx-init capture the VM after optionally killing its executions. -#[cfg(any( - feature = "cli", - all(feature = "guest", target_os = "linux", target_arch = "x86_64") -))] +#[cfg(all(feature = "guest", target_os = "linux", target_arch = "x86_64"))] pub(crate) const APP_CAPTURE: u8 = 7; /// Request that nvx-init activate a restored VM with its record and workload identity. -#[cfg(any( - feature = "cli", - all(feature = "guest", target_os = "linux", target_arch = "x86_64") -))] +#[cfg(all(feature = "guest", target_os = "linux", target_arch = "x86_64"))] pub(crate) const APP_ACTIVATE: u8 = 8; /// Response that acknowledges a request. #[cfg(any( - feature = "cli", feature = "testing", all(feature = "guest", target_os = "linux", target_arch = "x86_64") ))] pub(crate) const APP_READY: u8 = 0x81; /// Response that carries a chunk of a workload's standard output. #[cfg(any( - feature = "cli", feature = "testing", all(feature = "guest", target_os = "linux", target_arch = "x86_64") ))] pub(crate) const APP_STDOUT: u8 = 0x82; /// Response that carries a chunk of a workload's standard error. #[cfg(any( - feature = "cli", feature = "testing", all(feature = "guest", target_os = "linux", target_arch = "x86_64") ))] @@ -151,14 +140,12 @@ pub(crate) const APP_STDERR: u8 = 0x83; /// Response that ends a workload: the status is its exit status, and the payload its /// [`ExitCategory`]. #[cfg(any( - feature = "cli", feature = "testing", all(feature = "guest", target_os = "linux", target_arch = "x86_64") ))] pub(crate) const APP_EXIT: u8 = 0x84; /// Response that acknowledges [`APP_STOP`]. #[cfg(any( - feature = "cli", feature = "testing", all(feature = "guest", target_os = "linux", target_arch = "x86_64") ))] @@ -166,7 +153,6 @@ pub(crate) const APP_STOPPED: u8 = 0x85; /// Response that refuses a request: the status is an error number, and the payload an error /// category such as [`LAUNCH_FAILED`]. #[cfg(any( - feature = "cli", feature = "testing", all(feature = "guest", target_os = "linux", target_arch = "x86_64") ))] @@ -174,7 +160,6 @@ pub(crate) const APP_ERROR: u8 = 0xff; /// Error category with which a guest agent refuses a request kind that it does not know. #[cfg(any( - feature = "cli", feature = "testing", all(feature = "guest", target_os = "linux", target_arch = "x86_64") ))] @@ -222,7 +207,6 @@ pub(crate) const MAPPING_FAILED: &str = "mapping-failed"; /// Decoded application frame. #[cfg(any( - feature = "cli", feature = "testing", all(feature = "guest", target_os = "linux", target_arch = "x86_64") ))] @@ -240,7 +224,6 @@ pub(crate) struct AppFrame<'a> { /// Encodes one application frame. #[cfg(any( - feature = "cli", feature = "testing", all(feature = "guest", target_os = "linux", target_arch = "x86_64") ))] @@ -260,7 +243,6 @@ pub(crate) fn encode_app(kind: u8, request_id: u64, status: i32, payload: &[u8]) /// Decodes and validates an application frame. #[cfg(any( - feature = "cli", feature = "testing", all(feature = "guest", target_os = "linux", target_arch = "x86_64") ))] @@ -386,6 +368,13 @@ pub(crate) enum ExitCategory { impl ExitCategory { /// Parses the payload of an [`APP_EXIT`] response. #[cfg(feature = "cli")] + #[cfg_attr( + not(all(test, feature = "testing")), + expect( + dead_code, + reason = "NVX's host speaks NVXC version 2 since SL4.4c, and SL4.4d deletes version 1" + ) + )] pub(crate) fn parse(payload: &[u8]) -> Result { match payload { b"exit" => Ok(Self::Exit), diff --git a/nvx-rs/src/protocol/exec.rs b/nvx-rs/src/protocol/exec.rs index 2795f2cf9..189b7f849 100644 --- a/nvx-rs/src/protocol/exec.rs +++ b/nvx-rs/src/protocol/exec.rs @@ -30,6 +30,10 @@ pub(crate) const MAX_GUEST_CWD_BYTES: usize = MAX_ARGUMENT_BYTES; /// Largest number of workload environment entries. pub(crate) const MAX_ENVIRONMENT: usize = 256; /// Largest combined stdout and stderr volume the guest agent forwards for one execution. +#[cfg(any( + feature = "testing", + all(feature = "guest", target_os = "linux", target_arch = "x86_64") +))] pub(crate) const MAX_OUTPUT_BYTES: usize = 1024 * 1024; /// Extension field of a payload that carries flags, a working directory, and an environment. diff --git a/nvx-rs/src/protocol/features.rs b/nvx-rs/src/protocol/features.rs index 1c8e95e9f..351f004ce 100644 --- a/nvx-rs/src/protocol/features.rs +++ b/nvx-rs/src/protocol/features.rs @@ -1,4 +1,17 @@ -//! The control features that a guest image advertises in response to `APP_FEATURES`. +//! The control features that a guest image advertises in response to `APP_FEATURES`, which only +//! version 1 of NVXC has. +//! +//! NVX's host asked for them before it spoke version 2, whose Ready names the protocol version +//! instead. The guest agents and the fake OpenVMM still advertise them to version 1's `FEATURES` +//! until SL4.4d deletes them, and with them the host's side, which no host path uses any longer: +//! only some builds of the tests use it, so no expectation of dead code holds in every build. +#![cfg_attr( + feature = "cli", + allow( + dead_code, + reason = "NVX's host sends no FEATURES since SL4.4c, and SL4.4d deletes them" + ) +)] use std::ops::BitOr; @@ -8,8 +21,8 @@ use super::{ProtocolError, array, violation}; /// Control features of a guest image, advertised in the response to `APP_FEATURES`. /// /// The Rust guest agent uses these shared definitions and also speaks for the features of its -/// image's init script. A host drives a sandbox only when its guest provides every feature that -/// the host depends on, such as [`LAYERED_SANDBOX`](Self::LAYERED_SANDBOX) for `nvx-rs sandbox`; +/// image's init script. A host drove a sandbox only when its guest provided every feature that +/// the host depended on, such as [`LAYERED_SANDBOX`](Self::LAYERED_SANDBOX) for `nvx-rs sandbox`; /// an older image would silently ignore the policy or the request that depends on the missing /// feature. #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -54,15 +67,15 @@ impl GuestFeatures { #[cfg(feature = "testing")] pub(crate) const SANDBOX_AGENT: Self = Self(Self::CANCEL.0 | Self::EXEC_ENVIRONMENT.0 | Self::HOST_MAPPING_TABLE.0); - /// The features that `nvx-rs sandbox` depends on in the layered sandbox root: + /// The features that `nvx-rs sandbox` depended on in the layered sandbox root while it spoke + /// version 1: /// [`EXEC_ENVIRONMENT`](Self::EXEC_ENVIRONMENT), because `exec` sends a working directory and /// an environment in an extended `EXEC`, and /// [`HOST_MAPPING_TABLE`](Self::HOST_MAPPING_TABLE), because `start` sends the mappings of /// the sandbox's shares as `MAPS` requests. /// - /// It holds no [`CANCEL`](Self::CANCEL), because the CLI sends no `APP_CANCEL`: as `nvx.py` - /// did, Ctrl-C ends an `exec` by closing the session, and the guest agent ends the workload - /// when OpenVMM resets the session that it ran for. It holds no + /// It holds no [`CANCEL`](Self::CANCEL), because the CLI sent no `APP_CANCEL` before it spoke + /// version 2: as `nvx.py` did, Ctrl-C ended an `exec` by closing the session. It holds no /// [`WORKLOAD_ACCOUNT`](Self::WORKLOAD_ACCOUNT), because no sandbox launch requests account /// creation, which the layered root neither performs nor announces (microsoft/nvx#566). #[cfg(feature = "cli")] diff --git a/nvx-rs/src/protocol/mod.rs b/nvx-rs/src/protocol/mod.rs index e9ae7e0f7..39b748864 100644 --- a/nvx-rs/src/protocol/mod.rs +++ b/nvx-rs/src/protocol/mod.rs @@ -62,15 +62,9 @@ pub mod status; mod tests; pub mod view; -#[cfg(any( - feature = "cli", - all(feature = "guest", target_os = "linux", target_arch = "x86_64") -))] +#[cfg(all(feature = "guest", target_os = "linux", target_arch = "x86_64"))] pub(crate) use app::APP_ACTIVATE; -#[cfg(any( - feature = "cli", - all(feature = "guest", target_os = "linux", target_arch = "x86_64") -))] +#[cfg(all(feature = "guest", target_os = "linux", target_arch = "x86_64"))] pub(crate) use app::APP_CAPTURE; #[cfg(any( feature = "cli", @@ -80,13 +74,20 @@ pub(crate) use app::APP_CAPTURE; pub(crate) use app::APP_HEADER_LEN; #[cfg(all(feature = "guest", target_os = "linux", target_arch = "x86_64"))] pub(crate) use app::AppFrame; -#[cfg(any(feature = "cli", feature = "testing"))] +#[cfg(feature = "testing")] pub(crate) use app::ExitCategory; +#[cfg(any( + feature = "cli", + feature = "testing", + all(feature = "guest", target_os = "linux", target_arch = "x86_64") +))] +pub(crate) use app::MAPPING_FAILED; #[cfg(feature = "cli")] pub(crate) use app::encode_activate_payload; #[cfg(feature = "cli")] pub(crate) use app::encode_capture_payload; #[cfg(any( + feature = "cli", feature = "testing", all(feature = "guest", target_os = "linux", target_arch = "x86_64") ))] @@ -97,14 +98,12 @@ pub(crate) use app::v2; ))] pub(crate) use app::workload; #[cfg(any( - feature = "cli", feature = "testing", all(feature = "guest", target_os = "linux", target_arch = "x86_64") ))] pub(crate) use app::{ APP_CANCEL, APP_ERROR, APP_EXEC, APP_EXIT, APP_FEATURES, APP_MAPS, APP_PING, APP_READY, - APP_STDERR, APP_STDOUT, APP_STOP, APP_STOPPED, MAPPING_FAILED, UNSUPPORTED_OPERATION, - decode_app, encode_app, + APP_STDERR, APP_STDOUT, APP_STOP, APP_STOPPED, UNSUPPORTED_OPERATION, decode_app, encode_app, }; #[cfg(any( feature = "testing", @@ -115,7 +114,6 @@ pub(crate) use app::{BUSY, CWD_FAILED, INVALID_REQUEST, LAUNCH_FAILED, MAPPINGS_ pub(crate) use app::{decode_activate_payload, decode_capture_payload}; pub use boot::{MAPS_TOKEN, parse_host_mapping_count}; #[cfg(any( - feature = "cli", feature = "testing", all(feature = "guest", target_os = "linux", target_arch = "x86_64") ))] @@ -127,7 +125,6 @@ pub(crate) use exec::{GuestExecPayload, decode_guest_exec_payload}; #[cfg(feature = "cli")] pub(crate) use exec::{Workload, WorkloadEnvironment, encode_exec_payload}; #[cfg(any( - feature = "cli", feature = "testing", all(feature = "guest", target_os = "linux", target_arch = "x86_64") ))] diff --git a/nvx-rs/src/protocol/tests.rs b/nvx-rs/src/protocol/tests.rs index 215eba5f5..dc10cc73f 100644 --- a/nvx-rs/src/protocol/tests.rs +++ b/nvx-rs/src/protocol/tests.rs @@ -46,7 +46,12 @@ use crate::tests::{NVXC_V1, of_kind, vector}; feature = "testing", all(feature = "guest", target_os = "linux", target_arch = "x86_64") ))] -use super::app::{APP_HEADER_LEN, APP_VERSION, AppFrame}; +use super::app::APP_HEADER_LEN; +#[cfg(any( + feature = "testing", + all(feature = "guest", target_os = "linux", target_arch = "x86_64") +))] +use super::app::{APP_VERSION, AppFrame}; #[cfg(all( feature = "cli", feature = "testing", @@ -87,7 +92,6 @@ const MAX_CWD_BYTES: usize = 4095; const MAX_TIMEOUT_MS: u32 = u32::MAX; #[cfg(any( - feature = "cli", feature = "testing", all(feature = "guest", target_os = "linux", target_arch = "x86_64") ))] @@ -143,7 +147,6 @@ fn record_type(name: &str) -> RecordType { /// Returns the frame kind that a vector names. #[cfg(any( - feature = "cli", feature = "testing", all(feature = "guest", target_os = "linux", target_arch = "x86_64") ))] @@ -248,7 +251,6 @@ fn records_match_their_vectors() { /// Each `FRAME` vector decodes to its values, which encode to its bytes. #[cfg(any( - feature = "cli", feature = "testing", all(feature = "guest", target_os = "linux", target_arch = "x86_64") ))] @@ -757,7 +759,6 @@ fn exec_requests_are_refused_where_the_guest_agent_refuses_them() { } #[cfg(any( - feature = "cli", feature = "testing", all(feature = "guest", target_os = "linux", target_arch = "x86_64") ))] @@ -1046,7 +1047,6 @@ fn maps_decoders_refuse_the_invalid_vectors() { // them yet breaks compatibility with released peers. These tests pin each value literally. #[cfg(any( - feature = "cli", feature = "testing", all(feature = "guest", target_os = "linux", target_arch = "x86_64") ))] diff --git a/nvx-rs/tests/cli.rs b/nvx-rs/tests/cli.rs index bd3c65dc6..5066fd14a 100644 --- a/nvx-rs/tests/cli.rs +++ b/nvx-rs/tests/cli.rs @@ -121,62 +121,31 @@ fn text(data: &[u8]) -> String { } #[test] -fn start_requires_the_guest_features_of_layered_sandboxes() { +fn starts_ask_the_guest_for_no_features() { + // NVXC version 2 has no FEATURES: a guest that refuses that request of version 1, or that + // advertises none of the features, starts, runs workloads and stops all the same. let root = Root::new(); - // A guest that lacks per-execution environments or host path mapping tables, or that - // predates feature negotiation, is stopped, and its start fails like any other. - for (state, command_line, missing) in [ - ( - "cancel-only", - "fake_guest_features=1", - "per-execution environments, host path mapping tables", - ), - ( - "legacy", - "fake_legacy_guest=1", - "per-execution environments, host path mapping tables", - ), - ( - "environments-only", - "fake_guest_features=16", - "host path mapping tables", - ), + for (state, command_line) in [ + ("legacy", "fake_legacy_guest=1"), + ("featureless", "fake_guest_features=0"), ] { let output = run(&root, &provision(&root, state, command_line)); assert!(output.status.success(), "{}", text(&output.stderr)); let output = run(&root, &lifecycle("start", &root, state)); - assert_eq!(output.status.code(), Some(1)); - assert_eq!( - text(&output.stderr), - format!( - "error[guest-features-missing]: the guest image lacks control features that \ - layered sandboxes need ({missing}); use an NVX release or build that includes \ - them\n" - ) - ); - let mut names: Vec = fs::read_dir(root.path.join(state)) - .unwrap() - .map(|entry| entry.unwrap().file_name().into_string().unwrap()) - .collect(); - names.sort(); - assert_eq!(names, ["config.json", "openvmm.log", "sandbox.lock"]); - let output = run(&root, &lifecycle("deprovision", &root, state)); - assert!(output.status.success(), "{}", text(&output.stderr)); - } - // Per-execution environments and host path mapping tables are all that the CLI needs: - // neither cancellation nor workload accounts. - let output = run( - &root, - &provision(&root, "environments", "fake_guest_features=80"), - ); - assert!(output.status.success(), "{}", text(&output.stderr)); - for operation in ["start", "stop", "deprovision"] { - let output = run(&root, &lifecycle(operation, &root, "environments")); - assert!( - output.status.success(), - "{operation}: {}", - text(&output.stderr) - ); + assert!(output.status.success(), "{state}: {}", text(&output.stderr)); + let mut exec = lifecycle("exec", &root, state); + exec.extend(["--entrypoint", "/bin/echo", "--arg", "ran"].map(str::to_owned)); + let output = run(&root, &exec); + assert!(output.status.success(), "{state}: {}", text(&output.stderr)); + assert_eq!(text(&output.stdout), "ran\n"); + for operation in ["stop", "deprovision"] { + let output = run(&root, &lifecycle(operation, &root, state)); + assert!( + output.status.success(), + "{state} {operation}: {}", + text(&output.stderr) + ); + } } } @@ -3004,6 +2973,64 @@ mod interrupts { } } + #[test] + fn interrupting_an_exec_cancels_the_workload_that_streams() { + use std::io::{BufRead, BufReader, Write}; + + let root = Root::new(); + for arguments in [ + provision(&root, "state", ""), + lifecycle("start", &root, "state"), + ] { + let output = run(&root, &arguments); + assert!(output.status.success(), "{}", text(&output.stderr)); + } + let mut arguments = lifecycle("exec", &root, "state"); + arguments + .extend(["--entrypoint", "/bin/sh", "--arg=-c", "--arg", "cat"].map(str::to_owned)); + let mut child = Command::new(NVX_RS) + .arg("--root") + .arg(&root.path) + .args(&arguments) + .current_dir(&root.path) + .env_remove("NVX_OPENVMM") + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .unwrap(); + // The workload echoes its input as it reads it, which stays open. + let mut input = child.stdin.take().unwrap(); + input.write_all(b"streamed\n").unwrap(); + let mut echoed = String::new(); + BufReader::new(child.stdout.take().unwrap()) + .read_line(&mut echoed) + .unwrap(); + assert_eq!(echoed, "streamed\n"); + let interrupted = Instant::now(); + let pid = nix::unistd::Pid::from_raw(i32::try_from(child.id()).unwrap()); + nix::sys::signal::kill(pid, nix::sys::signal::Signal::SIGINT).unwrap(); + let output = child.wait_with_output().unwrap(); + assert_eq!(output.status.code(), Some(130), "{}", text(&output.stderr)); + assert_eq!(text(&output.stderr), "Interrupted\n"); + assert!(interrupted.elapsed() < Duration::from_secs(10)); + drop(input); + // The guest ended the workload, so the sandbox runs another, and stops. + let mut echo = lifecycle("exec", &root, "state"); + echo.extend(["--entrypoint", "/bin/echo", "--arg", "again"].map(str::to_owned)); + let output = run(&root, &echo); + assert!(output.status.success(), "{}", text(&output.stderr)); + assert_eq!(text(&output.stdout), "again\n"); + for operation in ["stop", "deprovision"] { + let output = run(&root, &lifecycle(operation, &root, "state")); + assert!( + output.status.success(), + "{operation}: {}", + text(&output.stderr) + ); + } + } + #[test] fn interrupting_a_provision_that_waits_for_a_download_provisions_nothing() { use std::io::{BufRead, BufReader, Read}; @@ -3088,8 +3115,8 @@ mod interrupts { #[test] fn interrupting_an_exec_ends_its_write_to_a_full_pipe() { - // The CLI writes a workload's output once the workload ends. Like Python's, the write - // ends at Ctrl-C, also while it waits for a pipe's reader that does not read. + // The CLI writes a workload's output as it comes. Like Python's, the write ends at + // Ctrl-C, also while it waits for a pipe's reader that does not read. let root = Root::new(); for arguments in [ provision(&root, "state", ""), diff --git a/nvx-rs/tests/cli/harness.rs b/nvx-rs/tests/cli/harness.rs index 1470a1cf5..e86fb5ecb 100644 --- a/nvx-rs/tests/cli/harness.rs +++ b/nvx-rs/tests/cli/harness.rs @@ -381,6 +381,36 @@ impl Root { Outcome::new(arguments, output) } + /// Runs `nvx-rs --root ROOT ARGUMENTS` in the root, as [`nvx_rs`](Self::nvx_rs) does, but with + /// `input` as its standard input, which then ends. + pub(crate) fn nvx_rs_with_input>( + &self, + arguments: &[S], + input: &[u8], + ) -> Outcome { + let arguments: Vec = arguments + .iter() + .map(|argument| self.expand(argument.as_ref())) + .collect(); + let mut child = self + .command(&arguments) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .unwrap(); + let mut stdin = child.stdin.take().unwrap(); + let input = input.to_vec(); + // A command that ends before it reads its whole input breaks the pipe, which the run's + // outcome then shows. + let writer = std::thread::spawn(move || { + let _ = std::io::Write::write_all(&mut stdin, &input); + }); + let output = child.wait_with_output().unwrap(); + writer.join().unwrap(); + Outcome::new(arguments, output) + } + /// Runs `nvx-rs --root ROOT ARGUMENTS` in the root, as [`nvx_rs`](Self::nvx_rs) does, but /// with the umask `umask`, rather than this process's, which the tests running in parallel /// share. diff --git a/nvx-rs/tests/cli/lifecycle.rs b/nvx-rs/tests/cli/lifecycle.rs index 07bfee820..c052ea250 100644 --- a/nvx-rs/tests/cli/lifecycle.rs +++ b/nvx-rs/tests/cli/lifecycle.rs @@ -787,7 +787,7 @@ struct Execution { status: i32, /// What the workload wrote to standard output. stdout: String, - /// What the workload wrote to standard error. + /// What the workload wrote to standard error, or the guest agent before it refused it. stderr: &'static str, /// The code of the error with which `exec` refuses the workload, if it does, and what the /// error names. @@ -900,6 +900,15 @@ fn executions() -> Vec { "", ); missing.stderr = "openvmm: /bin/missing: not found\n"; + // The guest agent's diagnostic streams before its refusal. + let mut cwd_refused = refused( + "missing working directory", + &["--cwd", "/missing"], + "exec-refused", + vec!["cwd-failed"], + ); + cwd_refused.stderr = + "nvx-managed-agent: cannot enter working directory /missing: No such file or directory\n"; let mut existing = refused( "existing outcome report", &["--outcome-report", "{root/reports/existing.json}"], @@ -988,12 +997,7 @@ fn executions() -> Vec { "", ), missing, - refused( - "missing working directory", - &["--cwd", "/missing"], - "exec-refused", - vec!["cwd-failed"], - ), + cwd_refused, refused( "relative working directory", &["--cwd", "relative"], @@ -1052,7 +1056,7 @@ fn managed_execs_exit_with_the_workload_status() { } let before = root.snapshot(); let started = Instant::now(); - let outcome = root.nvx_rs(&sandbox("exec", "{root/state}", &case.options)); + let mut outcome = root.nvx_rs(&sandbox("exec", "{root/state}", &case.options)); eprintln!("case: {}", case.name); match &case.refusal { Some((code, fragments)) => { @@ -1061,6 +1065,14 @@ fn managed_execs_exit_with_the_workload_status() { .map(|fragment| root.expand(fragment)) .collect(); let fragments: Vec<&str> = fragments.iter().map(String::as_str).collect(); + // What the guest wrote before it refused the workload streams before the error. + let streamed = case.stderr.as_bytes(); + assert!( + outcome.stderr.starts_with(streamed), + "{}", + outcome.describe() + ); + outcome.stderr.drain(..streamed.len()); outcome.assert_error(code, &fragments); // A refusal changes nothing, but the fake OpenVMM's log of the connections to its // control endpoint, and the workload did not run. @@ -1098,6 +1110,51 @@ fn managed_execs_exit_with_the_workload_status() { .assert_silent_success(); } +#[test] +fn managed_execs_stream_their_standard_input_and_output() { + let root = Root::new(); + root.nvx_rs(&provision("{root/state}", &[])) + .assert_silent_success(); + root.nvx_rs(&sandbox::<&str>("start", "{root/state}", &[])) + .assert_silent_success(); + // The workload reads more input than the window of 64 KiB holds, until its end, and echoes + // it as it reads it. + let input: Vec = (0..200 * 1024).map(|index| (index % 251) as u8).collect(); + let cat = [ + "--entrypoint", + "/bin/sh", + "--arg=-c", + "--arg", + "cat; echoerr ended", + ]; + let echoed = root.nvx_rs_with_input(&sandbox("exec", "{root/state}", &cat), &input); + assert_eq!(echoed.status, Some(0), "{}", echoed.describe()); + assert!(echoed.stdout == input, "{}", echoed.describe()); + assert_eq!(echoed.stderr(), "ended\n"); + // Output passes the old cap of 1 MiB whole. + let flood = root.nvx_rs(&sandbox( + "exec", + "{root/state}", + &[ + "--entrypoint", + "/bin/sh", + "--arg=-c", + "--arg", + "flood 2000000", + ], + )); + assert_eq!(flood.status, Some(0), "{}", flood.describe()); + assert!( + flood.stdout == vec![b'x'; 2_000_000], + "{}", + flood.describe() + ); + root.nvx_rs(&sandbox::<&str>("stop", "{root/state}", &[])) + .assert_silent_success(); + root.nvx_rs(&sandbox::<&str>("deprovision", "{root/state}", &[])) + .assert_silent_success(); +} + /// Runs the lifecycle operation `operation` of the sandbox in `{root/STATE}` with `options`, and /// returns how it ended. fn operate(root: &Root, operation: &str, state: &str, options: &[&str]) -> Outcome { diff --git a/nvx-rs/tests/cli/sandbox_run.rs b/nvx-rs/tests/cli/sandbox_run.rs index 8ed207373..53b2c535d 100644 --- a/nvx-rs/tests/cli/sandbox_run.rs +++ b/nvx-rs/tests/cli/sandbox_run.rs @@ -464,6 +464,25 @@ fn a_run_whose_guest_exits_during_startup_fails_as_a_start_does() { ); } +/// A run streams its workload's standard input, which the command's ends, and its output. +#[test] +fn a_run_streams_its_standard_input_and_output() { + let root = Root::new(); + let before = root.snapshot(); + let outcome = root.nvx_rs_with_input( + &sandbox_run(&[ + "--entrypoint", + "/bin/sh", + "--arg=-c", + "--arg", + "cat; echoerr ended", + ]), + b"line one\nline two\n", + ); + assert_ran(&root, &before, &outcome, 0, "line one\nline two\n"); + assert_eq!(outcome.stderr(), "ended\n", "{}", outcome.describe()); +} + #[test] fn a_run_whose_workload_the_guest_refuses_stops_its_vm() { let root = Root::new(); @@ -477,11 +496,13 @@ fn a_run_whose_workload_the_guest_refuses_stops_its_vm() { "{root/reports/run.json}", ])); assert_ran(&root, &before, &outcome, 1, ""); + // The guest agent's diagnostic streams before the refusal, which the run reports. assert_eq!( outcome.stderr(), format!( - "error[exec-refused]: managed guest rejected exec (status=2, category=cwd-failed)\ - {LINE_END}" + "nvx-managed-agent: cannot enter working directory /missing: No such file or \ + directory\nerror[exec-refused]: managed guest rejected exec (status=2, \ + category=cwd-failed){LINE_END}" ) ); assert_outcome_report( @@ -557,6 +578,52 @@ fn a_run_forces_down_a_vm_that_does_not_stop() { ); } +/// A run's own warnings start a line of their own after standard error that its workload left +/// without a line end, which the run otherwise leaves as it is. +#[test] +fn a_runs_warnings_start_lines_of_their_own_after_its_workloads_output() { + let root = Root::new(); + let before = root.snapshot(); + let outcome = root.nvx_rs(&sandbox_run(&[ + "--entrypoint", + "/bin/sh", + "--arg=-c", + "--arg", + "printerr partial", + "--timeout", + "1", + "--cmdline", + "fake_ignore_stop=1", + ])); + assert_ran(&root, &before, &outcome, 0, ""); + let stderr = outcome.stderr(); + assert!( + stderr.starts_with(&format!( + "partial{LINE_END}warning: managed guest shutdown failed: " + )), + "{}", + outcome.describe() + ); + assert!( + stderr.ends_with(&format!( + "{LINE_END}warning: OpenVMM was forcibly terminated after the run{LINE_END}" + )), + "{}", + outcome.describe() + ); + let root = Root::new(); + let before = root.snapshot(); + let outcome = root.nvx_rs(&sandbox_run(&[ + "--entrypoint", + "/bin/sh", + "--arg=-c", + "--arg", + "printerr partial", + ])); + assert_ran(&root, &before, &outcome, 0, ""); + assert_eq!(outcome.stderr(), "partial", "{}", outcome.describe()); +} + /// A run forwards an outcome report that the caller can read whatever its umask, even one that /// clears the owner's own permissions, and whose group's and others' permissions the caller's /// umask narrows: OpenVMM starts with the caller's umask without the owner's bits, and the run diff --git a/scripts/test-layout-allowlist.json b/scripts/test-layout-allowlist.json index 6ba886ddc..bf3399869 100644 --- a/scripts/test-layout-allowlist.json +++ b/scripts/test-layout-allowlist.json @@ -12,7 +12,7 @@ "nvx-rs/src/host/platform/windows/mod.rs": 1, "nvx-rs/src/host/platform/windows/process.rs": 2, "nvx-rs/src/host/platform/windows/spawn.rs": 1, - "nvx-rs/src/host/session.rs": 5, - "nvx-rs/src/host/vm.rs": 2 + "nvx-rs/src/host/vm.rs": 2, + "nvx-rs/src/protocol/app.rs": 1 } } From d66994591e99113453b3d4a0f85bef56e14f09b4 Mon Sep 17 00:00:00 2001 From: Pedro Henrique Penna Date: Thu, 12 Nov 2026 05:08:48 -0800 Subject: [PATCH 2/2] scripts: expect sandbox-lifecycle's output past NVXC v1's cap The sandbox-lifecycle scenario checked that an exec which writes 2 MiB ends with status 125 and the output-limit category after at most the 1 MiB that version 1 of NVXC forwarded. NVX's host speaks version 2 now, which streams the output without a total cap (#1128), so the scenario expects all 2 MiB, status 0 and the exit category instead. Its harness test's regressions follow: a guest that still caps the output fails the exec's status, and one that loses a chunk fails the count. The case matrix's row of the scenario says what changed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- doc/acceptance-case-matrix.md | 2 +- doc/run.md | 9 +++---- scripts/nvx_tools/sandbox_lifecycle_tests.py | 26 ++++++++------------ scripts/test_microvm_tests.py | 18 ++++++++------ 4 files changed, 25 insertions(+), 30 deletions(-) diff --git a/doc/acceptance-case-matrix.md b/doc/acceptance-case-matrix.md index 52def558e..fef690697 100644 --- a/doc/acceptance-case-matrix.md +++ b/doc/acceptance-case-matrix.md @@ -341,7 +341,7 @@ suite in its shards, and the public sandbox scenarios in its guest and sandbox j | `run-console-restore` | retired | SL3.8b removes `nvx-rs run`'s restore of a raw OpenVMM snapshot and the boot console that it reattached on the terminal: `run --restore-snapshot` restores a named snapshot through `snapshot restore`'s path | | `sandbox-blocks` | kept | | | `sandbox-gpt-layer` | kept | The [cold cells](#cold-boots) of a GPT disk, over a scratch upper and over a RAM upper, through `run`, and `provision` and `start` | -| `sandbox-lifecycle` | kept | A [cold cell](#cold-boots): the EROFS layer over a scratch upper, through `provision` and `start`, which also reattach to the VM from a new process for each command | +| `sandbox-lifecycle` | kept | A [cold cell](#cold-boots): the EROFS layer over a scratch upper, through `provision` and `start`, which also reattach to the VM from a new process for each command. Since SL4.4c, its request that writes 2 MiB checks that `exec` streams all of it, where it checked NVXC version 1's 1 MiB bound | | `sandbox-proxy-only` | kept | Its proxy-only network, over the EROFS layer and a scratch upper; it is no [cold cell](#cold-boots) of its own | | `sandbox-ram-upper` | kept | A [cold cell](#cold-boots): the EROFS layer over a RAM upper, through `run`, and `provision` and `start`. Since SL3.10c, it checks that `provision` records the configuration's format 10 | | `scratch-snapshot` | kept | | diff --git a/doc/run.md b/doc/run.md index c7647d94e..563eb97c2 100644 --- a/doc/run.md +++ b/doc/run.md @@ -1797,11 +1797,10 @@ template, so it has the same artifact requirements. It checks that: is on scratch; - `/sbin/nvx-sandbox-smoke` passes its security profile checks in a sandbox provisioned with `--hostname`, after earlier requests; -- a request that writes more than the 1 MiB output bound exits with status - 125 and an `output-limit` outcome report after the host has received more - than 1 MiB less one 32 KiB read chunk and at most 1 MiB, and a request whose - `--outcome-report` path exists fails before its workload runs and leaves the - file intact; +- a request that writes 2 MiB, more than the 1 MiB that version 1 of NVXC + forwarded, streams all of it and exits with status 0 and an `exit` outcome + report, and a request whose `--outcome-report` path exists fails before its + workload runs and leaves the file intact; - `stop` ends OpenVMM and its control socket or pipe, leaves only `config.json`, `sandbox.lock`, `exec.lock`, `openvmm.log`, and a successful `outcome.json`, and leaves a cleanly unmounted scratch file system, after diff --git a/scripts/nvx_tools/sandbox_lifecycle_tests.py b/scripts/nvx_tools/sandbox_lifecycle_tests.py index ffc35e348..7db444167 100644 --- a/scripts/nvx_tools/sandbox_lifecycle_tests.py +++ b/scripts/nvx_tools/sandbox_lifecycle_tests.py @@ -51,12 +51,9 @@ MEMORY_MIB = 256 # An identity that the Ubuntu layer's user database lacks. UNAVAILABLE_IDENTITY = "4242:4242" -# The guest agent forwards at most this much output for one exec request, and -# reads it in chunks of at most OUTPUT_CHUNK_BYTES. It drops the chunk that -# would exceed the bound, so more than OUTPUT_LIMIT_BYTES - OUTPUT_CHUNK_BYTES -# bytes reach the host. -OUTPUT_LIMIT_BYTES = 1024 * 1024 -OUTPUT_CHUNK_BYTES = 32 * 1024 +# More output than the 1 MiB that version 1 of NVXC forwarded for one exec +# request, which version 2 streams whole. +STREAMED_OUTPUT_BYTES = 2 * 1024 * 1024 STATE_PATH = "/tmp/nvx lifecycle state" STATE_ARGUMENTS = (" leading and trailing ", "tab\tseparated", "line one\nline two") STATE_CONTENT = "".join(f"{argument}\n" for argument in STATE_ARGUMENTS).encode() @@ -831,31 +828,28 @@ def check_security_profile(self) -> None: ) def check_bounded_results(self) -> None: - # The agent stops forwarding output at its bound, and the outcome + # The host receives all of the workload's output, and the outcome # report carries only the operation, category, and status. - report = self.root / "output-limit-outcome.json" + report = self.root / "streamed-output-outcome.json" result = self.exec( "/usr/bin/head", "--arg=-c", - f"--arg={2 * OUTPUT_LIMIT_BYTES}", + f"--arg={STREAMED_OUTPUT_BYTES}", "--arg=/dev/zero", "--outcome-report", str(report), - expected=125, ) if ( - not OUTPUT_LIMIT_BYTES - OUTPUT_CHUNK_BYTES - < len(result.stdout) - <= OUTPUT_LIMIT_BYTES + len(result.stdout) != STREAMED_OUTPUT_BYTES or result.stdout.strip(b"\0") or result.stderr ): raise RuntimeError( - "the output limit forwarded " + "the streamed output forwarded " f"{len(result.stdout)} stdout and {len(result.stderr)} stderr bytes" ) - read_exec_outcome(report, category="output-limit", status_code=125) - self.copy_evidence(report, "output-limit-outcome.json") + read_exec_outcome(report, category="exit", status_code=0) + self.copy_evidence(report, "streamed-output-outcome.json") # An existing report path fails the request before the workload runs. existing = self.root / "existing-outcome.json" diff --git a/scripts/test_microvm_tests.py b/scripts/test_microvm_tests.py index ef0df03ac..f48e54f8d 100644 --- a/scripts/test_microvm_tests.py +++ b/scripts/test_microvm_tests.py @@ -1311,12 +1311,14 @@ def _exec( + b"NVX-UBUNTU-SANDBOX-OK uid=65534 gid=65534\n" ) elif entrypoint == "/usr/bin/head": - size = sandbox_lifecycle_tests.OUTPUT_LIMIT_BYTES - if "unbounded-output" in self.faults: - size = int(arguments[1]) + size = int(arguments[1]) + if "capped-output" in self.faults: + # The guest killed the workload at version 1's bound. + returncode, category = 125, "output-limit" + size = 1024 * 1024 elif "short-output" in self.faults: - size -= sandbox_lifecycle_tests.OUTPUT_CHUNK_BYTES - returncode, category, stdout = 125, "output-limit", bytes(size) + size -= 32 * 1024 + stdout = bytes(size) elif entrypoint != "/bin/true": raise AssertionError(f"unexpected workload {entrypoint}") if report is not None: @@ -1640,7 +1642,7 @@ def test_acceptance_drives_the_complete_public_lifecycle(self): "sandbox-lifecycle-identity-outcome.json", "sandbox-lifecycle-runtime.json", "sandbox-lifecycle-probe.log", - "sandbox-lifecycle-output-limit-outcome.json", + "sandbox-lifecycle-streamed-output-outcome.json", "sandbox-lifecycle-outcome.json", "sandbox-lifecycle-openvmm.log", }, @@ -1713,8 +1715,8 @@ def test_acceptance_detects_lifecycle_regressions(self): "duplicate-start": "start returned 0, expected 1", "exec-loses-state": "the state-reading request returned status 1", "probe-fails": "exec returned 1, expected 0", - "unbounded-output": "the output limit forwarded 2097152 stdout", - "short-output": "the output limit forwarded 1015808 stdout", + "capped-output": "exec returned 125, expected 0", + "short-output": "the streamed output forwarded 2064384 stdout", "overwrite-report": "exec returned 0, expected 1", "stop-leaks-capability": "after stop: the state directory holds", "stop-leaks-process": "OpenVMM still runs after stop",