From 8853a0f913fe601cb46ba9f5f0184d99a526de6b Mon Sep 17 00:00:00 2001 From: Greg Colombo Date: Wed, 13 Nov 2024 20:44:38 +0000 Subject: [PATCH 1/4] server: remove config TOML processing Remove the propolis-server logic to add devices from the config TOML when fielding an instance ensure request. This logic never adds any devices in production anyway because the zone image's config TOML never specifies any. The config TOML does specify the path to the bootrom the server should load (and an optional version string to display to the guest). Supply these arguments on the command line instead of getting them from the TOML. Once this is done, the config TOML in the zone image is unnecessary, so remove it. --- Cargo.lock | 2 - 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 ++++++----- bin/propolis-server/src/lib/spec/builder.rs | 61 +---------------- bin/propolis-server/src/lib/spec/mod.rs | 21 ------ 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 +++++++-- 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 | 13 +--- 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 | 5 +- 18 files changed, 91 insertions(+), 240 deletions(-) delete mode 100644 packaging/smf/propolis-server/config.toml diff --git a/Cargo.lock b/Cargo.lock index cae74c2eb..3f8ca2b5b 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", @@ -4383,7 +4382,6 @@ dependencies = [ "oximeter-instruments", "oximeter-producer", "propolis", - "propolis-server-config", "propolis_api_types", "propolis_types", "reqwest 0.12.7", 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/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/mod.rs b/bin/propolis-server/src/lib/spec/mod.rs index dc133fa02..8ba51ba1d 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")] @@ -317,26 +316,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/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..be39b19ca 100644 --- a/phd-tests/framework/src/test_vm/config.rs +++ b/phd-tests/framework/src/test_vm/config.rs @@ -209,21 +209,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 +361,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..761fffd72 100644 --- a/phd-tests/framework/src/test_vm/spec.rs +++ b/phd-tests/framework/src/test_vm/spec.rs @@ -8,6 +8,7 @@ use crate::{ disk::{self, DiskConfig}, guest_os::GuestOsKind, }; +use camino::Utf8PathBuf; use propolis_client::types::{ ComponentV0, DiskRequest, InstanceMetadata, InstanceSpecV0, PciPath, Slot, }; @@ -27,8 +28,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, From f2ba178d57b1b1c697feca60247e9f822df05e09 Mon Sep 17 00:00:00 2001 From: Greg Colombo Date: Wed, 13 Nov 2024 23:05:45 +0000 Subject: [PATCH 2/4] rename propolis-server-config and add spec helper Now that config TOMLs no longer configure Propolis servers, rename the propolis-server-config crate to propolis-config-toml, then add code to convert a config TOML into a set of instance spec configuration options. Most of this code already existed in propolis-server's config TOML processor, which is now deleted. Also tweak a couple of field names in the SoftNPU port spec type to clarify that the name that a port has *within a SoftNPU setup* is distinct from its instance spec component key. --- Cargo.lock | 25 +- Cargo.toml | 2 +- .../src/lib/spec/api_spec_v0.rs | 3 +- .../src/lib/spec/config_toml.rs | 384 ----------------- bin/propolis-server/src/lib/spec/mod.rs | 1 + .../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 +- phd-tests/framework/src/test_vm/config.rs | 13 +- phd-tests/framework/src/test_vm/spec.rs | 9 +- 15 files changed, 448 insertions(+), 448 deletions(-) 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 diff --git a/Cargo.lock b/Cargo.lock index 3f8ca2b5b..611c9389d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4298,6 +4298,7 @@ dependencies = [ "base64 0.21.7", "futures", "progenitor", + "propolis_api_types", "rand", "reqwest 0.12.7", "schemars", @@ -4310,6 +4311,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" @@ -4408,17 +4422,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-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/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 8ba51ba1d..c211693fd 100644 --- a/bin/propolis-server/src/lib/spec/mod.rs +++ b/bin/propolis-server/src/lib/spec/mod.rs @@ -293,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, } 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/phd-tests/framework/src/test_vm/config.rs b/phd-tests/framework/src/test_vm/config.rs index be39b19ca..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; diff --git a/phd-tests/framework/src/test_vm/spec.rs b/phd-tests/framework/src/test_vm/spec.rs index 761fffd72..dc276b2df 100644 --- a/phd-tests/framework/src/test_vm/spec.rs +++ b/phd-tests/framework/src/test_vm/spec.rs @@ -9,8 +9,9 @@ use crate::{ guest_os::GuestOsKind, }; use camino::Utf8PathBuf; -use propolis_client::types::{ - ComponentV0, DiskRequest, InstanceMetadata, InstanceSpecV0, PciPath, Slot, +use propolis_client::{ + types::{ComponentV0, DiskRequest, InstanceMetadata, InstanceSpecV0, Slot}, + PciPath, }; use uuid::Uuid; @@ -91,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() )), } } From 5f04cc372749976e0c91f3597d1096180b59e57c Mon Sep 17 00:00:00 2001 From: Greg Colombo Date: Wed, 13 Nov 2024 23:44:14 +0000 Subject: [PATCH 3/4] make propolis-cli spec-driven --- Cargo.lock | 2 + bin/propolis-cli/Cargo.toml | 2 + bin/propolis-cli/README.md | 41 +++++ bin/propolis-cli/src/main.rs | 309 ++++++++++++++++++++++++++--------- 4 files changed, 275 insertions(+), 79 deletions(-) create mode 100644 bin/propolis-cli/README.md diff --git a/Cargo.lock b/Cargo.lock index 611c9389d..a4282d133 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4275,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", 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? From fb4b08159dee36275c5a33c0a05356d65cd70ee8 Mon Sep 17 00:00:00 2001 From: Greg Colombo Date: Thu, 14 Nov 2024 01:07:12 +0000 Subject: [PATCH 4/4] don't archive nonexistent TOML files --- .github/buildomat/phd-run-with-args.sh | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) 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