wip: teach propolis-server to understand configurable boot order

Author: iximeow

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

@@ -28,12 +28,18 @@ use propolis::hw::ibmpc;
 use propolis::hw::pci;
 use propolis::hw::ps2::ctrl::PS2Ctrl;
 use propolis::hw::qemu::pvpanic::QemuPvpanic;
-use propolis::hw::qemu::{debug::QemuDebugPort, fwcfg, ramfb};
+use propolis::hw::qemu::{
+    debug::QemuDebugPort,
+    fwcfg::{self, Entry},
+    ramfb,
+};
 use propolis::hw::uart::LpcUart;
 use propolis::hw::{nvme, virtio};
 use propolis::intr_pins;
 use propolis::vmm::{self, Builder, Machine};
-use propolis_api_types::instance_spec::{self, v0::InstanceSpecV0};
+use propolis_api_types::instance_spec::{
+    self, v0::BootDeclaration, v0::InstanceSpecV0,
+};
 use propolis_api_types::InstanceProperties;
 use slog::info;
 
@@ -117,6 +123,7 @@ pub struct MachineInitializer<'a> {
     pub(crate) properties: &'a InstanceProperties,
     pub(crate) toml_config: &'a crate::server::VmTomlConfig,
     pub(crate) producer_registry: Option<ProducerRegistry>,
+    pub(crate) boot_order: Option<Vec<BootDeclaration>>,
     pub(crate) state: MachineInitializerState,
 }
 
@@ -994,6 +1001,121 @@ impl<'a> MachineInitializer<'a> {
         smb_tables.commit()
     }
 
+    fn generate_bootorder(&self) -> Result<Option<Entry>, Error> {
+        eprintln!(
+            "generating bootorder with order: {:?}",
+            self.boot_order.as_ref()
+        );
+        let Some(boot_names) = self.boot_order.as_ref() else {
+            return Ok(None);
+        };
+
+        let mut order = fwcfg::formats::BootOrder::new();
+
+        let parse_bdf =
+            |pci_path: &propolis_api_types::instance_spec::PciPath| {
+                let bdf: Result<pci::Bdf, Error> =
+                    pci_path.to_owned().try_into().map_err(|e| {
+                        Error::new(
+                            ErrorKind::InvalidInput,
+                            format!(
+                                "Couldn't get PCI BDF for {}: {}",
+                                pci_path, e
+                            ),
+                        )
+                    });
+
+                bdf
+            };
+
+        for boot_entry in boot_names.iter() {
+            if boot_entry.first_boot_only {
+                continue;
+            }
+            // names may refer to a storage device or network device.
+            //
+            // realistically we won't be booting from network devices, but leave that as a question
+            // for plumbing on the next layer up.
+
+            let storage_backend = |spec| {
+                let spec: &instance_spec::v0::StorageDeviceV0 = spec;
+                match spec {
+                    instance_spec::v0::StorageDeviceV0::VirtioDisk(disk) => {
+                        &disk.backend_name
+                    }
+                    instance_spec::v0::StorageDeviceV0::NvmeDisk(disk) => {
+                        &disk.backend_name
+                    }
+                }
+            };
+
+            let network_backend = |spec| {
+                let spec: &instance_spec::v0::NetworkDeviceV0 = spec;
+                let instance_spec::v0::NetworkDeviceV0::VirtioNic(vnic_spec) =
+                    spec;
+
+                &vnic_spec.backend_name
+            };
+
+            if let Some(device_spec) =
+                self.spec.devices.storage_devices.iter().find_map(
+                    |(_name, spec)| {
+                        if storage_backend(spec) == &boot_entry.name {
+                            Some(spec)
+                        } else {
+                            None
+                        }
+                    },
+                )
+            {
+                match device_spec {
+                    instance_spec::v0::StorageDeviceV0::VirtioDisk(disk) => {
+                        let bdf = parse_bdf(&disk.pci_path)?;
+                        // TODO: check that bus is 0. only support boot devices
+                        // directly attached to the root bus for now.
+                        order.add_disk(bdf.location);
+                    }
+                    instance_spec::v0::StorageDeviceV0::NvmeDisk(disk) => {
+                        let bdf = parse_bdf(&disk.pci_path)?;
+                        // TODO: check that bus is 0. only support boot devices
+                        // directly attached to the root bus for now.
+                        //
+                        // TODO: separately, propolis-standalone passes an eui64
+                        // of 0, so do that here too. is that.. ok?
+                        order.add_nvme(bdf.location, 0);
+                    }
+                };
+            } else if let Some(vnic_spec) =
+                self.spec.devices.network_devices.iter().find_map(
+                    |(name, spec)| {
+                        if network_backend(spec) == &boot_entry.name {
+                            Some(spec)
+                        } else {
+                            None
+                        }
+                    },
+                )
+            {
+                let instance_spec::v0::NetworkDeviceV0::VirtioNic(vnic_spec) =
+                    vnic_spec;
+
+                let bdf = parse_bdf(&vnic_spec.pci_path)?;
+
+                order.add_pci(bdf.location, "ethernet");
+            } else {
+                eprintln!(
+                    "could not find {:?} in {:?} or {:?}",
+                    boot_entry,
+                    self.spec.devices.storage_devices,
+                    self.spec.devices.network_devices
+                );
+                panic!("TODO: return an error; the device name doesn't exist?");
+            }
+        }
+
+        Ok(Some(order.finish()))
+    }
+
     /// Initialize qemu `fw_cfg` device, and populate it with data including CPU
     /// count, SMBIOS tables, and attached RAM-FB device.
     ///
@@ -1010,6 +1132,13 @@ impl<'a> MachineInitializer<'a> {
             )
             .unwrap();
 
+        // TODO(ixi): extract `generate_smbios` and expect `smbios::TableBytes` as an argument to
+        // initialize_fwcfg. make `generate_smbios` accept rom size as an explicit parameter. this
+        // avoids the implicit panic if these are called out of order (and makes it harder to call
+        // out of order).
+        //
+        // one step towards sharing smbios init code between propolis-standalone and
+        // propolis-server.
         let smbios::TableBytes { entry_point, structure_table } =
             self.generate_smbios();
         fwcfg
@@ -1025,6 +1154,10 @@ impl<'a> MachineInitializer<'a> {
             )
             .unwrap();
 
+        if let Some(boot_order) = self.generate_bootorder()? {
+            fwcfg.insert_named("bootorder", boot_order).unwrap();
+        }
+
         let ramfb = ramfb::RamFb::create(
             self.log.new(slog::o!("component" => "ramfb")),
         );

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

@@ -177,6 +177,7 @@ impl<'a> VmEnsureNotStarted<'a> {
             properties,
             toml_config: &options.toml_config,
             producer_registry: options.oximeter_registry.clone(),
+            boot_order: v0_spec.devices.boot_order.clone(),
             state: MachineInitializerState::default(),
         };
 

bin/propolis-standalone/src/config.rs

@@ -136,7 +136,9 @@ pub fn block_backend(
     log: &slog::Logger,
 ) -> (Arc<dyn block::Backend>, String) {
     let backend_name = dev.options.get("block_dev").unwrap().as_str().unwrap();
-    let be = config.block_devs.get(backend_name).unwrap();
+    let Some(be) = config.block_devs.get(backend_name) else {
+        panic!("no configured block device named \"{}\"", backend_name);
+    };
     let opts = block::BackendOpts {
         block_size: be.block_opts.block_size,
         read_only: be.block_opts.read_only,

bin/propolis-standalone/src/main.rs

@@ -959,8 +959,13 @@ fn generate_smbios(params: SmbiosParams) -> anyhow::Result<smbios::TableBytes> {
     Ok(smb_tables.commit())
 }
 
-fn generate_bootorder(config: &config::Config) -> anyhow::Result<fwcfg::Entry> {
-    let names = config.main.boot_order.as_ref().unwrap();
+fn generate_bootorder(
+    config: &config::Config,
+) -> anyhow::Result<Option<fwcfg::Entry>> {
+    eprintln!("bootorder declared as {:?}", config.main.boot_order.as_ref());
+    let Some(names) = config.main.boot_order.as_ref() else {
+        return Ok(None);
+    };
 
     let mut order = fwcfg::formats::BootOrder::new();
     for name in names.iter() {
@@ -994,7 +999,7 @@ fn generate_bootorder(config: &config::Config) -> anyhow::Result<fwcfg::Entry> {
             }
         }
     }
-    Ok(order.finish())
+    Ok(Some(order.finish()))
 }
 
 fn setup_instance(
@@ -1306,14 +1311,10 @@ fn setup_instance(
 
     // It is "safe" to generate bootorder (if requested) now, given that PCI
     // device configuration has been validated by preceding logic
-    if config.main.boot_order.is_some() {
-        fwcfg
-            .insert_named(
-                "bootorder",
-                generate_bootorder(&config)
-                    .context("Failed to generate boot order")?,
-            )
-            .unwrap();
+    if let Some(boot_config) =
+        generate_bootorder(&config).context("Failed to generate boot order")?
+    {
+        fwcfg.insert_named("bootorder", boot_config).unwrap();
     }
 
     fwcfg.attach(pio, &machine.acc_mem);

crates/propolis-api-types/src/instance_spec/v0/mod.rs

@@ -109,6 +109,11 @@ pub struct DeviceSpecV0 {
     #[serde(skip_serializing_if = "Option::is_none")]
     pub qemu_pvpanic: Option<components::devices::QemuPvpanic>,
 
+    // same as above
+    #[serde(default)]
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub boot_order: Option<Vec<BootDeclaration>>,
+
     #[cfg(feature = "falcon")]
     pub softnpu_pci_port: Option<components::devices::SoftNpuPciPort>,
     #[cfg(feature = "falcon")]
@@ -177,6 +182,12 @@ impl DeviceSpecV0 {
     }
 }
 
+#[derive(Clone, Deserialize, Serialize, Debug, JsonSchema)]
+pub struct BootDeclaration {
+    pub name: SpecKey,
+    pub first_boot_only: bool,
+}
+
 #[derive(Clone, Deserialize, Serialize, Debug, JsonSchema)]
 #[serde(deny_unknown_fields, tag = "type", content = "component")]
 pub enum StorageBackendV0 {

lib/propolis-client/src/instance_spec.rs

@@ -184,6 +184,35 @@ impl SpecBuilderV0 {
         }
     }
 
+    /// Sets a boot order. Names here refer to devices included in this spec.
+    ///
+    /// Permissible to not this if the implicit boot order is desired, but the implicit boot order
+    /// may be unstable across device addition and removal.
+    ///
+    /// XXX: talk about what happens if names are included that do not name real devices..?
+    ///
+    /// XXX: this should certainly return `&mut Self` - all the builders here should. check if any
+    /// of these are chained..?
+    pub fn set_boot_order(
+        &mut self,
+        boot_order: Vec<String>,
+    ) -> Result<&Self, SpecBuilderError> {
+        let boot_declarations = boot_order
+            .into_iter()
+            .map(|name| crate::types::BootDeclaration {
+                name,
+                first_boot_only: false,
+            })
+            .collect();
+        eprintln!("setting boot order to {:?}", boot_declarations);
+        self.spec.devices.boot_order = Some(boot_declarations);
+
+        // TODO: would be nice to warn if any of the devices named here are not devices in the spec
+        // though.
+
+        Ok(self)
+    }
+
     /// Yields the completed spec, consuming the builder.
     pub fn finish(self) -> InstanceSpecV0 {
         self.spec

openapi/propolis-server.json

@@ -662,6 +662,13 @@
             "additionalProperties": {
               "$ref": "#/components/schemas/StorageDeviceV0"
             }
+          },
+          "boot_order": {
+            "nullable": true,
+            "type": "array",
+            "items": {
+              "$ref": "#/components/schemas/BootDeclaration"
+            }
           }
         },
         "required": [
@@ -1463,6 +1470,24 @@
           "vcr_matches"
         ]
       },
+      "BootDeclaration": {
+        "description": "A boot preference for some device.",
+        "type": "object",
+        "properties": {
+          "name": {
+            "description": "The name of the device to attempt booting from.",
+            "type": "string"
+          },
+          "first_boot_only": {
+            "description": "Should we include this device in the boot order for reboots of a started instance?",
+            "type": "boolean"
+          }
+        },
+        "required": [
+          "name",
+          "first_boot_only"
+        ]
+      },
       "SerialPort": {
         "description": "A serial port device.",
         "type": "object",