Derive vnic device names/backends from PCI path (#204)

Two instance specs' network device and backend names must agree for the specs to be migration-compatible. Today, the spec generator derives these names from the name of the host vNIC to which the instance should bind, which name can and will change over a migration. Instead, generate network device/backend names from the NIC's PCI path, which is already not allowed to change during migration.

Author: gjcolombo

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

@@ -79,6 +79,20 @@ fn slot_to_pci_path(
     .map_err(|_| SpecBuilderError::PciSlotInvalid(slot.0, ty))
 }
 
+/// 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,
+/// and that can change between incarnations of an instance. The PCI path is
+/// unique to each NIC but must remain stable over a migration, so it's suitable
+/// for use in this naming scheme.
+///
+/// N.B. Migrating a NIC requires the source and target to agree on these names,
+///      so changing this routine's behavior will prevent Propolis processes
+///      with the old behavior from migrating processes with the new behavior.
+fn pci_path_to_nic_names(path: PciPath) -> (String, String) {
+    (format!("vnic-{}", path), format!("vnic-{}-backend", path))
+}
+
 /// A helper for building instance specs out of component parts.
 pub struct SpecBuilder {
     spec: InstanceSpec,
@@ -138,11 +152,12 @@ impl SpecBuilder {
         let pci_path = slot_to_pci_path(nic.slot, SlotType::NIC)?;
         self.register_pci_device(pci_path)?;
 
+        let (device_name, backend_name) = pci_path_to_nic_names(pci_path);
         if self
             .spec
             .network_backends
             .insert(
-                nic.name.to_string(),
+                backend_name.clone(),
                 NetworkBackend {
                     kind: NetworkBackendKind::Virtio {
                         vnic_name: nic.name.to_string(),
@@ -159,10 +174,7 @@ impl SpecBuilder {
         if self
             .spec
             .network_devices
-            .insert(
-                nic.name.to_string(),
-                NetworkDevice { backend_name: nic.name.to_string(), pci_path },
-            )
+            .insert(device_name, NetworkDevice { backend_name, pci_path })
             .is_some()
         {
             return Err(SpecBuilderError::DeviceNameInUse(
@@ -405,11 +417,12 @@ impl SpecBuilder {
             ))
         })?;
 
+        let (device_name, backend_name) = pci_path_to_nic_names(pci_path);
         if self
             .spec
             .network_backends
             .insert(
-                vnic_name.to_string(),
+                backend_name.clone(),
                 NetworkBackend {
                     kind: NetworkBackendKind::Virtio {
                         vnic_name: vnic_name.to_string(),
@@ -426,10 +439,7 @@ impl SpecBuilder {
         if self
             .spec
             .network_devices
-            .insert(
-                name.to_string(),
-                NetworkDevice { backend_name: vnic_name.to_string(), pci_path },
-            )
+            .insert(device_name, NetworkDevice { backend_name, pci_path })
             .is_some()
         {
             return Err(SpecBuilderError::DeviceNameInUse(name.to_string()));

crates/propolis-types/src/lib.rs

@@ -5,6 +5,7 @@
 //! can all use those types (and implement their own conversions to/from them)
 //! without any layering oddities.
 
+use std::fmt::Display;
 use std::io::{Error, ErrorKind};
 use std::str::FromStr;
 
@@ -93,13 +94,18 @@ impl FromStr for PciPath {
     }
 }
 
+impl Display for PciPath {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(f, "{}.{}.{}", self.0, self.1, self.2)
+    }
+}
+
 impl Serialize for PciPath {
     fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
     where
         S: Serializer,
     {
-        serializer
-            .serialize_str(format!("{}.{}.{}", self.0, self.1, self.2).as_str())
+        serializer.serialize_str(format!("{}", self).as_str())
     }
 }