instance spec rework: remove most dependencies on InstanceSpecV0 from propolis-server

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

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

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)?;

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<VersionedInstanceSpec, SpecBuilderError> {
+) -> Result<Spec, SpecBuilderError> {
     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<Arc<DropshotEndpointContext>>,
-    request: api::InstanceSpecEnsureRequest,
+    properties: api::InstanceProperties,
+    migrate: Option<api::InstanceMigrateInitiateRequest>,
+    instance_spec: Spec,
 ) -> Result<HttpResponseCreated<api::InstanceEnsureResponse>, 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<Arc<DropshotEndpointContext>>,
     request: TypedBody<api::InstanceSpecEnsureRequest>,
 ) -> Result<HttpResponseCreated<api::InstanceEnsureResponse>, 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(

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<ParsedStorageDevice, DeviceRequestError> {
+) -> Result<ParsedDiskRequest, DeviceRequestError> {
     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,65 +91,55 @@ 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)
             })?,
         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<ParsedStorageDevice, DeviceRequestError> {
+) -> Result<ParsedDiskRequest, DeviceRequestError> {
     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<ParsedNetworkDevice, DeviceRequestError> {
+) -> Result<ParsedNicRequest, DeviceRequestError> {
     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]