Skip to content

Commit

Permalink
wip: teach propolis-server to understand configurable boot order
Browse files Browse the repository at this point in the history
  • Loading branch information
iximeow committed Sep 4, 2024
1 parent b278193 commit d0d140a
Show file tree
Hide file tree
Showing 10 changed files with 315 additions and 42 deletions.
137 changes: 135 additions & 2 deletions bin/propolis-server/src/lib/initializer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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,
}

Expand Down Expand Up @@ -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.
///
Expand All @@ -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 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
Expand All @@ -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")),
);
Expand Down
1 change: 1 addition & 0 deletions bin/propolis-server/src/lib/vm/ensure.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
};

Expand Down
4 changes: 3 additions & 1 deletion bin/propolis-standalone/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
23 changes: 12 additions & 11 deletions bin/propolis-standalone/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -994,7 +999,7 @@ fn generate_bootorder(config: &config::Config) -> anyhow::Result<fwcfg::Entry> {
}
}
}
Ok(order.finish())
Ok(Some(order.finish()))
}

fn setup_instance(
Expand Down Expand Up @@ -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);
Expand Down
11 changes: 11 additions & 0 deletions crates/propolis-api-types/src/instance_spec/v0/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")]
Expand Down Expand Up @@ -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 {
Expand Down
29 changes: 29 additions & 0 deletions lib/propolis-client/src/instance_spec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
25 changes: 25 additions & 0 deletions openapi/propolis-server.json
Original file line number Diff line number Diff line change
Expand Up @@ -662,6 +662,13 @@
"additionalProperties": {
"$ref": "#/components/schemas/StorageDeviceV0"
}
},
"boot_order": {
"nullable": true,
"type": "array",
"items": {
"$ref": "#/components/schemas/BootDeclaration"
}
}
},
"required": [
Expand Down Expand Up @@ -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",
Expand Down
Loading

0 comments on commit d0d140a

Please sign in to comment.