From 5cc2890a10bf231294e7c817523550c48e87ae5d Mon Sep 17 00:00:00 2001 From: "alex.lyn" Date: Fri, 22 Dec 2023 10:34:41 +0800 Subject: [PATCH] runtime-rs: refactor and re-implement pci path. Do refactor and re-implement to make the pci path more "rusty". Fixes: #8665 Signed-off-by: alex.lyn --- .../crates/hypervisor/src/ch/inner_device.rs | 2 +- .../hypervisor/src/device/driver/vfio.rs | 4 +- .../crates/hypervisor/src/device/pci_path.rs | 126 +++++++++++------- .../crates/resource/src/manager_inner.rs | 2 +- 4 files changed, 85 insertions(+), 49 deletions(-) diff --git a/src/runtime-rs/crates/hypervisor/src/ch/inner_device.rs b/src/runtime-rs/crates/hypervisor/src/ch/inner_device.rs index 6b9cbff22b..e8ca2d3793 100644 --- a/src/runtime-rs/crates/hypervisor/src/ch/inner_device.rs +++ b/src/runtime-rs/crates/hypervisor/src/ch/inner_device.rs @@ -277,7 +277,7 @@ impl CloudHypervisorInner { )); } - PciPath::convert_from_string(toks[0]) + PciPath::try_from(toks[0]) } async fn handle_hvsock_device(&mut self, device: HybridVsockDevice) -> Result { diff --git a/src/runtime-rs/crates/hypervisor/src/device/driver/vfio.rs b/src/runtime-rs/crates/hypervisor/src/device/driver/vfio.rs index e488281841..3e4534be3a 100644 --- a/src/runtime-rs/crates/hypervisor/src/device/driver/vfio.rs +++ b/src/runtime-rs/crates/hypervisor/src/device/driver/vfio.rs @@ -68,7 +68,7 @@ pub fn generate_guest_pci_path(bdf: String) -> Result { hg_map.write().unwrap().insert(host_bdf, guest_bdf); Ok(PciPath { - slots: vec![PciSlot::convert_from_u32(slot.into()).context("pci slot convert failed.")?], + slots: vec![PciSlot::new(slot)], }) } @@ -501,7 +501,7 @@ impl Device for VfioDevice { self.device_options.push(format!( "0000:{}={}", hostdev.bus_slot_func.clone(), - pci_path.convert_to_string() + pci_path.to_string() )); } } diff --git a/src/runtime-rs/crates/hypervisor/src/device/pci_path.rs b/src/runtime-rs/crates/hypervisor/src/device/pci_path.rs index dea8c0a438..2d0be7647f 100644 --- a/src/runtime-rs/crates/hypervisor/src/device/pci_path.rs +++ b/src/runtime-rs/crates/hypervisor/src/device/pci_path.rs @@ -4,6 +4,8 @@ // SPDX-License-Identifier: Apache-2.0 // +use std::convert::TryFrom; + use anyhow::{anyhow, Context, Result}; // Tips: @@ -26,13 +28,30 @@ const MAX_PCI_SLOTS: u32 = (1 << PCI_SLOT_BITS) - 1; pub struct PciSlot(pub u8); impl PciSlot { - pub fn convert_from_string(s: &str) -> Result { + pub fn new(v: u8) -> PciSlot { + PciSlot(v) + } +} + +impl ToString for PciSlot { + fn to_string(&self) -> String { + format!("{:02x}", self.0) + } +} + +impl TryFrom<&str> for PciSlot { + type Error = anyhow::Error; + + fn try_from(s: &str) -> Result { if s.is_empty() || s.len() > 2 { return Err(anyhow!("string given is invalid.")); } let base = 16; - let n = u64::from_str_radix(s, base).context("convert string to number failed")?; + let n = u64::from_str_radix(s, base).context(format!( + "convert string to number with base {:?} failed.", + base + ))?; if n >> PCI_SLOT_BITS > 0 { return Err(anyhow!( "number {:?} exceeds MAX:{:?}, failed.", @@ -43,18 +62,18 @@ impl PciSlot { Ok(PciSlot(n as u8)) } +} - pub fn convert_from_u32(v: u32) -> Result { +impl TryFrom for PciSlot { + type Error = anyhow::Error; + + fn try_from(v: u32) -> Result { if v > MAX_PCI_SLOTS { return Err(anyhow!("value {:?} exceeds MAX: {:?}", v, MAX_PCI_SLOTS)); } Ok(PciSlot(v as u8)) } - - pub fn convert_to_string(&self) -> String { - format!("{:02x}", self.0) - } } // A PciPath describes where a PCI sits in a PCI hierarchy. @@ -78,34 +97,7 @@ pub struct PciPath { } impl PciPath { - // method to format the PciPath into a string - pub fn convert_to_string(&self) -> String { - self.slots - .iter() - .map(|pci_slot| format!("{:02x}", pci_slot.0)) - .collect::>() - .join("/") - } - - // method to parse a PciPath from a string - pub fn convert_from_string(path: &str) -> Result { - if path.is_empty() { - return Err(anyhow!("path given is empty.")); - } - - let mut pci_slots: Vec = Vec::new(); - let slots: Vec<&str> = path.split('/').collect(); - for slot in slots { - match PciSlot::convert_from_string(slot) { - Ok(s) => pci_slots.push(s), - Err(e) => return Err(anyhow!("slot is invalid with: {:?}", e)), - } - } - - Ok(PciPath { slots: pci_slots }) - } - - pub fn from_pci_slots(slots: Vec) -> Option { + pub fn new(slots: Vec) -> Option { if slots.is_empty() { return None; } @@ -124,6 +116,50 @@ impl PciPath { } } +impl ToString for PciPath { + // method to format the PciPath into a string + fn to_string(&self) -> String { + self.slots + .iter() + .map(|pci_slot| format!("{:02x}", pci_slot.0)) + .collect::>() + .join("/") + } +} + +// convert from u32 +impl TryFrom for PciPath { + type Error = anyhow::Error; + + fn try_from(slot: u32) -> Result { + Ok(PciPath { + slots: vec![PciSlot::try_from(slot).context("pci slot convert failed.")?], + }) + } +} + +impl TryFrom<&str> for PciPath { + type Error = anyhow::Error; + + // method to parse a PciPath from a string + fn try_from(path: &str) -> Result { + if path.is_empty() { + return Err(anyhow!("path given is empty.")); + } + + let mut pci_slots: Vec = Vec::new(); + let slots: Vec<&str> = path.split('/').collect(); + for slot in slots { + match PciSlot::try_from(slot) { + Ok(s) => pci_slots.push(s), + Err(e) => return Err(anyhow!("slot is invalid with: {:?}", e)), + } + } + + Ok(PciPath { slots: pci_slots }) + } +} + #[cfg(test)] mod tests { use super::*; @@ -131,46 +167,46 @@ mod tests { #[test] fn test_pci_slot() { // min - let pci_slot_01 = PciSlot::convert_from_string("00"); + let pci_slot_01 = PciSlot::try_from("00"); assert!(pci_slot_01.is_ok()); // max - let pci_slot_02 = PciSlot::convert_from_string("1f"); + let pci_slot_02 = PciSlot::try_from("1f"); assert!(pci_slot_02.is_ok()); // exceed - let pci_slot_03 = PciSlot::convert_from_string("20"); + let pci_slot_03 = PciSlot::try_from("20"); assert!(pci_slot_03.is_err()); // valid number - let pci_slot_04 = PciSlot::convert_from_u32(1_u32); + let pci_slot_04 = PciSlot::try_from(1_u32); assert!(pci_slot_04.is_ok()); assert_eq!(pci_slot_04.as_ref().unwrap().0, 1_u8); - let pci_slot_str = pci_slot_04.as_ref().unwrap().convert_to_string(); + let pci_slot_str = pci_slot_04.as_ref().unwrap().to_string(); assert_eq!(pci_slot_str, format!("{:02x}", pci_slot_04.unwrap().0)); // max number - let pci_slot_05 = PciSlot::convert_from_u32(31_u32); + let pci_slot_05 = PciSlot::try_from(31_u32); assert!(pci_slot_05.is_ok()); assert_eq!(pci_slot_05.unwrap().0, 31_u8); // exceed and error - let pci_slot_06 = PciSlot::convert_from_u32(32_u32); + let pci_slot_06 = PciSlot::try_from(32_u32); assert!(pci_slot_06.is_err()); } #[test] fn test_pci_patch() { - let pci_path_0 = PciPath::convert_from_string("01/0a/05"); + let pci_path_0 = PciPath::try_from("01/0a/05"); assert!(pci_path_0.is_ok()); let pci_path_unwrap = pci_path_0.unwrap(); assert_eq!(pci_path_unwrap.slots[0].0, 1); assert_eq!(pci_path_unwrap.slots[1].0, 10); assert_eq!(pci_path_unwrap.slots[2].0, 5); - let pci_path_01 = PciPath::from_pci_slots(vec![PciSlot(1), PciSlot(10), PciSlot(5)]); + let pci_path_01 = PciPath::new(vec![PciSlot(1), PciSlot(10), PciSlot(5)]); assert!(pci_path_01.is_some()); let pci_path = pci_path_01.unwrap(); - let pci_path_02 = pci_path.convert_to_string(); + let pci_path_02 = pci_path.to_string(); assert_eq!(pci_path_02, "01/0a/05".to_string()); let dev_slot = pci_path.get_device_slot(); diff --git a/src/runtime-rs/crates/resource/src/manager_inner.rs b/src/runtime-rs/crates/resource/src/manager_inner.rs index f266b7966a..2b70fe90c0 100644 --- a/src/runtime-rs/crates/resource/src/manager_inner.rs +++ b/src/runtime-rs/crates/resource/src/manager_inner.rs @@ -313,7 +313,7 @@ impl ResourceManagerInner { // The following would work for drivers virtio-blk-pci and mmio. // Once scsi support is added, need to handle scsi identifiers. let id = if let Some(pci_path) = device.config.pci_path { - pci_path.convert_to_string() + pci_path.to_string() } else { device.config.virt_path.clone() };