Add support for programmable SMBIOS Type 1 table (#977)

Author: andrewjstone

bin/propolis-cli/src/main.rs

@@ -18,14 +18,14 @@ use futures::{future, SinkExt};
 use newtype_uuid::{GenericUuid, TypedUuid, TypedUuidKind, TypedUuidTag};
 use propolis_client::instance_spec::{
     BlobStorageBackend, Board, Chipset, ComponentV0, CrucibleStorageBackend,
-    GuestHypervisorInterface, HyperVFeatureFlag, I440Fx, InstanceSpecV0,
-    NvmeDisk, PciPath, QemuPvpanic, ReplacementComponent, SerialPort,
-    SerialPortNumber, SpecKey, VirtioDisk,
+    GuestHypervisorInterface, HyperVFeatureFlag, I440Fx, InstanceMetadata,
+    InstanceProperties, InstanceSpec, InstanceSpecGetResponse, NvmeDisk,
+    PciPath, QemuPvpanic, ReplacementComponent, SerialPort, SerialPortNumber,
+    SpecKey, VirtioDisk,
 };
 use propolis_client::support::nvme_serial_from_str;
 use propolis_client::types::{
-    InstanceEnsureRequest, InstanceInitializationMethod, InstanceMetadata,
-    InstanceSpecGetResponse,
+    InstanceEnsureRequest, InstanceInitializationMethod,
 };
 use propolis_config_toml::spec::toml_cpuid_to_spec_cpuid;
 use propolis_config_toml::spec::SpecConfig;
@@ -40,10 +40,7 @@ use uuid::Uuid;
 
 use propolis_client::{
     support::{InstanceSerialConsoleHelper, WSClientOffset},
-    types::{
-        InstanceProperties, InstanceStateRequested, InstanceVcrReplace,
-        MigrationState,
-    },
+    types::{InstanceStateRequested, InstanceVcrReplace, MigrationState},
     Client,
 };
 
@@ -208,7 +205,7 @@ struct VmConfig {
 }
 
 fn add_component_to_spec(
-    spec: &mut InstanceSpecV0,
+    spec: &mut InstanceSpec,
     id: SpecKey,
     component: ComponentV0,
 ) -> anyhow::Result<()> {
@@ -290,7 +287,7 @@ impl DiskRequest {
 }
 
 impl VmConfig {
-    fn instance_spec(&self) -> anyhow::Result<InstanceSpecV0> {
+    fn instance_spec(&self) -> anyhow::Result<InstanceSpec> {
         // If the configuration specifies an instance spec path, just read the
         // spec from that path and return it. Otherwise, construct a spec from
         // this configuration's component parts.
@@ -328,7 +325,7 @@ impl VmConfig {
             })
             .transpose()?;
 
-        let mut spec = InstanceSpecV0 {
+        let mut spec = InstanceSpec {
             board: Board {
                 chipset: Chipset::I440Fx(I440Fx { enable_pcie }),
                 cpuid: cpuid_profile,
@@ -345,6 +342,7 @@ impl VmConfig {
                 },
             },
             components: Default::default(),
+            smbios: None,
         };
 
         if let Some(from_toml) = from_toml {
@@ -519,7 +517,7 @@ async fn new_instance(
     client: &Client,
     name: String,
     id: Uuid,
-    spec: InstanceSpecV0,
+    spec: InstanceSpec,
     metadata: InstanceMetadata,
 ) -> anyhow::Result<()> {
     let properties = InstanceProperties {

bin/propolis-server/src/lib/initializer.rs

@@ -1002,21 +1002,26 @@ impl MachineInitializer<'_> {
             ..Default::default()
         };
 
-        let smb_type1 = smbios::table::Type1 {
-            manufacturer: "Oxide".try_into().unwrap(),
-            product_name: "OxVM".try_into().unwrap(),
-
-            serial_number: self
-                .properties
-                .id
-                .to_string()
-                .try_into()
-                .unwrap_or_default(),
-            uuid: self.properties.id.to_bytes_le(),
-
-            wake_up_type: type1::WakeUpType::PowerSwitch,
-            ..Default::default()
+        // If `spec` contains smbios_type1_input then use it. Otherwise use
+        // defaults.
+        let mut smb_type1 = smbios::table::Type1::default();
+        if let Some(smbios) = self.spec.smbios_type1_input.clone() {
+            smb_type1.manufacturer =
+                smbios.manufacturer.try_into().unwrap_or_default();
+            smb_type1.product_name =
+                smbios.product_name.try_into().unwrap_or_default();
+            smb_type1.serial_number =
+                smbios.serial_number.try_into().unwrap_or_default();
+            smb_type1.version =
+                smbios.version.to_string().try_into().unwrap_or_default();
+        } else {
+            smb_type1.manufacturer = "Oxide".try_into().unwrap();
+            smb_type1.product_name = "OxVM".try_into().unwrap();
+            smb_type1.serial_number =
+                self.properties.id.to_string().try_into().unwrap_or_default();
         };
+        smb_type1.uuid = self.properties.id.to_bytes_le();
+        smb_type1.wake_up_type = type1::WakeUpType::PowerSwitch;
 
         // The processor vendor, family/model/stepping, and brand string should
         // correspond to the values the guest will see if it queries CPUID.

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

@@ -37,7 +37,10 @@ use internal_dns_types::names::ServiceName;
 pub use nexus_client::Client as NexusClient;
 use oximeter::types::ProducerRegistry;
 use propolis_api_types as api;
-use propolis_api_types::instance_spec::SpecKey;
+use propolis_api_types::instance_spec::{
+    v0::InstanceSpecGetResponseV0, SpecKey,
+};
+use propolis_api_types::v0::InstanceInitializationMethodV0;
 use propolis_api_types::InstanceInitializationMethod;
 use propolis_server_api::PropolisServerApi;
 use rfb::tungstenite::BinaryWs;
@@ -190,7 +193,15 @@ async fn find_local_nexus_client(
     }
 }
 
-async fn instance_get_common(
+// DEPRECATED
+async fn v0_instance_get(
+    rqctx: &RequestContext<Arc<DropshotEndpointContext>>,
+) -> Result<InstanceSpecGetResponseV0, HttpError> {
+    let ctx = rqctx.context();
+    ctx.vm.v0_get().await.ok_or_else(not_created_error)
+}
+
+async fn instance_get(
     rqctx: &RequestContext<Arc<DropshotEndpointContext>>,
 ) -> Result<api::InstanceSpecGetResponse, HttpError> {
     let ctx = rqctx.context();
@@ -291,16 +302,111 @@ impl PropolisServerApi for PropolisServerImpl {
             })
     }
 
+    async fn v0_instance_ensure(
+        rqctx: RequestContext<Self::Context>,
+        request: TypedBody<api::v0::InstanceEnsureRequestV0>,
+    ) -> Result<HttpResponseCreated<api::InstanceEnsureResponse>, HttpError>
+    {
+        let server_context = rqctx.context();
+        let api::v0::InstanceEnsureRequestV0 { properties, init } =
+            request.into_inner();
+        let oximeter_registry = server_context
+            .static_config
+            .metrics
+            .as_ref()
+            .map(|_| ProducerRegistry::with_id(properties.id));
+
+        let nexus_client =
+            find_local_nexus_client(rqctx.server.local_addr, rqctx.log.clone())
+                .await;
+
+        let ensure_options = crate::vm::EnsureOptions {
+            bootrom_path: server_context.static_config.bootrom_path.clone(),
+            bootrom_version: server_context
+                .static_config
+                .bootrom_version
+                .clone(),
+            use_reservoir: server_context.static_config.use_reservoir,
+            metrics_config: server_context.static_config.metrics.clone(),
+            oximeter_registry,
+            nexus_client,
+            vnc_server: server_context.vnc_server.clone(),
+            local_server_addr: rqctx.server.local_addr,
+        };
+
+        let vm_init = match init {
+            InstanceInitializationMethodV0::Spec { spec } => spec
+                .try_into()
+                .map(|s| VmInitializationMethod::Spec(Box::new(s)))
+                .map_err(|e| {
+                    if let Some(s) = e.source() {
+                        format!("{e}: {s}")
+                    } else {
+                        e.to_string()
+                    }
+                }),
+            InstanceInitializationMethodV0::MigrationTarget {
+                migration_id,
+                src_addr,
+                replace_components,
+            } => Ok(VmInitializationMethod::Migration(MigrationTargetInfo {
+                migration_id,
+                src_addr,
+                replace_components,
+            })),
+        }
+        .map_err(|e| {
+            HttpError::for_bad_request(
+                None,
+                format!("failed to generate internal instance spec: {e}"),
+            )
+        })?;
+
+        let request = VmEnsureRequest { properties, init: vm_init };
+        server_context
+            .vm
+            .ensure(&server_context.log, request, ensure_options)
+            .await
+            .map(HttpResponseCreated)
+            .map_err(|e| match e {
+                VmError::ResultChannelClosed => HttpError::for_internal_error(
+                    "state driver unexpectedly dropped result channel"
+                        .to_string(),
+                ),
+                VmError::WaitingToInitialize
+                | VmError::AlreadyInitialized
+                | VmError::RundownInProgress => HttpError::for_client_error(
+                    Some(api::ErrorCode::AlreadyInitialized.to_string()),
+                    ClientErrorStatusCode::CONFLICT,
+                    "instance already initialized".to_string(),
+                ),
+                VmError::InitializationFailed(e) => {
+                    HttpError::for_internal_error(format!(
+                        "VM initialization failed: {e}"
+                    ))
+                }
+                _ => HttpError::for_internal_error(format!(
+                    "unexpected error from VM controller: {e}"
+                )),
+            })
+    }
+
     async fn instance_spec_get(
         rqctx: RequestContext<Self::Context>,
     ) -> Result<HttpResponseOk<api::InstanceSpecGetResponse>, HttpError> {
-        Ok(HttpResponseOk(instance_get_common(&rqctx).await?))
+        Ok(HttpResponseOk(instance_get(&rqctx).await?))
+    }
+
+    async fn v0_instance_spec_get(
+        rqctx: RequestContext<Self::Context>,
+    ) -> Result<HttpResponseOk<InstanceSpecGetResponseV0>, HttpError> {
+        Ok(HttpResponseOk(v0_instance_get(&rqctx).await?))
     }
 
     async fn instance_get(
         rqctx: RequestContext<Self::Context>,
     ) -> Result<HttpResponseOk<api::InstanceGetResponse>, HttpError> {
-        instance_get_common(&rqctx).await.map(|full| {
+        instance_get(&rqctx).await.map(|full| {
             HttpResponseOk(api::InstanceGetResponse {
                 instance: api::Instance {
                     properties: full.properties,

bin/propolis-server/src/lib/spec/api_spec_v0.rs

@@ -69,6 +69,10 @@ impl From<Spec> for InstanceSpecV0 {
             migration_failure,
             #[cfg(feature = "falcon")]
             softnpu,
+
+            // Not part of `InstanceSpecV0`. Added in `InstanceSpec` in API
+            // Version 2.0.0.
+            smbios_type1_input: _,
         } = val;
 
         // Inserts a component entry into the supplied map, asserting first that

bin/propolis-server/src/lib/spec/mod.rs

@@ -16,6 +16,7 @@
 
 use std::collections::BTreeMap;
 
+use crate::spec::api_spec_v0::ApiSpecError;
 use cpuid_utils::CpuidSet;
 use propolis_api_types::instance_spec::{
     components::{
@@ -29,9 +30,10 @@ use propolis_api_types::instance_spec::{
             SerialPortNumber, VirtioDisk, VirtioNic,
         },
     },
-    v0::ComponentV0,
+    v0::{ComponentV0, InstanceSpecV0},
     PciPath, SpecKey,
 };
+use propolis_api_types::{InstanceSpec, SmbiosType1Input};
 use thiserror::Error;
 
 #[cfg(feature = "failure-injection")]
@@ -47,6 +49,28 @@ use propolis_api_types::instance_spec::components::{
 pub(crate) mod api_spec_v0;
 pub(crate) mod builder;
 
+/// The code related to latest types does not go into a versioned module
+impl From<Spec> for InstanceSpec {
+    fn from(val: Spec) -> Self {
+        let smbios = val.smbios_type1_input.clone();
+        let InstanceSpecV0 { board, components } = InstanceSpecV0::from(val);
+        InstanceSpec { board, components, smbios }
+    }
+}
+
+/// The code related to latest types does not go into a versioned module
+impl TryFrom<InstanceSpec> for Spec {
+    type Error = ApiSpecError;
+
+    fn try_from(value: InstanceSpec) -> Result<Self, Self::Error> {
+        let InstanceSpec { board, components, smbios } = value;
+        let v0 = InstanceSpecV0 { board, components };
+        let mut spec: Spec = v0.try_into()?;
+        spec.smbios_type1_input = smbios;
+        Ok(spec)
+    }
+}
+
 #[derive(Debug, Error)]
 #[error("input component type can't convert to output type")]
 pub struct ComponentTypeMismatch;
@@ -78,6 +102,18 @@ pub(crate) struct Spec {
 
     #[cfg(feature = "falcon")]
     pub softnpu: SoftNpu,
+
+    // TODO: This is an option because there is no good way to generate a
+    // default implementation of `SmbiosType1Input`. The default `serial_number`
+    // field of `SmbiosType1Input` should be equivalent to the VM UUID for
+    // backwards compatibility, but that isn't currently possible.
+    //
+    // One way to fix this would be to remove the `Builder` and directly
+    // construct `Spec` from a function that takes an `InstanceSpecV0` and the
+    // VM UUID. This would replace `impl TryFrom<InstanceSpecV0> for Spec`, and
+    // would allow removing the `Default` derive on `Spec`, and the `Option`
+    // from the `smbios_type1_input` field.
+    pub smbios_type1_input: Option<SmbiosType1Input>,
 }
 
 /// The VM's mainboard.