instance spec rework: move migration compat checks to propolis-server

bin/propolis-server/src/lib/migrate/destination.rs

@@ -346,14 +346,15 @@ impl<T: MigrateConn> RonV0<T> {
         }?;
         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

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<MigrateError> for HttpError {
             | MigrateError::ProtocolParse(_, _)
             | MigrateError::NoMatchingProtocol(_, _)
             | MigrateError::TargetInstanceInitializationFailed(_)
+            | MigrateError::PreambleParse(_)
+            | MigrateError::InstanceSpecsIncompatible(_)
             | MigrateError::InvalidInstanceState
             | MigrateError::Codec(_)
             | MigrateError::UnexpectedMessage

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<String>,
+    pub instance_spec: VersionedInstanceSpec,
     pub blobs: Vec<Vec<u8>>,
 }
 
-fn get_spec_backend_keys(spec: &InstanceSpecV0) -> BTreeSet<String> {
-    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.
 

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<StorageBackend> 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,16 @@ pub struct SoftNpu {
     pub p9fs: Option<P9fs>,
 }
 
+#[cfg(feature = "falcon")]
+impl SoftNpu {
+    pub fn is_empty(&self) -> bool {
+        self.pci_port.is_none()
+            && self.p9_device.is_none()
+            && self.p9fs.is_none()
+            && self.ports.is_empty()
+    }
+}
+
 struct ParsedDiskRequest {
     name: String,
     disk: Disk,

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,44 +78,10 @@ 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)]
 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),
-}