Skip to content

Commit

Permalink
convert in-repo clients to use SpecKey
Browse files Browse the repository at this point in the history
  • Loading branch information
@gjcolombo
gjcolombo committed Dec 2, 2024
1 parent 594842f commit 4da775b
Show file tree
Hide file tree
Showing 5 changed files with 133 additions and 80 deletions.
50 changes: 26 additions & 24 deletions bin/propolis-cli/src/main.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use propolis_client::types::{
QemuPvpanic, ReplacementComponent, SerialPort, SerialPortNumber,
VirtioDisk,
};
use propolis_client::PciPath;
use propolis_client::{PciPath, SpecKey};
use propolis_config_toml::spec::SpecConfig;
use serde::{Deserialize, Serialize};
use slog::{o, Drain, Level, Logger};
Expand Down Expand Up @@ -192,11 +192,11 @@ struct VmConfig {

fn add_component_to_spec(
spec: &mut InstanceSpecV0,
name: String,
id: SpecKey,
component: ComponentV0,
) -> anyhow::Result<()> {
if spec.components.insert(name.clone(), component).is_some() {
anyhow::bail!("duplicate component ID {name:?}");
if spec.components.insert(id.to_string(), component).is_some() {
anyhow::bail!("duplicate component ID {id:?}");
}
Ok(())
}
Expand All @@ -214,9 +214,9 @@ struct DiskRequest {

#[derive(Clone, Debug)]
struct ParsedDiskRequest {
device_name: String,
device_id: SpecKey,
device_spec: ComponentV0,
backend_name: String,
backend_id: SpecKey,
backend_spec: CrucibleStorageBackend,
}

Expand All @@ -229,17 +229,17 @@ impl DiskRequest {
}

let slot = self.slot + 0x10;
let backend_name = format!("{}-backend", self.name);
let backend_id = SpecKey::Name(format!("{}-backend", self.name));
let pci_path = PciPath::new(0, slot, 0).with_context(|| {
format!("processing disk request {:?}", self.name)
})?;
let device_spec = match self.device.as_ref() {
"virtio" => ComponentV0::VirtioDisk(VirtioDisk {
backend_name: backend_name.clone(),
backend_id: backend_id.clone(),
pci_path,
}),
"nvme" => ComponentV0::NvmeDisk(NvmeDisk {
backend_name: backend_name.clone(),
backend_id: backend_id.clone(),
pci_path,
}),
_ => anyhow::bail!(
Expand All @@ -256,9 +256,9 @@ impl DiskRequest {
};

Ok(ParsedDiskRequest {
device_name: self.name.clone(),
device_id: SpecKey::Name(self.name.clone()),
device_spec,
backend_name,
backend_id,
backend_spec,
})
}
Expand Down Expand Up @@ -314,15 +314,15 @@ impl VmConfig {
.flatten()
{
let ParsedDiskRequest {
device_name,
device_id,
device_spec,
backend_name,
backend_id,
backend_spec,
} = disk_request.parse()?;
add_component_to_spec(&mut spec, device_name, device_spec)?;
add_component_to_spec(&mut spec, device_id, device_spec)?;
add_component_to_spec(
&mut spec,
backend_name,
backend_id,
ComponentV0::CrucibleStorageBackend(backend_spec),
)?;
}
Expand All @@ -338,16 +338,18 @@ impl VmConfig {

add_component_to_spec(
&mut spec,
CLOUD_INIT_NAME.to_owned(),
SpecKey::Name(CLOUD_INIT_NAME.to_owned()),
ComponentV0::VirtioDisk(VirtioDisk {
backend_name: CLOUD_INIT_BACKEND_NAME.to_owned(),
backend_id: SpecKey::Name(
CLOUD_INIT_BACKEND_NAME.to_owned(),
),
pci_path: PciPath::new(0, 0x18, 0).unwrap(),
}),
)?;

add_component_to_spec(
&mut spec,
CLOUD_INIT_BACKEND_NAME.to_owned(),
SpecKey::Name(CLOUD_INIT_BACKEND_NAME.to_owned()),
ComponentV0::BlobStorageBackend(BlobStorageBackend {
base64: bytes,
readonly: true,
Expand All @@ -362,7 +364,7 @@ impl VmConfig {
] {
add_component_to_spec(
&mut spec,
name.to_owned(),
SpecKey::Name(name.to_owned()),
ComponentV0::SerialPort(SerialPort { num: port }),
)?;
}
Expand All @@ -375,7 +377,7 @@ impl VmConfig {
{
add_component_to_spec(
&mut spec,
"com4".to_owned(),
SpecKey::Name("com4".to_owned()),
ComponentV0::SerialPort(SerialPort {
num: SerialPortNumber::Com4,
}),
Expand All @@ -384,7 +386,7 @@ impl VmConfig {

add_component_to_spec(
&mut spec,
"pvpanic".to_owned(),
SpecKey::Name("pvpanic".to_owned()),
ComponentV0::QemuPvpanic(QemuPvpanic { enable_isa: true }),
)?;

Expand Down Expand Up @@ -725,17 +727,17 @@ async fn migrate_instance(

let mut replace_components = HashMap::new();
for disk in disks {
let ParsedDiskRequest { backend_name, backend_spec, .. } =
let ParsedDiskRequest { backend_id, backend_spec, .. } =
disk.parse()?;

let old = replace_components.insert(
backend_name.clone(),
backend_id.to_string(),
ReplacementComponent::CrucibleStorageBackend(backend_spec),
);

if old.is_some() {
anyhow::bail!(
"duplicate backend name {backend_name} in replacement disk \
"duplicate backend name {backend_id} in replacement disk \
list"
);
}
Expand Down
47 changes: 24 additions & 23 deletions crates/propolis-config-toml/src/spec.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,12 @@ use propolis_client::{
SoftNpuPciPort, SoftNpuPort, VirtioDisk, VirtioNetworkBackend,
VirtioNic,
},
PciPath,
PciPath, SpecKey,
};
use thiserror::Error;

pub const MIGRATION_FAILURE_DEVICE_NAME: &str = "test-migration-failure";

type SpecKey = String;

#[derive(Debug, Error)]
pub enum TomlToSpecError {
#[error("unrecognized device type {0:?}")]
Expand Down Expand Up @@ -111,7 +109,7 @@ impl TryFrom<&super::Config> for SpecConfig {
.max(0) as u32;

spec.components.insert(
MIGRATION_FAILURE_DEVICE_NAME.to_owned(),
SpecKey::Name(MIGRATION_FAILURE_DEVICE_NAME.to_owned()),
ComponentV0::MigrationFailureInjector(
MigrationFailureInjector { fail_exports, fail_imports },
),
Expand Down Expand Up @@ -177,13 +175,14 @@ impl TryFrom<&super::Config> for SpecConfig {
TomlToSpecError::NoVnicName(device_name.to_owned())
})?;

let backend_name = format!("{}:backend", device_id);
let backend_name =
SpecKey::Name(format!("{device_id}:backend"));

spec.components.insert(
device_id,
ComponentV0::SoftNpuPort(SoftNpuPort {
link_name: device_name.to_string(),
backend_name: backend_name.clone(),
backend_id: backend_name.clone(),
}),
);

Expand Down Expand Up @@ -234,7 +233,7 @@ impl TryFrom<&super::Config> for SpecConfig {
})?;

spec.components.insert(
format!("pci-bridge-{}", bridge.pci_path),
SpecKey::Name(format!("pci-bridge-{}", bridge.pci_path)),
ComponentV0::PciPciBridge(PciPciBridge {
downstream_bus: bridge.downstream_bus,
pci_path,
Expand Down Expand Up @@ -266,30 +265,32 @@ fn parse_storage_device_from_config(
}
};

let backend_name = device
.options
.get("block_dev")
.ok_or_else(|| {
TomlToSpecError::NoBackendNameForStorageDevice(name.to_owned())
})?
.as_str()
.ok_or_else(|| {
TomlToSpecError::NoBackendNameForStorageDevice(name.to_owned())
})?
.to_string();
let backend_id = SpecKey::from_str(
device
.options
.get("block_dev")
.ok_or_else(|| {
TomlToSpecError::NoBackendNameForStorageDevice(name.to_owned())
})?
.as_str()
.ok_or_else(|| {
TomlToSpecError::NoBackendNameForStorageDevice(name.to_owned())
})?,
)
.expect("SpecKey::from_str is infallible");

let pci_path: PciPath = device
.get("pci-path")
.ok_or_else(|| TomlToSpecError::InvalidPciPath(name.to_owned()))?;

let id_to_return = backend_name.clone();
let id_to_return = backend_id.clone();
Ok((
match interface {
Interface::Virtio => {
ComponentV0::VirtioDisk(VirtioDisk { backend_name, pci_path })
ComponentV0::VirtioDisk(VirtioDisk { backend_id, pci_path })
}
Interface::Nvme => {
ComponentV0::NvmeDisk(NvmeDisk { backend_name, pci_path })
ComponentV0::NvmeDisk(NvmeDisk { backend_id, pci_path })
}
},
id_to_return,
Expand Down Expand Up @@ -356,10 +357,10 @@ fn parse_network_device_from_config(
.get("pci-path")
.ok_or_else(|| TomlToSpecError::InvalidPciPath(name.to_owned()))?;

let backend_id = format!("{name}-backend");
let backend_id = SpecKey::Name(format!("{name}-backend"));
Ok(ParsedNic {
device_spec: VirtioNic {
backend_name: backend_id.clone(),
backend_id: backend_id.clone(),
interface_id: uuid::Uuid::nil(),
pci_path,
},
Expand Down
7 changes: 4 additions & 3 deletions lib/propolis-client/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@

//! A client for the Propolis hypervisor frontend's server API.
// Re-export the PCI path type from propolis_api_types so that users can access
// its constructor and From impls.
pub use propolis_api_types::instance_spec::PciPath;
// Re-export types from propolis_api_types where callers may want to use
// constructors or From impls.
pub use propolis_api_types::instance_spec::{PciPath, SpecKey};

// Re-export Crucible client types that appear in their serialized forms in
// instance specs. This allows clients to ensure they serialize/deserialize
Expand All @@ -20,6 +20,7 @@ progenitor::generate_api!(
tags = Separate,
replace = {
PciPath = crate::PciPath,
SpecKey = crate::SpecKey,
},
patch = {
BootOrderEntry = { derives = [schemars::JsonSchema] },
Expand Down
Loading

0 comments on commit 4da775b

Please sign in to comment.