diff --git a/Cargo.lock b/Cargo.lock index 88819d0cb..5627f7451 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4298,6 +4298,7 @@ version = "0.1.0" dependencies = [ "async-trait", "base64 0.21.7", + "crucible-client-types", "futures", "progenitor", "propolis_api_types", 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 9603fb882..bd245b86b 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, o, Logger}; +use slog::{error, o, Logger}; use thiserror::Error; use tokio::sync::{watch, Mutex}; use tokio_tungstenite::tungstenite::protocol::{Role, WebSocketConfig}; @@ -22,10 +20,7 @@ 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 { @@ -141,12 +136,7 @@ async fn instance_ensure( ) -> 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; // 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/src/main.rs b/bin/propolis-cli/src/main.rs index 78c8069ec..543bb6b82 100644 --- a/bin/propolis-cli/src/main.rs +++ b/bin/propolis-cli/src/main.rs @@ -2,6 +2,7 @@ // 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::HashMap; use std::fs::File; use std::io::BufReader; use std::path::{Path, PathBuf}; @@ -17,13 +18,14 @@ use futures::{future, SinkExt}; use newtype_uuid::{GenericUuid, TypedUuid, TypedUuidKind, TypedUuidTag}; use propolis_client::types::{ BlobStorageBackend, Board, Chipset, ComponentV0, CrucibleStorageBackend, - I440Fx, InstanceMetadata, InstanceSpecEnsureRequest, - InstanceSpecGetResponse, InstanceSpecStatus, InstanceSpecV0, NvmeDisk, - QemuPvpanic, SerialPort, SerialPortNumber, VersionedInstanceSpec, + I440Fx, InstanceEnsureRequest, InstanceInitializationMethod, + InstanceMetadata, InstanceSpecGetResponse, InstanceSpecV0, NvmeDisk, + QemuPvpanic, ReplacementComponent, SerialPort, SerialPortNumber, VirtioDisk, }; use propolis_client::PciPath; use propolis_config_toml::spec::SpecConfig; +use serde::{Deserialize, Serialize}; use slog::{o, Drain, Level, Logger}; use tokio::io::{AsyncReadExt, AsyncWriteExt}; use tokio_tungstenite::tungstenite::{ @@ -35,8 +37,8 @@ use uuid::Uuid; use propolis_client::{ support::{InstanceSerialConsoleHelper, WSClientOffset}, types::{ - DiskRequest, InstanceMigrateInitiateRequest, InstanceProperties, - InstanceStateRequested, InstanceVcrReplace, MigrationState, + InstanceProperties, InstanceStateRequested, InstanceVcrReplace, + MigrationState, }, Client, }; @@ -199,40 +201,67 @@ fn add_component_to_spec( Ok(()) } -fn add_disk_request_to_spec( - spec: &mut InstanceSpecV0, - req: &DiskRequest, -) -> anyhow::Result<()> { - let slot = req.slot.0 + 0x10; - let backend_name = format!("{}-backend", req.name); - let pci_path = PciPath::new(0, slot, 0) - .with_context(|| format!("processing disk request {:?}", req.name))?; - let device_spec = match req.device.as_ref() { - "virtio" => ComponentV0::VirtioDisk(VirtioDisk { - backend_name: backend_name.clone(), - pci_path, - }), - "nvme" => ComponentV0::NvmeDisk(NvmeDisk { - backend_name: backend_name.clone(), - pci_path, - }), - _ => anyhow::bail!( - "invalid device type in disk request: {:?}", - req.device - ), - }; +/// A legacy Propolis API disk request, preserved here for compatibility with +/// the `--crucible-disks` option. +#[derive(Clone, Debug, Deserialize, Serialize)] +struct DiskRequest { + name: String, + slot: u8, + read_only: bool, + device: String, + volume_construction_request: propolis_client::VolumeConstructionRequest, +} + +#[derive(Clone, Debug)] +struct ParsedDiskRequest { + device_name: String, + device_spec: ComponentV0, + backend_name: String, + backend_spec: CrucibleStorageBackend, +} + +impl DiskRequest { + fn parse(&self) -> anyhow::Result { + // Preserve compatibility with the old Propolis API by adding 16 to the + // slot number, which must be between 0 and 7 inclusive. + if !(0..8).contains(&self.slot) { + anyhow::bail!("disk request slots must be in [0..7]"); + } - let backend_spec = - ComponentV0::CrucibleStorageBackend(CrucibleStorageBackend { - readonly: req.read_only, + let slot = self.slot + 0x10; + let backend_name = format!("{}-backend", self.name); + let pci_path = PciPath::new(0, slot, 0).with_context(|| { + format!("processing disk request {:?}", self.name) + })?; + let device_spec = match self.device.as_ref() { + "virtio" => ComponentV0::VirtioDisk(VirtioDisk { + backend_name: backend_name.clone(), + pci_path, + }), + "nvme" => ComponentV0::NvmeDisk(NvmeDisk { + backend_name: backend_name.clone(), + pci_path, + }), + _ => anyhow::bail!( + "invalid device type in disk request: {:?}", + self.device + ), + }; + + let backend_spec = CrucibleStorageBackend { + readonly: self.read_only, request_json: serde_json::to_string( - &req.volume_construction_request, + &self.volume_construction_request, )?, - }); + }; - add_component_to_spec(spec, req.name.clone(), device_spec)?; - add_component_to_spec(spec, backend_name, backend_spec)?; - Ok(()) + Ok(ParsedDiskRequest { + device_name: self.name.clone(), + device_spec, + backend_name, + backend_spec, + }) + } } impl VmConfig { @@ -284,7 +313,18 @@ impl VmConfig { .iter() .flatten() { - add_disk_request_to_spec(&mut spec, disk_request)?; + let ParsedDiskRequest { + device_name, + device_spec, + backend_name, + backend_spec, + } = disk_request.parse()?; + add_component_to_spec(&mut spec, device_name, device_spec)?; + add_component_to_spec( + &mut spec, + backend_name, + ComponentV0::CrucibleStorageBackend(backend_spec), + )?; } if let Some(cloud_init) = self.cloud_init.as_ref() { @@ -437,15 +477,14 @@ async fn new_instance( metadata, }; - let request = InstanceSpecEnsureRequest { - instance_spec: VersionedInstanceSpec::V0(spec), - migrate: None, + let request = InstanceEnsureRequest { properties, + init: InstanceInitializationMethod::Spec { spec }, }; // Try to create the instance client - .instance_spec_ensure() + .instance_ensure() .body(request) .send() .await @@ -675,7 +714,7 @@ async fn migrate_instance( disks: Vec, ) -> anyhow::Result<()> { // Grab the instance details - let InstanceSpecGetResponse { mut properties, spec, .. } = src_client + let InstanceSpecGetResponse { mut properties, .. } = src_client .instance_spec_get() .send() .await @@ -683,29 +722,37 @@ async fn migrate_instance( .into_inner(); let src_uuid = properties.id; properties.id = dst_uuid; - let InstanceSpecStatus::Present(spec) = spec else { - anyhow::bail!("source instance doesn't have a spec yet"); - }; - let VersionedInstanceSpec::V0(mut spec) = spec; - spec.components.clear(); - for disk in disks.iter() { - add_disk_request_to_spec(&mut spec, disk)?; + let mut replace_components = HashMap::new(); + for disk in disks { + let ParsedDiskRequest { backend_name, backend_spec, .. } = + disk.parse()?; + + let old = replace_components.insert( + backend_name.clone(), + ReplacementComponent::CrucibleStorageBackend(backend_spec), + ); + + if old.is_some() { + anyhow::bail!( + "duplicate backend name {backend_name} in replacement disk \ + list" + ); + } } - let request = InstanceSpecEnsureRequest { + let request = InstanceEnsureRequest { properties, - migrate: Some(InstanceMigrateInitiateRequest { + init: InstanceInitializationMethod::MigrationTarget { migration_id: Uuid::new_v4(), src_addr: src_addr.to_string(), - src_uuid, - }), - instance_spec: VersionedInstanceSpec::V0(spec), + replace_components, + }, }; // Initiate the migration via the destination instance let migration_res = - dst_client.instance_spec_ensure().body(request).send().await?; + dst_client.instance_ensure().body(request).send().await?; let migration_id = migration_res .migrate .as_ref() diff --git a/bin/propolis-server/src/lib/migrate/destination.rs b/bin/propolis-server/src/lib/migrate/destination.rs index 9156a02b2..6d29e1115 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::ReplacementComponent; use slog::{error, info, trace, warn}; +use std::collections::BTreeMap; use std::convert::TryInto; use std::io; use std::net::SocketAddr; @@ -36,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: BTreeMap, +} + /// The interface to an arbitrary version of the target half of the live /// migration protocol. // @@ -58,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; @@ -350,7 +357,12 @@ impl RonV0 { }?; info!(self.log(), "Destination read Preamble: {:?}", preamble); - let spec = match preamble.amend_spec(ensure_ctx.instance_spec()) { + let spec = match preamble.amend_spec( + &ensure_ctx + .migration_info() + .expect("migration in was requested") + .replace_components, + ) { Ok(spec) => spec, Err(e) => { error!( diff --git a/bin/propolis-server/src/lib/migrate/preamble.rs b/bin/propolis-server/src/lib/migrate/preamble.rs index a2946c556..6729f92e2 100644 --- a/bin/propolis-server/src/lib/migrate/preamble.rs +++ b/bin/propolis-server/src/lib/migrate/preamble.rs @@ -2,12 +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::{ - v0::ComponentV0, VersionedInstanceSpec, +use std::collections::BTreeMap; + +use propolis_api_types::{ + instance_spec::{v0::ComponentV0, VersionedInstanceSpec}, + ReplacementComponent, }; use serde::{Deserialize, Serialize}; -use crate::spec::{api_spec_v0::ApiSpecError, Spec, StorageBackend}; +use crate::spec::{api_spec_v0::ApiSpecError, Spec}; use super::MigrateError; @@ -30,73 +33,65 @@ impl Preamble { /// Any such backends will replace the corresponding backend entries in the /// source spec. If the target spec contains a replacement backend that is /// not present in the source spec, this routine fails. - pub fn amend_spec(self, target_spec: &Spec) -> Result { - let VersionedInstanceSpec::V0(mut source_spec) = self.instance_spec; - for disk in target_spec.disks.values() { - let StorageBackend::Crucible(crucible) = &disk.backend_spec else { - continue; - }; - - let Some(to_amend) = - source_spec.components.get_mut(disk.device_spec.backend_name()) - else { - return Err(MigrateError::InstanceSpecsIncompatible(format!( - "replacement component {} not in source spec", - disk.device_spec.backend_name() - ))); - }; - - if !matches!(to_amend, ComponentV0::CrucibleStorageBackend(_)) { - return Err(MigrateError::InstanceSpecsIncompatible(format!( - "component {} is not a Crucible backend in the source spec", - disk.device_spec.backend_name() - ))); - } - - *to_amend = ComponentV0::CrucibleStorageBackend(crucible.clone()); - } - - for nic in target_spec.nics.values() { - let Some(to_amend) = - source_spec.components.get_mut(&nic.device_spec.backend_name) - else { - return Err(MigrateError::InstanceSpecsIncompatible(format!( - "replacement component {} not in source spec", - nic.device_spec.backend_name - ))); - }; - - if !matches!(to_amend, ComponentV0::VirtioNetworkBackend(_)) { - return Err(MigrateError::InstanceSpecsIncompatible(format!( - "component {} is not a virtio network backend \ - in the source spec", - nic.device_spec.backend_name - ))); - } - - *to_amend = - ComponentV0::VirtioNetworkBackend(nic.backend_spec.clone()); + pub fn amend_spec( + self, + replacements: &BTreeMap, + ) -> Result { + fn wrong_type_error(id: &str, kind: &str) -> MigrateError { + let msg = + format!("component {id} is not a {kind} in the source spec"); + MigrateError::InstanceSpecsIncompatible(msg) } - #[cfg(not(feature = "omicron-build"))] - if let Some(mig) = &target_spec.migration_failure { - let Some(to_amend) = source_spec.components.get_mut(&mig.name) + let VersionedInstanceSpec::V0(mut source_spec) = self.instance_spec; + for (id, comp) in replacements { + let Some(to_amend) = source_spec.components.get_mut(id.as_str()) else { return Err(MigrateError::InstanceSpecsIncompatible(format!( - "replacement component {} not in source spec", - mig.name + "replacement component {id} not in source spec", ))); }; - if !matches!(to_amend, ComponentV0::MigrationFailureInjector(_)) { - return Err(MigrateError::InstanceSpecsIncompatible(format!( - "component {} is not a migration failure injector \ - in the source spec", - mig.name - ))); + match comp { + #[cfg(feature = "omicron-build")] + ReplacementComponent::MigrationFailureInjector(_) => { + return Err(MigrateError::InstanceSpecsIncompatible( + format!( + "replacing migration failure injector {id} is \ + impossible because the feature is compiled out" + ), + )); + } + + #[cfg(not(feature = "omicron-build"))] + ReplacementComponent::MigrationFailureInjector(comp) => { + let ComponentV0::MigrationFailureInjector(src) = to_amend + else { + return Err(wrong_type_error( + id, + "migration failure injector", + )); + }; + + *src = comp.clone(); + } + ReplacementComponent::CrucibleStorageBackend(comp) => { + let ComponentV0::CrucibleStorageBackend(src) = to_amend + else { + return Err(wrong_type_error(id, "crucible backend")); + }; + + *src = comp.clone(); + } + ReplacementComponent::VirtioNetworkBackend(comp) => { + let ComponentV0::VirtioNetworkBackend(src) = to_amend + else { + return Err(wrong_type_error(id, "viona backend")); + }; + + *src = comp.clone(); + } } - - *to_amend = ComponentV0::MigrationFailureInjector(mig.spec.clone()); } let amended_spec = diff --git a/bin/propolis-server/src/lib/server.rs b/bin/propolis-server/src/lib/server.rs index 0a32d4464..00059b501 100644 --- a/bin/propolis-server/src/lib/server.rs +++ b/bin/propolis-server/src/lib/server.rs @@ -17,14 +17,10 @@ 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}, }; @@ -40,9 +36,7 @@ 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, -}; +use propolis_api_types::InstanceInitializationMethod; use rfb::tungstenite::BinaryWs; use slog::{error, warn, Logger}; use tokio::sync::MutexGuard; @@ -117,55 +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, -) -> Result { - let mut spec_builder = SpecBuilder::new(request.vcpus, request.memory); - - 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. /// @@ -242,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 @@ -270,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 generate internal instance spec: {e}"), + ) + })?; + + let request = VmEnsureRequest { properties, init: vm_init }; server_context .vm .ensure(&server_context.log, request, ensure_options) @@ -296,55 +266,6 @@ async fn instance_ensure_common( }) } -#[endpoint { - method = PUT, - path = "/instance", -}] -async fn instance_ensure( - rqctx: RequestContext>, - request: TypedBody, -) -> Result, HttpError> { - let request = request.into_inner(); - let instance_spec = instance_spec_from_request(&request).map_err(|e| { - HttpError::for_bad_request( - None, - format!("failed to generate instance spec from request: {:#?}", e), - ) - })?; - - instance_ensure_common( - rqctx, - 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 { @@ -718,7 +639,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/builder.rs b/bin/propolis-server/src/lib/spec/builder.rs index 877317b9a..8bbd4e195 100644 --- a/bin/propolis-server/src/lib/spec/builder.rs +++ b/bin/propolis-server/src/lib/spec/builder.rs @@ -7,15 +7,12 @@ 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, }; use thiserror::Error; @@ -27,7 +24,6 @@ use propolis_api_types::instance_spec::components::devices::{ use crate::spec::SerialPortDevice; use super::{ - api_request::{self, DeviceRequestError}, Board, BootOrderEntry, BootSettings, Disk, Nic, QemuPvpanic, SerialPort, }; @@ -40,9 +36,6 @@ 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 device in ensure request")] - DeviceRequest(#[from] DeviceRequestError), - #[error("device {0} has the same name as its backend")] DeviceAndBackendNamesIdentical(String), @@ -81,19 +74,6 @@ pub(crate) struct SpecBuilder { } 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 { @@ -121,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`. /// @@ -177,17 +135,6 @@ impl SpecBuilder { 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 a PCI path to this builder's record of PCI locations with an /// attached device. If the path is already in use, returns an error. fn register_pci_device( @@ -336,33 +283,26 @@ impl SpecBuilder { } #[cfg(feature = "falcon")] - pub fn set_softnpu_com4( + pub fn set_softnpu_pci_port( &mut self, - name: String, + pci_port: SoftNpuPciPort, ) -> Result<&Self, SpecBuilderError> { - if self.component_names.contains(&name) { - return Err(SpecBuilderError::ComponentNameInUse(name)); + // SoftNPU squats on COM4. + let id = "com4".to_string(); + let num = SerialPortNumber::Com4; + if self.component_names.contains(&id) { + return Err(SpecBuilderError::ComponentNameInUse(id)); } - let num = SerialPortNumber::Com4; if self.serial_ports.contains(&num) { return Err(SpecBuilderError::SerialPortInUse(num)); } - let desc = SerialPort { num, device: SerialPortDevice::SoftNpu }; - self.spec.serial.insert(name.clone(), desc); - self.component_names.insert(name); - self.serial_ports.insert(num); - Ok(self) - } - - #[cfg(feature = "falcon")] - pub fn set_softnpu_pci_port( - &mut self, - pci_port: SoftNpuPciPort, - ) -> Result<&Self, SpecBuilderError> { 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,12 +358,10 @@ impl SpecBuilder { #[cfg(test)] mod test { - use propolis_api_types::{ - instance_spec::components::{ - backends::{BlobStorageBackend, VirtioNetworkBackend}, - devices::{VirtioDisk, VirtioNic}, - }, - Slot, VolumeConstructionRequest, + use propolis_api_types::instance_spec::components::{ + backends::{BlobStorageBackend, VirtioNetworkBackend}, + board::{Chipset, I440Fx}, + devices::{VirtioDisk, VirtioNic}, }; use uuid::Uuid; @@ -432,39 +370,51 @@ mod test { use super::*; fn test_builder() -> SpecBuilder { - SpecBuilder::new(4, 512) + let board = Board { + cpus: 4, + memory_mb: 512, + chipset: Chipset::I440Fx(I440Fx { enable_pcie: false }), + }; + + SpecBuilder { + spec: crate::spec::Spec { board, ..Default::default() }, + ..Default::default() + } } #[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() - }, - }) + .add_storage_device( + "storage".to_owned(), + Disk { + device_spec: StorageDevice::Virtio(VirtioDisk { + backend_name: "storage-backend".to_owned(), + pci_path: PciPath::new(0, 4, 0).unwrap() + }), + backend_spec: StorageBackend::Blob(BlobStorageBackend { + base64: "".to_string(), + readonly: false + }) + } + ) .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() - }, - }) + .add_network_device( + "network".to_owned(), + Nic { + device_spec: VirtioNic { + backend_name: "network-backend".to_owned(), + interface_id: Uuid::nil(), + pci_path: PciPath::new(0, 4, 0).unwrap() + }, + backend_spec: VirtioNetworkBackend { + vnic_name: "vnic0".to_owned() + } + } + ) .is_err()); } @@ -488,24 +438,6 @@ mod test { .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(); diff --git a/bin/propolis-server/src/lib/spec/mod.rs b/bin/propolis-server/src/lib/spec/mod.rs index c211693fd..10258265c 100644 --- a/bin/propolis-server/src/lib/spec/mod.rs +++ b/bin/propolis-server/src/lib/spec/mod.rs @@ -43,7 +43,7 @@ use propolis_api_types::instance_spec::components::{ devices::{P9fs, SoftNpuP9, SoftNpuPciPort}, }; -mod api_request; +// mod api_request; pub(crate) mod api_spec_v0; pub(crate) mod builder; @@ -306,27 +306,3 @@ pub struct SoftNpu { pub p9_device: Option, pub p9fs: Option, } - -struct ParsedDiskRequest { - name: String, - disk: Disk, -} - -struct ParsedNicRequest { - name: String, - nic: Nic, -} - -/// 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/ensure.rs b/bin/propolis-server/src/lib/vm/ensure.rs index ae6040f4d..1c0a2080d 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,6 +39,7 @@ use crate::{ initializer::{ build_instance, MachineInitializer, MachineInitializerState, }, + migrate::destination::MigrationTargetInfo, spec::Spec, stats::{create_kstat_sampler, VirtualMachine}, vm::{ @@ -55,10 +56,34 @@ 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 { + pub(crate) fn is_migration(&self) -> bool { + matches!(self.init, VmInitializationMethod::Migration(_)) + } + + pub(crate) fn migration_info(&self) -> Option<&MigrationTargetInfo> { + match &self.init { + VmInitializationMethod::Spec(_) => None, + VmInitializationMethod::Migration(info) => Some(info), + } + } + + pub(crate) fn spec(&self) -> Option<&Spec> { + match &self.init { + VmInitializationMethod::Spec(spec) => Some(spec), + VmInitializationMethod::Migration(_) => None, + } + } } /// Holds state about an instance ensure request that has not yet produced any @@ -97,18 +122,25 @@ 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_from_request( self, ) -> anyhow::Result> { - let spec = self.ensure_request.instance_spec.clone(); + let spec = self + .ensure_request + .spec() + .expect( + "create_objects_from_request is called with an explicit spec", + ) + .clone(); + self.create_objects(spec).await } @@ -129,9 +161,10 @@ impl<'a> VmEnsureNotStarted<'a> { 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 }, )); @@ -267,7 +300,7 @@ 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| { + migrate: self.ensure_request.migration_info().map(|req| { InstanceMigrateInitiateResponse { migration_id: req.migration_id, } diff --git a/bin/propolis-server/src/lib/vm/mod.rs b/bin/propolis-server/src/lib/vm/mod.rs index c820b7dea..f680702fc 100644 --- a/bin/propolis-server/src/lib/vm/mod.rs +++ b/bin/propolis-server/src/lib/vm/mod.rs @@ -562,13 +562,13 @@ impl Vm { &log_for_driver, InstanceStateMonitorResponse { gen: 1, - state: if ensure_request.migrate.is_some() { + state: if ensure_request.is_migration() { InstanceState::Migrating } else { InstanceState::Creating }, migration: InstanceMigrateStatusResponse { - migration_in: ensure_request.migrate.as_ref().map(|req| { + migration_in: ensure_request.migration_info().map(|req| { InstanceMigrationStatus { id: req.migration_id, state: MigrationState::Sync, @@ -598,10 +598,13 @@ impl Vm { }; let properties = ensure_request.properties.clone(); - let spec = if ensure_request.migrate.is_some() { - MaybeSpec::WaitingForMigrationSource - } else { - MaybeSpec::Present(ensure_request.instance_spec.clone()) + let spec = match &ensure_request.init { + ensure::VmInitializationMethod::Spec(s) => { + MaybeSpec::Present(s.clone()) + } + ensure::VmInitializationMethod::Migration(_) => { + MaybeSpec::WaitingForMigrationSource + } }; let vm_for_driver = self.clone(); diff --git a/bin/propolis-server/src/lib/vm/state_driver.rs b/bin/propolis-server/src/lib/vm/state_driver.rs index 73e1768be..e565f96da 100644 --- a/bin/propolis-server/src/lib/vm/state_driver.rs +++ b/bin/propolis-server/src/lib/vm/state_driver.rs @@ -304,8 +304,7 @@ pub(super) async fn run_state_driver( // new external callers can access its objects or services. match vmm_rt_hdl .spawn(async move { - let output = - state_driver.run(ensure_request.migrate.is_some()).await; + let output = state_driver.run(ensure_request.is_migration()).await; vm.set_rundown().await; output }) @@ -335,7 +334,7 @@ async fn create_and_activate_vm<'a>( state_publisher, ); - if let Some(migrate_request) = ensure_request.migrate.as_ref() { + if let Some(migrate_request) = ensure_request.migration_info() { let migration = match crate::migrate::destination::initiate( log, migrate_request, diff --git a/crates/propolis-api-types/src/lib.rs b/crates/propolis-api-types/src/lib.rs index db007667c..1944b3b13 100644 --- a/crates/propolis-api-types/src/lib.rs +++ b/crates/propolis-api-types/src/lib.rs @@ -4,8 +4,9 @@ //! Definitions for types exposed by the propolis-server API -use std::{fmt, net::SocketAddr}; +use std::{collections::BTreeMap, fmt, net::SocketAddr}; +use instance_spec::v0::InstanceSpecV0; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use uuid::Uuid; @@ -15,7 +16,9 @@ use uuid::Uuid; pub use crate::instance_spec::components::devices::{ BootOrderEntry, BootSettings, }; -use crate::instance_spec::VersionedInstanceSpec; +use crate::instance_spec::{ + components, v0::ComponentV0, VersionedInstanceSpec, +}; // Re-export volume construction requests since they're part of a disk request. pub use crucible_client_types::VolumeConstructionRequest; @@ -48,36 +51,49 @@ pub struct InstanceMetadata { pub sled_model: String, } +/// Describes an instance spec component that should be replaced during a live +/// migration. #[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, - - #[serde(default)] - pub nics: Vec, - - #[serde(default)] - pub disks: Vec, - - #[serde(default)] - pub boot_settings: Option, - - pub migrate: Option, +#[serde(deny_unknown_fields, tag = "component", content = "spec")] +pub enum ReplacementComponent { + MigrationFailureInjector(components::devices::MigrationFailureInjector), + CrucibleStorageBackend(components::backends::CrucibleStorageBackend), + VirtioNetworkBackend(components::backends::VirtioNetworkBackend), +} + +impl From for instance_spec::v0::ComponentV0 { + fn from(value: ReplacementComponent) -> Self { + match value { + ReplacementComponent::MigrationFailureInjector(c) => { + ComponentV0::MigrationFailureInjector(c) + } + ReplacementComponent::CrucibleStorageBackend(c) => { + ComponentV0::CrucibleStorageBackend(c) + } + ReplacementComponent::VirtioNetworkBackend(c) => { + ComponentV0::VirtioNetworkBackend(c) + } + } + } +} - // base64 encoded cloud-init ISO - pub cloud_init_bytes: Option, +#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)] +#[serde(tag = "method", content = "value")] +pub enum InstanceInitializationMethod { + Spec { + spec: InstanceSpecV0, + }, + MigrationTarget { + migration_id: Uuid, + src_addr: SocketAddr, + replace_components: BTreeMap, + }, } #[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)] @@ -301,72 +317,6 @@ 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, diff --git a/lib/propolis-client/Cargo.toml b/lib/propolis-client/Cargo.toml index 355efa94a..7bf325034 100644 --- a/lib/propolis-client/Cargo.toml +++ b/lib/propolis-client/Cargo.toml @@ -9,6 +9,7 @@ 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 diff --git a/lib/propolis-client/src/lib.rs b/lib/propolis-client/src/lib.rs index 03305aeaf..2ec411af6 100644 --- a/lib/propolis-client/src/lib.rs +++ b/lib/propolis-client/src/lib.rs @@ -8,6 +8,12 @@ // its constructor and From impls. pub use propolis_api_types::instance_spec::PciPath; +// Re-export Crucible client types that appear in their serialized forms in +// instance specs. This allows clients to ensure they serialize/deserialize +// these types using the same versions as the Propolis client associated with +// the server they want to talk to. +pub use crucible_client_types::{CrucibleOpts, VolumeConstructionRequest}; + progenitor::generate_api!( spec = "../../openapi/propolis-server.json", interface = Builder, @@ -16,20 +22,9 @@ progenitor::generate_api!( PciPath = crate::PciPath, }, 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 ] }, } ); diff --git a/openapi/propolis-server.json b/openapi/propolis-server.json index b02e9036c..ff27cd36b 100644 --- a/openapi/propolis-server.json +++ b/openapi/propolis-server.json @@ -362,37 +362,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": { @@ -984,58 +953,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", @@ -1055,33 +972,6 @@ ], "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.", "type": "object", @@ -1166,61 +1056,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": { @@ -1247,6 +1092,74 @@ "instance" ] }, + "InstanceInitializationMethod": { + "oneOf": [ + { + "type": "object", + "properties": { + "method": { + "type": "string", + "enum": [ + "Spec" + ] + }, + "value": { + "type": "object", + "properties": { + "spec": { + "$ref": "#/components/schemas/InstanceSpecV0" + } + }, + "required": [ + "spec" + ] + } + }, + "required": [ + "method", + "value" + ] + }, + { + "type": "object", + "properties": { + "method": { + "type": "string", + "enum": [ + "MigrationTarget" + ] + }, + "value": { + "type": "object", + "properties": { + "migration_id": { + "type": "string", + "format": "uuid" + }, + "replace_components": { + "type": "object", + "additionalProperties": { + "$ref": "#/components/schemas/ReplacementComponent" + } + }, + "src_addr": { + "type": "string" + } + }, + "required": [ + "migration_id", + "replace_components", + "src_addr" + ] + } + }, + "required": [ + "method", + "value" + ] + } + ] + }, "InstanceMetadata": { "type": "object", "properties": { @@ -1283,27 +1196,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": { @@ -1420,29 +1312,6 @@ "last_byte_offset" ] }, - "InstanceSpecEnsureRequest": { - "type": "object", - "properties": { - "instance_spec": { - "$ref": "#/components/schemas/VersionedInstanceSpec" - }, - "migrate": { - "nullable": true, - "allOf": [ - { - "$ref": "#/components/schemas/InstanceMigrateInitiateRequest" - } - ] - }, - "properties": { - "$ref": "#/components/schemas/InstanceProperties" - } - }, - "required": [ - "instance_spec", - "properties" - ] - }, "InstanceSpecGetResponse": { "type": "object", "properties": { @@ -1628,26 +1497,6 @@ "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", @@ -1780,6 +1629,68 @@ "vcr_matches" ] }, + "ReplacementComponent": { + "description": "Describes an instance spec component that should be replaced during a live migration.", + "oneOf": [ + { + "type": "object", + "properties": { + "component": { + "type": "string", + "enum": [ + "MigrationFailureInjector" + ] + }, + "spec": { + "$ref": "#/components/schemas/MigrationFailureInjector" + } + }, + "required": [ + "component", + "spec" + ], + "additionalProperties": false + }, + { + "type": "object", + "properties": { + "component": { + "type": "string", + "enum": [ + "CrucibleStorageBackend" + ] + }, + "spec": { + "$ref": "#/components/schemas/CrucibleStorageBackend" + } + }, + "required": [ + "component", + "spec" + ], + "additionalProperties": false + }, + { + "type": "object", + "properties": { + "component": { + "type": "string", + "enum": [ + "VirtioNetworkBackend" + ] + }, + "spec": { + "$ref": "#/components/schemas/VirtioNetworkBackend" + } + }, + "required": [ + "component", + "spec" + ], + "additionalProperties": false + } + ] + }, "SerialPort": { "description": "A serial port device.", "type": "object", @@ -1808,12 +1719,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", @@ -1959,150 +1864,6 @@ ], "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/phd-tests/framework/src/disk/crucible.rs b/phd-tests/framework/src/disk/crucible.rs index 244f87309..36fec53c7 100644 --- a/phd-tests/framework/src/disk/crucible.rs +++ b/phd-tests/framework/src/disk/crucible.rs @@ -12,9 +12,9 @@ use std::{ }; use anyhow::Context; -use propolis_client::types::{ - ComponentV0, CrucibleOpts, CrucibleStorageBackend, - VolumeConstructionRequest, +use propolis_client::{ + types::{ComponentV0, CrucibleStorageBackend}, + CrucibleOpts, VolumeConstructionRequest, }; use rand::{rngs::StdRng, RngCore, SeedableRng}; use tracing::{error, info}; @@ -288,11 +288,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/mod.rs b/phd-tests/framework/src/test_vm/mod.rs index c8f419dcc..872856592 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, 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 }, @@ -271,19 +274,12 @@ 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 { id: self.id, @@ -292,14 +288,18 @@ 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 init = match migrate { + None => InstanceInitializationMethod::Spec { + spec: self.spec.instance_spec.clone(), + }, + Some(info) => InstanceInitializationMethod::MigrationTarget { + migration_id: info.migration_id, + replace_components: info.replace_components, + src_addr: info.src_addr.to_string(), + }, }; + let ensure_req = + InstanceEnsureRequest { properties: properties.clone(), init }; // There is a brief period where the Propolis server process has begun // to run but hasn't started its Dropshot server yet. Ensure requests @@ -310,39 +310,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(_) => { @@ -433,29 +402,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 }; } @@ -558,11 +505,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), ) @@ -616,6 +563,33 @@ impl TestVm { } } + fn generate_replacement_components(&self) -> ReplacementComponents { + let mut map = ReplacementComponents::new(); + for (id, comp) in &self.spec.instance_spec.components { + match comp { + ComponentV0::MigrationFailureInjector(inj) => { + map.insert( + id.clone(), + ReplacementComponent::MigrationFailureInjector( + inj.clone(), + ), + ); + } + ComponentV0::CrucibleStorageBackend(be) => { + 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/spec.rs b/phd-tests/framework/src/test_vm/spec.rs index dc276b2df..573537c08 100644 --- a/phd-tests/framework/src/test_vm/spec.rs +++ b/phd-tests/framework/src/test_vm/spec.rs @@ -9,10 +9,7 @@ use crate::{ guest_os::GuestOsKind, }; use camino::Utf8PathBuf; -use propolis_client::{ - types::{ComponentV0, DiskRequest, InstanceMetadata, InstanceSpecV0, Slot}, - PciPath, -}; +use propolis_client::types::{ComponentV0, InstanceMetadata, InstanceSpecV0}; use uuid::Uuid; /// The set of objects needed to start and run a guest in a `TestVm`. @@ -76,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/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;