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 036951799..bc4ea64fd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3810,6 +3810,7 @@ dependencies = [ "camino", "cfg-if", "cpuid_utils", + "crucible-client-types", "errno 0.2.8", "fatfs", "flate2", @@ -3818,7 +3819,6 @@ dependencies = [ "libc", "newtype_derive", "propolis-client", - "propolis-server-config", "rand", "reqwest 0.12.7", "ring 0.17.8", @@ -4261,10 +4261,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", @@ -4282,8 +4284,10 @@ version = "0.1.0" dependencies = [ "async-trait", "base64 0.21.7", + "crucible-client-types", "futures", "progenitor", + "propolis_api_types", "rand", "reqwest 0.12.7", "schemars", @@ -4296,6 +4300,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" @@ -4368,7 +4385,6 @@ dependencies = [ "oximeter-instruments", "oximeter-producer", "propolis", - "propolis-server-config", "propolis_api_types", "propolis_types", "reqwest 0.12.7", @@ -4395,17 +4411,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" @@ -4459,6 +4464,7 @@ dependencies = [ "propolis_types", "schemars", "serde", + "serde_with", "thiserror", "uuid", ] @@ -5364,9 +5370,9 @@ dependencies = [ [[package]] name = "serde_with" -version = "3.9.0" +version = "3.11.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "69cecfa94848272156ea67b2b1a53f20fc7bc638c4a46d2f8abde08f05f4b857" +checksum = "8e28bdad6db2b8340e449f7108f020b3b092e8583a9e3fb82713e1d4e71fe817" dependencies = [ "base64 0.22.1", "chrono", @@ -5382,9 +5388,9 @@ dependencies = [ [[package]] name = "serde_with_macros" -version = "3.9.0" +version = "3.11.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a8fee4991ef4f274617a51ad4af30519438dacb2f56ac773b08a1922ff743350" +checksum = "9d846214a9854ef724f3da161b426242d8de7c1fc7de2f89bb1efcb154dca79d" dependencies = [ "darling", "proc-macro2", diff --git a/Cargo.toml b/Cargo.toml index 4d93671e8..f7ed30caf 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -46,7 +46,7 @@ bhyve_api_sys = { path = "crates/bhyve-api/sys" } cpuid_utils = { path = "crates/cpuid-utils" } cpuid_profile_config = { path = "crates/cpuid-profile-config" } dladm = { path = "crates/dladm" } -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" } @@ -145,6 +145,7 @@ serde_arrays = "0.1" serde_derive = "1.0" serde_json = "1.0" serde_test = "1.0.138" +serde_with = "3.11.0" slog = "2.7" slog-async = "2.8" slog-bunyan = "2.4.0" diff --git a/README.md b/README.md index 6386bae20..fe17c4f7f 100644 --- a/README.md +++ b/README.md @@ -36,7 +36,8 @@ implementation details, consumed by Propolis components. - bhyve-api: API (ioctls & structs) for the illumos bhyve kernel VMM - dladm: Some thin wrappers around `dladm` queries -- propolis-server-config: Type definitions for `propolis-server` config file +- propolis-config-toml: Type definitions for expressing static Propolis server + configurations in TOML format - propolis-types: Publically exposed (via `propolis-server`) types, intergral to the `propolis` library - viona-api: API (ioctls & structs) for the illumos viona driver diff --git a/bin/mock-server/src/lib/api_types.rs b/bin/mock-server/src/lib/api_types.rs index 0f9586007..c39b8ea33 100644 --- a/bin/mock-server/src/lib/api_types.rs +++ b/bin/mock-server/src/lib/api_types.rs @@ -22,7 +22,6 @@ impl TryFrom for propolis_types::PciPath { .map_err(|e| e.to_string()) } } -pub use propolis_types::PciPath; // Duplicate the parameter types for the endpoints related to the serial console diff --git a/bin/mock-server/src/lib/copied.rs b/bin/mock-server/src/lib/copied.rs deleted file mode 100644 index 8692394c6..000000000 --- a/bin/mock-server/src/lib/copied.rs +++ /dev/null @@ -1,60 +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/. - -//! Bits copied from propolis-server, rather than splitting them out into some -//! shared dependency - -use crate::api_types::{types as api, PciPath}; - -use thiserror::Error; - -#[derive(Clone, Copy, Debug)] -pub(crate) enum SlotType { - Nic, - Disk, - CloudInit, -} - -#[allow(unused)] -#[derive(Debug, Error)] -pub(crate) enum ServerSpecBuilderError { - #[error("The string {0} could not be converted to a PCI path")] - PciPathNotParseable(String), - - #[error( - "Could not translate PCI slot {0} for device type {1:?} to a PCI path" - )] - PciSlotInvalid(u8, SlotType), - - #[error("Unrecognized storage device interface {0}")] - UnrecognizedStorageDevice(String), - - #[error("Unrecognized storage backend type {0}")] - UnrecognizedStorageBackend(String), - - #[error("Device {0} requested missing backend {1}")] - DeviceMissingBackend(String, String), - - #[error("Error in server config TOML: {0}")] - ConfigTomlError(String), - - #[error("Error serializing {0} into spec element: {1}")] - SerializationError(String, serde_json::error::Error), -} - -pub(crate) fn slot_to_pci_path( - slot: api::Slot, - ty: SlotType, -) -> Result { - match ty { - // Slots for NICS: 0x08 -> 0x0F - SlotType::Nic if slot.0 <= 7 => PciPath::new(0, slot.0 + 0x8, 0), - // Slots for Disks: 0x10 -> 0x17 - SlotType::Disk if slot.0 <= 7 => PciPath::new(0, slot.0 + 0x10, 0), - // Slot for CloudInit - SlotType::CloudInit if slot.0 == 0 => PciPath::new(0, slot.0 + 0x18, 0), - _ => return Err(ServerSpecBuilderError::PciSlotInvalid(slot.0, ty)), - } - .map_err(|_| ServerSpecBuilderError::PciSlotInvalid(slot.0, ty)) -} diff --git a/bin/mock-server/src/lib/lib.rs b/bin/mock-server/src/lib/lib.rs index 0c14738e7..c879fa0b5 100644 --- a/bin/mock-server/src/lib/lib.rs +++ b/bin/mock-server/src/lib/lib.rs @@ -4,17 +4,15 @@ //! Implementation of a mock Propolis server -use std::io::{Error as IoError, ErrorKind}; use std::sync::Arc; -use base64::Engine; use dropshot::{ channel, endpoint, ApiDescription, HttpError, HttpResponseCreated, HttpResponseOk, HttpResponseUpdatedNoContent, Query, RequestContext, TypedBody, WebsocketConnection, }; use futures::SinkExt; -use slog::{error, info, Logger}; +use slog::{error, Logger}; use thiserror::Error; use tokio::sync::{watch, Mutex}; use tokio_tungstenite::tungstenite::protocol::{Role, WebSocketConfig}; @@ -22,10 +20,8 @@ use tokio_tungstenite::tungstenite::Message; use tokio_tungstenite::WebSocketStream; mod api_types; -mod copied; -use crate::copied::{slot_to_pci_path, SlotType}; -use api_types::types as api; +use api_types::types::{self as api, InstanceEnsureRequest}; #[derive(Debug, Eq, PartialEq, Error)] pub enum Error { @@ -140,13 +136,7 @@ async fn instance_ensure( request: TypedBody, ) -> Result, HttpError> { let server_context = rqctx.context(); - let request = request.into_inner(); - let (properties, nics, disks, cloud_init_bytes) = ( - request.properties, - request.nics, - request.disks, - request.cloud_init_bytes, - ); + let InstanceEnsureRequest { properties, .. } = request.into_inner(); // Handle an already-initialized instance let mut instance = server_context.instance.lock().await; @@ -160,58 +150,6 @@ async fn instance_ensure( migrate: None, })); } - - // Perform some basic validation of the requested properties - for nic in &nics { - info!(server_context.log, "Creating NIC: {:#?}", nic); - slot_to_pci_path(nic.slot, SlotType::Nic).map_err(|e| { - let err = IoError::new( - ErrorKind::InvalidData, - format!("Cannot parse vnic PCI: {}", e), - ); - HttpError::for_internal_error(format!( - "Cannot build instance: {}", - err - )) - })?; - } - - for disk in &disks { - info!(server_context.log, "Creating Disk: {:#?}", disk); - slot_to_pci_path(disk.slot, SlotType::Disk).map_err(|e| { - let err = IoError::new( - ErrorKind::InvalidData, - format!("Cannot parse disk PCI: {}", e), - ); - HttpError::for_internal_error(format!( - "Cannot build instance: {}", - err - )) - })?; - info!(server_context.log, "Disk {} created successfully", disk.name); - } - - if let Some(cloud_init_bytes) = &cloud_init_bytes { - info!(server_context.log, "Creating cloud-init disk"); - slot_to_pci_path(api::Slot(0), SlotType::CloudInit).map_err(|e| { - let err = IoError::new(ErrorKind::InvalidData, e.to_string()); - HttpError::for_internal_error(format!( - "Cannot build instance: {}", - err - )) - })?; - base64::engine::general_purpose::STANDARD - .decode(cloud_init_bytes) - .map_err(|e| { - let err = IoError::new(ErrorKind::InvalidInput, e.to_string()); - HttpError::for_internal_error(format!( - "Cannot build instance: {}", - err - )) - })?; - info!(server_context.log, "cloud-init disk created"); - } - *instance = Some(InstanceContext::new(properties, &server_context.log)); Ok(HttpResponseCreated(api::InstanceEnsureResponse { migrate: None })) } diff --git a/bin/propolis-cli/Cargo.toml b/bin/propolis-cli/Cargo.toml index f20e5ccaa..55b9d2aa0 100644 --- a/bin/propolis-cli/Cargo.toml +++ b/bin/propolis-cli/Cargo.toml @@ -7,9 +7,11 @@ 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-config-toml.workspace = true propolis-client.workspace = true slog.workspace = true slog-async.workspace = true diff --git a/bin/propolis-cli/README.md b/bin/propolis-cli/README.md new file mode 100644 index 000000000..e4334b78e --- /dev/null +++ b/bin/propolis-cli/README.md @@ -0,0 +1,43 @@ +# 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 +bootrom = "/path/to/bootrom/OVMF_CODE.fd" + +[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 ba00f0a87..e2067e587 100644 --- a/bin/propolis-cli/src/main.rs +++ b/bin/propolis-cli/src/main.rs @@ -12,10 +12,17 @@ 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, VersionedInstanceSpec}; +use propolis_client::types::{ + BlobStorageBackend, Board, Chipset, ComponentV0, CrucibleStorageBackend, + I440Fx, InstanceInitializationMethod, InstanceMetadata, InstanceSpecV0, + NvmeDisk, QemuPvpanic, ReplacementComponent, SerialPort, SerialPortNumber, + VirtioDisk, +}; +use propolis_client::{PciPath, SpecKey}; +use propolis_config_toml::spec::SpecConfig; use slog::{o, Drain, Level, Logger}; use tokio::io::{AsyncReadExt, AsyncWriteExt}; use tokio_tungstenite::tungstenite::{ @@ -27,9 +34,8 @@ use uuid::Uuid; use propolis_client::{ support::{InstanceSerialConsoleHelper, WSClientOffset}, types::{ - DiskRequest, InstanceEnsureRequest, InstanceMigrateInitiateRequest, - InstanceProperties, InstanceStateRequested, InstanceVcrReplace, - MigrationState, + InstanceEnsureRequest, InstanceProperties, InstanceStateRequested, + InstanceVcrReplace, MigrationState, }, Client, }; @@ -66,21 +72,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)] @@ -167,6 +160,270 @@ 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", conflicts_with = "spec", action)] + vcpus: u8, + + /// Memory allocated to instance (MiB) + #[clap(short, default_value = "1024", conflicts_with = "spec", action)] + memory: u64, + + /// File containing a legacy propolis-server configuration TOML + #[clap(long, conflicts_with = "spec", action)] + config_toml: Option, + + /// File with a JSON array of DiskRequest structs + #[clap(long, conflicts_with = "spec", action)] + crucible_disks: Option, + + // cloud_init ISO file + #[clap(long, conflicts_with = "spec", action)] + cloud_init: Option, +} + +/// A legacy request to attach a disk to an instance. +/// +/// Previous versions of the Propolis server API accepted an array of these +/// structs that specified the Crucible disks to attach to a new VM. The +/// CLI's `--crucible-disks` flag accepts to a path containing a JSON array of +/// disk requests to attach to the request. The server API no longer accepts +/// requests in this format; this type exists to maintain compatibility for +/// existing CLI users. +#[derive(Clone, Debug, serde::Deserialize)] +struct DiskRequest { + pub name: String, + pub slot: u8, + pub read_only: bool, + pub device: String, + pub volume_construction_request: + crucible_client_types::VolumeConstructionRequest, +} + +#[derive(Clone, Debug)] +struct DiskComponents { + device_name: String, + device_spec: ComponentV0, + backend_name: String, + backend_spec: CrucibleStorageBackend, +} + +impl DiskRequest { + fn get_components(&self) -> anyhow::Result { + let slot = self.slot + 0x10; + let backend_name = format!("{}-backend", self.name); + let device_spec = match self.device.as_ref() { + "virtio" => ComponentV0::VirtioDisk(VirtioDisk { + backend_id: SpecKey::Name(backend_name.clone()), + pci_path: PciPath::new(0, slot, 0)?, + }), + "nvme" => ComponentV0::NvmeDisk(NvmeDisk { + backend_id: SpecKey::Name(backend_name.clone()), + pci_path: PciPath::new(0, slot, 0)?, + }), + _ => anyhow::bail!( + "invalid device type in disk request: {:?}", + self.device + ), + }; + + let backend_spec = CrucibleStorageBackend { + readonly: self.read_only, + request_json: serde_json::to_string( + &self.volume_construction_request, + )?, + }; + + Ok(DiskComponents { + device_name: self.name.clone(), + device_spec, + backend_name, + backend_spec, + }) + } +} + +impl VmConfig { + fn instance_spec(&self) -> anyhow::Result { + // If the configuration specifies an instance spec path, just read the + // spec from that path without combining it with any other parts. (All + // the other parts should be absent anyway, since they're mutually + // exclusive with the `spec` option in the arguments struct.) + if let Some(spec_path) = &self.spec { + return parse_json_file(spec_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() { + if spec + .components + .insert(id.to_string(), component.clone()) + .is_some() + { + anyhow::bail!("duplicate component with ID {id:?}"); + } + } + } + + for disk_request in self + .crucible_disks + .as_ref() + .map(|path| parse_json_file::>(path)) + .transpose()? + .iter() + .flatten() + { + let components = disk_request.get_components()?; + if spec + .components + .insert(components.device_name.clone(), components.device_spec) + .is_some() + { + anyhow::bail!( + "duplicate component with ID {:?}", + components.device_name + ); + } + + if spec + .components + .insert( + components.backend_name.clone(), + ComponentV0::CrucibleStorageBackend( + components.backend_spec, + ), + ) + .is_some() + { + anyhow::bail!( + "duplicate component with ID {:?}", + components.backend_name + ); + } + } + + if let Some(cloud_init) = &self.cloud_init { + 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"; + if spec + .components + .insert( + CLOUD_INIT_NAME.to_owned(), + ComponentV0::VirtioDisk(VirtioDisk { + backend_id: SpecKey::Name( + "cloud-init-backend".to_owned(), + ), + pci_path: PciPath::new(0, 0x18, 0).unwrap(), + }), + ) + .is_some() + { + anyhow::bail!( + "duplicate component with ID '{CLOUD_INIT_NAME}'" + ); + } + + if spec + .components + .insert( + CLOUD_INIT_BACKEND_NAME.to_owned(), + ComponentV0::BlobStorageBackend(BlobStorageBackend { + base64: bytes, + readonly: true, + }), + ) + .is_some() + { + anyhow::bail!( + "duplicate component with ID '{CLOUD_INIT_BACKEND_NAME}'" + ); + } + } + + for (name, port) in [ + ("com1", SerialPortNumber::Com1), + ("com2", SerialPortNumber::Com2), + ("com3", SerialPortNumber::Com3), + ] { + if spec + .components + .insert( + name.to_owned(), + ComponentV0::SerialPort(SerialPort { num: port }), + ) + .is_some() + { + anyhow::bail!("duplicate component with ID {name:?}"); + } + } + + // If no softnpu devices have been specified at this point, COM4 is also + // available, so add it to the spec. + if !spec + .components + .iter() + .any(|(_, comp)| matches!(comp, ComponentV0::SoftNpuPort(_))) + { + let (name, port) = ("com4", SerialPortNumber::Com4); + if spec + .components + .insert( + name.to_owned(), + ComponentV0::SerialPort(SerialPort { num: port }), + ) + .is_some() + { + anyhow::bail!("duplicate component with ID {name:?}"); + } + } + + const PVPANIC_NAME: &str = "pvpanic"; + if spec + .components + .insert( + PVPANIC_NAME.to_owned(), + ComponentV0::QemuPvpanic(QemuPvpanic { enable_isa: true }), + ) + .is_some() + { + anyhow::bail!("duplicate component with ID '{PVPANIC_NAME}'"); + } + + Ok(spec) + } +} + fn parse_state(state: &str) -> anyhow::Result { match state.to_lowercase().as_str() { "run" => Ok(InstanceStateRequested::Run), @@ -242,10 +499,7 @@ async fn new_instance( client: &Client, name: String, id: Uuid, - vcpus: u8, - memory: u64, - disks: Vec, - cloud_init_bytes: Option, + config: VmConfig, metadata: InstanceMetadata, ) -> anyhow::Result<()> { let properties = InstanceProperties { @@ -257,14 +511,9 @@ async fn new_instance( let request = InstanceEnsureRequest { properties, - vcpus, - memory, - // TODO: Allow specifying NICs - nics: vec![], - disks, - boot_settings: None, - migrate: None, - cloud_init_bytes, + init: InstanceInitializationMethod::Spec { + spec: config.instance_spec()?, + }, }; // Try to create the instance @@ -504,7 +753,20 @@ async fn migrate_instance( anyhow!("failed to get src instance properties") })?; let src_uuid = src_instance.properties.id; - let VersionedInstanceSpec::V0(spec) = &src_instance.spec; + + let replace_components = disks + .iter() + .map(|d| -> anyhow::Result<(String, ReplacementComponent)> { + let components = d.get_components()?; + + Ok(( + components.backend_name, + ReplacementComponent::CrucibleStorageBackend( + components.backend_spec, + ), + )) + }) + .collect::>()?; let request = InstanceEnsureRequest { properties: InstanceProperties { @@ -512,20 +774,11 @@ async fn migrate_instance( 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, - migrate: Some(InstanceMigrateInitiateRequest { + init: InstanceInitializationMethod::MigrationTarget { migration_id: Uuid::new_v4(), + replace_components, src_addr: src_addr.to_string(), - src_uuid, - }), - cloud_init_bytes: None, + }, }; // Initiate the migration via the destination instance @@ -655,10 +908,7 @@ async fn main() -> anyhow::Result<()> { Command::New { name, uuid, - vcpus, - memory, - crucible_disks, - cloud_init, + config, silo_id, project_id, sled_id, @@ -666,19 +916,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) @@ -697,10 +934,7 @@ async fn main() -> anyhow::Result<()> { &client, name.to_string(), uuid.unwrap_or_else(Uuid::new_v4), - vcpus, - memory, - disks, - cloud_init_bytes, + config, 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..e1bcd2b68 100644 --- a/bin/propolis-server/README.md +++ b/bin/propolis-server/README.md @@ -1,66 +1,28 @@ # Propolis Server +The Propolis server binary provides a REST API to create and manage Propolis +VMs. It typically runs in the context of a complete Oxide deployment, where it +is operated by the sled agent, but it can also be run as a freestanding binary +for ad hoc testing and management of Propolis VMs. + ## 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: +The server binary requires a path to a [guest bootrom +image](../propolis-standalone#guest-bootrom) on the local filesystem. It also +must run with privileges sufficient to create `bhyve` virtual machines; the +`pfexec(1)` utility can help enable these privileges for sufficiently-privileged +users. -``` -# propolis-server run -``` +To build and run the server: -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 - -**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" - -[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" +# 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: +The API will be served on `ip:port`. -### 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 easiest way to interact with the server is to use +[`propolis-cli`](../propolis-cli), but you can also interact directly with the +REST API using utilities like cURL. The server's [OpenAPI +specification](../../openapi/propolis-server.json) is checked into the repo. 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 5e8069b7d..f8dd44b0d 100644 --- a/bin/propolis-server/src/lib/initializer.rs +++ b/bin/propolis-server/src/lib/initializer.rs @@ -44,14 +44,13 @@ use propolis::hw::uart::LpcUart; use propolis::hw::{nvme, virtio}; use propolis::intr_pins; use propolis::vmm::{self, Builder, Machine}; -use propolis_api_types::instance_spec; use propolis_api_types::instance_spec::components::devices::SerialPortNumber; +use propolis_api_types::instance_spec::{self, SpecKey}; use propolis_api_types::InstanceProperties; use propolis_types::{CpuidIdent, CpuidVendor}; use slog::info; use strum::IntoEnumIterator; use thiserror::Error; -use uuid::Uuid; /// An error that can arise while initializing a new machine. #[derive(Debug, Error)] @@ -84,13 +83,13 @@ pub enum MachineInitError { InMemoryBackendDecodeFailed(#[from] base64::DecodeError), #[error("multiple Crucible disks with ID {0}")] - DuplicateCrucibleBackendId(Uuid), + DuplicateCrucibleBackendId(SpecKey), #[error("boot order entry {0:?} does not refer to an attached disk")] - BootOrderEntryWithoutDevice(String), + BootOrderEntryWithoutDevice(SpecKey), #[error("boot entry {0:?} refers to a device on non-zero PCI bus {1}")] - BootDeviceOnDownstreamPciBus(String, u8), + BootDeviceOnDownstreamPciBus(SpecKey, u8), #[error("failed to insert {0} fwcfg entry")] FwcfgInsertFailed(&'static str, #[source] fwcfg::InsertError), @@ -171,7 +170,7 @@ impl RegisteredChipset { struct StorageBackendInstance { be: Arc, - crucible: Option<(uuid::Uuid, Arc)>, + crucible: Option>, } #[derive(Default)] @@ -187,11 +186,11 @@ 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, pub(crate) stats_vm: crate::stats::VirtualMachine, + pub(crate) bootrom_version: Option, } impl<'a> MachineInitializer<'a> { @@ -267,7 +266,8 @@ impl<'a> MachineInitializer<'a> { pub fn initialize_hpet(&mut self) { let hpet = BhyveHpet::create(self.machine.hdl.clone()); - self.devices.insert(hpet.type_name().into(), hpet.clone()); + self.devices + .insert(SpecKey::Name(hpet.type_name().to_owned()), hpet.clone()); } pub fn initialize_chipset( @@ -343,20 +343,35 @@ impl<'a> MachineInitializer<'a> { do_pci_attach(i440fx::DEFAULT_PM_BDF, chipset_pm.clone()); chipset_pm.attach(&self.machine.bus_pio); - self.devices - .insert(chipset_hb.type_name().into(), chipset_hb.clone()); self.devices.insert( - chipset_lpc.type_name().into(), + SpecKey::Name(chipset_hb.type_name().to_owned()), + chipset_hb.clone(), + ); + + self.devices.insert( + SpecKey::Name(chipset_lpc.type_name().to_owned()), chipset_lpc.clone(), ); - self.devices.insert(chipset_pm.type_name().into(), chipset_pm); + + self.devices.insert( + SpecKey::Name(chipset_pm.type_name().to_owned()), + chipset_pm, + ); // Record attachment for any bridges in PCI topology too for (bdf, bridge) in bridges { - self.devices.insert( - format!("{}-{bdf}", bridge.type_name()), - bridge, - ); + let spec_element = self + .spec + .pci_pci_bridges + .iter() + .find(|(_, spec_bridge)| { + bdf == spec_bridge.pci_path.into() + }) + .expect( + "all PCI bridges should appear in the topology", + ); + + self.devices.insert(spec_element.0.clone(), bridge); } Ok(RegisteredChipset { chipset: chipset_hb, isa: chipset_lpc }) @@ -384,7 +399,7 @@ impl<'a> MachineInitializer<'a> { let dev = LpcUart::new(chipset.irq_pin(irq).unwrap()); dev.set_autodiscard(true); LpcUart::attach(&dev, &self.machine.bus_pio, port); - self.devices.insert(name.to_owned(), dev.clone()); + self.devices.insert(name.clone(), dev.clone()); if desc.num == SerialPortNumber::Com1 { assert!(com1.is_none()); com1 = Some(dev); @@ -408,7 +423,10 @@ impl<'a> MachineInitializer<'a> { chipset.irq_pin(ibmpc::IRQ_PS2_AUX).unwrap(), chipset.reset_pin(), ); - self.devices.insert(ps2_ctrl.type_name().into(), ps2_ctrl.clone()); + self.devices.insert( + SpecKey::Name(ps2_ctrl.type_name().to_owned()), + ps2_ctrl.clone(), + ); ps2_ctrl } @@ -422,7 +440,7 @@ impl<'a> MachineInitializer<'a> { let poller = chardev::BlockingFileOutput::new(debug_file); poller.attach(Arc::clone(&dbg) as Arc); - self.devices.insert(dbg.type_name().into(), dbg); + self.devices.insert(SpecKey::Name(dbg.type_name().to_owned()), dbg); Ok(()) } @@ -431,14 +449,13 @@ impl<'a> MachineInitializer<'a> { &mut self, virtual_machine: VirtualMachine, ) -> Result<(), MachineInitError> { - if let Some(pvpanic) = &self.spec.pvpanic { - if pvpanic.spec.enable_isa { + if let Some(spec::QemuPvpanic { id, spec }) = &self.spec.pvpanic { + if spec.enable_isa { let pvpanic = QemuPvpanic::create( self.log.new(slog::o!("dev" => "qemu-pvpanic")), ); pvpanic.attach_pio(&self.machine.bus_pio); - self.devices - .insert(pvpanic.type_name().into(), pvpanic.clone()); + self.devices.insert(id.clone(), pvpanic.clone()); if let Some(ref registry) = self.producer_registry { let producer = crate::stats::PvpanicProducer::new( @@ -458,13 +475,13 @@ impl<'a> MachineInitializer<'a> { async fn create_storage_backend_from_spec( &self, backend_spec: &StorageBackend, - backend_name: &str, + backend_id: &SpecKey, nexus_client: &Option, ) -> Result { match backend_spec { StorageBackend::Crucible(spec) => { info!(self.log, "Creating Crucible disk"; - "backend_name" => backend_name); + "backend_id" => %backend_id); let vcr: VolumeConstructionRequest = serde_json::from_str(&spec.request_json) @@ -498,13 +515,7 @@ impl<'a> MachineInitializer<'a> { .await .context("failed to create Crucible backend")?; - let crucible = Some(( - be.get_uuid() - .await - .context("failed to get Crucible backend ID")?, - be.clone(), - )); - + let crucible = Some(be.clone()); Ok(StorageBackendInstance { be, crucible }) } StorageBackend::File(spec) => { @@ -587,22 +598,22 @@ impl<'a> MachineInitializer<'a> { Nvme, } - for (disk_name, disk) in &self.spec.disks { + for (disk_id, disk) in &self.spec.disks { info!( self.log, "Creating storage device"; - "name" => disk_name, + "name" => %disk_id, "spec" => ?disk.device_spec ); - let (device_interface, backend_name, pci_path) = match &disk + let (device_interface, backend_id, pci_path) = match &disk .device_spec { spec::StorageDevice::Virtio(disk) => { - (DeviceInterface::Virtio, &disk.backend_name, disk.pci_path) + (DeviceInterface::Virtio, &disk.backend_id, disk.pci_path) } spec::StorageDevice::Nvme(disk) => { - (DeviceInterface::Nvme, &disk.backend_name, disk.pci_path) + (DeviceInterface::Nvme, &disk.backend_id, disk.pci_path) } }; @@ -611,18 +622,17 @@ impl<'a> MachineInitializer<'a> { let StorageBackendInstance { be: backend, crucible } = self .create_storage_backend_from_spec( &disk.backend_spec, - backend_name, + backend_id, &nexus_client, ) .await?; - self.block_backends.insert(backend_name.clone(), backend.clone()); + self.block_backends.insert(backend_id.clone(), backend.clone()); let block_dev: Arc = match device_interface { DeviceInterface::Virtio => { let vioblk = virtio::PciVirtioBlock::new(0x100); - self.devices - .insert(format!("pci-virtio-{}", bdf), vioblk.clone()); + self.devices.insert(disk_id.clone(), vioblk.clone()); block::attach(vioblk.clone(), backend).unwrap(); chipset.pci_attach(bdf, vioblk.clone()); vioblk @@ -630,26 +640,27 @@ impl<'a> MachineInitializer<'a> { DeviceInterface::Nvme => { // Limit data transfers to 1MiB (2^8 * 4k) in size let mdts = Some(8); - let component = format!("nvme-{}", disk_name); + let component = format!("nvme-{}", disk_id); let nvme = nvme::PciNvme::create( - disk_name.to_owned(), + disk_id.to_string(), mdts, self.log.new(slog::o!("component" => component)), ); - self.devices - .insert(format!("pci-nvme-{bdf}"), nvme.clone()); + self.devices.insert(disk_id.clone(), nvme.clone()); block::attach(nvme.clone(), backend).unwrap(); chipset.pci_attach(bdf, nvme.clone()); nvme } }; - if let Some((disk_id, backend)) = crucible { + if let Some(backend) = crucible { let block_size = backend.block_size().await; - let prev = self.crucible_backends.insert(disk_id, backend); + let prev = + self.crucible_backends.insert(backend_id.clone(), backend); + if prev.is_some() { return Err(MachineInitError::DuplicateCrucibleBackendId( - disk_id, + backend_id.clone(), )); } @@ -663,14 +674,16 @@ impl<'a> MachineInitializer<'a> { continue; }; - // Register the block device as a metric producer, if we've been - // setup to do so. Note we currently only do this for the Crucible - // backend, in which case we have the disk ID. - if let Some(registry) = &self.producer_registry { + // Register the block device as a metric producer, provided that + // metrics are enabled and this Crucible backend is identified + // by its UUID. + if let (Some(registry), SpecKey::Uuid(disk_id)) = + (&self.producer_registry, &backend_id) + { let stats = VirtualDiskProducer::new( block_size, self.properties.id, - disk_id, + *disk_id, &self.properties.metadata, ); if let Err(e) = registry.register_producer(stats.clone()) { @@ -709,8 +722,8 @@ impl<'a> MachineInitializer<'a> { let mut interface_ids: Option = self.kstat_sampler.as_ref().map(|_| Vec::new()); - for (device_name, nic) in &self.spec.nics { - info!(self.log, "Creating vNIC {}", device_name); + for (device_id, nic) in &self.spec.nics { + info!(self.log, "Creating vNIC {}", device_id); let bdf: pci::Bdf = nic.device_spec.pci_path.into(); let viona = virtio::PciVirtioViona::new( @@ -719,11 +732,10 @@ impl<'a> MachineInitializer<'a> { &self.machine.hdl, ) .with_context(|| { - format!("failed to create viona device {device_name:?}") + format!("failed to create viona device {device_id:?}") })?; - self.devices - .insert(format!("pci-virtio-viona-{}", bdf), viona.clone()); + self.devices.insert(device_id.clone(), viona.clone()); // Only push to interface_ids if kstat_sampler exists if let Some(ref mut ids) = interface_ids { @@ -732,7 +744,7 @@ impl<'a> MachineInitializer<'a> { viona.instance_id().with_context(|| { format!( "failed to get viona instance ID for network \ - device {device_name:?}" + device {device_id:?}" ) })?, )); @@ -755,49 +767,34 @@ impl<'a> MachineInitializer<'a> { } #[cfg(not(feature = "omicron-build"))] - pub fn initialize_test_devices( - &mut self, - toml_cfg: &std::collections::BTreeMap< - String, - propolis_server_config::Device, - >, - ) { + pub fn initialize_test_devices(&mut self) { use propolis::hw::testdev::{ MigrationFailureDevice, MigrationFailures, }; - if let Some(dev) = toml_cfg.get(MigrationFailureDevice::NAME) { + if let Some(crate::spec::MigrationFailure { id, spec }) = + &self.spec.migration_failure + { const FAIL_EXPORTS: &str = "fail_exports"; const FAIL_IMPORTS: &str = "fail_imports"; - let fail_exports = dev - .options - .get(FAIL_EXPORTS) - .and_then(|val| val.as_integer()) - .unwrap_or(0); - let fail_imports = dev - .options - .get(FAIL_IMPORTS) - .and_then(|val| val.as_integer()) - .unwrap_or(0); - - if fail_exports <= 0 && fail_imports <= 0 { + if spec.fail_exports == 0 && spec.fail_imports == 0 { info!( self.log, "migration failure device will not fail, as both `{FAIL_EXPORTS}` and `{FAIL_IMPORTS}` are 0"; - FAIL_EXPORTS => ?fail_exports, - FAIL_IMPORTS => ?fail_imports, + FAIL_EXPORTS => ?spec.fail_exports, + FAIL_IMPORTS => ?spec.fail_imports, ); } let dev = MigrationFailureDevice::create( &self.log, MigrationFailures { - exports: fail_exports as usize, - imports: fail_imports as usize, + exports: spec.fail_exports as usize, + imports: spec.fail_imports as usize, }, ); - self.devices.insert(MigrationFailureDevice::NAME.into(), dev); + self.devices.insert(id.clone(), dev); } } @@ -806,6 +803,8 @@ impl<'a> MachineInitializer<'a> { &mut self, chipset: &RegisteredChipset, ) -> Result<(), MachineInitError> { + use crate::spec::SoftNpuPort; + let softnpu = &self.spec.softnpu; // Check to make sure we actually have both a pci port and at least one @@ -820,23 +819,27 @@ impl<'a> MachineInitializer<'a> { // Get a Vec of references to the ports which will then be sorted by // port name. - let mut ports: Vec<_> = softnpu.ports.iter().collect(); + let mut ports: Vec = + softnpu.ports.values().cloned().collect(); // SoftNpu ports are named __vnic by falcon, where // indicates the intended order. - ports.sort_by_key(|p| p.0.as_str()); + ports.sort_by(|p1, p2| p1.link_name.cmp(&p2.link_name)); let data_links = ports .iter() - .map(|port| port.1.backend_spec.vnic_name.clone()) + .map(|port| port.backend_spec.vnic_name.clone()) .collect(); // Set up an LPC uart for ASIC management comms from the guest. // - // NOTE: SoftNpu squats on com4. + // NOTE: SoftNpu uses COM4 to provide the guest with a P4 program + // management interface. See the docs for the Propolis `SoftNpu` + // structure for further details. let uart = LpcUart::new(chipset.irq_pin(ibmpc::IRQ_COM4).unwrap()); uart.set_autodiscard(true); LpcUart::attach(&uart, &self.machine.bus_pio, ibmpc::PORT_COM4); - self.devices.insert("softnpu-uart".to_string(), uart.clone()); + self.devices + .insert(SpecKey::Name("softnpu-uart".to_string()), uart.clone()); // Start with no pipeline. The guest must load the initial P4 program. let pipeline = Arc::new(std::sync::Mutex::new(None)); @@ -852,7 +855,8 @@ impl<'a> MachineInitializer<'a> { ); let vio9p = virtio::p9fs::PciVirtio9pfs::new(0x40, Arc::new(p9_handler)); - self.devices.insert("softnpu-p9fs".to_string(), vio9p.clone()); + self.devices + .insert(SpecKey::Name("softnpu-p9fs".to_string()), vio9p.clone()); let bdf = softnpu .p9_device .as_ref() @@ -873,13 +877,16 @@ impl<'a> MachineInitializer<'a> { ) .context("failed to register softnpu")?; - self.devices.insert("softnpu-main".to_string(), softnpu.clone()); + self.devices + .insert(SpecKey::Name("softnpu-main".to_string()), softnpu.clone()); // Create the SoftNpu PCI port. - self.devices - .insert("softnpu-pciport".to_string(), softnpu.pci_port.clone()); - chipset.pci_attach(pci_port.pci_path.into(), softnpu.pci_port.clone()); + self.devices.insert( + SpecKey::Name("softnpu-pciport".to_string()), + softnpu.pci_port.clone(), + ); + chipset.pci_attach(pci_port.pci_path.into(), softnpu.pci_port.clone()); Ok(()) } @@ -899,7 +906,8 @@ impl<'a> MachineInitializer<'a> { self.log.clone(), ); let vio9p = virtio::p9fs::PciVirtio9pfs::new(0x40, Arc::new(handler)); - self.devices.insert("falcon-p9fs".to_string(), vio9p.clone()); + self.devices + .insert(SpecKey::Name("falcon-p9fs".to_string()), vio9p.clone()); chipset.pci_attach(p9fs.pci_path.into(), vio9p); } @@ -908,16 +916,14 @@ impl<'a> MachineInitializer<'a> { let rom_size = self.state.rom_size_bytes.expect("ROM is already populated"); - let bios_version = self - .toml_config - .bootrom_version - .as_deref() - .unwrap_or("v0.8") - .try_into() - .expect("bootrom version string doesn't contain NUL bytes"); let smb_type0 = smbios::table::Type0 { vendor: "Oxide".try_into().unwrap(), - bios_version, + bios_version: self + .bootrom_version + .as_deref() + .unwrap_or("v0.8") + .try_into() + .unwrap(), bios_release_date: "The Aftermath 30, 3185 YOLD" .try_into() .unwrap(), @@ -1095,14 +1101,14 @@ impl<'a> MachineInitializer<'a> { // Theoretically we could support booting from network devices by // matching them here and adding their PCI paths, but exactly what // would happen is ill-understood. So, only check disks here. - if let Some(spec) = self.spec.disks.get(boot_entry.name.as_str()) { + if let Some(spec) = self.spec.disks.get(&boot_entry.component_id) { match &spec.device_spec { StorageDevice::Virtio(disk) => { let bdf: pci::Bdf = disk.pci_path.into(); if bdf.bus.get() != 0 { return Err( MachineInitError::BootDeviceOnDownstreamPciBus( - boot_entry.name.clone(), + boot_entry.component_id.clone(), bdf.bus.get(), ), ); @@ -1115,7 +1121,7 @@ impl<'a> MachineInitializer<'a> { if bdf.bus.get() != 0 { return Err( MachineInitError::BootDeviceOnDownstreamPciBus( - boot_entry.name.clone(), + boot_entry.component_id.clone(), bdf.bus.get(), ), ); @@ -1130,7 +1136,7 @@ impl<'a> MachineInitializer<'a> { // This should be unreachable - we check that the boot disk is // valid when constructing the spec we're initializing from. return Err(MachineInitError::BootOrderEntryWithoutDevice( - boot_entry.name.clone(), + boot_entry.component_id.clone(), )); } } @@ -1190,8 +1196,9 @@ impl<'a> MachineInitializer<'a> { fwcfg.attach(&self.machine.bus_pio, &self.machine.acc_mem); - self.devices.insert(fwcfg.type_name().into(), fwcfg); - self.devices.insert(ramfb.type_name().into(), ramfb.clone()); + self.devices.insert(SpecKey::Name(fwcfg.type_name().to_owned()), fwcfg); + self.devices + .insert(SpecKey::Name(ramfb.type_name().to_owned()), ramfb.clone()); Ok(ramfb) } @@ -1226,7 +1233,10 @@ impl<'a> MachineInitializer<'a> { .context("failed to set vcpu capabilities")?; // The vCPUs behave like devices, so add them to the list as well - self.devices.insert(format!("vcpu-{}", vcpu.id), vcpu.clone()); + self.devices.insert( + SpecKey::Name(format!("vcpu-{}", vcpu.id)), + vcpu.clone(), + ); } if let Some(sampler) = self.kstat_sampler.as_ref() { track_vcpu_kstats(&self.log, sampler, &self.stats_vm).await; diff --git a/bin/propolis-server/src/lib/migrate/compat.rs b/bin/propolis-server/src/lib/migrate/compat.rs deleted file mode 100644 index a2d7fbfe8..000000000 --- a/bin/propolis-server/src/lib/migrate/compat.rs +++ /dev/null @@ -1,938 +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/. - -//! Associated functions for the [`crate::spec::Spec`] type that determine -//! whether two specs describe migration-compatible VMs. - -use std::collections::HashMap; - -use crate::spec::{self, SerialPortDevice}; - -use cpuid_utils::CpuidVendor; -use propolis_api_types::instance_spec::{ - components::{ - board::Chipset, - devices::{PciPciBridge, SerialPortNumber}, - }, - PciPath, -}; -use thiserror::Error; - -trait CompatCheck { - type Error; - - fn is_compatible_with(&self, other: &Self) -> Result<(), Self::Error>; -} - -#[derive(Debug, Error)] -pub enum CompatibilityError { - #[error(transparent)] - Board(#[from] BoardIncompatibility), - - #[error(transparent)] - Pvpanic(#[from] PvpanicIncompatibility), - - #[error("collection {0} incompatible")] - Collection(String, #[source] CollectionIncompatibility), - - #[cfg(feature = "falcon")] - #[error("can't migrate instances containing softnpu devices")] - SoftNpu, -} - -#[derive(Debug, Error)] -pub enum BoardIncompatibility { - #[error("boards have different CPU counts (self: {this}, other: {other})")] - CpuCount { this: u8, other: u8 }, - - #[error( - "boards have different memory sizes (self: {this}, other: {other})" - )] - MemorySize { this: u64, other: u64 }, - - #[error( - "chipsets have different PCIe settings (self: {this}, other: {other})" - )] - PcieEnabled { this: bool, other: bool }, - - #[error(transparent)] - Cpuid(#[from] CpuidMismatch), -} - -#[derive(Debug, Error)] -pub enum CpuidMismatch { - #[error("CPUID is explicit in one spec but not the other (self: {this}, other: {other}")] - Explicitness { this: bool, other: bool }, - - #[error( - "CPUID sets have different CPU vendors (self: {this}, other: {other})" - )] - Vendor { this: CpuidVendor, other: CpuidVendor }, - - #[error(transparent)] - LeavesOrValues(#[from] cpuid_utils::CpuidSetMismatch), -} - -#[derive(Debug, Error)] -pub enum DiskIncompatibility { - #[error( - "disks have different device interfaces (self: {this}, other: {other})" - )] - Interface { this: &'static str, other: &'static str }, - - #[error("disks have different PCI paths (self: {this}, other: {other})")] - PciPath { this: PciPath, other: PciPath }, - - #[error( - "disks have different backend names (self: {this:?}, other: {other:?})" - )] - BackendName { this: String, other: String }, - - #[error( - "disks have different backend kinds (self: {this}, other: {other})" - )] - BackendKind { this: &'static str, other: &'static str }, - - #[error( - "disks have different read-only settings (self: {this}, other: {other})" - )] - ReadOnly { this: bool, other: bool }, -} - -#[derive(Debug, Error)] -pub enum NicIncompatibility { - #[error("NICs have different PCI paths (self: {this}, other: {other})")] - PciPath { this: PciPath, other: PciPath }, - - #[error( - "NICs have different backend names (self: {this}, other: {other})" - )] - BackendName { this: String, other: String }, -} - -#[derive(Debug, Error)] -pub enum SerialPortIncompatibility { - #[error("ports have different numbers (self: {this:?}, other: {other:?})")] - Number { this: SerialPortNumber, other: SerialPortNumber }, - - #[error("ports have different devices (self: {this}, other: {other})")] - Device { this: SerialPortDevice, other: SerialPortDevice }, -} - -#[derive(Debug, Error)] -pub enum BridgeIncompatibility { - #[error("bridges have different PCI paths (self: {this}, other: {other})")] - PciPath { this: PciPath, other: PciPath }, - - #[error("bridges have different downstream buses (self: {this}, other: {other})")] - DownstreamBus { this: u8, other: u8 }, -} - -#[derive(Debug, Error)] -pub enum PvpanicIncompatibility { - #[error("pvpanic presence differs (self: {this}, other: {other})")] - Presence { this: bool, other: bool }, - - #[error( - "pvpanic devices have different names (self: {this:?}, other: {other:?})" - )] - Name { this: String, other: String }, - - #[error( - "pvpanic devices have different ISA settings (self: {this}, other: {other})" - )] - EnableIsa { this: bool, other: bool }, -} - -#[derive(Debug, Error)] -pub enum ComponentIncompatibility { - #[error(transparent)] - Board(#[from] BoardIncompatibility), - - #[error(transparent)] - Disk(#[from] DiskIncompatibility), - - #[error(transparent)] - Nic(#[from] NicIncompatibility), - - #[error(transparent)] - SerialPort(#[from] SerialPortIncompatibility), - - #[error(transparent)] - PciPciBridge(#[from] BridgeIncompatibility), -} - -#[derive(Debug, Error)] -pub enum CollectionIncompatibility { - #[error( - "collections have different lengths (self: {this}, other: {other})" - )] - Length { this: usize, other: usize }, - - #[error("collection key {0} present in self but not other")] - KeyAbsent(String), - - #[error("component {0} incompatible")] - Component(String, #[source] ComponentIncompatibility), -} - -impl> CompatCheck - for HashMap -{ - type Error = CollectionIncompatibility; - - fn is_compatible_with( - &self, - other: &Self, - ) -> Result<(), CollectionIncompatibility> { - if self.len() != other.len() { - return Err(CollectionIncompatibility::Length { - this: self.len(), - other: other.len(), - }); - } - - for (key, this_val) in self.iter() { - let other_val = other.get(key).ok_or_else(|| { - CollectionIncompatibility::KeyAbsent(key.clone()) - })?; - - this_val.is_compatible_with(other_val).map_err(|e| { - CollectionIncompatibility::Component(key.clone(), e) - })?; - } - - Ok(()) - } -} - -impl spec::Spec { - fn is_board_compatible( - &self, - other: &Self, - ) -> Result<(), BoardIncompatibility> { - self.is_chipset_compatible(other)?; - self.is_cpuid_compatible(other)?; - - let this = &self.board; - let other = &other.board; - if this.cpus != other.cpus { - Err(BoardIncompatibility::CpuCount { - this: this.cpus, - other: other.cpus, - }) - } else if this.memory_mb != other.memory_mb { - Err(BoardIncompatibility::MemorySize { - this: this.memory_mb, - other: other.memory_mb, - }) - } else { - Ok(()) - } - } - - fn is_chipset_compatible( - &self, - other: &Self, - ) -> Result<(), BoardIncompatibility> { - let Chipset::I440Fx(this) = self.board.chipset; - let Chipset::I440Fx(other) = other.board.chipset; - - if this.enable_pcie != other.enable_pcie { - Err(BoardIncompatibility::PcieEnabled { - this: this.enable_pcie, - other: other.enable_pcie, - }) - } else { - Ok(()) - } - } - - fn is_cpuid_compatible(&self, other: &Self) -> Result<(), CpuidMismatch> { - match (&self.cpuid, &other.cpuid) { - (None, None) => Ok(()), - (Some(_), None) | (None, Some(_)) => { - Err(CpuidMismatch::Explicitness { - this: self.cpuid.is_some(), - other: other.cpuid.is_some(), - }) - } - (Some(this), Some(other)) => { - if this.vendor() != other.vendor() { - return Err(CpuidMismatch::Vendor { - this: this.vendor(), - other: other.vendor(), - }); - } - - this.is_equivalent_to(other)?; - Ok(()) - } - } - } - - fn is_pvpanic_compatible( - &self, - other: &Self, - ) -> Result<(), PvpanicIncompatibility> { - match (&self.pvpanic, &other.pvpanic) { - (None, None) => Ok(()), - (Some(this), Some(other)) if this.name != other.name => { - Err(PvpanicIncompatibility::Name { - this: this.name.clone(), - other: other.name.clone(), - }) - } - (Some(this), Some(other)) - if this.spec.enable_isa != other.spec.enable_isa => - { - Err(PvpanicIncompatibility::EnableIsa { - this: this.spec.enable_isa, - other: other.spec.enable_isa, - }) - } - (Some(_), Some(_)) => Ok(()), - (this, other) => Err(PvpanicIncompatibility::Presence { - this: this.is_some(), - other: other.is_some(), - }), - } - } - - pub(super) fn is_migration_compatible( - &self, - other: &Self, - ) -> Result<(), CompatibilityError> { - self.is_board_compatible(other)?; - self.disks.is_compatible_with(&other.disks).map_err(|e| { - CompatibilityError::Collection("disks".to_string(), e) - })?; - - self.nics.is_compatible_with(&other.nics).map_err(|e| { - CompatibilityError::Collection("nics".to_string(), e) - })?; - - self.serial.is_compatible_with(&other.serial).map_err(|e| { - CompatibilityError::Collection("serial ports".to_string(), e) - })?; - - self.pci_pci_bridges - .is_compatible_with(&other.pci_pci_bridges) - .map_err(|e| { - CompatibilityError::Collection("PCI bridges".to_string(), e) - })?; - - self.is_pvpanic_compatible(other)?; - - #[cfg(feature = "falcon")] - if self.softnpu.has_components() || other.softnpu.has_components() { - return Err(CompatibilityError::SoftNpu); - } - - Ok(()) - } -} - -impl spec::StorageDevice { - fn is_compatible_with( - &self, - other: &Self, - ) -> Result<(), DiskIncompatibility> { - if std::mem::discriminant(self) != std::mem::discriminant(other) { - Err(DiskIncompatibility::Interface { - this: self.kind(), - other: other.kind(), - }) - } else if self.pci_path() != other.pci_path() { - Err(DiskIncompatibility::PciPath { - this: self.pci_path(), - other: other.pci_path(), - }) - } else if self.backend_name() != other.backend_name() { - Err(DiskIncompatibility::BackendName { - this: self.backend_name().to_owned(), - other: other.backend_name().to_owned(), - }) - } else { - Ok(()) - } - } -} - -impl spec::StorageBackend { - fn is_compatible_with( - &self, - other: &Self, - ) -> Result<(), DiskIncompatibility> { - if std::mem::discriminant(self) != std::mem::discriminant(other) { - Err(DiskIncompatibility::BackendKind { - this: self.kind(), - other: other.kind(), - }) - } else if self.read_only() != other.read_only() { - Err(DiskIncompatibility::ReadOnly { - this: self.read_only(), - other: other.read_only(), - }) - } else { - Ok(()) - } - } -} - -impl CompatCheck for spec::Disk { - type Error = ComponentIncompatibility; - - fn is_compatible_with( - &self, - other: &Self, - ) -> Result<(), ComponentIncompatibility> { - self.device_spec.is_compatible_with(&other.device_spec)?; - self.backend_spec.is_compatible_with(&other.backend_spec)?; - Ok(()) - } -} - -impl CompatCheck for spec::Nic { - type Error = ComponentIncompatibility; - - fn is_compatible_with( - &self, - other: &Self, - ) -> Result<(), ComponentIncompatibility> { - if self.device_spec.pci_path != other.device_spec.pci_path { - Err(NicIncompatibility::PciPath { - this: self.device_spec.pci_path, - other: other.device_spec.pci_path, - }) - } else if self.device_spec.backend_name - != other.device_spec.backend_name - { - Err(NicIncompatibility::BackendName { - this: self.device_spec.backend_name.clone(), - other: other.device_spec.backend_name.clone(), - }) - } else { - Ok(()) - } - .map_err(ComponentIncompatibility::Nic) - } -} - -impl CompatCheck for spec::SerialPort { - type Error = ComponentIncompatibility; - - fn is_compatible_with( - &self, - other: &Self, - ) -> Result<(), ComponentIncompatibility> { - if self.num != other.num { - Err(SerialPortIncompatibility::Number { - this: self.num, - other: other.num, - }) - } else if std::mem::discriminant(&self.device) - != std::mem::discriminant(&other.device) - { - Err(SerialPortIncompatibility::Device { - this: self.device, - other: other.device, - }) - } else { - Ok(()) - } - .map_err(ComponentIncompatibility::SerialPort) - } -} - -impl CompatCheck for PciPciBridge { - type Error = ComponentIncompatibility; - - fn is_compatible_with( - &self, - other: &Self, - ) -> Result<(), ComponentIncompatibility> { - if self.pci_path != other.pci_path { - Err(BridgeIncompatibility::PciPath { - this: self.pci_path, - other: other.pci_path, - }) - } else if self.downstream_bus != other.downstream_bus { - Err(BridgeIncompatibility::DownstreamBus { - this: self.downstream_bus, - other: other.downstream_bus, - }) - } else { - Ok(()) - } - .map_err(ComponentIncompatibility::PciPciBridge) - } -} - -#[cfg(test)] -mod test { - use cpuid_utils::{CpuidIdent, CpuidSet, CpuidValues}; - use propolis_api_types::instance_spec::components::{ - backends::{ - CrucibleStorageBackend, FileStorageBackend, VirtioNetworkBackend, - }, - board::I440Fx, - devices::{ - NvmeDisk, QemuPvpanic as QemuPvpanicDesc, VirtioDisk, VirtioNic, - }, - }; - use spec::{QemuPvpanic, StorageDevice}; - use uuid::Uuid; - - use super::*; - - fn new_spec() -> spec::Spec { - let mut spec = spec::Spec::default(); - spec.board.cpus = 2; - spec.board.memory_mb = 512; - spec - } - - fn file_backend() -> spec::StorageBackend { - spec::StorageBackend::File(FileStorageBackend { - path: "/tmp/file.raw".to_owned(), - readonly: false, - }) - } - - fn crucible_backend() -> spec::StorageBackend { - spec::StorageBackend::Crucible(CrucibleStorageBackend { - request_json: "{}".to_owned(), - readonly: false, - }) - } - - fn nic() -> spec::Nic { - spec::Nic { - device_spec: VirtioNic { - pci_path: PciPath::new(0, 16, 0).unwrap(), - interface_id: Uuid::new_v4(), - backend_name: "vnic".to_owned(), - }, - backend_spec: VirtioNetworkBackend { - vnic_name: "vnic0".to_owned(), - }, - } - } - - fn serial_port() -> spec::SerialPort { - spec::SerialPort { - num: SerialPortNumber::Com1, - device: SerialPortDevice::Uart, - } - } - - fn bridge() -> PciPciBridge { - PciPciBridge { - downstream_bus: 1, - pci_path: PciPath::new(0, 24, 0).unwrap(), - } - } - - #[test] - fn cpu_mismatch() { - let s1 = new_spec(); - let mut s2 = s1.clone(); - s2.board.cpus += 1; - assert!(s1.is_migration_compatible(&s2).is_err()); - } - - #[test] - fn memory_mismatch() { - let s1 = new_spec(); - let mut s2 = s1.clone(); - s2.board.memory_mb *= 2; - assert!(s1.is_migration_compatible(&s2).is_err()); - } - - #[test] - fn pcie_mismatch() { - let mut s1 = new_spec(); - let mut s2 = s1.clone(); - s1.board.chipset = Chipset::I440Fx(I440Fx { enable_pcie: false }); - s2.board.chipset = Chipset::I440Fx(I440Fx { enable_pcie: true }); - assert!(s1.is_migration_compatible(&s2).is_err()); - } - - #[test] - fn pvpanic_name_mismatch() { - let mut s1 = new_spec(); - s1.pvpanic = Some(QemuPvpanic { - name: "pvpanic1".to_string(), - spec: QemuPvpanicDesc { enable_isa: true }, - }); - let mut s2 = s1.clone(); - s2.pvpanic.as_mut().unwrap().name = "pvpanic2".to_string(); - assert!(s1.is_migration_compatible(&s2).is_err()); - } - - #[test] - fn pvpanic_enable_isa_mismatch() { - let mut s1 = new_spec(); - s1.pvpanic = Some(QemuPvpanic { - name: "pvpanic".to_string(), - spec: QemuPvpanicDesc { enable_isa: true }, - }); - let mut s2 = s1.clone(); - s2.pvpanic.as_mut().unwrap().spec.enable_isa = false; - assert!(s1.is_migration_compatible(&s2).is_err()); - } - - #[test] - fn compatible_disks() { - let mut s1 = new_spec(); - let mut s2 = s1.clone(); - let disk = spec::Disk { - device_spec: StorageDevice::Virtio(VirtioDisk { - pci_path: PciPath::new(0, 4, 0).unwrap(), - backend_name: "backend".to_string(), - }), - backend_spec: crucible_backend(), - }; - - s1.disks.insert("disk".to_owned(), disk.clone()); - s2.disks.insert("disk".to_owned(), disk); - assert!(s1.is_migration_compatible(&s2).is_ok()); - } - - #[test] - fn disk_name_mismatch() { - let mut s1 = new_spec(); - let mut s2 = s1.clone(); - let d1 = spec::Disk { - device_spec: StorageDevice::Virtio(VirtioDisk { - pci_path: PciPath::new(0, 4, 0).unwrap(), - backend_name: "backend".to_owned(), - }), - backend_spec: crucible_backend(), - }; - - s1.disks.insert("disk1".to_owned(), d1.clone()); - s2.disks.insert("disk2".to_owned(), d1); - assert!(s1.is_migration_compatible(&s2).is_err()); - } - - #[test] - fn disk_length_mismatch() { - let mut s1 = new_spec(); - let mut s2 = s1.clone(); - let d1 = spec::Disk { - device_spec: StorageDevice::Virtio(VirtioDisk { - pci_path: PciPath::new(0, 4, 0).unwrap(), - backend_name: "backend".to_owned(), - }), - backend_spec: crucible_backend(), - }; - - s1.disks.insert("disk1".to_owned(), d1.clone()); - s2.disks.insert("disk1".to_owned(), d1.clone()); - s2.disks.insert("disk2".to_owned(), d1); - assert!(s1.is_migration_compatible(&s2).is_err()); - } - - #[test] - fn disk_interface_mismatch() { - let mut s1 = new_spec(); - let mut s2 = s1.clone(); - let d1 = spec::Disk { - device_spec: StorageDevice::Virtio(VirtioDisk { - pci_path: PciPath::new(0, 4, 0).unwrap(), - backend_name: "backend".to_owned(), - }), - backend_spec: crucible_backend(), - }; - - let mut d2 = d1.clone(); - d2.device_spec = StorageDevice::Nvme(NvmeDisk { - pci_path: PciPath::new(0, 4, 0).unwrap(), - backend_name: "backend".to_owned(), - }); - - s1.disks.insert("disk".to_owned(), d1); - s2.disks.insert("disk".to_owned(), d2); - assert!(s1.is_migration_compatible(&s2).is_err()); - } - - #[test] - fn disk_pci_mismatch() { - let mut s1 = new_spec(); - let mut s2 = s1.clone(); - let d1 = spec::Disk { - device_spec: StorageDevice::Virtio(VirtioDisk { - pci_path: PciPath::new(0, 4, 0).unwrap(), - backend_name: "backend".to_owned(), - }), - backend_spec: crucible_backend(), - }; - - let mut d2 = d1.clone(); - d2.device_spec = StorageDevice::Virtio(VirtioDisk { - pci_path: PciPath::new(0, 5, 0).unwrap(), - backend_name: "backend".to_owned(), - }); - - s1.disks.insert("disk".to_owned(), d1); - s2.disks.insert("disk".to_owned(), d2); - assert!(s1.is_migration_compatible(&s2).is_err()); - } - - #[test] - fn disk_backend_name_mismatch() { - let mut s1 = new_spec(); - let mut s2 = s1.clone(); - let d1 = spec::Disk { - device_spec: StorageDevice::Virtio(VirtioDisk { - pci_path: PciPath::new(0, 4, 0).unwrap(), - backend_name: "backend".to_owned(), - }), - backend_spec: crucible_backend(), - }; - - let mut d2 = d1.clone(); - d2.device_spec = StorageDevice::Virtio(VirtioDisk { - pci_path: PciPath::new(0, 4, 0).unwrap(), - backend_name: "other_backend".to_owned(), - }); - - s1.disks.insert("disk".to_owned(), d1); - s2.disks.insert("disk".to_owned(), d2); - assert!(s1.is_migration_compatible(&s2).is_err()); - } - - #[test] - fn disk_backend_kind_mismatch() { - let mut s1 = new_spec(); - let mut s2 = s1.clone(); - let d1 = spec::Disk { - device_spec: StorageDevice::Virtio(VirtioDisk { - pci_path: PciPath::new(0, 4, 0).unwrap(), - backend_name: "backend".to_owned(), - }), - backend_spec: file_backend(), - }; - - let mut d2 = d1.clone(); - d2.backend_spec = crucible_backend(); - s1.disks.insert("disk".to_owned(), d1); - s2.disks.insert("disk".to_owned(), d2); - assert!(s1.is_migration_compatible(&s2).is_err()); - } - - #[test] - fn disk_backend_readonly_mismatch() { - let mut s1 = new_spec(); - let mut s2 = s1.clone(); - let d1 = spec::Disk { - device_spec: StorageDevice::Virtio(VirtioDisk { - pci_path: PciPath::new(0, 4, 0).unwrap(), - backend_name: "backend".to_owned(), - }), - backend_spec: file_backend(), - }; - - let mut d2 = d1.clone(); - d2.backend_spec = spec::StorageBackend::File(FileStorageBackend { - path: "/tmp/file.raw".to_owned(), - readonly: true, - }); - - s1.disks.insert("disk".to_owned(), d1); - s2.disks.insert("disk".to_owned(), d2); - assert!(s1.is_migration_compatible(&s2).is_err()); - } - - #[test] - fn compatible_nics() { - let mut s1 = new_spec(); - let mut s2 = s1.clone(); - let nic = nic(); - s1.nics.insert("nic".to_owned(), nic.clone()); - s2.nics.insert("nic".to_owned(), nic); - assert!(s1.is_migration_compatible(&s2).is_ok()); - } - - #[test] - fn nic_pci_mismatch() { - let mut s1 = new_spec(); - let mut s2 = s1.clone(); - let n1 = nic(); - let mut n2 = n1.clone(); - n2.device_spec.pci_path = PciPath::new(0, 24, 0).unwrap(); - s1.nics.insert("nic".to_owned(), n1); - s2.nics.insert("nic".to_owned(), n2); - assert!(s1.is_migration_compatible(&s2).is_err()); - } - - #[test] - fn nic_backend_name_mismatch() { - let mut s1 = new_spec(); - let mut s2 = s1.clone(); - let n1 = nic(); - let mut n2 = n1.clone(); - "other_backend".clone_into(&mut n2.device_spec.backend_name); - s1.nics.insert("nic".to_owned(), n1); - s2.nics.insert("nic".to_owned(), n2); - assert!(s1.is_migration_compatible(&s2).is_err()); - } - - #[test] - fn compatible_serial_ports() { - let mut s1 = new_spec(); - let mut s2 = s1.clone(); - let serial = serial_port(); - s1.serial.insert("com1".to_owned(), serial.clone()); - s2.serial.insert("com1".to_owned(), serial); - assert!(s1.is_migration_compatible(&s2).is_ok()); - } - - #[test] - fn serial_port_number_mismatch() { - let mut s1 = new_spec(); - let mut s2 = s1.clone(); - let serial1 = serial_port(); - let mut serial2 = serial1.clone(); - serial2.num = SerialPortNumber::Com2; - s1.serial.insert("com1".to_owned(), serial1); - s2.serial.insert("com1".to_owned(), serial2); - assert!(s1.is_migration_compatible(&s2).is_err()); - } - - #[test] - fn compatible_bridges() { - let mut s1 = new_spec(); - let mut s2 = s1.clone(); - let bridge = bridge(); - s1.pci_pci_bridges.insert("bridge1".to_owned(), bridge); - s2.pci_pci_bridges.insert("bridge1".to_owned(), bridge); - assert!(s1.is_migration_compatible(&s2).is_ok()); - } - - #[test] - fn bridge_downstream_bus_mismatch() { - let mut s1 = new_spec(); - let mut s2 = s1.clone(); - let b1 = bridge(); - let mut b2 = b1; - b2.downstream_bus += 1; - s1.pci_pci_bridges.insert("bridge1".to_owned(), b1); - s2.pci_pci_bridges.insert("bridge1".to_owned(), b2); - assert!(s1.is_migration_compatible(&s2).is_err()); - } - - #[test] - fn bridge_pci_path_mismatch() { - let mut s1 = new_spec(); - let mut s2 = s1.clone(); - let b1 = bridge(); - let mut b2 = b1; - b2.pci_path = PciPath::new(0, 30, 0).unwrap(); - s1.pci_pci_bridges.insert("bridge1".to_owned(), b1); - s2.pci_pci_bridges.insert("bridge1".to_owned(), b2); - assert!(s1.is_migration_compatible(&s2).is_err()); - } - - #[test] - fn compatible_cpuid() { - let mut s1 = new_spec(); - let mut s2 = s1.clone(); - let mut set1 = CpuidSet::new(CpuidVendor::Intel); - let mut set2 = CpuidSet::new(CpuidVendor::Intel); - - s1.cpuid = Some(set1.clone()); - s2.cpuid = Some(set2.clone()); - s1.is_migration_compatible(&s2).unwrap(); - - set1.insert(CpuidIdent::leaf(0x1337), CpuidValues::default()).unwrap(); - set2.insert(CpuidIdent::leaf(0x1337), CpuidValues::default()).unwrap(); - - s1.cpuid = Some(set1.clone()); - s2.cpuid = Some(set2.clone()); - s1.is_migration_compatible(&s2).unwrap(); - - let values = CpuidValues { eax: 5, ebx: 6, ecx: 7, edx: 8 }; - set1.insert(CpuidIdent::subleaf(3, 4), values).unwrap(); - set2.insert(CpuidIdent::subleaf(3, 4), values).unwrap(); - s1.is_migration_compatible(&s2).unwrap(); - } - - #[test] - fn cpuid_explicitness_mismatch() { - let mut s1 = new_spec(); - let s2 = s1.clone(); - s1.cpuid = Some(CpuidSet::new(CpuidVendor::Intel)); - assert!(s1.is_migration_compatible(&s2).is_err()); - } - - #[test] - fn cpuid_vendor_mismatch() { - let mut s1 = new_spec(); - let mut s2 = s1.clone(); - s1.cpuid = Some(CpuidSet::new(CpuidVendor::Intel)); - s2.cpuid = Some(CpuidSet::new(CpuidVendor::Amd)); - assert!(s1.is_migration_compatible(&s2).is_err()); - } - - #[test] - fn cpuid_leaf_set_mismatch() { - let mut s1 = new_spec(); - let mut s2 = s1.clone(); - let mut set1 = CpuidSet::new(CpuidVendor::Amd); - let mut set2 = CpuidSet::new(CpuidVendor::Amd); - - // Give the first set an entry the second set doesn't have. - set1.insert(CpuidIdent::leaf(0), CpuidValues::default()).unwrap(); - set1.insert(CpuidIdent::leaf(1), CpuidValues::default()).unwrap(); - set2.insert(CpuidIdent::leaf(0), CpuidValues::default()).unwrap(); - - s1.cpuid = Some(set1); - s2.cpuid = Some(set2.clone()); - assert!(s1.is_migration_compatible(&s2).is_err()); - - // Make the sets have the same number of entries, but with a difference - // in which entries they have. - set2.insert(CpuidIdent::leaf(3), CpuidValues::default()).unwrap(); - s2.cpuid = Some(set2); - assert!(s1.is_migration_compatible(&s2).is_err()); - } - - #[test] - fn cpuid_leaf_value_mismatch() { - let mut s1 = new_spec(); - let mut s2 = s1.clone(); - let mut set1 = CpuidSet::new(CpuidVendor::Amd); - let mut set2 = CpuidSet::new(CpuidVendor::Amd); - - let v1 = CpuidValues { eax: 4, ebx: 5, ecx: 6, edx: 7 }; - let v2 = CpuidValues { eax: 100, ebx: 200, ecx: 300, edx: 400 }; - set1.insert(CpuidIdent::leaf(0), v1).unwrap(); - set2.insert(CpuidIdent::leaf(0), v2).unwrap(); - s1.cpuid = Some(set1); - s2.cpuid = Some(set2); - assert!(s1.is_migration_compatible(&s2).is_err()); - } - - #[test] - fn cpuid_leaf_subleaf_conflict() { - let mut s1 = new_spec(); - let mut s2 = s1.clone(); - let mut set1 = CpuidSet::new(CpuidVendor::Amd); - let mut set2 = CpuidSet::new(CpuidVendor::Amd); - - // Check that leaf 0 with no subleaf is not compatible with leaf 0 and a - // subleaf of 0. These are semantically different: the former matches - // leaf 0 with any subleaf value, while the latter technically matches - // only leaf 0 and subleaf 0 (with leaf-specific behavior if a different - // subleaf is specified). - set1.insert(CpuidIdent::leaf(0), CpuidValues::default()).unwrap(); - set2.insert(CpuidIdent::subleaf(0, 0), CpuidValues::default()).unwrap(); - s1.cpuid = Some(set1); - s2.cpuid = Some(set2); - assert!(s1.is_migration_compatible(&s2).is_err()); - } -} diff --git a/bin/propolis-server/src/lib/migrate/destination.rs b/bin/propolis-server/src/lib/migrate/destination.rs index eb5982f15..1a733a5d4 100644 --- a/bin/propolis-server/src/lib/migrate/destination.rs +++ b/bin/propolis-server/src/lib/migrate/destination.rs @@ -9,8 +9,9 @@ use propolis::migrate::{ MigrateCtx, MigrateStateError, Migrator, PayloadOffer, PayloadOffers, }; use propolis::vmm; -use propolis_api_types::InstanceMigrateInitiateRequest; +use propolis_api_types::{instance_spec::SpecKey, ReplacementComponent}; use slog::{error, info, trace, warn}; +use std::collections::HashMap; use std::convert::TryInto; use std::io; use std::net::SocketAddr; @@ -27,6 +28,7 @@ use crate::migrate::probes; use crate::migrate::{ Device, MigrateError, MigratePhase, MigrateRole, MigrationState, PageIter, }; +use crate::spec::Spec; use crate::vm::ensure::{VmEnsureActive, VmEnsureNotStarted}; use crate::vm::state_publisher::{ ExternalStateUpdate, MigrationStateUpdate, StatePublisher, @@ -35,6 +37,12 @@ use crate::vm::state_publisher::{ use super::protocol::Protocol; use super::MigrateConn; +pub(crate) struct MigrationTargetInfo { + pub migration_id: Uuid, + pub src_addr: SocketAddr, + pub replace_components: HashMap, +} + /// The interface to an arbitrary version of the target half of the live /// migration protocol. // @@ -57,7 +65,7 @@ pub(crate) trait DestinationProtocol { /// that the caller can use to run the migration. pub(crate) async fn initiate( log: &slog::Logger, - migrate_info: &InstanceMigrateInitiateRequest, + migrate_info: &MigrationTargetInfo, local_addr: SocketAddr, ) -> Result { let migration_id = migrate_info.migration_id; @@ -169,23 +177,26 @@ impl DestinationProtocol for RonV0 { info!(self.log(), "entering destination migration task"); let result = async { - // Run the sync phase to ensure that the source's instance spec is - // compatible with the spec supplied in the ensure parameters. - if let Err(e) = self.run_sync_phases(&mut ensure).await { - self.update_state( - ensure.state_publisher(), - MigrationState::Error, - ); - let e = ensure.fail(e.into()).await; - return Err(e - .downcast::() - .expect("original error was a MigrateError")); - } + let spec = match self.run_sync_phases(&mut ensure).await { + Ok(spec) => spec, + Err(e) => { + self.update_state( + ensure.state_publisher(), + MigrationState::Error, + ); + let e = ensure.fail(e.into()).await; + return Err(e + .downcast::() + .expect("original error was a MigrateError")); + } + }; // The sync phase succeeded, so it's OK to go ahead with creating // the objects in the target's instance spec. - let mut objects_created = - ensure.create_objects().await.map_err(|e| { + let mut objects_created = ensure + .create_objects_for_migration(spec) + .await + .map_err(|e| { MigrateError::TargetInstanceInitializationFailed( e.to_string(), ) @@ -260,14 +271,14 @@ impl RonV0 { async fn run_sync_phases( &mut self, ensure_ctx: &mut VmEnsureNotStarted<'_>, - ) -> Result<(), MigrateError> { + ) -> Result { let step = MigratePhase::MigrateSync; probes::migrate_phase_begin!(|| { step.to_string() }); - self.sync(ensure_ctx).await?; + let res = self.sync(ensure_ctx).await; probes::migrate_phase_end!(|| { step.to_string() }); - Ok(()) + res } async fn run_import_phases( @@ -330,7 +341,7 @@ impl RonV0 { async fn sync( &mut self, ensure_ctx: &mut VmEnsureNotStarted<'_>, - ) -> Result<(), MigrateError> { + ) -> Result { self.update_state(ensure_ctx.state_publisher(), MigrationState::Sync); let preamble: Preamble = match self.read_msg().await? { codec::Message::Serialized(s) => { @@ -346,18 +357,25 @@ impl RonV0 { }?; info!(self.log(), "Destination read Preamble: {:?}", preamble); - if let Err(e) = - preamble.check_compatibility(&ensure_ctx.instance_spec().clone()) - { - error!( - self.log(), - "source and destination instance specs incompatible"; - "error" => #%e - ); - return Err(MigrateError::InstanceSpecsIncompatible(e.to_string())); + match preamble.get_amended_spec( + &ensure_ctx + .migration_info() + .expect("migration destination implies target info") + .replace_components, + ) { + Ok(spec) => { + self.send_msg(codec::Message::Okay).await?; + Ok(spec) + } + Err(e) => { + error!( + self.log(), + "source and destination instance specs incompatible"; + "error" => #%e + ); + Err(MigrateError::InstanceSpecsIncompatible(e.to_string())) + } } - - self.send_msg(codec::Message::Okay).await } async fn ram_push( @@ -505,10 +523,14 @@ impl RonV0 { ); let target = vm_objects - .device_by_name(&device.instance_name) + .device_by_name(&SpecKey::from( + device.instance_name.clone(), + )) .ok_or_else(|| { - MigrateError::UnknownDevice(device.instance_name.clone()) - })?; + MigrateError::UnknownDevice( + device.instance_name.clone(), + ) + })?; self.import_device(&target, &device, &migrate_ctx)?; } } diff --git a/bin/propolis-server/src/lib/migrate/mod.rs b/bin/propolis-server/src/lib/migrate/mod.rs index 8a955958e..5ecc50330 100644 --- a/bin/propolis-server/src/lib/migrate/mod.rs +++ b/bin/propolis-server/src/lib/migrate/mod.rs @@ -12,7 +12,6 @@ use thiserror::Error; use tokio::io::{AsyncRead, AsyncWrite}; mod codec; -mod compat; pub mod destination; mod memx; mod preamble; diff --git a/bin/propolis-server/src/lib/migrate/preamble.rs b/bin/propolis-server/src/lib/migrate/preamble.rs index f80c4a767..8cdf30d9f 100644 --- a/bin/propolis-server/src/lib/migrate/preamble.rs +++ b/bin/propolis-server/src/lib/migrate/preamble.rs @@ -2,10 +2,15 @@ // 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/. -use propolis_api_types::instance_spec::VersionedInstanceSpec; +use std::collections::HashMap; + +use propolis_api_types::{ + instance_spec::{SpecKey, VersionedInstanceSpec}, + ReplacementComponent, +}; use serde::{Deserialize, Serialize}; -use crate::spec::{self, api_spec_v0::ApiSpecError}; +use crate::spec::{api_spec_v0::ApiSpecError, Spec}; use super::MigrateError; @@ -20,26 +25,31 @@ impl Preamble { Preamble { instance_spec, blobs: Vec::new() } } - /// Checks to see if the serialized spec in this preamble is compatible with - /// the supplied `other_spec`. - /// - /// This check runs on the destination. - pub fn check_compatibility( + /// Creates a migration target instance spec by taking the spec supplied in + /// this preamble and replacing any backend components found therein with + /// the backend components listed in `replace_components`. + pub fn get_amended_spec( self, - other_spec: &spec::Spec, - ) -> Result<(), MigrateError> { - let VersionedInstanceSpec::V0(v0_spec) = self.instance_spec; - let this_spec: spec::Spec = - v0_spec.try_into().map_err(|e: ApiSpecError| { - MigrateError::PreambleParse(e.to_string()) - })?; - - this_spec.is_migration_compatible(other_spec).map_err(|e| { - MigrateError::InstanceSpecsIncompatible(e.to_string()) - })?; - - // TODO: Compare opaque blobs. - - Ok(()) + replace_components: &HashMap, + ) -> Result { + let VersionedInstanceSpec::V0(mut v0_spec) = self.instance_spec; + for (id, component) in replace_components { + match v0_spec.components.get_mut(id) { + Some(ent) => { + *ent = component.clone().into(); + } + None => { + return Err(MigrateError::InstanceSpecsIncompatible( + format!( + "replacement component {id} not in source spec", + ), + )); + } + } + } + + Spec::try_from(v0_spec).map_err(|e: ApiSpecError| { + MigrateError::PreambleParse(e.to_string()) + }) } } diff --git a/bin/propolis-server/src/lib/server.rs b/bin/propolis-server/src/lib/server.rs index a5e669cb0..06292c69d 100644 --- a/bin/propolis-server/src/lib/server.rs +++ b/bin/propolis-server/src/lib/server.rs @@ -14,16 +14,13 @@ 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::migrate::destination::MigrationTargetInfo; +use crate::vm::ensure::VmInitializationMethod; use crate::{ serial::history_buffer::SerialHistoryOffset, - spec::{ - self, - api_spec_v0::ApiSpecError, - builder::{SpecBuilder, SpecBuilderError}, - Spec, - }, vm::{ensure::VmEnsureRequest, VmError}, vnc::{self, VncServer}, }; @@ -39,10 +36,8 @@ use internal_dns::ServiceName; pub use nexus_client::Client as NexusClient; use oximeter::types::ProducerRegistry; 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 propolis_api_types::instance_spec::SpecKey; +use propolis_api_types::InstanceInitializationMethod; use rfb::tungstenite::BinaryWs; use slog::{error, warn, Logger}; use tokio::sync::MutexGuard; @@ -70,8 +65,11 @@ 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 to supply to this server's guests. + pub bootrom_path: Arc, + + /// The bootrom version to display to the guest. + pub bootrom_version: Option, /// Whether to use the host's guest memory reservoir to back guest memory. pub use_reservoir: bool, @@ -92,7 +90,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 +99,8 @@ impl DropshotEndpointContext { let vnc_server = VncServer::new(log.clone()); Self { static_config: StaticConfig { - vm: Arc::new(config), + bootrom_path: Arc::new(bootrom_path), + bootrom_version, use_reservoir, metrics: metric_config, }, @@ -111,58 +111,6 @@ impl DropshotEndpointContext { } } -/// Creates an instance spec from an ensure request. (Both types are foreign to -/// 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)?; - } - - for disk in &request.disks { - spec_builder.add_disk_from_request(disk)?; - } - - if let Some(boot_settings) = request.boot_settings.as_ref() { - let order = boot_settings.order.clone(); - spec_builder.add_boot_order( - "boot-settings".to_string(), - order.into_iter().map(Into::into), - )?; - } - - if let Some(base64) = &request.cloud_init_bytes { - spec_builder.add_cloud_init_from_request(base64.clone())?; - } - - for (name, port) in [ - ("com1", instance_spec::components::devices::SerialPortNumber::Com1), - ("com2", instance_spec::components::devices::SerialPortNumber::Com2), - ("com3", instance_spec::components::devices::SerialPortNumber::Com3), - // SoftNpu uses this port for ASIC management. - #[cfg(not(feature = "falcon"))] - ("com4", instance_spec::components::devices::SerialPortNumber::Com4), - ] { - spec_builder.add_serial_port(name.to_owned(), port)?; - } - - #[cfg(feature = "falcon")] - spec_builder.set_softnpu_com4("com4".to_owned())?; - - spec_builder.add_pvpanic_device(spec::QemuPvpanic { - name: "pvpanic".to_string(), - spec: QemuPvpanic { enable_isa: true }, - })?; - - Ok(spec_builder.finish()) -} - /// Wrapper around a [`NexusClient`] object, which allows deferring /// the DNS lookup until accessed. /// @@ -239,13 +187,16 @@ async fn find_local_nexus_client( } } -async fn instance_ensure_common( +#[endpoint { + method = PUT, + path = "/instance" +}] +async fn instance_ensure( rqctx: RequestContext>, - properties: api::InstanceProperties, - migrate: Option, - instance_spec: Spec, + request: TypedBody, ) -> Result, HttpError> { let server_context = rqctx.context(); + let api::InstanceEnsureRequest { properties, init } = request.into_inner(); let oximeter_registry = server_context .static_config .metrics @@ -257,7 +208,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, @@ -266,7 +218,29 @@ async fn instance_ensure_common( local_server_addr: rqctx.server.local_addr, }; - let request = VmEnsureRequest { properties, migrate, instance_spec }; + let vm_init = match init { + InstanceInitializationMethod::Spec { spec } => spec + .try_into() + .map(VmInitializationMethod::Spec) + .map_err(|e| e.to_string()), + InstanceInitializationMethod::MigrationTarget { + migration_id, + src_addr, + replace_components, + } => Ok(VmInitializationMethod::Migration(MigrationTargetInfo { + migration_id, + src_addr, + replace_components, + })), + } + .map_err(|e| { + HttpError::for_bad_request( + None, + format!("failed to create instance spec: {e}"), + ) + })?; + + let request = VmEnsureRequest { properties, init: vm_init }; server_context .vm .ensure(&server_context.log, request, ensure_options) @@ -292,61 +266,6 @@ async fn instance_ensure_common( }) } -#[endpoint { - method = PUT, - path = "/instance", -}] -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 - ), - ) - })?; - - instance_ensure_common( - rqctx, - request.properties, - request.migrate, - instance_spec, - ) - .await -} - -#[endpoint { - method = PUT, - path = "/instance/spec", -}] -async fn instance_spec_ensure( - rqctx: RequestContext>, - request: TypedBody, -) -> Result, HttpError> { - let request = request.into_inner(); - let VersionedInstanceSpec::V0(v0_spec) = request.instance_spec; - let spec = Spec::try_from(v0_spec).map_err(|e: ApiSpecError| { - HttpError::for_bad_request( - None, - format!( - "failed to create internal instance spec from API spec: {:#?}", - e - ), - ) - })?; - - instance_ensure_common(rqctx, request.properties, request.migrate, spec) - .await -} - async fn instance_get_common( rqctx: &RequestContext>, ) -> Result { @@ -619,16 +538,16 @@ async fn instance_issue_crucible_snapshot_request( rqctx: RequestContext>, path_params: Path, ) -> Result, HttpError> { + let path_params = path_params.into_inner(); + let key = SpecKey::from(path_params.id); let vm = rqctx.context().vm.active_vm().await.ok_or_else(not_created_error)?; let objects = vm.objects().lock_shared().await; - let path_params = path_params.into_inner(); - let backend = - objects.crucible_backends().get(&path_params.id).ok_or_else(|| { - let s = format!("no disk with id {}!", path_params.id); - HttpError::for_not_found(Some(s.clone()), s) - })?; + let backend = objects.crucible_backends().get(&key).ok_or_else(|| { + let s = format!("no disk with id {}!", key); + HttpError::for_not_found(Some(s.clone()), s) + })?; backend.snapshot(path_params.snapshot_id).await.map_err(|e| { HttpError::for_bad_request(Some(e.to_string()), e.to_string()) })?; @@ -646,14 +565,14 @@ async fn disk_volume_status( path_params: Path, ) -> Result, HttpError> { let path_params = path_params.into_inner(); + let key = SpecKey::from(path_params.id); let vm = rqctx.context().vm.active_vm().await.ok_or_else(not_created_error)?; let objects = vm.objects().lock_shared().await; - let backend = - objects.crucible_backends().get(&path_params.id).ok_or_else(|| { - let s = format!("No crucible backend for id {}", path_params.id); - HttpError::for_not_found(Some(s.clone()), s) - })?; + let backend = objects.crucible_backends().get(&key).ok_or_else(|| { + let s = format!("No crucible backend for id {}", key); + HttpError::for_not_found(Some(s.clone()), s) + })?; Ok(HttpResponseOk(api::VolumeStatus { active: backend.volume_is_active().await.map_err(|e| { @@ -675,22 +594,25 @@ async fn instance_issue_crucible_vcr_request( let path_params = path_params.into_inner(); let request = request.into_inner(); let new_vcr_json = request.vcr_json; - let disk_name = request.name; let (tx, rx) = tokio::sync::oneshot::channel(); let vm = rqctx.context().vm.active_vm().await.ok_or_else(not_created_error)?; - vm.reconfigure_crucible_volume(disk_name, path_params.id, new_vcr_json, tx) - .map_err(|e| match e { - VmError::ForbiddenStateChange(reason) => HttpError::for_status( - Some(format!("instance state change not allowed: {}", reason)), - hyper::StatusCode::FORBIDDEN, - ), - _ => HttpError::for_internal_error(format!( - "unexpected error from VM controller: {e}" - )), - })?; + vm.reconfigure_crucible_volume( + SpecKey::from(path_params.id), + new_vcr_json, + tx, + ) + .map_err(|e| match e { + VmError::ForbiddenStateChange(reason) => HttpError::for_status( + Some(format!("instance state change not allowed: {}", reason)), + hyper::StatusCode::FORBIDDEN, + ), + _ => HttpError::for_internal_error(format!( + "unexpected error from VM controller: {e}" + )), + })?; let result = rx.await.map_err(|_| { HttpError::for_internal_error( @@ -720,7 +642,6 @@ async fn instance_issue_nmi( pub fn api() -> ApiDescription> { let mut api = ApiDescription::new(); api.register(instance_ensure).unwrap(); - api.register(instance_spec_ensure).unwrap(); api.register(instance_get).unwrap(); api.register(instance_spec_get).unwrap(); api.register(instance_state_monitor).unwrap(); diff --git a/bin/propolis-server/src/lib/spec/api_request.rs b/bin/propolis-server/src/lib/spec/api_request.rs deleted file mode 100644 index 3cc115976..000000000 --- a/bin/propolis-server/src/lib/spec/api_request.rs +++ /dev/null @@ -1,138 +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/. - -//! Converts device descriptions from an -//! [`propolis_api_types::InstanceEnsureRequest`] into elements that can be -//! added to a spec. - -use propolis_api_types::{ - instance_spec::{ - components::{ - backends::{ - BlobStorageBackend, CrucibleStorageBackend, - VirtioNetworkBackend, - }, - devices::{NvmeDisk, VirtioDisk, VirtioNic}, - }, - PciPath, - }, - DiskRequest, NetworkInterfaceRequest, Slot, -}; -use thiserror::Error; - -use super::{ - Disk, Nic, ParsedDiskRequest, ParsedNicRequest, StorageBackend, - StorageDevice, -}; - -#[derive(Debug, Error)] -pub(crate) enum DeviceRequestError { - #[error("invalid storage interface {0} for disk in slot {1}")] - InvalidStorageInterface(String, u8), - - #[error("invalid PCI slot {0} for device type {1:?}")] - PciSlotInvalid(u8, SlotType), - - #[error("error serializing {0}")] - SerializationError(String, #[source] serde_json::error::Error), -} - -/// A type of PCI device. Device numbers on the PCI bus are partitioned by slot -/// type. If a client asks to attach a device of type X to PCI slot Y, the -/// server will assign the Yth device number in X's partition. The partitioning -/// scheme is defined by the implementation of the `slot_to_pci_path` utility -/// function. -#[derive(Clone, Copy, Debug)] -pub(crate) enum SlotType { - Nic, - Disk, - CloudInit, -} - -/// Translates a device type and PCI slot (as presented in an instance creation -/// request) into a concrete PCI path. See the documentation for [`SlotType`]. -fn slot_to_pci_path( - slot: Slot, - ty: SlotType, -) -> Result { - match ty { - // Slots for NICS: 0x08 -> 0x0F - SlotType::Nic if slot.0 <= 7 => PciPath::new(0, slot.0 + 0x8, 0), - // Slots for Disks: 0x10 -> 0x17 - SlotType::Disk if slot.0 <= 7 => PciPath::new(0, slot.0 + 0x10, 0), - // Slot for CloudInit - SlotType::CloudInit if slot.0 == 0 => PciPath::new(0, slot.0 + 0x18, 0), - _ => return Err(DeviceRequestError::PciSlotInvalid(slot.0, ty)), - } - .map_err(|_| DeviceRequestError::PciSlotInvalid(slot.0, ty)) -} - -pub(super) fn parse_disk_from_request( - disk: &DiskRequest, -) -> Result { - let pci_path = slot_to_pci_path(disk.slot, SlotType::Disk)?; - let device_name = disk.name.clone(); - let backend_name = format!("{}-backend", disk.name); - let device_spec = match disk.device.as_ref() { - "virtio" => { - StorageDevice::Virtio(VirtioDisk { backend_name, pci_path }) - } - "nvme" => StorageDevice::Nvme(NvmeDisk { backend_name, pci_path }), - _ => { - return Err(DeviceRequestError::InvalidStorageInterface( - disk.device.clone(), - disk.slot.0, - )) - } - }; - - let backend_spec = StorageBackend::Crucible(CrucibleStorageBackend { - request_json: serde_json::to_string(&disk.volume_construction_request) - .map_err(|e| { - DeviceRequestError::SerializationError(disk.name.clone(), e) - })?, - readonly: disk.read_only, - }); - - Ok(ParsedDiskRequest { - name: device_name, - disk: Disk { device_spec, backend_spec }, - }) -} - -pub(super) fn parse_cloud_init_from_request( - base64: String, -) -> Result { - let name = "cloud-init"; - let pci_path = slot_to_pci_path(Slot(0), SlotType::CloudInit)?; - let backend_name = "cloud-init-backend".to_string(); - let backend_spec = - StorageBackend::Blob(BlobStorageBackend { base64, readonly: true }); - - let device_spec = - StorageDevice::Virtio(VirtioDisk { backend_name, pci_path }); - - Ok(ParsedDiskRequest { - name: name.to_owned(), - disk: Disk { device_spec, backend_spec }, - }) -} - -pub(super) fn parse_nic_from_request( - nic: &NetworkInterfaceRequest, -) -> Result { - let pci_path = slot_to_pci_path(nic.slot, SlotType::Nic)?; - let (device_name, backend_name) = super::pci_path_to_nic_names(pci_path); - let device_spec = VirtioNic { - backend_name: backend_name.clone(), - interface_id: nic.interface_id, - pci_path, - }; - - let backend_spec = VirtioNetworkBackend { vnic_name: nic.name.to_string() }; - Ok(ParsedNicRequest { - name: device_name, - nic: Nic { device_spec, backend_spec }, - }) -} 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 1bceaa049..c333d8e1c 100644 --- a/bin/propolis-server/src/lib/spec/api_spec_v0.rs +++ b/bin/propolis-server/src/lib/spec/api_spec_v0.rs @@ -14,6 +14,7 @@ use propolis_api_types::instance_spec::{ devices::{BootSettings, SerialPort as SerialPortDesc}, }, v0::{ComponentV0, InstanceSpecV0}, + SpecKey, }; use thiserror::Error; @@ -35,17 +36,17 @@ pub(crate) enum ApiSpecError { Builder(#[from] SpecBuilderError), #[error("storage backend {backend} not found for device {device}")] - StorageBackendNotFound { backend: String, device: String }, + StorageBackendNotFound { backend: SpecKey, device: SpecKey }, #[error("network backend {backend} not found for device {device}")] - NetworkBackendNotFound { backend: String, device: String }, + NetworkBackendNotFound { backend: SpecKey, device: SpecKey }, - #[cfg(not(feature = "falcon"))] - #[error("softnpu component {0} compiled out")] - SoftNpuCompiledOut(String), + #[allow(dead_code)] + #[error("component {0} disabled by compile-time options")] + FeatureCompiledOut(SpecKey), #[error("backend {0} not used by any device")] - BackendNotUsed(String), + BackendNotUsed(SpecKey), } impl From for InstanceSpecV0 { @@ -61,6 +62,10 @@ impl From for InstanceSpecV0 { serial, pci_pci_bridges, pvpanic, + + #[cfg(not(feature = "omicron-build"))] + migration_failure, + #[cfg(feature = "falcon")] softnpu, } = val; @@ -74,12 +79,12 @@ impl From for InstanceSpecV0 { #[track_caller] fn insert_component( spec: &mut InstanceSpecV0, - key: String, + key: SpecKey, val: ComponentV0, ) { assert!( !spec.components.contains_key(&key), - "component name {} already exists in output spec", + "component {} already exists in output spec", &key ); spec.components.insert(key, val); @@ -93,24 +98,23 @@ impl From for InstanceSpecV0 { }; let mut spec = InstanceSpecV0 { board, components: Default::default() }; - for (disk_name, disk) in disks { - let backend_name = disk.device_spec.backend_name().to_owned(); - insert_component(&mut spec, disk_name, disk.device_spec.into()); - - insert_component(&mut spec, backend_name, disk.backend_spec.into()); + for (disk_id, disk) in disks { + let backend_id = disk.device_spec.backend_id().to_owned(); + insert_component(&mut spec, disk_id, disk.device_spec.into()); + insert_component(&mut spec, backend_id, disk.backend_spec.into()); } - for (nic_name, nic) in nics { - let backend_name = nic.device_spec.backend_name.clone(); + for (nic_id, nic) in nics { + let backend_id = nic.device_spec.backend_id.clone(); insert_component( &mut spec, - nic_name, + nic_id, ComponentV0::VirtioNic(nic.device_spec), ); insert_component( &mut spec, - backend_name, + backend_id, ComponentV0::VirtioNetworkBackend(nic.backend_spec), ); } @@ -136,7 +140,7 @@ impl From for InstanceSpecV0 { if let Some(pvpanic) = pvpanic { insert_component( &mut spec, - pvpanic.name, + pvpanic.id, ComponentV0::QemuPvpanic(pvpanic.spec), ); } @@ -144,19 +148,31 @@ impl From for InstanceSpecV0 { if let Some(settings) = boot_settings { insert_component( &mut spec, - settings.name, + settings.component_id, ComponentV0::BootSettings(BootSettings { order: settings.order.into_iter().map(Into::into).collect(), }), ); } + #[cfg(not(feature = "omicron-build"))] + if let Some(migration_failure) = migration_failure { + insert_component( + &mut spec, + migration_failure.id, + ComponentV0::MigrationFailureInjector(migration_failure.spec), + ); + } + #[cfg(feature = "falcon")] { if let Some(softnpu_pci) = softnpu.pci_port { insert_component( &mut spec, - format!("softnpu-pci-{}", softnpu_pci.pci_path), + SpecKey::Name(format!( + "softnpu-pci-{}", + softnpu_pci.pci_path + )), ComponentV0::SoftNpuPciPort(softnpu_pci), ); } @@ -164,7 +180,7 @@ impl From for InstanceSpecV0 { if let Some(p9) = softnpu.p9_device { insert_component( &mut spec, - format!("softnpu-p9-{}", p9.pci_path), + SpecKey::Name(format!("softnpu-p9-{}", p9.pci_path)), ComponentV0::SoftNpuP9(p9), ); } @@ -172,24 +188,24 @@ impl From for InstanceSpecV0 { if let Some(p9fs) = softnpu.p9fs { insert_component( &mut spec, - format!("p9fs-{}", p9fs.pci_path), + SpecKey::Name(format!("p9fs-{}", p9fs.pci_path)), ComponentV0::P9fs(p9fs), ); } - for (port_name, port) in softnpu.ports { + for (port_id, port) in softnpu.ports { insert_component( &mut spec, - port_name.clone(), + port_id, ComponentV0::SoftNpuPort(SoftNpuPortSpec { - name: port_name, - backend_name: port.backend_name.clone(), + link_name: port.link_name, + backend_id: port.backend_id.clone(), }), ); insert_component( &mut spec, - port.backend_name, + port.backend_id, ComponentV0::DlpiNetworkBackend(port.backend_spec), ); } @@ -204,83 +220,83 @@ impl TryFrom for Spec { fn try_from(value: InstanceSpecV0) -> Result { let mut builder = SpecBuilder::with_instance_spec_board(value.board)?; - let mut devices: Vec<(String, ComponentV0)> = vec![]; + let mut devices: Vec<(SpecKey, ComponentV0)> = vec![]; let mut boot_settings = None; - let mut storage_backends: HashMap = + let mut storage_backends: HashMap = HashMap::new(); - let mut viona_backends: HashMap = + let mut viona_backends: HashMap = HashMap::new(); - let mut dlpi_backends: HashMap = + let mut dlpi_backends: HashMap = HashMap::new(); - for (name, component) in value.components.into_iter() { + for (id, component) in value.components.into_iter() { match component { ComponentV0::CrucibleStorageBackend(_) | ComponentV0::FileStorageBackend(_) | ComponentV0::BlobStorageBackend(_) => { storage_backends.insert( - name, + id, component.try_into().expect( "component is known to be a storage backend", ), ); } ComponentV0::VirtioNetworkBackend(viona) => { - viona_backends.insert(name, viona); + viona_backends.insert(id, viona); } ComponentV0::DlpiNetworkBackend(dlpi) => { - dlpi_backends.insert(name, dlpi); + dlpi_backends.insert(id, dlpi); } device => { - devices.push((name, device)); + devices.push((id, device)); } } } - for (device_name, device_spec) in devices { + for (device_id, device_spec) in devices { match device_spec { ComponentV0::VirtioDisk(_) | ComponentV0::NvmeDisk(_) => { let device_spec = StorageDevice::try_from(device_spec) .expect("component is known to be a disk"); let (_, backend_spec) = storage_backends - .remove_entry(device_spec.backend_name()) + .remove_entry(device_spec.backend_id()) .ok_or_else(|| { ApiSpecError::StorageBackendNotFound { - backend: device_spec.backend_name().to_owned(), - device: device_name.clone(), + backend: device_spec.backend_id().to_owned(), + device: device_id.clone(), } })?; builder.add_storage_device( - device_name, + device_id, Disk { device_spec, backend_spec }, )?; } ComponentV0::VirtioNic(nic) => { let (_, backend_spec) = viona_backends - .remove_entry(&nic.backend_name) + .remove_entry(&nic.backend_id) .ok_or_else(|| { ApiSpecError::NetworkBackendNotFound { - backend: nic.backend_name.clone(), - device: device_name.clone(), + backend: nic.backend_id.clone(), + device: device_id.clone(), } })?; builder.add_network_device( - device_name, + device_id, Nic { device_spec: nic, backend_spec }, )?; } ComponentV0::SerialPort(port) => { - builder.add_serial_port(device_name, port.num)?; + builder.add_serial_port(device_id, port.num)?; } ComponentV0::PciPciBridge(bridge) => { - builder.add_pci_bridge(device_name, bridge)?; + builder.add_pci_bridge(device_id, bridge)?; } ComponentV0::QemuPvpanic(pvpanic) => { builder.add_pvpanic_device(QemuPvpanic { - name: device_name, + id: device_id, spec: pvpanic, })?; } @@ -290,14 +306,14 @@ impl TryFrom for Spec { // Since there may be more disk devices left in the // component map, just capture the boot order for now and // apply it to the builder later. - boot_settings = Some((device_name, settings)); + boot_settings = Some((device_id, settings)); } #[cfg(not(feature = "falcon"))] ComponentV0::SoftNpuPciPort(_) | ComponentV0::SoftNpuPort(_) | ComponentV0::SoftNpuP9(_) | ComponentV0::P9fs(_) => { - return Err(ApiSpecError::SoftNpuCompiledOut(device_name)); + return Err(ApiSpecError::FeatureCompiledOut(device_id)); } #[cfg(feature = "falcon")] ComponentV0::SoftNpuPciPort(port) => { @@ -306,20 +322,21 @@ impl TryFrom for Spec { #[cfg(feature = "falcon")] ComponentV0::SoftNpuPort(port) => { let (_, backend_spec) = dlpi_backends - .remove_entry(&port.backend_name) + .remove_entry(&port.backend_id) .ok_or_else(|| { ApiSpecError::NetworkBackendNotFound { - backend: port.backend_name.clone(), - device: device_name.clone(), + backend: port.backend_id.clone(), + device: device_id.clone(), } })?; let port = SoftNpuPort { - backend_name: port.backend_name, + link_name: port.link_name, + backend_id: port.backend_id, backend_spec, }; - builder.add_softnpu_port(device_name, port)?; + builder.add_softnpu_port(device_id, port)?; } #[cfg(feature = "falcon")] ComponentV0::SoftNpuP9(p9) => { @@ -329,6 +346,22 @@ impl TryFrom for Spec { ComponentV0::P9fs(p9fs) => { builder.set_p9fs(p9fs)?; } + + #[cfg(not(feature = "omicron-build"))] + ComponentV0::MigrationFailureInjector(injector) => { + builder.add_migration_failure_device( + super::MigrationFailure { + id: device_id, + spec: injector, + }, + )?; + } + + #[cfg(feature = "omicron-build")] + ComponentV0::MigrationFailureInjector(_) => { + return Err(ApiSpecError::FeatureCompiledOut(device_id)); + } + ComponentV0::CrucibleStorageBackend(_) | ComponentV0::FileStorageBackend(_) | ComponentV0::BlobStorageBackend(_) diff --git a/bin/propolis-server/src/lib/spec/builder.rs b/bin/propolis-server/src/lib/spec/builder.rs index 1a5dee420..64bc1ff6f 100644 --- a/bin/propolis-server/src/lib/spec/builder.rs +++ b/bin/propolis-server/src/lib/spec/builder.rs @@ -7,48 +7,40 @@ use std::collections::{BTreeSet, HashSet}; use cpuid_utils::CpuidMapConversionError; -use propolis_api_types::{ - instance_spec::{ - components::{ - board::{Board as InstanceSpecBoard, Chipset, I440Fx}, - devices::{PciPciBridge, SerialPortNumber}, - }, - PciPath, +use propolis_api_types::instance_spec::{ + components::{ + board::Board as InstanceSpecBoard, + devices::{PciPciBridge, SerialPortNumber}, }, - DiskRequest, NetworkInterfaceRequest, + PciPath, SpecKey, }; use thiserror::Error; +#[cfg(not(feature = "omicron-build"))] +use super::MigrationFailure; + #[cfg(feature = "falcon")] 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, }; #[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), - - #[error("device {0} has the same name as its backend")] - DeviceAndBackendNamesIdentical(String), + #[error("device {0} has the same ID as its backend")] + DeviceAndBackendNamesIdentical(SpecKey), - #[error("a component with name {0} already exists")] - ComponentNameInUse(String), + #[error("a component with ID {0} already exists")] + ComponentIdInUse(SpecKey), #[error("a PCI device is already attached at {0:?}")] PciPathInUse(PciPath), @@ -62,8 +54,12 @@ pub(crate) enum SpecBuilderError { #[error("boot settings were already specified")] BootSettingsInUse, + #[cfg(not(feature = "omicron-build"))] + #[error("migration failure injection settings were already specified")] + MigrationFailureInjectionInUse, + #[error("boot option {0} is not an attached device")] - BootOptionMissing(String), + BootOptionMissing(SpecKey), #[error("instance spec's CPUID entries are invalid")] CpuidEntriesInvalid(#[from] cpuid_utils::CpuidMapConversionError), @@ -74,23 +70,10 @@ pub(crate) struct SpecBuilder { spec: super::Spec, pci_paths: BTreeSet, serial_ports: HashSet, - component_names: BTreeSet, + component_ids: BTreeSet, } impl SpecBuilder { - pub fn new(cpus: u8, memory_mb: u64) -> Self { - let board = Board { - cpus, - memory_mb, - chipset: Chipset::I440Fx(I440Fx { enable_pcie: false }), - }; - - Self { - spec: super::Spec { board, ..Default::default() }, - ..Default::default() - } - } - pub(super) fn with_instance_spec_board( board: InstanceSpecBoard, ) -> Result { @@ -118,28 +101,6 @@ impl SpecBuilder { }) } - /// Converts an HTTP API request to add a NIC to an instance into - /// device/backend entries in the spec under construction. - pub fn add_nic_from_request( - &mut self, - nic: &NetworkInterfaceRequest, - ) -> Result<(), SpecBuilderError> { - let parsed = api_request::parse_nic_from_request(nic)?; - self.add_network_device(parsed.name, parsed.nic)?; - Ok(()) - } - - /// Converts an HTTP API request to add a disk to an instance into - /// device/backend entries in the spec under construction. - pub fn add_disk_from_request( - &mut self, - disk: &DiskRequest, - ) -> Result<(), SpecBuilderError> { - let parsed = api_request::parse_disk_from_request(disk)?; - self.add_storage_device(parsed.name, parsed.disk)?; - Ok(()) - } - /// Sets the spec's boot order to the list of disk devices specified in /// `boot_options`. /// @@ -147,11 +108,11 @@ impl SpecBuilder { /// in the spec's disk map. pub fn add_boot_order( &mut self, - component_name: String, + component_id: SpecKey, boot_options: impl Iterator, ) -> Result<(), SpecBuilderError> { - if self.component_names.contains(&component_name) { - return Err(SpecBuilderError::ComponentNameInUse(component_name)); + if self.component_ids.contains(&component_id) { + return Err(SpecBuilderError::ComponentIdInUse(component_id)); } if self.spec.boot_settings.is_some() { @@ -160,81 +121,18 @@ impl SpecBuilder { let mut order = vec![]; for item in boot_options { - if !self.spec.disks.contains_key(item.name.as_str()) { + if !self.spec.disks.contains_key(&item.component_id) { return Err(SpecBuilderError::BootOptionMissing( - item.name.clone(), + item.component_id, )); } - order.push(crate::spec::BootOrderEntry { name: item.name.clone() }); - } - - self.spec.boot_settings = - Some(BootSettings { name: component_name, order }); - Ok(()) - } - - /// Converts an HTTP API request to add a cloud-init disk to an instance - /// into device/backend entries in the spec under construction. - pub fn add_cloud_init_from_request( - &mut self, - base64: String, - ) -> Result<(), SpecBuilderError> { - let parsed = api_request::parse_cloud_init_from_request(base64)?; - self.add_storage_device(parsed.name, parsed.disk)?; - 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)?; + order.push(crate::spec::BootOrderEntry { + component_id: item.component_id, + }); } + self.spec.boot_settings = Some(BootSettings { component_id, order }); Ok(()) } @@ -255,29 +153,29 @@ impl SpecBuilder { /// Adds a storage device with an associated backend. pub(super) fn add_storage_device( &mut self, - disk_name: String, + disk_id: SpecKey, disk: Disk, ) -> Result<&Self, SpecBuilderError> { - if disk_name == disk.device_spec.backend_name() { + if disk_id == *disk.device_spec.backend_id() { return Err(SpecBuilderError::DeviceAndBackendNamesIdentical( - disk_name, + disk_id, )); } - if self.component_names.contains(&disk_name) { - return Err(SpecBuilderError::ComponentNameInUse(disk_name)); + if self.component_ids.contains(&disk_id) { + return Err(SpecBuilderError::ComponentIdInUse(disk_id)); } - if self.component_names.contains(disk.device_spec.backend_name()) { - return Err(SpecBuilderError::ComponentNameInUse( - disk.device_spec.backend_name().to_owned(), + if self.component_ids.contains(disk.device_spec.backend_id()) { + return Err(SpecBuilderError::ComponentIdInUse( + disk.device_spec.backend_id().to_owned(), )); } self.register_pci_device(disk.device_spec.pci_path())?; - self.component_names.insert(disk_name.clone()); - self.component_names.insert(disk.device_spec.backend_name().to_owned()); - let _old = self.spec.disks.insert(disk_name, disk); + self.component_ids.insert(disk_id.clone()); + self.component_ids.insert(disk.device_spec.backend_id().to_owned()); + let _old = self.spec.disks.insert(disk_id, disk); assert!(_old.is_none()); Ok(self) } @@ -285,29 +183,29 @@ impl SpecBuilder { /// Adds a network device with an associated backend. pub(super) fn add_network_device( &mut self, - nic_name: String, + nic_id: SpecKey, nic: Nic, ) -> Result<&Self, SpecBuilderError> { - if nic_name == nic.device_spec.backend_name { + if nic_id == nic.device_spec.backend_id { return Err(SpecBuilderError::DeviceAndBackendNamesIdentical( - nic_name, + nic_id, )); } - if self.component_names.contains(&nic_name) { - return Err(SpecBuilderError::ComponentNameInUse(nic_name)); + if self.component_ids.contains(&nic_id) { + return Err(SpecBuilderError::ComponentIdInUse(nic_id)); } - if self.component_names.contains(&nic.device_spec.backend_name) { - return Err(SpecBuilderError::ComponentNameInUse( - nic.device_spec.backend_name, + if self.component_ids.contains(&nic.device_spec.backend_id) { + return Err(SpecBuilderError::ComponentIdInUse( + nic.device_spec.backend_id, )); } self.register_pci_device(nic.device_spec.pci_path)?; - self.component_names.insert(nic_name.clone()); - self.component_names.insert(nic.device_spec.backend_name.clone()); - let _old = self.spec.nics.insert(nic_name, nic); + self.component_ids.insert(nic_id.clone()); + self.component_ids.insert(nic.device_spec.backend_id.clone()); + let _old = self.spec.nics.insert(nic_id, nic); assert!(_old.is_none()); Ok(self) } @@ -315,16 +213,16 @@ impl SpecBuilder { /// Adds a PCI-PCI bridge. pub fn add_pci_bridge( &mut self, - name: String, + id: SpecKey, bridge: PciPciBridge, ) -> Result<&Self, SpecBuilderError> { - if self.component_names.contains(&name) { - return Err(SpecBuilderError::ComponentNameInUse(name)); + if self.component_ids.contains(&id) { + return Err(SpecBuilderError::ComponentIdInUse(id)); } self.register_pci_device(bridge.pci_path)?; - self.component_names.insert(name.clone()); - let _old = self.spec.pci_pci_bridges.insert(name, bridge); + self.component_ids.insert(id.clone()); + let _old = self.spec.pci_pci_bridges.insert(id, bridge); assert!(_old.is_none()); Ok(self) } @@ -332,11 +230,11 @@ impl SpecBuilder { /// Adds a serial port. pub fn add_serial_port( &mut self, - name: String, + id: SpecKey, num: SerialPortNumber, ) -> Result<&Self, SpecBuilderError> { - if self.component_names.contains(&name) { - return Err(SpecBuilderError::ComponentNameInUse(name)); + if self.component_ids.contains(&id) { + return Err(SpecBuilderError::ComponentIdInUse(id)); } if self.serial_ports.contains(&num) { @@ -344,8 +242,8 @@ impl SpecBuilder { } let desc = SerialPort { num, device: SerialPortDevice::Uart }; - self.spec.serial.insert(name.clone(), desc); - self.component_names.insert(name); + self.spec.serial.insert(id.clone(), desc); + self.component_ids.insert(id); self.serial_ports.insert(num); Ok(self) } @@ -354,37 +252,34 @@ impl SpecBuilder { &mut self, pvpanic: QemuPvpanic, ) -> Result<&Self, SpecBuilderError> { - if self.component_names.contains(&pvpanic.name) { - return Err(SpecBuilderError::ComponentNameInUse(pvpanic.name)); + if self.component_ids.contains(&pvpanic.id) { + return Err(SpecBuilderError::ComponentIdInUse(pvpanic.id)); } if self.spec.pvpanic.is_some() { return Err(SpecBuilderError::PvpanicInUse); } - self.component_names.insert(pvpanic.name.clone()); + self.component_ids.insert(pvpanic.id.clone()); self.spec.pvpanic = Some(pvpanic); Ok(self) } - #[cfg(feature = "falcon")] - pub fn set_softnpu_com4( + #[cfg(not(feature = "omicron-build"))] + pub fn add_migration_failure_device( &mut self, - name: String, + device: MigrationFailure, ) -> Result<&Self, SpecBuilderError> { - if self.component_names.contains(&name) { - return Err(SpecBuilderError::ComponentNameInUse(name)); + if self.component_ids.contains(&device.id) { + return Err(SpecBuilderError::ComponentIdInUse(device.id)); } - let num = SerialPortNumber::Com4; - if self.serial_ports.contains(&num) { - return Err(SpecBuilderError::SerialPortInUse(num)); + if self.spec.migration_failure.is_some() { + return Err(SpecBuilderError::MigrationFailureInjectionInUse); } - let desc = SerialPort { num, device: SerialPortDevice::SoftNpu }; - self.spec.serial.insert(name.clone(), desc); - self.component_names.insert(name); - self.serial_ports.insert(num); + self.component_ids.insert(device.id.clone()); + self.spec.migration_failure = Some(device); Ok(self) } @@ -393,8 +288,22 @@ impl SpecBuilder { &mut self, pci_port: SoftNpuPciPort, ) -> Result<&Self, SpecBuilderError> { + // SoftNPU squats on COM4. + let id = SpecKey::Name("com4".to_string()); + let num = SerialPortNumber::Com4; + if self.component_ids.contains(&id) { + return Err(SpecBuilderError::ComponentIdInUse(id)); + } + + if self.serial_ports.contains(&num) { + return Err(SpecBuilderError::SerialPortInUse(num)); + } + self.register_pci_device(pci_port.pci_path)?; self.spec.softnpu.pci_port = Some(pci_port); + self.spec + .serial + .insert(id, SerialPort { num, device: SerialPortDevice::SoftNpu }); Ok(self) } @@ -418,26 +327,24 @@ impl SpecBuilder { #[cfg(feature = "falcon")] pub fn add_softnpu_port( &mut self, - port_name: String, + port_id: SpecKey, port: SoftNpuPort, ) -> Result<&Self, SpecBuilderError> { - if port_name == port.backend_name { + if port_id == port.backend_id { return Err(SpecBuilderError::DeviceAndBackendNamesIdentical( - port_name, + port_id, )); } - if self.component_names.contains(&port_name) { - return Err(SpecBuilderError::ComponentNameInUse(port_name)); + if self.component_ids.contains(&port_id) { + return Err(SpecBuilderError::ComponentIdInUse(port_id)); } - if self.component_names.contains(&port.backend_name) { - return Err(SpecBuilderError::ComponentNameInUse( - port.backend_name, - )); + if self.component_ids.contains(&port.backend_id) { + return Err(SpecBuilderError::ComponentIdInUse(port.backend_id)); } - let _old = self.spec.softnpu.ports.insert(port_name, port); + let _old = self.spec.softnpu.ports.insert(port_id, port); assert!(_old.is_none()); Ok(self) } @@ -447,130 +354,3 @@ impl SpecBuilder { self.spec } } - -#[cfg(test)] -mod test { - use propolis_api_types::{ - instance_spec::components::{ - backends::{BlobStorageBackend, VirtioNetworkBackend}, - devices::{VirtioDisk, VirtioNic}, - }, - Slot, VolumeConstructionRequest, - }; - use uuid::Uuid; - - use crate::spec::{StorageBackend, StorageDevice}; - - use super::*; - - fn test_builder() -> SpecBuilder { - SpecBuilder::new(4, 512) - } - - #[test] - fn duplicate_pci_slot() { - let mut builder = test_builder(); - // Adding the same disk device twice should fail. - assert!(builder - .add_disk_from_request(&DiskRequest { - name: "disk1".to_string(), - slot: Slot(0), - read_only: true, - device: "nvme".to_string(), - volume_construction_request: VolumeConstructionRequest::File { - id: Uuid::new_v4(), - block_size: 512, - path: "disk1.img".to_string() - }, - }) - .is_ok()); - - assert!(builder - .add_disk_from_request(&DiskRequest { - name: "disk2".to_string(), - slot: Slot(0), - read_only: true, - device: "virtio".to_string(), - volume_construction_request: VolumeConstructionRequest::File { - id: Uuid::new_v4(), - block_size: 512, - path: "disk2.img".to_string() - }, - }) - .is_err()); - } - - #[test] - fn duplicate_serial_port() { - let mut builder = test_builder(); - assert!(builder - .add_serial_port("com1".to_owned(), SerialPortNumber::Com1) - .is_ok()); - assert!(builder - .add_serial_port("com2".to_owned(), SerialPortNumber::Com2) - .is_ok()); - assert!(builder - .add_serial_port("com3".to_owned(), SerialPortNumber::Com3) - .is_ok()); - assert!(builder - .add_serial_port("com4".to_owned(), SerialPortNumber::Com4) - .is_ok()); - assert!(builder - .add_serial_port("com1".to_owned(), SerialPortNumber::Com1) - .is_err()); - } - - #[test] - fn unknown_storage_device_type() { - let mut builder = test_builder(); - assert!(builder - .add_disk_from_request(&DiskRequest { - name: "disk3".to_string(), - slot: Slot(0), - read_only: true, - device: "virtio-scsi".to_string(), - volume_construction_request: VolumeConstructionRequest::File { - id: Uuid::new_v4(), - block_size: 512, - path: "disk3.img".to_string() - }, - }) - .is_err()); - } - - #[test] - fn device_with_same_name_as_backend() { - let mut builder = test_builder(); - assert!(builder - .add_storage_device( - "storage".to_owned(), - Disk { - device_spec: StorageDevice::Virtio(VirtioDisk { - backend_name: "storage".to_owned(), - pci_path: PciPath::new(0, 4, 0).unwrap() - }), - backend_spec: StorageBackend::Blob(BlobStorageBackend { - base64: "".to_string(), - readonly: false - }) - } - ) - .is_err()); - - assert!(builder - .add_network_device( - "network".to_owned(), - Nic { - device_spec: VirtioNic { - backend_name: "network".to_owned(), - interface_id: Uuid::nil(), - pci_path: PciPath::new(0, 5, 0).unwrap() - }, - backend_spec: VirtioNetworkBackend { - vnic_name: "vnic0".to_owned() - } - } - ) - .is_err()); - } -} 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 414453553..618823ea7 100644 --- a/bin/propolis-server/src/lib/spec/mod.rs +++ b/bin/propolis-server/src/lib/spec/mod.rs @@ -25,25 +25,26 @@ use propolis_api_types::instance_spec::{ }, board::{Chipset, I440Fx}, devices::{ - NvmeDisk, PciPciBridge, QemuPvpanic as QemuPvpanicDesc, + NvmeDisk, PciPciBridge, QemuPvpanic as QemuPvpanicSpec, SerialPortNumber, VirtioDisk, VirtioNic, }, }, v0::ComponentV0, - PciPath, + PciPath, SpecKey, }; use thiserror::Error; +#[cfg(not(feature = "omicron-build"))] +use propolis_api_types::instance_spec::components::devices::MigrationFailureInjector; + #[cfg(feature = "falcon")] use propolis_api_types::instance_spec::components::{ backends::DlpiNetworkBackend, devices::{P9fs, SoftNpuP9, SoftNpuPciPort}, }; -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")] @@ -62,15 +63,18 @@ pub struct ComponentTypeMismatch; pub(crate) struct Spec { pub board: Board, pub cpuid: Option, - pub disks: HashMap, - pub nics: HashMap, + pub disks: HashMap, + pub nics: HashMap, pub boot_settings: Option, - pub serial: HashMap, + pub serial: HashMap, - pub pci_pci_bridges: HashMap, + pub pci_pci_bridges: HashMap, pub pvpanic: Option, + #[cfg(not(feature = "omicron-build"))] + pub migration_failure: Option, + #[cfg(feature = "falcon")] pub softnpu: SoftNpu, } @@ -101,13 +105,13 @@ impl Default for Board { #[derive(Clone, Debug)] pub(crate) struct BootSettings { - pub name: String, + pub component_id: SpecKey, pub order: Vec, } -#[derive(Clone, Debug, Default)] +#[derive(Clone, Debug)] pub(crate) struct BootOrderEntry { - pub name: String, + pub component_id: SpecKey, } impl @@ -117,7 +121,7 @@ impl fn from( value: propolis_api_types::instance_spec::components::devices::BootOrderEntry, ) -> Self { - Self { name: value.name.clone() } + Self { component_id: value.component_id } } } @@ -125,7 +129,7 @@ impl From for propolis_api_types::instance_spec::components::devices::BootOrderEntry { fn from(value: BootOrderEntry) -> Self { - Self { name: value.name } + Self { component_id: value.component_id } } } @@ -151,10 +155,10 @@ impl StorageDevice { } } - pub fn backend_name(&self) -> &str { + pub fn backend_id(&self) -> &SpecKey { match self { - StorageDevice::Virtio(disk) => &disk.backend_name, - StorageDevice::Nvme(disk) => &disk.backend_name, + StorageDevice::Virtio(disk) => &disk.backend_id, + StorageDevice::Nvme(disk) => &disk.backend_id, } } } @@ -273,15 +277,22 @@ pub struct SerialPort { #[derive(Clone, Debug)] pub struct QemuPvpanic { - #[allow(dead_code)] - pub name: String, - pub spec: QemuPvpanicDesc, + pub id: SpecKey, + pub spec: QemuPvpanicSpec, +} + +#[cfg(not(feature = "omicron-build"))] +#[derive(Clone, Debug)] +pub struct MigrationFailure { + pub id: SpecKey, + pub spec: MigrationFailureInjector, } #[cfg(feature = "falcon")] #[derive(Clone, Debug)] pub struct SoftNpuPort { - pub backend_name: String, + pub link_name: String, + pub backend_id: SpecKey, pub backend_spec: DlpiNetworkBackend, } @@ -289,62 +300,7 @@ pub struct SoftNpuPort { #[derive(Clone, Debug, Default)] pub struct SoftNpu { pub pci_port: Option, - pub ports: HashMap, + pub ports: HashMap, pub p9_device: Option, pub p9fs: Option, } - -#[cfg(feature = "falcon")] -impl SoftNpu { - /// Returns `true` if this struct specifies at least one SoftNPU component. - pub fn has_components(&self) -> bool { - self.pci_port.is_some() - || self.p9_device.is_some() - || self.p9fs.is_some() - || !self.ports.is_empty() - } -} - -struct ParsedDiskRequest { - name: String, - disk: Disk, -} - -struct ParsedNicRequest { - name: String, - 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, -/// and that can change between incarnations of an instance. The PCI path is -/// unique to each NIC but must remain stable over a migration, so it's suitable -/// for use in this naming scheme. -/// -/// N.B. Migrating a NIC requires the source and target to agree on these names, -/// so changing this routine's behavior will prevent Propolis processes -/// with the old behavior from migrating processes with the new behavior. -fn pci_path_to_nic_names(path: PciPath) -> (String, String) { - (format!("vnic-{}", path), format!("vnic-{}-backend", path)) -} diff --git a/bin/propolis-server/src/lib/vm/active.rs b/bin/propolis-server/src/lib/vm/active.rs index 1274782f9..ef5c191b5 100644 --- a/bin/propolis-server/src/lib/vm/active.rs +++ b/bin/propolis-server/src/lib/vm/active.rs @@ -6,7 +6,9 @@ use std::sync::Arc; -use propolis_api_types::{InstanceProperties, InstanceStateRequested}; +use propolis_api_types::{ + instance_spec::SpecKey, InstanceProperties, InstanceStateRequested, +}; use slog::info; use uuid::Uuid; @@ -99,16 +101,14 @@ impl ActiveVm { /// replacement result after it completes this operation. pub(crate) fn reconfigure_crucible_volume( &self, - disk_name: String, - backend_id: Uuid, + disk_id: SpecKey, new_vcr_json: String, result_tx: CrucibleReplaceResultTx, ) -> Result<(), VmError> { self.state_driver_queue .queue_external_request( ExternalRequest::ReconfigureCrucibleVolume { - disk_name, - backend_id, + disk_id, new_vcr_json, result_tx, }, diff --git a/bin/propolis-server/src/lib/vm/ensure.rs b/bin/propolis-server/src/lib/vm/ensure.rs index ff4fec107..c4b6ae77c 100644 --- a/bin/propolis-server/src/lib/vm/ensure.rs +++ b/bin/propolis-server/src/lib/vm/ensure.rs @@ -30,8 +30,8 @@ use std::sync::Arc; use oximeter::types::ProducerRegistry; use oximeter_instruments::kstat::KstatSampler; use propolis_api_types::{ - InstanceEnsureResponse, InstanceMigrateInitiateRequest, - InstanceMigrateInitiateResponse, InstanceProperties, InstanceState, + InstanceEnsureResponse, InstanceMigrateInitiateResponse, + InstanceProperties, InstanceState, }; use slog::{debug, info}; @@ -39,9 +39,13 @@ use crate::{ initializer::{ build_instance, MachineInitializer, MachineInitializerState, }, + migrate::destination::MigrationTargetInfo, spec::Spec, stats::{create_kstat_sampler, VirtualMachine}, - vm::request_queue::InstanceAutoStart, + vm::{ + request_queue::InstanceAutoStart, VMM_BASE_RT_THREADS, + VMM_MIN_RT_THREADS, + }, }; use super::{ @@ -52,10 +56,39 @@ use super::{ EnsureOptions, InstanceEnsureResponseTx, VmError, }; +pub(crate) enum VmInitializationMethod { + Spec(Spec), + Migration(MigrationTargetInfo), +} + pub(crate) struct VmEnsureRequest { pub(crate) properties: InstanceProperties, - pub(crate) migrate: Option, - pub(crate) instance_spec: Spec, + pub(crate) init: VmInitializationMethod, +} + +impl VmEnsureRequest { + /// Returns `true` if this is a request to initialize via live migration. + pub(crate) fn is_migration(&self) -> bool { + matches!(self.init, VmInitializationMethod::Migration(_)) + } + + /// Returns the migration target information if this is a request to + /// initialize via live migration and `None` otherwise. + pub(crate) fn migration_info(&self) -> Option<&MigrationTargetInfo> { + match &self.init { + VmInitializationMethod::Migration(info) => Some(info), + VmInitializationMethod::Spec(_) => None, + } + } + + /// Returns the instance spec to use to initialize this VM if this is a + /// request to initialize a VM from scratch; returns `None` otherwise. + pub(crate) fn spec(&self) -> Option<&Spec> { + match &self.init { + VmInitializationMethod::Migration(_) => None, + VmInitializationMethod::Spec(spec) => Some(spec), + } + } } /// Holds state about an instance ensure request that has not yet produced any @@ -64,7 +97,7 @@ pub(crate) struct VmEnsureNotStarted<'a> { log: &'a slog::Logger, vm: &'a Arc, ensure_request: &'a VmEnsureRequest, - ensure_options: &'a EnsureOptions, + ensure_options: &'a Arc, ensure_response_tx: InstanceEnsureResponseTx, state_publisher: &'a mut StatePublisher, } @@ -74,7 +107,7 @@ impl<'a> VmEnsureNotStarted<'a> { log: &'a slog::Logger, vm: &'a Arc, ensure_request: &'a VmEnsureRequest, - ensure_options: &'a EnsureOptions, + ensure_options: &'a Arc, ensure_response_tx: InstanceEnsureResponseTx, state_publisher: &'a mut StatePublisher, ) -> Self { @@ -88,30 +121,86 @@ impl<'a> VmEnsureNotStarted<'a> { } } - pub(crate) fn instance_spec(&self) -> &Spec { - &self.ensure_request.instance_spec - } - pub(crate) fn state_publisher(&mut self) -> &mut StatePublisher { self.state_publisher } + pub(crate) fn migration_info(&self) -> Option<&MigrationTargetInfo> { + self.ensure_request.migration_info() + } + + pub(crate) async fn create_objects_for_new_vm( + self, + ) -> anyhow::Result> { + let VmInitializationMethod::Spec(spec) = &self.ensure_request.init + else { + panic!("create_objects_for_new_vm requires init via explicit spec"); + }; + + self.create_objects(spec.clone()).await + } + + pub(crate) async fn create_objects_for_migration( + self, + spec: Spec, + ) -> anyhow::Result> { + assert!(self.ensure_request.is_migration()); + self.create_objects(spec).await + } + /// Creates a set of VM objects using the instance spec stored in this /// ensure request, but does not install them as an active VM. - pub(crate) async fn create_objects( + async fn create_objects( self, + spec: Spec, ) -> anyhow::Result> { debug!(self.log, "creating VM objects"); let input_queue = Arc::new(InputQueue::new( self.log.new(slog::o!("component" => "request_queue")), - match &self.ensure_request.migrate { - Some(_) => InstanceAutoStart::Yes, - None => InstanceAutoStart::No, + if self.ensure_request.is_migration() { + InstanceAutoStart::Yes + } else { + InstanceAutoStart::No }, )); - match self.initialize_vm_objects_from_spec(&input_queue).await { + // Create the runtime that will host tasks created by VMM components + // (e.g. block device runtime tasks). + let vmm_rt = tokio::runtime::Builder::new_multi_thread() + .thread_name("tokio-rt-vmm") + .worker_threads(usize::max( + VMM_MIN_RT_THREADS, + VMM_BASE_RT_THREADS + spec.board.cpus as usize, + )) + .enable_all() + .build()?; + + // Run VM object creation on the new runtime so that if a component + // calls `tokio::spawn`, the task will spawn onto the VMM runtime and + // not the main server runtime. + let log_for_init = self.log.clone(); + let properties = self.ensure_request.properties.clone(); + let options = self.ensure_options.clone(); + let queue_for_init = input_queue.clone(); + let init_result = vmm_rt + .spawn(async move { + let options = options.as_ref(); + initialize_vm_objects( + log_for_init, + spec, + properties, + options, + queue_for_init, + ) + .await + }) + .await + .map_err(|e| { + anyhow::anyhow!("failed to join VM object creation task: {e}") + })?; + + match init_result { Ok(objects) => { // N.B. Once these `VmObjects` exist, it is no longer safe to // call `vm_init_failed`. @@ -124,6 +213,7 @@ impl<'a> VmEnsureNotStarted<'a> { Ok(VmEnsureObjectsCreated { log: self.log, vm: self.vm, + vmm_rt, ensure_request: self.ensure_request, ensure_options: self.ensure_options, ensure_response_tx: self.ensure_response_tx, @@ -148,114 +238,6 @@ impl<'a> VmEnsureNotStarted<'a> { reason } - - async fn initialize_vm_objects_from_spec( - &self, - event_queue: &Arc, - ) -> anyhow::Result { - let properties = &self.ensure_request.properties; - let spec = &self.ensure_request.instance_spec; - let options = self.ensure_options; - - info!(self.log, "initializing new VM"; - "spec" => #?spec, - "properties" => #?properties, - "use_reservoir" => options.use_reservoir, - "bootrom" => %options.toml_config.bootrom.display()); - - let vmm_log = self.log.new(slog::o!("component" => "vmm")); - - // Set up the 'shell' instance into which the rest of this routine will - // add components. - let machine = build_instance( - &properties.vm_name(), - spec, - options.use_reservoir, - vmm_log, - )?; - - let mut init = MachineInitializer { - log: self.log.clone(), - machine: &machine, - devices: Default::default(), - block_backends: Default::default(), - crucible_backends: Default::default(), - spec, - properties, - toml_config: &options.toml_config, - producer_registry: options.oximeter_registry.clone(), - state: MachineInitializerState::default(), - kstat_sampler: initialize_kstat_sampler( - self.log, - self.instance_spec(), - options.oximeter_registry.clone(), - ), - stats_vm: VirtualMachine::new(spec.board.cpus, properties), - }; - - init.initialize_rom(options.toml_config.bootrom.as_path())?; - let chipset = init.initialize_chipset( - &(event_queue.clone() - as Arc), - )?; - - init.initialize_rtc(&chipset)?; - init.initialize_hpet(); - - let com1 = Arc::new(init.initialize_uart(&chipset)); - let ps2ctrl = init.initialize_ps2(&chipset); - init.initialize_qemu_debug_port()?; - init.initialize_qemu_pvpanic(VirtualMachine::new( - self.instance_spec().board.cpus, - properties, - ))?; - init.initialize_network_devices(&chipset).await?; - - #[cfg(not(feature = "omicron-build"))] - init.initialize_test_devices(&options.toml_config.devices); - #[cfg(feature = "omicron-build")] - info!( - self.log, - "`omicron-build` feature enabled, ignoring any test devices" - ); - - #[cfg(feature = "falcon")] - { - init.initialize_softnpu_ports(&chipset)?; - init.initialize_9pfs(&chipset); - } - - init.initialize_storage_devices(&chipset, options.nexus_client.clone()) - .await?; - - let ramfb = init.initialize_fwcfg(self.instance_spec().board.cpus)?; - init.initialize_cpus().await?; - let vcpu_tasks = Box::new(crate::vcpu_tasks::VcpuTasks::new( - &machine, - event_queue.clone() - as Arc, - self.log.new(slog::o!("component" => "vcpu_tasks")), - )?); - - let MachineInitializer { - devices, - block_backends, - crucible_backends, - .. - } = init; - - Ok(InputVmObjects { - instance_spec: spec.clone(), - vcpu_tasks, - machine, - devices, - block_backends, - crucible_backends, - com1, - framebuffer: Some(ramfb), - ps2ctrl, - }) - } } /// Represents an instance ensure request that has proceeded far enough to @@ -264,6 +246,7 @@ impl<'a> VmEnsureNotStarted<'a> { pub(crate) struct VmEnsureObjectsCreated<'a> { log: &'a slog::Logger, vm: &'a Arc, + vmm_rt: tokio::runtime::Runtime, ensure_request: &'a VmEnsureRequest, ensure_options: &'a EnsureOptions, ensure_response_tx: InstanceEnsureResponseTx, @@ -301,11 +284,17 @@ impl<'a> VmEnsureObjectsCreated<'a> { ) .await; + // The VMM runtime itself lives in the `ActiveVm` structure created by + // this call. Preserve a handle to it to be passed back to the + // initialization process so that it can launch the state driver task + // onto this runtime. + let vmm_rt_hdl = self.vmm_rt.handle().clone(); self.vm .make_active( self.log, self.input_queue.clone(), &self.vm_objects, + self.vmm_rt, vm_services, ) .await; @@ -316,15 +305,19 @@ impl<'a> VmEnsureObjectsCreated<'a> { // state and using the state change API to send commands to the state // driver. let _ = self.ensure_response_tx.send(Ok(InstanceEnsureResponse { - migrate: self.ensure_request.migrate.as_ref().map(|req| { - InstanceMigrateInitiateResponse { - migration_id: req.migration_id, + migrate: match &self.ensure_request.init { + VmInitializationMethod::Spec(_) => None, + VmInitializationMethod::Migration(info) => { + Some(InstanceMigrateInitiateResponse { + migration_id: info.migration_id, + }) } - }), + }, })); VmEnsureActive { vm: self.vm, + vmm_rt_hdl, state_publisher: self.state_publisher, vm_objects: self.vm_objects, input_queue: self.input_queue, @@ -338,12 +331,19 @@ impl<'a> VmEnsureObjectsCreated<'a> { /// not started yet. pub(crate) struct VmEnsureActive<'a> { vm: &'a Arc, + vmm_rt_hdl: tokio::runtime::Handle, state_publisher: &'a mut StatePublisher, vm_objects: Arc, input_queue: Arc, kernel_vm_paused: bool, } +pub(super) struct VmEnsureActiveOutput { + pub vm_objects: Arc, + pub input_queue: Arc, + pub vmm_rt_hdl: tokio::runtime::Handle, +} + impl<'a> VmEnsureActive<'a> { pub(crate) fn vm_objects(&self) -> &Arc { &self.vm_objects @@ -373,11 +373,120 @@ impl<'a> VmEnsureActive<'a> { /// Yields the VM objects and input queue for this VM so that they can be /// used to start a state driver loop. - pub(super) fn into_inner(self) -> (Arc, Arc) { - (self.vm_objects, self.input_queue) + pub(super) fn into_inner(self) -> VmEnsureActiveOutput { + VmEnsureActiveOutput { + vm_objects: self.vm_objects, + input_queue: self.input_queue, + vmm_rt_hdl: self.vmm_rt_hdl, + } } } +/// Initializes a set of VM objects from the supplied specification and options. +/// +/// This function should be called from the VMM runtime. This ensures that if +/// the new VM objects create tokio tasks, they will run on the VMM runtime and +/// not the Dropshot server runtime. +async fn initialize_vm_objects( + log: slog::Logger, + spec: Spec, + properties: InstanceProperties, + options: &EnsureOptions, + event_queue: Arc, +) -> anyhow::Result { + info!(log, "initializing new VM"; + "spec" => #?spec, + "properties" => #?properties, + "use_reservoir" => options.use_reservoir, + "bootrom" => %options.bootrom_path.display()); + + let vmm_log = log.new(slog::o!("component" => "vmm")); + + // Set up the 'shell' instance into which the rest of this routine will + // add components. + let machine = build_instance( + &properties.vm_name(), + &spec, + options.use_reservoir, + vmm_log, + )?; + + let mut init = MachineInitializer { + log: log.clone(), + machine: &machine, + devices: Default::default(), + block_backends: Default::default(), + crucible_backends: Default::default(), + spec: &spec, + properties: &properties, + producer_registry: options.oximeter_registry.clone(), + state: MachineInitializerState::default(), + kstat_sampler: initialize_kstat_sampler( + &log, + &spec, + options.oximeter_registry.clone(), + ), + stats_vm: VirtualMachine::new(spec.board.cpus, &properties), + bootrom_version: options.bootrom_version.clone(), + }; + + init.initialize_rom(options.bootrom_path.as_path())?; + let chipset = init.initialize_chipset( + &(event_queue.clone() + as Arc), + )?; + + init.initialize_rtc(&chipset)?; + init.initialize_hpet(); + + let com1 = Arc::new(init.initialize_uart(&chipset)); + let ps2ctrl = init.initialize_ps2(&chipset); + init.initialize_qemu_debug_port()?; + init.initialize_qemu_pvpanic(VirtualMachine::new( + spec.board.cpus, + &properties, + ))?; + init.initialize_network_devices(&chipset).await?; + + #[cfg(not(feature = "omicron-build"))] + init.initialize_test_devices(); + #[cfg(feature = "omicron-build")] + info!(log, "`omicron-build` feature enabled, ignoring any test devices"); + + #[cfg(feature = "falcon")] + { + init.initialize_softnpu_ports(&chipset)?; + init.initialize_9pfs(&chipset); + } + + init.initialize_storage_devices(&chipset, options.nexus_client.clone()) + .await?; + + let ramfb = init.initialize_fwcfg(spec.board.cpus)?; + init.initialize_cpus().await?; + let vcpu_tasks = Box::new(crate::vcpu_tasks::VcpuTasks::new( + &machine, + event_queue.clone() as Arc, + log.new(slog::o!("component" => "vcpu_tasks")), + )?); + + let MachineInitializer { + devices, block_backends, crucible_backends, .. + } = init; + + Ok(InputVmObjects { + instance_spec: spec, + vcpu_tasks, + machine, + devices, + block_backends, + crucible_backends, + com1, + framebuffer: Some(ramfb), + ps2ctrl, + }) +} + /// Create an object used to sample kstats. fn initialize_kstat_sampler( log: &slog::Logger, diff --git a/bin/propolis-server/src/lib/vm/mod.rs b/bin/propolis-server/src/lib/vm/mod.rs index fea20119f..2fb9c0587 100644 --- a/bin/propolis-server/src/lib/vm/mod.rs +++ b/bin/propolis-server/src/lib/vm/mod.rs @@ -79,16 +79,16 @@ //! 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; +use ensure::{VmEnsureRequest, VmInitializationMethod}; use oximeter::types::ProducerRegistry; use propolis_api_types::{ - instance_spec::VersionedInstanceSpec, InstanceEnsureResponse, - InstanceMigrateStatusResponse, InstanceMigrationStatus, InstanceProperties, - InstanceSpecGetResponse, InstanceState, InstanceStateMonitorResponse, - MigrationState, + instance_spec::{SpecKey, VersionedInstanceSpec}, + InstanceEnsureResponse, InstanceMigrateStatusResponse, + InstanceMigrationStatus, InstanceProperties, InstanceSpecGetResponse, + InstanceState, InstanceStateMonitorResponse, MigrationState, }; use slog::info; use state_driver::StateDriverOutput; @@ -109,7 +109,7 @@ pub(crate) mod state_publisher; /// Maps component names to lifecycle trait objects that allow /// components to be started, paused, resumed, and halted. pub(crate) type DeviceMap = - BTreeMap>; + BTreeMap>; /// Mapping of NIC identifiers to viona device instance IDs. /// We use a Vec here due to the limited size of the NIC array. @@ -117,11 +117,11 @@ pub(crate) type NetworkInterfaceIds = Vec<(uuid::Uuid, KstatInstanceId)>; /// Maps component names to block backend trait objects. pub(crate) type BlockBackendMap = - BTreeMap>; + BTreeMap>; -/// Maps component names to Crucible backend objects. +/// Maps disk IDs to Crucible backend objects. pub(crate) type CrucibleBackendMap = - BTreeMap>; + BTreeMap>; /// Type alias for the sender side of the channel that receives /// externally-visible instance state updates. @@ -179,9 +179,6 @@ pub(crate) enum VmError { #[error("Forbidden state change")] ForbiddenStateChange(#[from] request_queue::RequestDeniedReason), - - #[error("Failed to initialize VM's tokio runtime")] - TokioRuntimeInitializationFailed(#[source] std::io::Error), } /// The top-level VM wrapper type. @@ -215,11 +212,16 @@ struct VmDescription { /// The VM's API-level instance properties. properties: InstanceProperties, - /// The VM's last-known instance specification. - spec: Spec, + /// The VM's last-known instance specification, or None if no specification + /// has yet been supplied for this VM. + spec: Option, /// The runtime on which the VM's state driver is running (or on which it /// ran). + /// + /// This is preserved in the VM state machine so that the state driver task + /// doesn't drop its runtime out from under itself when it signals that the + /// state machine should transition from Active to Rundown. tokio_rt: Option, } @@ -262,9 +264,11 @@ 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, + /// A reference to this server process's bootrom path. + pub(super) bootrom_path: Arc, + + /// The bootrom version string to display to the guest. + pub(super) bootrom_version: Option, /// True if VMs should allocate memory from the kernel VMM reservoir. pub(super) use_reservoir: bool, @@ -332,7 +336,7 @@ impl Vm { let state = vm.external_state_rx.borrow().clone(); Some(InstanceSpecGetResponse { properties: vm.properties.clone(), - spec: VersionedInstanceSpec::V0(spec.into()), + spec: Some(VersionedInstanceSpec::V0(spec.into())), state: state.state, }) } @@ -345,7 +349,10 @@ impl Vm { | VmState::RundownComplete(vm) => Some(InstanceSpecGetResponse { properties: vm.properties.clone(), state: vm.external_state_rx.borrow().state, - spec: VersionedInstanceSpec::V0(vm.spec.clone().into()), + spec: vm + .spec + .clone() + .map(|s| VersionedInstanceSpec::V0(s.into())), }), } } @@ -382,6 +389,7 @@ impl Vm { log: &slog::Logger, state_driver_queue: Arc, objects: &Arc, + vmm_rt: tokio::runtime::Runtime, services: services::VmServices, ) { info!(self.log, "installing active VM"); @@ -396,7 +404,7 @@ impl Vm { properties: vm.properties, objects: objects.clone(), services, - tokio_rt: vm.tokio_rt.expect("WaitingForInit has runtime"), + tokio_rt: vmm_rt, }); } state => unreachable!( @@ -454,7 +462,7 @@ impl Vm { guard.state = VmState::Rundown(VmDescription { external_state_rx, properties, - spec, + spec: Some(spec), tokio_rt: Some(tokio_rt), }); vm.services @@ -530,18 +538,22 @@ impl Vm { &log_for_driver, InstanceStateMonitorResponse { gen: 1, - state: if ensure_request.migrate.is_some() { - InstanceState::Migrating - } else { - InstanceState::Creating + state: match ensure_request.init { + VmInitializationMethod::Spec(_) => InstanceState::Creating, + VmInitializationMethod::Migration(_) => { + InstanceState::Migrating + } }, migration: InstanceMigrateStatusResponse { - migration_in: ensure_request.migrate.as_ref().map(|req| { - InstanceMigrationStatus { - id: req.migration_id, - state: MigrationState::Sync, + migration_in: match &ensure_request.init { + VmInitializationMethod::Spec(_) => None, + VmInitializationMethod::Migration(info) => { + Some(InstanceMigrationStatus { + id: info.migration_id, + state: MigrationState::Sync, + }) } - }), + }, migration_out: None, }, }, @@ -561,23 +573,10 @@ impl Vm { _ => {} }; - let thread_count = usize::max( - VMM_MIN_RT_THREADS, - VMM_BASE_RT_THREADS - + ensure_request.instance_spec.board.cpus as usize, - ); - - let tokio_rt = tokio::runtime::Builder::new_multi_thread() - .thread_name("tokio-rt-vmm") - .worker_threads(thread_count) - .enable_all() - .build() - .map_err(VmError::TokioRuntimeInitializationFailed)?; - let properties = ensure_request.properties.clone(); - let spec = ensure_request.instance_spec.clone(); + let spec = ensure_request.spec().cloned(); let vm_for_driver = self.clone(); - guard.driver = Some(tokio_rt.spawn(async move { + guard.driver = Some(tokio::spawn(async move { state_driver::run_state_driver( log_for_driver, vm_for_driver, @@ -593,7 +592,7 @@ impl Vm { external_state_rx: external_rx.clone(), properties, spec, - tokio_rt: Some(tokio_rt), + tokio_rt: None, }); } diff --git a/bin/propolis-server/src/lib/vm/objects.rs b/bin/propolis-server/src/lib/vm/objects.rs index a79cee1d7..49f4c04ed 100644 --- a/bin/propolis-server/src/lib/vm/objects.rs +++ b/bin/propolis-server/src/lib/vm/objects.rs @@ -17,6 +17,7 @@ use propolis::{ vmm::VmmHdl, Machine, }; +use propolis_api_types::instance_spec::SpecKey; use slog::{error, info}; use tokio::sync::{RwLock, RwLockReadGuard, RwLockWriteGuard}; @@ -159,12 +160,12 @@ impl VmObjectsLocked { } /// Obtains a handle to the lifecycle trait object for the component with - /// the supplied `name`. + /// the supplied `id`. pub(crate) fn device_by_name( &self, - name: &str, + id: &SpecKey, ) -> Option> { - self.devices.get(name).cloned() + self.devices.get(id).cloned() } /// Yields the VM's current Crucible backend map. @@ -192,7 +193,7 @@ impl VmObjectsLocked { /// `func` on each one. pub(crate) fn for_each_device( &self, - mut func: impl FnMut(&str, &Arc), + mut func: impl FnMut(&SpecKey, &Arc), ) { for (name, dev) in self.devices.iter() { func(name, dev); @@ -205,7 +206,7 @@ impl VmObjectsLocked { pub(crate) fn for_each_device_fallible( &self, mut func: impl FnMut( - &str, + &SpecKey, &Arc, ) -> std::result::Result<(), E>, ) -> std::result::Result<(), E> { @@ -377,7 +378,7 @@ impl VmObjectsLocked { .iter() .map(|(name, dev)| { info!(self.log, "got paused future from dev {}", name); - NamedFuture { name: name.clone(), future: dev.paused() } + NamedFuture { name: name.to_string(), future: dev.paused() } }) .collect(); @@ -413,7 +414,7 @@ impl VmObjectsLocked { backend.stop().await; if let Err(err) = backend.detach() { error!(self.log, "error detaching block backend"; - "name" => name, + "name" => %name, "error" => ?err); } } diff --git a/bin/propolis-server/src/lib/vm/request_queue.rs b/bin/propolis-server/src/lib/vm/request_queue.rs index 6cea692a1..f5e72a10b 100644 --- a/bin/propolis-server/src/lib/vm/request_queue.rs +++ b/bin/propolis-server/src/lib/vm/request_queue.rs @@ -23,6 +23,7 @@ use std::collections::VecDeque; +use propolis_api_types::instance_spec::SpecKey; use slog::{debug, info, Logger}; use thiserror::Error; use uuid::Uuid; @@ -78,11 +79,8 @@ pub enum ExternalRequest { /// is only allowed once the VM is started and the volume has activated, but /// it should be allowed even before the VM has started. ReconfigureCrucibleVolume { - /// The name of the Crucible backend component in the instance spec. - disk_name: String, - - /// The ID of the Crucible backend in the VM's Crucible backend map. - backend_id: Uuid, + /// The + disk_id: SpecKey, /// The new volume construction request to supply to the Crucible /// upstairs. @@ -103,12 +101,9 @@ impl std::fmt::Debug for ExternalRequest { .finish(), Self::Reboot => write!(f, "Reboot"), Self::Stop => write!(f, "Stop"), - Self::ReconfigureCrucibleVolume { - disk_name, backend_id, .. - } => f + Self::ReconfigureCrucibleVolume { disk_id, .. } => f .debug_struct("ReconfigureCrucibleVolume") - .field("disk_name", disk_name) - .field("backend_id", backend_id) + .field("disk_id", disk_id) .finish(), } } @@ -473,8 +468,7 @@ mod test { fn make_reconfigure_crucible_request() -> ExternalRequest { let (tx, _rx) = tokio::sync::oneshot::channel(); ExternalRequest::ReconfigureCrucibleVolume { - disk_name: "".to_string(), - backend_id: Uuid::new_v4(), + disk_id: SpecKey::Uuid(Uuid::new_v4()), new_vcr_json: "".to_string(), result_tx: tx, } diff --git a/bin/propolis-server/src/lib/vm/state_driver.rs b/bin/propolis-server/src/lib/vm/state_driver.rs index a0298b4e4..e6350212f 100644 --- a/bin/propolis-server/src/lib/vm/state_driver.rs +++ b/bin/propolis-server/src/lib/vm/state_driver.rs @@ -11,8 +11,8 @@ use std::{ use anyhow::Context; use propolis_api_types::{ - instance_spec::components::backends::CrucibleStorageBackend, InstanceState, - MigrationState, + instance_spec::{components::backends::CrucibleStorageBackend, SpecKey}, + InstanceState, MigrationState, }; use slog::{error, info}; use tokio::sync::Notify; @@ -27,7 +27,10 @@ use crate::{ }; use super::{ - ensure::{VmEnsureActive, VmEnsureNotStarted, VmEnsureRequest}, + ensure::{ + VmEnsureActive, VmEnsureActiveOutput, VmEnsureNotStarted, + VmEnsureRequest, VmInitializationMethod, + }, guest_event::{self, GuestEvent}, objects::VmObjects, request_queue::{self, ExternalRequest, InstanceAutoStart}, @@ -264,6 +267,9 @@ pub(super) async fn run_state_driver( ensure_result_tx: InstanceEnsureResponseTx, ensure_options: super::EnsureOptions, ) -> StateDriverOutput { + // Wrap the ensure options in an Arc so that callees can create cloned + // references that they can pass out to new tasks. + let ensure_options = Arc::new(ensure_options); let activated_vm = match create_and_activate_vm( &log, &vm, @@ -284,21 +290,29 @@ pub(super) async fn run_state_driver( } }; - let (objects, input_queue) = activated_vm.into_inner(); + let VmEnsureActiveOutput { vm_objects, input_queue, vmm_rt_hdl } = + activated_vm.into_inner(); let state_driver = StateDriver { log, - objects, + objects: vm_objects, input_queue, external_state: state_publisher, paused: false, migration_src_state: Default::default(), }; - // Run the VM until it exits, then set rundown on the parent VM so that no - // new external callers can access its objects or services. - let output = state_driver.run(ensure_request.migrate.is_some()).await; - vm.set_rundown().await; - output + let vm_for_driver = vm.clone(); + match vmm_rt_hdl + .spawn(async move { + let output = state_driver.run(ensure_request.is_migration()).await; + vm_for_driver.set_rundown().await; + output + }) + .await + { + Ok(output) => output, + Err(e) => panic!("failed to join state driver task: {e}"), + } } /// Processes the supplied `ensure_request` to create a set of VM objects that @@ -309,7 +323,7 @@ async fn create_and_activate_vm<'a>( state_publisher: &'a mut StatePublisher, ensure_request: &'a VmEnsureRequest, ensure_result_tx: InstanceEnsureResponseTx, - ensure_options: &'a super::EnsureOptions, + ensure_options: &'a Arc, ) -> anyhow::Result> { let ensure = VmEnsureNotStarted::new( log, @@ -320,10 +334,10 @@ async fn create_and_activate_vm<'a>( state_publisher, ); - if let Some(migrate_request) = ensure_request.migrate.as_ref() { + if let VmInitializationMethod::Migration(info) = &ensure_request.init { let migration = match crate::migrate::destination::initiate( log, - migrate_request, + info, ensure_options.local_server_addr, ) .await @@ -347,7 +361,7 @@ async fn create_and_activate_vm<'a>( .context("running live migration protocol")?) } else { let created = ensure - .create_objects() + .create_objects_for_new_vm() .await .context("creating VM objects for new instance")?; @@ -496,18 +510,13 @@ impl StateDriver { } } ExternalRequest::ReconfigureCrucibleVolume { - disk_name, - backend_id, + disk_id, new_vcr_json, result_tx, } => { let _ = result_tx.send( - self.reconfigure_crucible_volume( - disk_name, - &backend_id, - new_vcr_json, - ) - .await, + self.reconfigure_crucible_volume(disk_id, new_vcr_json) + .await, ); HandleEventOutcome::Continue } @@ -646,15 +655,13 @@ impl StateDriver { async fn reconfigure_crucible_volume( &self, - disk_name: String, - backend_id: &Uuid, + disk_id: SpecKey, new_vcr_json: String, ) -> super::CrucibleReplaceResult { info!(self.log, "request to replace Crucible VCR"; - "disk_name" => %disk_name, - "backend_id" => %backend_id); + "disk_id" => %disk_id); - fn spec_element_not_found(disk_name: &str) -> dropshot::HttpError { + fn spec_element_not_found(disk_name: &SpecKey) -> dropshot::HttpError { let msg = format!("Crucible backend for {:?} not found", disk_name); dropshot::HttpError::for_not_found(Some(msg.clone()), msg) } @@ -662,16 +669,16 @@ impl StateDriver { let mut objects = self.objects.lock_exclusive().await; let backend = objects .crucible_backends() - .get(backend_id) + .get(&disk_id) .ok_or_else(|| { - let msg = format!("No crucible backend for id {backend_id}"); + let msg = format!("No crucible backend for id {disk_id}"); dropshot::HttpError::for_not_found(Some(msg.clone()), msg) })? .clone(); - let Some(disk) = objects.instance_spec_mut().disks.get_mut(&disk_name) + let Some(disk) = objects.instance_spec_mut().disks.get_mut(&disk_id) else { - return Err(spec_element_not_found(&disk_name)); + return Err(spec_element_not_found(&disk_id)); }; let StorageBackend::Crucible(CrucibleStorageBackend { @@ -679,7 +686,7 @@ impl StateDriver { readonly, }) = &disk.backend_spec else { - return Err(spec_element_not_found(&disk_name)); + return Err(spec_element_not_found(&disk_id)); }; let replace_result = backend @@ -697,7 +704,7 @@ impl StateDriver { request_json: new_vcr_json, }); - info!(self.log, "replaced Crucible VCR"; "backend_id" => %backend_id); + info!(self.log, "replaced Crucible VCR"; "disk_id" => %disk_id); Ok(replace_result) } diff --git a/bin/propolis-server/src/main.rs b/bin/propolis-server/src/main.rs index b4127ec31..217243c19 100644 --- a/bin/propolis-server/src/main.rs +++ b/bin/propolis-server/src/main.rs @@ -75,7 +75,10 @@ enum Args { /// Runs the Propolis server. Run { #[clap(action)] - cfg: PathBuf, + bootrom_path: PathBuf, + + #[clap(long, action)] + bootrom_version: Option, #[clap(name = "PROPOLIS_IP:PORT", action)] propolis_addr: SocketAddr, @@ -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/Cargo.toml b/crates/propolis-api-types/Cargo.toml index 5deedb3dd..6a510015d 100644 --- a/crates/propolis-api-types/Cargo.toml +++ b/crates/propolis-api-types/Cargo.toml @@ -12,5 +12,6 @@ crucible-client-types.workspace = true propolis_types.workspace = true schemars.workspace = true serde.workspace = true +serde_with.workspace = true thiserror.workspace = true uuid.workspace = true diff --git a/crates/propolis-api-types/src/instance_spec/components/backends.rs b/crates/propolis-api-types/src/instance_spec/components/backends.rs index 79fc161cf..83bb04fa3 100644 --- a/crates/propolis-api-types/src/instance_spec/components/backends.rs +++ b/crates/propolis-api-types/src/instance_spec/components/backends.rs @@ -78,10 +78,10 @@ pub struct VirtioNetworkBackend { pub vnic_name: String, } -/// A network backend associated with a DLPI VNIC on the host. +/// A network backend associated with a DLPI network device on the host. #[derive(Clone, Deserialize, Serialize, Debug, JsonSchema)] #[serde(deny_unknown_fields)] pub struct DlpiNetworkBackend { - /// The name of the VNIC to use as a backend. + /// The name of the DLPI device to use as a backend. pub vnic_name: String, } 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 06ced9844..d15e30e88 100644 --- a/crates/propolis-api-types/src/instance_spec/components/devices.rs +++ b/crates/propolis-api-types/src/instance_spec/components/devices.rs @@ -5,7 +5,7 @@ //! Device configuration data: components that define VM properties that are //! visible to a VM's guest software. -use crate::instance_spec::PciPath; +use crate::instance_spec::{PciPath, SpecKey}; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; @@ -14,7 +14,7 @@ use serde::{Deserialize, Serialize}; #[serde(deny_unknown_fields)] pub struct VirtioDisk { /// The name of the disk's backend component. - pub backend_name: String, + pub backend_id: SpecKey, /// The PCI bus/device/function at which this disk should be attached. pub pci_path: PciPath, @@ -25,7 +25,7 @@ pub struct VirtioDisk { #[serde(deny_unknown_fields)] pub struct NvmeDisk { /// The name of the disk's backend component. - pub backend_name: String, + pub backend_id: SpecKey, /// The PCI bus/device/function at which this disk should be attached. pub pci_path: PciPath, @@ -35,8 +35,8 @@ pub struct NvmeDisk { #[derive(Clone, Deserialize, Serialize, Debug, JsonSchema)] #[serde(deny_unknown_fields)] pub struct VirtioNic { - /// The name of the device's backend. - pub backend_name: String, + /// The ID of the device's backend. + pub backend_id: SpecKey, /// A caller-defined correlation identifier for this interface. If Propolis /// is configured to collect network interface kstats in its Oximeter @@ -115,13 +115,13 @@ pub struct BootSettings { } /// An entry in the boot order stored in a [`BootSettings`] component. -#[derive(Clone, Deserialize, Serialize, Debug, JsonSchema, Default)] +#[derive(Clone, Deserialize, Serialize, Debug, JsonSchema)] pub struct BootOrderEntry { - /// The name of another component in the spec that Propolis should try to + /// The ID of another component in the spec that Propolis should try to /// boot from. /// /// Currently, only disk device components are supported. - pub name: String, + pub component_id: SpecKey, } // @@ -139,18 +139,18 @@ pub struct SoftNpuPciPort { pub pci_path: PciPath, } -/// Describes a SoftNPU network port. +/// Describes a data link port on 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. - pub backend_name: String, + /// The name of the DLPI backend with which to associate this port. + pub backend_id: SpecKey, } /// Describes a PCI device that shares host files with the guest using the P9 @@ -185,3 +185,18 @@ pub struct P9fs { /// The PCI path at which to attach the guest to this P9 filesystem. pub pci_path: PciPath, } + +/// Describes a synthetic device that registers for VM lifecycle notifications +/// and returns errors during attempts to migrate. +/// +/// This is only supported by Propolis servers compiled without the +/// `omicron-build` feature. +#[derive(Clone, Deserialize, Serialize, Debug, JsonSchema)] +#[serde(deny_unknown_fields)] +pub struct MigrationFailureInjector { + /// The number of times this device should fail requests to export state. + pub fail_exports: u32, + + /// The number of times this device should fail requests to import state. + pub fail_imports: u32, +} diff --git a/crates/propolis-api-types/src/instance_spec/mod.rs b/crates/propolis-api-types/src/instance_spec/mod.rs index 53b234f5a..ed1f4c9b0 100644 --- a/crates/propolis-api-types/src/instance_spec/mod.rs +++ b/crates/propolis-api-types/src/instance_spec/mod.rs @@ -155,14 +155,72 @@ use schemars::JsonSchema; use serde::{Deserialize, Serialize}; +use serde_with::{DeserializeFromStr, SerializeDisplay}; pub use propolis_types::{CpuidIdent, CpuidValues, CpuidVendor, PciPath}; +use uuid::Uuid; pub mod components; pub mod v0; -/// Type alias for keys in the instance spec's maps. -type SpecKey = String; +/// A key identifying a component in an instance spec. +// +// This type impls `SerializeDisplay` and `DeserializeFromStr` so that it can be +// used as a key type in maps that can be serialized to JSON (which requires +// strings to be used as keys). +#[derive( + Clone, + Debug, + SerializeDisplay, + DeserializeFromStr, + Hash, + Eq, + PartialEq, + Ord, + PartialOrd, + JsonSchema, +)] +pub enum SpecKey { + Uuid(uuid::Uuid), + Name(String), +} + +impl std::fmt::Display for SpecKey { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::Uuid(id) => write!(f, "{id}"), + Self::Name(name) => write!(f, "{name}"), + } + } +} + +impl std::str::FromStr for SpecKey { + // This conversion is infallible, but the error type needs to implement + // `Display` in order to derive `DeserializeFromStr`. + type Err = &'static str; + + fn from_str(s: &str) -> Result { + Ok(match Uuid::parse_str(s) { + Ok(uuid) => Self::Uuid(uuid), + Err(_) => Self::Name(s.to_owned()), + }) + } +} + +impl From for SpecKey { + fn from(s: String) -> Self { + match Uuid::parse_str(&s) { + Ok(uuid) => Self::Uuid(uuid), + Err(_) => Self::Name(s), + } + } +} + +impl From for SpecKey { + fn from(value: Uuid) -> Self { + Self::Uuid(value) + } +} /// A versioned instance spec. #[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)] diff --git a/crates/propolis-api-types/src/instance_spec/v0.rs b/crates/propolis-api-types/src/instance_spec/v0.rs index 634e5fe1d..68d7709e8 100644 --- a/crates/propolis-api-types/src/instance_spec/v0.rs +++ b/crates/propolis-api-types/src/instance_spec/v0.rs @@ -4,10 +4,12 @@ use std::collections::HashMap; -use crate::instance_spec::{components, SpecKey}; +use crate::instance_spec::components; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; +use super::SpecKey; + #[derive(Clone, Deserialize, Serialize, Debug, JsonSchema)] #[serde(deny_unknown_fields, tag = "type", content = "component")] pub enum ComponentV0 { @@ -22,6 +24,7 @@ pub enum ComponentV0 { SoftNpuPort(components::devices::SoftNpuPort), SoftNpuP9(components::devices::SoftNpuP9), P9fs(components::devices::P9fs), + MigrationFailureInjector(components::devices::MigrationFailureInjector), CrucibleStorageBackend(components::backends::CrucibleStorageBackend), FileStorageBackend(components::backends::FileStorageBackend), BlobStorageBackend(components::backends::BlobStorageBackend), diff --git a/crates/propolis-api-types/src/lib.rs b/crates/propolis-api-types/src/lib.rs index 44bd7a68d..73d2877f8 100644 --- a/crates/propolis-api-types/src/lib.rs +++ b/crates/propolis-api-types/src/lib.rs @@ -4,8 +4,13 @@ //! Definitions for types exposed by the propolis-server API -use std::{fmt, net::SocketAddr}; +use std::{collections::HashMap, fmt, net::SocketAddr}; +use instance_spec::{ + components::{backends, devices}, + v0::InstanceSpecV0, + SpecKey, +}; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use uuid::Uuid; @@ -24,7 +29,6 @@ pub mod instance_spec; #[derive(Clone, Deserialize, Serialize, JsonSchema)] pub struct InstanceVCRReplace { - pub name: String, pub vcr_json: String, } @@ -48,36 +52,71 @@ pub struct InstanceMetadata { pub sled_model: String, } -#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)] -pub struct InstanceEnsureRequest { - pub properties: InstanceProperties, - - /// Number of vCPUs to be allocated to the Instance. - pub vcpus: u8, - - /// Size of memory allocated to the Instance, in MiB. - pub memory: u64, +#[derive(Clone, Deserialize, Serialize, Debug, JsonSchema)] +#[serde(deny_unknown_fields, tag = "type", content = "component")] +pub enum ReplacementComponent { + MigrationFailureInjector(devices::MigrationFailureInjector), + CrucibleStorageBackend(backends::CrucibleStorageBackend), + FileStorageBackend(backends::FileStorageBackend), + BlobStorageBackend(backends::BlobStorageBackend), + VirtioNetworkBackend(backends::VirtioNetworkBackend), + DlpiNetworkBackend(backends::DlpiNetworkBackend), +} - #[serde(default)] - pub nics: Vec, +impl From for instance_spec::v0::ComponentV0 { + fn from(value: ReplacementComponent) -> Self { + use instance_spec::v0::ComponentV0; + match value { + ReplacementComponent::MigrationFailureInjector(dev) => { + ComponentV0::MigrationFailureInjector(dev) + } + ReplacementComponent::CrucibleStorageBackend(be) => { + ComponentV0::CrucibleStorageBackend(be) + } + ReplacementComponent::FileStorageBackend(be) => { + ComponentV0::FileStorageBackend(be) + } + ReplacementComponent::BlobStorageBackend(be) => { + ComponentV0::BlobStorageBackend(be) + } + ReplacementComponent::VirtioNetworkBackend(be) => { + ComponentV0::VirtioNetworkBackend(be) + } + ReplacementComponent::DlpiNetworkBackend(be) => { + ComponentV0::DlpiNetworkBackend(be) + } + } + } +} - #[serde(default)] - pub disks: Vec, +/// The mechanism to use to create a new Propolis VM. +#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)] +#[serde(tag = "method", content = "value")] +pub enum InstanceInitializationMethod { + /// Create a brand new VM with the devies specified in the supplied spec. + Spec { + /// The component manifest for the new VM. + spec: InstanceSpecV0, + }, - #[serde(default)] - pub boot_settings: Option, + /// Initialize the VM via migration. + MigrationTarget { + /// The ID to assign to this migration attempt. + migration_id: Uuid, - pub migrate: Option, + /// The address of the Propolis server that will serve as the migration + /// source. + src_addr: SocketAddr, - // base64 encoded cloud-init ISO - pub cloud_init_bytes: Option, + /// A list of components in the source VM's instance spec to replace. + replace_components: HashMap, + }, } #[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)] -pub struct InstanceSpecEnsureRequest { +pub struct InstanceEnsureRequest { pub properties: InstanceProperties, - pub instance_spec: VersionedInstanceSpec, - pub migrate: Option, + pub init: InstanceInitializationMethod, } #[derive(Clone, Deserialize, Serialize, JsonSchema)] @@ -85,13 +124,6 @@ pub struct InstanceEnsureResponse { pub migrate: Option, } -#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)] -pub struct InstanceMigrateInitiateRequest { - pub migration_id: Uuid, - pub src_addr: SocketAddr, - pub src_uuid: Uuid, -} - #[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)] pub struct InstanceMigrateInitiateResponse { pub migration_id: Uuid, @@ -169,7 +201,12 @@ pub struct InstanceGetResponse { pub struct InstanceSpecGetResponse { pub properties: InstanceProperties, pub state: InstanceState, - pub spec: VersionedInstanceSpec, + + /// The instance's component manifest, if it is known at this point. + /// (Instances that initialize via live migration receive specs from their + /// sources, so this field will be None for an instance that is still + /// initializing via migration.) + pub spec: Option, } #[derive(Clone, Deserialize, Serialize, JsonSchema)] @@ -294,86 +331,20 @@ pub enum InstanceSerialConsoleControlMessage { Migrating { destination: SocketAddr, from_start: u64 }, } -/// Describes how to connect to one or more storage agent services. -#[derive(Clone, Deserialize, Serialize, JsonSchema)] -pub struct StorageAgentDescription { - /// Addresses of storage agents. - pub agents: Vec, - - /// Opaque key material for encryption and decryption. - /// May become more structured as encryption scheme is solidified. - pub key: Vec, - - /// Minimum number of redundant copies of a block which must - /// be written until data is considered "persistent". - pub write_redundancy_threshold: u32, -} - -/// Refer to RFD 135 for more information on Virtual Storage Interfaces. -/// This describes the type of disk which should be exposed to the guest VM. -#[derive(Clone, Copy, Deserialize, Serialize, JsonSchema)] -pub enum DiskType { - NVMe, - VirtioBlock, -} - -/// Describes a virtual disk. -#[derive(Clone, Deserialize, Serialize, JsonSchema)] -pub struct Disk { - /// Unique identifier for this disk. - pub id: Uuid, - - /// Storage agents which implement networked block device servers. - pub storage_agents: StorageAgentDescription, - - /// Size of the disk (blocks). - pub block_count: u64, - - /// Block size (bytes). - pub block_size: u32, - - /// Storage interface. - pub interface: DiskType, -} - -#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)] -pub struct DiskRequest { - pub name: String, - pub slot: Slot, - pub read_only: bool, - pub device: String, - - // Crucible related opts - pub volume_construction_request: - crucible_client_types::VolumeConstructionRequest, -} - -/// A stable index which is translated by Propolis -/// into a PCI BDF, visible to the guest. -#[derive(Copy, Clone, Debug, Deserialize, Serialize, JsonSchema)] -pub struct Slot(pub u8); - -#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)] -pub struct NetworkInterfaceRequest { - pub interface_id: Uuid, - pub name: String, - pub slot: Slot, -} - #[derive(Deserialize, JsonSchema)] pub struct SnapshotRequestPathParams { - pub id: Uuid, + pub id: String, pub snapshot_id: Uuid, } #[derive(Deserialize, JsonSchema)] pub struct VCRRequestPathParams { - pub id: Uuid, + pub id: String, } #[derive(Deserialize, JsonSchema)] pub struct VolumeStatusPathParams { - pub id: Uuid, + pub id: String, } #[derive(Debug, Serialize, Deserialize, JsonSchema)] diff --git a/crates/propolis-server-config/Cargo.toml b/crates/propolis-config-toml/Cargo.toml similarity index 70% rename from crates/propolis-server-config/Cargo.toml rename to crates/propolis-config-toml/Cargo.toml index e5259e418..36c6e4ba0 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,12 @@ 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 + +[features] +falcon = [] diff --git a/crates/propolis-server-config/src/lib.rs b/crates/propolis-config-toml/src/lib.rs similarity index 94% rename from crates/propolis-server-config/src/lib.rs rename to crates/propolis-config-toml/src/lib.rs index 796683485..cf7f2b568 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); @@ -183,10 +178,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..e012420b3 --- /dev/null +++ b/crates/propolis-config-toml/src/spec.rs @@ -0,0 +1,403 @@ +// 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::HashMap, + str::{FromStr, ParseBoolError}, +}; + +use propolis_client::{ + types::{ + ComponentV0, FileStorageBackend, MigrationFailureInjector, NvmeDisk, + PciPciBridge, VirtioDisk, VirtioNetworkBackend, VirtioNic, + }, + PciPath, SpecKey, +}; +use thiserror::Error; + +#[cfg(feature = "falcon")] +use propolis_client::types::{ + DlpiNetworkBackend, P9fs, SoftNpuP9, SoftNpuPciPort, SoftNpuPort, +}; + +pub const MIGRATION_FAILURE_DEVICE_NAME: &str = "test-migration-failure"; + +#[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), + + #[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(Clone, Debug, Default)] +pub struct SpecConfig { + pub enable_pcie: bool, + pub components: HashMap, +} + +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( + SpecKey::Name(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), + ); + } + #[cfg(feature = "falcon")] + "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, + }), + ); + } + #[cfg(feature = "falcon")] + "softnpu-port" => { + let vnic_name = + device.get_string("vnic").ok_or_else(|| { + TomlToSpecError::NoVnicName(device_name.to_owned()) + })?; + + let backend_id = + SpecKey::Name(format!("{}:backend", device_id)); + + spec.components.insert( + device_id, + ComponentV0::SoftNpuPort(SoftNpuPort { + link_name: device_name.to_string(), + backend_id: backend_id.clone(), + }), + ); + + spec.components.insert( + backend_id, + ComponentV0::DlpiNetworkBackend(DlpiNetworkBackend { + vnic_name: vnic_name.to_owned(), + }), + ); + } + #[cfg(feature = "falcon")] + "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 }), + ); + } + #[cfg(feature = "falcon")] + "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( + SpecKey::Name(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_id = SpecKey::from( + 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_id.clone(); + Ok(( + match interface { + Interface::Virtio => { + ComponentV0::VirtioDisk(VirtioDisk { backend_id, pci_path }) + } + Interface::Nvme => { + ComponentV0::NvmeDisk(NvmeDisk { backend_id, 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 = SpecKey::Name(format!("{name}-backend")); + Ok(ParsedNic { + device_spec: VirtioNic { + backend_id: backend_id.clone(), + interface_id: uuid::Uuid::nil(), + pci_path, + }, + backend_spec: VirtioNetworkBackend { vnic_name: vnic_name.to_owned() }, + backend_id, + }) +} + +#[cfg(feature = "falcon")] +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..7bf325034 100644 --- a/lib/propolis-client/Cargo.toml +++ b/lib/propolis-client/Cargo.toml @@ -9,8 +9,10 @@ doctest = false [dependencies] async-trait.workspace = true base64.workspace = true +crucible-client-types.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..f19213b52 100644 --- a/lib/propolis-client/src/lib.rs +++ b/lib/propolis-client/src/lib.rs @@ -4,27 +4,76 @@ //! A client for the Propolis hypervisor frontend's server API. +pub use propolis_api_types::instance_spec::{PciPath, SpecKey}; + +// Re-export Crucible VCRs to give Omicron a way to refer to the same VCR type +// that Propolis uses. +pub use crucible_client_types::{CrucibleOpts, VolumeConstructionRequest}; + progenitor::generate_api!( spec = "../../openapi/propolis-server.json", interface = Builder, tags = Separate, + replace = { + PciPath = crate::PciPath, + SpecKey = crate::SpecKey, + }, patch = { // Some Crucible-related bits are re-exported through simulated // sled-agent and thus require JsonSchema BootOrderEntry = { derives = [schemars::JsonSchema] }, BootSettings = { derives = [Default, schemars::JsonSchema] }, CpuidEntry = { derives = [PartialEq, Eq, Copy] }, - DiskRequest = { derives = [schemars::JsonSchema] }, - VolumeConstructionRequest = { derives = [schemars::JsonSchema] }, - CrucibleOpts = { derives = [schemars::JsonSchema] }, Slot = { derives = [schemars::JsonSchema] }, - - PciPath = { derives = [ - Copy, Ord, Eq, PartialEq, PartialOrd - ] }, - InstanceMetadata = { derives = [ PartialEq ] }, } ); pub mod support; + +impl types::InstanceSpecV0 { + /// Returns an instance spec with the following configuration: + /// + /// - CPUs and memory set to their supplied values + /// - Serial devices named "com1", "com2", "com3", and "com4" attached to + /// the components + pub fn with_basic_config(cpus: u8, memory_mb: u64) -> Self { + use types::*; + Self { + board: Board { + chipset: Chipset::I440Fx(I440Fx { enable_pcie: false }), + cpuid: None, + cpus, + memory_mb, + }, + components: [ + ( + "com1".to_string(), + ComponentV0::SerialPort(SerialPort { + num: SerialPortNumber::Com1, + }), + ), + ( + "com2".to_string(), + ComponentV0::SerialPort(SerialPort { + num: SerialPortNumber::Com2, + }), + ), + ( + "com3".to_string(), + ComponentV0::SerialPort(SerialPort { + num: SerialPortNumber::Com3, + }), + ), + ( + "com4".to_string(), + ComponentV0::SerialPort(SerialPort { + num: SerialPortNumber::Com4, + }), + ), + ] + .into_iter() + .collect(), + } + } +} diff --git a/lib/propolis-client/src/support.rs b/lib/propolis-client/src/support.rs index bcf5f10bc..be1554f2f 100644 --- a/lib/propolis-client/src/support.rs +++ b/lib/propolis-client/src/support.rs @@ -18,27 +18,10 @@ 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::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 }) - } - } -} +use crate::{ + types::{Chipset, I440Fx}, + Client as PropolisClient, +}; impl Default for Chipset { fn default() -> Self { diff --git a/openapi/propolis-server.json b/openapi/propolis-server.json index 964b630c8..7b51acddc 100644 --- a/openapi/propolis-server.json +++ b/openapi/propolis-server.json @@ -74,8 +74,7 @@ "name": "id", "required": true, "schema": { - "type": "string", - "format": "uuid" + "type": "string" } }, { @@ -122,8 +121,7 @@ "name": "id", "required": true, "schema": { - "type": "string", - "format": "uuid" + "type": "string" } } ], @@ -157,8 +155,7 @@ "name": "id", "required": true, "schema": { - "type": "string", - "format": "uuid" + "type": "string" } } ], @@ -362,37 +359,6 @@ "$ref": "#/components/responses/Error" } } - }, - "put": { - "operationId": "instance_spec_ensure", - "requestBody": { - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/InstanceSpecEnsureRequest" - } - } - }, - "required": true - }, - "responses": { - "201": { - "description": "successful creation", - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/InstanceEnsureResponse" - } - } - } - }, - "4XX": { - "$ref": "#/components/responses/Error" - }, - "5XX": { - "$ref": "#/components/responses/Error" - } - } } }, "/instance/state": { @@ -521,13 +487,17 @@ "description": "An entry in the boot order stored in a [`BootSettings`] component.", "type": "object", "properties": { - "name": { - "description": "The name of another component in the spec that Propolis should try to boot from.\n\nCurrently, only disk device components are supported.", - "type": "string" + "component_id": { + "description": "The ID of another component in the spec that Propolis should try to boot from.\n\nCurrently, only disk device components are supported.", + "allOf": [ + { + "$ref": "#/components/schemas/SpecKey" + } + ] } }, "required": [ - "name" + "component_id" ] }, "BootSettings": { @@ -783,6 +753,25 @@ ], "additionalProperties": false }, + { + "type": "object", + "properties": { + "component": { + "$ref": "#/components/schemas/MigrationFailureInjector" + }, + "type": { + "type": "string", + "enum": [ + "MigrationFailureInjector" + ] + } + }, + "required": [ + "component", + "type" + ], + "additionalProperties": false + }, { "type": "object", "properties": { @@ -965,58 +954,6 @@ "Intel" ] }, - "CrucibleOpts": { - "type": "object", - "properties": { - "cert_pem": { - "nullable": true, - "type": "string" - }, - "control": { - "nullable": true, - "type": "string" - }, - "flush_timeout": { - "nullable": true, - "type": "number", - "format": "float" - }, - "id": { - "type": "string", - "format": "uuid" - }, - "key": { - "nullable": true, - "type": "string" - }, - "key_pem": { - "nullable": true, - "type": "string" - }, - "lossy": { - "type": "boolean" - }, - "read_only": { - "type": "boolean" - }, - "root_cert_pem": { - "nullable": true, - "type": "string" - }, - "target": { - "type": "array", - "items": { - "type": "string" - } - } - }, - "required": [ - "id", - "lossy", - "read_only", - "target" - ] - }, "CrucibleStorageBackend": { "description": "A Crucible storage backend.", "type": "object", @@ -1036,39 +973,12 @@ ], "additionalProperties": false }, - "DiskRequest": { - "type": "object", - "properties": { - "device": { - "type": "string" - }, - "name": { - "type": "string" - }, - "read_only": { - "type": "boolean" - }, - "slot": { - "$ref": "#/components/schemas/Slot" - }, - "volume_construction_request": { - "$ref": "#/components/schemas/VolumeConstructionRequest" - } - }, - "required": [ - "device", - "name", - "read_only", - "slot", - "volume_construction_request" - ] - }, "DlpiNetworkBackend": { - "description": "A network backend associated with a DLPI VNIC on the host.", + "description": "A network backend associated with a DLPI network device on the host.", "type": "object", "properties": { "vnic_name": { - "description": "The name of the VNIC to use as a backend.", + "description": "The name of the DLPI device to use as a backend.", "type": "string" } }, @@ -1147,61 +1057,16 @@ "InstanceEnsureRequest": { "type": "object", "properties": { - "boot_settings": { - "nullable": true, - "default": null, - "allOf": [ - { - "$ref": "#/components/schemas/BootSettings" - } - ] - }, - "cloud_init_bytes": { - "nullable": true, - "type": "string" - }, - "disks": { - "default": [], - "type": "array", - "items": { - "$ref": "#/components/schemas/DiskRequest" - } - }, - "memory": { - "description": "Size of memory allocated to the Instance, in MiB.", - "type": "integer", - "format": "uint64", - "minimum": 0 - }, - "migrate": { - "nullable": true, - "allOf": [ - { - "$ref": "#/components/schemas/InstanceMigrateInitiateRequest" - } - ] - }, - "nics": { - "default": [], - "type": "array", - "items": { - "$ref": "#/components/schemas/NetworkInterfaceRequest" - } + "init": { + "$ref": "#/components/schemas/InstanceInitializationMethod" }, "properties": { "$ref": "#/components/schemas/InstanceProperties" - }, - "vcpus": { - "description": "Number of vCPUs to be allocated to the Instance.", - "type": "integer", - "format": "uint8", - "minimum": 0 } }, "required": [ - "memory", - "properties", - "vcpus" + "init", + "properties" ] }, "InstanceEnsureResponse": { @@ -1228,6 +1093,85 @@ "instance" ] }, + "InstanceInitializationMethod": { + "description": "The mechanism to use to create a new Propolis VM.", + "oneOf": [ + { + "description": "Create a brand new VM with the devies specified in the supplied spec.", + "type": "object", + "properties": { + "method": { + "type": "string", + "enum": [ + "Spec" + ] + }, + "value": { + "type": "object", + "properties": { + "spec": { + "description": "The component manifest for the new VM.", + "allOf": [ + { + "$ref": "#/components/schemas/InstanceSpecV0" + } + ] + } + }, + "required": [ + "spec" + ] + } + }, + "required": [ + "method", + "value" + ] + }, + { + "description": "Initialize the VM via migration.", + "type": "object", + "properties": { + "method": { + "type": "string", + "enum": [ + "MigrationTarget" + ] + }, + "value": { + "type": "object", + "properties": { + "migration_id": { + "description": "The ID to assign to this migration attempt.", + "type": "string", + "format": "uuid" + }, + "replace_components": { + "description": "A list of components in the source VM's instance spec to replace.", + "type": "object", + "additionalProperties": { + "$ref": "#/components/schemas/ReplacementComponent" + } + }, + "src_addr": { + "description": "The address of the Propolis server that will serve as the migration source.", + "type": "string" + } + }, + "required": [ + "migration_id", + "replace_components", + "src_addr" + ] + } + }, + "required": [ + "method", + "value" + ] + } + ] + }, "InstanceMetadata": { "type": "object", "properties": { @@ -1264,27 +1208,6 @@ "sled_serial" ] }, - "InstanceMigrateInitiateRequest": { - "type": "object", - "properties": { - "migration_id": { - "type": "string", - "format": "uuid" - }, - "src_addr": { - "type": "string" - }, - "src_uuid": { - "type": "string", - "format": "uuid" - } - }, - "required": [ - "migration_id", - "src_addr", - "src_uuid" - ] - }, "InstanceMigrateInitiateResponse": { "type": "object", "properties": { @@ -1401,45 +1324,27 @@ "last_byte_offset" ] }, - "InstanceSpecEnsureRequest": { + "InstanceSpecGetResponse": { "type": "object", "properties": { - "instance_spec": { - "$ref": "#/components/schemas/VersionedInstanceSpec" + "properties": { + "$ref": "#/components/schemas/InstanceProperties" }, - "migrate": { + "spec": { "nullable": true, + "description": "The instance's component manifest, if it is known at this point. (Instances that initialize via live migration receive specs from their sources, so this field will be None for an instance that is still initializing via migration.)", "allOf": [ { - "$ref": "#/components/schemas/InstanceMigrateInitiateRequest" + "$ref": "#/components/schemas/VersionedInstanceSpec" } ] }, - "properties": { - "$ref": "#/components/schemas/InstanceProperties" - } - }, - "required": [ - "instance_spec", - "properties" - ] - }, - "InstanceSpecGetResponse": { - "type": "object", - "properties": { - "properties": { - "$ref": "#/components/schemas/InstanceProperties" - }, - "spec": { - "$ref": "#/components/schemas/VersionedInstanceSpec" - }, "state": { "$ref": "#/components/schemas/InstanceState" } }, "required": [ "properties", - "spec", "state" ] }, @@ -1523,18 +1428,37 @@ "InstanceVCRReplace": { "type": "object", "properties": { - "name": { - "type": "string" - }, "vcr_json": { "type": "string" } }, "required": [ - "name", "vcr_json" ] }, + "MigrationFailureInjector": { + "description": "Describes a synthetic device that registers for VM lifecycle notifications and returns errors during attempts to migrate.\n\nThis is only supported by Propolis servers compiled without the `omicron-build` feature.", + "type": "object", + "properties": { + "fail_exports": { + "description": "The number of times this device should fail requests to export state.", + "type": "integer", + "format": "uint32", + "minimum": 0 + }, + "fail_imports": { + "description": "The number of times this device should fail requests to import state.", + "type": "integer", + "format": "uint32", + "minimum": 0 + } + }, + "required": [ + "fail_exports", + "fail_imports" + ], + "additionalProperties": false + }, "MigrationState": { "type": "string", "enum": [ @@ -1550,33 +1474,17 @@ "Error" ] }, - "NetworkInterfaceRequest": { - "type": "object", - "properties": { - "interface_id": { - "type": "string", - "format": "uuid" - }, - "name": { - "type": "string" - }, - "slot": { - "$ref": "#/components/schemas/Slot" - } - }, - "required": [ - "interface_id", - "name", - "slot" - ] - }, "NvmeDisk": { "description": "A disk that presents an NVMe interface to the guest.", "type": "object", "properties": { - "backend_name": { + "backend_id": { "description": "The name of the disk's backend component.", - "type": "string" + "allOf": [ + { + "$ref": "#/components/schemas/SpecKey" + } + ] }, "pci_path": { "description": "The PCI bus/device/function at which this disk should be attached.", @@ -1588,7 +1496,7 @@ } }, "required": [ - "backend_name", + "backend_id", "pci_path" ], "additionalProperties": false @@ -1702,6 +1610,124 @@ "vcr_matches" ] }, + "ReplacementComponent": { + "oneOf": [ + { + "type": "object", + "properties": { + "component": { + "$ref": "#/components/schemas/MigrationFailureInjector" + }, + "type": { + "type": "string", + "enum": [ + "MigrationFailureInjector" + ] + } + }, + "required": [ + "component", + "type" + ], + "additionalProperties": false + }, + { + "type": "object", + "properties": { + "component": { + "$ref": "#/components/schemas/CrucibleStorageBackend" + }, + "type": { + "type": "string", + "enum": [ + "CrucibleStorageBackend" + ] + } + }, + "required": [ + "component", + "type" + ], + "additionalProperties": false + }, + { + "type": "object", + "properties": { + "component": { + "$ref": "#/components/schemas/FileStorageBackend" + }, + "type": { + "type": "string", + "enum": [ + "FileStorageBackend" + ] + } + }, + "required": [ + "component", + "type" + ], + "additionalProperties": false + }, + { + "type": "object", + "properties": { + "component": { + "$ref": "#/components/schemas/BlobStorageBackend" + }, + "type": { + "type": "string", + "enum": [ + "BlobStorageBackend" + ] + } + }, + "required": [ + "component", + "type" + ], + "additionalProperties": false + }, + { + "type": "object", + "properties": { + "component": { + "$ref": "#/components/schemas/VirtioNetworkBackend" + }, + "type": { + "type": "string", + "enum": [ + "VirtioNetworkBackend" + ] + } + }, + "required": [ + "component", + "type" + ], + "additionalProperties": false + }, + { + "type": "object", + "properties": { + "component": { + "$ref": "#/components/schemas/DlpiNetworkBackend" + }, + "type": { + "type": "string", + "enum": [ + "DlpiNetworkBackend" + ] + } + }, + "required": [ + "component", + "type" + ], + "additionalProperties": false + } + ] + }, "SerialPort": { "description": "A serial port device.", "type": "object", @@ -1730,12 +1756,6 @@ "com4" ] }, - "Slot": { - "description": "A stable index which is translated by Propolis into a PCI BDF, visible to the guest.", - "type": "integer", - "format": "uint8", - "minimum": 0 - }, "SoftNpuP9": { "description": "Describes a PCI device that shares host files with the guest using the P9 protocol.\n\nThis is only supported by Propolis servers compiled with the `falcon` feature.", "type": "object", @@ -1773,24 +1793,58 @@ "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 data link port on 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.", - "type": "string" + "backend_id": { + "description": "The name of the DLPI backend with which to associate this port.", + "allOf": [ + { + "$ref": "#/components/schemas/SpecKey" + } + ] }, - "name": { - "description": "The name of the SoftNpu port.", + "link_name": { + "description": "The data link name for this port.", "type": "string" } }, "required": [ - "backend_name", - "name" + "backend_id", + "link_name" ], "additionalProperties": false }, + "SpecKey": { + "description": "A key identifying a component in an instance spec.", + "oneOf": [ + { + "type": "object", + "properties": { + "Uuid": { + "type": "string", + "format": "uuid" + } + }, + "required": [ + "Uuid" + ], + "additionalProperties": false + }, + { + "type": "object", + "properties": { + "Name": { + "type": "string" + } + }, + "required": [ + "Name" + ], + "additionalProperties": false + } + ] + }, "VersionedInstanceSpec": { "description": "A versioned instance spec.", "oneOf": [ @@ -1819,9 +1873,13 @@ "description": "A disk that presents a virtio-block interface to the guest.", "type": "object", "properties": { - "backend_name": { + "backend_id": { "description": "The name of the disk's backend component.", - "type": "string" + "allOf": [ + { + "$ref": "#/components/schemas/SpecKey" + } + ] }, "pci_path": { "description": "The PCI bus/device/function at which this disk should be attached.", @@ -1833,7 +1891,7 @@ } }, "required": [ - "backend_name", + "backend_id", "pci_path" ], "additionalProperties": false @@ -1856,9 +1914,13 @@ "description": "A network card that presents a virtio-net interface to the guest.", "type": "object", "properties": { - "backend_name": { - "description": "The name of the device's backend.", - "type": "string" + "backend_id": { + "description": "The ID of the device's backend.", + "allOf": [ + { + "$ref": "#/components/schemas/SpecKey" + } + ] }, "interface_id": { "description": "A caller-defined correlation identifier for this interface. If Propolis is configured to collect network interface kstats in its Oximeter metrics, the metric series for this interface will be associated with this identifier.", @@ -1875,156 +1937,12 @@ } }, "required": [ - "backend_name", + "backend_id", "interface_id", "pci_path" ], "additionalProperties": false }, - "VolumeConstructionRequest": { - "oneOf": [ - { - "type": "object", - "properties": { - "block_size": { - "type": "integer", - "format": "uint64", - "minimum": 0 - }, - "id": { - "type": "string", - "format": "uuid" - }, - "read_only_parent": { - "nullable": true, - "allOf": [ - { - "$ref": "#/components/schemas/VolumeConstructionRequest" - } - ] - }, - "sub_volumes": { - "type": "array", - "items": { - "$ref": "#/components/schemas/VolumeConstructionRequest" - } - }, - "type": { - "type": "string", - "enum": [ - "volume" - ] - } - }, - "required": [ - "block_size", - "id", - "sub_volumes", - "type" - ] - }, - { - "type": "object", - "properties": { - "block_size": { - "type": "integer", - "format": "uint64", - "minimum": 0 - }, - "id": { - "type": "string", - "format": "uuid" - }, - "type": { - "type": "string", - "enum": [ - "url" - ] - }, - "url": { - "type": "string" - } - }, - "required": [ - "block_size", - "id", - "type", - "url" - ] - }, - { - "type": "object", - "properties": { - "block_size": { - "type": "integer", - "format": "uint64", - "minimum": 0 - }, - "blocks_per_extent": { - "type": "integer", - "format": "uint64", - "minimum": 0 - }, - "extent_count": { - "type": "integer", - "format": "uint32", - "minimum": 0 - }, - "gen": { - "type": "integer", - "format": "uint64", - "minimum": 0 - }, - "opts": { - "$ref": "#/components/schemas/CrucibleOpts" - }, - "type": { - "type": "string", - "enum": [ - "region" - ] - } - }, - "required": [ - "block_size", - "blocks_per_extent", - "extent_count", - "gen", - "opts", - "type" - ] - }, - { - "type": "object", - "properties": { - "block_size": { - "type": "integer", - "format": "uint64", - "minimum": 0 - }, - "id": { - "type": "string", - "format": "uuid" - }, - "path": { - "type": "string" - }, - "type": { - "type": "string", - "enum": [ - "file" - ] - } - }, - "required": [ - "block_size", - "id", - "path", - "type" - ] - } - ] - }, "VolumeStatus": { "type": "object", "properties": { 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..7e7cf9d8d 100644 --- a/phd-tests/framework/Cargo.toml +++ b/phd-tests/framework/Cargo.toml @@ -16,6 +16,7 @@ bhyve_api.workspace = true camino = { workspace = true, features = ["serde1"] } cfg-if.workspace = true cpuid_utils.workspace = true +crucible-client-types.workspace = true errno.workspace = true fatfs.workspace = true futures.workspace = true @@ -24,7 +25,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/disk/crucible.rs b/phd-tests/framework/src/disk/crucible.rs index 244f87309..202a5c36e 100644 --- a/phd-tests/framework/src/disk/crucible.rs +++ b/phd-tests/framework/src/disk/crucible.rs @@ -12,10 +12,8 @@ use std::{ }; use anyhow::Context; -use propolis_client::types::{ - ComponentV0, CrucibleOpts, CrucibleStorageBackend, - VolumeConstructionRequest, -}; +use crucible_client_types::{CrucibleOpts, VolumeConstructionRequest}; +use propolis_client::types::{ComponentV0, CrucibleStorageBackend}; use rand::{rngs::StdRng, RngCore, SeedableRng}; use tracing::{error, info}; use uuid::Uuid; @@ -288,11 +286,8 @@ impl super::DiskConfig for CrucibleDisk { fn backend_spec(&self) -> ComponentV0 { let gen = self.generation.load(Ordering::Relaxed); - let downstairs_addrs = self - .downstairs_instances - .iter() - .map(|ds| ds.address.to_string()) - .collect(); + let downstairs_addrs = + self.downstairs_instances.iter().map(|ds| ds.address).collect(); let vcr = VolumeConstructionRequest::Volume { id: self.id, diff --git a/phd-tests/framework/src/test_vm/config.rs b/phd-tests/framework/src/test_vm/config.rs index b5a288dd7..a8e86c522 100644 --- a/phd-tests/framework/src/test_vm/config.rs +++ b/phd-tests/framework/src/test_vm/config.rs @@ -2,15 +2,18 @@ // 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/. -use std::collections::BTreeMap; 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, NvmeDisk, - PciPath, SerialPort, SerialPortNumber, VirtioDisk, +use propolis_client::types::MigrationFailureInjector; +use propolis_client::{ + types::{ + Board, BootOrderEntry, BootSettings, Chipset, ComponentV0, Cpuid, + CpuidEntry, CpuidVendor, InstanceMetadata, InstanceSpecV0, NvmeDisk, + SerialPort, SerialPortNumber, VirtioDisk, + }, + PciPath, SpecKey, }; use uuid::Uuid; @@ -50,8 +53,8 @@ pub struct VmConfig<'dr> { cpuid: Option>, bootrom_artifact: String, boot_order: Option>, + migration_failure: Option, disks: Vec>, - devices: BTreeMap, } const MIGRATION_FAILURE_DEVICE: &str = "test-migration-failure"; @@ -71,8 +74,8 @@ impl<'dr> VmConfig<'dr> { cpuid: None, bootrom_artifact: bootrom.to_owned(), boot_order: None, + migration_failure: None, disks: Vec::new(), - devices: BTreeMap::new(), }; config.boot_disk( @@ -111,20 +114,24 @@ impl<'dr> VmConfig<'dr> { } pub fn fail_migration_exports(&mut self, exports: u32) -> &mut Self { - self.devices - .entry(MIGRATION_FAILURE_DEVICE.to_owned()) - .or_insert_with(default_migration_failure_device) - .options - .insert("fail_exports".to_string(), exports.into()); + let injector = + self.migration_failure.get_or_insert(MigrationFailureInjector { + fail_exports: 0, + fail_imports: 0, + }); + + injector.fail_exports = exports; self } pub fn fail_migration_imports(&mut self, imports: u32) -> &mut Self { - self.devices - .entry(MIGRATION_FAILURE_DEVICE.to_owned()) - .or_insert_with(default_migration_failure_device) - .options - .insert("fail_imports".to_string(), imports.into()); + let injector = + self.migration_failure.get_or_insert(MigrationFailureInjector { + fail_exports: 0, + fail_imports: 0, + }); + + injector.fail_imports = imports; self } @@ -198,22 +205,25 @@ impl<'dr> VmConfig<'dr> { &self, framework: &Framework, ) -> anyhow::Result { - // Figure out where the bootrom is and generate the serialized contents - // of a Propolis server config TOML that points to it. + // Exhaustively destructure to break the build if a new field is added + // but not considered here. + let VmConfig { + vm_name, + cpus, + memory_mib, + cpuid, + bootrom_artifact, + boot_order, + migration_failure, + disks, + } = &self; + let bootrom = framework .artifact_store - .get_bootrom(&self.bootrom_artifact) + .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(), - devices: self.devices.clone(), - ..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. // @@ -232,7 +242,7 @@ impl<'dr> VmConfig<'dr> { .iter() .find(|d| d.name == "boot-disk") .or_else(|| { - if let Some(boot_order) = self.boot_order.as_ref() { + if let Some(boot_order) = boot_order.as_ref() { boot_order.first().and_then(|name| { self.disks.iter().find(|d| &d.name == name) }) @@ -240,7 +250,7 @@ impl<'dr> VmConfig<'dr> { None } }) - .or_else(|| self.disks.first()) + .or_else(|| disks.first()) .expect("VM config includes at least one disk"); // XXX: assuming all bootable images are equivalent to the first, or at @@ -256,7 +266,7 @@ impl<'dr> VmConfig<'dr> { .context("getting guest OS kind for boot disk")?; let mut disk_handles = Vec::new(); - for disk in self.disks.iter() { + for disk in disks.iter() { disk_handles.push( make_disk(disk.name.to_owned(), framework, disk) .await @@ -274,10 +284,10 @@ impl<'dr> VmConfig<'dr> { let mut spec = InstanceSpecV0 { board: Board { - cpus: self.cpus, - memory_mb: self.memory_mib, + cpus: *cpus, + memory_mb: *memory_mib, chipset: Chipset::default(), - cpuid: self.cpuid.as_ref().map(|entries| Cpuid { + cpuid: cpuid.as_ref().map(|entries| Cpuid { entries: entries.clone(), vendor: match host_vendor { cpuid_utils::CpuidVendor::Amd => CpuidVendor::Amd, @@ -292,19 +302,21 @@ impl<'dr> VmConfig<'dr> { // elements for all of them. This assumes the disk handles were created // in the correct order: boot disk first, then in the data disks' // iteration order. - let all_disks = self.disks.iter().zip(disk_handles.iter()); + let all_disks = disks.iter().zip(disk_handles.iter()); for (req, hdl) in all_disks { let pci_path = PciPath::new(0, req.pci_device_num, 0).unwrap(); let backend_spec = hdl.backend_spec(); let device_name = hdl.device_name().clone(); - let backend_name = device_name.clone().into_backend_name(); + let backend_id = SpecKey::Name( + device_name.clone().into_backend_name().into_string(), + ); let device_spec = match req.interface { DiskInterface::Virtio => ComponentV0::VirtioDisk(VirtioDisk { - backend_name: backend_name.clone().into_string(), + backend_id: backend_id.clone(), pci_path, }), DiskInterface::Nvme => ComponentV0::NvmeDisk(NvmeDisk { - backend_name: backend_name.clone().into_string(), + backend_id: backend_id.clone(), pci_path, }), }; @@ -312,9 +324,8 @@ impl<'dr> VmConfig<'dr> { let _old = spec.components.insert(device_name.into_string(), device_spec); assert!(_old.is_none()); - let _old = spec - .components - .insert(backend_name.into_string(), backend_spec); + let _old = + spec.components.insert(backend_id.to_string(), backend_spec); assert!(_old.is_none()); } @@ -324,19 +335,31 @@ impl<'dr> VmConfig<'dr> { ); assert!(_old.is_none()); - if let Some(boot_order) = self.boot_order.as_ref() { + if let Some(boot_order) = boot_order.as_ref() { let _old = spec.components.insert( "boot-settings".to_string(), ComponentV0::BootSettings(BootSettings { order: boot_order .iter() - .map(|item| BootOrderEntry { name: item.to_string() }) + .map(|item| BootOrderEntry { + component_id: SpecKey::from(item.to_string()), + }) .collect(), }), ); assert!(_old.is_none()); } + if let Some(migration_failure) = migration_failure { + let _old = spec.components.insert( + MIGRATION_FAILURE_DEVICE.to_owned(), + ComponentV0::MigrationFailureInjector( + migration_failure.clone(), + ), + ); + assert!(_old.is_none()); + } + // Generate random identifiers for this instance's timeseries metadata. let sled_id = Uuid::new_v4(); let metadata = InstanceMetadata { @@ -349,11 +372,11 @@ impl<'dr> VmConfig<'dr> { }; Ok(VmSpec { - vm_name: self.vm_name.clone(), + vm_name: vm_name.clone(), instance_spec: spec, disk_handles, guest_os_kind, - config_toml_contents, + bootrom_path: bootrom, metadata, }) } @@ -400,10 +423,3 @@ async fn make_disk<'req>( as Arc, }) } - -fn default_migration_failure_device() -> propolis_server_config::Device { - propolis_server_config::Device { - driver: MIGRATION_FAILURE_DEVICE.to_owned(), - options: Default::default(), - } -} diff --git a/phd-tests/framework/src/test_vm/mod.rs b/phd-tests/framework/src/test_vm/mod.rs index ca2fef4f9..4412487e0 100644 --- a/phd-tests/framework/src/test_vm/mod.rs +++ b/phd-tests/framework/src/test_vm/mod.rs @@ -5,7 +5,10 @@ //! 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::{ + collections::HashMap, fmt::Debug, net::SocketAddr, sync::Arc, + time::Duration, +}; use crate::{ guest_os::{ @@ -24,11 +27,11 @@ use core::result::Result as StdResult; use propolis_client::{ support::{InstanceSerialConsoleHelper, WSClientOffset}, types::{ - InstanceEnsureRequest, InstanceGetResponse, - InstanceMigrateInitiateRequest, InstanceMigrateStatusResponse, + ComponentV0, InstanceEnsureRequest, InstanceGetResponse, + InstanceInitializationMethod, InstanceMigrateStatusResponse, InstanceProperties, InstanceSerialConsoleHistoryResponse, - InstanceSpecEnsureRequest, InstanceSpecGetResponse, InstanceState, - InstanceStateRequested, MigrationState, VersionedInstanceSpec, + InstanceSpecGetResponse, InstanceState, InstanceStateRequested, + MigrationState, ReplacementComponent, }, }; use propolis_client::{Client, ResponseValue}; @@ -66,6 +69,15 @@ pub enum VmStateError { InstanceAlreadyEnsured, } +type ReplacementComponents = HashMap; + +#[derive(Clone, Debug)] +struct MigrationInfo { + migration_id: Uuid, + src_addr: SocketAddr, + replace_components: ReplacementComponents, +} + /// Specifies the timeout to apply to an attempt to migrate. pub enum MigrationTimeout { /// Time out after the specified duration. @@ -128,15 +140,6 @@ enum InstanceConsoleSource<'a> { InheritFrom(&'a TestVm), } -/// Specifies the propolis-server interface to use when starting a VM. -enum InstanceEnsureApi { - /// Use the `instance_spec_ensure` interface. - SpecEnsure, - - /// Use the `instance_ensure` interface. - Ensure, -} - enum VmState { New, Ensured { serial: SerialConsole }, @@ -227,33 +230,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)); @@ -292,18 +274,11 @@ impl TestVm { #[instrument(skip_all, fields(vm = self.spec.vm_name, vm_id = %self.id))] async fn instance_ensure_internal<'a>( &self, - api: InstanceEnsureApi, - migrate: Option, + migrate: Option, console_source: InstanceConsoleSource<'a>, ) -> Result { - let (vcpus, memory_mib) = match self.state { - VmState::New => ( - self.spec.instance_spec.board.cpus, - self.spec.instance_spec.board.memory_mb, - ), - VmState::Ensured { .. } => { - return Err(VmStateError::InstanceAlreadyEnsured.into()) - } + if let VmState::Ensured { .. } = self.state { + return Err(VmStateError::InstanceAlreadyEnsured.into()); }; let properties = InstanceProperties { @@ -313,13 +288,21 @@ impl TestVm { description: "Pheidippides-managed VM".to_string(), }; - // The non-spec ensure interface requires a set of `DiskRequest` - // structures to specify disks. Create those once and clone them if the - // ensure call needs to be retried. - let disk_reqs = if let InstanceEnsureApi::Ensure = api { - Some(self.spec.make_disk_requests()?) - } else { - None + let ensure_req = match migrate { + None => InstanceEnsureRequest { + properties: properties.clone(), + init: InstanceInitializationMethod::Spec { + spec: self.spec.instance_spec.clone(), + }, + }, + Some(info) => InstanceEnsureRequest { + properties: properties.clone(), + init: InstanceInitializationMethod::MigrationTarget { + migration_id: info.migration_id, + replace_components: info.replace_components, + src_addr: info.src_addr.to_string(), + }, + }, }; // There is a brief period where the Propolis server process has begun @@ -331,39 +314,8 @@ impl TestVm { // it's possible to create a boxed future that abstracts over the // caller's chosen endpoint. let ensure_fn = || async { - let result = match api { - InstanceEnsureApi::SpecEnsure => { - let versioned_spec = VersionedInstanceSpec::V0( - self.spec.instance_spec.clone(), - ); - - let ensure_req = InstanceSpecEnsureRequest { - properties: properties.clone(), - instance_spec: versioned_spec, - migrate: migrate.clone(), - }; - - self.client - .instance_spec_ensure() - .body(&ensure_req) - .send() - .await - } - InstanceEnsureApi::Ensure => { - let ensure_req = InstanceEnsureRequest { - cloud_init_bytes: None, - vcpus, - memory: memory_mib, - disks: disk_reqs.clone().unwrap(), - migrate: migrate.clone(), - nics: vec![], - boot_settings: None, - properties: properties.clone(), - }; - - self.client.instance_ensure().body(&ensure_req).send().await - } - }; + let result = + self.client.instance_ensure().body(&ensure_req).send().await; if let Err(e) = result { match e { propolis_client::Error::CommunicationError(_) => { @@ -454,29 +406,7 @@ impl TestVm { match self.state { VmState::New => { let console = self - .instance_ensure_internal( - InstanceEnsureApi::SpecEnsure, - None, - InstanceConsoleSource::New, - ) - .await?; - self.state = VmState::Ensured { serial: console }; - } - VmState::Ensured { .. } => {} - } - - Ok(()) - } - - pub async fn instance_ensure_using_api_request(&mut self) -> Result<()> { - match self.state { - VmState::New => { - let console = self - .instance_ensure_internal( - InstanceEnsureApi::Ensure, - None, - InstanceConsoleSource::New, - ) + .instance_ensure_internal(None, InstanceConsoleSource::New) .await?; self.state = VmState::Ensured { serial: console }; } @@ -579,11 +509,11 @@ impl TestVm { let serial = self .instance_ensure_internal( - InstanceEnsureApi::SpecEnsure, - Some(InstanceMigrateInitiateRequest { + Some(MigrationInfo { migration_id, - src_addr: server_addr.to_string(), - src_uuid: Uuid::default(), + src_addr: SocketAddr::V4(server_addr), + replace_components: self + .generate_replacement_components(), }), InstanceConsoleSource::InheritFrom(source), ) @@ -637,6 +567,26 @@ impl TestVm { } } + fn generate_replacement_components(&self) -> ReplacementComponents { + let mut map = ReplacementComponents::new(); + for (id, comp) in &self.spec.instance_spec.components { + if let ComponentV0::MigrationFailureInjector(inj) = comp { + map.insert( + id.clone(), + ReplacementComponent::MigrationFailureInjector(inj.clone()), + ); + } + if let ComponentV0::CrucibleStorageBackend(be) = comp { + map.insert( + id.clone(), + ReplacementComponent::CrucibleStorageBackend(be.clone()), + ); + } + } + + map + } + pub async fn get_migration_state( &self, ) -> Result { diff --git a/phd-tests/framework/src/test_vm/server.rs b/phd-tests/framework/src/test_vm/server.rs index 92f5fb18b..f7b57429d 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..573537c08 100644 --- a/phd-tests/framework/src/test_vm/spec.rs +++ b/phd-tests/framework/src/test_vm/spec.rs @@ -8,9 +8,8 @@ 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, InstanceMetadata, InstanceSpecV0}; use uuid::Uuid; /// The set of objects needed to start and run a guest in a `TestVm`. @@ -27,8 +26,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, @@ -74,80 +73,4 @@ impl VmSpec { self.metadata.sled_id = id; self.metadata.sled_serial = id.to_string(); } - - /// Generates a set of [`propolis_client::types::DiskRequest`] structures - /// corresponding to the disks in this VM spec. - /// - /// All of the disks in the spec must be Crucible disks. If one is not, this - /// routine returns an error. - pub(crate) fn make_disk_requests( - &self, - ) -> anyhow::Result> { - struct DeviceInfo<'a> { - backend_name: &'a str, - interface: &'static str, - slot: Slot, - } - - fn convert_to_slot(pci_path: PciPath) -> anyhow::Result { - match pci_path.device { - dev @ 0x10..=0x17 => Ok(Slot(dev - 0x10)), - _ => Err(anyhow::anyhow!( - "PCI device number {} out of range", - pci_path.device - )), - } - } - - fn get_device_info(device: &ComponentV0) -> anyhow::Result { - match device { - ComponentV0::VirtioDisk(d) => Ok(DeviceInfo { - backend_name: &d.backend_name, - interface: "virtio", - slot: convert_to_slot(d.pci_path)?, - }), - ComponentV0::NvmeDisk(d) => Ok(DeviceInfo { - backend_name: &d.backend_name, - interface: "nvme", - slot: convert_to_slot(d.pci_path)?, - }), - _ => { - panic!("asked to get device info for a non-storage device") - } - } - } - - let mut reqs = vec![]; - for (name, device) in - self.instance_spec.components.iter().filter(|(_, c)| { - matches!( - c, - ComponentV0::VirtioDisk(_) | ComponentV0::NvmeDisk(_) - ) - }) - { - let info = get_device_info(device)?; - let backend = self - .instance_spec - .components - .get(info.backend_name) - .expect("storage device should have a matching backend"); - - let ComponentV0::CrucibleStorageBackend(backend) = backend else { - anyhow::bail!("disk {name} does not have a Crucible backend"); - }; - - reqs.push(DiskRequest { - device: info.interface.to_owned(), - name: name.clone(), - read_only: backend.readonly, - slot: info.slot, - volume_construction_request: serde_json::from_str( - &backend.request_json, - )?, - }) - } - - Ok(reqs) - } } diff --git a/phd-tests/tests/src/cpuid.rs b/phd-tests/tests/src/cpuid.rs index 3ec773761..056546173 100644 --- a/phd-tests/tests/src/cpuid.rs +++ b/phd-tests/tests/src/cpuid.rs @@ -35,7 +35,7 @@ async fn cpuid_instance_spec_round_trip_test(ctx: &Framework) { let spec_get_response = vm.get_spec().await?; let propolis_client::types::VersionedInstanceSpec::V0(spec) = - spec_get_response.spec; + spec_get_response.spec.expect("launched VM should have a spec"); let cpuid = spec.board.cpuid.expect("board should have explicit CPUID"); assert_eq!(cpuid.entries.len(), entries.len()); diff --git a/phd-tests/tests/src/ensure_api.rs b/phd-tests/tests/src/ensure_api.rs deleted file mode 100644 index 8407a1360..000000000 --- a/phd-tests/tests/src/ensure_api.rs +++ /dev/null @@ -1,33 +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/. - -//! Tests that explicitly exercise the `instance_ensure` form of the VM start -//! API. - -use phd_framework::{ - disk::BlockSize, - test_vm::{DiskBackend, DiskInterface}, -}; -use phd_testcase::*; - -#[phd_testcase] -async fn instance_ensure_api_test(ctx: &Framework) { - if !ctx.crucible_enabled() { - phd_skip!("test requires Crucible to be enabled"); - } - - let mut config = ctx.vm_config_builder("instance_ensure_api_test"); - config.boot_disk( - ctx.default_guest_os_artifact(), - DiskInterface::Nvme, - DiskBackend::Crucible { - min_disk_size_gib: 10, - block_size: BlockSize::Bytes512, - }, - 0x10, - ); - - let mut vm = ctx.spawn_vm(&config, None).await?; - vm.instance_ensure_using_api_request().await?; -} diff --git a/phd-tests/tests/src/lib.rs b/phd-tests/tests/src/lib.rs index e2f872f11..b76f0bec1 100644 --- a/phd-tests/tests/src/lib.rs +++ b/phd-tests/tests/src/lib.rs @@ -8,7 +8,6 @@ mod boot_order; mod cpuid; mod crucible; mod disk; -mod ensure_api; mod framework; mod hw; mod migrate; diff --git a/phd-tests/tests/src/migrate.rs b/phd-tests/tests/src/migrate.rs index 78438be79..a3506d107 100644 --- a/phd-tests/tests/src/migrate.rs +++ b/phd-tests/tests/src/migrate.rs @@ -274,56 +274,6 @@ mod running_process { } } -#[phd_testcase] -async fn incompatible_vms(ctx: &Framework) { - let mut builders = vec![ - ctx.vm_config_builder("migration_incompatible_target_1"), - ctx.vm_config_builder("migration_incompatible_target_2"), - ]; - - builders[0].cpus(8); - builders[1].memory_mib(1024); - - for (i, cfg) in builders.into_iter().enumerate() { - let mut source = ctx - .spawn_vm( - ctx.vm_config_builder(&format!( - "migration_incompatible_source_{}", - i - )) - .cpus(4) - .memory_mib(512), - None, - ) - .await?; - - source.launch().await?; - let mut target = ctx.spawn_vm(&cfg, None).await?; - - let migration_id = Uuid::new_v4(); - assert!(target - .migrate_from(&source, migration_id, MigrationTimeout::default()) - .await - .is_err()); - - let src_migration_state = source - .get_migration_state() - .await? - .migration_out - .expect("source should have a migration-out status") - .state; - assert_eq!(src_migration_state, MigrationState::Error); - - let target_migration_state = target - .get_migration_state() - .await? - .migration_in - .expect("target should have a migration-in status") - .state; - assert_eq!(target_migration_state, MigrationState::Error); - } -} - #[phd_testcase] async fn multiple_migrations(ctx: &Framework) { let mut vm0 = ctx.spawn_default_vm("multiple_migrations_0").await?; diff --git a/phd-tests/tests/src/smoke.rs b/phd-tests/tests/src/smoke.rs index 592145b60..e139836a6 100644 --- a/phd-tests/tests/src/smoke.rs +++ b/phd-tests/tests/src/smoke.rs @@ -47,7 +47,7 @@ async fn instance_spec_get_test(ctx: &Framework) { let spec_get_response = vm.get_spec().await?; let propolis_client::types::VersionedInstanceSpec::V0(spec) = - spec_get_response.spec; + spec_get_response.spec.expect("launched VM should have a spec"); assert_eq!(spec.board.cpus, 4); assert_eq!(spec.board.memory_mb, 3072); }