diff --git a/bin/propolis-server/src/lib/initializer.rs b/bin/propolis-server/src/lib/initializer.rs index 6f5ac4209..941c8cd7d 100644 --- a/bin/propolis-server/src/lib/initializer.rs +++ b/bin/propolis-server/src/lib/initializer.rs @@ -11,6 +11,7 @@ use std::sync::Arc; use std::time::{SystemTime, UNIX_EPOCH}; use crate::serial::Serial; +use crate::spec::{self, Spec, StorageBackend}; use crate::stats::{ track_network_interface_kstats, track_vcpu_kstats, VirtualDiskProducer, VirtualMachine, @@ -38,15 +39,16 @@ 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::{self, v0::InstanceSpecV0}; +use propolis_api_types::instance_spec; +use propolis_api_types::instance_spec::components::devices::SerialPortNumber; use propolis_api_types::InstanceProperties; use slog::info; /// Arbitrary ROM limit for now const MAX_ROM_SIZE: usize = 0x20_0000; -fn get_spec_guest_ram_limits(spec: &InstanceSpecV0) -> (usize, usize) { - let memsize = spec.devices.board.memory_mb as usize * MB; +fn get_spec_guest_ram_limits(spec: &Spec) -> (usize, usize) { + let memsize = spec.board.memory_mb as usize * MB; let lowmem = memsize.min(3 * GB); let highmem = memsize.saturating_sub(3 * GB); (lowmem, highmem) @@ -54,7 +56,7 @@ fn get_spec_guest_ram_limits(spec: &InstanceSpecV0) -> (usize, usize) { pub fn build_instance( name: &str, - spec: &InstanceSpecV0, + spec: &Spec, use_reservoir: bool, _log: slog::Logger, ) -> Result { @@ -65,7 +67,7 @@ pub fn build_instance( track_dirty: true, }; let mut builder = Builder::new(name, create_opts)? - .max_cpus(spec.devices.board.cpus)? + .max_cpus(spec.board.cpus)? .add_mem_region(0, lowmem, "lowmem")? .add_rom_region(0x1_0000_0000 - MAX_ROM_SIZE, MAX_ROM_SIZE, "bootrom")? .add_mmio_region(0xc000_0000_usize, 0x2000_0000_usize, "dev32")? @@ -118,7 +120,7 @@ pub struct MachineInitializer<'a> { pub(crate) devices: DeviceMap, pub(crate) block_backends: BlockBackendMap, pub(crate) crucible_backends: CrucibleBackendMap, - pub(crate) spec: &'a InstanceSpecV0, + pub(crate) spec: &'a Spec, pub(crate) properties: &'a InstanceProperties, pub(crate) toml_config: &'a crate::server::VmTomlConfig, pub(crate) producer_registry: Option, @@ -194,7 +196,7 @@ impl<'a> MachineInitializer<'a> { event_handler: &Arc, ) -> Result { let mut pci_builder = pci::topology::Builder::new(); - for (name, bridge) in &self.spec.devices.pci_pci_bridges { + for (name, bridge) in &self.spec.pci_pci_bridges { let desc = pci::topology::BridgeDescription::new( pci::topology::LogicalBusId(bridge.downstream_bus), bridge.pci_path.try_into().map_err(|e| { @@ -212,7 +214,7 @@ impl<'a> MachineInitializer<'a> { let pci::topology::FinishedTopology { topology: pci_topology, bridges } = pci_builder.finish(self.machine)?; - match self.spec.devices.board.chipset { + match self.spec.board.chipset { instance_spec::components::board::Chipset::I440Fx(i440fx) => { let power_ref = Arc::downgrade(event_handler); let reset_ref = Arc::downgrade(event_handler); @@ -295,22 +297,35 @@ impl<'a> MachineInitializer<'a> { &mut self, chipset: &RegisteredChipset, ) -> Result, Error> { - use instance_spec::components::devices::SerialPortNumber; - let mut com1 = None; - for (name, serial_spec) in &self.spec.devices.serial_ports { - let (irq, port) = match serial_spec.num { - SerialPortNumber::Com1 => (ibmpc::IRQ_COM1, ibmpc::PORT_COM1), - SerialPortNumber::Com2 => (ibmpc::IRQ_COM2, ibmpc::PORT_COM2), - SerialPortNumber::Com3 => (ibmpc::IRQ_COM3, ibmpc::PORT_COM3), - SerialPortNumber::Com4 => (ibmpc::IRQ_COM4, ibmpc::PORT_COM4), + + // Create UART devices for all of the serial ports in the spec that + // requested one. + for (num, user) in self.spec.serial.iter() { + if *user != spec::SerialPortDevice::Uart { + continue; + } + + let (name, irq, port) = match num { + SerialPortNumber::Com1 => { + ("com1", ibmpc::IRQ_COM1, ibmpc::PORT_COM1) + } + SerialPortNumber::Com2 => { + ("com2", ibmpc::IRQ_COM2, ibmpc::PORT_COM2) + } + SerialPortNumber::Com3 => { + ("com3", ibmpc::IRQ_COM3, ibmpc::PORT_COM3) + } + SerialPortNumber::Com4 => { + ("com4", ibmpc::IRQ_COM4, ibmpc::PORT_COM4) + } }; 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.clone(), dev.clone()); - if matches!(serial_spec.num, SerialPortNumber::Com1) { + self.devices.insert(name.to_owned(), dev.clone()); + if *num == SerialPortNumber::Com1 { assert!(com1.is_none()); com1 = Some(dev); } @@ -353,8 +368,8 @@ impl<'a> MachineInitializer<'a> { &mut self, virtual_machine: VirtualMachine, ) -> Result<(), anyhow::Error> { - if let Some(ref spec) = self.spec.devices.qemu_pvpanic { - if spec.enable_isa { + if let Some(pvpanic) = &self.spec.pvpanic { + if pvpanic.spec.enable_isa { let pvpanic = QemuPvpanic::create( self.log.new(slog::o!("dev" => "qemu-pvpanic")), ); @@ -379,12 +394,12 @@ impl<'a> MachineInitializer<'a> { async fn create_storage_backend_from_spec( &self, - backend_spec: &instance_spec::v0::StorageBackendV0, + backend_spec: &StorageBackend, backend_name: &str, nexus_client: &Option, ) -> Result { match backend_spec { - instance_spec::v0::StorageBackendV0::Crucible(spec) => { + StorageBackend::Crucible(spec) => { info!(self.log, "Creating Crucible disk"; "backend_name" => backend_name); @@ -421,7 +436,7 @@ impl<'a> MachineInitializer<'a> { let crucible = Some((be.get_uuid().await?, be.clone())); Ok(StorageBackendInstance { be, crucible }) } - instance_spec::v0::StorageBackendV0::File(spec) => { + StorageBackend::File(spec) => { info!(self.log, "Creating file disk backend"; "path" => &spec.path); @@ -447,7 +462,7 @@ impl<'a> MachineInitializer<'a> { Ok(StorageBackendInstance { be, crucible: None }) } - instance_spec::v0::StorageBackendV0::Blob(spec) => { + StorageBackend::Blob(spec) => { let bytes = base64::Engine::decode( &base64::engine::general_purpose::STANDARD, &spec.base64, @@ -497,52 +512,38 @@ impl<'a> MachineInitializer<'a> { Nvme, } - 'devloop: for (name, device_spec) in &self.spec.devices.storage_devices - { + for (disk_name, disk) in &self.spec.disks { info!( self.log, - "Creating storage device {} with properties {:?}", - name, - device_spec + "Creating storage device"; + "name" => disk_name, + "spec" => ?disk.device_spec ); - let (device_interface, backend_name, pci_path) = match device_spec { - instance_spec::v0::StorageDeviceV0::VirtioDisk(disk) => { + let (device_interface, backend_name, pci_path) = match &disk + .device_spec + { + spec::StorageDevice::Virtio(disk) => { (DeviceInterface::Virtio, &disk.backend_name, disk.pci_path) } - instance_spec::v0::StorageDeviceV0::NvmeDisk(disk) => { + spec::StorageDevice::Nvme(disk) => { (DeviceInterface::Nvme, &disk.backend_name, disk.pci_path) } }; - let backend_spec = self - .spec - .backends - .storage_backends - .get(backend_name) - .ok_or_else(|| { - Error::new( - ErrorKind::InvalidInput, - format!( - "Backend {} not found for storage device {}", - backend_name, name - ), - ) - })?; - let bdf: pci::Bdf = pci_path.try_into().map_err(|e| { Error::new( ErrorKind::InvalidInput, format!( "Couldn't get PCI BDF for storage device {}: {}", - name, e + disk_name, e ), ) })?; let StorageBackendInstance { be: backend, crucible } = self .create_storage_backend_from_spec( - backend_spec, + &disk.backend_spec, backend_name, &nexus_client, ) @@ -562,12 +563,11 @@ 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 nvme = nvme::PciNvme::create( - name.to_string(), + disk_name.to_owned(), mdts, - self.log.new( - slog::o!("component" => format!("nvme-{}", name)), - ), + self.log.new(slog::o!("component" => component)), ); self.devices .insert(format!("pci-nvme-{bdf}"), nvme.clone()); @@ -594,7 +594,7 @@ impl<'a> MachineInitializer<'a> { virtual disk metrics can't be reported for it"; "disk_id" => %disk_id, ); - continue 'devloop; + continue; }; // Register the block device as a metric producer, if we've been @@ -615,7 +615,7 @@ impl<'a> MachineInitializer<'a> { "disk_id" => %disk_id, "error" => ?e, ); - continue 'devloop; + continue; }; // Set the on-completion callback for the block device, to @@ -643,49 +643,21 @@ impl<'a> MachineInitializer<'a> { let mut interface_ids: Option = self.kstat_sampler.as_ref().map(|_| Vec::new()); - for (name, vnic_spec) in &self.spec.devices.network_devices { - info!(self.log, "Creating vNIC {}", name); - let instance_spec::v0::NetworkDeviceV0::VirtioNic(vnic_spec) = - vnic_spec; - - let backend_spec = self - .spec - .backends - .network_backends - .get(&vnic_spec.backend_name) - .ok_or_else(|| { + for (device_name, nic) in &self.spec.nics { + info!(self.log, "Creating vNIC {}", device_name); + let bdf: pci::Bdf = + nic.device_spec.pci_path.try_into().map_err(|e| { Error::new( ErrorKind::InvalidInput, format!( - "Backend {} not found for vNIC {}", - vnic_spec.backend_name, name + "Couldn't get PCI BDF for vNIC {}: {}", + device_name, e ), ) })?; - let bdf: pci::Bdf = vnic_spec.pci_path.try_into().map_err(|e| { - Error::new( - ErrorKind::InvalidInput, - format!("Couldn't get PCI BDF for vNIC {}: {}", name, e), - ) - })?; - - let vnic_name = match backend_spec { - instance_spec::v0::NetworkBackendV0::Virtio(spec) => { - &spec.vnic_name - } - instance_spec::v0::NetworkBackendV0::Dlpi(_) => { - return Err(Error::new( - ErrorKind::InvalidInput, - format!( - "Network backend must be virtio for vNIC {}", - name, - ), - )); - } - }; let viona = virtio::PciVirtioViona::new( - vnic_name, + &nic.backend_spec.vnic_name, 0x100, &self.machine.hdl, )?; @@ -694,7 +666,7 @@ impl<'a> MachineInitializer<'a> { // Only push to interface_ids if kstat_sampler exists if let Some(ref mut ids) = interface_ids { - ids.push((vnic_spec.interface_id, viona.instance_id()?)); + ids.push((nic.device_spec.interface_id, viona.instance_id()?)); } chipset.pci_attach(bdf, viona); @@ -767,56 +739,29 @@ impl<'a> MachineInitializer<'a> { &mut self, chipset: &RegisteredChipset, ) -> Result<(), Error> { + let softnpu = &self.spec.softnpu; + // Check to make sure we actually have both a pci port and at least one // regular SoftNpu port, otherwise just return. - let pci_port = match &self.spec.devices.softnpu_pci_port { + let pci_port = match &softnpu.pci_port { Some(tfp) => tfp, None => return Ok(()), }; - if self.spec.devices.softnpu_ports.is_empty() { + if softnpu.ports.is_empty() { return Ok(()); } - let mut ports: Vec<&instance_spec::components::devices::SoftNpuPort> = - self.spec.devices.softnpu_ports.values().collect(); + // Get a Vec of references to the ports which will then be sorted by + // port name. + let mut ports: Vec<_> = softnpu.ports.iter().collect(); // SoftNpu ports are named __vnic by falcon, where // indicates the intended order. - ports.sort_by_key(|p| &p.name); - - let mut data_links: Vec = Vec::new(); - for x in &ports { - let backend = self - .spec - .backends - .network_backends - .get(&x.backend_name) - .ok_or_else(|| { - Error::new( - ErrorKind::InvalidInput, - format!( - "Backend {} not found for softnpu port", - x.backend_name - ), - ) - })?; - - let vnic = match &backend { - instance_spec::v0::NetworkBackendV0::Dlpi(dlpi) => { - &dlpi.vnic_name - } - _ => { - return Err(Error::new( - ErrorKind::InvalidInput, - format!( - "Softnpu port must have DLPI backend: {}", - x.backend_name - ), - )); - } - }; - data_links.push(vnic.clone()); - } + ports.sort_by_key(|p| p.0.as_str()); + let data_links = ports + .iter() + .map(|port| port.1.backend_spec.vnic_name.clone()) + .collect(); // Set up an LPC uart for ASIC management comms from the guest. // @@ -834,17 +779,15 @@ impl<'a> MachineInitializer<'a> { let p9_handler = virtio::softnpu::SoftNpuP9Handler::new( "/dev/softnpufs".to_owned(), "/dev/softnpufs".to_owned(), - self.spec.devices.softnpu_ports.len() as u16, + ports.len() as u16, pipeline.clone(), self.log.clone(), ); let vio9p = virtio::p9fs::PciVirtio9pfs::new(0x40, Arc::new(p9_handler)); self.devices.insert("softnpu-p9fs".to_string(), vio9p.clone()); - let bdf: pci::Bdf = self - .spec - .devices - .softnpu_p9 + let bdf = softnpu + .p9_device .as_ref() .ok_or_else(|| { Error::new( @@ -903,11 +846,11 @@ impl<'a> MachineInitializer<'a> { &mut self, chipset: &RegisteredChipset, ) -> Result<(), Error> { + let softnpu = &self.spec.softnpu; // Check that there is actually a p9fs device to register, if not bail // early. - let p9fs = match &self.spec.devices.p9fs { - Some(p9fs) => p9fs, - None => return Ok(()), + let Some(p9fs) = &softnpu.p9fs else { + return Ok(()); }; let bdf: pci::Bdf = p9fs.pci_path.try_into().map_err(|e| { diff --git a/bin/propolis-server/src/lib/migrate/destination.rs b/bin/propolis-server/src/lib/migrate/destination.rs index 7f23beb15..21b12f09c 100644 --- a/bin/propolis-server/src/lib/migrate/destination.rs +++ b/bin/propolis-server/src/lib/migrate/destination.rs @@ -346,8 +346,8 @@ impl RonV0 { }?; info!(self.log(), "Destination read Preamble: {:?}", preamble); - if let Err(e) = - preamble.is_migration_compatible(ensure_ctx.instance_spec()) + if let Err(e) = preamble + .is_migration_compatible(&ensure_ctx.instance_spec().clone().into()) { error!( self.log(), diff --git a/bin/propolis-server/src/lib/migrate/source.rs b/bin/propolis-server/src/lib/migrate/source.rs index a8c3c5d9a..5abc41aa1 100644 --- a/bin/propolis-server/src/lib/migrate/source.rs +++ b/bin/propolis-server/src/lib/migrate/source.rs @@ -468,7 +468,7 @@ impl<'vm, T: MigrateConn> RonV0Runner<'vm, T> { async fn sync(&mut self) -> Result<(), MigrateError> { self.update_state(MigrationState::Sync); let preamble = Preamble::new(VersionedInstanceSpec::V0( - self.vm.lock_shared().await.instance_spec().clone(), + self.vm.lock_shared().await.instance_spec().clone().into(), )); let s = ron::ser::to_string(&preamble) .map_err(codec::ProtocolError::from)?; diff --git a/bin/propolis-server/src/lib/server.rs b/bin/propolis-server/src/lib/server.rs index 6dd0c9066..2440e18a3 100644 --- a/bin/propolis-server/src/lib/server.rs +++ b/bin/propolis-server/src/lib/server.rs @@ -16,8 +16,18 @@ use std::net::SocketAddr; use std::net::SocketAddrV6; use std::sync::Arc; -use crate::serial::history_buffer::SerialHistoryOffset; -use crate::vm::VmError; +use crate::{ + serial::history_buffer::SerialHistoryOffset, + spec::{ + self, + api_spec_v0::ApiSpecError, + builder::{SpecBuilder, SpecBuilderError}, + Spec, + }, + vm::{ensure::VmEnsureRequest, VmError}, + vnc::{self, VncServer}, +}; + use dropshot::{ channel, endpoint, ApiDescription, HttpError, HttpResponseCreated, HttpResponseOk, HttpResponseUpdatedNoContent, Path, Query, RequestContext, @@ -29,18 +39,17 @@ 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::components::devices::QemuPvpanic; -use propolis_api_types::instance_spec::{self, VersionedInstanceSpec}; - +use propolis_api_types::instance_spec::{ + self, components::devices::QemuPvpanic, VersionedInstanceSpec, +}; pub use propolis_server_config::Config as VmTomlConfig; use rfb::tungstenite::BinaryWs; use slog::{error, warn, Logger}; use tokio::sync::MutexGuard; -use tokio_tungstenite::tungstenite::protocol::{Role, WebSocketConfig}; -use tokio_tungstenite::WebSocketStream; - -use crate::spec::builder::{SpecBuilder, SpecBuilderError}; -use crate::vnc::{self, VncServer}; +use tokio_tungstenite::{ + tungstenite::protocol::{Role, WebSocketConfig}, + WebSocketStream, +}; /// Configuration used to set this server up to provide Oximeter metrics. #[derive(Debug, Clone)] @@ -107,7 +116,7 @@ impl DropshotEndpointContext { fn instance_spec_from_request( request: &api::InstanceEnsureRequest, toml_config: &VmTomlConfig, -) -> Result { +) -> Result { let mut spec_builder = SpecBuilder::new(&request.properties); spec_builder.add_devices_from_config(toml_config)?; @@ -135,8 +144,15 @@ fn instance_spec_from_request( spec_builder.add_serial_port(port)?; } - spec_builder.add_pvpanic_device(QemuPvpanic { enable_isa: true })?; - Ok(VersionedInstanceSpec::V0(spec_builder.finish())) + #[cfg(feature = "falcon")] + spec_builder.set_softnpu_com4()?; + + 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 @@ -217,14 +233,16 @@ async fn find_local_nexus_client( async fn instance_ensure_common( rqctx: RequestContext>, - request: api::InstanceSpecEnsureRequest, + properties: api::InstanceProperties, + migrate: Option, + instance_spec: Spec, ) -> Result, HttpError> { let server_context = rqctx.context(); let oximeter_registry = server_context .static_config .metrics .as_ref() - .map(|_| ProducerRegistry::with_id(request.properties.id)); + .map(|_| ProducerRegistry::with_id(properties.id)); let nexus_client = find_local_nexus_client(rqctx.server.local_addr, rqctx.log.clone()) @@ -240,6 +258,7 @@ async fn instance_ensure_common( local_server_addr: rqctx.server.local_addr, }; + let request = VmEnsureRequest { properties, migrate, instance_spec }; server_context .vm .ensure(&server_context.log, request, ensure_options) @@ -289,11 +308,9 @@ async fn instance_ensure( instance_ensure_common( rqctx, - api::InstanceSpecEnsureRequest { - properties: request.properties, - instance_spec, - migrate: request.migrate, - }, + request.properties, + request.migrate, + instance_spec, ) .await } @@ -306,7 +323,14 @@ async fn instance_spec_ensure( rqctx: RequestContext>, request: TypedBody, ) -> Result, HttpError> { - instance_ensure_common(rqctx, request.into_inner()).await + 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, e.to_string()) + })?; + + instance_ensure_common(rqctx, request.properties, request.migrate, spec) + .await } async fn instance_get_common( diff --git a/bin/propolis-server/src/lib/spec/api_request.rs b/bin/propolis-server/src/lib/spec/api_request.rs index 7c84be5a5..b778a485f 100644 --- a/bin/propolis-server/src/lib/spec/api_request.rs +++ b/bin/propolis-server/src/lib/spec/api_request.rs @@ -15,17 +15,16 @@ use propolis_api_types::{ }, devices::{NvmeDisk, VirtioDisk, VirtioNic}, }, - v0::{ - NetworkBackendV0, NetworkDeviceV0, StorageBackendV0, - StorageDeviceV0, - }, PciPath, }, DiskRequest, NetworkInterfaceRequest, Slot, }; use thiserror::Error; -use super::{ParsedNetworkDevice, ParsedStorageDevice}; +use super::{ + Disk, Nic, ParsedDiskRequest, ParsedNicRequest, StorageBackend, + StorageDevice, +}; #[derive(Debug, Error)] pub(crate) enum DeviceRequestError { @@ -71,15 +70,15 @@ fn slot_to_pci_path( pub(super) fn parse_disk_from_request( disk: &DiskRequest, -) -> Result { +) -> Result { let pci_path = slot_to_pci_path(disk.slot, SlotType::Disk)?; let backend_name = format!("{}-backend", disk.name); let device_spec = match disk.device.as_ref() { - "virtio" => StorageDeviceV0::VirtioDisk(VirtioDisk { + "virtio" => StorageDevice::Virtio(VirtioDisk { backend_name: backend_name.clone(), pci_path, }), - "nvme" => StorageDeviceV0::NvmeDisk(NvmeDisk { + "nvme" => StorageDevice::Nvme(NvmeDisk { backend_name: backend_name.clone(), pci_path, }), @@ -92,7 +91,7 @@ pub(super) fn parse_disk_from_request( }; let device_name = disk.name.clone(); - let backend_spec = StorageBackendV0::Crucible(CrucibleStorageBackend { + 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) @@ -100,57 +99,47 @@ pub(super) fn parse_disk_from_request( readonly: disk.read_only, }); - Ok(ParsedStorageDevice { - device_name, - device_spec, - backend_name, - backend_spec, + Ok(ParsedDiskRequest { + name: device_name, + disk: Disk { device_spec, backend_name, backend_spec }, }) } pub(super) fn parse_cloud_init_from_request( base64: String, -) -> Result { +) -> Result { let name = "cloud-init"; let pci_path = slot_to_pci_path(Slot(0), SlotType::CloudInit)?; let backend_name = name.to_string(); let backend_spec = - StorageBackendV0::Blob(BlobStorageBackend { base64, readonly: true }); + StorageBackend::Blob(BlobStorageBackend { base64, readonly: true }); - let device_name = name.to_string(); - let device_spec = StorageDeviceV0::VirtioDisk(VirtioDisk { + let device_spec = StorageDevice::Virtio(VirtioDisk { backend_name: name.to_string(), pci_path, }); - Ok(ParsedStorageDevice { - device_name, - device_spec, - backend_name, - backend_spec, + Ok(ParsedDiskRequest { + name: name.to_owned(), + disk: Disk { device_spec, backend_name, backend_spec }, }) } pub(super) fn parse_nic_from_request( nic: &NetworkInterfaceRequest, -) -> Result { +) -> 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 = NetworkDeviceV0::VirtioNic(VirtioNic { + let device_spec = VirtioNic { backend_name: backend_name.clone(), interface_id: nic.interface_id, pci_path, - }); - - let backend_spec = NetworkBackendV0::Virtio(VirtioNetworkBackend { - vnic_name: nic.name.to_string(), - }); + }; - Ok(ParsedNetworkDevice { - device_name, - device_spec, - backend_name, - backend_spec, + let backend_spec = VirtioNetworkBackend { vnic_name: nic.name.to_string() }; + Ok(ParsedNicRequest { + name: device_name, + nic: Nic { device_spec, backend_name, backend_spec }, }) } @@ -161,15 +150,13 @@ mod test { use super::*; - fn check_parsed_storage_device_backend_pointer( - parsed: &ParsedStorageDevice, - ) { - let device_to_backend = match &parsed.device_spec { - StorageDeviceV0::VirtioDisk(d) => d.backend_name.clone(), - StorageDeviceV0::NvmeDisk(d) => d.backend_name.clone(), + fn check_parsed_storage_device_backend_pointer(parsed: &ParsedDiskRequest) { + let device_to_backend = match &parsed.disk.device_spec { + StorageDevice::Virtio(d) => d.backend_name.clone(), + StorageDevice::Nvme(d) => d.backend_name.clone(), }; - assert_eq!(device_to_backend, parsed.backend_name); + assert_eq!(device_to_backend, parsed.disk.backend_name); } #[test] @@ -201,8 +188,8 @@ mod test { }; let parsed = parse_nic_from_request(&req).unwrap(); - let NetworkDeviceV0::VirtioNic(nic) = &parsed.device_spec; - assert_eq!(nic.backend_name, parsed.backend_name); + let VirtioNic { backend_name, .. } = &parsed.nic.device_spec; + assert_eq!(*backend_name, parsed.nic.backend_name); } #[test] diff --git a/bin/propolis-server/src/lib/spec/api_spec_v0.rs b/bin/propolis-server/src/lib/spec/api_spec_v0.rs new file mode 100644 index 000000000..a41b5185e --- /dev/null +++ b/bin/propolis-server/src/lib/spec/api_spec_v0.rs @@ -0,0 +1,274 @@ +// 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/. + +//! Conversions from version-0 instance specs in the [`propolis_api_types`] +//! crate to the internal [`super::Spec`] representation. + +use std::collections::HashMap; + +use propolis_api_types::instance_spec::{ + components::devices::{SerialPort as SerialPortDesc, SerialPortNumber}, + v0::{InstanceSpecV0, NetworkBackendV0, NetworkDeviceV0, StorageDeviceV0}, +}; +use thiserror::Error; + +#[cfg(feature = "falcon")] +use propolis_api_types::instance_spec::components::devices::SoftNpuPort as SoftNpuPortSpec; + +#[cfg(feature = "falcon")] +use crate::spec::SoftNpuPort; + +use super::{ + builder::{SpecBuilder, SpecBuilderError}, + Disk, Nic, QemuPvpanic, SerialPortDevice, Spec, +}; + +#[derive(Debug, Error)] +pub(crate) enum ApiSpecError { + #[error(transparent)] + Builder(#[from] SpecBuilderError), + + #[error("backend {backend} not found for device {device}")] + BackendNotFound { backend: String, device: String }, + + #[error("backend {0} not used by any device")] + BackendNotUsed(String), + + #[error("network backend for guest NIC {0} is not a viona backend")] + GuestNicInvalidBackend(String), + + #[cfg(feature = "falcon")] + #[error("network backend for device {0} is not a DLPI backend")] + NotDlpiBackend(String), +} + +impl From for InstanceSpecV0 { + fn from(val: Spec) -> Self { + // Inserts a component entry into the supplied map, asserting first that + // the supplied key is not present in that map. + // + // This assertion is valid because internal instance specs should assign + // a unique name to each component they describe. The spec builder + // upholds this invariant at spec creation time. + #[track_caller] + fn insert_component( + map: &mut HashMap, + key: String, + val: T, + ) { + assert!( + !map.contains_key(&key), + "component name {} already exists in output spec", + &key + ); + map.insert(key, val); + } + + let mut spec = InstanceSpecV0::default(); + spec.devices.board = val.board; + for (disk_name, disk) in val.disks { + insert_component( + &mut spec.devices.storage_devices, + disk_name, + disk.device_spec.into(), + ); + + insert_component( + &mut spec.backends.storage_backends, + disk.backend_name, + disk.backend_spec.into(), + ); + } + + for (nic_name, nic) in val.nics { + insert_component( + &mut spec.devices.network_devices, + nic_name, + NetworkDeviceV0::VirtioNic(nic.device_spec), + ); + + insert_component( + &mut spec.backends.network_backends, + nic.backend_name, + NetworkBackendV0::Virtio(nic.backend_spec), + ); + } + + for (num, user) in val.serial.iter() { + if *user == SerialPortDevice::Uart { + let name = match num { + SerialPortNumber::Com1 => "com1", + SerialPortNumber::Com2 => "com2", + SerialPortNumber::Com3 => "com3", + SerialPortNumber::Com4 => "com4", + }; + + insert_component( + &mut spec.devices.serial_ports, + name.to_owned(), + SerialPortDesc { num: *num }, + ); + } + } + + for (bridge_name, bridge) in val.pci_pci_bridges { + insert_component( + &mut spec.devices.pci_pci_bridges, + bridge_name, + bridge, + ); + } + + spec.devices.qemu_pvpanic = val.pvpanic.map(|pvpanic| pvpanic.spec); + + #[cfg(feature = "falcon")] + { + spec.devices.softnpu_pci_port = val.softnpu.pci_port; + spec.devices.softnpu_p9 = val.softnpu.p9_device; + spec.devices.p9fs = val.softnpu.p9fs; + for (port_name, port) in val.softnpu.ports { + insert_component( + &mut spec.devices.softnpu_ports, + port_name.clone(), + SoftNpuPortSpec { + name: port_name, + backend_name: port.backend_name.clone(), + }, + ); + + insert_component( + &mut spec.backends.network_backends, + port.backend_name, + NetworkBackendV0::Dlpi(port.backend_spec), + ); + } + } + + spec + } +} + +impl TryFrom for Spec { + type Error = ApiSpecError; + + fn try_from(mut value: InstanceSpecV0) -> Result { + let mut builder = SpecBuilder::with_board(value.devices.board); + + // Examine each storage device and peel its backend off of the input + // spec. + for (device_name, device_spec) in value.devices.storage_devices { + let backend_name = match &device_spec { + StorageDeviceV0::VirtioDisk(disk) => &disk.backend_name, + StorageDeviceV0::NvmeDisk(disk) => &disk.backend_name, + }; + + let (backend_name, backend_spec) = value + .backends + .storage_backends + .remove_entry(backend_name) + .ok_or_else(|| ApiSpecError::BackendNotFound { + backend: backend_name.to_owned(), + device: device_name.clone(), + })?; + + builder.add_storage_device( + device_name, + Disk { + device_spec: device_spec.into(), + backend_name, + backend_spec: backend_spec.into(), + }, + )?; + } + + // Once all the devices have been checked, there should be no unpaired + // backends remaining. + if let Some(backend) = value.backends.storage_backends.keys().next() { + return Err(ApiSpecError::BackendNotUsed(backend.to_owned())); + } + + // Repeat this process for network devices. + for (device_name, device_spec) in value.devices.network_devices { + let NetworkDeviceV0::VirtioNic(device_spec) = device_spec; + let backend_name = &device_spec.backend_name; + let (backend_name, backend_spec) = value + .backends + .network_backends + .remove_entry(backend_name) + .ok_or_else(|| ApiSpecError::BackendNotFound { + backend: backend_name.to_owned(), + device: device_name.clone(), + })?; + + let NetworkBackendV0::Virtio(backend_spec) = backend_spec else { + return Err(ApiSpecError::GuestNicInvalidBackend(device_name)); + }; + + builder.add_network_device( + device_name, + Nic { device_spec, backend_name, backend_spec }, + )?; + } + + // SoftNPU ports can have network backends, so consume the SoftNPU + // device fields before checking to see if the network backend list is + // empty. + #[cfg(feature = "falcon")] + { + if let Some(softnpu_pci) = value.devices.softnpu_pci_port { + builder.set_softnpu_pci_port(softnpu_pci)?; + } + + if let Some(softnpu_p9) = value.devices.softnpu_p9 { + builder.set_softnpu_p9(softnpu_p9)?; + } + + if let Some(p9fs) = value.devices.p9fs { + builder.set_p9fs(p9fs)?; + } + + for (port_name, port) in value.devices.softnpu_ports { + let (backend_name, backend_spec) = value + .backends + .network_backends + .remove_entry(&port.backend_name) + .ok_or_else(|| ApiSpecError::BackendNotFound { + backend: port.backend_name, + device: port_name.clone(), + })?; + + let NetworkBackendV0::Dlpi(backend_spec) = backend_spec else { + return Err(ApiSpecError::NotDlpiBackend(port_name)); + }; + + builder.add_softnpu_port( + port_name, + SoftNpuPort { backend_name, backend_spec }, + )?; + } + } + + if let Some(backend) = value.backends.network_backends.keys().next() { + return Err(ApiSpecError::BackendNotUsed(backend.to_owned())); + } + + // TODO(#735): Serial ports need to have names like other devices. + for serial_port in value.devices.serial_ports.values() { + builder.add_serial_port(serial_port.num)?; + } + + for (name, bridge) in value.devices.pci_pci_bridges { + builder.add_pci_bridge(name, bridge)?; + } + + if let Some(pvpanic) = value.devices.qemu_pvpanic { + builder.add_pvpanic_device(QemuPvpanic { + name: "pvpanic".to_string(), + spec: pvpanic, + })?; + } + + Ok(builder.finish()) + } +} diff --git a/bin/propolis-server/src/lib/spec/builder.rs b/bin/propolis-server/src/lib/spec/builder.rs index adb5658d4..d13f25adc 100644 --- a/bin/propolis-server/src/lib/spec/builder.rs +++ b/bin/propolis-server/src/lib/spec/builder.rs @@ -10,11 +10,8 @@ use propolis_api_types::{ instance_spec::{ components::{ board::{Board, Chipset, I440Fx}, - devices::{ - PciPciBridge, QemuPvpanic, SerialPort, SerialPortNumber, - }, + devices::{PciPciBridge, SerialPortNumber}, }, - v0::{DeviceSpecV0, InstanceSpecV0, NetworkDeviceV0, StorageDeviceV0}, PciPath, }, DiskRequest, InstanceProperties, NetworkInterfaceRequest, @@ -22,27 +19,22 @@ use propolis_api_types::{ use thiserror::Error; #[cfg(feature = "falcon")] -use propolis_api_types::instance_spec::{ - components::{ - backends::DlpiNetworkBackend, - devices::{P9fs, SoftNpuP9, SoftNpuPciPort, SoftNpuPort}, - }, - v0::NetworkBackendV0, +use propolis_api_types::instance_spec::components::devices::{ + P9fs, SoftNpuP9, SoftNpuPciPort, }; -use crate::config; +use crate::{config, spec::SerialPortDevice}; use super::{ api_request::{self, DeviceRequestError}, config_toml::{ConfigTomlError, ParsedConfig}, - ParsedNetworkDevice, ParsedStorageDevice, + Disk, Nic, QemuPvpanic, }; #[cfg(feature = "falcon")] -use super::ParsedSoftNpu; +use super::{ParsedSoftNpu, SoftNpuPort}; /// Errors that can arise while building an instance spec from component parts. -#[allow(clippy::enum_variant_names)] #[derive(Debug, Error)] pub(crate) enum SpecBuilderError { #[error("error parsing config TOML")] @@ -51,11 +43,8 @@ pub(crate) enum SpecBuilderError { #[error("error parsing device in ensure request")] DeviceRequest(#[from] DeviceRequestError), - #[error("A device with name {0} already exists")] - DeviceNameInUse(String), - - #[error("A backend with name {0} already exists")] - BackendNameInUse(String), + #[error("A component with name {0} already exists")] + ComponentNameInUse(String), #[error("A PCI device is already attached at {0:?}")] PciPathInUse(PciPath), @@ -63,35 +52,15 @@ pub(crate) enum SpecBuilderError { #[error("Serial port {0:?} is already specified")] SerialPortInUse(SerialPortNumber), - #[cfg(feature = "falcon")] - #[error("SoftNpu port {0:?} is already specified")] - SoftNpuPortInUse(String), + #[error("pvpanic device already specified")] + PvpanicInUse, } +#[derive(Debug, Default)] pub(crate) struct SpecBuilder { - spec: InstanceSpecV0, + spec: super::Spec, pci_paths: BTreeSet, -} - -trait PciComponent { - fn pci_path(&self) -> PciPath; -} - -impl PciComponent for StorageDeviceV0 { - fn pci_path(&self) -> PciPath { - match self { - StorageDeviceV0::VirtioDisk(disk) => disk.pci_path, - StorageDeviceV0::NvmeDisk(disk) => disk.pci_path, - } - } -} - -impl PciComponent for NetworkDeviceV0 { - fn pci_path(&self) -> PciPath { - match self { - NetworkDeviceV0::VirtioNic(nic) => nic.pci_path, - } - } + component_names: BTreeSet, } impl SpecBuilder { @@ -103,11 +72,15 @@ impl SpecBuilder { }; Self { - spec: InstanceSpecV0 { - devices: DeviceSpecV0 { board, ..Default::default() }, - backends: Default::default(), - }, - pci_paths: Default::default(), + spec: super::Spec { board, ..Default::default() }, + ..Default::default() + } + } + + pub(super) fn with_board(board: Board) -> Self { + Self { + spec: super::Spec { board, ..Default::default() }, + ..Default::default() } } @@ -117,7 +90,8 @@ impl SpecBuilder { &mut self, nic: &NetworkInterfaceRequest, ) -> Result<(), SpecBuilderError> { - self.add_network_device(api_request::parse_nic_from_request(nic)?)?; + let parsed = api_request::parse_nic_from_request(nic)?; + self.add_network_device(parsed.name, parsed.nic)?; Ok(()) } @@ -127,7 +101,8 @@ impl SpecBuilder { &mut self, disk: &DiskRequest, ) -> Result<(), SpecBuilderError> { - self.add_storage_device(api_request::parse_disk_from_request(disk)?)?; + let parsed = api_request::parse_disk_from_request(disk)?; + self.add_storage_device(parsed.name, parsed.disk)?; Ok(()) } @@ -137,10 +112,8 @@ impl SpecBuilder { &mut self, base64: String, ) -> Result<(), SpecBuilderError> { - self.add_storage_device(api_request::parse_cloud_init_from_request( - base64, - )?)?; - + let parsed = api_request::parse_cloud_init_from_request(base64)?; + self.add_storage_device(parsed.name, parsed.disk)?; Ok(()) } @@ -152,15 +125,15 @@ impl SpecBuilder { ) -> Result<(), SpecBuilderError> { let parsed = ParsedConfig::try_from(config)?; - let Chipset::I440Fx(ref mut i440fx) = self.spec.devices.board.chipset; + 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)?; + self.add_storage_device(disk.name, disk.disk)?; } for nic in parsed.nics { - self.add_network_device(nic)?; + self.add_network_device(nic.name, nic.nic)?; } for bridge in parsed.pci_bridges { @@ -178,19 +151,19 @@ impl SpecBuilder { &mut self, devices: ParsedSoftNpu, ) -> Result<(), SpecBuilderError> { - for pci_port in devices.pci_ports { + 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.clone(), port)?; + self.add_softnpu_port(port.name, port.port)?; } - for p9 in devices.p9_devices { + if let Some(p9) = devices.p9_device { self.set_softnpu_p9(p9)?; } - for p9fs in devices.p9fs { + if let Some(p9fs) = devices.p9fs { self.set_p9fs(p9fs)?; } @@ -214,31 +187,23 @@ impl SpecBuilder { /// Adds a storage device with an associated backend. pub(super) fn add_storage_device( &mut self, - ParsedStorageDevice { - device_name, - device_spec, - backend_name, - backend_spec, - }: ParsedStorageDevice, + disk_name: String, + disk: Disk, ) -> Result<&Self, SpecBuilderError> { - if self.spec.devices.storage_devices.contains_key(&device_name) { - return Err(SpecBuilderError::DeviceNameInUse(device_name)); + if self.component_names.contains(&disk_name) { + return Err(SpecBuilderError::ComponentNameInUse(disk_name)); } - if self.spec.backends.storage_backends.contains_key(&backend_name) { - return Err(SpecBuilderError::BackendNameInUse(backend_name)); + if self.component_names.contains(&disk.backend_name) { + return Err(SpecBuilderError::ComponentNameInUse( + disk.backend_name, + )); } - self.register_pci_device(device_spec.pci_path())?; - let _old = - self.spec.devices.storage_devices.insert(device_name, device_spec); - - assert!(_old.is_none()); - let _old = self - .spec - .backends - .storage_backends - .insert(backend_name, backend_spec); + self.register_pci_device(disk.device_spec.pci_path())?; + self.component_names.insert(disk_name.clone()); + self.component_names.insert(disk.backend_name.clone()); + let _old = self.spec.disks.insert(disk_name, disk); assert!(_old.is_none()); Ok(self) } @@ -246,32 +211,21 @@ impl SpecBuilder { /// Adds a network device with an associated backend. pub(super) fn add_network_device( &mut self, - ParsedNetworkDevice { - device_name, - device_spec, - backend_name, - backend_spec, - }: ParsedNetworkDevice, + nic_name: String, + nic: Nic, ) -> Result<&Self, SpecBuilderError> { - if self.spec.devices.network_devices.contains_key(&device_name) { - return Err(SpecBuilderError::DeviceNameInUse(device_name)); + if self.component_names.contains(&nic_name) { + return Err(SpecBuilderError::ComponentNameInUse(nic_name)); } - if self.spec.backends.network_backends.contains_key(&backend_name) { - return Err(SpecBuilderError::BackendNameInUse(backend_name)); + if self.component_names.contains(&nic.backend_name) { + return Err(SpecBuilderError::ComponentNameInUse(nic.backend_name)); } - self.register_pci_device(device_spec.pci_path())?; - let _old = - self.spec.devices.network_devices.insert(device_name, device_spec); - - assert!(_old.is_none()); - let _old = self - .spec - .backends - .network_backends - .insert(backend_name, backend_spec); - + self.register_pci_device(nic.device_spec.pci_path)?; + self.component_names.insert(nic_name.clone()); + self.component_names.insert(nic.backend_name.clone()); + let _old = self.spec.nics.insert(nic_name, nic); assert!(_old.is_none()); Ok(self) } @@ -279,17 +233,15 @@ impl SpecBuilder { /// Adds a PCI-PCI bridge. pub fn add_pci_bridge( &mut self, - bridge_name: String, - bridge_spec: PciPciBridge, + name: String, + bridge: PciPciBridge, ) -> Result<&Self, SpecBuilderError> { - if self.spec.devices.pci_pci_bridges.contains_key(&bridge_name) { - return Err(SpecBuilderError::DeviceNameInUse(bridge_name)); + if self.component_names.contains(&name) { + return Err(SpecBuilderError::ComponentNameInUse(name)); } - self.register_pci_device(bridge_spec.pci_path)?; - let _old = - self.spec.devices.pci_pci_bridges.insert(bridge_name, bridge_spec); - + self.register_pci_device(bridge.pci_path)?; + let _old = self.spec.pci_pci_bridges.insert(name, bridge); assert!(_old.is_none()); Ok(self) } @@ -299,22 +251,7 @@ impl SpecBuilder { &mut self, port: SerialPortNumber, ) -> Result<&Self, SpecBuilderError> { - if self - .spec - .devices - .serial_ports - .insert( - match port { - SerialPortNumber::Com1 => "com1", - SerialPortNumber::Com2 => "com2", - SerialPortNumber::Com3 => "com3", - SerialPortNumber::Com4 => "com4", - } - .to_string(), - SerialPort { num: port }, - ) - .is_some() - { + if self.spec.serial.insert(port, SerialPortDevice::Uart).is_some() { Err(SpecBuilderError::SerialPortInUse(port)) } else { Ok(self) @@ -325,24 +262,31 @@ impl SpecBuilder { &mut self, pvpanic: QemuPvpanic, ) -> Result<&Self, SpecBuilderError> { - if self.spec.devices.qemu_pvpanic.is_some() { - return Err(SpecBuilderError::DeviceNameInUse( - "pvpanic".to_string(), - )); + if self.spec.pvpanic.is_some() { + return Err(SpecBuilderError::PvpanicInUse); } - self.spec.devices.qemu_pvpanic = Some(pvpanic); - + self.spec.pvpanic = Some(pvpanic); Ok(self) } + #[cfg(feature = "falcon")] + pub fn set_softnpu_com4(&mut self) -> Result<&Self, SpecBuilderError> { + let port = SerialPortNumber::Com4; + if self.spec.serial.insert(port, SerialPortDevice::SoftNpu).is_some() { + Err(SpecBuilderError::SerialPortInUse(port)) + } else { + 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.devices.softnpu_pci_port = Some(pci_port); + self.spec.softnpu.pci_port = Some(pci_port); Ok(self) } @@ -352,39 +296,40 @@ impl SpecBuilder { p9: SoftNpuP9, ) -> Result<&Self, SpecBuilderError> { self.register_pci_device(p9.pci_path)?; - self.spec.devices.softnpu_p9 = Some(p9); + self.spec.softnpu.p9_device = Some(p9); Ok(self) } #[cfg(feature = "falcon")] pub fn set_p9fs(&mut self, p9fs: P9fs) -> Result<&Self, SpecBuilderError> { self.register_pci_device(p9fs.pci_path)?; - self.spec.devices.p9fs = Some(p9fs); + self.spec.softnpu.p9fs = Some(p9fs); Ok(self) } #[cfg(feature = "falcon")] pub fn add_softnpu_port( &mut self, - key: String, + port_name: String, port: SoftNpuPort, ) -> Result<&Self, SpecBuilderError> { - let _old = self.spec.backends.network_backends.insert( - port.backend_name.clone(), - NetworkBackendV0::Dlpi(DlpiNetworkBackend { - vnic_name: port.backend_name.clone(), - }), - ); - assert!(_old.is_none()); - if self.spec.devices.softnpu_ports.insert(key, port.clone()).is_some() { - Err(SpecBuilderError::SoftNpuPortInUse(port.name)) - } else { - Ok(self) + if self.component_names.contains(&port_name) { + return Err(SpecBuilderError::ComponentNameInUse(port_name)); + } + + if self.component_names.contains(&port.backend_name) { + return Err(SpecBuilderError::ComponentNameInUse( + port.backend_name, + )); } + + let _old = self.spec.softnpu.ports.insert(port_name, port); + assert!(_old.is_none()); + Ok(self) } /// Yields the completed spec, consuming the builder. - pub fn finish(self) -> InstanceSpecV0 { + pub fn finish(self) -> super::Spec { self.spec } } diff --git a/bin/propolis-server/src/lib/spec/config_toml.rs b/bin/propolis-server/src/lib/spec/config_toml.rs index 337c24bb3..2de57ad8f 100644 --- a/bin/propolis-server/src/lib/spec/config_toml.rs +++ b/bin/propolis-server/src/lib/spec/config_toml.rs @@ -11,24 +11,24 @@ use propolis_api_types::instance_spec::{ backends::{FileStorageBackend, VirtioNetworkBackend}, devices::{NvmeDisk, PciPciBridge, VirtioDisk, VirtioNic}, }, - v0::{ - NetworkBackendV0, NetworkDeviceV0, StorageBackendV0, StorageDeviceV0, - }, PciPath, }; use thiserror::Error; #[cfg(feature = "falcon")] use propolis_api_types::instance_spec::components::devices::{ - P9fs, SoftNpuP9, SoftNpuPciPort, SoftNpuPort, + P9fs, SoftNpuP9, SoftNpuPciPort, }; use crate::config; -use super::{ParsedNetworkDevice, ParsedStorageDevice}; +use super::{ + Disk, Nic, ParsedDiskRequest, ParsedNicRequest, ParsedPciBridgeRequest, + StorageBackend, StorageDevice, +}; #[cfg(feature = "falcon")] -use super::ParsedSoftNpu; +use super::{ParsedSoftNpu, ParsedSoftNpuPort, SoftNpuPort}; #[derive(Debug, Error)] pub(crate) enum ConfigTomlError { @@ -77,9 +77,9 @@ pub(crate) enum ConfigTomlError { #[derive(Default)] pub(super) struct ParsedConfig { pub(super) enable_pcie: bool, - pub(super) disks: Vec, - pub(super) nics: Vec, - pub(super) pci_bridges: Vec, + pub(super) disks: Vec, + pub(super) nics: Vec, + pub(super) pci_bridges: Vec, #[cfg(feature = "falcon")] pub(super) softnpu: ParsedSoftNpu, @@ -114,12 +114,10 @@ impl TryFrom<&config::Config> for ParsedConfig { parse_storage_device_from_config(device_name, device)?; let backend_name = match &device_spec { - StorageDeviceV0::VirtioDisk(disk) => { - disk.backend_name.clone() - } - StorageDeviceV0::NvmeDisk(disk) => { + StorageDevice::Virtio(disk) => { disk.backend_name.clone() } + StorageDevice::Nvme(disk) => disk.backend_name.clone(), }; let backend_config = @@ -135,11 +133,9 @@ impl TryFrom<&config::Config> for ParsedConfig { backend_config, )?; - parsed.disks.push(ParsedStorageDevice { - device_name: device_name.to_owned(), - device_spec, - backend_name, - backend_spec, + parsed.disks.push(ParsedDiskRequest { + name: device_name.to_owned(), + disk: Disk { device_spec, backend_name, backend_spec }, }); } "pci-virtio-viona" => { @@ -150,12 +146,11 @@ impl TryFrom<&config::Config> for ParsedConfig { } #[cfg(feature = "falcon")] "softnpu-pci-port" => { - parsed.softnpu.pci_ports.push( - parse_softnpu_pci_port_from_config( + parsed.softnpu.pci_port = + Some(parse_softnpu_pci_port_from_config( device_name, device, - )?, - ); + )?); } #[cfg(feature = "falcon")] "softnpu-port" => { @@ -166,16 +161,14 @@ impl TryFrom<&config::Config> for ParsedConfig { } #[cfg(feature = "falcon")] "softnpu-p9" => { - parsed.softnpu.p9_devices.push( + parsed.softnpu.p9_device = Some( parse_softnpu_p9_from_config(device_name, device)?, ); } #[cfg(feature = "falcon")] "pci-virtio-9p" => { - parsed - .softnpu - .p9fs - .push(parse_p9fs_from_config(device_name, device)?); + parsed.softnpu.p9fs = + Some(parse_p9fs_from_config(device_name, device)?); } _ => { return Err(ConfigTomlError::UnrecognizedDeviceType( @@ -196,9 +189,9 @@ impl TryFrom<&config::Config> for ParsedConfig { pub(super) fn parse_storage_backend_from_config( name: &str, backend: &config::BlockDevice, -) -> Result { +) -> Result { let backend_spec = match backend.bdtype.as_str() { - "file" => StorageBackendV0::File(FileStorageBackend { + "file" => StorageBackend::File(FileStorageBackend { path: backend .options .get("path") @@ -238,7 +231,7 @@ pub(super) fn parse_storage_backend_from_config( pub(super) fn parse_storage_device_from_config( name: &str, device: &config::Device, -) -> Result { +) -> Result { enum Interface { Virtio, Nvme, @@ -273,10 +266,10 @@ pub(super) fn parse_storage_device_from_config( Ok(match interface { Interface::Virtio => { - StorageDeviceV0::VirtioDisk(VirtioDisk { backend_name, pci_path }) + StorageDevice::Virtio(VirtioDisk { backend_name, pci_path }) } Interface::Nvme => { - StorageDeviceV0::NvmeDisk(NvmeDisk { backend_name, pci_path }) + StorageDevice::Nvme(NvmeDisk { backend_name, pci_path }) } }) } @@ -284,7 +277,7 @@ pub(super) fn parse_storage_device_from_config( pub(super) fn parse_network_device_from_config( name: &str, device: &config::Device, -) -> Result { +) -> Result { let vnic_name = device .get_string("vnic") .ok_or_else(|| ConfigTomlError::NoVnicName(name.to_owned()))?; @@ -294,40 +287,30 @@ pub(super) fn parse_network_device_from_config( .ok_or_else(|| ConfigTomlError::InvalidPciPath(name.to_owned()))?; let (device_name, backend_name) = super::pci_path_to_nic_names(pci_path); - let backend_spec = NetworkBackendV0::Virtio(VirtioNetworkBackend { - vnic_name: vnic_name.to_owned(), - }); - - let device_spec = NetworkDeviceV0::VirtioNic(VirtioNic { + let backend_spec = VirtioNetworkBackend { vnic_name: vnic_name.to_owned() }; + let device_spec = VirtioNic { backend_name: backend_name.clone(), - // We don't allow for configuration to specify the interface_id, so we - // generate a new one. + // NICs added by the configuration TOML have no control plane- + // supplied correlation IDs. interface_id: uuid::Uuid::nil(), pci_path, - }); + }; - Ok(ParsedNetworkDevice { - device_name, - device_spec, - backend_name, - backend_spec, + Ok(ParsedNicRequest { + name: device_name, + nic: Nic { device_spec, backend_name, backend_spec }, }) } -pub(super) struct ParsedPciPciBridge { - pub(super) name: String, - pub(super) bridge: PciPciBridge, -} - pub(super) fn parse_pci_bridge_from_config( bridge: &config::PciBridge, -) -> Result { - let name = format!("pci-bridge-{}", bridge.downstream_bus); +) -> Result { let pci_path = PciPath::from_str(&bridge.pci_path).map_err(|e| { ConfigTomlError::PciPathParseFailed(bridge.pci_path.to_string(), e) })?; - Ok(ParsedPciPciBridge { + let name = format!("pci-bridge-{}", bridge.pci_path); + Ok(ParsedPciBridgeRequest { name, bridge: PciPciBridge { downstream_bus: bridge.downstream_bus, @@ -364,14 +347,21 @@ pub(super) fn parse_softnpu_pci_port_from_config( pub(super) fn parse_softnpu_port_from_config( name: &str, device: &config::Device, -) -> Result { +) -> 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(SoftNpuPort { + Ok(ParsedSoftNpuPort { name: name.to_owned(), - backend_name: vnic_name.to_owned(), + port: SoftNpuPort { + backend_name: vnic_name.to_owned(), + backend_spec: DlpiNetworkBackend { + vnic_name: vnic_name.to_owned(), + }, + }, }) } diff --git a/bin/propolis-server/src/lib/spec/mod.rs b/bin/propolis-server/src/lib/spec/mod.rs index 3826d119f..706bef589 100644 --- a/bin/propolis-server/src/lib/spec/mod.rs +++ b/bin/propolis-server/src/lib/spec/mod.rs @@ -2,44 +2,207 @@ // 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/. -//! Helper functions for building instance specs from server parameters. +//! Instance specs describe how to configure a VM and what components it has. +//! +//! This module defines a crate-internal instance spec type, [`Spec`], and its +//! constituent types, like [`Disk`] and [`Nic`]. Unlike the types in +//! [`propolis_api_types::instance_spec`], these internal types are not +//! `Serialize` and are never meant to be used over the wire in API requests or +//! the migration protocol. This allows them to change freely between Propolis +//! versions, so long as they can consistently be converted to and from the +//! wire-format types in the [`propolis_api_types`] crate. This, in turn, allows +//! [`Spec`] and its component types to take forms that might otherwise be hard +//! to change in a backward-compatible way. -use propolis_api_types::instance_spec::{v0::*, PciPath}; +use std::collections::HashMap; + +use propolis_api_types::instance_spec::{ + components::{ + backends::{ + BlobStorageBackend, CrucibleStorageBackend, FileStorageBackend, + VirtioNetworkBackend, + }, + board::Board, + devices::{ + NvmeDisk, PciPciBridge, QemuPvpanic as QemuPvpanicDesc, + SerialPortNumber, VirtioDisk, VirtioNic, + }, + }, + v0::{StorageBackendV0, StorageDeviceV0}, + PciPath, +}; #[cfg(feature = "falcon")] -use propolis_api_types::instance_spec::components::devices::{ - P9fs, SoftNpuP9, SoftNpuPciPort, SoftNpuPort, +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; -/// Describes a storage device/backend pair parsed from an input source like an -/// API request or a config TOML entry. -struct ParsedStorageDevice { - device_name: String, - device_spec: StorageDeviceV0, - backend_name: String, - backend_spec: StorageBackendV0, +/// An instance specification that describes a VM's configuration and +/// components. +/// +/// NOTE: This struct's fields are `pub` to make it convenient to access the +/// individual parts of a fully-constructed spec. Modules that consume specs may +/// assert that they are valid (no duplicate component names, no duplicate PCI +/// device paths, etc.). When constructing a new spec, use the +/// [`builder::SpecBuilder`] struct to catch requests that violate these +/// invariants. +#[derive(Clone, Debug, Default)] +pub(crate) struct Spec { + pub board: Board, + pub disks: HashMap, + pub nics: HashMap, + + pub serial: HashMap, + + pub pci_pci_bridges: HashMap, + pub pvpanic: Option, + + #[cfg(feature = "falcon")] + pub softnpu: SoftNpu, +} + +/// Describes the device half of a [`Disk`]. +#[derive(Clone, Debug)] +pub enum StorageDevice { + Virtio(VirtioDisk), + Nvme(NvmeDisk), +} + +impl StorageDevice { + pub fn pci_path(&self) -> PciPath { + match self { + StorageDevice::Virtio(disk) => disk.pci_path, + StorageDevice::Nvme(disk) => disk.pci_path, + } + } +} + +impl From for StorageDeviceV0 { + fn from(value: StorageDevice) -> Self { + match value { + StorageDevice::Virtio(d) => Self::VirtioDisk(d), + StorageDevice::Nvme(d) => Self::NvmeDisk(d), + } + } +} + +impl From for StorageDevice { + fn from(value: StorageDeviceV0) -> Self { + match value { + StorageDeviceV0::VirtioDisk(d) => Self::Virtio(d), + StorageDeviceV0::NvmeDisk(d) => Self::Nvme(d), + } + } +} + +/// Describes the backend half of a [`Disk`]. +#[derive(Clone, Debug)] +pub enum StorageBackend { + Crucible(CrucibleStorageBackend), + File(FileStorageBackend), + Blob(BlobStorageBackend), +} + +impl From for StorageBackendV0 { + fn from(value: StorageBackend) -> Self { + match value { + StorageBackend::Crucible(be) => Self::Crucible(be), + StorageBackend::File(be) => Self::File(be), + StorageBackend::Blob(be) => Self::Blob(be), + } + } +} + +impl From for StorageBackend { + fn from(value: StorageBackendV0) -> Self { + match value { + StorageBackendV0::Crucible(be) => Self::Crucible(be), + StorageBackendV0::File(be) => Self::File(be), + StorageBackendV0::Blob(be) => Self::Blob(be), + } + } +} + +#[derive(Clone, Debug)] +pub struct Disk { + pub device_spec: StorageDevice, + pub backend_name: String, + pub backend_spec: StorageBackend, +} + +#[derive(Clone, Debug)] +pub struct Nic { + pub device_spec: VirtioNic, + pub backend_name: String, + pub backend_spec: VirtioNetworkBackend, +} + +/// A kind of device to install as the listener on a COM port. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum SerialPortDevice { + Uart, + + #[cfg(feature = "falcon")] + SoftNpu, +} + +#[derive(Clone, Debug)] +pub struct QemuPvpanic { + #[allow(dead_code)] + pub name: String, + pub spec: QemuPvpanicDesc, +} + +#[cfg(feature = "falcon")] +#[derive(Clone, Debug)] +pub struct SoftNpuPort { + pub backend_name: String, + pub backend_spec: DlpiNetworkBackend, } -/// Describes a network device/backend pair parsed from an input source like an -/// API request or a config TOML entry. -struct ParsedNetworkDevice { - device_name: String, - device_spec: NetworkDeviceV0, - backend_name: String, - backend_spec: NetworkBackendV0, +#[cfg(feature = "falcon")] +#[derive(Clone, Debug, Default)] +pub struct SoftNpu { + pub pci_port: Option, + pub ports: HashMap, + pub p9_device: Option, + pub p9fs: Option, +} + +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 { - pci_ports: Vec, - ports: Vec, - p9_devices: Vec, - p9fs: Vec, + 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 diff --git a/bin/propolis-server/src/lib/stats/mod.rs b/bin/propolis-server/src/lib/stats/mod.rs index 683626ea3..0a23d3e84 100644 --- a/bin/propolis-server/src/lib/stats/mod.rs +++ b/bin/propolis-server/src/lib/stats/mod.rs @@ -14,12 +14,11 @@ use oximeter::{ }; use oximeter_instruments::kstat::KstatSampler; use oximeter_producer::{Config, Error, Server}; -use propolis_api_types::{ - instance_spec::v0::InstanceSpecV0, InstanceProperties, -}; +use propolis_api_types::InstanceProperties; use slog::Logger; use uuid::Uuid; +use crate::spec::Spec; use crate::{server::MetricsEndpointConfig, vm::NetworkInterfaceIds}; mod network_interface; @@ -190,11 +189,11 @@ pub fn start_oximeter_server( pub(crate) fn create_kstat_sampler( log: &Logger, properties: &InstanceProperties, - spec: &InstanceSpecV0, + spec: &Spec, ) -> Option { let kstat_limit = usize::try_from( (u32::from(properties.vcpus) * KSTAT_LIMIT_PER_VCPU) - + (spec.devices.network_devices.len() as u32 * SAMPLE_BUFFER), + + (spec.nics.len() as u32 * SAMPLE_BUFFER), ) .unwrap(); diff --git a/bin/propolis-server/src/lib/vm/ensure.rs b/bin/propolis-server/src/lib/vm/ensure.rs index 65c97fd14..3aff7d3bf 100644 --- a/bin/propolis-server/src/lib/vm/ensure.rs +++ b/bin/propolis-server/src/lib/vm/ensure.rs @@ -30,9 +30,8 @@ use std::sync::Arc; use oximeter::types::ProducerRegistry; use oximeter_instruments::kstat::KstatSampler; use propolis_api_types::{ - instance_spec::{v0::InstanceSpecV0, VersionedInstanceSpec}, - InstanceEnsureResponse, InstanceMigrateInitiateResponse, - InstanceProperties, InstanceSpecEnsureRequest, InstanceState, + InstanceEnsureResponse, InstanceMigrateInitiateRequest, + InstanceMigrateInitiateResponse, InstanceProperties, InstanceState, }; use slog::{debug, info}; @@ -40,6 +39,7 @@ use crate::{ initializer::{ build_instance, MachineInitializer, MachineInitializerState, }, + spec::Spec, stats::create_kstat_sampler, vm::request_queue::InstanceAutoStart, }; @@ -52,12 +52,18 @@ use super::{ EnsureOptions, InstanceEnsureResponseTx, VmError, }; +pub(crate) struct VmEnsureRequest { + pub(crate) properties: InstanceProperties, + pub(crate) migrate: Option, + pub(crate) instance_spec: Spec, +} + /// Holds state about an instance ensure request that has not yet produced any /// VM objects or driven the VM state machine to the `ActiveVm` state. pub(crate) struct VmEnsureNotStarted<'a> { log: &'a slog::Logger, vm: &'a Arc, - ensure_request: &'a InstanceSpecEnsureRequest, + ensure_request: &'a VmEnsureRequest, ensure_options: &'a EnsureOptions, ensure_response_tx: InstanceEnsureResponseTx, state_publisher: &'a mut StatePublisher, @@ -67,7 +73,7 @@ impl<'a> VmEnsureNotStarted<'a> { pub(super) fn new( log: &'a slog::Logger, vm: &'a Arc, - ensure_request: &'a InstanceSpecEnsureRequest, + ensure_request: &'a VmEnsureRequest, ensure_options: &'a EnsureOptions, ensure_response_tx: InstanceEnsureResponseTx, state_publisher: &'a mut StatePublisher, @@ -82,9 +88,8 @@ impl<'a> VmEnsureNotStarted<'a> { } } - pub(crate) fn instance_spec(&self) -> &InstanceSpecV0 { - let VersionedInstanceSpec::V0(v0) = &self.ensure_request.instance_spec; - v0 + pub(crate) fn instance_spec(&self) -> &Spec { + &self.ensure_request.instance_spec } pub(crate) fn state_publisher(&mut self) -> &mut StatePublisher { @@ -162,10 +167,9 @@ impl<'a> VmEnsureNotStarted<'a> { // Set up the 'shell' instance into which the rest of this routine will // add components. - let VersionedInstanceSpec::V0(v0_spec) = &spec; let machine = build_instance( &properties.vm_name(), - v0_spec, + spec, options.use_reservoir, vmm_log, )?; @@ -176,7 +180,7 @@ impl<'a> VmEnsureNotStarted<'a> { devices: Default::default(), block_backends: Default::default(), crucible_backends: Default::default(), - spec: v0_spec, + spec, properties, toml_config: &options.toml_config, producer_registry: options.oximeter_registry.clone(), @@ -184,7 +188,7 @@ impl<'a> VmEnsureNotStarted<'a> { kstat_sampler: initialize_kstat_sampler( self.log, properties, - v0_spec, + self.instance_spec(), options.oximeter_registry.clone(), ), }; @@ -221,10 +225,8 @@ impl<'a> VmEnsureNotStarted<'a> { init.initialize_storage_devices(&chipset, options.nexus_client.clone()) .await?; - let ramfb = init.initialize_fwcfg(v0_spec.devices.board.cpus)?; - + 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() @@ -240,7 +242,7 @@ impl<'a> VmEnsureNotStarted<'a> { } = init; Ok(InputVmObjects { - instance_spec: v0_spec.clone(), + instance_spec: spec.clone(), vcpu_tasks, machine, devices, @@ -259,7 +261,7 @@ impl<'a> VmEnsureNotStarted<'a> { pub(crate) struct VmEnsureObjectsCreated<'a> { log: &'a slog::Logger, vm: &'a Arc, - ensure_request: &'a InstanceSpecEnsureRequest, + ensure_request: &'a VmEnsureRequest, ensure_options: &'a EnsureOptions, ensure_response_tx: InstanceEnsureResponseTx, state_publisher: &'a mut StatePublisher, @@ -377,7 +379,7 @@ impl<'a> VmEnsureActive<'a> { fn initialize_kstat_sampler( log: &slog::Logger, properties: &InstanceProperties, - spec: &InstanceSpecV0, + spec: &Spec, producer_registry: Option, ) -> Option { let registry = producer_registry?; diff --git a/bin/propolis-server/src/lib/vm/mod.rs b/bin/propolis-server/src/lib/vm/mod.rs index 1608ef069..87364fa36 100644 --- a/bin/propolis-server/src/lib/vm/mod.rs +++ b/bin/propolis-server/src/lib/vm/mod.rs @@ -82,11 +82,11 @@ use std::{collections::BTreeMap, net::SocketAddr, sync::Arc}; use active::ActiveVm; +use ensure::VmEnsureRequest; use oximeter::types::ProducerRegistry; use propolis_api_types::{ - instance_spec::{v0::InstanceSpecV0, VersionedInstanceSpec}, - InstanceEnsureResponse, InstanceMigrateStatusResponse, - InstanceMigrationStatus, InstanceProperties, InstanceSpecEnsureRequest, + instance_spec::VersionedInstanceSpec, InstanceEnsureResponse, + InstanceMigrateStatusResponse, InstanceMigrationStatus, InstanceProperties, InstanceSpecGetResponse, InstanceState, InstanceStateMonitorResponse, MigrationState, }; @@ -95,7 +95,7 @@ use state_driver::StateDriverOutput; use state_publisher::StatePublisher; use tokio::sync::{oneshot, watch, RwLock, RwLockReadGuard}; -use crate::{server::MetricsEndpointConfig, vnc::VncServer}; +use crate::{server::MetricsEndpointConfig, spec::Spec, vnc::VncServer}; mod active; pub(crate) mod ensure; @@ -219,7 +219,7 @@ struct VmDescription { properties: InstanceProperties, /// The VM's last-known instance specification. - spec: InstanceSpecV0, + spec: Spec, /// The runtime on which the VM's state driver is running (or on which it /// ran). @@ -330,7 +330,7 @@ impl Vm { let state = vm.external_state_rx.borrow().clone(); Ok(InstanceSpecGetResponse { properties: vm.properties.clone(), - spec: VersionedInstanceSpec::V0(spec), + spec: VersionedInstanceSpec::V0(spec.into()), state: state.state, }) } @@ -343,7 +343,7 @@ impl Vm { | VmState::RundownComplete(vm) => Ok(InstanceSpecGetResponse { properties: vm.properties.clone(), state: vm.external_state_rx.borrow().state, - spec: VersionedInstanceSpec::V0(vm.spec.clone()), + spec: VersionedInstanceSpec::V0(vm.spec.clone().into()), }), } } @@ -503,7 +503,7 @@ impl Vm { pub(crate) async fn ensure( self: &Arc, log: &slog::Logger, - ensure_request: InstanceSpecEnsureRequest, + ensure_request: VmEnsureRequest, options: EnsureOptions, ) -> Result { let log_for_driver = @@ -554,12 +554,10 @@ impl Vm { _ => {} }; - let VersionedInstanceSpec::V0(v0_spec) = - ensure_request.instance_spec.clone(); - let thread_count = usize::max( VMM_MIN_RT_THREADS, - VMM_BASE_RT_THREADS + v0_spec.devices.board.cpus as usize, + VMM_BASE_RT_THREADS + + ensure_request.instance_spec.board.cpus as usize, ); let tokio_rt = tokio::runtime::Builder::new_multi_thread() @@ -570,6 +568,7 @@ impl Vm { .map_err(VmError::TokioRuntimeInitializationFailed)?; let properties = ensure_request.properties.clone(); + let spec = ensure_request.instance_spec.clone(); let vm_for_driver = self.clone(); guard.driver = Some(tokio_rt.spawn(async move { state_driver::run_state_driver( @@ -586,7 +585,7 @@ impl Vm { guard.state = VmState::WaitingForInit(VmDescription { external_state_rx: external_rx.clone(), properties, - spec: v0_spec, + spec, tokio_rt: Some(tokio_rt), }); } diff --git a/bin/propolis-server/src/lib/vm/objects.rs b/bin/propolis-server/src/lib/vm/objects.rs index 79e9897ee..a79cee1d7 100644 --- a/bin/propolis-server/src/lib/vm/objects.rs +++ b/bin/propolis-server/src/lib/vm/objects.rs @@ -17,11 +17,10 @@ use propolis::{ vmm::VmmHdl, Machine, }; -use propolis_api_types::instance_spec::v0::InstanceSpecV0; use slog::{error, info}; use tokio::sync::{RwLock, RwLockReadGuard, RwLockWriteGuard}; -use crate::{serial::Serial, vcpu_tasks::VcpuTaskController}; +use crate::{serial::Serial, spec::Spec, vcpu_tasks::VcpuTaskController}; use super::{ state_driver::VmStartReason, BlockBackendMap, CrucibleBackendMap, DeviceMap, @@ -44,7 +43,7 @@ pub(crate) struct VmObjects { /// A collection of objects that should eventually be wrapped in a lock and /// stored in a `VmObjects` structure. See [`VmObjectsLocked`]. pub(super) struct InputVmObjects { - pub instance_spec: InstanceSpecV0, + pub instance_spec: Spec, pub vcpu_tasks: Box, pub machine: Machine, pub devices: DeviceMap, @@ -61,7 +60,7 @@ pub(crate) struct VmObjectsLocked { log: slog::Logger, /// The instance spec that describes this collection of objects. - instance_spec: InstanceSpecV0, + instance_spec: Spec, /// The set of tasks that run this VM's vCPUs. vcpu_tasks: Box, @@ -132,12 +131,12 @@ impl VmObjectsLocked { } /// Yields the VM's current instance spec. - pub(crate) fn instance_spec(&self) -> &InstanceSpecV0 { + pub(crate) fn instance_spec(&self) -> &Spec { &self.instance_spec } /// Yields a mutable reference to the VM's current instance spec. - pub(crate) fn instance_spec_mut(&mut self) -> &mut InstanceSpecV0 { + pub(crate) fn instance_spec_mut(&mut self) -> &mut Spec { &mut self.instance_spec } diff --git a/bin/propolis-server/src/lib/vm/state_driver.rs b/bin/propolis-server/src/lib/vm/state_driver.rs index a5fa9cdae..a0298b4e4 100644 --- a/bin/propolis-server/src/lib/vm/state_driver.rs +++ b/bin/propolis-server/src/lib/vm/state_driver.rs @@ -11,10 +11,8 @@ use std::{ use anyhow::Context; use propolis_api_types::{ - instance_spec::{ - components::backends::CrucibleStorageBackend, v0::StorageBackendV0, - }, - InstanceSpecEnsureRequest, InstanceState, MigrationState, + instance_spec::components::backends::CrucibleStorageBackend, InstanceState, + MigrationState, }; use slog::{error, info}; use tokio::sync::Notify; @@ -24,11 +22,12 @@ use crate::{ migrate::{ destination::DestinationProtocol, source::SourceProtocol, MigrateRole, }, + spec::StorageBackend, vm::state_publisher::ExternalStateUpdate, }; use super::{ - ensure::{VmEnsureActive, VmEnsureNotStarted}, + ensure::{VmEnsureActive, VmEnsureNotStarted, VmEnsureRequest}, guest_event::{self, GuestEvent}, objects::VmObjects, request_queue::{self, ExternalRequest, InstanceAutoStart}, @@ -261,7 +260,7 @@ pub(super) async fn run_state_driver( log: slog::Logger, vm: Arc, mut state_publisher: StatePublisher, - ensure_request: InstanceSpecEnsureRequest, + ensure_request: VmEnsureRequest, ensure_result_tx: InstanceEnsureResponseTx, ensure_options: super::EnsureOptions, ) -> StateDriverOutput { @@ -308,7 +307,7 @@ async fn create_and_activate_vm<'a>( log: &'a slog::Logger, vm: &'a Arc, state_publisher: &'a mut StatePublisher, - ensure_request: &'a InstanceSpecEnsureRequest, + ensure_request: &'a VmEnsureRequest, ensure_result_tx: InstanceEnsureResponseTx, ensure_options: &'a super::EnsureOptions, ) -> anyhow::Result> { @@ -661,51 +660,43 @@ impl StateDriver { } let mut objects = self.objects.lock_exclusive().await; - let (readonly, old_vcr_json) = { - let StorageBackendV0::Crucible(bes) = objects - .instance_spec() - .backends - .storage_backends - .get(&disk_name) - .ok_or_else(|| spec_element_not_found(&disk_name))? - else { - return Err(spec_element_not_found(&disk_name)); - }; - - (bes.readonly, &bes.request_json) + let backend = objects + .crucible_backends() + .get(backend_id) + .ok_or_else(|| { + let msg = format!("No crucible backend for id {backend_id}"); + dropshot::HttpError::for_not_found(Some(msg.clone()), msg) + })? + .clone(); + + let Some(disk) = objects.instance_spec_mut().disks.get_mut(&disk_name) + else { + return Err(spec_element_not_found(&disk_name)); }; - let replace_result = { - let backend = objects - .crucible_backends() - .get(backend_id) - .ok_or_else(|| { - let msg = - format!("No crucible backend for id {backend_id}"); - dropshot::HttpError::for_not_found(Some(msg.clone()), msg) - })?; - - backend.vcr_replace(old_vcr_json, &new_vcr_json).await.map_err( - |e| { - dropshot::HttpError::for_bad_request( - Some(e.to_string()), - e.to_string(), - ) - }, - ) - }?; - - let new_bes = StorageBackendV0::Crucible(CrucibleStorageBackend { + let StorageBackend::Crucible(CrucibleStorageBackend { + request_json: old_vcr_json, readonly, + }) = &disk.backend_spec + else { + return Err(spec_element_not_found(&disk_name)); + }; + + let replace_result = backend + .vcr_replace(old_vcr_json, &new_vcr_json) + .await + .map_err(|e| { + dropshot::HttpError::for_bad_request( + Some(e.to_string()), + e.to_string(), + ) + })?; + + disk.backend_spec = StorageBackend::Crucible(CrucibleStorageBackend { + readonly: *readonly, request_json: new_vcr_json, }); - objects - .instance_spec_mut() - .backends - .storage_backends - .insert(disk_name, new_bes); - info!(self.log, "replaced Crucible VCR"; "backend_id" => %backend_id); Ok(replace_result) 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 9f4e2e6c7..b99e5c3b9 100644 --- a/crates/propolis-api-types/src/instance_spec/components/devices.rs +++ b/crates/propolis-api-types/src/instance_spec/components/devices.rs @@ -125,7 +125,7 @@ impl MigrationElement for VirtioNic { /// A serial port identifier, which determines what I/O ports a guest can use to /// access a port. #[derive( - Clone, Copy, Deserialize, Serialize, Debug, PartialEq, Eq, JsonSchema, + Clone, Copy, Deserialize, Serialize, Debug, PartialEq, Eq, JsonSchema, Hash, )] #[serde(deny_unknown_fields, rename_all = "snake_case")] pub enum SerialPortNumber {