From 0def9444dc645ccb73c2a30fc9ddee542dc6e070 Mon Sep 17 00:00:00 2001 From: John Starks Date: Fri, 1 May 2026 02:15:45 +0000 Subject: [PATCH 1/2] squash --- Guide/src/reference/openvmm/management/cli.md | 123 +++- .../src/reference/openvmm/management/grpc.md | 11 +- openvmm/openvmm_entry/src/cli_args.rs | 557 ++++++++++++++++-- openvmm/openvmm_entry/src/lib.rs | 340 ++++++++--- openvmm/openvmm_entry/src/meshworker.rs | 6 + openvmm/openvmm_entry/src/repl.rs | 28 +- openvmm/openvmm_entry/src/ttrpc/mod.rs | 145 ++--- openvmm/openvmm_entry/src/vm_connect.rs | 146 +++++ openvmm/openvmm_entry/src/vm_controller.rs | 302 ++++++++-- vmm_tests/vmm_tests/tests/tests/ttrpc.rs | 28 +- 10 files changed, 1363 insertions(+), 323 deletions(-) create mode 100644 openvmm/openvmm_entry/src/vm_connect.rs diff --git a/Guide/src/reference/openvmm/management/cli.md b/Guide/src/reference/openvmm/management/cli.md index 8ffc414455..0e4f64b5ff 100644 --- a/Guide/src/reference/openvmm/management/cli.md +++ b/Guide/src/reference/openvmm/management/cli.md @@ -1,12 +1,39 @@ # CLI +This page summarizes common `openvmm` command-line operations and VM launch +options. + ```admonish danger title="Disclaimer" The following list is not exhaustive, and may be out of date. -The most up to date reference is always the [code itself](https://openvmm.dev/rustdoc/linux/openvmm_entry/struct.Options.html), -as well as the generated CLI help (via `cargo run -- --help`). +The most up to date reference is always the [`RunOptions` rustdoc][run] and +[`ServeOptions` rustdoc][serve], as well as the generated CLI help (via +`cargo run -- run --help` or `cargo run -- serve --help`). ``` +## Commands + +Use `openvmm run` to launch a VM. For compatibility, launch options are also +accepted at the top level, so existing invocations like `openvmm -p 2 ...` +continue to work as an implicit `run` command. Command-style operations such as +`attach` and `inspect` are separate subcommands and do not accept VM launch +options before the command name. + +* `run [OPTIONS]`: Launch a VM using the configured firmware, devices, + memory, and processor topology. +* `serve --transport [--pidfile ] `: Host the + management RPC API on the specified Unix socket. The RPC API can create and + control VMs through the server process. +* `attach `: Connect to a VM started with `--mesh-listen` and run + an interactive REPL. Exiting the attached REPL disconnects the client without + stopping the VM. +* `inspect [OPTIONS] [ELEMENT]`: Connect to a VM started with + `--mesh-listen`, inspect one element path, print the result, and exit. Use + `-r`/`--recursive` to enumerate recursively, `--limit ` to bound recursive + depth, and `-v`/`--paravisor` to target the paravisor. + +## VM Launch Options + * `--processors `: The number of processors. Defaults to 1. * `--memory `: Configure guest RAM. Defaults to `size=1G`. `SPEC` can be a size-only shorthand, such as `--memory 4G`, or a @@ -61,7 +88,12 @@ as well as the generated CLI help (via `cargo run -- --help`). --hypervisor kvm ``` * `--uefi`: Boot using `mu_msvm` UEFI -* `--uefi-firmware `: Path to the UEFI firmware file (`MSVM.fd`). When `--uefi` is specified, this option is required only if you do not set the environment variable `OPENVMM_UEFI_FIRMWARE` (or the architecture-specific variants `X86_64_OPENVMM_UEFI_FIRMWARE`, or `AARCH64_OPENVMM_UEFI_FIRMWARE`). If omitted, the default is read from `OPENVMM_UEFI_FIRMWARE` first, then falls back to the architecture-specific variables. +* `--uefi-firmware `: Path to the UEFI firmware file (`MSVM.fd`). When + `--uefi` is specified, this option is required only if you do not set the + environment variable `OPENVMM_UEFI_FIRMWARE`, + `X86_64_OPENVMM_UEFI_FIRMWARE`, or `AARCH64_OPENVMM_UEFI_FIRMWARE`. If + omitted, the default is read from `OPENVMM_UEFI_FIRMWARE` first, then falls + back to the architecture-specific variables. * `--pcat`: Boot using the Microsoft Hyper-V PCAT BIOS * `--disk file:`: Exposes a single disk over VMBus. You must also pass `--hv`. The `DISK` argument can be: @@ -81,22 +113,36 @@ as well as the generated CLI help (via `cargo run -- --help`). crashes, the pidfile is not removed — consumers should verify the PID is still alive. No file locking is performed; concurrent launches with the same pidfile path will overwrite each other. Not written for short-lived utility - modes such as `--write-saved-state-proto`. + modes such as `--write-saved-state-proto`. Also accepted by `serve`. +* `--mesh-listen `: Listen for REPL attach connections on the + specified socket path. It requires the process mesh and conflicts with + `--single-process`. The socket is removed during clean shutdown. Any local + process that can connect to the socket can control the VM, so place it in a + directory restricted to the intended user. * `--nic`: Exposes a NIC using the Consomme user-mode NAT. * `--gfx`: Enable a graphical console over VNC (see below) -* `--virtio-9p`: Expose a virtio 9p file system. Uses the format `tag,root_path`, e.g. `myfs,C:\\`. - The file system can be mounted in a Linux guest using `mount -t 9p -o trans=virtio tag /mnt/point`. - You can specify this argument multiple times to create multiple file systems. -* `--virtio-fs`: Expose a virtio-fs file system. The format is the same as `--virtio-9p`. The - file system can be mounted in a Linux guest using `mount -t virtiofs tag /mnt/point`. - You can specify this argument multiple times to create multiple file systems. -* `--virtio-rng`: Add a virtio entropy (RNG) device, exposing `/dev/hwrng` in the Linux guest. - The guest kernel must have `CONFIG_HW_RANDOM_VIRTIO` enabled. -* `--virtio-rng-bus `: Select the bus for the virtio-rng device (`auto`, `mmio`, `pci`, `vpci`). - Defaults to `auto`. -* `--vhost-user ,type=[,tag=][,num_queues=][,queue_size=][,pcie_port=]`: Attach a - vhost-user device backed by an external process over a Unix socket (Linux - only). The backend process must already be listening on `SOCKET_PATH`. +* `--virtio-9p`: Expose a virtio 9p file system. Uses the format + `tag,root_path`, e.g. `myfs,C:\`. The file system can be mounted in a + Linux guest using `mount -t 9p -o trans=virtio tag /mnt/point`. You can + specify this argument multiple times to create multiple file systems. +* `--virtio-fs`: Expose a virtio-fs file system. The format is the same as + `--virtio-9p`. The file system can be mounted in a Linux guest using + `mount -t virtiofs tag /mnt/point`. You can specify this argument multiple + times to create multiple file systems. +* `--virtio-rng`: Add a virtio entropy (RNG) device, exposing `/dev/hwrng` in + the Linux guest. The guest kernel must have `CONFIG_HW_RANDOM_VIRTIO` + enabled. +* `--virtio-rng-bus `: Select the bus for the virtio-rng device (`auto`, + `mmio`, `pci`, `vpci`). Defaults to `auto`. +* `--vhost-user `: Attach a vhost-user device backed by an external + process over a Unix socket (Linux only). The backend process must already be + listening on `SOCKET_PATH`. The spec uses this format: + + ```text + ,type=[,tag=] + [,num_queues=][,queue_size=][,pcie_port=] + ``` + Supported `type` values: `blk`, `fs`. For `type=fs`, `tag=` is required and specifies the mount tag exposed to the guest (max 36 bytes). `num_queues` and `queue_size` control the queue layout (defaults: blk @@ -104,7 +150,7 @@ as well as the generated CLI help (via `cargo run -- --help`). Alternatively, use `device_id=` instead of `type=` to specify the numeric virtio device ID directly, with `queue_sizes=[N,N,N]` for per-queue sizes. Examples: - ```sh + ```bash --vhost-user /tmp/vhost-blk.sock,type=blk --vhost-user /tmp/vhost-blk.sock,type=blk,num_queues=4,queue_size=512 --vhost-user /tmp/vhost-blk.sock,type=blk,pcie_port=rp0 @@ -113,7 +159,8 @@ as well as the generated CLI help (via `cargo run -- --help`). --vhost-user /tmp/vhost.sock,device_id=26,queue_sizes=[256,256] ``` -Serial devices can be configured to appear as different devices inside the guest: +Serial devices can be configured to appear as different devices inside the +guest: * `--com1/com2 `: Configure a COM port serial device. * `--virtio-console `: Expose a virtio console device (appears as @@ -131,6 +178,27 @@ The `BACKEND` argument is the same for all serial devices: connections on the given IP address and port. Typically IP will be 127.0.0.1, to restrict connections to the current host. +## Management RPC Server + +Use `openvmm serve` to host the management RPC API without launching a VM from +the command line: + +```bash +openvmm serve --transport grpc /tmp/openvmm-rpc.sock +openvmm serve --transport ttrpc /tmp/openvmm-rpc.sock +openvmm serve --transport ttrpc --pidfile /tmp/openvmm.pid /tmp/openvmm.sock +``` + +`--transport` is required. Builds that do not include the selected transport +will reject the command. Use `--pidfile ` to write the server process ID +on startup and remove it on clean exit. + +The positional `SOCKETPATH` is the management RPC server socket. This is +separate from `--mesh-listen `, which exposes the interactive attach +and inspect socket used by `openvmm attach` and `openvmm inspect`. When passed to +`serve`, `--mesh-listen` controls whether VMs managed by the server also expose +that attach/inspect socket. + ## PCIe Device Support OpenVMM can emulate a PCI Express topology using `--pcie-root-complex` and @@ -139,7 +207,7 @@ attached to a root port to appear as PCIe devices in the guest. ### Setting up a PCIe topology -```sh +```bash # Create a root complex and root port --pcie-root-complex rc0 --pcie-root-port rc0:rp0 ``` @@ -151,7 +219,7 @@ PCIe root port. The syntax varies slightly between device types: **Disks** (comma-separated option): `--disk`, `--nvme`, `--virtio-blk` -```sh +```bash --virtio-blk file:/path/to/disk.raw,pcie_port=rp0 --nvme file:/path/to/disk.raw,pcie_port=rp0 --disk file:/path/to/disk.raw,pcie_port=rp0 @@ -159,7 +227,7 @@ PCIe root port. The syntax varies slightly between device types: **NICs** (colon-prefixed): `--net`, `--virtio-net`, `--mana` -```sh +```bash --virtio-net pcie_port=rp0:tap:tap0 # TAP is Linux-only --net pcie_port=rp0:consomme --mana pcie_port=rp0:tap:tap0 # TAP is Linux-only @@ -168,7 +236,7 @@ PCIe root port. The syntax varies slightly between device types: **Filesystems and other virtio devices** (colon-prefixed): `--virtio-fs`, `--virtio-fs-shmem`, `--virtio-9p`, `--virtio-pmem` -```sh +```bash --virtio-fs pcie_port=rp0:myfs,/path/to/share --virtio-fs-shmem pcie_port=rp0:myfs,/path/to/share --virtio-9p pcie_port=rp0:myfs,/path/to/share @@ -177,20 +245,23 @@ PCIe root port. The syntax varies slightly between device types: For `--virtio-rng` and `--virtio-console`, use their separate PCIe port flags: -```sh +```bash --virtio-rng --virtio-rng-pcie-port rp0 --virtio-console console --virtio-console-pcie-port rp0 ``` **vhost-user devices** (comma-separated option, Linux only): `--vhost-user` -```sh +```bash --vhost-user /tmp/vhost-blk.sock,type=blk,pcie_port=rp0 --vhost-user /tmp/virtiofsd.sock,type=fs,tag=myfs,pcie_port=rp0 ``` **VFIO device assignment** (Linux only): `--vfio` -```sh +```bash --vfio rp0:0000:01:00.0 ``` + +[run]: https://openvmm.dev/rustdoc/linux/openvmm_entry/struct.RunOptions.html +[serve]: https://openvmm.dev/rustdoc/linux/openvmm_entry/struct.ServeOptions.html diff --git a/Guide/src/reference/openvmm/management/grpc.md b/Guide/src/reference/openvmm/management/grpc.md index 5be1d8eaa5..6d48061ec6 100644 --- a/Guide/src/reference/openvmm/management/grpc.md +++ b/Guide/src/reference/openvmm/management/grpc.md @@ -1,8 +1,13 @@ # gRPC / ttrpc -To enable gRPC or ttrpc management interfaces, pass `--grpc ` or -`--trpc `. This will spawn an OpenVMM process acting as a gRPC or -ttrpc server. +To enable gRPC or ttrpc management interfaces, use `openvmm serve` with +`--transport grpc` or `--transport ttrpc` and a Unix socket path. This will +spawn an OpenVMM process acting as a gRPC or ttrpc server. + +```bash +openvmm serve --transport grpc /tmp/openvmm-rpc.sock +openvmm serve --transport ttrpc /tmp/openvmm-rpc.sock +``` Here is a list of supported RPCs: diff --git a/openvmm/openvmm_entry/src/cli_args.rs b/openvmm/openvmm_entry/src/cli_args.rs index f060fd3a8c..8fb68ada75 100644 --- a/openvmm/openvmm_entry/src/cli_args.rs +++ b/openvmm/openvmm_entry/src/cli_args.rs @@ -58,7 +58,176 @@ pub struct MemoryCli { /// This is not yet a stable interface and may change radically between /// versions. #[derive(Parser)] +#[clap( + after_help = "For now, omitting a subcommand is treated as `run` for compatibility.\nThis will change in the future. Use `openvmm run ...` for VM launch options." +)] pub struct Options { + /// Command to run. + #[clap(subcommand)] + pub command: Command, +} + +#[derive(Parser)] +#[clap(args_conflicts_with_subcommands = true)] +struct LegacyOptions { + #[clap(subcommand)] + command: Option, + + #[clap(flatten)] + pub run: RunOptions, + + /// Legacy gRPC server socket path. + #[clap(long = "grpc", value_name = "SOCKETPATH", hide = true)] + pub legacy_grpc: Option, + + /// Legacy ttrpc server socket path. + #[clap(long = "ttrpc", value_name = "SOCKETPATH", hide = true)] + pub legacy_ttrpc: Option, + + /// prefetch guest RAM + #[clap(long = "prefetch", hide = true)] + pub deprecated_prefetch: bool, + + /// back guest RAM with a file instead of anonymous memory. + /// The file is created/opened and sized to the guest RAM size. + /// Enables snapshot save (fsync) and restore (open + mmap). + #[clap( + long = "memory-backing-file", + value_name = "FILE", + hide = true, + conflicts_with = "deprecated_private_memory" + )] + pub deprecated_memory_backing_file: Option, + + /// use private anonymous memory for guest RAM + #[clap(long = "private-memory", hide = true, conflicts_with_all = ["deprecated_memory_backing_file", "restore_snapshot"])] + pub deprecated_private_memory: bool, + + /// enable transparent huge pages for guest RAM (Linux only, requires --private-memory) + #[clap(long = "thp", hide = true)] + pub deprecated_thp: bool, +} + +pub(crate) fn parse_options() -> Command { + match try_parse_options_from(std::env::args_os()) { + Ok(options) => options, + Err(err) => err.exit(), + } +} + +fn try_parse_options_from(args: I) -> Result +where + I: IntoIterator, + T: Into, +{ + // In non-optimized builds, clap uses an embarassing amount of stack space + // to construct the `Command` instance for `Options`, more than the Windows + // default of 1MB. This has been known since 2023: + // , but no one has stepped up + // to fix it. + // + // Work around this by running the code on a thread with lots of stack + // space. This is easier and more reliable than configuring the PE binary to + // have a larger stack. + fn on_big_stack(f: impl Send + FnOnce() -> R) -> R { + if cfg!(windows) { + std::thread::scope(|s| { + std::thread::Builder::new() + .stack_size(0x400000) + .spawn_scoped(s, f) + .unwrap() + .join() + .unwrap() + }) + } else { + f() + } + } + + let args = args.into_iter().map(Into::into).collect::>(); + on_big_stack(|| match Options::try_parse_from(args.clone()) { + Ok(Options { command }) => Ok(command), + Err(err) + if matches!( + err.kind(), + clap::error::ErrorKind::DisplayHelp | clap::error::ErrorKind::DisplayVersion + ) => + { + Err(err) + } + Err(primary_err) => legacy_options_from(primary_err, args.as_slice()), + }) +} + +fn preferred_parse_error(primary_err: clap::Error, legacy_err: clap::Error) -> clap::Error { + match legacy_err.kind() { + clap::error::ErrorKind::ArgumentConflict => { + mixed_legacy_command_error(&legacy_err).unwrap_or(legacy_err) + } + clap::error::ErrorKind::InvalidValue | clap::error::ErrorKind::ValueValidation => { + legacy_err + } + _ => primary_err, + } +} + +fn mixed_legacy_command_error(err: &clap::Error) -> Option { + let command = match err.get(clap::error::ContextKind::InvalidSubcommand)? { + clap::error::ContextValue::String(command) => command.as_str(), + _ => return None, + }; + + let message = if command == "run" { + "VM launch options must be passed after the `run` subcommand.\n\nUse:\n openvmm run [VM launch options]\n\nFor example:\n openvmm run --memory 4\n\nIf you intended `run` as a value, pass it to the option that should receive it, for example:\n openvmm --memory 4 -c run\n" + .to_owned() + } else { + format!( + "VM launch options cannot be used with the `{command}` command.\n\nUse `openvmm run ...` for VM launch options, or remove the launch options before `{command}`.\n" + ) + }; + + Some(clap::Error::raw( + clap::error::ErrorKind::ArgumentConflict, + message, + )) +} + +fn legacy_options_from( + primary_err: clap::Error, + args: &[OsString], +) -> Result { + let opt = LegacyOptions::try_parse_from(args.iter().cloned()) + .map_err(|err| preferred_parse_error(primary_err, err))?; + if opt.legacy_grpc.is_some() || opt.legacy_ttrpc.is_some() { + return Err(clap::Error::raw( + clap::error::ErrorKind::UnknownArgument, + "the --grpc and --ttrpc VM launch options have moved to the serve command\n\nUse one of:\n openvmm serve --transport grpc \n openvmm serve --transport ttrpc \n", + )); + } + if opt.command.is_some() { + unreachable!("should have been rejected by clap due to args_conflicts_with_subcommands") + } + let mut run = opt.run; + run.deprecated_memory = DeprecatedMemoryOptions { + prefetch: opt.deprecated_prefetch, + memory_backing_file: opt.deprecated_memory_backing_file, + private_memory: opt.deprecated_private_memory, + thp: opt.deprecated_thp, + }; + Ok(Command::Run(run)) +} + +#[derive(Default)] +struct DeprecatedMemoryOptions { + prefetch: bool, + memory_backing_file: Option, + private_memory: bool, + thp: bool, +} + +/// VM launch options. +#[derive(clap::Args)] +pub struct RunOptions { /// processor count #[clap(short = 'p', long, value_name = "COUNT", default_value = "1")] pub processors: u32, @@ -94,6 +263,9 @@ Examples: )] pub memory: MemoryCli, + #[clap(skip)] + deprecated_memory: DeprecatedMemoryOptions, + /// per-NUMA-node guest RAM sizes (comma-separated, e.g. "2G,2G"). /// Distributes memory across vNUMA nodes reported to the guest. Mutually /// exclusive with --memory. This is for test-only usage. @@ -107,38 +279,11 @@ Examples: #[clap(short = 'M', long, hide = true)] pub shared_memory: bool, - /// prefetch guest RAM - #[clap(long = "prefetch", hide = true)] - pub deprecated_prefetch: bool, - - /// back guest RAM with a file instead of anonymous memory. - /// The file is created/opened and sized to the guest RAM size. - /// Enables snapshot save (fsync) and restore (open + mmap). - #[clap( - long = "memory-backing-file", - value_name = "FILE", - hide = true, - conflicts_with = "deprecated_private_memory" - )] - pub deprecated_memory_backing_file: Option, - /// Restore VM from a snapshot directory (implies file-backed memory from /// the snapshot's memory.bin). Cannot be used with --memory-backing-file. - #[clap( - long, - value_name = "DIR", - conflicts_with = "deprecated_memory_backing_file" - )] + #[clap(long, value_name = "DIR")] pub restore_snapshot: Option, - /// use private anonymous memory for guest RAM - #[clap(long = "private-memory", hide = true, conflicts_with_all = ["deprecated_memory_backing_file", "restore_snapshot"])] - pub deprecated_private_memory: bool, - - /// enable transparent huge pages for guest RAM (Linux only, requires --private-memory) - #[clap(long = "thp", hide = true)] - pub deprecated_thp: bool, - /// start in paused state #[clap(short = 'P', long)] pub paused: bool, @@ -403,11 +548,11 @@ options: pub com4: Option, /// vmbus com1 serial binding (console | stderr | listen=\ | file=\ (overwrites) | listen=tcp:\:\ | term[=\]\[,name=\\] | none) - #[structopt(long, value_name = "SERIAL")] + #[clap(long, value_name = "SERIAL")] pub vmbus_com1_serial: Option, /// vmbus com2 serial binding (console | stderr | listen=\ | file=\ (overwrites) | listen=tcp:\:\ | term[=\]\[,name=\\] | none) - #[structopt(long, value_name = "SERIAL")] + #[clap(long, value_name = "SERIAL")] pub vmbus_com2_serial: Option, /// Only allow guest to host serial traffic @@ -547,13 +692,11 @@ options: #[clap(long, value_name = "PATH")] pub pidfile: Option, - /// run as a ttrpc server on the specified Unix socket - #[clap(long, value_name = "SOCKETPATH")] - pub ttrpc: Option, - - /// run as a grpc server on the specified Unix socket - #[clap(long, value_name = "SOCKETPATH", conflicts_with("ttrpc"))] - pub grpc: Option, + /// Listen for REPL attach connections on the specified socket path. + /// + /// Requires a process mesh. + #[clap(long, value_name = "SOCKETPATH", conflicts_with = "single_process")] + pub mesh_listen: Option, /// do not launch child processes #[clap(long)] @@ -911,7 +1054,7 @@ Syntax: : pub vfio: Vec, } -impl Options { +impl RunOptions { /// Returns the effective guest RAM size. pub fn memory_size(&self) -> u64 { self.memory.mem_size @@ -919,17 +1062,17 @@ impl Options { /// Returns whether guest RAM should be prefetched. pub fn prefetch_memory(&self) -> bool { - self.memory.prefetch || self.deprecated_prefetch + self.memory.prefetch || self.deprecated_memory.prefetch } /// Returns whether guest RAM should use private anonymous backing. pub fn private_memory(&self) -> bool { - self.memory.shared == Some(false) || self.deprecated_private_memory + self.memory.shared == Some(false) || self.deprecated_memory.private_memory } /// Returns whether guest RAM should be marked THP-eligible. pub fn transparent_hugepages(&self) -> bool { - self.memory.transparent_hugepages || self.deprecated_thp + self.memory.transparent_hugepages || self.deprecated_memory.thp } /// Returns the effective file backing path for guest RAM. @@ -937,18 +1080,34 @@ impl Options { self.memory .file .as_ref() - .or(self.deprecated_memory_backing_file.as_ref()) + .or(self.deprecated_memory.memory_backing_file.as_ref()) + } + + pub(crate) fn deprecated_prefetch(&self) -> bool { + self.deprecated_memory.prefetch + } + + pub(crate) fn deprecated_private_memory(&self) -> bool { + self.deprecated_memory.private_memory + } + + pub(crate) fn deprecated_thp(&self) -> bool { + self.deprecated_memory.thp + } + + pub(crate) fn deprecated_memory_backing_file(&self) -> bool { + self.deprecated_memory.memory_backing_file.is_some() } /// Validates combinations that span the new `--memory` parser and legacy aliases. pub fn validate_memory_options(&self) -> anyhow::Result<()> { - if self.memory.file.is_some() && self.deprecated_memory_backing_file.is_some() { + if self.memory.file.is_some() && self.deprecated_memory.memory_backing_file.is_some() { anyhow::bail!("--memory file=... conflicts with --memory-backing-file"); } if self.memory.file.is_some() && self.restore_snapshot.is_some() { anyhow::bail!("--memory file=... conflicts with --restore-snapshot"); } - if self.memory.shared == Some(true) && self.deprecated_private_memory { + if self.memory.shared == Some(true) && self.deprecated_memory.private_memory { anyhow::bail!("--memory shared=on conflicts with --private-memory"); } if self.memory_backing_file().is_some() && self.private_memory() { @@ -975,6 +1134,72 @@ impl Options { } } +/// Management RPC server options. +#[derive(clap::Args)] +pub struct ServeOptions { + /// RPC transport to host. + #[clap(long, value_enum, value_name = "grpc|ttrpc", required = true)] + pub transport: RpcTransportCli, + + /// Management RPC server socket path. + pub socket_path: PathBuf, + + /// Write the process ID to the specified file on startup, and remove it on + /// clean exit. + #[clap(long, value_name = "PATH")] + pub pidfile: Option, + + /// Listen for REPL attach connections on the specified socket path. + #[clap(long, value_name = "SOCKETPATH", conflicts_with = "single_process")] + pub mesh_listen: Option, + + /// do not launch child processes + #[clap(long)] + pub single_process: bool, +} + +/// Management RPC server transport. +#[derive(Copy, Clone, Debug, PartialEq, ValueEnum)] +pub enum RpcTransportCli { + /// gRPC over Unix socket. + Grpc, + /// ttrpc over Unix socket. + Ttrpc, +} + +/// Top-level openvmm commands. +#[expect(clippy::large_enum_variant)] +#[derive(clap::Subcommand)] +pub enum Command { + /// Launch a VM. + Run(RunOptions), + + /// Host the management RPC API. + Serve(ServeOptions), + + /// Attach an interactive REPL to a running VM. + Attach { + /// Socket path exposed by --mesh-listen. + socket_path: PathBuf, + }, + /// Inspect a running VM once and exit. + Inspect { + /// Enumerate state recursively. + #[clap(short, long)] + recursive: bool, + /// The recursive depth limit. + #[clap(short, long, requires("recursive"))] + limit: Option, + /// Target the paravisor. + #[clap(short = 'v', long)] + paravisor: bool, + /// Socket path exposed by --mesh-listen. + socket_path: PathBuf, + /// Inspect element path. + element: Option, + }, +} + #[derive(Clone, Debug, PartialEq)] pub struct FsArgs { pub tag: String, @@ -3670,7 +3895,7 @@ mod tests { #[test] fn test_memory_options_merge_legacy_aliases() { - let opt = Options::try_parse_from([ + let opt = try_parse_options_from([ "openvmm", "--memory", "2G", @@ -3679,6 +3904,9 @@ mod tests { "--thp", ]) .unwrap(); + let Command::Run(opt) = opt else { + panic!("expected run command"); + }; opt.validate_memory_options().unwrap(); assert_eq!(opt.memory_size(), 2 * 1024 * 1024 * 1024); assert!(opt.prefetch_memory()); @@ -3688,7 +3916,10 @@ mod tests { #[test] fn test_memory_options_allow_legacy_thp_with_new_private_memory() { - let opt = Options::try_parse_from(["openvmm", "--memory", "shared=off", "--thp"]).unwrap(); + let opt = try_parse_options_from(["openvmm", "--memory", "shared=off", "--thp"]).unwrap(); + let Command::Run(opt) = opt else { + panic!("expected run command"); + }; opt.validate_memory_options().unwrap(); assert!(opt.private_memory()); assert!(opt.transparent_hugepages()); @@ -3696,19 +3927,25 @@ mod tests { #[test] fn test_memory_options_reject_conflicting_legacy_aliases() { - let opt = Options::try_parse_from(["openvmm", "--memory", "shared=on", "--private-memory"]) + let opt = try_parse_options_from(["openvmm", "--memory", "shared=on", "--private-memory"]) .unwrap(); + let Command::Run(opt) = opt else { + panic!("expected run command"); + }; assert!(opt.validate_memory_options().is_err()); } #[test] fn test_memory_options_reject_hugepage_legacy_conflicts() { let opt = - Options::try_parse_from(["openvmm", "--memory", "hugepages=on", "--private-memory"]) + try_parse_options_from(["openvmm", "--memory", "hugepages=on", "--private-memory"]) .unwrap(); + let Command::Run(opt) = opt else { + panic!("expected run command"); + }; assert!(opt.validate_memory_options().is_err()); - let opt = Options::try_parse_from([ + let opt = try_parse_options_from([ "openvmm", "--memory", "hugepages=on", @@ -3716,12 +3953,230 @@ mod tests { "/tmp/memory.bin", ]) .unwrap(); + let Command::Run(opt) = opt else { + panic!("expected run command"); + }; assert!(opt.validate_memory_options().is_err()); } + #[test] + fn test_run_subcommand_rejects_deprecated_memory_aliases() { + let err = match try_parse_options_from(["openvmm", "run", "--prefetch"]) { + Ok(_) => panic!("deprecated memory alias should not be accepted by run"), + Err(err) => err, + }; + assert_eq!(err.kind(), clap::error::ErrorKind::UnknownArgument); + } + #[test] fn test_pidfile_option_parsed() { - let opt = Options::try_parse_from(["openvmm", "--pidfile", "/tmp/test.pid"]).unwrap(); - assert_eq!(opt.pidfile, Some(PathBuf::from("/tmp/test.pid"))); + let opt = try_parse_options_from(["openvmm", "--pidfile", "/tmp/test.pid"]).unwrap(); + let Command::Run(run) = opt else { + panic!("expected implicit run options"); + }; + assert_eq!(run.pidfile, Some(PathBuf::from("/tmp/test.pid"))); + } + + #[test] + fn test_run_subcommand_parses_launch_options() { + let opt = try_parse_options_from(["openvmm", "run", "-p", "2"]).unwrap(); + let Command::Run(run) = opt else { + panic!("expected run command"); + }; + assert_eq!(run.processors, 2); + } + + #[test] + fn test_serve_subcommand_parses_grpc_transport() { + let opt = try_parse_options_from([ + "openvmm", + "serve", + "--transport", + "grpc", + "/tmp/openvmm.sock", + ]) + .unwrap(); + let Command::Serve(serve) = opt else { + panic!("expected serve command"); + }; + assert_eq!(serve.transport, RpcTransportCli::Grpc); + assert_eq!(serve.socket_path, PathBuf::from("/tmp/openvmm.sock")); + } + + #[test] + fn test_serve_subcommand_parses_ttrpc_transport() { + let opt = try_parse_options_from([ + "openvmm", + "serve", + "--transport", + "ttrpc", + "--pidfile", + "/tmp/openvmm.pid", + "/tmp/openvmm.sock", + ]) + .unwrap(); + let Command::Serve(serve) = opt else { + panic!("expected serve command"); + }; + assert_eq!(serve.transport, RpcTransportCli::Ttrpc); + assert_eq!(serve.socket_path, PathBuf::from("/tmp/openvmm.sock")); + assert_eq!(serve.pidfile, Some(PathBuf::from("/tmp/openvmm.pid"))); + } + + #[test] + fn test_attach_subcommand_parses_socket_path() { + let opt = + try_parse_options_from(["openvmm", "attach", "/tmp/openvmm-attach.sock"]).unwrap(); + let Command::Attach { socket_path } = opt else { + panic!("expected attach command"); + }; + assert_eq!(socket_path, PathBuf::from("/tmp/openvmm-attach.sock")); + } + + #[test] + fn test_inspect_subcommand_parses_options() { + let opt = try_parse_options_from([ + "openvmm", + "inspect", + "--recursive", + "--limit", + "2", + "--paravisor", + "/tmp/openvmm-attach.sock", + "devices/vmbus", + ]) + .unwrap(); + let Command::Inspect { + recursive, + limit, + paravisor, + socket_path, + element, + } = opt + else { + panic!("expected inspect command"); + }; + assert!(recursive); + assert_eq!(limit, Some(2)); + assert!(paravisor); + assert_eq!(socket_path, PathBuf::from("/tmp/openvmm-attach.sock")); + assert_eq!(element.as_deref(), Some("devices/vmbus")); + } + + #[test] + fn test_inspect_limit_requires_recursive() { + let err = match try_parse_options_from([ + "openvmm", + "inspect", + "--limit", + "2", + "/tmp/openvmm-attach.sock", + ]) { + Ok(_) => panic!("--limit without --recursive should fail"), + Err(err) => err, + }; + assert_eq!(err.kind(), clap::error::ErrorKind::MissingRequiredArgument); + assert!(err.to_string().contains("--recursive")); + } + + #[test] + fn test_serve_transport_required() { + let err = match try_parse_options_from(["openvmm", "serve", "/tmp/openvmm.sock"]) { + Ok(_) => panic!("missing --transport should fail"), + Err(err) => err, + }; + assert_eq!(err.kind(), clap::error::ErrorKind::MissingRequiredArgument); + } + + #[test] + fn test_unknown_option_before_command_reports_top_level_error() { + let err = match try_parse_options_from(["openvmm", "--foo", "inspect"]) { + Ok(_) => panic!("unknown option should fail"), + Err(err) => err, + }; + let message = err.to_string(); + println!("Error message: {}", message); + assert_eq!(err.kind(), clap::error::ErrorKind::UnknownArgument); + assert!(message.contains("unexpected argument '--foo'")); + assert!(message.contains("Usage: openvmm ")); + assert!(!message.contains("--uefi-console-mode")); + } + + #[test] + fn test_invalid_legacy_run_option_reports_run_error() { + let err = match try_parse_options_from(["openvmm", "--memory", "nope"]) { + Ok(_) => panic!("invalid memory value should fail"), + Err(err) => err, + }; + let message = err.to_string(); + assert_eq!(err.kind(), clap::error::ErrorKind::ValueValidation); + assert!(message.contains("invalid value 'nope'")); + assert!(message.contains("--memory ")); + } + + #[test] + fn test_legacy_rpc_options_report_migration_error() { + for args in [ + ["openvmm", "--grpc", "/tmp/sock"].as_slice(), + ["openvmm", "--ttrpc", "/tmp/sock"].as_slice(), + ] { + let err = match try_parse_options_from(args) { + Ok(_) => panic!("legacy RPC option should fail"), + Err(err) => err, + }; + let message = err.to_string(); + println!("Error message: {}", message); + assert_eq!(err.kind(), clap::error::ErrorKind::UnknownArgument); + assert!(message.contains("openvmm serve --transport grpc ")); + assert!(message.contains("openvmm serve --transport ttrpc ")); + } + } + + #[test] + fn test_launch_options_conflict_with_other_commands() { + let err = match try_parse_options_from([ + "openvmm", + "-p", + "2", + "inspect", + "/tmp/openvmm-attach.sock", + "/", + ]) { + Ok(_) => panic!("expected launch options to conflict with inspect"), + Err(err) => err, + }; + assert_eq!(err.kind(), clap::error::ErrorKind::ArgumentConflict); + let message = err.to_string(); + assert!(message.contains("VM launch options cannot be used with the `inspect` command")); + assert!(message.contains("Use `openvmm run ...` for VM launch options")); + + let err = match try_parse_options_from([ + "openvmm", + "-p", + "2", + "serve", + "--transport", + "grpc", + "/tmp/openvmm.sock", + ]) { + Ok(_) => panic!("expected launch options to conflict with serve"), + Err(err) => err, + }; + assert_eq!(err.kind(), clap::error::ErrorKind::ArgumentConflict); + let message = err.to_string(); + assert!(message.contains("VM launch options cannot be used with the `serve` command")); + } + + #[test] + fn test_launch_options_before_run_suggests_run_order() { + let err = match try_parse_options_from(["openvmm", "--memory", "4", "run"]) { + Ok(_) => panic!("legacy launch options before run should fail"), + Err(err) => err, + }; + let message = err.to_string(); + assert_eq!(err.kind(), clap::error::ErrorKind::ArgumentConflict); + assert!(message.contains("VM launch options must be passed after the `run` subcommand")); + assert!(message.contains("openvmm run [VM launch options]")); + assert!(message.contains("openvmm --memory 4 -c run")); } } diff --git a/openvmm/openvmm_entry/src/lib.rs b/openvmm/openvmm_entry/src/lib.rs index c83aa291d1..1b4e423539 100644 --- a/openvmm/openvmm_entry/src/lib.rs +++ b/openvmm/openvmm_entry/src/lib.rs @@ -16,18 +16,21 @@ mod serial_io; mod storage_builder; mod tracing_init; mod ttrpc; +mod vm_connect; mod vm_controller; // `pub` so that the missing_docs warning fires for options without // documentation. pub use cli_args::Options; -use console_relay::ConsoleLaunchOptions; +pub use cli_args::RpcTransportCli; +pub use cli_args::RunOptions; +pub use cli_args::ServeOptions; use crate::cli_args::SecureBootTemplateCli; use anyhow::Context; use anyhow::bail; use chipset_resources::battery::HostBatteryUpdate; -use clap::Parser; +use cli_args::Command; use cli_args::DiskCliKind; use cli_args::EfiDiagnosticsLogLevelCli; use cli_args::EndpointConfigCli; @@ -37,6 +40,7 @@ use cli_args::SerialConfigCli; use cli_args::UefiConsoleModeCli; use cli_args::VirtioBusCli; use cli_args::VmgsCli; +use console_relay::ConsoleLaunchOptions; use crash_dump::spawn_dump_handler; use disk_backend_resources::DelayDiskHandle; use disk_backend_resources::DiskLayerDescription; @@ -57,6 +61,7 @@ use gdma_resources::VportDefinition; use guid::Guid; use input_core::MultiplexedInputHandle; use inspect::InspectMut; +use inspect::InspectionBuilder; use io::Read; use memory_range::MemoryRange; use mesh::CancelContext; @@ -219,7 +224,7 @@ fn build_switch_list(all_switches: &[cli_args::GenericPcieSwitchCli]) -> Vec anyhow::Result<(Config, VmResources)> { let (_, serial_driver) = DefaultPool::spawn_on_thread("serial"); // Ensure the serial driver stays alive with no tasks. @@ -409,16 +414,16 @@ async fn vm_config_from_command_line( if opt.shared_memory { tracing::warn!("--shared-memory/-M flag has no effect and will be removed"); } - if opt.deprecated_prefetch { + if opt.deprecated_prefetch() { tracing::warn!("--prefetch is deprecated; use --memory prefetch=on"); } - if opt.deprecated_private_memory { + if opt.deprecated_private_memory() { tracing::warn!("--private-memory is deprecated; use --memory shared=off"); } - if opt.deprecated_thp { + if opt.deprecated_thp() { tracing::warn!("--thp is deprecated; use --memory shared=off,thp=on"); } - if opt.deprecated_memory_backing_file.is_some() { + if opt.deprecated_memory_backing_file() { tracing::warn!("--memory-backing-file is deprecated; use --memory file="); } @@ -1695,19 +1700,23 @@ pub(crate) fn openvmm_terminal_app() -> Option { } // Tries to remove `path` if it is confirmed to be a Unix socket. -fn cleanup_socket(path: &Path) { - #[cfg(windows)] - let is_socket = pal::windows::fs::is_unix_socket(path).unwrap_or(false); - #[cfg(not(windows))] - let is_socket = path - .metadata() - .is_ok_and(|meta| std::os::unix::fs::FileTypeExt::is_socket(&meta.file_type())); - - if is_socket { +pub(crate) fn cleanup_socket(path: &Path) { + if path_is_socket(path).unwrap_or(false) { let _ = std::fs::remove_file(path); } } +#[cfg(windows)] +fn path_is_socket(path: &Path) -> io::Result { + pal::windows::fs::is_unix_socket(path) +} + +#[cfg(not(windows))] +fn path_is_socket(path: &Path) -> io::Result { + let file_type = std::fs::symlink_metadata(path)?.file_type(); + Ok(std::os::unix::fs::FileTypeExt::is_socket(&file_type)) +} + #[cfg(windows)] const DEFAULT_SWITCH: &str = "C08CB7B8-9B3C-408E-8E30-5E16A3AEB444"; @@ -2062,7 +2071,7 @@ pub(crate) const GUEST_ARCH: &str = if cfg!(guest_arch = "x86_64") { /// Returns the shared memory fd (from memory.bin) and the saved device state. fn prepare_snapshot_restore( snapshot_dir: &Path, - opt: &Options, + opt: &RunOptions, ) -> anyhow::Result<( openvmm_defs::worker::SharedMemoryFd, mesh::payload::message::ProtobufMessage, @@ -2116,58 +2125,219 @@ fn do_main(pidfile_path: &mut Option) -> anyhow::Result<()> { // not return). Any worker host setup errors are return and bubbled up. meshworker::run_vmm_mesh_host()?; - let opt = Options::parse(); - if let Some(path) = &opt.write_saved_state_proto { + let run_options = match cli_args::parse_options() { + Command::Run(run_options) => run_options, + command => { + return DefaultPool::run_with(async |driver| run_command(&driver, command).await); + } + }; + + if let Some(path) = &run_options.write_saved_state_proto { mesh::payload::protofile::DescriptorWriter::new(vmcore::save_restore::saved_state_roots()) .write_to_path(path) .context("failed to write protobuf descriptors")?; return Ok(()); } - if let Some(ref path) = opt.pidfile { + *pidfile_path = write_pidfile(&run_options.pidfile)?; + + if let Some(path) = run_options.relay_console_path { + let console_title = run_options.relay_console_title.unwrap_or_default(); + return console_relay::relay_console(&path, console_title.as_str()); + } + + DefaultPool::run_with(async |driver| run_control(&driver, run_options).await) +} + +fn write_pidfile(path: &Option) -> anyhow::Result> { + if let Some(path) = path { std::fs::write(path, format!("{}\n", std::process::id())) .context("failed to write pidfile")?; - *pidfile_path = Some(path.clone()); + Ok(Some(path.clone())) + } else { + Ok(None) } +} - if let Some(path) = opt.relay_console_path { - let console_title = opt.relay_console_title.unwrap_or_default(); - return console_relay::relay_console(&path, console_title.as_str()); +#[cfg(any(feature = "grpc", feature = "ttrpc"))] +async fn run_management_server(driver: &DefaultDriver, opt: ServeOptions) -> anyhow::Result<()> { + let path = opt.socket_path.clone(); + cleanup_socket(&path); + let listener = unix_socket::UnixListener::bind(&path) + .with_context(|| format!("failed to bind to socket {}", path.display()))?; + + let transport = match opt.transport { + #[cfg(feature = "grpc")] + RpcTransportCli::Grpc => ttrpc::RpcTransport::Grpc, + #[cfg(not(feature = "grpc"))] + RpcTransportCli::Grpc => bail!("grpc transport is not enabled in this build"), + #[cfg(feature = "ttrpc")] + RpcTransportCli::Ttrpc => ttrpc::RpcTransport::Ttrpc, + #[cfg(not(feature = "ttrpc"))] + RpcTransportCli::Ttrpc => bail!("ttrpc transport is not enabled in this build"), + }; + + let mesh = VmmMesh::new(driver, opt.single_process)?; + let (vm_controller_send, vm_controller_recv) = mesh::channel(); + let (event_send, event_recv) = mesh::channel(); + let mut controller = vm_controller::VmController { + driver: driver.clone(), + mesh, + vm_controller: vm_controller_send.clone(), + current_vm: None, + attach_path: opt.mesh_listen, + attach_listener: None, + exit_on_vm_stop: false, + }; + if let Err(err) = controller.start_attach_listener().await { + vm_controller_send.send(vm_controller::VmControllerRpc::Quit); + controller.run(vm_controller_recv, event_send).await; + return Err(err); } - #[cfg(any(feature = "grpc", feature = "ttrpc"))] - if let Some(path) = opt.ttrpc.as_ref().or(opt.grpc.as_ref()) { - return block_on(async { - let _ = std::fs::remove_file(path); - let listener = - unix_socket::UnixListener::bind(path).context("failed to bind to socket")?; + let controller_task = driver.spawn( + "vm-controller", + controller.run(vm_controller_recv, event_send), + ); - let transport = if opt.ttrpc.is_some() { - ttrpc::RpcTransport::Ttrpc - } else { - ttrpc::RpcTransport::Grpc - }; + let mut server_worker = + match mesh_worker::launch_local_worker::(ttrpc::Parameters { + listener, + transport, + vm_controller: vm_controller_send.clone(), + vm_controller_events: event_recv, + }) + .await + { + Ok(worker) => worker, + Err(err) => { + vm_controller_send.send(vm_controller::VmControllerRpc::Quit); + controller_task.await; + return Err(err); + } + }; - // This is a local launch - let mut handle = - mesh_worker::launch_local_worker::(ttrpc::Parameters { - listener, - transport, - }) - .await?; + tracing::info!(%transport, path = %path.display(), "listening"); - tracing::info!(%transport, path = %path.display(), "listening"); + let pidfile_path = write_pidfile(&opt.pidfile)?; - // Signal the the parent process that the server is ready. - pal::close_stdout().context("failed to close stdout")?; + // Signal the the parent process that the server is ready. + let result = async { + pal::close_stdout().context("failed to close stdout")?; - handle.join().await?; + let server_result = server_worker.join().await; + vm_controller_send.send(vm_controller::VmControllerRpc::Quit); + controller_task.await; - Ok(()) - }); + server_result } + .await; - DefaultPool::run_with(async |driver| run_control(&driver, opt).await) + if let Some(path) = pidfile_path { + let _ = std::fs::remove_file(path); + } + + result +} + +#[cfg(not(any(feature = "grpc", feature = "ttrpc")))] +async fn run_management_server(_driver: &DefaultDriver, opt: ServeOptions) -> anyhow::Result<()> { + match opt.transport { + RpcTransportCli::Grpc => bail!("grpc transport is not enabled in this build"), + RpcTransportCli::Ttrpc => bail!("ttrpc transport is not enabled in this build"), + } +} + +async fn run_command(driver: &DefaultDriver, command: Command) -> anyhow::Result<()> { + match command { + Command::Run(_) => unreachable!("run commands are handled before run_command"), + Command::Serve(opt) => run_management_server(driver, opt).await, + Command::Attach { socket_path } => attach_repl(driver, &socket_path).await, + Command::Inspect { + recursive, + limit, + paravisor, + socket_path, + element, + } => { + inspect_attached_vm( + driver, + &socket_path, + element.as_deref(), + recursive, + limit, + paravisor, + ) + .await + } + } +} + +async fn connect_to_vm( + driver: &DefaultDriver, + socket_path: &Path, +) -> anyhow::Result<(vm_connect::VmConnectResponse, mesh_process::Connection)> { + let (request_send, connection) = + mesh_process::connect::(driver.clone(), socket_path) + .await + .with_context(|| format!("failed to connect to {}", socket_path.display()))?; + let (response_send, response_recv) = mesh::oneshot(); + request_send.send(vm_connect::VmConnectRequest { + response: response_send, + }); + + match response_recv + .await + .context("failed to receive VM connect response") + { + Ok(Ok(response)) => Ok((response, connection)), + Ok(Err(err)) => { + connection.shutdown().await; + Err(err.into()) + } + Err(err) => { + connection.shutdown().await; + Err(err) + } + } +} + +async fn attach_repl(driver: &DefaultDriver, socket_path: &Path) -> anyhow::Result<()> { + let (response, connection) = connect_to_vm(driver, socket_path).await?; + let result = repl::run_repl(driver, response.into_repl_resources()).await; + connection.shutdown().await; + result +} + +async fn inspect_attached_vm( + driver: &DefaultDriver, + socket_path: &Path, + element: Option<&str>, + recursive: bool, + limit: Option, + paravisor: bool, +) -> anyhow::Result<()> { + let (response, connection) = connect_to_vm(driver, socket_path).await?; + let vm_controller = response.vm_controller; + let target = if paravisor { + vm_controller::InspectTarget::Paravisor + } else { + vm_controller::InspectTarget::Host + }; + let element = element.unwrap_or_default(); + let depth = if recursive { limit } else { Some(0) }; + let mut inspection = InspectionBuilder::new(element) + .depth(depth) + .inspect(inspect::adhoc_mut(|req| { + vm_controller.send(vm_controller::VmControllerRpc::Inspect(target, req.defer())); + })); + let _ = CancelContext::new() + .with_timeout(Duration::from_secs(1)) + .until_cancelled(inspection.resolve()) + .await; + println!("{:#}", inspection.results()); + connection.shutdown().await; + Ok(()) } fn new_hvsock_service_id(port: u32) -> Guid { @@ -2179,24 +2349,17 @@ fn new_hvsock_service_id(port: u32) -> Guid { } } -async fn run_control(driver: &DefaultDriver, opt: Options) -> anyhow::Result<()> { - let mut mesh = Some(VmmMesh::new(&driver, opt.single_process)?); - let result = run_control_inner(driver, &mut mesh, opt).await; - // If setup failed before the mesh was handed to the controller, shut it - // down so the child host process exits cleanly without noisy logs. - if let Some(mesh) = mesh { - mesh.shutdown().await; - } - result +async fn run_control(driver: &DefaultDriver, opt: RunOptions) -> anyhow::Result<()> { + let mesh = VmmMesh::new(driver, opt.single_process)?; + run_control_inner(driver, mesh, opt).await } async fn run_control_inner( driver: &DefaultDriver, - mesh_slot: &mut Option, - opt: Options, + mesh: VmmMesh, + opt: RunOptions, ) -> anyhow::Result<()> { - let mesh = mesh_slot.as_ref().unwrap(); - let (mut vm_config, mut resources) = vm_config_from_command_line(driver, mesh, &opt).await?; + let (mut vm_config, mut resources) = vm_config_from_command_line(driver, &mesh, &opt).await?; let mut vnc_worker = None; if opt.gfx || opt.vnc { @@ -2332,27 +2495,47 @@ async fn run_control_inner( let has_vtl2 = resources.vtl2_settings.is_some(); // Build the VmController with exclusive resources. - let controller = vm_controller::VmController { - mesh: mesh_slot.take().unwrap(), - vm_worker, - vnc_worker, - gdb_worker, - diag_inspector: Some(diag_inspector), - vtl2_settings: resources.vtl2_settings, - ged_rpc: resources.ged_rpc.clone(), - vm_rpc: vm_rpc.clone(), - paravisor_diag: Some(paravisor_diag), - igvm_path: opt.igvm.clone(), - memory_backing_file: opt.memory_backing_file().cloned(), - memory: opt.memory_size(), - processors: opt.processors, - log_file: opt.log_file.clone(), + let mut controller = vm_controller::VmController { + driver: driver.clone(), + mesh, + vm_controller: vm_controller_send.clone(), + current_vm: Some(vm_controller::CurrentVm { + vm_worker, + vnc_worker, + gdb_worker, + diag_inspector: Some(diag_inspector), + vtl2_settings: resources.vtl2_settings, + ged_rpc: resources.ged_rpc.clone(), + vm_rpc: vm_rpc.clone(), + scsi_rpc: resources.scsi_rpc.clone(), + nvme_vtl2_rpc: resources.nvme_vtl2_rpc.clone(), + shutdown_ic: resources.shutdown_ic.clone(), + kvp_ic: resources.kvp_ic.clone(), + paravisor_diag: Some(paravisor_diag), + igvm_path: opt.igvm.clone(), + memory_backing_file: opt.memory_backing_file().cloned(), + memory: opt.memory_size(), + processors: opt.processors, + log_file: opt.log_file.clone(), + notify_recv, + }), + attach_path: opt.mesh_listen.clone(), + attach_listener: None, + exit_on_vm_stop: true, }; + if let Err(err) = controller.start_attach_listener().await { + vm_controller_send.send(vm_controller::VmControllerRpc::Quit); + controller + .run(vm_controller_recv, vm_controller_event_send) + .await; + return Err(err); + } + // Spawn the VmController as a task. let controller_task = driver.spawn( "vm-controller", - controller.run(vm_controller_recv, vm_controller_event_send, notify_recv), + controller.run(vm_controller_recv, vm_controller_event_send), ); // Run the REPL with shareable resources. @@ -2368,6 +2551,7 @@ async fn run_control_inner( kvp_ic: resources.kvp_ic, console_in: resources.console_in, has_vtl2, + quit_behavior: repl::ReplQuitBehavior::QuitVm, }, ) .await; diff --git a/openvmm/openvmm_entry/src/meshworker.rs b/openvmm/openvmm_entry/src/meshworker.rs index 1df9d4ccc9..0f5ebbd932 100644 --- a/openvmm/openvmm_entry/src/meshworker.rs +++ b/openvmm/openvmm_entry/src/meshworker.rs @@ -77,6 +77,12 @@ impl VmmMesh { Ok(host) } + pub fn process_mesh(&self) -> anyhow::Result<&Mesh> { + self.mesh + .as_ref() + .context("external mesh listeners require process mesh") + } + pub async fn shutdown(self) { if let Some(mesh) = self.mesh { mesh.shutdown().await; diff --git a/openvmm/openvmm_entry/src/repl.rs b/openvmm/openvmm_entry/src/repl.rs index 2b35faa115..2bb231d28e 100644 --- a/openvmm/openvmm_entry/src/repl.rs +++ b/openvmm/openvmm_entry/src/repl.rs @@ -390,6 +390,12 @@ pub(crate) struct ReplResources { pub kvp_ic: Option>, pub console_in: Option>, pub has_vtl2: bool, + pub quit_behavior: ReplQuitBehavior, +} + +pub(crate) enum ReplQuitBehavior { + QuitVm, + Detach, } /// Run the interactive REPL. @@ -407,6 +413,7 @@ pub(crate) async fn run_repl( kvp_ic, console_in, has_vtl2, + quit_behavior, } = resources; let (console_command_send, console_command_recv) = mesh::channel(); @@ -1315,12 +1322,21 @@ pub(crate) async fn run_repl( } } InteractiveCommand::Quit => { - tracing::info!("quitting"); - // Work around the detached SCSI task holding up worker stop. - // TODO: Fix the underlying bug - drop(scsi_rpc.take()); - drop(nvme_vtl2_rpc.take()); - vm_controller.send(VmControllerRpc::Quit); + match quit_behavior { + ReplQuitBehavior::QuitVm => { + tracing::info!("quitting"); + // Work around the detached SCSI task holding up worker stop. + // TODO: Fix the underlying bug + drop(scsi_rpc.take()); + drop(nvme_vtl2_rpc.take()); + vm_controller.send(VmControllerRpc::Quit); + break; + } + ReplQuitBehavior::Detach => { + tracing::info!("detaching"); + break; + } + } } InteractiveCommand::ReadMemory { gpa, size, file } => { let size = size as usize; diff --git a/openvmm/openvmm_entry/src/ttrpc/mod.rs b/openvmm/openvmm_entry/src/ttrpc/mod.rs index 83d0007089..14245a6da8 100644 --- a/openvmm/openvmm_entry/src/ttrpc/mod.rs +++ b/openvmm/openvmm_entry/src/ttrpc/mod.rs @@ -5,10 +5,9 @@ #![cfg(any(feature = "ttrpc", feature = "grpc"))] -use crate::meshworker::VmmMesh; use crate::serial_io::bind_serial; use crate::vm_controller::InspectTarget; -use crate::vm_controller::VmController; +use crate::vm_controller::ServerVmStartParams; use crate::vm_controller::VmControllerEvent; use crate::vm_controller::VmControllerRpc; use anyhow::Context; @@ -42,7 +41,6 @@ use openvmm_defs::config::VirtioBus; use openvmm_defs::config::VmbusConfig; use openvmm_defs::config::VpciDeviceConfig; use openvmm_defs::rpc::VmRpc; -use openvmm_defs::worker::VM_WORKER; use openvmm_defs::worker::VmWorkerParameters; use openvmm_helpers::disk::OpenDiskOptions; use openvmm_helpers::disk::open_disk_type; @@ -70,6 +68,8 @@ use vm_resource::kind::VmbusDeviceHandleKind; pub struct Parameters { pub listener: UnixListener, pub transport: RpcTransport, + pub vm_controller: mesh::Sender, + pub vm_controller_events: mesh::Receiver, } #[derive(Copy, Clone, mesh::MeshPayload)] @@ -98,6 +98,8 @@ enum ResolvedTransport { pub struct TtrpcWorker { listener: UnixListener, transport: ResolvedTransport, + vm_controller: mesh::Sender, + vm_controller_events: mesh::Receiver, } pub const TTRPC_WORKER: WorkerId = WorkerId::new("TtrpcWorker"); @@ -119,6 +121,8 @@ impl Worker for TtrpcWorker { #[allow(unreachable_patterns)] transport => bail!("unsupported transport {transport}"), }, + vm_controller: parameters.vm_controller, + vm_controller_events: parameters.vm_controller_events, }) } @@ -131,16 +135,14 @@ impl Worker for TtrpcWorker { let mut service = VmService { driver, vm: None, - vm_controller: None, - vm_controller_events: None, - controller_task: None, + vm_controller: self.vm_controller, + vm_controller_events: Some(self.vm_controller_events), wait_vm_response: None, halted: false, rpc_tasks: Vec::new(), transport: self.transport, }; - service.run(self.listener, recv).await?; - Ok(()) + service.run(self.listener, recv).await }) } } @@ -257,7 +259,7 @@ impl VmService { break false; } Action::ControllerEvent(Some(event)) => { - self.handle_controller_event(event); + self.handle_controller_event(event).await; } Action::ControllerEvent(None) => {} // handled above Action::WaitVmCancelled(reason) => { @@ -269,14 +271,8 @@ impl VmService { } }; - // If the controller is still alive (non-Quit exit), shut it down. if !quit { - if let Some(controller) = self.vm_controller.take() { - controller.send(VmControllerRpc::Quit); - } - } - if let Some(task) = self.controller_task.take() { - task.await; + self.vm_controller.send(VmControllerRpc::Quit); } // Complete any pending WaitVm with an error. @@ -321,9 +317,8 @@ struct Vm { struct VmService { driver: DefaultDriver, vm: Option>, - vm_controller: Option>, + vm_controller: mesh::Sender, vm_controller_events: Option>, - controller_task: Option>, wait_vm_response: Option<(mesh::CancelContext, mesh::OneshotSender>)>, /// Set when the guest has halted, so that a later `WaitVm` completes /// immediately instead of blocking forever. Cleared on `CreateVm`. @@ -371,13 +366,7 @@ impl VmService { response.send(map_grpc(self.teardown_vm().await)) } vmservice::Vm::Quit((), response) => { - // Shut down the controller (which stops and joins the worker). - if let Some(controller) = self.vm_controller.take() { - controller.send(VmControllerRpc::Quit); - } - if let Some(task) = self.controller_task.take() { - task.await; - } + self.vm_controller.send(VmControllerRpc::Quit); self.vm.take(); self.vm_controller_events.take(); if let Some((_, wait_response)) = self.wait_vm_response.take() { @@ -452,9 +441,8 @@ impl VmService { let mut inspection = InspectionBuilder::new(&request.path) .depth(Some(request.depth as usize)) .inspect(inspect::adhoc(|req| { - if let Some(controller) = &self.vm_controller { - controller.send(VmControllerRpc::Inspect(InspectTarget::Host, req.defer())); - } + self.vm_controller + .send(VmControllerRpc::Inspect(InspectTarget::Host, req.defer())); })); async move { let _ = ctx @@ -476,9 +464,8 @@ impl VmService { &request.path, &request.value, inspect::adhoc(|req| { - if let Some(controller) = &self.vm_controller { - controller.send(VmControllerRpc::Inspect(InspectTarget::Host, req.defer())); - } + self.vm_controller + .send(VmControllerRpc::Inspect(InspectTarget::Host, req.defer())); }), ); async move { @@ -677,77 +664,44 @@ impl VmService { let (send, recv) = mesh::channel(); let (notify_send, notify_recv) = mesh::channel(); - // Create a VmmMesh for local/in-process workers. - let mesh = VmmMesh::new(&self.driver, true)?; - let vm_host = mesh - .make_host("vm", None) - .await - .context("spawning vm process failed")?; - - let worker = vm_host - .launch_worker( - VM_WORKER, - VmWorkerParameters { - hypervisor: openvmm_helpers::hypervisor::choose_hypervisor()?, - cfg: config, - saved_state: None, - shared_memory: None, - rpc: recv, - notify: notify_send, - }, + let handles = self + .vm_controller + .call( + VmControllerRpc::CreateVm, + Box::new(ServerVmStartParams { + worker_params: VmWorkerParameters { + hypervisor: openvmm_helpers::hypervisor::choose_hypervisor()?, + cfg: config, + saved_state: None, + shared_memory: None, + rpc: recv, + notify: notify_send, + }, + vm_rpc: send, + notify_recv, + scsi_rpc, + memory: config_mem_size, + processors: config_proc_count, + }), ) - .await?; - - let memory = config_mem_size; - let processors = config_proc_count; - - // Create channels for VmController. - let (vm_controller_send, vm_controller_recv) = mesh::channel(); - let (event_send, event_recv) = mesh::channel(); - - // Build VmController with no paravisor-specific fields. - let controller = VmController { - mesh, - vm_worker: worker, - vnc_worker: None, - gdb_worker: None, - diag_inspector: None, - vtl2_settings: None, - ged_rpc: None, - vm_rpc: send.clone(), - paravisor_diag: None, - igvm_path: None, - memory_backing_file: None, - memory, - processors, - log_file: None, - }; - - // Spawn the controller task. - let controller_task = self.driver.spawn( - "vm-controller", - controller.run(vm_controller_recv, event_send, notify_recv), - ); + .await + .map_err(anyhow::Error::from) + .and_then(|r| Ok(r?))?; - self.vm_controller = Some(vm_controller_send); - self.vm_controller_events = Some(event_recv); - self.controller_task = Some(controller_task); self.vm = Some(Arc::new(Vm { - scsi_rpc, - worker_rpc: send, + scsi_rpc: handles.scsi_rpc, + worker_rpc: handles.vm_rpc, })); Ok(()) } async fn teardown_vm(&mut self) -> anyhow::Result<()> { - let controller = self.vm_controller.take().context("vm not created")?; - controller.send(VmControllerRpc::Quit); - drop(controller); - if let Some(task) = self.controller_task.take() { - task.await; - } + self.vm_controller + .call(VmControllerRpc::TeardownVm, ()) + .await + .map_err(anyhow::Error::from) + .and_then(|r| Ok(r?))?; self.vm.take(); - self.vm_controller_events.take(); if let Some((_, response)) = self.wait_vm_response.take() { response.send(Err(grpc_error(anyhow!("VM torn down")))); } @@ -764,7 +718,7 @@ impl VmService { async move { recv.await.map(drop).context("resume failed") } } - fn handle_controller_event(&mut self, event: VmControllerEvent) { + async fn handle_controller_event(&mut self, event: VmControllerEvent) { match event { VmControllerEvent::GuestHalt(reason) => { tracing::info!(%reason, "guest halted (via controller)"); @@ -787,10 +741,7 @@ impl VmService { }; response.send(Err(status)); } - // Clear VM state since the worker is gone. The controller - // task will be awaited during final cleanup. self.vm.take(); - self.vm_controller.take(); } VmControllerEvent::VncWorkerStopped { error } => { if let Some(err) = &error { diff --git a/openvmm/openvmm_entry/src/vm_connect.rs b/openvmm/openvmm_entry/src/vm_connect.rs new file mode 100644 index 0000000000..8aa16b1200 --- /dev/null +++ b/openvmm/openvmm_entry/src/vm_connect.rs @@ -0,0 +1,146 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +//! Attach protocol for connecting a REPL to an already running VM. + +use crate::repl; +use crate::vm_controller::VmControllerRpc; +use anyhow::Context; +use futures::FutureExt; +use futures::StreamExt; +use mesh::rpc::RpcSend; +use nvme_resources::NvmeControllerRequest; +use openvmm_defs::rpc::VmRpc; +use pal_async::task::Spawn; +use pal_async::task::Task; +use std::path::Path; +use std::path::PathBuf; +use storvsp_resources::ScsiControllerRequest; + +#[derive(mesh::MeshPayload)] +pub struct VmConnectRequest { + pub response: mesh::OneshotSender>, +} + +#[derive(mesh::MeshPayload)] +pub struct VmConnectResponse { + pub vm_rpc: mesh::Sender, + pub vm_controller: mesh::Sender, + pub scsi_rpc: Option>, + pub nvme_vtl2_rpc: Option>, + pub shutdown_ic: Option>, + pub kvp_ic: Option>, + pub has_vtl2: bool, +} + +impl VmConnectResponse { + pub fn into_repl_resources(self) -> repl::ReplResources { + let (send, recv) = mesh::channel(); + drop(send); + repl::ReplResources { + vm_rpc: self.vm_rpc, + vm_controller: self.vm_controller, + vm_controller_events: recv, + scsi_rpc: self.scsi_rpc, + nvme_vtl2_rpc: self.nvme_vtl2_rpc, + shutdown_ic: self.shutdown_ic, + kvp_ic: self.kvp_ic, + console_in: None, + has_vtl2: self.has_vtl2, + quit_behavior: repl::ReplQuitBehavior::Detach, + } + } +} + +#[derive(Clone, mesh::MeshPayload)] +pub struct AttachResources { + pub vm_rpc: mesh::Sender, + pub vm_controller: mesh::Sender, + pub scsi_rpc: Option>, + pub nvme_vtl2_rpc: Option>, + pub shutdown_ic: Option>, + pub kvp_ic: Option>, + pub has_vtl2: bool, +} + +impl From for VmConnectResponse { + fn from(resources: AttachResources) -> Self { + Self { + vm_rpc: resources.vm_rpc, + vm_controller: resources.vm_controller, + scsi_rpc: resources.scsi_rpc, + nvme_vtl2_rpc: resources.nvme_vtl2_rpc, + shutdown_ic: resources.shutdown_ic, + kvp_ic: resources.kvp_ic, + has_vtl2: resources.has_vtl2, + } + } +} + +pub struct AttachListener { + stop: mesh::OneshotSender<()>, + task: Task<()>, + path: PathBuf, +} + +impl AttachListener { + pub async fn shutdown(self) { + self.stop.send(()); + self.task.await; + crate::cleanup_socket(&self.path); + } +} + +/// Start a mesh listener that hands out VM control channels to connecting +/// clients. +/// +/// Returns a handle that must be kept alive. Call [`AttachListener::shutdown`] +/// to stop accepting, shut down the mesh, join the listener task, and remove +/// the socket file. +pub async fn start_attach_listener( + mesh: &mesh_process::Mesh, + driver: &impl Spawn, + path: &Path, + vm_controller: mesh::Sender, +) -> anyhow::Result { + let mut listener = mesh + .listen::(path) + .await + .with_context(|| format!("failed to listen on attach socket {}", path.display()))?; + let (stop_send, stop_recv) = mesh::oneshot::<()>(); + let path = path.to_owned(); + + let task = driver.spawn("attach-listener", async move { + let mut stop_recv = stop_recv.fuse(); + loop { + enum Action { + Stop, + Request(Option), + } + + let action = futures::select! { + _ = stop_recv => Action::Stop, + request = listener.next().fuse() => Action::Request(request), + }; + + match action { + Action::Stop => break, + Action::Request(Some(request)) => { + tracing::info!("accepted REPL attach connection"); + let response = vm_controller + .call(VmControllerRpc::Connect, ()) + .await + .unwrap_or_else(|err| Err(mesh::error::RemoteError::new(err))); + request.response.send(response); + } + Action::Request(None) => break, + } + } + }); + + Ok(AttachListener { + stop: stop_send, + task, + path, + }) +} diff --git a/openvmm/openvmm_entry/src/vm_controller.rs b/openvmm/openvmm_entry/src/vm_controller.rs index 60c7f20c63..dd7c33e882 100644 --- a/openvmm/openvmm_entry/src/vm_controller.rs +++ b/openvmm/openvmm_entry/src/vm_controller.rs @@ -6,6 +6,7 @@ use crate::DiagInspector; use crate::meshworker::VmmMesh; +use crate::vm_connect; use anyhow::Context; use futures::FutureExt; use futures::StreamExt; @@ -18,6 +19,9 @@ use mesh::rpc::RpcSend; use mesh_worker::WorkerEvent; use mesh_worker::WorkerHandle; use openvmm_defs::rpc::VmRpc; +use openvmm_defs::worker::VM_WORKER; +use openvmm_defs::worker::VmWorkerParameters; +use pal_async::DefaultDriver; use std::path::Path; use std::path::PathBuf; use std::pin::pin; @@ -37,6 +41,12 @@ pub enum InspectTarget { /// remotable in the future. #[derive(mesh::MeshPayload)] pub enum VmControllerRpc { + /// Connect an attach client to the currently running VM. + Connect(Rpc<(), Result>), + /// Create and start a VM worker in server mode. + CreateVm(Rpc, Result>), + /// Stop and drop the current VM worker in server mode. + TeardownVm(Rpc<(), Result<(), mesh::error::RemoteError>>), /// Restart the VM worker. Restart(Rpc<(), Result<(), mesh::error::RemoteError>>), /// Restart the VNC worker. @@ -61,6 +71,22 @@ pub enum VmControllerRpc { Quit, } +#[derive(mesh::MeshPayload)] +pub struct ServerVmStartParams { + pub worker_params: VmWorkerParameters, + pub vm_rpc: mesh::Sender, + pub notify_recv: mesh::Receiver, + pub scsi_rpc: Option>, + pub memory: u64, + pub processors: u32, +} + +#[derive(Clone, mesh::MeshPayload)] +pub struct ServerVmHandles { + pub vm_rpc: mesh::Sender, + pub scsi_rpc: Option>, +} + #[derive(mesh::MeshPayload)] pub struct AddVtl0ScsiDiskParams { pub controller_guid: Guid, @@ -102,9 +128,7 @@ pub enum VmControllerEvent { GuestHalt(String), } -/// Owns exclusive VM resources and services RPCs from the REPL. -pub struct VmController { - pub(crate) mesh: VmmMesh, +pub struct CurrentVm { pub(crate) vm_worker: WorkerHandle, pub(crate) vnc_worker: Option, pub(crate) gdb_worker: Option, @@ -112,12 +136,28 @@ pub struct VmController { pub(crate) vtl2_settings: Option, pub(crate) ged_rpc: Option>, pub(crate) vm_rpc: mesh::Sender, + pub(crate) scsi_rpc: Option>, + pub(crate) nvme_vtl2_rpc: Option>, + pub(crate) shutdown_ic: Option>, + pub(crate) kvp_ic: Option>, pub(crate) paravisor_diag: Option>, pub(crate) igvm_path: Option, pub(crate) memory_backing_file: Option, pub(crate) memory: u64, pub(crate) processors: u32, pub(crate) log_file: Option, + pub(crate) notify_recv: mesh::Receiver, +} + +/// Owns exclusive VM resources and services RPCs from the REPL. +pub struct VmController { + pub(crate) driver: DefaultDriver, + pub(crate) mesh: VmmMesh, + pub(crate) vm_controller: mesh::Sender, + pub(crate) current_vm: Option, + pub(crate) attach_path: Option, + pub(crate) attach_listener: Option, + pub(crate) exit_on_vm_stop: bool, } impl VmController { @@ -127,7 +167,6 @@ impl VmController { mut self, mut rpc_recv: mesh::Receiver, event_send: mesh::Sender, - mut notify_recv: mesh::Receiver, ) { enum Event { Rpc(VmControllerRpc), @@ -138,43 +177,43 @@ impl VmController { } let mut quit = false; - let mut rpc_closed = false; loop { let event = { let rpc = pin!(async { - if rpc_closed { - std::future::pending().await - } else { - match rpc_recv.next().await { - Some(msg) => Event::Rpc(msg), - None => Event::RpcClosed, - } + match rpc_recv.next().await { + Some(msg) => Event::Rpc(msg), + None => Event::RpcClosed, } }); - let vm = (&mut self.vm_worker).map(Event::Worker); - let vnc = futures::stream::iter(self.vnc_worker.as_mut()) - .flatten() - .map(Event::VncWorker); - let halt = (&mut notify_recv).map(Event::Halt); - - (rpc.into_stream(), vm, vnc, halt) - .merge() - .next() - .await - .unwrap() + if let Some(current_vm) = &mut self.current_vm { + let vm = (&mut current_vm.vm_worker).map(Event::Worker); + let vnc = futures::stream::iter(current_vm.vnc_worker.as_mut()) + .flatten() + .map(Event::VncWorker); + let halt = (&mut current_vm.notify_recv).map(Event::Halt); + + (rpc.into_stream(), vm, vnc, halt) + .merge() + .next() + .await + .unwrap() + } else { + rpc.into_stream().next().await.unwrap() + } }; match event { Event::Rpc(rpc) => { self.handle_rpc(rpc, &mut quit).await; + if quit { + break; + } } Event::RpcClosed => { // Controller RPC channel closed (REPL/ttrpc disconnected). // Stop the VM. tracing::info!("controller RPC channel closed, stopping VM"); - self.vm_worker.stop(); - quit = true; - rpc_closed = true; + break; } Event::Worker(event) => match event { WorkerEvent::Stopped => { @@ -184,14 +223,20 @@ impl VmController { tracing::error!("vm worker unexpectedly stopped"); } event_send.send(VmControllerEvent::WorkerStopped { error: None }); - break; + self.teardown_current_vm_after_worker_stop().await; + if self.exit_on_vm_stop { + break; + } } WorkerEvent::Failed(err) => { tracing::error!(error = &err as &dyn std::error::Error, "vm worker failed"); event_send.send(VmControllerEvent::WorkerStopped { error: Some(format!("{err:#}")), }); - break; + self.teardown_current_vm_after_worker_stop().await; + if self.exit_on_vm_stop { + break; + } } WorkerEvent::RestartFailed(err) => { tracing::error!( @@ -234,16 +279,67 @@ impl VmController { } } - // Ensure all workers are cleaned up before shutting down the mesh. - self.vm_worker.stop(); - if let Err(err) = self.vm_worker.join().await { + self.stop_attach_listener().await; + self.stop_current_vm().await; + self.mesh.shutdown().await; + } + + pub(crate) async fn start_attach_listener(&mut self) -> anyhow::Result<()> { + let Some(path) = &self.attach_path else { + return Ok(()); + }; + let listener = vm_connect::start_attach_listener( + self.mesh.process_mesh()?, + &self.driver, + path, + self.vm_controller.clone(), + ) + .await?; + self.attach_listener = Some(listener); + Ok(()) + } + + async fn stop_attach_listener(&mut self) { + if let Some(listener) = self.attach_listener.take() { + listener.shutdown().await; + } + } + + fn attach_resources(&self) -> anyhow::Result { + let current_vm = self.current_vm.as_ref().context("VM not created")?; + Ok(vm_connect::AttachResources { + vm_rpc: current_vm.vm_rpc.clone(), + vm_controller: self.vm_controller.clone(), + scsi_rpc: current_vm.scsi_rpc.clone(), + nvme_vtl2_rpc: current_vm.nvme_vtl2_rpc.clone(), + shutdown_ic: current_vm.shutdown_ic.clone(), + kvp_ic: current_vm.kvp_ic.clone(), + has_vtl2: current_vm.vtl2_settings.is_some(), + }) + } + + fn current_vm(&self) -> anyhow::Result<&CurrentVm> { + self.current_vm.as_ref().context("VM not created") + } + + fn current_vm_mut(&mut self) -> anyhow::Result<&mut CurrentVm> { + self.current_vm.as_mut().context("VM not created") + } + + async fn stop_current_vm(&mut self) { + let Some(mut current_vm) = self.current_vm.take() else { + return; + }; + + current_vm.vm_worker.stop(); + if let Err(err) = current_vm.vm_worker.join().await { tracing::error!( error = err.as_ref() as &dyn std::error::Error, "vm worker join failed" ); } - if let Some(mut vnc) = self.vnc_worker.take() { + if let Some(mut vnc) = current_vm.vnc_worker.take() { vnc.stop(); if let Err(err) = vnc.join().await { tracing::error!( @@ -253,7 +349,7 @@ impl VmController { } } - if let Some(mut gdb) = self.gdb_worker.take() { + if let Some(mut gdb) = current_vm.gdb_worker.take() { gdb.stop(); if let Err(err) = gdb.join().await { tracing::error!( @@ -262,12 +358,36 @@ impl VmController { ); } } + } - self.mesh.shutdown().await; + async fn teardown_current_vm_after_worker_stop(&mut self) { + if let Some(mut current_vm) = self.current_vm.take() { + if let Some(mut vnc) = current_vm.vnc_worker.take() { + vnc.stop(); + let _ = vnc.join().await; + } + if let Some(mut gdb) = current_vm.gdb_worker.take() { + gdb.stop(); + let _ = gdb.join().await; + } + } } async fn handle_rpc(&mut self, rpc: VmControllerRpc, quit: &mut bool) { match rpc { + VmControllerRpc::Connect(req) => { + let result = self.attach_resources().map(Into::into); + req.complete(result.map_err(mesh::error::RemoteError::new)); + } + VmControllerRpc::CreateVm(req) => { + let (params, req) = req.split(); + let result = self.handle_create_vm(*params).await; + req.complete(result.map_err(mesh::error::RemoteError::new)); + } + VmControllerRpc::TeardownVm(req) => { + let result = self.handle_teardown_vm().await; + req.complete(result.map_err(mesh::error::RemoteError::new)); + } VmControllerRpc::Restart(req) => { let result = self.handle_restart().await; req.complete(result.map_err(mesh::error::RemoteError::new)); @@ -281,8 +401,9 @@ impl VmController { } VmControllerRpc::GetVtl2Settings(req) => { let bytes = self - .vtl2_settings + .current_vm .as_ref() + .and_then(|vm| vm.vtl2_settings.as_ref()) .map(prost::Message::encode_to_vec); req.complete(bytes); } @@ -313,30 +434,85 @@ impl VmController { } VmControllerRpc::Quit => { tracing::info!("quitting"); - self.vm_worker.stop(); *quit = true; } } } + async fn handle_create_vm( + &mut self, + params: ServerVmStartParams, + ) -> anyhow::Result { + if self.current_vm.is_some() { + anyhow::bail!("VM already created"); + } + + let vm_host = self + .mesh + .make_host("vm", None) + .await + .context("spawning vm process failed")?; + + let worker = vm_host + .launch_worker(VM_WORKER, params.worker_params) + .await?; + let handles = ServerVmHandles { + vm_rpc: params.vm_rpc.clone(), + scsi_rpc: params.scsi_rpc.clone(), + }; + self.current_vm = Some(CurrentVm { + vm_worker: worker, + vnc_worker: None, + gdb_worker: None, + diag_inspector: None, + vtl2_settings: None, + ged_rpc: None, + vm_rpc: params.vm_rpc, + scsi_rpc: params.scsi_rpc, + nvme_vtl2_rpc: None, + shutdown_ic: None, + kvp_ic: None, + paravisor_diag: None, + igvm_path: None, + memory_backing_file: None, + memory: params.memory, + processors: params.processors, + log_file: None, + notify_recv: params.notify_recv, + }); + + Ok(handles) + } + + async fn handle_teardown_vm(&mut self) -> anyhow::Result<()> { + self.current_vm.as_ref().context("VM not created")?; + self.stop_current_vm().await; + Ok(()) + } + async fn handle_restart(&mut self) -> anyhow::Result<()> { + let log_file = self.current_vm()?.log_file.clone(); let vm_host = self .mesh - .make_host("vm", self.log_file.clone()) + .make_host("vm", log_file) .await .context("spawning vm process failed")?; - self.vm_worker.restart(&vm_host); + self.current_vm_mut()?.vm_worker.restart(&vm_host); Ok(()) } async fn handle_restart_vnc(&mut self) -> anyhow::Result<()> { - if let Some(vnc) = &mut self.vnc_worker { + if self.current_vm()?.vnc_worker.is_some() { let vnc_host = self .mesh .make_host("vnc", None) .await .context("spawning vnc process failed")?; - vnc.restart(&vnc_host); + self.current_vm_mut()? + .vnc_worker + .as_mut() + .expect("checked above") + .restart(&vnc_host); Ok(()) } else { anyhow::bail!("no VNC server running") @@ -347,13 +523,18 @@ impl VmController { let obj = inspect::adhoc_mut(|req| match target { InspectTarget::Host => { let mut resp = req.respond(); + let current_vm = self.current_vm.as_ref(); resp.field("mesh", &self.mesh) - .field("vm", &self.vm_worker) - .field("vnc", self.vnc_worker.as_ref()) - .field("gdb", self.gdb_worker.as_ref()); + .field("vm", current_vm.map(|vm| &vm.vm_worker)) + .field("vnc", current_vm.and_then(|vm| vm.vnc_worker.as_ref())) + .field("gdb", current_vm.and_then(|vm| vm.gdb_worker.as_ref())); } InspectTarget::Paravisor => { - if let Some(inspector) = &mut self.diag_inspector { + if let Some(inspector) = self + .current_vm + .as_mut() + .and_then(|vm| vm.diag_inspector.as_mut()) + { inspector.inspect_mut(req); } } @@ -362,19 +543,21 @@ impl VmController { } async fn handle_save_snapshot(&self, dir: &Path) -> anyhow::Result<()> { - let memory_file_path = self + let current_vm = self.current_vm()?; + let memory_file_path = current_vm .memory_backing_file .as_ref() .context("save-snapshot requires --memory-backing-file")?; // Pause the VM. - self.vm_rpc + current_vm + .vm_rpc .call(VmRpc::Pause, ()) .await .context("failed to pause VM")?; // Get device state via existing VmRpc::Save. - let saved_state_msg = self + let saved_state_msg = current_vm .vm_rpc .call_failable(VmRpc::Save, ()) .await @@ -394,8 +577,8 @@ impl VmController { version: openvmm_helpers::snapshot::MANIFEST_VERSION, created_at: std::time::SystemTime::now().into(), openvmm_version: env!("CARGO_PKG_VERSION").to_string(), - memory_size_bytes: self.memory, - vp_count: self.processors, + memory_size_bytes: current_vm.memory, + vp_count: current_vm.processors, page_size: crate::system_page_size(), architecture: crate::GUEST_ARCH.to_string(), }; @@ -413,10 +596,12 @@ impl VmController { } async fn handle_service_vtl2(&self, params: ServiceVtl2Params) -> anyhow::Result { + let current_vm = self.current_vm()?; let start; if params.user_mode_only { start = Instant::now(); - self.paravisor_diag + current_vm + .paravisor_diag .as_ref() .context("no paravisor diagnostics client")? .restart() @@ -425,13 +610,13 @@ impl VmController { let igvm = params .igvm .map(PathBuf::from) - .or_else(|| self.igvm_path.clone()) + .or_else(|| current_vm.igvm_path.clone()) .context("no igvm file loaded")?; let file = fs_err::File::open(igvm)?; start = Instant::now(); - let ged_rpc = self.ged_rpc.as_ref().context("no GED")?; + let ged_rpc = current_vm.ged_rpc.as_ref().context("no GED")?; openvmm_helpers::underhill::save_underhill( - &self.vm_rpc, + ¤t_vm.vm_rpc, ged_rpc, GuestServicingFlags { nvme_keepalive: params.nvme_keepalive, @@ -440,7 +625,7 @@ impl VmController { file.into(), ) .await?; - openvmm_helpers::underhill::restore_underhill(&self.vm_rpc, ged_rpc).await?; + openvmm_helpers::underhill::restore_underhill(¤t_vm.vm_rpc, ged_rpc).await?; } let elapsed = Instant::now() - start; Ok(elapsed.as_millis() as u64) @@ -451,13 +636,18 @@ impl VmController { f: impl FnOnce(&mut vtl2_settings_proto::Vtl2Settings), ) -> anyhow::Result<()> { let mut settings_copy = self + .current_vm()? .vtl2_settings .clone() .context("vtl2 settings not configured")?; f(&mut settings_copy); - let ged_rpc = self.ged_rpc.as_ref().context("no GED configured")?; + let ged_rpc = self + .current_vm()? + .ged_rpc + .as_ref() + .context("no GED configured")?; ged_rpc .call_failable( @@ -466,7 +656,7 @@ impl VmController { ) .await?; - self.vtl2_settings = Some(settings_copy); + self.current_vm_mut()?.vtl2_settings = Some(settings_copy); Ok(()) } diff --git a/vmm_tests/vmm_tests/tests/tests/ttrpc.rs b/vmm_tests/vmm_tests/tests/tests/ttrpc.rs index 75a006ec6a..292f7ff1fd 100644 --- a/vmm_tests/vmm_tests/tests/tests/ttrpc.rs +++ b/vmm_tests/vmm_tests/tests/tests/ttrpc.rs @@ -13,6 +13,7 @@ use pal_async::task::Spawn; use petri::ResolvedArtifact; use petri_artifacts_vmm_test::artifacts; use std::io::Read; +use std::process::ExitStatus; use std::process::Stdio; use unix_socket::UnixStream; @@ -41,10 +42,12 @@ fn test_ttrpc_interface( let (stderr_read, stderr_write) = pal::pipe_pair()?; let mut child = std::process::Command::new(openvmm) - .arg("--ttrpc") - .arg(&socket_path) + .arg("serve") + .arg("--transport") + .arg("ttrpc") .arg("--pidfile") .arg(&pidfile_path) + .arg(&socket_path) .stdin(Stdio::null()) .stdout(Stdio::piped()) .stderr(stderr_write) @@ -63,9 +66,9 @@ fn test_ttrpc_interface( "pidfile should contain the child PID" ); - DefaultPool::run_with(async |driver| { + let status = DefaultPool::run_with(async |driver| -> anyhow::Result { let driver = driver; - let _stderr_task = driver.spawn( + let stderr_task = driver.spawn( "stderr", petri::log_task( params.logger.log_file("stderr").unwrap(), @@ -182,11 +185,24 @@ fn test_ttrpc_interface( _ => unreachable!(), } } - }); - child.wait()?; + let (status_send, status_recv) = mesh::oneshot(); + std::thread::Builder::new() + .name("wait-openvmm".into()) + .spawn(move || status_send.send(child.wait())) + .context("failed to spawn openvmm wait thread")?; + + let status = status_recv + .await + .context("openvmm wait thread exited without status")??; + stderr_task.await?; + Ok(status) + })?; + let _ = std::fs::remove_file(&socket_path); + assert!(status.success(), "openvmm exited with {status}"); + // Verify the pidfile was cleaned up on exit. assert!( !pidfile_path.exists(), From 0ee5e487a80fe1bff8b2fb14699631368af120cd Mon Sep 17 00:00:00 2001 From: John Starks Date: Fri, 1 May 2026 04:23:18 +0000 Subject: [PATCH 2/2] cleanup --- openvmm/openvmm_entry/src/cli_args.rs | 144 +++++++++++++++----------- openvmm/openvmm_entry/src/lib.rs | 16 --- 2 files changed, 85 insertions(+), 75 deletions(-) diff --git a/openvmm/openvmm_entry/src/cli_args.rs b/openvmm/openvmm_entry/src/cli_args.rs index 8fb68ada75..71ad300ee8 100644 --- a/openvmm/openvmm_entry/src/cli_args.rs +++ b/openvmm/openvmm_entry/src/cli_args.rs @@ -84,6 +84,10 @@ struct LegacyOptions { #[clap(long = "ttrpc", value_name = "SOCKETPATH", hide = true)] pub legacy_ttrpc: Option, + /// use shared memory segment + #[clap(short = 'M', long = "shared-memory", hide = true)] + pub deprecated_shared_memory: bool, + /// prefetch guest RAM #[clap(long = "prefetch", hide = true)] pub deprecated_prefetch: bool, @@ -208,23 +212,42 @@ fn legacy_options_from( unreachable!("should have been rejected by clap due to args_conflicts_with_subcommands") } let mut run = opt.run; - run.deprecated_memory = DeprecatedMemoryOptions { - prefetch: opt.deprecated_prefetch, - memory_backing_file: opt.deprecated_memory_backing_file, - private_memory: opt.deprecated_private_memory, - thp: opt.deprecated_thp, - }; + if opt.deprecated_prefetch { + run.memory.prefetch = true; + } + if opt.deprecated_shared_memory { + if run.memory.shared == Some(false) { + return Err(clap::Error::raw( + clap::error::ErrorKind::ValueValidation, + "--memory shared=off conflicts with --shared-memory", + )); + } + run.memory.shared = Some(true); + } + if let Some(path) = opt.deprecated_memory_backing_file { + if run.memory.file.is_some() { + return Err(clap::Error::raw( + clap::error::ErrorKind::ValueValidation, + "--memory file=... conflicts with --memory-backing-file", + )); + } + run.memory.file = Some(path); + } + if opt.deprecated_private_memory { + if run.memory.shared == Some(true) { + return Err(clap::Error::raw( + clap::error::ErrorKind::ValueValidation, + "--memory shared=on conflicts with --private-memory", + )); + } + run.memory.shared = Some(false); + } + if opt.deprecated_thp { + run.memory.transparent_hugepages = true; + } Ok(Command::Run(run)) } -#[derive(Default)] -struct DeprecatedMemoryOptions { - prefetch: bool, - memory_backing_file: Option, - private_memory: bool, - thp: bool, -} - /// VM launch options. #[derive(clap::Args)] pub struct RunOptions { @@ -263,9 +286,6 @@ Examples: )] pub memory: MemoryCli, - #[clap(skip)] - deprecated_memory: DeprecatedMemoryOptions, - /// per-NUMA-node guest RAM sizes (comma-separated, e.g. "2G,2G"). /// Distributes memory across vNUMA nodes reported to the guest. Mutually /// exclusive with --memory. This is for test-only usage. @@ -275,10 +295,6 @@ Examples: #[clap(long, value_name = "SIZES", value_parser = parse_memory, value_delimiter = ',', conflicts_with = "memory")] pub numa_memory: Option>, - /// use shared memory segment - #[clap(short = 'M', long, hide = true)] - pub shared_memory: bool, - /// Restore VM from a snapshot directory (implies file-backed memory from /// the snapshot's memory.bin). Cannot be used with --memory-backing-file. #[clap(long, value_name = "DIR")] @@ -1062,54 +1078,29 @@ impl RunOptions { /// Returns whether guest RAM should be prefetched. pub fn prefetch_memory(&self) -> bool { - self.memory.prefetch || self.deprecated_memory.prefetch + self.memory.prefetch } /// Returns whether guest RAM should use private anonymous backing. pub fn private_memory(&self) -> bool { - self.memory.shared == Some(false) || self.deprecated_memory.private_memory + self.memory.shared == Some(false) } /// Returns whether guest RAM should be marked THP-eligible. pub fn transparent_hugepages(&self) -> bool { - self.memory.transparent_hugepages || self.deprecated_memory.thp + self.memory.transparent_hugepages } /// Returns the effective file backing path for guest RAM. pub fn memory_backing_file(&self) -> Option<&PathBuf> { - self.memory - .file - .as_ref() - .or(self.deprecated_memory.memory_backing_file.as_ref()) - } - - pub(crate) fn deprecated_prefetch(&self) -> bool { - self.deprecated_memory.prefetch - } - - pub(crate) fn deprecated_private_memory(&self) -> bool { - self.deprecated_memory.private_memory - } - - pub(crate) fn deprecated_thp(&self) -> bool { - self.deprecated_memory.thp - } - - pub(crate) fn deprecated_memory_backing_file(&self) -> bool { - self.deprecated_memory.memory_backing_file.is_some() + self.memory.file.as_ref() } /// Validates combinations that span the new `--memory` parser and legacy aliases. pub fn validate_memory_options(&self) -> anyhow::Result<()> { - if self.memory.file.is_some() && self.deprecated_memory.memory_backing_file.is_some() { - anyhow::bail!("--memory file=... conflicts with --memory-backing-file"); - } if self.memory.file.is_some() && self.restore_snapshot.is_some() { anyhow::bail!("--memory file=... conflicts with --restore-snapshot"); } - if self.memory.shared == Some(true) && self.deprecated_memory.private_memory { - anyhow::bail!("--memory shared=on conflicts with --private-memory"); - } if self.memory_backing_file().is_some() && self.private_memory() { anyhow::bail!("file-backed memory conflicts with private memory"); } @@ -3926,13 +3917,46 @@ mod tests { } #[test] - fn test_memory_options_reject_conflicting_legacy_aliases() { - let opt = try_parse_options_from(["openvmm", "--memory", "shared=on", "--private-memory"]) - .unwrap(); + fn test_memory_options_allow_legacy_shared_memory_alias() { + let opt = try_parse_options_from(["openvmm", "--memory", "2G", "--shared-memory"]).unwrap(); let Command::Run(opt) = opt else { panic!("expected run command"); }; - assert!(opt.validate_memory_options().is_err()); + opt.validate_memory_options().unwrap(); + assert_eq!(opt.memory_size(), 2 * 1024 * 1024 * 1024); + assert_eq!(opt.memory.shared, Some(true)); + + let opt = try_parse_options_from(["openvmm", "--memory", "2G", "-M"]).unwrap(); + let Command::Run(opt) = opt else { + panic!("expected run command"); + }; + opt.validate_memory_options().unwrap(); + assert_eq!(opt.memory.shared, Some(true)); + } + + #[test] + fn test_memory_options_reject_conflicting_legacy_aliases() { + let err = match try_parse_options_from([ + "openvmm", + "--memory", + "shared=on", + "--private-memory", + ]) { + Ok(_) => panic!("conflicting legacy memory alias should fail"), + Err(err) => err, + }; + assert_eq!(err.kind(), clap::error::ErrorKind::ValueValidation); + + let err = match try_parse_options_from([ + "openvmm", + "--memory", + "shared=off", + "--shared-memory", + ]) { + Ok(_) => panic!("conflicting legacy memory alias should fail"), + Err(err) => err, + }; + assert_eq!(err.kind(), clap::error::ErrorKind::ValueValidation); } #[test] @@ -3961,11 +3985,13 @@ mod tests { #[test] fn test_run_subcommand_rejects_deprecated_memory_aliases() { - let err = match try_parse_options_from(["openvmm", "run", "--prefetch"]) { - Ok(_) => panic!("deprecated memory alias should not be accepted by run"), - Err(err) => err, - }; - assert_eq!(err.kind(), clap::error::ErrorKind::UnknownArgument); + for arg in ["--prefetch", "--shared-memory", "-M"] { + let err = match try_parse_options_from(["openvmm", "run", arg]) { + Ok(_) => panic!("deprecated memory alias should not be accepted by run"), + Err(err) => err, + }; + assert_eq!(err.kind(), clap::error::ErrorKind::UnknownArgument); + } } #[test] diff --git a/openvmm/openvmm_entry/src/lib.rs b/openvmm/openvmm_entry/src/lib.rs index 1b4e423539..adf4e589de 100644 --- a/openvmm/openvmm_entry/src/lib.rs +++ b/openvmm/openvmm_entry/src/lib.rs @@ -411,22 +411,6 @@ async fn vm_config_from_command_line( console_str = device; } - if opt.shared_memory { - tracing::warn!("--shared-memory/-M flag has no effect and will be removed"); - } - if opt.deprecated_prefetch() { - tracing::warn!("--prefetch is deprecated; use --memory prefetch=on"); - } - if opt.deprecated_private_memory() { - tracing::warn!("--private-memory is deprecated; use --memory shared=off"); - } - if opt.deprecated_thp() { - tracing::warn!("--thp is deprecated; use --memory shared=off,thp=on"); - } - if opt.deprecated_memory_backing_file() { - tracing::warn!("--memory-backing-file is deprecated; use --memory file="); - } - opt.validate_memory_options()?; const MAX_PROCESSOR_COUNT: u32 = 1024;