diff --git a/bin/propolis-server/src/lib/migrate/compat.rs b/bin/propolis-server/src/lib/migrate/compat.rs new file mode 100644 index 000000000..2da008b5a --- /dev/null +++ b/bin/propolis-server/src/lib/migrate/compat.rs @@ -0,0 +1,795 @@ +// 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/. + +//! Checks for compatibility of two instance specs. + +use std::collections::HashMap; + +use crate::spec::{self, SerialPortDevice}; + +use propolis_api_types::instance_spec::{ + components::{ + board::Chipset, + devices::{PciPciBridge, SerialPortNumber}, + }, + PciPath, +}; +use thiserror::Error; + +trait CompatCheck { + type Error; + + fn is_compatible_with(&self, other: &Self) -> Result<(), Self::Error>; +} + +#[derive(Debug, Error)] +pub enum CompatibilityError { + #[error(transparent)] + Board(#[from] BoardIncompatibility), + + #[error(transparent)] + Pvpanic(#[from] PvpanicIncompatibility), + + #[error("collection {0} incompatible")] + Collection(String, #[source] CollectionIncompatibility), + + #[cfg(feature = "falcon")] + #[error("can't migrate instances containing softnpu devices")] + SoftNpu, +} + +#[derive(Debug, Error)] +pub enum BoardIncompatibility { + #[error("boards have different CPU counts (self: {this}, other: {other})")] + CpuCount { this: u8, other: u8 }, + + #[error( + "boards have different memory sizes (self: {this}, other: {other})" + )] + MemorySize { this: u64, other: u64 }, + + #[error( + "chipsets have different PCIe settings (self: {this}, other: {other})" + )] + PcieEnabled { this: bool, other: bool }, +} + +#[derive(Debug, Error)] +pub enum DiskIncompatibility { + #[error( + "disks have different device interfaces (self: {this}, other: {other})" + )] + Interface { this: &'static str, other: &'static str }, + + #[error("disks have different PCI paths (self: {this}, other: {other})")] + PciPath { this: PciPath, other: PciPath }, + + #[error( + "disks have different backend names (self: {this:?}, other: {other:?})" + )] + BackendName { this: String, other: String }, + + #[error( + "disks have different backend kinds (self: {this}, other: {other})" + )] + BackendKind { this: &'static str, other: &'static str }, + + #[error( + "disks have different read-only settings (self: {this}, other: {other})" + )] + ReadOnly { this: bool, other: bool }, +} + +#[derive(Debug, Error)] +pub enum NicIncompatibility { + #[error("NICs have different PCI paths (self: {this}, other: {other})")] + PciPath { this: PciPath, other: PciPath }, + + #[error( + "NICs have different backend names (self: {this}, other: {other})" + )] + BackendName { this: String, other: String }, +} + +#[derive(Debug, Error)] +pub enum SerialPortIncompatibility { + #[error("ports have different numbers (self: {this:?}, other: {other:?})")] + Number { this: SerialPortNumber, other: SerialPortNumber }, + + #[error("ports have different devices (self: {this}, other: {other})")] + Device { this: SerialPortDevice, other: SerialPortDevice }, +} + +#[derive(Debug, Error)] +pub enum BridgeIncompatibility { + #[error("bridges have different PCI paths (self: {this}, other: {other})")] + PciPath { this: PciPath, other: PciPath }, + + #[error("bridges have different downstream buses (self: {this}, other: {other})")] + DownstreamBus { this: u8, other: u8 }, +} + +#[derive(Debug, Error)] +pub enum PvpanicIncompatibility { + #[error("pvpanic presence differs (self: {this}, other: {other})")] + Presence { this: bool, other: bool }, + + #[error( + "pvpanic devices have different names (self: {this:?}, other: {other:?})" + )] + Name { this: String, other: String }, + + #[error( + "pvpanic devices have different ISA settings (self: {this}, other: {other})" + )] + EnableIsa { this: bool, other: bool }, +} + +#[derive(Debug, Error)] +pub enum ComponentIncompatibility { + #[error(transparent)] + Board(#[from] BoardIncompatibility), + + #[error(transparent)] + Disk(#[from] DiskIncompatibility), + + #[error(transparent)] + Nic(#[from] NicIncompatibility), + + #[error(transparent)] + SerialPort(#[from] SerialPortIncompatibility), + + #[error(transparent)] + PciPciBridge(#[from] BridgeIncompatibility), +} + +#[derive(Debug, Error)] +pub enum CollectionIncompatibility { + #[error( + "collections have different lengths (self: {this}, other: {other})" + )] + Length { this: usize, other: usize }, + + #[error("collection key {0} present in self but not other")] + KeyAbsent(String), + + #[error("component {0} incompatible")] + Component(String, #[source] ComponentIncompatibility), +} + +impl> CompatCheck + for HashMap +{ + type Error = CollectionIncompatibility; + + fn is_compatible_with( + &self, + other: &Self, + ) -> Result<(), CollectionIncompatibility> { + if self.len() != other.len() { + return Err(CollectionIncompatibility::Length { + this: self.len(), + other: other.len(), + }); + } + + for (key, this_val) in self.iter() { + let other_val = other.get(key).ok_or_else(|| { + CollectionIncompatibility::KeyAbsent(key.clone()) + })?; + + this_val.is_compatible_with(other_val).map_err(|e| { + CollectionIncompatibility::Component(key.clone(), e) + })?; + } + + Ok(()) + } +} + +impl spec::Spec { + fn is_board_compatible( + &self, + other: &Self, + ) -> Result<(), BoardIncompatibility> { + self.is_chipset_compatible(other)?; + + let this = &self.board; + let other = &other.board; + if this.cpus != other.cpus { + Err(BoardIncompatibility::CpuCount { + this: this.cpus, + other: other.cpus, + }) + } else if this.memory_mb != other.memory_mb { + Err(BoardIncompatibility::MemorySize { + this: this.memory_mb, + other: other.memory_mb, + }) + } else { + Ok(()) + } + } + + fn is_chipset_compatible( + &self, + other: &Self, + ) -> Result<(), BoardIncompatibility> { + let Chipset::I440Fx(this) = self.board.chipset; + let Chipset::I440Fx(other) = other.board.chipset; + + if this.enable_pcie != other.enable_pcie { + Err(BoardIncompatibility::PcieEnabled { + this: this.enable_pcie, + other: other.enable_pcie, + }) + } else { + Ok(()) + } + } + + fn is_pvpanic_compatible( + &self, + other: &Self, + ) -> Result<(), PvpanicIncompatibility> { + match (&self.pvpanic, &other.pvpanic) { + (None, None) => Ok(()), + (Some(this), Some(other)) if this.name != other.name => { + Err(PvpanicIncompatibility::Name { + this: this.name.clone(), + other: other.name.clone(), + }) + } + (Some(this), Some(other)) + if this.spec.enable_isa != other.spec.enable_isa => + { + Err(PvpanicIncompatibility::EnableIsa { + this: this.spec.enable_isa, + other: other.spec.enable_isa, + }) + } + (Some(_), Some(_)) => Ok(()), + (this, other) => Err(PvpanicIncompatibility::Presence { + this: this.is_some(), + other: other.is_some(), + }), + } + } + + pub(super) fn is_migration_compatible( + &self, + other: &Self, + ) -> Result<(), CompatibilityError> { + self.is_board_compatible(other)?; + self.disks.is_compatible_with(&other.disks).map_err(|e| { + CompatibilityError::Collection("disks".to_string(), e) + })?; + + self.nics.is_compatible_with(&other.nics).map_err(|e| { + CompatibilityError::Collection("nics".to_string(), e) + })?; + + self.serial.is_compatible_with(&other.serial).map_err(|e| { + CompatibilityError::Collection("serial ports".to_string(), e) + })?; + + self.pci_pci_bridges + .is_compatible_with(&other.pci_pci_bridges) + .map_err(|e| { + CompatibilityError::Collection("PCI bridges".to_string(), e) + })?; + + self.is_pvpanic_compatible(other)?; + + #[cfg(feature = "falcon")] + if self.softnpu.has_components() || other.softnpu.has_components() { + return Err(CompatibilityError::SoftNpu); + } + + Ok(()) + } +} + +impl spec::StorageDevice { + fn is_compatible_with( + &self, + other: &Self, + ) -> Result<(), DiskIncompatibility> { + if std::mem::discriminant(self) != std::mem::discriminant(other) { + Err(DiskIncompatibility::Interface { + this: self.kind(), + other: other.kind(), + }) + } else if self.pci_path() != other.pci_path() { + Err(DiskIncompatibility::PciPath { + this: self.pci_path(), + other: other.pci_path(), + }) + } else if self.backend_name() != other.backend_name() { + Err(DiskIncompatibility::BackendName { + this: self.backend_name().to_owned(), + other: other.backend_name().to_owned(), + }) + } else { + Ok(()) + } + } +} + +impl spec::StorageBackend { + fn is_compatible_with( + &self, + other: &Self, + ) -> Result<(), DiskIncompatibility> { + if std::mem::discriminant(self) != std::mem::discriminant(other) { + Err(DiskIncompatibility::BackendKind { + this: self.kind(), + other: other.kind(), + }) + } else if self.read_only() != other.read_only() { + Err(DiskIncompatibility::ReadOnly { + this: self.read_only(), + other: other.read_only(), + }) + } else { + Ok(()) + } + } +} + +impl CompatCheck for spec::Disk { + type Error = ComponentIncompatibility; + + fn is_compatible_with( + &self, + other: &Self, + ) -> Result<(), ComponentIncompatibility> { + self.device_spec.is_compatible_with(&other.device_spec)?; + self.backend_spec.is_compatible_with(&other.backend_spec)?; + Ok(()) + } +} + +impl CompatCheck for spec::Nic { + type Error = ComponentIncompatibility; + + fn is_compatible_with( + &self, + other: &Self, + ) -> Result<(), ComponentIncompatibility> { + if self.device_spec.pci_path != other.device_spec.pci_path { + Err(NicIncompatibility::PciPath { + this: self.device_spec.pci_path, + other: other.device_spec.pci_path, + }) + } else if self.device_spec.backend_name + != other.device_spec.backend_name + { + Err(NicIncompatibility::BackendName { + this: self.device_spec.backend_name.clone(), + other: other.device_spec.backend_name.clone(), + }) + } else { + Ok(()) + } + .map_err(ComponentIncompatibility::Nic) + } +} + +impl CompatCheck for spec::SerialPort { + type Error = ComponentIncompatibility; + + fn is_compatible_with( + &self, + other: &Self, + ) -> Result<(), ComponentIncompatibility> { + if self.num != other.num { + Err(SerialPortIncompatibility::Number { + this: self.num, + other: other.num, + }) + } else if std::mem::discriminant(&self.device) + != std::mem::discriminant(&other.device) + { + Err(SerialPortIncompatibility::Device { + this: self.device, + other: other.device, + }) + } else { + Ok(()) + } + .map_err(ComponentIncompatibility::SerialPort) + } +} + +impl CompatCheck for PciPciBridge { + type Error = ComponentIncompatibility; + + fn is_compatible_with( + &self, + other: &Self, + ) -> Result<(), ComponentIncompatibility> { + if self.pci_path != other.pci_path { + Err(BridgeIncompatibility::PciPath { + this: self.pci_path, + other: other.pci_path, + }) + } else if self.downstream_bus != other.downstream_bus { + Err(BridgeIncompatibility::DownstreamBus { + this: self.downstream_bus, + other: other.downstream_bus, + }) + } else { + Ok(()) + } + .map_err(ComponentIncompatibility::PciPciBridge) + } +} + +#[cfg(test)] +mod test { + use propolis_api_types::instance_spec::components::{ + backends::{ + CrucibleStorageBackend, FileStorageBackend, VirtioNetworkBackend, + }, + board::I440Fx, + devices::{ + NvmeDisk, QemuPvpanic as QemuPvpanicDesc, VirtioDisk, VirtioNic, + }, + }; + use spec::{QemuPvpanic, StorageDevice}; + use uuid::Uuid; + + use super::*; + + fn new_spec() -> spec::Spec { + let mut spec = spec::Spec::default(); + spec.board.cpus = 2; + spec.board.memory_mb = 512; + spec + } + + fn file_backend() -> spec::StorageBackend { + spec::StorageBackend::File(FileStorageBackend { + path: "/tmp/file.raw".to_owned(), + readonly: false, + }) + } + + fn crucible_backend() -> spec::StorageBackend { + spec::StorageBackend::Crucible(CrucibleStorageBackend { + request_json: "{}".to_owned(), + readonly: false, + }) + } + + fn nic() -> spec::Nic { + spec::Nic { + device_spec: VirtioNic { + pci_path: PciPath::new(0, 16, 0).unwrap(), + interface_id: Uuid::new_v4(), + backend_name: "vnic".to_owned(), + }, + backend_spec: VirtioNetworkBackend { + vnic_name: "vnic0".to_owned(), + }, + } + } + + fn serial_port() -> spec::SerialPort { + spec::SerialPort { + num: SerialPortNumber::Com1, + device: SerialPortDevice::Uart, + } + } + + fn bridge() -> PciPciBridge { + PciPciBridge { + downstream_bus: 1, + pci_path: PciPath::new(0, 24, 0).unwrap(), + } + } + + #[test] + fn cpu_mismatch() { + let s1 = new_spec(); + let mut s2 = s1.clone(); + s2.board.cpus += 1; + assert!(s1.is_migration_compatible(&s2).is_err()); + } + + #[test] + fn memory_mismatch() { + let s1 = new_spec(); + let mut s2 = s1.clone(); + s2.board.memory_mb *= 2; + assert!(s1.is_migration_compatible(&s2).is_err()); + } + + #[test] + fn pcie_mismatch() { + let mut s1 = new_spec(); + let mut s2 = s1.clone(); + s1.board.chipset = Chipset::I440Fx(I440Fx { enable_pcie: false }); + s2.board.chipset = Chipset::I440Fx(I440Fx { enable_pcie: true }); + assert!(s1.is_migration_compatible(&s2).is_err()); + } + + #[test] + fn pvpanic_name_mismatch() { + let mut s1 = new_spec(); + s1.pvpanic = Some(QemuPvpanic { + name: "pvpanic1".to_string(), + spec: QemuPvpanicDesc { enable_isa: true }, + }); + let mut s2 = s1.clone(); + s2.pvpanic.as_mut().unwrap().name = "pvpanic2".to_string(); + assert!(s1.is_migration_compatible(&s2).is_err()); + } + + #[test] + fn pvpanic_enable_isa_mismatch() { + let mut s1 = new_spec(); + s1.pvpanic = Some(QemuPvpanic { + name: "pvpanic".to_string(), + spec: QemuPvpanicDesc { enable_isa: true }, + }); + let mut s2 = s1.clone(); + s2.pvpanic.as_mut().unwrap().spec.enable_isa = false; + assert!(s1.is_migration_compatible(&s2).is_err()); + } + + #[test] + fn compatible_disks() { + let mut s1 = new_spec(); + let mut s2 = s1.clone(); + let disk = spec::Disk { + device_spec: StorageDevice::Virtio(VirtioDisk { + pci_path: PciPath::new(0, 4, 0).unwrap(), + backend_name: "backend".to_string(), + }), + backend_spec: crucible_backend(), + }; + + s1.disks.insert("disk".to_owned(), disk.clone()); + s2.disks.insert("disk".to_owned(), disk); + assert!(s1.is_migration_compatible(&s2).is_ok()); + } + + #[test] + fn disk_name_mismatch() { + let mut s1 = new_spec(); + let mut s2 = s1.clone(); + let d1 = spec::Disk { + device_spec: StorageDevice::Virtio(VirtioDisk { + pci_path: PciPath::new(0, 4, 0).unwrap(), + backend_name: "backend".to_owned(), + }), + backend_spec: crucible_backend(), + }; + + s1.disks.insert("disk1".to_owned(), d1.clone()); + s2.disks.insert("disk2".to_owned(), d1); + assert!(s1.is_migration_compatible(&s2).is_err()); + } + + #[test] + fn disk_length_mismatch() { + let mut s1 = new_spec(); + let mut s2 = s1.clone(); + let d1 = spec::Disk { + device_spec: StorageDevice::Virtio(VirtioDisk { + pci_path: PciPath::new(0, 4, 0).unwrap(), + backend_name: "backend".to_owned(), + }), + backend_spec: crucible_backend(), + }; + + s1.disks.insert("disk1".to_owned(), d1.clone()); + s2.disks.insert("disk1".to_owned(), d1.clone()); + s2.disks.insert("disk2".to_owned(), d1); + assert!(s1.is_migration_compatible(&s2).is_err()); + } + + #[test] + fn disk_interface_mismatch() { + let mut s1 = new_spec(); + let mut s2 = s1.clone(); + let d1 = spec::Disk { + device_spec: StorageDevice::Virtio(VirtioDisk { + pci_path: PciPath::new(0, 4, 0).unwrap(), + backend_name: "backend".to_owned(), + }), + backend_spec: crucible_backend(), + }; + + let mut d2 = d1.clone(); + d2.device_spec = StorageDevice::Nvme(NvmeDisk { + pci_path: PciPath::new(0, 4, 0).unwrap(), + backend_name: "backend".to_owned(), + }); + + s1.disks.insert("disk".to_owned(), d1); + s2.disks.insert("disk".to_owned(), d2); + assert!(s1.is_migration_compatible(&s2).is_err()); + } + + #[test] + fn disk_pci_mismatch() { + let mut s1 = new_spec(); + let mut s2 = s1.clone(); + let d1 = spec::Disk { + device_spec: StorageDevice::Virtio(VirtioDisk { + pci_path: PciPath::new(0, 4, 0).unwrap(), + backend_name: "backend".to_owned(), + }), + backend_spec: crucible_backend(), + }; + + let mut d2 = d1.clone(); + d2.device_spec = StorageDevice::Virtio(VirtioDisk { + pci_path: PciPath::new(0, 5, 0).unwrap(), + backend_name: "backend".to_owned(), + }); + + s1.disks.insert("disk".to_owned(), d1); + s2.disks.insert("disk".to_owned(), d2); + assert!(s1.is_migration_compatible(&s2).is_err()); + } + + #[test] + fn disk_backend_name_mismatch() { + let mut s1 = new_spec(); + let mut s2 = s1.clone(); + let d1 = spec::Disk { + device_spec: StorageDevice::Virtio(VirtioDisk { + pci_path: PciPath::new(0, 4, 0).unwrap(), + backend_name: "backend".to_owned(), + }), + backend_spec: crucible_backend(), + }; + + let mut d2 = d1.clone(); + d2.device_spec = StorageDevice::Virtio(VirtioDisk { + pci_path: PciPath::new(0, 4, 0).unwrap(), + backend_name: "other_backend".to_owned(), + }); + + s1.disks.insert("disk".to_owned(), d1); + s2.disks.insert("disk".to_owned(), d2); + assert!(s1.is_migration_compatible(&s2).is_err()); + } + + #[test] + fn disk_backend_kind_mismatch() { + let mut s1 = new_spec(); + let mut s2 = s1.clone(); + let d1 = spec::Disk { + device_spec: StorageDevice::Virtio(VirtioDisk { + pci_path: PciPath::new(0, 4, 0).unwrap(), + backend_name: "backend".to_owned(), + }), + backend_spec: file_backend(), + }; + + let mut d2 = d1.clone(); + d2.backend_spec = crucible_backend(); + s1.disks.insert("disk".to_owned(), d1); + s2.disks.insert("disk".to_owned(), d2); + assert!(s1.is_migration_compatible(&s2).is_err()); + } + + #[test] + fn disk_backend_readonly_mismatch() { + let mut s1 = new_spec(); + let mut s2 = s1.clone(); + let d1 = spec::Disk { + device_spec: StorageDevice::Virtio(VirtioDisk { + pci_path: PciPath::new(0, 4, 0).unwrap(), + backend_name: "backend".to_owned(), + }), + backend_spec: file_backend(), + }; + + let mut d2 = d1.clone(); + d2.backend_spec = spec::StorageBackend::File(FileStorageBackend { + path: "/tmp/file.raw".to_owned(), + readonly: true, + }); + + s1.disks.insert("disk".to_owned(), d1); + s2.disks.insert("disk".to_owned(), d2); + assert!(s1.is_migration_compatible(&s2).is_err()); + } + + #[test] + fn compatible_nics() { + let mut s1 = new_spec(); + let mut s2 = s1.clone(); + let nic = nic(); + s1.nics.insert("nic".to_owned(), nic.clone()); + s2.nics.insert("nic".to_owned(), nic); + assert!(s1.is_migration_compatible(&s2).is_ok()); + } + + #[test] + fn nic_pci_mismatch() { + let mut s1 = new_spec(); + let mut s2 = s1.clone(); + let n1 = nic(); + let mut n2 = n1.clone(); + n2.device_spec.pci_path = PciPath::new(0, 24, 0).unwrap(); + s1.nics.insert("nic".to_owned(), n1); + s2.nics.insert("nic".to_owned(), n2); + assert!(s1.is_migration_compatible(&s2).is_err()); + } + + #[test] + fn nic_backend_name_mismatch() { + let mut s1 = new_spec(); + let mut s2 = s1.clone(); + let n1 = nic(); + let mut n2 = n1.clone(); + "other_backend".clone_into(&mut n2.device_spec.backend_name); + s1.nics.insert("nic".to_owned(), n1); + s2.nics.insert("nic".to_owned(), n2); + assert!(s1.is_migration_compatible(&s2).is_err()); + } + + #[test] + fn compatible_serial_ports() { + let mut s1 = new_spec(); + let mut s2 = s1.clone(); + let serial = serial_port(); + s1.serial.insert("com1".to_owned(), serial.clone()); + s2.serial.insert("com1".to_owned(), serial); + assert!(s1.is_migration_compatible(&s2).is_ok()); + } + + #[test] + fn serial_port_number_mismatch() { + let mut s1 = new_spec(); + let mut s2 = s1.clone(); + let serial1 = serial_port(); + let mut serial2 = serial1.clone(); + serial2.num = SerialPortNumber::Com2; + s1.serial.insert("com1".to_owned(), serial1); + s2.serial.insert("com1".to_owned(), serial2); + assert!(s1.is_migration_compatible(&s2).is_err()); + } + + #[test] + fn compatible_bridges() { + let mut s1 = new_spec(); + let mut s2 = s1.clone(); + let bridge = bridge(); + s1.pci_pci_bridges.insert("bridge1".to_owned(), bridge); + s2.pci_pci_bridges.insert("bridge1".to_owned(), bridge); + assert!(s1.is_migration_compatible(&s2).is_ok()); + } + + #[test] + fn bridge_downstream_bus_mismatch() { + let mut s1 = new_spec(); + let mut s2 = s1.clone(); + let b1 = bridge(); + let mut b2 = b1; + b2.downstream_bus += 1; + s1.pci_pci_bridges.insert("bridge1".to_owned(), b1); + s2.pci_pci_bridges.insert("bridge1".to_owned(), b2); + assert!(s1.is_migration_compatible(&s2).is_err()); + } + + #[test] + fn bridge_pci_path_mismatch() { + let mut s1 = new_spec(); + let mut s2 = s1.clone(); + let b1 = bridge(); + let mut b2 = b1; + b2.pci_path = PciPath::new(0, 30, 0).unwrap(); + s1.pci_pci_bridges.insert("bridge1".to_owned(), b1); + s2.pci_pci_bridges.insert("bridge1".to_owned(), b2); + assert!(s1.is_migration_compatible(&s2).is_err()); + } +} diff --git a/bin/propolis-server/src/lib/migrate/destination.rs b/bin/propolis-server/src/lib/migrate/destination.rs index 21b12f09c..eb5982f15 100644 --- a/bin/propolis-server/src/lib/migrate/destination.rs +++ b/bin/propolis-server/src/lib/migrate/destination.rs @@ -346,14 +346,15 @@ impl RonV0 { }?; info!(self.log(), "Destination read Preamble: {:?}", preamble); - if let Err(e) = preamble - .is_migration_compatible(&ensure_ctx.instance_spec().clone().into()) + if let Err(e) = + preamble.check_compatibility(&ensure_ctx.instance_spec().clone()) { error!( self.log(), - "Source and destination instance specs incompatible: {}", e + "source and destination instance specs incompatible"; + "error" => #%e ); - return Err(MigrateError::InvalidInstanceState); + return Err(MigrateError::InstanceSpecsIncompatible(e.to_string())); } self.send_msg(codec::Message::Okay).await diff --git a/bin/propolis-server/src/lib/migrate/mod.rs b/bin/propolis-server/src/lib/migrate/mod.rs index a59e8c85c..402adf30e 100644 --- a/bin/propolis-server/src/lib/migrate/mod.rs +++ b/bin/propolis-server/src/lib/migrate/mod.rs @@ -12,6 +12,7 @@ use thiserror::Error; use tokio::io::{AsyncRead, AsyncWrite}; mod codec; +mod compat; pub mod destination; mod memx; mod preamble; @@ -88,6 +89,14 @@ pub enum MigrateError { #[error("expected connection upgrade")] UpgradeExpected, + /// Error parsing the contents of the preamble + #[error("failed to parse preamble: {0}")] + PreambleParse(String), + + /// Source and target decided their configurations are incompatible + #[error("instance specs incompatible: {0}")] + InstanceSpecsIncompatible(String), + /// Attempted to migrate an uninitialized instance #[error("failed to initialize the target VM: {0}")] TargetInstanceInitializationFailed(String), @@ -172,6 +181,8 @@ impl From for HttpError { | MigrateError::ProtocolParse(_, _) | MigrateError::NoMatchingProtocol(_, _) | MigrateError::TargetInstanceInitializationFailed(_) + | MigrateError::PreambleParse(_) + | MigrateError::InstanceSpecsIncompatible(_) | MigrateError::InvalidInstanceState | MigrateError::Codec(_) | MigrateError::UnexpectedMessage diff --git a/bin/propolis-server/src/lib/migrate/preamble.rs b/bin/propolis-server/src/lib/migrate/preamble.rs index b45a0d9ac..f80c4a767 100644 --- a/bin/propolis-server/src/lib/migrate/preamble.rs +++ b/bin/propolis-server/src/lib/migrate/preamble.rs @@ -2,67 +2,41 @@ // 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::BTreeSet; - -use propolis_api_types::instance_spec::{ - migration::{CollectionCompatibilityError, MigrationCompatibilityError}, - v0::{DeviceSpecV0, InstanceSpecV0}, - VersionedInstanceSpec, -}; +use propolis_api_types::instance_spec::VersionedInstanceSpec; use serde::{Deserialize, Serialize}; +use crate::spec::{self, api_spec_v0::ApiSpecError}; + +use super::MigrateError; + #[derive(Deserialize, Serialize, Debug)] pub(crate) struct Preamble { - pub device_spec: DeviceSpecV0, - pub backend_keys: BTreeSet, + pub instance_spec: VersionedInstanceSpec, pub blobs: Vec>, } -fn get_spec_backend_keys(spec: &InstanceSpecV0) -> BTreeSet { - spec.backends - .storage_backends - .keys() - .chain(spec.backends.network_backends.keys()) - .cloned() - .collect() -} - impl Preamble { pub fn new(instance_spec: VersionedInstanceSpec) -> Preamble { - let VersionedInstanceSpec::V0(instance_spec) = instance_spec; - Preamble { - device_spec: instance_spec.devices.clone(), - backend_keys: get_spec_backend_keys(&instance_spec), - blobs: Vec::new(), - } + Preamble { instance_spec, blobs: Vec::new() } } - pub fn is_migration_compatible( - &self, - other_spec: &InstanceSpecV0, - ) -> Result<(), MigrationCompatibilityError> { - self.device_spec.can_migrate_devices_from(&other_spec.devices)?; - let other_keys = get_spec_backend_keys(other_spec); - if self.backend_keys.len() != other_keys.len() { - return Err(MigrationCompatibilityError::CollectionMismatch( - "backends".to_string(), - CollectionCompatibilityError::CollectionSize( - self.backend_keys.len(), - other_keys.len(), - ), - )); - } - - for this_key in self.backend_keys.iter() { - if !other_keys.contains(this_key) { - return Err(MigrationCompatibilityError::CollectionMismatch( - "backends".to_string(), - CollectionCompatibilityError::CollectionKeyAbsent( - this_key.clone(), - ), - )); - } - } + /// Checks to see if the serialized spec in this preamble is compatible with + /// the supplied `other_spec`. + /// + /// This check runs on the destination. + pub fn check_compatibility( + self, + other_spec: &spec::Spec, + ) -> Result<(), MigrateError> { + let VersionedInstanceSpec::V0(v0_spec) = self.instance_spec; + let this_spec: spec::Spec = + v0_spec.try_into().map_err(|e: ApiSpecError| { + MigrateError::PreambleParse(e.to_string()) + })?; + + this_spec.is_migration_compatible(other_spec).map_err(|e| { + MigrateError::InstanceSpecsIncompatible(e.to_string()) + })?; // TODO: Compare opaque blobs. diff --git a/bin/propolis-server/src/lib/spec/mod.rs b/bin/propolis-server/src/lib/spec/mod.rs index e26f51757..189463a6c 100644 --- a/bin/propolis-server/src/lib/spec/mod.rs +++ b/bin/propolis-server/src/lib/spec/mod.rs @@ -75,6 +75,13 @@ pub enum StorageDevice { } impl StorageDevice { + pub fn kind(&self) -> &'static str { + match self { + StorageDevice::Virtio(_) => "virtio", + StorageDevice::Nvme(_) => "nvme", + } + } + pub fn pci_path(&self) -> PciPath { match self { StorageDevice::Virtio(disk) => disk.pci_path, @@ -116,6 +123,24 @@ pub enum StorageBackend { Blob(BlobStorageBackend), } +impl StorageBackend { + pub fn kind(&self) -> &'static str { + match self { + StorageBackend::Crucible(_) => "crucible", + StorageBackend::File(_) => "file", + StorageBackend::Blob(_) => "backend", + } + } + + pub fn read_only(&self) -> bool { + match self { + StorageBackend::Crucible(be) => be.readonly, + StorageBackend::File(be) => be.readonly, + StorageBackend::Blob(be) => be.readonly, + } + } +} + impl From for StorageBackendV0 { fn from(value: StorageBackend) -> Self { match value { @@ -157,6 +182,21 @@ pub enum SerialPortDevice { SoftNpu, } +impl std::fmt::Display for SerialPortDevice { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!( + f, + "{}", + match self { + SerialPortDevice::Uart => "uart", + + #[cfg(feature = "falcon")] + SerialPortDevice::SoftNpu => "softnpu", + } + ) + } +} + #[derive(Clone, Debug)] pub struct SerialPort { pub num: SerialPortNumber, @@ -186,6 +226,17 @@ pub struct SoftNpu { pub p9fs: Option, } +#[cfg(feature = "falcon")] +impl SoftNpu { + /// Returns `true` if this struct specifies at least one SoftNPU component. + pub fn has_components(&self) -> bool { + self.pci_port.is_some() + || self.p9_device.is_some() + || self.p9fs.is_some() + || !self.ports.is_empty() + } +} + struct ParsedDiskRequest { name: String, disk: Disk, diff --git a/crates/propolis-api-types/src/instance_spec/components/backends.rs b/crates/propolis-api-types/src/instance_spec/components/backends.rs index c61778b51..79fc161cf 100644 --- a/crates/propolis-api-types/src/instance_spec/components/backends.rs +++ b/crates/propolis-api-types/src/instance_spec/components/backends.rs @@ -6,10 +6,8 @@ //! its components to talk to other services supplied by the host OS or the //! larger rack. -use crate::instance_spec::migration::MigrationElement; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; -use thiserror::Error; /// A Crucible storage backend. #[derive(Clone, Deserialize, Serialize, JsonSchema)] @@ -29,28 +27,6 @@ pub struct CrucibleStorageBackend { pub readonly: bool, } -impl MigrationElement for CrucibleStorageBackend { - fn kind(&self) -> &'static str { - "CrucibleStorageBackend" - } - - fn can_migrate_from_element( - &self, - other: &Self, - ) -> Result<(), crate::instance_spec::migration::ElementCompatibilityError> - { - if self.readonly != other.readonly { - Err(MigrationCompatibilityError::ComponentConfiguration(format!( - "read-only mismatch (self: {}, other: {})", - self.readonly, other.readonly, - )) - .into()) - } else { - Ok(()) - } - } -} - impl std::fmt::Debug for CrucibleStorageBackend { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { // Redact the contents of the VCR since they may contain volume @@ -73,28 +49,6 @@ pub struct FileStorageBackend { pub readonly: bool, } -impl MigrationElement for FileStorageBackend { - fn kind(&self) -> &'static str { - "FileStorageBackend" - } - - fn can_migrate_from_element( - &self, - other: &Self, - ) -> Result<(), crate::instance_spec::migration::ElementCompatibilityError> - { - if self.readonly != other.readonly { - Err(MigrationCompatibilityError::ComponentConfiguration(format!( - "read-only mismatch (self: {}, other: {})", - self.readonly, other.readonly, - )) - .into()) - } else { - Ok(()) - } - } -} - /// A storage backend for a disk whose initial contents are given explicitly /// by the specification. #[derive(Clone, Deserialize, Serialize, JsonSchema)] @@ -116,28 +70,6 @@ impl std::fmt::Debug for BlobStorageBackend { } } -impl MigrationElement for BlobStorageBackend { - fn kind(&self) -> &'static str { - "BlobStorageBackend" - } - - fn can_migrate_from_element( - &self, - other: &Self, - ) -> Result<(), crate::instance_spec::migration::ElementCompatibilityError> - { - if self.readonly != other.readonly { - Err(MigrationCompatibilityError::ComponentConfiguration(format!( - "read-only mismatch (self: {}, other: {})", - self.readonly, other.readonly, - )) - .into()) - } else { - Ok(()) - } - } -} - /// A network backend associated with a virtio-net (viona) VNIC on the host. #[derive(Clone, Deserialize, Serialize, Debug, JsonSchema)] #[serde(deny_unknown_fields)] @@ -146,20 +78,6 @@ pub struct VirtioNetworkBackend { pub vnic_name: String, } -impl MigrationElement for VirtioNetworkBackend { - fn kind(&self) -> &'static str { - "VirtioNetworkBackend" - } - - fn can_migrate_from_element( - &self, - _other: &Self, - ) -> Result<(), crate::instance_spec::migration::ElementCompatibilityError> - { - Ok(()) - } -} - /// A network backend associated with a DLPI VNIC on the host. #[derive(Clone, Deserialize, Serialize, Debug, JsonSchema)] #[serde(deny_unknown_fields)] @@ -167,23 +85,3 @@ pub struct DlpiNetworkBackend { /// The name of the VNIC to use as a backend. pub vnic_name: String, } - -impl MigrationElement for DlpiNetworkBackend { - fn kind(&self) -> &'static str { - "DlpiNetworkBackend" - } - - fn can_migrate_from_element( - &self, - _other: &Self, - ) -> Result<(), crate::instance_spec::migration::ElementCompatibilityError> - { - Ok(()) - } -} - -#[derive(Debug, Error)] -pub enum MigrationCompatibilityError { - #[error("component configurations incompatible: {0}")] - ComponentConfiguration(String), -} diff --git a/crates/propolis-api-types/src/instance_spec/components/board.rs b/crates/propolis-api-types/src/instance_spec/components/board.rs index 71370fea9..e9c3f59de 100644 --- a/crates/propolis-api-types/src/instance_spec/components/board.rs +++ b/crates/propolis-api-types/src/instance_spec/components/board.rs @@ -7,9 +7,6 @@ use schemars::JsonSchema; use serde::{Deserialize, Serialize}; -use thiserror::Error; - -use crate::instance_spec::migration::MigrationElement; /// An Intel 440FX-compatible chipset. #[derive( @@ -22,28 +19,6 @@ pub struct I440Fx { pub enable_pcie: bool, } -impl MigrationElement for I440Fx { - fn kind(&self) -> &'static str { - "i440fx" - } - - fn can_migrate_from_element( - &self, - other: &Self, - ) -> Result<(), crate::instance_spec::migration::ElementCompatibilityError> - { - if self.enable_pcie != other.enable_pcie { - Err(MigrationCompatibilityError::PcieMismatch( - self.enable_pcie, - other.enable_pcie, - ) - .into()) - } else { - Ok(()) - } - } -} - /// A kind of virtual chipset. #[derive( Clone, Copy, Deserialize, Serialize, Debug, PartialEq, Eq, JsonSchema, @@ -59,23 +34,6 @@ pub enum Chipset { I440Fx(I440Fx), } -impl MigrationElement for Chipset { - fn kind(&self) -> &'static str { - match self { - Self::I440Fx(_) => "i440fx", - } - } - - fn can_migrate_from_element( - &self, - other: &Self, - ) -> Result<(), crate::instance_spec::migration::ElementCompatibilityError> - { - let (Self::I440Fx(this), Self::I440Fx(other)) = (self, other); - this.can_migrate_from_element(other) - } -} - /// A VM's mainboard. #[derive(Clone, Deserialize, Serialize, Debug, PartialEq, Eq, JsonSchema)] #[serde(deny_unknown_fields)] @@ -101,81 +59,3 @@ impl Default for Board { } } } - -impl MigrationElement for Board { - fn kind(&self) -> &'static str { - "Board" - } - - fn can_migrate_from_element( - &self, - other: &Self, - ) -> Result<(), crate::instance_spec::migration::ElementCompatibilityError> - { - if self.cpus != other.cpus { - Err(MigrationCompatibilityError::CpuCount(self.cpus, other.cpus) - .into()) - } else if self.memory_mb != other.memory_mb { - Err(MigrationCompatibilityError::MemorySize( - self.memory_mb, - other.memory_mb, - ) - .into()) - } else if let Err(e) = - self.chipset.can_migrate_from_element(&other.chipset) - { - Err(e) - } else { - Ok(()) - } - } -} - -#[derive(Debug, Error)] -pub enum MigrationCompatibilityError { - #[error("Boards have different CPU counts (self: {0}, other: {1})")] - CpuCount(u8, u8), - - #[error("Boards have different memory amounts (self: {0}, other: {1})")] - MemorySize(u64, u64), - - #[error("Chipsets have different PCIe settings (self: {0}, other: {1})")] - PcieMismatch(bool, bool), -} - -#[cfg(test)] -mod test { - use super::*; - - #[test] - fn compatible_boards() { - let b1 = Board { - cpus: 8, - memory_mb: 8192, - chipset: Chipset::I440Fx(I440Fx { enable_pcie: false }), - }; - - assert!(b1.can_migrate_from_element(&b1).is_ok()); - } - - #[test] - fn incompatible_boards() { - let b1 = Board { - cpus: 4, - memory_mb: 4096, - chipset: Chipset::I440Fx(I440Fx { enable_pcie: true }), - }; - - let b2 = Board { cpus: 8, ..b1 }; - assert!(b1.can_migrate_from_element(&b2).is_err()); - - let b2 = Board { memory_mb: b1.memory_mb * 2, ..b1 }; - assert!(b1.can_migrate_from_element(&b2).is_err()); - - let b2 = Board { - chipset: Chipset::I440Fx(I440Fx { enable_pcie: false }), - ..b1 - }; - assert!(b1.can_migrate_from_element(&b2).is_err()); - } -} 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 b99e5c3b9..1aa9b7225 100644 --- a/crates/propolis-api-types/src/instance_spec/components/devices.rs +++ b/crates/propolis-api-types/src/instance_spec/components/devices.rs @@ -5,35 +5,9 @@ //! Device configuration data: components that define VM properties that are //! visible to a VM's guest software. -use crate::instance_spec::{migration::MigrationElement, PciPath}; +use crate::instance_spec::PciPath; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; -use thiserror::Error; - -fn backend_name_matches( - this: &str, - other: &str, -) -> Result<(), MigrationCompatibilityError> { - if this != other { - Err(MigrationCompatibilityError::BackendNameMismatch( - this.to_owned(), - other.to_owned(), - )) - } else { - Ok(()) - } -} - -fn pci_path_matches( - this: &PciPath, - other: &PciPath, -) -> Result<(), MigrationCompatibilityError> { - if this != other { - Err(MigrationCompatibilityError::PciPath(*this, *other)) - } else { - Ok(()) - } -} /// A disk that presents a virtio-block interface to the guest. #[derive(Clone, Deserialize, Serialize, Debug, JsonSchema)] @@ -46,22 +20,6 @@ pub struct VirtioDisk { pub pci_path: PciPath, } -impl MigrationElement for VirtioDisk { - fn kind(&self) -> &'static str { - "VirtioDisk" - } - - fn can_migrate_from_element( - &self, - other: &Self, - ) -> Result<(), crate::instance_spec::migration::ElementCompatibilityError> - { - backend_name_matches(&self.backend_name, &other.backend_name)?; - pci_path_matches(&self.pci_path, &other.pci_path)?; - Ok(()) - } -} - /// A disk that presents an NVMe interface to the guest. #[derive(Clone, Deserialize, Serialize, Debug, JsonSchema)] #[serde(deny_unknown_fields)] @@ -73,22 +31,6 @@ pub struct NvmeDisk { pub pci_path: PciPath, } -impl MigrationElement for NvmeDisk { - fn kind(&self) -> &'static str { - "NvmeDisk" - } - - fn can_migrate_from_element( - &self, - other: &Self, - ) -> Result<(), crate::instance_spec::migration::ElementCompatibilityError> - { - backend_name_matches(&self.backend_name, &other.backend_name)?; - pci_path_matches(&self.pci_path, &other.pci_path)?; - Ok(()) - } -} - /// A network card that presents a virtio-net interface to the guest. #[derive(Clone, Deserialize, Serialize, Debug, JsonSchema)] #[serde(deny_unknown_fields)] @@ -106,22 +48,6 @@ pub struct VirtioNic { pub pci_path: PciPath, } -impl MigrationElement for VirtioNic { - fn kind(&self) -> &'static str { - "VirtioNic" - } - - fn can_migrate_from_element( - &self, - other: &Self, - ) -> Result<(), crate::instance_spec::migration::ElementCompatibilityError> - { - backend_name_matches(&self.backend_name, &other.backend_name)?; - pci_path_matches(&self.pci_path, &other.pci_path)?; - Ok(()) - } -} - /// A serial port identifier, which determines what I/O ports a guest can use to /// access a port. #[derive( @@ -145,28 +71,6 @@ pub struct SerialPort { pub num: SerialPortNumber, } -impl MigrationElement for SerialPort { - fn kind(&self) -> &'static str { - "SerialPort" - } - - fn can_migrate_from_element( - &self, - other: &Self, - ) -> Result<(), crate::instance_spec::migration::ElementCompatibilityError> - { - if self != other { - Err(MigrationCompatibilityError::ComponentConfiguration(format!( - "serial port number mismatch (self: {0:?}, other: {1:?})", - self, other - )) - .into()) - } else { - Ok(()) - } - } -} - /// A PCI-PCI bridge. #[derive( Clone, Copy, Deserialize, Serialize, Debug, PartialEq, Eq, JsonSchema, @@ -182,44 +86,6 @@ pub struct PciPciBridge { pub pci_path: PciPath, } -impl MigrationElement for PciPciBridge { - fn kind(&self) -> &'static str { - "PciPciBridge" - } - - fn can_migrate_from_element( - &self, - other: &Self, - ) -> Result<(), crate::instance_spec::migration::ElementCompatibilityError> - { - pci_path_matches(&self.pci_path, &other.pci_path)?; - if self.downstream_bus != other.downstream_bus { - Err(MigrationCompatibilityError::ComponentConfiguration(format!( - "bridge downstream bus mismatch (self: {0}, other: {1})", - self.downstream_bus, other.downstream_bus - )) - .into()) - } else { - Ok(()) - } - } -} - -#[derive(Debug, Error)] -pub enum MigrationCompatibilityError { - /// The two devices have mismatched backend names. This means that migration - /// might fail to copy the source device's backend's state to the - /// corresponding target backend. - #[error("devices have different backend names (self: {0}, other: {1})")] - BackendNameMismatch(String, String), - - #[error("PCI devices have different paths (self: {0:?}, other: {1:?})")] - PciPath(PciPath, PciPath), - - #[error("component configurations incompatible: {0}")] - ComponentConfiguration(String), -} - #[derive( Clone, Copy, @@ -238,28 +104,6 @@ pub struct QemuPvpanic { // TODO(eliza): add support for the PCI PVPANIC device... } -impl MigrationElement for Option { - fn kind(&self) -> &'static str { - "QemuPvpanic" - } - - fn can_migrate_from_element( - &self, - other: &Self, - ) -> Result<(), crate::instance_spec::migration::ElementCompatibilityError> - { - if self != other { - Err(MigrationCompatibilityError::ComponentConfiguration(format!( - "pvpanic configuration mismatch (self: {0:?}, other: {1:?})", - self, other - )) - .into()) - } else { - Ok(()) - } - } -} - // // Structs for Falcon devices. These devices don't support live migration. // @@ -308,148 +152,3 @@ pub struct P9fs { /// The PCI path at which to attach the guest to this P9 filesystem. pub pci_path: PciPath, } - -#[cfg(test)] -mod test { - use super::*; - - #[test] - fn compatible_virtio_disk() { - let d1 = VirtioDisk { - backend_name: "storage_backend".to_string(), - pci_path: PciPath::new(0, 5, 0).unwrap(), - }; - assert!(d1.can_migrate_from_element(&d1).is_ok()); - } - - #[test] - fn incompatible_virtio_disk() { - let d1 = VirtioDisk { - backend_name: "storage_backend".to_string(), - pci_path: PciPath::new(0, 5, 0).unwrap(), - }; - - let d2 = VirtioDisk { backend_name: "other_backend".to_string(), ..d1 }; - assert!(d1.can_migrate_from_element(&d2).is_err()); - - let d2 = VirtioDisk { - pci_path: PciPath::new(0, 6, 0).unwrap(), - ..d1.clone() - }; - assert!(d1.can_migrate_from_element(&d2).is_err()); - } - - #[test] - fn compatible_nvme_disk() { - let d1 = NvmeDisk { - backend_name: "storage_backend".to_string(), - pci_path: PciPath::new(0, 5, 0).unwrap(), - }; - assert!(d1.can_migrate_from_element(&d1).is_ok()); - } - - #[test] - fn incompatible_nvme_disk() { - let d1 = NvmeDisk { - backend_name: "storage_backend".to_string(), - pci_path: PciPath::new(0, 5, 0).unwrap(), - }; - - let d2 = NvmeDisk { backend_name: "other_backend".to_string(), ..d1 }; - assert!(d1.can_migrate_from_element(&d2).is_err()); - - let d2 = - NvmeDisk { pci_path: PciPath::new(0, 6, 0).unwrap(), ..d1.clone() }; - assert!(d1.can_migrate_from_element(&d2).is_err()); - } - - #[test] - fn compatible_virtio_nic() { - let d1 = VirtioNic { - backend_name: "storage_backend".to_string(), - interface_id: uuid::Uuid::new_v4(), - pci_path: PciPath::new(0, 5, 0).unwrap(), - }; - assert!(d1.can_migrate_from_element(&d1).is_ok()); - } - - #[test] - fn incompatible_virtio_nic() { - let d1 = VirtioNic { - backend_name: "storage_backend".to_string(), - interface_id: uuid::Uuid::new_v4(), - pci_path: PciPath::new(0, 5, 0).unwrap(), - }; - - let d2 = VirtioNic { backend_name: "other_backend".to_string(), ..d1 }; - assert!(d1.can_migrate_from_element(&d2).is_err()); - - let d2 = VirtioNic { - pci_path: PciPath::new(0, 6, 0).unwrap(), - ..d1.clone() - }; - assert!(d1.can_migrate_from_element(&d2).is_err()); - } - - #[test] - fn serial_port_compatibility() { - let ports = [ - SerialPortNumber::Com1, - SerialPortNumber::Com2, - SerialPortNumber::Com3, - SerialPortNumber::Com4, - ]; - - for (p1, p2) in - ports.into_iter().flat_map(|p| std::iter::repeat(p).zip(ports)) - { - let can_migrate = SerialPort { num: p1 } - .can_migrate_from_element(&SerialPort { num: p2 }); - - assert_eq!( - p1 == p2, - can_migrate.is_ok(), - "p1: {:?}, p2: {:?}", - p1, - p2 - ); - } - } - - #[test] - fn pci_bridge_compatibility() { - let b1 = PciPciBridge { - downstream_bus: 1, - pci_path: PciPath::new(1, 2, 3).unwrap(), - }; - - let mut b2 = b1; - assert!(b1.can_migrate_from_element(&b2).is_ok()); - - b2.downstream_bus += 1; - assert!(b1.can_migrate_from_element(&b2).is_err()); - b2.downstream_bus = b1.downstream_bus; - - b2.pci_path = PciPath::new(4, 5, 6).unwrap(); - assert!(b1.can_migrate_from_element(&b2).is_err()); - } - - #[test] - fn incompatible_qemu_pvpanic() { - let d1 = Some(QemuPvpanic { enable_isa: true }); - let d2 = Some(QemuPvpanic { enable_isa: false }); - assert!(d1.can_migrate_from_element(&d2).is_err()); - assert!(d1.can_migrate_from_element(&None).is_err()); - } - - #[test] - fn compatible_qemu_pvpanic() { - let d1 = Some(QemuPvpanic { enable_isa: true }); - let d2 = Some(QemuPvpanic { enable_isa: true }); - assert!(d1.can_migrate_from_element(&d2).is_ok()); - - let d1 = Some(QemuPvpanic { enable_isa: false }); - let d2 = Some(QemuPvpanic { enable_isa: false }); - assert!(d1.can_migrate_from_element(&d2).is_ok()); - } -} diff --git a/crates/propolis-api-types/src/instance_spec/migration.rs b/crates/propolis-api-types/src/instance_spec/migration.rs deleted file mode 100644 index 061e81473..000000000 --- a/crates/propolis-api-types/src/instance_spec/migration.rs +++ /dev/null @@ -1,210 +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/. - -//! Traits for determining whether two instance specs describe VM configurations -//! that allow live migration from a Propolis using one spec to a Propolis using -//! the other. - -use std::collections::{HashMap, HashSet}; - -use crate::instance_spec::{components, SpecKey}; -use thiserror::Error; - -/// An error type describing mismatches between two spec elements--i.e., -/// descriptions of individual devices or components--that block migration. -#[derive(Debug, Error)] -pub enum ElementCompatibilityError { - #[error("component types aren't comparable (self: {0}, other: {1})")] - ComponentsIncomparable(&'static str, &'static str), - - #[error("boards not compatible: {0}")] - BoardsIncompatible(#[from] components::board::MigrationCompatibilityError), - - #[error("devices not compatible: {0}")] - DevicesIncompatible( - #[from] components::devices::MigrationCompatibilityError, - ), - - #[error("backends not compatible: {0}")] - BackendsIncompatible( - #[from] components::backends::MigrationCompatibilityError, - ), - - #[cfg(test)] - #[error("Test components differ")] - TestComponents(), -} - -/// An error type describing a mismatch between collections of elements that -/// blocks migration. -#[derive(Debug, Error)] -pub enum CollectionCompatibilityError { - #[error( - "Specs have collections with different lengths (self: {0}, other: {1})" - )] - CollectionSize(usize, usize), - - #[error("Collection key {0} present in self but absent from other")] - CollectionKeyAbsent(SpecKey), - - #[error("Spec element {0} mismatched: {1:?}")] - SpecElementMismatch(String, ElementCompatibilityError), -} - -/// The top-level migration compatibility error type. -#[derive(Debug, Error)] -pub enum MigrationCompatibilityError { - #[error("Collection {0} not compatible: {1}")] - CollectionMismatch(String, CollectionCompatibilityError), - - #[error("Spec element {0} not compatible: {1}")] - ElementMismatch(String, ElementCompatibilityError), -} - -/// Implementors of this trait are individual devices or VMM components who can -/// describe inconsistencies using an [`ElementCompatibilityError`] variant. -pub(crate) trait MigrationElement { - /// Returns a string indicating the kind of component this is for diagnostic - /// purposes. - fn kind(&self) -> &'static str; - - /// Returns true if `self` and `other` describe spec elements that are - /// similar enough to permit migration of this element from one VMM to - /// another. - fn can_migrate_from_element( - &self, - other: &Self, - ) -> Result<(), ElementCompatibilityError>; -} - -/// This trait implements migration compatibility checks for collection types, -/// which can be incompatible either because of a problem with the collection -/// itself or because of problems with one of the collection's members. -pub(crate) trait MigrationCollection { - fn can_migrate_from_collection( - &self, - other: &Self, - ) -> Result<(), CollectionCompatibilityError>; -} - -impl MigrationCollection for HashMap { - // Two keyed maps of components are compatible if they contain all the same - // keys and if, for each key, the corresponding values are - // migration-compatible. - fn can_migrate_from_collection( - &self, - other: &Self, - ) -> Result<(), CollectionCompatibilityError> { - // If the two maps have different sizes, then they have different key - // sets. - if self.len() != other.len() { - return Err(CollectionCompatibilityError::CollectionSize( - self.len(), - other.len(), - )); - } - - // Each key in `self`'s map must be present in `other`'s map, and the - // corresponding values must be compatible with one another. - for (key, this_val) in self.iter() { - let other_val = other.get(key).ok_or_else(|| { - CollectionCompatibilityError::CollectionKeyAbsent(key.clone()) - })?; - - this_val.can_migrate_from_element(other_val).map_err(|e| { - CollectionCompatibilityError::SpecElementMismatch( - key.clone(), - e, - ) - })?; - } - - Ok(()) - } -} - -impl MigrationCollection for HashSet { - // Two sets of spec keys are compatible if they have all the same members. - fn can_migrate_from_collection( - &self, - other: &Self, - ) -> Result<(), CollectionCompatibilityError> { - if self.len() != other.len() { - return Err(CollectionCompatibilityError::CollectionSize( - self.len(), - other.len(), - )); - } - - for key in self.iter() { - if !other.contains(key) { - return Err(CollectionCompatibilityError::CollectionKeyAbsent( - key.clone(), - )); - } - } - - Ok(()) - } -} - -#[cfg(test)] -mod test { - use super::*; - - #[derive(Clone, Copy, PartialEq, Eq)] - enum TestComponent { - Widget, - Gizmo, - Contraption, - } - - impl MigrationElement for TestComponent { - fn can_migrate_from_element( - &self, - other: &Self, - ) -> Result<(), ElementCompatibilityError> { - if self != other { - Err(ElementCompatibilityError::TestComponents()) - } else { - Ok(()) - } - } - - fn kind(&self) -> &'static str { - "TestComponent" - } - } - - // Verifies that the generic compatibility check for maps - // works correctly with a simple test type. - #[test] - fn generic_map_compatibility() { - let m1: HashMap = HashMap::from([ - ("widget".to_string(), TestComponent::Widget), - ("gizmo".to_string(), TestComponent::Gizmo), - ("contraption".to_string(), TestComponent::Contraption), - ]); - - let mut m2 = m1.clone(); - assert!(m1.can_migrate_from_collection(&m2).is_ok()); - - // Mismatched key counts make two maps incompatible. - m2.insert("second_widget".to_string(), TestComponent::Widget); - assert!(m1.can_migrate_from_collection(&m2).is_err()); - m2.remove("second_widget"); - - // Two maps are incompatible if their keys refer to components that are - // not compatible with each other. - *m2.get_mut("gizmo").unwrap() = TestComponent::Contraption; - assert!(m1.can_migrate_from_collection(&m2).is_err()); - *m2.get_mut("gizmo").unwrap() = TestComponent::Gizmo; - - // Two maps are incompatible if they have the same number of keys and - // values, but different sets of key names. - m2.remove("gizmo"); - m2.insert("other_gizmo".to_string(), TestComponent::Gizmo); - assert!(m1.can_migrate_from_collection(&m2).is_err()); - } -} diff --git a/crates/propolis-api-types/src/instance_spec/mod.rs b/crates/propolis-api-types/src/instance_spec/mod.rs index 643eb6d9a..a907eb01f 100644 --- a/crates/propolis-api-types/src/instance_spec/mod.rs +++ b/crates/propolis-api-types/src/instance_spec/mod.rs @@ -159,7 +159,6 @@ use serde::{Deserialize, Serialize}; pub use propolis_types::PciPath; pub mod components; -pub mod migration; pub mod v0; /// Type alias for keys in the instance spec's maps. diff --git a/crates/propolis-api-types/src/instance_spec/v0.rs b/crates/propolis-api-types/src/instance_spec/v0.rs index 344f83fcb..13b5cedbb 100644 --- a/crates/propolis-api-types/src/instance_spec/v0.rs +++ b/crates/propolis-api-types/src/instance_spec/v0.rs @@ -21,14 +21,7 @@ use std::collections::HashMap; -use crate::instance_spec::{ - components, - migration::{ - ElementCompatibilityError, MigrationCollection, - MigrationCompatibilityError, MigrationElement, - }, - SpecKey, -}; +use crate::instance_spec::{components, SpecKey}; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; @@ -39,53 +32,12 @@ pub enum StorageDeviceV0 { NvmeDisk(components::devices::NvmeDisk), } -impl MigrationElement for StorageDeviceV0 { - fn kind(&self) -> &'static str { - match self { - StorageDeviceV0::VirtioDisk(_) => "StorageDevice(VirtioDisk)", - StorageDeviceV0::NvmeDisk(_) => "StorageDevice(NvmeDisk)", - } - } - - fn can_migrate_from_element( - &self, - other: &Self, - ) -> Result<(), super::migration::ElementCompatibilityError> { - match (self, other) { - (Self::VirtioDisk(this), Self::VirtioDisk(other)) => { - this.can_migrate_from_element(other) - } - (Self::NvmeDisk(this), Self::NvmeDisk(other)) => { - this.can_migrate_from_element(other) - } - (_, _) => Err(ElementCompatibilityError::ComponentsIncomparable( - self.kind(), - other.kind(), - )), - } - } -} - #[derive(Clone, Deserialize, Serialize, Debug, JsonSchema)] #[serde(deny_unknown_fields, tag = "type", content = "component")] pub enum NetworkDeviceV0 { VirtioNic(components::devices::VirtioNic), } -impl MigrationElement for NetworkDeviceV0 { - fn kind(&self) -> &'static str { - "NetworkDevice(VirtioNic)" - } - - fn can_migrate_from_element( - &self, - other: &Self, - ) -> Result<(), ElementCompatibilityError> { - let (Self::VirtioNic(this), Self::VirtioNic(other)) = (self, other); - this.can_migrate_from_element(other) - } -} - #[derive(Default, Clone, Deserialize, Serialize, Debug, JsonSchema)] #[serde(deny_unknown_fields)] pub struct DeviceSpecV0 { @@ -119,64 +71,6 @@ pub struct DeviceSpecV0 { pub p9fs: Option, } -impl DeviceSpecV0 { - pub fn can_migrate_devices_from( - &self, - other: &Self, - ) -> Result<(), MigrationCompatibilityError> { - self.board.can_migrate_from_element(&other.board).map_err(|e| { - MigrationCompatibilityError::ElementMismatch("board".to_string(), e) - })?; - - self.storage_devices - .can_migrate_from_collection(&other.storage_devices) - .map_err(|e| { - MigrationCompatibilityError::CollectionMismatch( - "storage devices".to_string(), - e, - ) - })?; - - self.network_devices - .can_migrate_from_collection(&other.network_devices) - .map_err(|e| { - MigrationCompatibilityError::CollectionMismatch( - "storage devices".to_string(), - e, - ) - })?; - - self.serial_ports - .can_migrate_from_collection(&other.serial_ports) - .map_err(|e| { - MigrationCompatibilityError::CollectionMismatch( - "serial ports".to_string(), - e, - ) - })?; - - self.pci_pci_bridges - .can_migrate_from_collection(&other.pci_pci_bridges) - .map_err(|e| { - MigrationCompatibilityError::CollectionMismatch( - "PCI bridges".to_string(), - e, - ) - })?; - - self.qemu_pvpanic - .can_migrate_from_element(&other.qemu_pvpanic) - .map_err(|e| { - MigrationCompatibilityError::ElementMismatch( - "QEMU PVPANIC device".to_string(), - e, - ) - })?; - - Ok(()) - } -} - #[derive(Clone, Deserialize, Serialize, Debug, JsonSchema)] #[serde(deny_unknown_fields, tag = "type", content = "component")] pub enum StorageBackendV0 {