From 263fc2807d66bb0182057a898d2c2041a3784a3f Mon Sep 17 00:00:00 2001 From: Greg Colombo Date: Tue, 17 Dec 2024 16:06:49 -0800 Subject: [PATCH] server: make clients parse config TOMLs (#810) Remove config TOML processing from propolis-server. The only thing the config TOML is used for in Omicron is specifying a bootrom path (and possibly a version string); turn this into a propolis-server command-line argument instead. The instance spec-based ensure API already ignored the server's config TOML, and the non-spec ensure API is about to be removed anyway, so this is a good time to make this shift. To avoid breaking existing config TOML users, move the TOML-to-spec processing logic into the propolis-config-toml crate, and have it output propolis-client's generated instance spec types. (This would have been a layering violation previously, but is OK now that propolis-server no longer uses this crate.) Teach propolis-cli to use this crate to process TOMLs and to use the instance spec-based ensure endpoint to create and migrate VMs. Remove the config TOML from the Propolis zone image and change its command line invocation. Also change PHD's propolis-server invocations as needed. Finally, tweak the propolis-server and propolis-cli READMEs to describe the new state of affairs. --- .github/buildomat/phd-run-with-args.sh | 3 +- Cargo.lock | 29 +- Cargo.toml | 2 +- bin/propolis-cli/Cargo.toml | 2 + bin/propolis-cli/README.md | 41 ++ bin/propolis-cli/src/main.rs | 309 ++++++++++---- bin/propolis-server/Cargo.toml | 1 - bin/propolis-server/README.md | 74 +--- bin/propolis-server/src/lib/config.rs | 2 - bin/propolis-server/src/lib/initializer.rs | 13 +- bin/propolis-server/src/lib/server.rs | 40 +- .../src/lib/spec/api_spec_v0.rs | 3 +- bin/propolis-server/src/lib/spec/builder.rs | 61 +-- .../src/lib/spec/config_toml.rs | 384 ----------------- bin/propolis-server/src/lib/spec/mod.rs | 22 +- bin/propolis-server/src/lib/vm/ensure.rs | 9 +- bin/propolis-server/src/lib/vm/mod.rs | 11 +- bin/propolis-server/src/main.rs | 31 +- .../src/instance_spec/components/devices.rs | 8 +- .../Cargo.toml | 4 +- .../src/lib.rs | 16 +- crates/propolis-config-toml/src/spec.rs | 392 ++++++++++++++++++ lib/propolis-client/Cargo.toml | 1 + lib/propolis-client/src/lib.rs | 7 + lib/propolis-client/src/support.rs | 21 +- openapi/propolis-server.json | 10 +- packaging/smf/method_script.sh | 2 +- packaging/smf/propolis-server/config.toml | 14 - phd-tests/framework/Cargo.toml | 1 - phd-tests/framework/src/test_vm/config.rs | 26 +- phd-tests/framework/src/test_vm/mod.rs | 25 +- phd-tests/framework/src/test_vm/server.rs | 6 +- phd-tests/framework/src/test_vm/spec.rs | 14 +- 33 files changed, 815 insertions(+), 769 deletions(-) create mode 100644 bin/propolis-cli/README.md delete mode 100644 bin/propolis-server/src/lib/spec/config_toml.rs rename crates/{propolis-server-config => propolis-config-toml}/Cargo.toml (75%) rename crates/{propolis-server-config => propolis-config-toml}/src/lib.rs (93%) create mode 100644 crates/propolis-config-toml/src/spec.rs delete mode 100644 packaging/smf/propolis-server/config.toml diff --git a/.github/buildomat/phd-run-with-args.sh b/.github/buildomat/phd-run-with-args.sh index 55893944f..d7b6bb463 100755 --- a/.github/buildomat/phd-run-with-args.sh +++ b/.github/buildomat/phd-run-with-args.sh @@ -65,8 +65,7 @@ failcount=$? set -e tar -czvf /tmp/phd-tmp-files.tar.gz \ - -C /tmp/propolis-phd /tmp/propolis-phd/*.log \ - -C /tmp/propolis-phd /tmp/propolis-phd/*.toml + -C /tmp/propolis-phd /tmp/propolis-phd/*.log exitcode=0 if [ $failcount -eq 0 ]; then diff --git a/Cargo.lock b/Cargo.lock index cae74c2eb..a4282d133 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3833,7 +3833,6 @@ dependencies = [ "libc", "newtype_derive", "propolis-client", - "propolis-server-config", "rand", "reqwest 0.12.7", "ring 0.17.8", @@ -4276,10 +4275,12 @@ dependencies = [ "anyhow", "base64 0.21.7", "clap", + "crucible-client-types", "futures", "libc", "newtype-uuid", "propolis-client", + "propolis-config-toml", "reqwest 0.12.7", "serde", "serde_json", @@ -4299,6 +4300,7 @@ dependencies = [ "base64 0.21.7", "futures", "progenitor", + "propolis_api_types", "rand", "reqwest 0.12.7", "schemars", @@ -4311,6 +4313,19 @@ dependencies = [ "uuid", ] +[[package]] +name = "propolis-config-toml" +version = "0.0.0" +dependencies = [ + "cpuid_profile_config", + "propolis-client", + "serde", + "serde_derive", + "thiserror", + "toml 0.7.8", + "uuid", +] + [[package]] name = "propolis-mock-server" version = "0.0.0" @@ -4383,7 +4398,6 @@ dependencies = [ "oximeter-instruments", "oximeter-producer", "propolis", - "propolis-server-config", "propolis_api_types", "propolis_types", "reqwest 0.12.7", @@ -4410,17 +4424,6 @@ dependencies = [ "uuid", ] -[[package]] -name = "propolis-server-config" -version = "0.0.0" -dependencies = [ - "cpuid_profile_config", - "serde", - "serde_derive", - "thiserror", - "toml 0.7.8", -] - [[package]] name = "propolis-standalone" version = "0.1.0" diff --git a/Cargo.toml b/Cargo.toml index eb8676618..79e6de9ad 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -49,7 +49,7 @@ cpuid_profile_config = { path = "crates/cpuid-profile-config" } dladm = { path = "crates/dladm" } nvpair = { path = "crates/nvpair" } nvpair_sys = { path = "crates/nvpair/sys" } -propolis-server-config = { path = "crates/propolis-server-config" } +propolis-config-toml = { path = "crates/propolis-config-toml" } propolis_api_types = { path = "crates/propolis-api-types" } propolis_types = { path = "crates/propolis-types" } rfb = { path = "crates/rfb" } diff --git a/bin/propolis-cli/Cargo.toml b/bin/propolis-cli/Cargo.toml index f20e5ccaa..4eb8854d3 100644 --- a/bin/propolis-cli/Cargo.toml +++ b/bin/propolis-cli/Cargo.toml @@ -7,10 +7,12 @@ edition = "2021" [dependencies] anyhow.workspace = true clap = { workspace = true, features = ["derive"] } +crucible-client-types.workspace = true futures.workspace = true libc.workspace = true newtype-uuid.workspace = true propolis-client.workspace = true +propolis-config-toml.workspace = true slog.workspace = true slog-async.workspace = true slog-term.workspace = true diff --git a/bin/propolis-cli/README.md b/bin/propolis-cli/README.md new file mode 100644 index 000000000..ff8b08e09 --- /dev/null +++ b/bin/propolis-cli/README.md @@ -0,0 +1,41 @@ +# Propolis CLI + +The `propolis-cli` utility provides a user-friendly frontend to the +[`propolis-server`](../propolis-server) REST API. + +## Getting started + +The easiest way to launch a VM via the CLI is to write a TOML file describing +the VM's configuration. An example of such a file might be the following: + +```toml +[block_dev.alpine_iso] +type = "file" +path = "/path/to/alpine-extended-3.12.0-x86_64.iso" + +[dev.block0] +driver = "pci-virtio-block" +block_dev = "alpine_iso" +pci-path = "0.4.0" + +[dev.net0] +driver = "pci-virtio-viona" +vnic = "vnic_name" +pci-path = "0.5.0" +``` + +To create and run a Propolis VM using this configuration: + +``` +# propolis-cli -s -p new --config-toml +# propolis-cli -s -p state run +``` + +To connect to the VM's serial console: + +``` +# propolis-cli -s -p serial +``` + +Run `propolis-cli --help` to see the full list of supported commands and their +arguments. diff --git a/bin/propolis-cli/src/main.rs b/bin/propolis-cli/src/main.rs index 152e95178..78c8069ec 100644 --- a/bin/propolis-cli/src/main.rs +++ b/bin/propolis-cli/src/main.rs @@ -12,12 +12,18 @@ use std::{ }; use anyhow::{anyhow, Context}; -use clap::{Parser, Subcommand}; +use clap::{Args, Parser, Subcommand}; use futures::{future, SinkExt}; use newtype_uuid::{GenericUuid, TypedUuid, TypedUuidKind, TypedUuidTag}; use propolis_client::types::{ - InstanceMetadata, InstanceSpecStatus, VersionedInstanceSpec, + BlobStorageBackend, Board, Chipset, ComponentV0, CrucibleStorageBackend, + I440Fx, InstanceMetadata, InstanceSpecEnsureRequest, + InstanceSpecGetResponse, InstanceSpecStatus, InstanceSpecV0, NvmeDisk, + QemuPvpanic, SerialPort, SerialPortNumber, VersionedInstanceSpec, + VirtioDisk, }; +use propolis_client::PciPath; +use propolis_config_toml::spec::SpecConfig; use slog::{o, Drain, Level, Logger}; use tokio::io::{AsyncReadExt, AsyncWriteExt}; use tokio_tungstenite::tungstenite::{ @@ -29,9 +35,8 @@ use uuid::Uuid; use propolis_client::{ support::{InstanceSerialConsoleHelper, WSClientOffset}, types::{ - DiskRequest, InstanceEnsureRequest, InstanceMigrateInitiateRequest, - InstanceProperties, InstanceStateRequested, InstanceVcrReplace, - MigrationState, + DiskRequest, InstanceMigrateInitiateRequest, InstanceProperties, + InstanceStateRequested, InstanceVcrReplace, MigrationState, }, Client, }; @@ -68,21 +73,8 @@ enum Command { #[clap(short = 'u', action)] uuid: Option, - /// Number of vCPUs allocated to instance - #[clap(short = 'c', default_value = "4", action)] - vcpus: u8, - - /// Memory allocated to instance (MiB) - #[clap(short, default_value = "1024", action)] - memory: u64, - - /// File with a JSON array of DiskRequest structs - #[clap(long, action)] - crucible_disks: Option, - - // cloud_init ISO file - #[clap(long, action)] - cloud_init: Option, + #[clap(flatten)] + config: VmConfig, /// A UUID to use for the instance's silo, attached to instance metrics. #[clap(long)] @@ -169,6 +161,197 @@ enum Command { }, } +#[derive(Args, Clone, Debug)] +struct VmConfig { + /// A path to a file containing a JSON-formatted instance spec + #[clap(short = 's', long, action)] + spec: Option, + + /// Number of vCPUs allocated to instance + #[clap(short = 'c', default_value = "4", action, conflicts_with = "spec")] + vcpus: u8, + + /// Memory allocated to instance (MiB) + #[clap(short, default_value = "1024", action, conflicts_with = "spec")] + memory: u64, + + /// A path to a file containing a config TOML + #[clap(short = 't', long, action, conflicts_with = "spec")] + config_toml: Option, + + /// File with a JSON array of DiskRequest structs + #[clap(long, action, conflicts_with = "spec")] + crucible_disks: Option, + + // cloud_init ISO file + #[clap(long, action, conflicts_with = "spec")] + cloud_init: Option, +} + +fn add_component_to_spec( + spec: &mut InstanceSpecV0, + name: String, + component: ComponentV0, +) -> anyhow::Result<()> { + if spec.components.insert(name.clone(), component).is_some() { + anyhow::bail!("duplicate component ID {name:?}"); + } + Ok(()) +} + +fn add_disk_request_to_spec( + spec: &mut InstanceSpecV0, + req: &DiskRequest, +) -> anyhow::Result<()> { + let slot = req.slot.0 + 0x10; + let backend_name = format!("{}-backend", req.name); + let pci_path = PciPath::new(0, slot, 0) + .with_context(|| format!("processing disk request {:?}", req.name))?; + let device_spec = match req.device.as_ref() { + "virtio" => ComponentV0::VirtioDisk(VirtioDisk { + backend_name: backend_name.clone(), + pci_path, + }), + "nvme" => ComponentV0::NvmeDisk(NvmeDisk { + backend_name: backend_name.clone(), + pci_path, + }), + _ => anyhow::bail!( + "invalid device type in disk request: {:?}", + req.device + ), + }; + + let backend_spec = + ComponentV0::CrucibleStorageBackend(CrucibleStorageBackend { + readonly: req.read_only, + request_json: serde_json::to_string( + &req.volume_construction_request, + )?, + }); + + add_component_to_spec(spec, req.name.clone(), device_spec)?; + add_component_to_spec(spec, backend_name, backend_spec)?; + Ok(()) +} + +impl VmConfig { + fn instance_spec(&self) -> anyhow::Result { + // If the configuration specifies an instance spec path, just read the + // spec from that path and return it. Otherwise, construct a spec from + // this configuration's component parts. + if let Some(path) = &self.spec { + return parse_json_file(path); + } + + let from_toml = &self + .config_toml + .as_ref() + .map(propolis_config_toml::parse) + .transpose()? + .as_ref() + .map(SpecConfig::try_from) + .transpose()?; + + let enable_pcie = + from_toml.as_ref().map(|cfg| cfg.enable_pcie).unwrap_or(false); + + let mut spec = InstanceSpecV0 { + board: Board { + chipset: Chipset::I440Fx(I440Fx { enable_pcie }), + cpuid: None, + cpus: self.vcpus, + memory_mb: self.memory, + }, + components: Default::default(), + }; + + if let Some(from_toml) = from_toml { + for (id, component) in from_toml.components.iter() { + add_component_to_spec( + &mut spec, + id.clone(), + component.clone(), + )?; + } + } + + for disk_request in self + .crucible_disks + .as_ref() + .map(|path| parse_json_file::>(path)) + .transpose()? + .iter() + .flatten() + { + add_disk_request_to_spec(&mut spec, disk_request)?; + } + + if let Some(cloud_init) = self.cloud_init.as_ref() { + let bytes = base64::Engine::encode( + &base64::engine::general_purpose::STANDARD, + std::fs::read(cloud_init)?, + ); + + const CLOUD_INIT_NAME: &str = "cloud-init"; + const CLOUD_INIT_BACKEND_NAME: &str = "cloud-init-backend"; + + add_component_to_spec( + &mut spec, + CLOUD_INIT_NAME.to_owned(), + ComponentV0::VirtioDisk(VirtioDisk { + backend_name: CLOUD_INIT_BACKEND_NAME.to_owned(), + pci_path: PciPath::new(0, 0x18, 0).unwrap(), + }), + )?; + + add_component_to_spec( + &mut spec, + CLOUD_INIT_BACKEND_NAME.to_owned(), + ComponentV0::BlobStorageBackend(BlobStorageBackend { + base64: bytes, + readonly: true, + }), + )?; + } + + for (name, port) in [ + ("com1", SerialPortNumber::Com1), + ("com2", SerialPortNumber::Com2), + ("com3", SerialPortNumber::Com3), + ] { + add_component_to_spec( + &mut spec, + name.to_owned(), + ComponentV0::SerialPort(SerialPort { num: port }), + )?; + } + + // If there are no SoftNPU devices, also enable COM4. + if !spec + .components + .iter() + .any(|(_, c)| matches!(c, ComponentV0::SoftNpuPort(_))) + { + add_component_to_spec( + &mut spec, + "com4".to_owned(), + ComponentV0::SerialPort(SerialPort { + num: SerialPortNumber::Com4, + }), + )?; + } + + add_component_to_spec( + &mut spec, + "pvpanic".to_owned(), + ComponentV0::QemuPvpanic(QemuPvpanic { enable_isa: true }), + )?; + + Ok(spec) + } +} + fn parse_state(state: &str) -> anyhow::Result { match state.to_lowercase().as_str() { "run" => Ok(InstanceStateRequested::Run), @@ -244,10 +427,7 @@ async fn new_instance( client: &Client, name: String, id: Uuid, - vcpus: u8, - memory: u64, - disks: Vec, - cloud_init_bytes: Option, + spec: InstanceSpecV0, metadata: InstanceMetadata, ) -> anyhow::Result<()> { let properties = InstanceProperties { @@ -257,21 +437,15 @@ async fn new_instance( metadata, }; - let request = InstanceEnsureRequest { - properties, - vcpus, - memory, - // TODO: Allow specifying NICs - nics: vec![], - disks, - boot_settings: None, + let request = InstanceSpecEnsureRequest { + instance_spec: VersionedInstanceSpec::V0(spec), migrate: None, - cloud_init_bytes, + properties, }; // Try to create the instance client - .instance_ensure() + .instance_spec_ensure() .body(request) .send() .await @@ -501,41 +675,37 @@ async fn migrate_instance( disks: Vec, ) -> anyhow::Result<()> { // Grab the instance details - let src_instance = - src_client.instance_spec_get().send().await.with_context(|| { - anyhow!("failed to get src instance properties") - })?; - let src_uuid = src_instance.properties.id; - let InstanceSpecStatus::Present(spec) = &src_instance.spec else { + let InstanceSpecGetResponse { mut properties, spec, .. } = src_client + .instance_spec_get() + .send() + .await + .with_context(|| anyhow!("failed to get src instance properties"))? + .into_inner(); + let src_uuid = properties.id; + properties.id = dst_uuid; + let InstanceSpecStatus::Present(spec) = spec else { anyhow::bail!("source instance doesn't have a spec yet"); }; - let VersionedInstanceSpec::V0(spec) = &spec; - - let request = InstanceEnsureRequest { - properties: InstanceProperties { - // Use a new ID for the destination instance we're creating - id: dst_uuid, - ..src_instance.properties.clone() - }, - vcpus: spec.board.cpus, - memory: spec.board.memory_mb, - // TODO: Handle migrating NICs - nics: vec![], - disks, - // TODO: Handle retaining boot settings? Or extant boot settings - // forwarded along outside InstanceEnsure anyway. - boot_settings: None, + + let VersionedInstanceSpec::V0(mut spec) = spec; + spec.components.clear(); + for disk in disks.iter() { + add_disk_request_to_spec(&mut spec, disk)?; + } + + let request = InstanceSpecEnsureRequest { + properties, migrate: Some(InstanceMigrateInitiateRequest { migration_id: Uuid::new_v4(), src_addr: src_addr.to_string(), src_uuid, }), - cloud_init_bytes: None, + instance_spec: VersionedInstanceSpec::V0(spec), }; // Initiate the migration via the destination instance let migration_res = - dst_client.instance_ensure().body(request).send().await?; + dst_client.instance_spec_ensure().body(request).send().await?; let migration_id = migration_res .migrate .as_ref() @@ -660,10 +830,7 @@ async fn main() -> anyhow::Result<()> { Command::New { name, uuid, - vcpus, - memory, - crucible_disks, - cloud_init, + config, silo_id, project_id, sled_id, @@ -671,19 +838,6 @@ async fn main() -> anyhow::Result<()> { sled_revision, sled_serial, } => { - let disks = if let Some(crucible_disks) = crucible_disks { - parse_json_file(&crucible_disks)? - } else { - vec![] - }; - let cloud_init_bytes = if let Some(cloud_init) = cloud_init { - Some(base64::Engine::encode( - &base64::engine::general_purpose::STANDARD, - std::fs::read(cloud_init)?, - )) - } else { - None - }; let metadata = InstanceMetadata { project_id: project_id .unwrap_or_else(TypedUuid::new_v4) @@ -702,10 +856,7 @@ async fn main() -> anyhow::Result<()> { &client, name.to_string(), uuid.unwrap_or_else(Uuid::new_v4), - vcpus, - memory, - disks, - cloud_init_bytes, + config.instance_spec()?, metadata, ) .await? diff --git a/bin/propolis-server/Cargo.toml b/bin/propolis-server/Cargo.toml index 5132955fd..c748d4dfb 100644 --- a/bin/propolis-server/Cargo.toml +++ b/bin/propolis-server/Cargo.toml @@ -59,7 +59,6 @@ strum = { workspace = true, features = ["derive"] } propolis = { workspace = true, features = ["crucible-full", "oximeter"] } propolis_api_types = { workspace = true } propolis_types.workspace = true -propolis-server-config.workspace = true rgb_frame.workspace = true rfb = { workspace = true, features = ["tungstenite"] } uuid.workspace = true diff --git a/bin/propolis-server/README.md b/bin/propolis-server/README.md index 49cf967ad..80de49048 100644 --- a/bin/propolis-server/README.md +++ b/bin/propolis-server/README.md @@ -1,66 +1,24 @@ # Propolis Server -## Running - -Propolis is mostly intended to be used via a REST API to drive all of its -functionality. The standard `cargo build` will produce a `propolis-server` -binary you can run: - -``` -# propolis-server run -``` - -Note that the server must run as root. One way to ensure propolis-server has -sufficient privileges is by using `pfexec(1)`, as such: - -``` -# pfexec propolis-server run -``` - -## Example Configuration +This binary provides a REST API to create and manage a Propolis VM. It typically +runs in the context of a complete Oxide control plane deployment, but it can +also be run as a freestanding binary for ad hoc testing of Propolis VMs. -**Note**: the goal is to move the device config from the toml to instead be -configured via REST API calls. - -```toml -bootrom = "/path/to/bootrom/OVMF_CODE.fd" +## Running -[block_dev.alpine_iso] -type = "file" -path = "/path/to/alpine-extended-3.12.0-x86_64.iso" +The server requires a path to a [guest bootrom +image](../propolis-standalone#guest-bootrom) on the local filesystem. It also +must be run with privileges sufficient to create bhyve virtual machines. The +`pfexec(1)` utility can help enable these privileges. -[dev.block0] -driver = "pci-virtio-block" -block_dev = "alpine_iso" -pci-path = "0.4.0" +To build and run the server: -[dev.net0] -driver = "pci-virtio-viona" -vnic = "vnic_name" -pci-path = "0.5.0" +```bash +cargo build --bin propolis-server +pfexec target/debug/propolis-server ``` -## Prerequisites - -When running the server by hand, the appropriate bootrom is required to start -guests properly. See the [standalone -documentation](../propolis-standalone#guest-bootrom) for more details. Details -for [creating necessary vnics](../propolis-standalone#vnic) can be found there -as well, if exposing network devices to the guest. - -## CLI Interaction - -Once you've got `propolis-server` running you can interact with it via the REST -API with any of the usual suspects (e.g. cURL, wget). Alternatively, there's a -`propolis-cli` binary to make things a bit easier: - -### Running - -The following CLI commands will create a VM, start the VM, and then attach to -its serial console: - -``` -# propolis-cli -s -p new -# propolis-cli -s -p state run -# propolis-cli -s -p serial -``` +The API will be served on `ip:port`. The easiest way to interact with the server +is to use [`propolis-cli`](../propolis-cli), but you can also use tools like +cURL to interact with the API directly. The server's OpenAPI specification is +[checked into the repo](../../openapi/propolis-server.json). diff --git a/bin/propolis-server/src/lib/config.rs b/bin/propolis-server/src/lib/config.rs index 20dd6e12c..909e0a982 100644 --- a/bin/propolis-server/src/lib/config.rs +++ b/bin/propolis-server/src/lib/config.rs @@ -4,8 +4,6 @@ //! Describes a server config which may be parsed from a TOML file. -pub use propolis_server_config::*; - #[cfg(not(feature = "omicron-build"))] pub fn reservoir_decide(log: &slog::Logger) -> bool { // Automatically enable use of the memory reservoir (rather than transient diff --git a/bin/propolis-server/src/lib/initializer.rs b/bin/propolis-server/src/lib/initializer.rs index a3ea3fb45..5bb3213ee 100644 --- a/bin/propolis-server/src/lib/initializer.rs +++ b/bin/propolis-server/src/lib/initializer.rs @@ -187,7 +187,6 @@ pub struct MachineInitializer<'a> { pub(crate) crucible_backends: CrucibleBackendMap, pub(crate) spec: &'a Spec, pub(crate) properties: &'a InstanceProperties, - pub(crate) toml_config: &'a crate::server::VmTomlConfig, pub(crate) producer_registry: Option, pub(crate) state: MachineInitializerState, pub(crate) kstat_sampler: Option, @@ -911,14 +910,15 @@ impl<'a> MachineInitializer<'a> { chipset.pci_attach(p9fs.pci_path.into(), vio9p); } - fn generate_smbios(&self) -> smbios::TableBytes { + fn generate_smbios( + &self, + bootrom_version: &Option, + ) -> smbios::TableBytes { use smbios::table::{type0, type1, type16, type4}; let rom_size = self.state.rom_size_bytes.expect("ROM is already populated"); - let bios_version = self - .toml_config - .bootrom_version + let bios_version = bootrom_version .as_deref() .unwrap_or("v0.8") .try_into() @@ -1153,6 +1153,7 @@ impl<'a> MachineInitializer<'a> { pub fn initialize_fwcfg( &mut self, cpus: u8, + bootrom_version: &Option, ) -> Result, MachineInitError> { let fwcfg = fwcfg::FwCfg::new(); fwcfg @@ -1163,7 +1164,7 @@ impl<'a> MachineInitializer<'a> { .map_err(|e| MachineInitError::FwcfgInsertFailed("cpu count", e))?; let smbios::TableBytes { entry_point, structure_table } = - self.generate_smbios(); + self.generate_smbios(bootrom_version); fwcfg .insert_named( "etc/smbios/smbios-tables", diff --git a/bin/propolis-server/src/lib/server.rs b/bin/propolis-server/src/lib/server.rs index a5e669cb0..0a32d4464 100644 --- a/bin/propolis-server/src/lib/server.rs +++ b/bin/propolis-server/src/lib/server.rs @@ -14,6 +14,7 @@ use std::net::IpAddr; use std::net::Ipv6Addr; use std::net::SocketAddr; use std::net::SocketAddrV6; +use std::path::PathBuf; use std::sync::Arc; use crate::{ @@ -42,7 +43,6 @@ use propolis_api_types as api; use propolis_api_types::instance_spec::{ self, components::devices::QemuPvpanic, VersionedInstanceSpec, }; -pub use propolis_server_config::Config as VmTomlConfig; use rfb::tungstenite::BinaryWs; use slog::{error, warn, Logger}; use tokio::sync::MutexGuard; @@ -70,8 +70,12 @@ pub struct MetricsEndpointConfig { /// this configuration at startup time and refers to it when manipulating its /// objects. pub struct StaticConfig { - /// The TOML-driven configuration for this server's instances. - pub vm: Arc, + /// The path to the bootrom image to expose to the guest. + pub bootrom_path: PathBuf, + + /// The bootrom version string to expose to the guest. If None, machine + /// initialization chooses a default value. + pub bootrom_version: Option, /// Whether to use the host's guest memory reservoir to back guest memory. pub use_reservoir: bool, @@ -92,7 +96,8 @@ pub struct DropshotEndpointContext { impl DropshotEndpointContext { /// Creates a new server context object. pub fn new( - config: VmTomlConfig, + bootrom_path: PathBuf, + bootrom_version: Option, use_reservoir: bool, log: slog::Logger, metric_config: Option, @@ -100,7 +105,8 @@ impl DropshotEndpointContext { let vnc_server = VncServer::new(log.clone()); Self { static_config: StaticConfig { - vm: Arc::new(config), + bootrom_path, + bootrom_version, use_reservoir, metrics: metric_config, }, @@ -115,12 +121,9 @@ impl DropshotEndpointContext { /// this crate, so implementing TryFrom for them is not allowed.) fn instance_spec_from_request( request: &api::InstanceEnsureRequest, - toml_config: &VmTomlConfig, ) -> Result { let mut spec_builder = SpecBuilder::new(request.vcpus, request.memory); - spec_builder.add_devices_from_config(toml_config)?; - for nic in &request.nics { spec_builder.add_nic_from_request(nic)?; } @@ -257,7 +260,8 @@ async fn instance_ensure_common( .await; let ensure_options = crate::vm::EnsureOptions { - toml_config: server_context.static_config.vm.clone(), + bootrom_path: server_context.static_config.bootrom_path.clone(), + bootrom_version: server_context.static_config.bootrom_version.clone(), use_reservoir: server_context.static_config.use_reservoir, metrics_config: server_context.static_config.metrics.clone(), oximeter_registry, @@ -300,19 +304,13 @@ async fn instance_ensure( rqctx: RequestContext>, request: TypedBody, ) -> Result, HttpError> { - let server_context = rqctx.context(); let request = request.into_inner(); - let instance_spec = - instance_spec_from_request(&request, &server_context.static_config.vm) - .map_err(|e| { - HttpError::for_bad_request( - None, - format!( - "failed to generate instance spec from request: {:#?}", - e - ), - ) - })?; + let instance_spec = instance_spec_from_request(&request).map_err(|e| { + HttpError::for_bad_request( + None, + format!("failed to generate instance spec from request: {:#?}", e), + ) + })?; instance_ensure_common( rqctx, diff --git a/bin/propolis-server/src/lib/spec/api_spec_v0.rs b/bin/propolis-server/src/lib/spec/api_spec_v0.rs index 1f88e7e11..6dca51918 100644 --- a/bin/propolis-server/src/lib/spec/api_spec_v0.rs +++ b/bin/propolis-server/src/lib/spec/api_spec_v0.rs @@ -196,7 +196,7 @@ impl From for InstanceSpecV0 { &mut spec, port_name.clone(), ComponentV0::SoftNpuPort(SoftNpuPortSpec { - name: port_name, + link_name: port.link_name, backend_name: port.backend_name.clone(), }), ); @@ -346,6 +346,7 @@ impl TryFrom for Spec { })?; let port = SoftNpuPort { + link_name: port.link_name, backend_name: port.backend_name, backend_spec, }; diff --git a/bin/propolis-server/src/lib/spec/builder.rs b/bin/propolis-server/src/lib/spec/builder.rs index b9012d212..877317b9a 100644 --- a/bin/propolis-server/src/lib/spec/builder.rs +++ b/bin/propolis-server/src/lib/spec/builder.rs @@ -24,11 +24,10 @@ use propolis_api_types::instance_spec::components::devices::{ P9fs, SoftNpuP9, SoftNpuPciPort, }; -use crate::{config, spec::SerialPortDevice}; +use crate::spec::SerialPortDevice; use super::{ api_request::{self, DeviceRequestError}, - config_toml::{ConfigTomlError, ParsedConfig}, Board, BootOrderEntry, BootSettings, Disk, Nic, QemuPvpanic, SerialPort, }; @@ -36,14 +35,11 @@ use super::{ use super::MigrationFailure; #[cfg(feature = "falcon")] -use super::{ParsedSoftNpu, SoftNpuPort}; +use super::SoftNpuPort; /// Errors that can arise while building an instance spec from component parts. #[derive(Debug, Error)] pub(crate) enum SpecBuilderError { - #[error("error parsing config TOML")] - ConfigToml(#[from] ConfigTomlError), - #[error("error parsing device in ensure request")] DeviceRequest(#[from] DeviceRequestError), @@ -192,59 +188,6 @@ impl SpecBuilder { Ok(()) } - /// Adds all the devices and backends specified in the supplied - /// configuration TOML to the spec under construction. - pub fn add_devices_from_config( - &mut self, - config: &config::Config, - ) -> Result<(), SpecBuilderError> { - let parsed = ParsedConfig::try_from(config)?; - - let Chipset::I440Fx(ref mut i440fx) = self.spec.board.chipset; - i440fx.enable_pcie = parsed.enable_pcie; - - for disk in parsed.disks { - self.add_storage_device(disk.name, disk.disk)?; - } - - for nic in parsed.nics { - self.add_network_device(nic.name, nic.nic)?; - } - - for bridge in parsed.pci_bridges { - self.add_pci_bridge(bridge.name, bridge.bridge)?; - } - - #[cfg(feature = "falcon")] - self.add_parsed_softnpu_devices(parsed.softnpu)?; - - Ok(()) - } - - #[cfg(feature = "falcon")] - fn add_parsed_softnpu_devices( - &mut self, - devices: ParsedSoftNpu, - ) -> Result<(), SpecBuilderError> { - if let Some(pci_port) = devices.pci_port { - self.set_softnpu_pci_port(pci_port)?; - } - - for port in devices.ports { - self.add_softnpu_port(port.name, port.port)?; - } - - if let Some(p9) = devices.p9_device { - self.set_softnpu_p9(p9)?; - } - - if let Some(p9fs) = devices.p9fs { - self.set_p9fs(p9fs)?; - } - - Ok(()) - } - /// Adds a PCI path to this builder's record of PCI locations with an /// attached device. If the path is already in use, returns an error. fn register_pci_device( diff --git a/bin/propolis-server/src/lib/spec/config_toml.rs b/bin/propolis-server/src/lib/spec/config_toml.rs deleted file mode 100644 index 141c26630..000000000 --- a/bin/propolis-server/src/lib/spec/config_toml.rs +++ /dev/null @@ -1,384 +0,0 @@ -// This Source Code Form is subject to the terms of the Mozilla Public -// License, v. 2.0. If a copy of the MPL was not distributed with this -// file, You can obtain one at https://mozilla.org/MPL/2.0/. - -//! Functions for converting a config TOML into instance spec elements. - -use std::str::{FromStr, ParseBoolError}; - -use propolis_api_types::instance_spec::{ - components::{ - backends::{FileStorageBackend, VirtioNetworkBackend}, - devices::{NvmeDisk, PciPciBridge, VirtioDisk, VirtioNic}, - }, - PciPath, -}; -use thiserror::Error; - -#[cfg(feature = "falcon")] -use propolis_api_types::instance_spec::components::devices::{ - P9fs, SoftNpuP9, SoftNpuPciPort, -}; - -use crate::config; - -use super::{ - Disk, Nic, ParsedDiskRequest, ParsedNicRequest, ParsedPciBridgeRequest, - StorageBackend, StorageDevice, -}; - -#[cfg(feature = "falcon")] -use super::{ParsedSoftNpu, ParsedSoftNpuPort, SoftNpuPort}; - -#[derive(Debug, Error)] -pub(crate) enum ConfigTomlError { - #[error("unrecognized device type {0:?}")] - UnrecognizedDeviceType(String), - - #[error("invalid value {0:?} for enable-pcie flag in chipset")] - EnablePcieParseFailed(String), - - #[error("failed to get PCI path for device {0:?}")] - InvalidPciPath(String), - - #[error("failed to parse PCI path string {0:?}")] - PciPathParseFailed(String, #[source] std::io::Error), - - #[error("invalid storage device kind {kind:?} for device {name:?}")] - InvalidStorageDeviceType { kind: String, name: String }, - - #[error("no backend name for storage device {0:?}")] - NoBackendNameForStorageDevice(String), - - #[error("invalid storage backend kind {kind:?} for backend {name:?}")] - InvalidStorageBackendType { kind: String, name: String }, - - #[error("couldn't find storage device {device:?}'s backend {backend:?}")] - StorageDeviceBackendNotFound { device: String, backend: String }, - - #[error("couldn't get path for file backend {0:?}")] - InvalidFileBackendPath(String), - - #[error("failed to parse read-only option for file backend {0:?}")] - FileBackendReadonlyParseFailed(String, #[source] ParseBoolError), - - #[error("failed to get VNIC name for device {0:?}")] - NoVnicName(String), - - #[cfg(feature = "falcon")] - #[error("failed to get source for p9 device {0:?}")] - NoP9Source(String), - - #[cfg(feature = "falcon")] - #[error("failed to get source for p9 device {0:?}")] - NoP9Target(String), -} - -#[derive(Default)] -pub(super) struct ParsedConfig { - pub(super) enable_pcie: bool, - pub(super) disks: Vec, - pub(super) nics: Vec, - pub(super) pci_bridges: Vec, - - #[cfg(feature = "falcon")] - pub(super) softnpu: ParsedSoftNpu, -} - -impl TryFrom<&config::Config> for ParsedConfig { - type Error = ConfigTomlError; - - fn try_from(config: &config::Config) -> Result { - let mut parsed = ParsedConfig { - enable_pcie: config - .chipset - .options - .get("enable-pcie") - .map(|v| { - v.as_bool().ok_or_else(|| { - ConfigTomlError::EnablePcieParseFailed(v.to_string()) - }) - }) - .transpose()? - .unwrap_or(false), - ..Default::default() - }; - - for (device_name, device) in config.devices.iter() { - let driver = device.driver.as_str(); - match driver { - // If this is a storage device, parse its "block_dev" property - // to get the name of its corresponding backend. - "pci-virtio-block" | "pci-nvme" => { - let device_spec = - parse_storage_device_from_config(device_name, device)?; - - let backend_name = device_spec.backend_name(); - let backend_config = - config.block_devs.get(backend_name).ok_or_else( - || ConfigTomlError::StorageDeviceBackendNotFound { - device: device_name.to_owned(), - backend: backend_name.to_owned(), - }, - )?; - - let backend_spec = parse_storage_backend_from_config( - backend_name, - backend_config, - )?; - - parsed.disks.push(ParsedDiskRequest { - name: device_name.to_owned(), - disk: Disk { device_spec, backend_spec }, - }); - } - "pci-virtio-viona" => { - parsed.nics.push(parse_network_device_from_config( - device_name, - device, - )?); - } - #[cfg(feature = "falcon")] - "softnpu-pci-port" => { - parsed.softnpu.pci_port = - Some(parse_softnpu_pci_port_from_config( - device_name, - device, - )?); - } - #[cfg(feature = "falcon")] - "softnpu-port" => { - parsed.softnpu.ports.push(parse_softnpu_port_from_config( - device_name, - device, - )?); - } - #[cfg(feature = "falcon")] - "softnpu-p9" => { - parsed.softnpu.p9_device = Some( - parse_softnpu_p9_from_config(device_name, device)?, - ); - } - #[cfg(feature = "falcon")] - "pci-virtio-9p" => { - parsed.softnpu.p9fs = - Some(parse_p9fs_from_config(device_name, device)?); - } - _ => { - return Err(ConfigTomlError::UnrecognizedDeviceType( - driver.to_owned(), - )) - } - } - } - - for bridge in config.pci_bridges.iter() { - parsed.pci_bridges.push(parse_pci_bridge_from_config(bridge)?); - } - - Ok(parsed) - } -} - -pub(super) fn parse_storage_backend_from_config( - name: &str, - backend: &config::BlockDevice, -) -> Result { - let backend_spec = match backend.bdtype.as_str() { - "file" => StorageBackend::File(FileStorageBackend { - path: backend - .options - .get("path") - .ok_or_else(|| { - ConfigTomlError::InvalidFileBackendPath(name.to_owned()) - })? - .as_str() - .ok_or_else(|| { - ConfigTomlError::InvalidFileBackendPath(name.to_owned()) - })? - .to_string(), - readonly: match backend.options.get("readonly") { - Some(toml::Value::Boolean(ro)) => Some(*ro), - Some(toml::Value::String(v)) => { - Some(v.parse::().map_err(|e| { - ConfigTomlError::FileBackendReadonlyParseFailed( - name.to_owned(), - e, - ) - })?) - } - _ => None, - } - .unwrap_or(false), - }), - _ => { - return Err(ConfigTomlError::InvalidStorageBackendType { - kind: backend.bdtype.clone(), - name: name.to_owned(), - }); - } - }; - - Ok(backend_spec) -} - -pub(super) fn parse_storage_device_from_config( - name: &str, - device: &config::Device, -) -> Result { - enum Interface { - Virtio, - Nvme, - } - - let interface = match device.driver.as_str() { - "pci-virtio-block" => Interface::Virtio, - "pci-nvme" => Interface::Nvme, - _ => { - return Err(ConfigTomlError::InvalidStorageDeviceType { - kind: device.driver.clone(), - name: name.to_owned(), - }); - } - }; - - let backend_name = device - .options - .get("block_dev") - .ok_or_else(|| { - ConfigTomlError::NoBackendNameForStorageDevice(name.to_owned()) - })? - .as_str() - .ok_or_else(|| { - ConfigTomlError::NoBackendNameForStorageDevice(name.to_owned()) - })? - .to_owned(); - - let pci_path: PciPath = device - .get("pci-path") - .ok_or_else(|| ConfigTomlError::InvalidPciPath(name.to_owned()))?; - - Ok(match interface { - Interface::Virtio => { - StorageDevice::Virtio(VirtioDisk { backend_name, pci_path }) - } - Interface::Nvme => { - StorageDevice::Nvme(NvmeDisk { backend_name, pci_path }) - } - }) -} - -pub(super) fn parse_network_device_from_config( - name: &str, - device: &config::Device, -) -> Result { - let vnic_name = device - .get_string("vnic") - .ok_or_else(|| ConfigTomlError::NoVnicName(name.to_owned()))?; - - let pci_path: PciPath = device - .get("pci-path") - .ok_or_else(|| ConfigTomlError::InvalidPciPath(name.to_owned()))?; - - let (device_name, backend_name) = super::pci_path_to_nic_names(pci_path); - let backend_spec = VirtioNetworkBackend { vnic_name: vnic_name.to_owned() }; - let device_spec = VirtioNic { - backend_name: backend_name.clone(), - // NICs added by the configuration TOML have no control plane- - // supplied correlation IDs. - interface_id: uuid::Uuid::nil(), - pci_path, - }; - - Ok(ParsedNicRequest { - name: device_name, - nic: Nic { device_spec, backend_spec }, - }) -} - -pub(super) fn parse_pci_bridge_from_config( - bridge: &config::PciBridge, -) -> Result { - let pci_path = PciPath::from_str(&bridge.pci_path).map_err(|e| { - ConfigTomlError::PciPathParseFailed(bridge.pci_path.to_string(), e) - })?; - - let name = format!("pci-bridge-{}", bridge.pci_path); - Ok(ParsedPciBridgeRequest { - name, - bridge: PciPciBridge { - downstream_bus: bridge.downstream_bus, - pci_path, - }, - }) -} - -#[cfg(feature = "falcon")] -pub(super) fn parse_softnpu_p9_from_config( - name: &str, - device: &config::Device, -) -> Result { - let pci_path: PciPath = device - .get("pci-path") - .ok_or_else(|| ConfigTomlError::InvalidPciPath(name.to_owned()))?; - - Ok(SoftNpuP9 { pci_path }) -} - -#[cfg(feature = "falcon")] -pub(super) fn parse_softnpu_pci_port_from_config( - name: &str, - device: &config::Device, -) -> Result { - let pci_path: PciPath = device - .get("pci-path") - .ok_or_else(|| ConfigTomlError::InvalidPciPath(name.to_owned()))?; - - Ok(SoftNpuPciPort { pci_path }) -} - -#[cfg(feature = "falcon")] -pub(super) fn parse_softnpu_port_from_config( - name: &str, - device: &config::Device, -) -> Result { - use propolis_api_types::instance_spec::components::backends::DlpiNetworkBackend; - - let vnic_name = device - .get_string("vnic") - .ok_or_else(|| ConfigTomlError::NoVnicName(name.to_owned()))?; - - Ok(ParsedSoftNpuPort { - name: name.to_owned(), - port: SoftNpuPort { - backend_name: vnic_name.to_owned(), - backend_spec: DlpiNetworkBackend { - vnic_name: vnic_name.to_owned(), - }, - }, - }) -} - -#[cfg(feature = "falcon")] -pub(super) fn parse_p9fs_from_config( - name: &str, - device: &config::Device, -) -> Result { - let source = device - .get_string("source") - .ok_or_else(|| ConfigTomlError::NoP9Source(name.to_owned()))?; - let target = device - .get_string("target") - .ok_or_else(|| ConfigTomlError::NoP9Target(name.to_owned()))?; - let pci_path: PciPath = device - .get("pci-path") - .ok_or_else(|| ConfigTomlError::InvalidPciPath(name.to_owned()))?; - - let chunk_size = device.get("chunk_size").unwrap_or(65536); - Ok(P9fs { - source: source.to_owned(), - target: target.to_owned(), - chunk_size, - pci_path, - }) -} diff --git a/bin/propolis-server/src/lib/spec/mod.rs b/bin/propolis-server/src/lib/spec/mod.rs index dc133fa02..c211693fd 100644 --- a/bin/propolis-server/src/lib/spec/mod.rs +++ b/bin/propolis-server/src/lib/spec/mod.rs @@ -46,7 +46,6 @@ use propolis_api_types::instance_spec::components::{ mod api_request; pub(crate) mod api_spec_v0; pub(crate) mod builder; -mod config_toml; #[derive(Debug, Error)] #[error("input component type can't convert to output type")] @@ -294,6 +293,7 @@ pub struct MigrationFailure { #[cfg(feature = "falcon")] #[derive(Clone, Debug)] pub struct SoftNpuPort { + pub link_name: String, pub backend_name: String, pub backend_spec: DlpiNetworkBackend, } @@ -317,26 +317,6 @@ struct ParsedNicRequest { nic: Nic, } -struct ParsedPciBridgeRequest { - name: String, - bridge: PciPciBridge, -} - -#[cfg(feature = "falcon")] -struct ParsedSoftNpuPort { - name: String, - port: SoftNpuPort, -} - -#[cfg(feature = "falcon")] -#[derive(Default)] -struct ParsedSoftNpu { - pub pci_port: Option, - pub ports: Vec, - pub p9_device: Option, - pub p9fs: Option, -} - /// Generates NIC device and backend names from the NIC's PCI path. This is /// needed because the `name` field in a propolis-client /// `NetworkInterfaceRequest` is actually the name of the host vNIC to bind to, diff --git a/bin/propolis-server/src/lib/vm/ensure.rs b/bin/propolis-server/src/lib/vm/ensure.rs index 41632194e..ae6040f4d 100644 --- a/bin/propolis-server/src/lib/vm/ensure.rs +++ b/bin/propolis-server/src/lib/vm/ensure.rs @@ -352,7 +352,7 @@ async fn initialize_vm_objects( "spec" => #?spec, "properties" => #?properties, "use_reservoir" => options.use_reservoir, - "bootrom" => %options.toml_config.bootrom.display()); + "bootrom" => %options.bootrom_path.display()); let vmm_log = log.new(slog::o!("component" => "vmm")); @@ -373,7 +373,6 @@ async fn initialize_vm_objects( crucible_backends: Default::default(), spec: &spec, properties: &properties, - toml_config: &options.toml_config, producer_registry: options.oximeter_registry.clone(), state: MachineInitializerState::default(), kstat_sampler: initialize_kstat_sampler( @@ -384,7 +383,7 @@ async fn initialize_vm_objects( stats_vm: VirtualMachine::new(spec.board.cpus, &properties), }; - init.initialize_rom(options.toml_config.bootrom.as_path())?; + init.initialize_rom(options.bootrom_path.as_path())?; let chipset = init.initialize_chipset( &(event_queue.clone() as Arc), @@ -416,7 +415,9 @@ async fn initialize_vm_objects( init.initialize_storage_devices(&chipset, options.nexus_client.clone()) .await?; - let ramfb = init.initialize_fwcfg(spec.board.cpus)?; + let ramfb = + init.initialize_fwcfg(spec.board.cpus, &options.bootrom_version)?; + init.initialize_cpus().await?; let vcpu_tasks = Box::new(crate::vcpu_tasks::VcpuTasks::new( &machine, diff --git a/bin/propolis-server/src/lib/vm/mod.rs b/bin/propolis-server/src/lib/vm/mod.rs index 258c1976f..c820b7dea 100644 --- a/bin/propolis-server/src/lib/vm/mod.rs +++ b/bin/propolis-server/src/lib/vm/mod.rs @@ -79,7 +79,7 @@ //! In the latter case, the driver moves to `Rundown` and allows `VmObjects` //! teardown to drive the state machine to `RundownComplete`. -use std::{collections::BTreeMap, net::SocketAddr, sync::Arc}; +use std::{collections::BTreeMap, net::SocketAddr, path::PathBuf, sync::Arc}; use active::ActiveVm; use ensure::VmEnsureRequest; @@ -279,9 +279,12 @@ impl std::fmt::Display for VmState { /// Parameters to an instance ensure operation. pub(super) struct EnsureOptions { - /// A reference to the VM configuration specified in the config TOML passed - /// to this propolis-server process. - pub(super) toml_config: Arc, + /// The path to the bootrom to load into the guest. + pub(super) bootrom_path: PathBuf, + + /// The bootrom version string to expose to the guest. If None, the machine + /// initializer chooses a default. + pub(super) bootrom_version: Option, /// True if VMs should allocate memory from the kernel VMM reservoir. pub(super) use_reservoir: bool, diff --git a/bin/propolis-server/src/main.rs b/bin/propolis-server/src/main.rs index b4127ec31..a45718405 100644 --- a/bin/propolis-server/src/main.rs +++ b/bin/propolis-server/src/main.rs @@ -75,11 +75,14 @@ enum Args { /// Runs the Propolis server. Run { #[clap(action)] - cfg: PathBuf, + bootrom_path: PathBuf, #[clap(name = "PROPOLIS_IP:PORT", action)] propolis_addr: SocketAddr, + #[clap(long, action)] + bootrom_version: Option, + /// Method for registering as an Oximeter metric producer. /// /// The following values are supported: @@ -117,7 +120,8 @@ pub fn run_openapi() -> Result<(), String> { } fn run_server( - config_app: config::Config, + bootrom_path: PathBuf, + bootrom_version: Option, config_dropshot: dropshot::ConfigDropshot, config_metrics: Option, vnc_addr: Option, @@ -146,7 +150,8 @@ fn run_server( let use_reservoir = config::reservoir_decide(&log); let context = server::DropshotEndpointContext::new( - config_app, + bootrom_path, + bootrom_version, use_reservoir, log.new(slog::o!()), config_metrics, @@ -279,9 +284,14 @@ fn main() -> anyhow::Result<()> { match args { Args::OpenApi => run_openapi() .map_err(|e| anyhow!("Cannot generate OpenAPI spec: {}", e)), - Args::Run { cfg, propolis_addr, metric_addr, vnc_addr, log_level } => { - let config = config::parse(cfg)?; - + Args::Run { + bootrom_path, + bootrom_version, + propolis_addr, + metric_addr, + vnc_addr, + log_level, + } => { // Dropshot configuration. let config_dropshot = ConfigDropshot { bind_address: propolis_addr, @@ -298,7 +308,14 @@ fn main() -> anyhow::Result<()> { propolis_addr.ip(), )?; - run_server(config, config_dropshot, metric_config, vnc_addr, log) + run_server( + bootrom_path, + bootrom_version, + config_dropshot, + metric_config, + vnc_addr, + log, + ) } } } diff --git a/crates/propolis-api-types/src/instance_spec/components/devices.rs b/crates/propolis-api-types/src/instance_spec/components/devices.rs index 25505cbe6..dcbb8d7e4 100644 --- a/crates/propolis-api-types/src/instance_spec/components/devices.rs +++ b/crates/propolis-api-types/src/instance_spec/components/devices.rs @@ -139,17 +139,17 @@ pub struct SoftNpuPciPort { pub pci_path: PciPath, } -/// Describes a SoftNPU network port. +/// Describes a port in a SoftNPU emulated ASIC. /// /// This is only supported by Propolis servers compiled with the `falcon` /// feature. #[derive(Clone, Deserialize, Serialize, Debug, JsonSchema)] #[serde(deny_unknown_fields)] pub struct SoftNpuPort { - /// The name of the SoftNpu port. - pub name: String, + /// The data link name for this port. + pub link_name: String, - /// The name of the device's backend. + /// The name of the port's associated DLPI backend. pub backend_name: String, } diff --git a/crates/propolis-server-config/Cargo.toml b/crates/propolis-config-toml/Cargo.toml similarity index 75% rename from crates/propolis-server-config/Cargo.toml rename to crates/propolis-config-toml/Cargo.toml index e5259e418..937bf1f7b 100644 --- a/crates/propolis-server-config/Cargo.toml +++ b/crates/propolis-config-toml/Cargo.toml @@ -1,5 +1,5 @@ [package] -name = "propolis-server-config" +name = "propolis-config-toml" version = "0.0.0" license = "MPL-2.0" edition = "2021" @@ -10,7 +10,9 @@ doctest = false [dependencies] cpuid_profile_config.workspace = true +propolis-client.workspace = true serde.workspace = true serde_derive.workspace = true toml.workspace = true thiserror.workspace = true +uuid.workspace = true diff --git a/crates/propolis-server-config/src/lib.rs b/crates/propolis-config-toml/src/lib.rs similarity index 93% rename from crates/propolis-server-config/src/lib.rs rename to crates/propolis-config-toml/src/lib.rs index 796683485..43189a934 100644 --- a/crates/propolis-server-config/src/lib.rs +++ b/crates/propolis-config-toml/src/lib.rs @@ -3,7 +3,7 @@ // file, You can obtain one at https://mozilla.org/MPL/2.0/. use std::collections::BTreeMap; -use std::path::{Path, PathBuf}; +use std::path::Path; use std::str::FromStr; use serde_derive::{Deserialize, Serialize}; @@ -11,15 +11,13 @@ use thiserror::Error; pub use cpuid_profile_config::CpuidProfile; +pub mod spec; + /// Configuration for the Propolis server. // NOTE: This is expected to change over time; portions of the hard-coded // configuration will likely become more dynamic. #[derive(Serialize, Deserialize, Debug, PartialEq)] pub struct Config { - pub bootrom: PathBuf, - - pub bootrom_version: Option, - #[serde(default, rename = "pci_bridge")] pub pci_bridges: Vec, @@ -38,8 +36,6 @@ pub struct Config { impl Default for Config { fn default() -> Self { Self { - bootrom: PathBuf::new(), - bootrom_version: None, pci_bridges: Vec::new(), chipset: Chipset { options: BTreeMap::new() }, devices: BTreeMap::new(), @@ -151,8 +147,7 @@ mod test { #[test] fn config_can_be_serialized_as_toml() { - let dummy_config = - Config { bootrom: "/boot".into(), ..Default::default() }; + let dummy_config = Config { ..Default::default() }; let serialized = toml::ser::to_string(&dummy_config).unwrap(); let deserialized: Config = toml::de::from_str(&serialized).unwrap(); assert_eq!(dummy_config, deserialized); @@ -161,7 +156,6 @@ mod test { #[test] fn parse_basic_config() { let raw = r#" -bootrom = "/path/to/bootrom" [chipset] chipset-opt = "copt" @@ -183,10 +177,8 @@ path = "/etc/passwd" "#; let cfg: Config = toml::de::from_str(raw).unwrap(); - use std::path::PathBuf; use toml::Value; - assert_eq!(cfg.bootrom, PathBuf::from("/path/to/bootrom")); assert_eq!(cfg.chipset.get_string("chipset-opt"), Some("copt")); assert!(cfg.devices.contains_key("drv0")); diff --git a/crates/propolis-config-toml/src/spec.rs b/crates/propolis-config-toml/src/spec.rs new file mode 100644 index 000000000..73584afdf --- /dev/null +++ b/crates/propolis-config-toml/src/spec.rs @@ -0,0 +1,392 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +//! Functions for converting a [`super::Config`] into instance spec elements. + +use std::{ + collections::BTreeMap, + str::{FromStr, ParseBoolError}, +}; + +use propolis_client::{ + types::{ + ComponentV0, DlpiNetworkBackend, FileStorageBackend, + MigrationFailureInjector, NvmeDisk, P9fs, PciPciBridge, SoftNpuP9, + SoftNpuPciPort, SoftNpuPort, VirtioDisk, VirtioNetworkBackend, + VirtioNic, + }, + PciPath, +}; +use thiserror::Error; + +pub const MIGRATION_FAILURE_DEVICE_NAME: &str = "test-migration-failure"; + +type SpecKey = String; + +#[derive(Debug, Error)] +pub enum TomlToSpecError { + #[error("unrecognized device type {0:?}")] + UnrecognizedDeviceType(String), + + #[error("invalid value {0:?} for enable-pcie flag in chipset")] + EnablePcieParseFailed(String), + + #[error("failed to get PCI path for device {0:?}")] + InvalidPciPath(String), + + #[error("failed to parse PCI path string {0:?}")] + PciPathParseFailed(String, #[source] std::io::Error), + + #[error("invalid storage device kind {kind:?} for device {name:?}")] + InvalidStorageDeviceType { kind: String, name: String }, + + #[error("no backend name for storage device {0:?}")] + NoBackendNameForStorageDevice(String), + + #[error("invalid storage backend kind {kind:?} for backend {name:?}")] + InvalidStorageBackendType { kind: String, name: String }, + + #[error("couldn't find storage device {device:?}'s backend {backend:?}")] + StorageDeviceBackendNotFound { device: String, backend: String }, + + #[error("couldn't get path for file backend {0:?}")] + InvalidFileBackendPath(String), + + #[error("failed to parse read-only option for file backend {0:?}")] + FileBackendReadonlyParseFailed(String, #[source] ParseBoolError), + + #[error("failed to get VNIC name for device {0:?}")] + NoVnicName(String), + + #[error("failed to get source for p9 device {0:?}")] + NoP9Source(String), + + #[error("failed to get source for p9 device {0:?}")] + NoP9Target(String), +} + +#[derive(Clone, Debug, Default)] +pub struct SpecConfig { + pub enable_pcie: bool, + pub components: BTreeMap, +} + +impl TryFrom<&super::Config> for SpecConfig { + type Error = TomlToSpecError; + + fn try_from(config: &super::Config) -> Result { + let mut spec = SpecConfig { + enable_pcie: config + .chipset + .options + .get("enable-pcie") + .map(|v| { + v.as_bool().ok_or_else(|| { + TomlToSpecError::EnablePcieParseFailed(v.to_string()) + }) + }) + .transpose()? + .unwrap_or(false), + ..Default::default() + }; + + for (device_name, device) in config.devices.iter() { + let device_id = SpecKey::from(device_name.clone()); + let driver = device.driver.as_str(); + if device_name == MIGRATION_FAILURE_DEVICE_NAME { + const FAIL_EXPORTS: &str = "fail_exports"; + const FAIL_IMPORTS: &str = "fail_imports"; + let fail_exports = device + .options + .get(FAIL_EXPORTS) + .and_then(|val| val.as_integer()) + .unwrap_or(0) + .max(0) as u32; + let fail_imports = device + .options + .get(FAIL_IMPORTS) + .and_then(|val| val.as_integer()) + .unwrap_or(0) + .max(0) as u32; + + spec.components.insert( + MIGRATION_FAILURE_DEVICE_NAME.to_owned(), + ComponentV0::MigrationFailureInjector( + MigrationFailureInjector { fail_exports, fail_imports }, + ), + ); + + continue; + } + + match driver { + // If this is a storage device, parse its "block_dev" property + // to get the name of its corresponding backend. + "pci-virtio-block" | "pci-nvme" => { + let (device_spec, backend_id) = + parse_storage_device_from_config(device_name, device)?; + + let backend_name = backend_id.to_string(); + let backend_config = + config.block_devs.get(&backend_name).ok_or_else( + || TomlToSpecError::StorageDeviceBackendNotFound { + device: device_name.to_owned(), + backend: backend_name.to_string(), + }, + )?; + + let backend_spec = parse_storage_backend_from_config( + &backend_name, + backend_config, + )?; + + spec.components.insert(device_id, device_spec); + spec.components.insert(backend_id, backend_spec); + } + "pci-virtio-viona" => { + let ParsedNic { device_spec, backend_spec, backend_id } = + parse_network_device_from_config(device_name, device)?; + + spec.components + .insert(device_id, ComponentV0::VirtioNic(device_spec)); + + spec.components.insert( + backend_id, + ComponentV0::VirtioNetworkBackend(backend_spec), + ); + } + "softnpu-pci-port" => { + let pci_path: PciPath = + device.get("pci-path").ok_or_else(|| { + TomlToSpecError::InvalidPciPath( + device_name.to_owned(), + ) + })?; + + spec.components.insert( + device_id, + ComponentV0::SoftNpuPciPort(SoftNpuPciPort { + pci_path, + }), + ); + } + "softnpu-port" => { + let vnic_name = + device.get_string("vnic").ok_or_else(|| { + TomlToSpecError::NoVnicName(device_name.to_owned()) + })?; + + let backend_name = format!("{}:backend", device_id); + + spec.components.insert( + device_id, + ComponentV0::SoftNpuPort(SoftNpuPort { + link_name: device_name.to_string(), + backend_name: backend_name.clone(), + }), + ); + + spec.components.insert( + backend_name, + ComponentV0::DlpiNetworkBackend(DlpiNetworkBackend { + vnic_name: vnic_name.to_owned(), + }), + ); + } + "softnpu-p9" => { + let pci_path: PciPath = + device.get("pci-path").ok_or_else(|| { + TomlToSpecError::InvalidPciPath( + device_name.to_owned(), + ) + })?; + + spec.components.insert( + device_id, + ComponentV0::SoftNpuP9(SoftNpuP9 { pci_path }), + ); + } + "pci-virtio-9p" => { + spec.components.insert( + device_id, + ComponentV0::P9fs(parse_p9fs_from_config( + device_name, + device, + )?), + ); + } + _ => { + return Err(TomlToSpecError::UnrecognizedDeviceType( + driver.to_owned(), + )) + } + } + } + + for bridge in config.pci_bridges.iter() { + let pci_path = + PciPath::from_str(&bridge.pci_path).map_err(|e| { + TomlToSpecError::PciPathParseFailed( + bridge.pci_path.to_string(), + e, + ) + })?; + + spec.components.insert( + format!("pci-bridge-{}", bridge.pci_path), + ComponentV0::PciPciBridge(PciPciBridge { + downstream_bus: bridge.downstream_bus, + pci_path, + }), + ); + } + + Ok(spec) + } +} + +fn parse_storage_device_from_config( + name: &str, + device: &super::Device, +) -> Result<(ComponentV0, SpecKey), TomlToSpecError> { + enum Interface { + Virtio, + Nvme, + } + + let interface = match device.driver.as_str() { + "pci-virtio-block" => Interface::Virtio, + "pci-nvme" => Interface::Nvme, + _ => { + return Err(TomlToSpecError::InvalidStorageDeviceType { + kind: device.driver.clone(), + name: name.to_owned(), + }); + } + }; + + let backend_name = device + .options + .get("block_dev") + .ok_or_else(|| { + TomlToSpecError::NoBackendNameForStorageDevice(name.to_owned()) + })? + .as_str() + .ok_or_else(|| { + TomlToSpecError::NoBackendNameForStorageDevice(name.to_owned()) + })? + .to_string(); + + let pci_path: PciPath = device + .get("pci-path") + .ok_or_else(|| TomlToSpecError::InvalidPciPath(name.to_owned()))?; + + let id_to_return = backend_name.clone(); + Ok(( + match interface { + Interface::Virtio => { + ComponentV0::VirtioDisk(VirtioDisk { backend_name, pci_path }) + } + Interface::Nvme => { + ComponentV0::NvmeDisk(NvmeDisk { backend_name, pci_path }) + } + }, + id_to_return, + )) +} + +fn parse_storage_backend_from_config( + name: &str, + backend: &super::BlockDevice, +) -> Result { + let backend_spec = match backend.bdtype.as_str() { + "file" => ComponentV0::FileStorageBackend(FileStorageBackend { + path: backend + .options + .get("path") + .ok_or_else(|| { + TomlToSpecError::InvalidFileBackendPath(name.to_owned()) + })? + .as_str() + .ok_or_else(|| { + TomlToSpecError::InvalidFileBackendPath(name.to_owned()) + })? + .to_string(), + readonly: match backend.options.get("readonly") { + Some(toml::Value::Boolean(ro)) => Some(*ro), + Some(toml::Value::String(v)) => { + Some(v.parse::().map_err(|e| { + TomlToSpecError::FileBackendReadonlyParseFailed( + name.to_owned(), + e, + ) + })?) + } + _ => None, + } + .unwrap_or(false), + }), + _ => { + return Err(TomlToSpecError::InvalidStorageBackendType { + kind: backend.bdtype.clone(), + name: name.to_owned(), + }); + } + }; + + Ok(backend_spec) +} + +struct ParsedNic { + device_spec: VirtioNic, + backend_spec: VirtioNetworkBackend, + backend_id: SpecKey, +} + +fn parse_network_device_from_config( + name: &str, + device: &super::Device, +) -> Result { + let vnic_name = device + .get_string("vnic") + .ok_or_else(|| TomlToSpecError::NoVnicName(name.to_owned()))?; + + let pci_path: PciPath = device + .get("pci-path") + .ok_or_else(|| TomlToSpecError::InvalidPciPath(name.to_owned()))?; + + let backend_id = format!("{name}-backend"); + Ok(ParsedNic { + device_spec: VirtioNic { + backend_name: backend_id.clone(), + interface_id: uuid::Uuid::nil(), + pci_path, + }, + backend_spec: VirtioNetworkBackend { vnic_name: vnic_name.to_owned() }, + backend_id, + }) +} + +fn parse_p9fs_from_config( + name: &str, + device: &super::Device, +) -> Result { + let source = device + .get_string("source") + .ok_or_else(|| TomlToSpecError::NoP9Source(name.to_owned()))?; + let target = device + .get_string("target") + .ok_or_else(|| TomlToSpecError::NoP9Target(name.to_owned()))?; + let pci_path: PciPath = device + .get("pci-path") + .ok_or_else(|| TomlToSpecError::InvalidPciPath(name.to_owned()))?; + + let chunk_size = device.get("chunk_size").unwrap_or(65536); + Ok(P9fs { + source: source.to_owned(), + target: target.to_owned(), + chunk_size, + pci_path, + }) +} diff --git a/lib/propolis-client/Cargo.toml b/lib/propolis-client/Cargo.toml index 93b3cec86..355efa94a 100644 --- a/lib/propolis-client/Cargo.toml +++ b/lib/propolis-client/Cargo.toml @@ -11,6 +11,7 @@ async-trait.workspace = true base64.workspace = true futures.workspace = true progenitor.workspace = true +propolis_api_types.workspace = true rand.workspace = true reqwest = { workspace = true, features = ["json", "rustls-tls"] } schemars = { workspace = true, features = ["uuid1"] } diff --git a/lib/propolis-client/src/lib.rs b/lib/propolis-client/src/lib.rs index 7e780f831..03305aeaf 100644 --- a/lib/propolis-client/src/lib.rs +++ b/lib/propolis-client/src/lib.rs @@ -4,10 +4,17 @@ //! A client for the Propolis hypervisor frontend's server API. +// Re-export the PCI path type from propolis_api_types so that users can access +// its constructor and From impls. +pub use propolis_api_types::instance_spec::PciPath; + progenitor::generate_api!( spec = "../../openapi/propolis-server.json", interface = Builder, tags = Separate, + replace = { + PciPath = crate::PciPath, + }, patch = { // Some Crucible-related bits are re-exported through simulated // sled-agent and thus require JsonSchema diff --git a/lib/propolis-client/src/support.rs b/lib/propolis-client/src/support.rs index bcf5f10bc..228d309f3 100644 --- a/lib/propolis-client/src/support.rs +++ b/lib/propolis-client/src/support.rs @@ -18,28 +18,9 @@ use tokio_tungstenite::tungstenite::{Error as WSError, Message as WSMessage}; // re-export as an escape hatch for crate-version-matching problems pub use tokio_tungstenite::{tungstenite, WebSocketStream}; -use crate::types::{Chipset, I440Fx, PciPath}; +use crate::types::{Chipset, I440Fx}; use crate::Client as PropolisClient; -const PCI_DEV_PER_BUS: u8 = 32; -const PCI_FUNC_PER_DEV: u8 = 8; - -impl PciPath { - pub const fn new( - bus: u8, - device: u8, - function: u8, - ) -> Result { - if device > PCI_DEV_PER_BUS { - Err("device outside possible range") - } else if function > PCI_FUNC_PER_DEV { - Err("function outside possible range") - } else { - Ok(Self { bus, device, function }) - } - } -} - impl Default for Chipset { fn default() -> Self { Self::I440Fx(I440Fx { enable_pcie: false }) diff --git a/openapi/propolis-server.json b/openapi/propolis-server.json index 1d69ebadf..b02e9036c 100644 --- a/openapi/propolis-server.json +++ b/openapi/propolis-server.json @@ -1851,21 +1851,21 @@ "additionalProperties": false }, "SoftNpuPort": { - "description": "Describes a SoftNPU network port.\n\nThis is only supported by Propolis servers compiled with the `falcon` feature.", + "description": "Describes a port in a SoftNPU emulated ASIC.\n\nThis is only supported by Propolis servers compiled with the `falcon` feature.", "type": "object", "properties": { "backend_name": { - "description": "The name of the device's backend.", + "description": "The name of the port's associated DLPI backend.", "type": "string" }, - "name": { - "description": "The name of the SoftNpu port.", + "link_name": { + "description": "The data link name for this port.", "type": "string" } }, "required": [ "backend_name", - "name" + "link_name" ], "additionalProperties": false }, diff --git a/packaging/smf/method_script.sh b/packaging/smf/method_script.sh index 5c0768697..23a72412b 100755 --- a/packaging/smf/method_script.sh +++ b/packaging/smf/method_script.sh @@ -30,7 +30,7 @@ route get -inet6 default -inet6 "$GATEWAY" || route add -inet6 default -inet6 "$ args=( 'run' - '/var/svc/manifest/site/propolis-server/config.toml' + '/opt/oxide/propolis-server/blob/OVMF_CODE.fd' "[$LISTEN_ADDR]:$LISTEN_PORT" '--metric-addr' "$METRIC_ADDR" ) diff --git a/packaging/smf/propolis-server/config.toml b/packaging/smf/propolis-server/config.toml deleted file mode 100644 index c325ce3f0..000000000 --- a/packaging/smf/propolis-server/config.toml +++ /dev/null @@ -1,14 +0,0 @@ -# Configuration for propolis server. -# -# Refer to https://github.com/oxidecomputer/propolis#readme -# for more detail on the config format. - -bootrom = "/opt/oxide/propolis-server/blob/OVMF_CODE.fd" - -# NOTE: This VNIC is here for reference, but VNICs are typically managed by the -# Sled Agent. - -# [dev.net0] -# driver = "pci-virtio-viona" -# vnic = "vnic_prop0" -# pci-path = "0.5.0" diff --git a/phd-tests/framework/Cargo.toml b/phd-tests/framework/Cargo.toml index afb7856f4..ee666d722 100644 --- a/phd-tests/framework/Cargo.toml +++ b/phd-tests/framework/Cargo.toml @@ -24,7 +24,6 @@ hex.workspace = true libc.workspace = true newtype_derive.workspace = true propolis-client.workspace = true -propolis-server-config.workspace = true reqwest = { workspace = true, features = ["blocking"] } ring.workspace = true serde = { workspace = true, features = ["derive"] } diff --git a/phd-tests/framework/src/test_vm/config.rs b/phd-tests/framework/src/test_vm/config.rs index 68ddaaa9b..9c3497dbd 100644 --- a/phd-tests/framework/src/test_vm/config.rs +++ b/phd-tests/framework/src/test_vm/config.rs @@ -6,11 +6,14 @@ use std::sync::Arc; use anyhow::Context; use cpuid_utils::CpuidIdent; -use propolis_client::types::{ - Board, BootOrderEntry, BootSettings, Chipset, ComponentV0, Cpuid, - CpuidEntry, CpuidVendor, InstanceMetadata, InstanceSpecV0, - MigrationFailureInjector, NvmeDisk, PciPath, SerialPort, SerialPortNumber, - VirtioDisk, +use propolis_client::{ + types::{ + Board, BootOrderEntry, BootSettings, Chipset, ComponentV0, Cpuid, + CpuidEntry, CpuidVendor, InstanceMetadata, InstanceSpecV0, + MigrationFailureInjector, NvmeDisk, SerialPort, SerialPortNumber, + VirtioDisk, + }, + PciPath, }; use uuid::Uuid; @@ -209,21 +212,12 @@ impl<'dr> VmConfig<'dr> { migration_failure, } = self; - // Figure out where the bootrom is and generate the serialized contents - // of a Propolis server config TOML that points to it. - let bootrom = framework + let bootrom_path = framework .artifact_store .get_bootrom(bootrom_artifact) .await .context("looking up bootrom artifact")?; - let config_toml_contents = - toml::ser::to_string(&propolis_server_config::Config { - bootrom: bootrom.clone().into(), - ..Default::default() - }) - .context("serializing Propolis server config")?; - // The first disk in the boot list might not be the disk a test // *actually* expects to boot. // @@ -370,7 +364,7 @@ impl<'dr> VmConfig<'dr> { instance_spec: spec, disk_handles, guest_os_kind, - config_toml_contents, + bootrom_path, metadata, }) } diff --git a/phd-tests/framework/src/test_vm/mod.rs b/phd-tests/framework/src/test_vm/mod.rs index ca2fef4f9..c8f419dcc 100644 --- a/phd-tests/framework/src/test_vm/mod.rs +++ b/phd-tests/framework/src/test_vm/mod.rs @@ -5,7 +5,7 @@ //! Routines for starting VMs, changing their states, and interacting with their //! guest OSes. -use std::{fmt::Debug, io::Write, sync::Arc, time::Duration}; +use std::{fmt::Debug, sync::Arc, time::Duration}; use crate::{ guest_os::{ @@ -227,33 +227,12 @@ impl TestVm { params: ServerProcessParameters, cleanup_task_tx: UnboundedSender>, ) -> Result { - let config_filename = format!("{}.config.toml", &vm_spec.vm_name); - let mut config_toml_path = params.data_dir.to_path_buf(); - config_toml_path.push(config_filename); - let mut config_file = std::fs::OpenOptions::new() - .write(true) - .truncate(true) - .create(true) - .open(&config_toml_path) - .with_context(|| { - format!("opening config file {} for writing", config_toml_path) - })?; - - config_file - .write_all(vm_spec.config_toml_contents.as_bytes()) - .with_context(|| { - format!( - "writing config toml to config file {}", - config_toml_path - ) - })?; - let data_dir = params.data_dir.to_path_buf(); let server_addr = params.server_addr; let server = server::PropolisServer::new( &vm_spec.vm_name, params, - &config_toml_path, + &vm_spec.bootrom_path, )?; let client = Client::new(&format!("http://{}", server_addr)); diff --git a/phd-tests/framework/src/test_vm/server.rs b/phd-tests/framework/src/test_vm/server.rs index a1be00a01..01da19d37 100644 --- a/phd-tests/framework/src/test_vm/server.rs +++ b/phd-tests/framework/src/test_vm/server.rs @@ -42,7 +42,7 @@ impl PropolisServer { pub(crate) fn new( vm_name: &str, process_params: ServerProcessParameters, - config_toml_path: &Utf8Path, + bootrom_path: &Utf8Path, ) -> Result { let ServerProcessParameters { server_path, @@ -54,7 +54,7 @@ impl PropolisServer { info!( ?server_path, - ?config_toml_path, + ?bootrom_path, ?server_addr, "Launching Propolis server" ); @@ -67,7 +67,7 @@ impl PropolisServer { .args([ server_path.as_str(), "run", - config_toml_path.as_str(), + bootrom_path.as_str(), server_addr.to_string().as_str(), vnc_addr.to_string().as_str(), ]) diff --git a/phd-tests/framework/src/test_vm/spec.rs b/phd-tests/framework/src/test_vm/spec.rs index 97899c025..dc276b2df 100644 --- a/phd-tests/framework/src/test_vm/spec.rs +++ b/phd-tests/framework/src/test_vm/spec.rs @@ -8,8 +8,10 @@ use crate::{ disk::{self, DiskConfig}, guest_os::GuestOsKind, }; -use propolis_client::types::{ - ComponentV0, DiskRequest, InstanceMetadata, InstanceSpecV0, PciPath, Slot, +use camino::Utf8PathBuf; +use propolis_client::{ + types::{ComponentV0, DiskRequest, InstanceMetadata, InstanceSpecV0, Slot}, + PciPath, }; use uuid::Uuid; @@ -27,8 +29,8 @@ pub struct VmSpec { /// The guest OS adapter to use for the VM. pub guest_os_kind: GuestOsKind, - /// The contents of the config TOML to write to run this VM. - pub config_toml_contents: String, + /// The bootrom path to pass to this VM's Propolis server processes. + pub bootrom_path: Utf8PathBuf, /// Metadata used to track instance timeseries data. pub metadata: InstanceMetadata, @@ -90,11 +92,11 @@ impl VmSpec { } fn convert_to_slot(pci_path: PciPath) -> anyhow::Result { - match pci_path.device { + match pci_path.device() { dev @ 0x10..=0x17 => Ok(Slot(dev - 0x10)), _ => Err(anyhow::anyhow!( "PCI device number {} out of range", - pci_path.device + pci_path.device() )), } }