server: remove config TOML processing

Remove the propolis-server logic to add devices from the config TOML when fielding an instance ensure request. This logic never adds any devices in production anyway because the zone image's config TOML never specifies any. The config TOML does specify the path to the bootrom the server should load (and an optional version string to display to the guest). Supply these arguments on the command line instead of getting them from the TOML. Once this is done, the config TOML in the zone image is unnecessary, so remove it.
oxidecomputer · Dec 2, 2024 · 8740130 · 8740130
1 parent 15ad57c
commit 8740130
Show file tree

Hide file tree

Showing 18 changed files with 91 additions and 240 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/bin/propolis-server/Cargo.toml b/bin/propolis-server/Cargo.toml
@@ -59,7 +59,6 @@ strum = { workspace = true, features = ["derive"] }
 propolis = { workspace = true, features = ["crucible-full", "oximeter"] }
 propolis_api_types = { workspace = true }
 propolis_types.workspace = true
-propolis-server-config.workspace = true
 rgb_frame.workspace = true
 rfb = { workspace = true, features = ["tungstenite"] }
 uuid.workspace = true

diff --git a/bin/propolis-server/README.md b/bin/propolis-server/README.md
@@ -1,66 +1,24 @@
 # Propolis Server
 
-## Running
-
-Propolis is mostly intended to be used via a REST API to drive all of its
-functionality. The standard `cargo build` will produce a `propolis-server`
-binary you can run:
-
-```
-# propolis-server run <config_file> <ip:port>
-```
-
-Note that the server must run as root. One way to ensure propolis-server has
-sufficient privileges is by using `pfexec(1)`, as such:
-
-```
-# pfexec propolis-server run <config_file> <ip:port>
-```
-
-## Example Configuration
+This binary provides a REST API to create and manage a Propolis VM. It typically
+runs in the context of a complete Oxide control plane deployment, but it can
+also be run as a freestanding binary for ad hoc testing of Propolis VMs.
 
-**Note**: the goal is to move the device config from the toml to instead be
-configured via REST API calls.
-
-```toml
-bootrom = "/path/to/bootrom/OVMF_CODE.fd"
+## Running
 
-[block_dev.alpine_iso]
-type = "file"
-path = "/path/to/alpine-extended-3.12.0-x86_64.iso"
+The server requires a path to a [guest bootrom
+image](../propolis-standalone#guest-bootrom) on the local filesystem. It also
+must be run with privileges sufficient to create bhyve virtual machines. The
+`pfexec(1)` utility can help enable these privileges.
 
-[dev.block0]
-driver = "pci-virtio-block"
-block_dev = "alpine_iso"
-pci-path = "0.4.0"
+To build and run the server:
 
-[dev.net0]
-driver = "pci-virtio-viona"
-vnic = "vnic_name"
-pci-path = "0.5.0"
+```bash
+cargo build --bin propolis-server
+pfexec target/debug/propolis-server <path_to_bootrom> <ip:port> <vnc_ip:port>
 ```
 
-## Prerequisites
-
-When running the server by hand, the appropriate bootrom is required to start
-guests properly.  See the [standalone
-documentation](../propolis-standalone#guest-bootrom) for more details.  Details
-for [creating necessary vnics](../propolis-standalone#vnic) can be found there
-as well, if exposing network devices to the guest.
-
-## CLI Interaction
-
-Once you've got `propolis-server` running you can interact with it via the REST
-API with any of the usual suspects (e.g. cURL, wget). Alternatively, there's a
-`propolis-cli` binary to make things a bit easier:
-
-### Running
-
-The following CLI commands will create a VM, start the VM, and then attach to
-its serial console:
-
-```
-# propolis-cli -s <propolis ip> -p <propolis port> new <VM name>
-# propolis-cli -s <propolis ip> -p <propolis port> state <VM name> run
-# propolis-cli -s <propolis ip> -p <propolis port> serial <VM name>
-```
+The API will be served on `ip:port`. The easiest way to interact with the server
+is to use [`propolis-cli`](../propolis-cli), but you can also use tools like
+cURL to interact with the API directly. The server's OpenAPI specification is
+[checked into the repo](../../openapi/propolis-server.json).
diff --git a/bin/propolis-server/src/lib/config.rs b/bin/propolis-server/src/lib/config.rs
@@ -4,8 +4,6 @@
 
 //! Describes a server config which may be parsed from a TOML file.
 
-pub use propolis_server_config::*;
-
 #[cfg(not(feature = "omicron-build"))]
 pub fn reservoir_decide(log: &slog::Logger) -> bool {
     // Automatically enable use of the memory reservoir (rather than transient

diff --git a/bin/propolis-server/src/lib/initializer.rs b/bin/propolis-server/src/lib/initializer.rs
@@ -187,7 +187,6 @@ pub struct MachineInitializer<'a> {
     pub(crate) crucible_backends: CrucibleBackendMap,
     pub(crate) spec: &'a Spec,
     pub(crate) properties: &'a InstanceProperties,
-    pub(crate) toml_config: &'a crate::server::VmTomlConfig,
     pub(crate) producer_registry: Option<ProducerRegistry>,
     pub(crate) state: MachineInitializerState,
     pub(crate) kstat_sampler: Option<KstatSampler>,
@@ -911,14 +910,15 @@ impl<'a> MachineInitializer<'a> {
         chipset.pci_attach(p9fs.pci_path.into(), vio9p);
     }
 
-    fn generate_smbios(&self) -> smbios::TableBytes {
+    fn generate_smbios(
+        &self,
+        bootrom_version: &Option<String>,
+    ) -> smbios::TableBytes {
         use smbios::table::{type0, type1, type16, type4};
 
         let rom_size =
             self.state.rom_size_bytes.expect("ROM is already populated");
-        let bios_version = self
-            .toml_config
-            .bootrom_version
+        let bios_version = bootrom_version
             .as_deref()
             .unwrap_or("v0.8")
             .try_into()
@@ -1153,6 +1153,7 @@ impl<'a> MachineInitializer<'a> {
     pub fn initialize_fwcfg(
         &mut self,
         cpus: u8,
+        bootrom_version: &Option<String>,
     ) -> Result<Arc<ramfb::RamFb>, MachineInitError> {
         let fwcfg = fwcfg::FwCfg::new();
         fwcfg
@@ -1163,7 +1164,7 @@ impl<'a> MachineInitializer<'a> {
             .map_err(|e| MachineInitError::FwcfgInsertFailed("cpu count", e))?;
 
         let smbios::TableBytes { entry_point, structure_table } =
-            self.generate_smbios();
+            self.generate_smbios(bootrom_version);
         fwcfg
             .insert_named(
                 "etc/smbios/smbios-tables",

diff --git a/bin/propolis-server/src/lib/server.rs b/bin/propolis-server/src/lib/server.rs
@@ -14,6 +14,7 @@ use std::net::IpAddr;
 use std::net::Ipv6Addr;
 use std::net::SocketAddr;
 use std::net::SocketAddrV6;
+use std::path::PathBuf;
 use std::sync::Arc;
 
 use crate::{
@@ -42,7 +43,6 @@ use propolis_api_types as api;
 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;
@@ -70,8 +70,12 @@ pub struct MetricsEndpointConfig {
 /// this configuration at startup time and refers to it when manipulating its
 /// objects.
 pub struct StaticConfig {
-    /// The TOML-driven configuration for this server's instances.
-    pub vm: Arc<VmTomlConfig>,
+    /// The path to the bootrom image to expose to the guest.
+    pub bootrom_path: PathBuf,
+
+    /// The bootrom version string to expose to the guest. If None, machine
+    /// initialization chooses a default value.
+    pub bootrom_version: Option<String>,
 
     /// Whether to use the host's guest memory reservoir to back guest memory.
     pub use_reservoir: bool,
@@ -92,15 +96,17 @@ pub struct DropshotEndpointContext {
 impl DropshotEndpointContext {
     /// Creates a new server context object.
     pub fn new(
-        config: VmTomlConfig,
+        bootrom_path: PathBuf,
+        bootrom_version: Option<String>,
         use_reservoir: bool,
         log: slog::Logger,
         metric_config: Option<MetricsEndpointConfig>,
     ) -> Self {
         let vnc_server = VncServer::new(log.clone());
         Self {
             static_config: StaticConfig {
-                vm: Arc::new(config),
+                bootrom_path,
+                bootrom_version,
                 use_reservoir,
                 metrics: metric_config,
             },
@@ -115,12 +121,9 @@ impl DropshotEndpointContext {
 /// this crate, so implementing TryFrom for them is not allowed.)
 fn instance_spec_from_request(
     request: &api::InstanceEnsureRequest,
-    toml_config: &VmTomlConfig,
 ) -> Result<Spec, SpecBuilderError> {
     let mut spec_builder = SpecBuilder::new(request.vcpus, request.memory);
 
-    spec_builder.add_devices_from_config(toml_config)?;
-
     for nic in &request.nics {
         spec_builder.add_nic_from_request(nic)?;
     }
@@ -257,7 +260,8 @@ async fn instance_ensure_common(
             .await;
 
     let ensure_options = crate::vm::EnsureOptions {
-        toml_config: server_context.static_config.vm.clone(),
+        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,
@@ -300,19 +304,13 @@ async fn instance_ensure(
     rqctx: RequestContext<Arc<DropshotEndpointContext>>,
     request: TypedBody<api::InstanceEnsureRequest>,
 ) -> Result<HttpResponseCreated<api::InstanceEnsureResponse>, HttpError> {
-    let server_context = rqctx.context();
     let request = request.into_inner();
-    let instance_spec =
-        instance_spec_from_request(&request, &server_context.static_config.vm)
-            .map_err(|e| {
-                HttpError::for_bad_request(
-                    None,
-                    format!(
-                        "failed to generate instance spec from request: {:#?}",
-                        e
-                    ),
-                )
-            })?;
+    let instance_spec = instance_spec_from_request(&request).map_err(|e| {
+        HttpError::for_bad_request(
+            None,
+            format!("failed to generate instance spec from request: {:#?}", e),
+        )
+    })?;
 
     instance_ensure_common(
         rqctx,

diff --git a/bin/propolis-server/src/lib/spec/builder.rs b/bin/propolis-server/src/lib/spec/builder.rs
@@ -24,26 +24,22 @@ use propolis_api_types::instance_spec::components::devices::{
     P9fs, SoftNpuP9, SoftNpuPciPort,
 };
 
-use crate::{config, spec::SerialPortDevice};
+use crate::spec::SerialPortDevice;
 
 use super::{
     api_request::{self, DeviceRequestError},
-    config_toml::{ConfigTomlError, ParsedConfig},
     Board, BootOrderEntry, BootSettings, Disk, Nic, QemuPvpanic, SerialPort,
 };
 
 #[cfg(not(feature = "omicron-build"))]
 use super::MigrationFailure;
 
 #[cfg(feature = "falcon")]
-use super::{ParsedSoftNpu, SoftNpuPort};
+use super::SoftNpuPort;
 
 /// Errors that can arise while building an instance spec from component parts.
 #[derive(Debug, Error)]
 pub(crate) enum SpecBuilderError {
-    #[error("error parsing config TOML")]
-    ConfigToml(#[from] ConfigTomlError),
-
     #[error("error parsing device in ensure request")]
     DeviceRequest(#[from] DeviceRequestError),
 
@@ -192,59 +188,6 @@ impl SpecBuilder {
         Ok(())
     }
 
-    /// Adds all the devices and backends specified in the supplied
-    /// configuration TOML to the spec under construction.
-    pub fn add_devices_from_config(
-        &mut self,
-        config: &config::Config,
-    ) -> Result<(), SpecBuilderError> {
-        let parsed = ParsedConfig::try_from(config)?;
-
-        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.name, disk.disk)?;
-        }
-
-        for nic in parsed.nics {
-            self.add_network_device(nic.name, nic.nic)?;
-        }
-
-        for bridge in parsed.pci_bridges {
-            self.add_pci_bridge(bridge.name, bridge.bridge)?;
-        }
-
-        #[cfg(feature = "falcon")]
-        self.add_parsed_softnpu_devices(parsed.softnpu)?;
-
-        Ok(())
-    }
-
-    #[cfg(feature = "falcon")]
-    fn add_parsed_softnpu_devices(
-        &mut self,
-        devices: ParsedSoftNpu,
-    ) -> Result<(), SpecBuilderError> {
-        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, port.port)?;
-        }
-
-        if let Some(p9) = devices.p9_device {
-            self.set_softnpu_p9(p9)?;
-        }
-
-        if let Some(p9fs) = devices.p9fs {
-            self.set_p9fs(p9fs)?;
-        }
-
-        Ok(())
-    }
-
     /// Adds a PCI path to this builder's record of PCI locations with an
     /// attached device. If the path is already in use, returns an error.
     fn register_pci_device(

diff --git a/bin/propolis-server/src/lib/spec/mod.rs b/bin/propolis-server/src/lib/spec/mod.rs
@@ -46,7 +46,6 @@ use propolis_api_types::instance_spec::components::{
 mod api_request;
 pub(crate) mod api_spec_v0;
 pub(crate) mod builder;
-mod config_toml;
 
 #[derive(Debug, Error)]
 #[error("input component type can't convert to output type")]
@@ -317,26 +316,6 @@ struct ParsedNicRequest {
     nic: Nic,
 }
 
-struct ParsedPciBridgeRequest {
-    name: String,
-    bridge: PciPciBridge,
-}
-
-#[cfg(feature = "falcon")]
-struct ParsedSoftNpuPort {
-    name: String,
-    port: SoftNpuPort,
-}
-
-#[cfg(feature = "falcon")]
-#[derive(Default)]
-struct ParsedSoftNpu {
-    pub pci_port: Option<SoftNpuPciPort>,
-    pub ports: Vec<ParsedSoftNpuPort>,
-    pub p9_device: Option<SoftNpuP9>,
-    pub p9fs: Option<P9fs>,
-}
-
 /// Generates NIC device and backend names from the NIC's PCI path. This is
 /// needed because the `name` field in a propolis-client
 /// `NetworkInterfaceRequest` is actually the name of the host vNIC to bind to,