Squash all history due to jj merge fumble

dev-tools/omdb/src/bin/omdb/db.rs

@@ -58,6 +58,7 @@ use nexus_db_model::HwBaseboardId;
 use nexus_db_model::Image;
 use nexus_db_model::Instance;
 use nexus_db_model::InvCollection;
+use nexus_db_model::InvNvmeDiskFirmware;
 use nexus_db_model::InvPhysicalDisk;
 use nexus_db_model::IpAttachState;
 use nexus_db_model::IpKind;
@@ -4317,36 +4318,62 @@ async fn cmd_db_inventory_physical_disks(
         model: String,
         serial: String,
         variant: String,
+        firmware: String,
+        next_firmware: String,
     }
 
-    use db::schema::inv_physical_disk::dsl;
-    let mut query = dsl::inv_physical_disk.into_boxed();
+    use db::schema::inv_nvme_disk_firmware::dsl as firmware_dsl;
+    use db::schema::inv_physical_disk::dsl as disk_dsl;
+
+    let mut query = disk_dsl::inv_physical_disk.into_boxed();
     query = query.limit(i64::from(u32::from(limit)));
 
     if let Some(collection_id) = args.collection_id {
         query = query.filter(
-            dsl::inv_collection_id.eq(collection_id.into_untyped_uuid()),
+            disk_dsl::inv_collection_id.eq(collection_id.into_untyped_uuid()),
         );
     }
 
     if let Some(sled_id) = args.sled_id {
-        query = query.filter(dsl::sled_id.eq(sled_id.into_untyped_uuid()));
+        query = query.filter(disk_dsl::sled_id.eq(sled_id.into_untyped_uuid()));
     }
 
     let disks = query
-        .select(InvPhysicalDisk::as_select())
+        .left_join(
+            firmware_dsl::inv_nvme_disk_firmware.on(
+                firmware_dsl::inv_collection_id
+                    .eq(disk_dsl::inv_collection_id)
+                    .and(firmware_dsl::sled_id.eq(disk_dsl::sled_id))
+                    .and(firmware_dsl::slot.eq(disk_dsl::slot)),
+            ),
+        )
+        .select((
+            InvPhysicalDisk::as_select(),
+            Option::<InvNvmeDiskFirmware>::as_select(),
+        ))
         .load_async(&**conn)
         .await
         .context("loading physical disks")?;
 
-    let rows = disks.into_iter().map(|disk| DiskRow {
-        inv_collection_id: disk.inv_collection_id.into_untyped_uuid(),
-        sled_id: disk.sled_id.into_untyped_uuid(),
-        slot: disk.slot,
-        vendor: disk.vendor,
-        model: disk.model.clone(),
-        serial: disk.serial.clone(),
-        variant: format!("{:?}", disk.variant),
+    let rows = disks.into_iter().map(|(disk, firmware)| {
+        let (active_firmware, next_firmware) =
+            if let Some(firmware) = firmware.as_ref() {
+                (firmware.current_version(), firmware.next_version())
+            } else {
+                (None, None)
+            };
+
+        DiskRow {
+            inv_collection_id: disk.inv_collection_id.into_untyped_uuid(),
+            sled_id: disk.sled_id.into_untyped_uuid(),
+            slot: disk.slot,
+            vendor: disk.vendor,
+            model: disk.model.clone(),
+            serial: disk.serial.clone(),
+            variant: format!("{:?}", disk.variant),
+            firmware: active_firmware.unwrap_or("UNKNOWN").to_string(),
+            next_firmware: next_firmware.unwrap_or("").to_string(),
+        }
     });
 
     let table = tabled::Table::new(rows)
@@ -4752,6 +4779,7 @@ fn inv_collection_print_sleds(collection: &Collection) {
                 identity,
                 variant,
                 slot,
+                ..
             } = disk;
             println!("      {variant:?}: {identity:?} in {slot}");
         }

nexus-sled-agent-shared/src/inventory.rs

@@ -30,6 +30,14 @@ pub struct InventoryDisk {
     pub identity: omicron_common::disk::DiskIdentity,
     pub variant: DiskVariant,
     pub slot: i64,
+    // Today we only have NVMe disks so we embedded the firmware metadata here.
+    // In the future we can track firmware metadata in a unique type if we
+    // support more than one disk format.
+    pub active_firmware_slot: u8,
+    pub next_active_firmware_slot: Option<u8>,
+    pub number_of_firmware_slots: u8,
+    pub slot1_is_read_only: bool,
+    pub slot_firmware_versions: Vec<Option<String>>,
 }
 
 /// Identifies information about zpools managed by the control plane

nexus/db-model/src/inventory.rs

@@ -7,10 +7,10 @@
 use crate::omicron_zone_config::{self, OmicronZoneNic};
 use crate::schema::{
     hw_baseboard_id, inv_caboose, inv_collection, inv_collection_error,
-    inv_dataset, inv_omicron_zone, inv_omicron_zone_nic, inv_physical_disk,
-    inv_root_of_trust, inv_root_of_trust_page, inv_service_processor,
-    inv_sled_agent, inv_sled_omicron_zones, inv_zpool, sw_caboose,
-    sw_root_of_trust_page,
+    inv_dataset, inv_nvme_disk_firmware, inv_omicron_zone,
+    inv_omicron_zone_nic, inv_physical_disk, inv_root_of_trust,
+    inv_root_of_trust_page, inv_service_processor, inv_sled_agent,
+    inv_sled_omicron_zones, inv_zpool, sw_caboose, sw_root_of_trust_page,
 };
 use crate::typed_uuid::DbTypedUuid;
 use crate::PhysicalDiskKind;
@@ -33,7 +33,8 @@ use nexus_sled_agent_shared::inventory::{
     OmicronZoneConfig, OmicronZoneType, OmicronZonesConfig,
 };
 use nexus_types::inventory::{
-    BaseboardId, Caboose, Collection, PowerState, RotPage, RotSlot,
+    BaseboardId, Caboose, Collection, NvmeFirmware, PowerState, RotPage,
+    RotSlot,
 };
 use omicron_common::api::internal::shared::NetworkInterface;
 use omicron_common::zpool_name::ZpoolName;
@@ -885,16 +886,61 @@ impl InvPhysicalDisk {
     }
 }
 
-impl From<InvPhysicalDisk> for nexus_types::inventory::PhysicalDisk {
-    fn from(disk: InvPhysicalDisk) -> Self {
+/// See [`nexus_types::inventory::PhysicalDiskFirmware::Nvme`].
+#[derive(Queryable, Clone, Debug, Selectable, Insertable)]
+#[diesel(table_name = inv_nvme_disk_firmware)]
+pub struct InvNvmeDiskFirmware {
+    pub inv_collection_id: DbTypedUuid<CollectionKind>,
+    pub sled_id: DbTypedUuid<SledKind>,
+    pub slot: i64,
+    pub active_slot: SqlU8,
+    pub next_active_slot: Option<SqlU8>,
+    pub number_of_slots: SqlU8,
+    pub slot1_is_read_only: bool,
+    pub slot_firmware_versions: Vec<Option<String>>,
+}
+
+impl InvNvmeDiskFirmware {
+    pub fn new(
+        inv_collection_id: CollectionUuid,
+        sled_id: SledUuid,
+        sled_slot: i64,
+        firmware: &NvmeFirmware,
+    ) -> Self {
         Self {
-            identity: omicron_common::disk::DiskIdentity {
-                vendor: disk.vendor,
-                serial: disk.serial,
-                model: disk.model,
-            },
-            variant: disk.variant.into(),
-            slot: disk.slot,
+            inv_collection_id: inv_collection_id.into(),
+            sled_id: sled_id.into(),
+            slot: sled_slot,
+            active_slot: firmware.active_slot.into(),
+            next_active_slot: firmware.next_active_slot.map(|nas| nas.into()),
+            number_of_slots: firmware.number_of_slots.into(),
+            slot1_is_read_only: firmware.slot1_is_read_only,
+            slot_firmware_versions: firmware.slot_firmware_versions.clone(),
+        }
+    }
+
+    /// Attempt to read the current firmware version.
+    pub fn current_version(&self) -> Option<&str> {
+        match self.active_slot.0 {
+            // be paranoid that we have a value within the NVMe spec
+            slot @ 1..=7 => self
+                .slot_firmware_versions
+                .get(usize::from(slot) - 1)
+                .and_then(|v| v.as_deref()),
+            _ => None,
+        }
+    }
+
+    /// Attempt to read the staged firmware version that will be active upon
+    /// next device reset.
+    pub fn next_version(&self) -> Option<&str> {
+        match self.next_active_slot {
+            // be paranoid that we have a value within the NVMe spec
+            Some(slot) if slot.0 <= 7 && slot.0 >= 1 => self
+                .slot_firmware_versions
+                .get(usize::from(slot.0) - 1)
+                .and_then(|v| v.as_deref()),
+            _ => None,
         }
     }
 }
@@ -1551,3 +1597,105 @@ impl InvOmicronZoneNic {
         zone_nic.into_network_interface_for_zone(zone_id)
     }
 }
+
+#[cfg(test)]
+mod test {
+    use omicron_uuid_kinds::TypedUuid;
+
+    use crate::{typed_uuid, InvNvmeDiskFirmware};
+
+    #[test]
+    fn test_inv_nvme_disk_firmware() {
+        // We can retrieve active_firmware with sane values
+        let inv_firmware = InvNvmeDiskFirmware {
+            inv_collection_id: typed_uuid::DbTypedUuid(TypedUuid::new_v4()),
+            sled_id: TypedUuid::new_v4().into(),
+            slot: 1,
+            active_slot: 1.into(),
+            next_active_slot: None,
+            number_of_slots: 1.into(),
+            slot1_is_read_only: true,
+            slot_firmware_versions: vec![Some("firmware".to_string())],
+        };
+        assert_eq!(inv_firmware.current_version(), Some("firmware"));
+        assert_eq!(inv_firmware.next_version(), None);
+
+        // We can retrieve active_firmware and next_active_firmware with sane
+        // values
+        let inv_firmware = InvNvmeDiskFirmware {
+            inv_collection_id: typed_uuid::DbTypedUuid(TypedUuid::new_v4()),
+            sled_id: TypedUuid::new_v4().into(),
+            slot: 1,
+            active_slot: 1.into(),
+            next_active_slot: Some(2.into()),
+            number_of_slots: 2.into(),
+            slot1_is_read_only: true,
+            slot_firmware_versions: vec![
+                Some("ONE".to_string()),
+                Some("TWO".to_string()),
+            ],
+        };
+        assert_eq!(inv_firmware.current_version(), Some("ONE"));
+        assert_eq!(inv_firmware.next_version(), Some("TWO"));
+
+        // A missing fw version string returns None (although technically
+        // invalid as a device _shouldn't_ report an empty slot as being
+        // activated upon reset)
+        let inv_firmware = InvNvmeDiskFirmware {
+            inv_collection_id: typed_uuid::DbTypedUuid(TypedUuid::new_v4()),
+            sled_id: TypedUuid::new_v4().into(),
+            slot: 1,
+            active_slot: 1.into(),
+            next_active_slot: Some(2.into()),
+            number_of_slots: 2.into(),
+            slot1_is_read_only: true,
+            slot_firmware_versions: vec![Some("ONE".to_string()), None],
+        };
+        assert_eq!(inv_firmware.current_version(), Some("ONE"));
+        assert_eq!(inv_firmware.next_version(), None);
+
+        // Number of slots and slot firmware versions disagreement returns None
+        let inv_firmware = InvNvmeDiskFirmware {
+            inv_collection_id: typed_uuid::DbTypedUuid(TypedUuid::new_v4()),
+            sled_id: TypedUuid::new_v4().into(),
+            slot: 1,
+            active_slot: 1.into(),
+            next_active_slot: Some(4.into()),
+            number_of_slots: 4.into(),
+            slot1_is_read_only: true,
+            // number_of_slots reports 4 but the device only gave us two slots
+            slot_firmware_versions: vec![Some("ONE".to_string()), None],
+        };
+        assert_eq!(inv_firmware.current_version(), Some("ONE"));
+        assert_eq!(inv_firmware.next_version(), None);
+
+        // Verify active_slot and next_active_slot  below 1 or above 7 results
+        // in `None`
+        for i in [0u8, 8] {
+            let inv_firmware = InvNvmeDiskFirmware {
+                inv_collection_id: typed_uuid::DbTypedUuid(TypedUuid::new_v4()),
+                sled_id: TypedUuid::new_v4().into(),
+                slot: 1,
+                active_slot: i.into(),
+                next_active_slot: Some(i.into()),
+                number_of_slots: 7.into(),
+                slot1_is_read_only: true,
+                slot_firmware_versions: vec![
+                    Some("ONE".to_string()),
+                    Some("TWO".to_string()),
+                    Some("THREE".to_string()),
+                    Some("FOUR".to_string()),
+                    Some("FIVE".to_string()),
+                    Some("SIX".to_string()),
+                    Some("SEVEN".to_string()),
+                    // Invalid in the spec but we are testing that the active
+                    // and next active slot can't reach this value
+                    Some("EIGHT".to_string()),
+                ],
+            };
+
+            assert_eq!(inv_firmware.current_version(), None);
+            assert_eq!(inv_firmware.next_version(), None);
+        }
+    }
+}

nexus/db-model/src/schema.rs

@@ -1432,6 +1432,20 @@ table! {
     }
 }
 
+table! {
+    inv_nvme_disk_firmware (inv_collection_id, sled_id, slot) {
+        inv_collection_id -> Uuid,
+        sled_id -> Uuid,
+        slot -> Int8,
+
+        number_of_slots -> Int2,
+        active_slot -> Int2,
+        next_active_slot -> Nullable<Int2>,
+        slot1_is_read_only -> Bool,
+        slot_firmware_versions -> Array<Nullable<Text>>,
+    }
+}
+
 table! {
     inv_zpool (inv_collection_id, sled_id, id) {
         inv_collection_id -> Uuid,
@@ -1865,6 +1879,7 @@ allow_tables_to_appear_in_same_query!(
     network_interface,
     instance_network_interface,
     inv_physical_disk,
+    inv_nvme_disk_firmware,
     service_network_interface,
     oximeter,
     physical_disk,

nexus/db-model/src/schema_versions.rs

@@ -17,7 +17,7 @@ use std::collections::BTreeMap;
 ///
 /// This must be updated when you change the database schema.  Refer to
 /// schema/crdb/README.adoc in the root of this repository for details.
-pub const SCHEMA_VERSION: SemverVersion = SemverVersion::new(98, 0, 0);
+pub const SCHEMA_VERSION: SemverVersion = SemverVersion::new(99, 0, 0);
 
 /// List of all past database schema versions, in *reverse* order
 ///
@@ -29,6 +29,7 @@ static KNOWN_VERSIONS: Lazy<Vec<KnownVersion>> = Lazy::new(|| {
         // |  leaving the first copy as an example for the next person.
         // v
         // KnownVersion::new(next_int, "unique-dirname-with-the-sql-files"),
+        KnownVersion::new(99, "inventory-nvme-firmware"),
         KnownVersion::new(98, "oximeter-add-time-expunged"),
         KnownVersion::new(97, "lookup-region-snapshot-by-region-id"),
         KnownVersion::new(96, "inv-dataset"),