From fda48a9bf0117e725b27aaca743985ec85bdf79e Mon Sep 17 00:00:00 2001 From: David Gibson Date: Tue, 15 Dec 2020 13:15:58 +1100 Subject: [PATCH] agent/device: Use pci::Path type, name things consistently pcipath_to_sysfs takes a PCI path, with a particular format. A number of places implicitly need strings in that format, many of them repeat the description. To make things safer and briefer use the pci::Path type for the purpose more widely, and just describe the string formatting of it at the type definition. Then, update variable names and comments throughout to call things in this format "PCI path", rather than "PCI identifier", which is vague, or "PCI address" which is just plain wrong. Likewise we change names and comments which incorrectly refer to sysfs paths as a "PCI address". This changes the grpc proto definitions, but because it's just changing the name of a field without changing the field number, it shouldn't change the actual protocol. A loose forward port of https://github.com/kata-containers/agent/pull/855/commits/da4bc1d1849320baa18eebaef2853202bb3d5c3e Signed-off-by: David Gibson --- src/agent/src/device.rs | 20 +++++++++++--------- src/agent/src/mount.rs | 9 ++++++--- 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/src/agent/src/device.rs b/src/agent/src/device.rs index 7fd66f9109..a8c49b1231 100644 --- a/src/agent/src/device.rs +++ b/src/agent/src/device.rs @@ -50,8 +50,7 @@ pub fn online_device(path: &str) -> Result<()> { // pciPathToSysfs fetches the sysfs path for a PCI path, relative to // the syfs path for the PCI host bridge, based on the PCI path // provided. -fn pcipath_to_sysfs(pcipath: &str) -> Result { - let pcipath = pci::Path::from_str(pcipath)?; +fn pcipath_to_sysfs(pcipath: &pci::Path) -> Result { let mut bus = "0000:00".to_string(); let mut relpath = String::new(); let root_bus_sysfs = format!("{}{}", SYSFS_DIR, create_pci_root_bus_path()); @@ -145,7 +144,10 @@ pub async fn get_scsi_device_name( get_device_name(sandbox, &dev_sub_path).await } -pub async fn get_pci_device_name(sandbox: &Arc>, pcipath: &str) -> Result { +pub async fn get_pci_device_name( + sandbox: &Arc>, + pcipath: &pci::Path, +) -> Result { let sysfs_rel_path = pcipath_to_sysfs(pcipath)?; rescan_pci_bus()?; @@ -284,9 +286,7 @@ async fn virtiommio_blk_device_handler( update_spec_device_list(device, spec, devidx) } -// device.Id should be the PCI address in the format "bridgeAddr/deviceAddr". -// Here, bridgeAddr is the address at which the brige is attached on the root bus, -// while deviceAddr is the address at which the device is attached on the bridge. +// device.Id should be a PCI path string async fn virtio_blk_device_handler( device: &Device, spec: &mut Spec, @@ -295,10 +295,12 @@ async fn virtio_blk_device_handler( ) -> Result<()> { let mut dev = device.clone(); - // When "Id (PCIAddr)" is not set, we allow to use the predicted "VmPath" passed from kata-runtime - // Note this is a special code path for cloud-hypervisor when BDF information is not available + // When "Id (PCI path)" is not set, we allow to use the predicted + // "VmPath" passed from kata-runtime Note this is a special code + // path for cloud-hypervisor when BDF information is not available if device.id != "" { - dev.vm_path = get_pci_device_name(sandbox, &device.id).await?; + let pcipath = pci::Path::from_str(&device.id)?; + dev.vm_path = get_pci_device_name(sandbox, &pcipath).await?; } update_spec_device_list(&dev, spec, devidx) diff --git a/src/agent/src/mount.rs b/src/agent/src/mount.rs index 8073d96f04..188c2073a6 100644 --- a/src/agent/src/mount.rs +++ b/src/agent/src/mount.rs @@ -12,6 +12,7 @@ use std::os::unix::fs::PermissionsExt; use std::path::Path; use std::ptr::null; +use std::str::FromStr; use std::sync::Arc; use tokio::sync::Mutex; @@ -26,6 +27,7 @@ use crate::device::{ get_pci_device_name, get_pmem_device_name, get_scsi_device_name, online_device, }; use crate::linux_abi::*; +use crate::pci; use crate::protocols::agent::Storage; use crate::Sandbox; use anyhow::{anyhow, Context, Result}; @@ -310,8 +312,8 @@ async fn virtio_blk_storage_handler( sandbox: Arc>, ) -> Result { let mut storage = storage.clone(); - // If hot-plugged, get the device node path based on the PCI address else - // use the virt path provided in Storage Source + // If hot-plugged, get the device node path based on the PCI path + // otherwise use the virt path provided in Storage Source if storage.source.starts_with("/dev") { let metadata = fs::metadata(&storage.source) .context(format!("get metadata on file {:?}", &storage.source))?; @@ -321,7 +323,8 @@ async fn virtio_blk_storage_handler( return Err(anyhow!("Invalid device {}", &storage.source)); } } else { - let dev_path = get_pci_device_name(&sandbox, &storage.source).await?; + let pcipath = pci::Path::from_str(&storage.source)?; + let dev_path = get_pci_device_name(&sandbox, &pcipath).await?; storage.source = dev_path; }