From 8c2f9e6949823381b2e62da2920f0a33ad7c2693 Mon Sep 17 00:00:00 2001 From: David Gibson Date: Fri, 30 Oct 2020 13:51:36 +1100 Subject: [PATCH 01/13] gitignore: Ignore *~ editor backup files We ignore some other formats for backup files, but add this one, used by emacs. Signed-off-by: David Gibson --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 7e78e3fb89..529bab04a7 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,5 @@ **/*.bk +**/*~ **/*.orig **/*.rej **/target From b22259ad9b33cf7a61d70102d6be6b7c0d1b6826 Mon Sep 17 00:00:00 2001 From: David Gibson Date: Fri, 30 Oct 2020 15:13:34 +1100 Subject: [PATCH 02/13] agent: PCI slot type Add a Rust type for representing a PCI slot on a single bus. This is essentially just an integer from 0..31 (inclusive), but includes the code for converting from integers with appropriate validation and formatting back to a string. Signed-off-by: David Gibson --- src/agent/src/main.rs | 1 + src/agent/src/pci.rs | 88 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 89 insertions(+) create mode 100644 src/agent/src/pci.rs diff --git a/src/agent/src/main.rs b/src/agent/src/main.rs index 84ca33821f..9ac0dd6416 100644 --- a/src/agent/src/main.rs +++ b/src/agent/src/main.rs @@ -49,6 +49,7 @@ mod mount; mod namespace; mod netlink; mod network; +mod pci; pub mod random; mod sandbox; #[cfg(test)] diff --git a/src/agent/src/pci.rs b/src/agent/src/pci.rs new file mode 100644 index 0000000000..7f85e53949 --- /dev/null +++ b/src/agent/src/pci.rs @@ -0,0 +1,88 @@ +// Copyright Red Hat. +// +// SPDX-License-Identifier: Apache-2.0 +// +use std::convert::TryInto; +use std::fmt; +use std::str::FromStr; + +use anyhow::anyhow; + +// The PCI spec reserves 5 bits for slot number (a.k.a. device +// number), giving slots 0..31 +const SLOT_BITS: u8 = 5; +const SLOT_MAX: u8 = (1 << SLOT_BITS) - 1; + +// Represents a PCI function's slot number (a.k.a. device number), +// giving its location on a single bus +#[derive(Copy, Clone, Debug, PartialEq, Eq)] +pub struct Slot(u8); + +impl Slot { + pub fn new + fmt::Display + Copy>(v: T) -> anyhow::Result { + if let Ok(v8) = v.try_into() { + if v8 <= SLOT_MAX { + return Ok(Slot(v8)); + } + } + Err(anyhow!( + "PCI slot {} should be in range [0..{:#x}]", + v, + SLOT_MAX + )) + } +} + +impl FromStr for Slot { + type Err = anyhow::Error; + + fn from_str(s: &str) -> anyhow::Result { + let v = isize::from_str_radix(s, 16)?; + Slot::new(v) + } +} + +impl fmt::Display for Slot { + fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> { + write!(f, "{:02x}", self.0) + } +} + +#[cfg(test)] +mod tests { + use crate::pci::Slot; + use std::str::FromStr; + + #[test] + fn test_slot() { + // Valid slots + let slot = Slot::new(0x00).unwrap(); + assert_eq!(format!("{}", slot), "00"); + + let slot = Slot::from_str("00").unwrap(); + assert_eq!(format!("{}", slot), "00"); + + let slot = Slot::new(31).unwrap(); + let slot2 = Slot::from_str("1f").unwrap(); + assert_eq!(slot, slot2); + + // Bad slots + let slot = Slot::new(-1); + assert!(slot.is_err()); + + let slot = Slot::new(32); + assert!(slot.is_err()); + + let slot = Slot::from_str("20"); + assert!(slot.is_err()); + + let slot = Slot::from_str("xy"); + assert!(slot.is_err()); + + let slot = Slot::from_str("00/"); + assert!(slot.is_err()); + + let slot = Slot::from_str(""); + assert!(slot.is_err()); + } +} From 7464d055a7f1c1d50b4eb4fe25340656c3b05943 Mon Sep 17 00:00:00 2001 From: David Gibson Date: Fri, 30 Oct 2020 16:22:46 +1100 Subject: [PATCH 03/13] agent: PCI path type Introduce a Rust type to represent a "PCI path" - that is a way of locating a PCI device from a given root by listing the slots of all the bridges leading to it and finally the slot of the device itself. It's implemented as a vector of the previously added pci::Slot type, and includes the necessary validation and conversions to/from strings. Signed-off-by: David Gibson --- src/agent/src/pci.rs | 82 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 81 insertions(+), 1 deletion(-) diff --git a/src/agent/src/pci.rs b/src/agent/src/pci.rs index 7f85e53949..176d90c678 100644 --- a/src/agent/src/pci.rs +++ b/src/agent/src/pci.rs @@ -4,6 +4,7 @@ // use std::convert::TryInto; use std::fmt; +use std::ops::Deref; use std::str::FromStr; use anyhow::anyhow; @@ -48,9 +49,50 @@ impl fmt::Display for Slot { } } +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct Path(Vec); + +impl Path { + pub fn new(slots: Vec) -> anyhow::Result { + if slots.is_empty() { + return Err(anyhow!("PCI path must have at least one element")); + } + Ok(Path(slots)) + } +} + +// Let Path be treated as a slice of Slots +impl Deref for Path { + type Target = [Slot]; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +impl fmt::Display for Path { + fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> { + let sslots: Vec = self + .0 + .iter() + .map(std::string::ToString::to_string) + .collect(); + write!(f, "{}", sslots.join("/")) + } +} + +impl FromStr for Path { + type Err = anyhow::Error; + + fn from_str(s: &str) -> anyhow::Result { + let rslots: anyhow::Result> = s.split('/').map(Slot::from_str).collect(); + Path::new(rslots?) + } +} + #[cfg(test)] mod tests { - use crate::pci::Slot; + use crate::pci::{Path, Slot}; use std::str::FromStr; #[test] @@ -85,4 +127,42 @@ mod tests { let slot = Slot::from_str(""); assert!(slot.is_err()); } + + #[test] + fn test_path() { + let slot3 = Slot::new(0x03).unwrap(); + let slot4 = Slot::new(0x04).unwrap(); + let slot5 = Slot::new(0x05).unwrap(); + + // Valid paths + let pcipath = Path::new(vec![slot3]).unwrap(); + assert_eq!(format!("{}", pcipath), "03"); + let pcipath2 = Path::from_str("03").unwrap(); + assert_eq!(pcipath, pcipath2); + assert_eq!(pcipath.len(), 1); + assert_eq!(pcipath[0], slot3); + + let pcipath = Path::new(vec![slot3, slot4]).unwrap(); + assert_eq!(format!("{}", pcipath), "03/04"); + let pcipath2 = Path::from_str("03/04").unwrap(); + assert_eq!(pcipath, pcipath2); + assert_eq!(pcipath.len(), 2); + assert_eq!(pcipath[0], slot3); + assert_eq!(pcipath[1], slot4); + + let pcipath = Path::new(vec![slot3, slot4, slot5]).unwrap(); + assert_eq!(format!("{}", pcipath), "03/04/05"); + let pcipath2 = Path::from_str("03/04/05").unwrap(); + assert_eq!(pcipath, pcipath2); + assert_eq!(pcipath.len(), 3); + assert_eq!(pcipath[0], slot3); + assert_eq!(pcipath[1], slot4); + assert_eq!(pcipath[2], slot5); + + // Bad paths + assert!(Path::new(vec!()).is_err()); + assert!(Path::from_str("20").is_err()); + assert!(Path::from_str("//").is_err()); + assert!(Path::from_str("xyz").is_err()); + } } From 8e5fd8ee84ea4594e2f32c33b8180fd66059ba3e Mon Sep 17 00:00:00 2001 From: David Gibson Date: Thu, 8 Oct 2020 13:16:05 +1100 Subject: [PATCH 04/13] runtime: Introduce PciSlot and PciPath types This is a dedicated data type for representing PCI paths, that is, PCI devices described by the slot numbers of the bridges we need to reach them. There are a number of places that uses strings with that structure for things. The plan is to use this data type to consolidate their handling. These are essentially Go equivalents of the pci::Slot and pci::Path types introduced in the Rust agent. Forward port of https://github.com/kata-containers/runtime/pull/3003/commits/185b3ab0446e04350a6685bd9f8f0ad0dc7d7b09 Signed-off-by: David Gibson --- .../virtcontainers/pkg/types/pcipath.go | 100 ++++++++++++++++ .../virtcontainers/pkg/types/pcipath_test.go | 110 ++++++++++++++++++ 2 files changed, 210 insertions(+) create mode 100644 src/runtime/virtcontainers/pkg/types/pcipath.go create mode 100644 src/runtime/virtcontainers/pkg/types/pcipath_test.go diff --git a/src/runtime/virtcontainers/pkg/types/pcipath.go b/src/runtime/virtcontainers/pkg/types/pcipath.go new file mode 100644 index 0000000000..786ab67a3e --- /dev/null +++ b/src/runtime/virtcontainers/pkg/types/pcipath.go @@ -0,0 +1,100 @@ +// Copyright Red Hat. +// +// SPDX-License-Identifier: Apache-2.0 +// + +package types + +import ( + "fmt" + "strconv" + "strings" +) + +const ( + // The PCI spec reserves 5 bits for slot number (a.k.a. device + // number), giving slots 0..31 + pciSlotBits = 5 + maxPciSlot = (1 << pciSlotBits) - 1 +) + +// A PciSlot describes where a PCI device sits on a single bus +// +// This encapsulates the PCI slot number a.k.a device number, which is +// limited to a 5 bit value [0x00..0x1f] by the PCI specification +// +// XXX In order to support multifunction device's we'll need to extend +// this to include the PCI 3-bit function number as well. +type PciSlot struct{ slot uint8 } + +func PciSlotFromString(s string) (PciSlot, error) { + v, err := strconv.ParseUint(s, 16, pciSlotBits) + if err != nil { + return PciSlot{}, err + } + // The 5 bit width passed to ParseUint ensures the value is <= + // maxPciSlot + return PciSlot{slot: uint8(v)}, nil +} + +func PciSlotFromInt(v int) (PciSlot, error) { + if v < 0 || v > maxPciSlot { + return PciSlot{}, fmt.Errorf("PCI slot 0x%x should be in range [0..0x%x]", v, maxPciSlot) + } + return PciSlot{slot: uint8(v)}, nil +} + +func (slot PciSlot) String() string { + return fmt.Sprintf("%02x", slot.slot) +} + +// A PciPath describes where a PCI sits in a PCI hierarchy. +// +// Consists of a list of PCI slots, giving the slot of each bridge +// that must be traversed from the PCI root to reach the device, +// followed by the slot of the device itself +// +// When formatted into a string is written as "xx/.../yy/zz" Here, zz +// is the slot of the device on its PCI bridge, yy is the slot of the +// bridge on its parent bridge and so forth until xx is the slot of +// the "most upstream" bridge on the root bus. If a device is +// connected directly to the root bus, its PciPath is just "zz" +type PciPath struct { + slots []PciSlot +} + +func (p PciPath) String() string { + tokens := make([]string, len(p.slots)) + for i, slot := range p.slots { + tokens[i] = slot.String() + } + return strings.Join(tokens, "/") +} + +func (p PciPath) IsNil() bool { + return p.slots == nil +} + +func PciPathFromString(s string) (PciPath, error) { + if s == "" { + return PciPath{}, nil + } + + tokens := strings.Split(s, "/") + slots := make([]PciSlot, len(tokens)) + for i, t := range tokens { + var err error + slots[i], err = PciSlotFromString(t) + if err != nil { + return PciPath{}, err + } + } + return PciPath{slots: slots}, nil +} + +func PciPathFromSlots(slots ...PciSlot) (PciPath, error) { + if len(slots) == 0 { + return PciPath{}, fmt.Errorf("PCI path needs at least one component") + } + return PciPath{slots: slots}, nil +} diff --git a/src/runtime/virtcontainers/pkg/types/pcipath_test.go b/src/runtime/virtcontainers/pkg/types/pcipath_test.go new file mode 100644 index 0000000000..9f73036572 --- /dev/null +++ b/src/runtime/virtcontainers/pkg/types/pcipath_test.go @@ -0,0 +1,110 @@ +// Copyright Red Hat. +// +// SPDX-License-Identifier: Apache-2.0 +// + +package types + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestPciSlot(t *testing.T) { + assert := assert.New(t) + + // Valid slots + slot, err := PciSlotFromInt(0x00) + assert.NoError(err) + assert.Equal(slot, PciSlot{}) + assert.Equal(slot.String(), "00") + + slot, err = PciSlotFromString("00") + assert.NoError(err) + assert.Equal(slot, PciSlot{}) + + slot, err = PciSlotFromInt(31) + assert.NoError(err) + slot2, err := PciSlotFromString("1f") + assert.NoError(err) + assert.Equal(slot, slot2) + + // Bad slots + _, err = PciSlotFromInt(-1) + assert.Error(err) + + _, err = PciSlotFromInt(32) + assert.Error(err) + + _, err = PciSlotFromString("20") + assert.Error(err) + + _, err = PciSlotFromString("xy") + assert.Error(err) + + _, err = PciSlotFromString("00/") + assert.Error(err) + + _, err = PciSlotFromString("") + assert.Error(err) +} + +func TestPciPath(t *testing.T) { + assert := assert.New(t) + + slot3, err := PciSlotFromInt(0x03) + assert.NoError(err) + slot4, err := PciSlotFromInt(0x04) + assert.NoError(err) + slot5, err := PciSlotFromInt(0x05) + assert.NoError(err) + + // Empty/nil paths + pcipath := PciPath{} + assert.True(pcipath.IsNil()) + + pcipath, err = PciPathFromString("") + assert.NoError(err) + assert.True(pcipath.IsNil()) + assert.Equal(pcipath, PciPath{}) + + // Valid paths + pcipath, err = PciPathFromSlots(slot3) + assert.NoError(err) + assert.False(pcipath.IsNil()) + assert.Equal(pcipath.String(), "03") + pcipath2, err := PciPathFromString("03") + assert.NoError(err) + assert.Equal(pcipath, pcipath2) + + pcipath, err = PciPathFromSlots(slot3, slot4) + assert.NoError(err) + assert.False(pcipath.IsNil()) + assert.Equal(pcipath.String(), "03/04") + pcipath2, err = PciPathFromString("03/04") + assert.NoError(err) + assert.Equal(pcipath, pcipath2) + + pcipath, err = PciPathFromSlots(slot3, slot4, slot5) + assert.NoError(err) + assert.False(pcipath.IsNil()) + assert.Equal(pcipath.String(), "03/04/05") + pcipath2, err = PciPathFromString("03/04/05") + assert.NoError(err) + assert.Equal(pcipath, pcipath2) + + // Bad paths + _, err = PciPathFromSlots() + assert.Error(err) + + _, err = PciPathFromString("20") + assert.Error(err) + + _, err = PciPathFromString("//") + assert.Error(err) + + _, err = PciPathFromString("xyz") + assert.Error(err) + +} From 7e92831c7a6853c73b6594412fea64811d456f88 Mon Sep 17 00:00:00 2001 From: David Gibson Date: Tue, 15 Dec 2020 20:46:54 +1100 Subject: [PATCH 05/13] protocols: Update PCI path names / terminology in agent protocol def Now that we have types to represent PCI paths on both the agent and runtime sides, we can update the protocol definitionto use clearer terminology. Note that this doesn't actually change the agent protocol, because it just renames a field without changing its field ID or type. While we're there fix a trivial rustfmt error in src/agent/protocols/build.rs Signed-off-by: David Gibson --- src/agent/protocols/protos/types.proto | 6 +- src/runtime/virtcontainers/network.go | 2 +- .../pkg/agent/protocols/types.pb.go | 84 +++++++++---------- src/runtime/virtcontainers/sandbox.go | 2 +- 4 files changed, 45 insertions(+), 49 deletions(-) diff --git a/src/agent/protocols/protos/types.proto b/src/agent/protocols/protos/types.proto index a639d8e1a9..8ed1aa38a5 100644 --- a/src/agent/protocols/protos/types.proto +++ b/src/agent/protocols/protos/types.proto @@ -29,10 +29,8 @@ message Interface { uint64 mtu = 4; string hwAddr = 5; - // pciAddr is the PCI address in the format "bridgeAddr/deviceAddr". - // Here, bridgeAddr is the address at which the bridge is attached on the root bus, - // while deviceAddr is the address at which the network device is attached on the bridge. - string pciAddr = 6; + // PCI path for the device (see the pci::Path (Rust) or types.PciPath (Go) type for format details) + string pciPath = 6; // Type defines the type of interface described by this structure. // The expected values are the one that are defined by the netlink diff --git a/src/runtime/virtcontainers/network.go b/src/runtime/virtcontainers/network.go index 0ec7d10b1b..f848cdb92f 100644 --- a/src/runtime/virtcontainers/network.go +++ b/src/runtime/virtcontainers/network.go @@ -984,7 +984,7 @@ func generateVCNetworkStructures(networkNS NetworkNamespace) ([]*pbTypes.Interfa Mtu: uint64(endpoint.Properties().Iface.MTU), RawFlags: noarp, HwAddr: endpoint.HardwareAddr(), - PciAddr: endpoint.PciAddr(), + PciPath: endpoint.PciAddr(), } ifaces = append(ifaces, &ifc) diff --git a/src/runtime/virtcontainers/pkg/agent/protocols/types.pb.go b/src/runtime/virtcontainers/pkg/agent/protocols/types.pb.go index eab7caa591..497cb55a40 100644 --- a/src/runtime/virtcontainers/pkg/agent/protocols/types.pb.go +++ b/src/runtime/virtcontainers/pkg/agent/protocols/types.pb.go @@ -96,10 +96,8 @@ type Interface struct { IPAddresses []*IPAddress `protobuf:"bytes,3,rep,name=IPAddresses,proto3" json:"IPAddresses,omitempty"` Mtu uint64 `protobuf:"varint,4,opt,name=mtu,proto3" json:"mtu,omitempty"` HwAddr string `protobuf:"bytes,5,opt,name=hwAddr,proto3" json:"hwAddr,omitempty"` - // pciAddr is the PCI address in the format "bridgeAddr/deviceAddr". - // Here, bridgeAddr is the address at which the bridge is attached on the root bus, - // while deviceAddr is the address at which the network device is attached on the bridge. - PciAddr string `protobuf:"bytes,6,opt,name=pciAddr,proto3" json:"pciAddr,omitempty"` + // PCI path for the device (see the pci::Path (Rust) or types.PciPath (Go) type for format details) + PciPath string `protobuf:"bytes,6,opt,name=pciPath,proto3" json:"pciPath,omitempty"` // Type defines the type of interface described by this structure. // The expected values are the one that are defined by the netlink // library, regarding each type of link. Here is a non exhaustive @@ -242,37 +240,37 @@ func init() { } var fileDescriptor_b0c68e5b9fabcd81 = []byte{ - // 470 bytes of a gzipped FileDescriptorProto - 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x94, 0x93, 0x31, 0x8f, 0xda, 0x30, - 0x14, 0xc7, 0x31, 0x21, 0x39, 0x62, 0x74, 0x2d, 0xb2, 0xaa, 0x93, 0xd5, 0x4a, 0x51, 0xc4, 0xd2, - 0xa8, 0x52, 0x89, 0x44, 0xab, 0xee, 0xd7, 0xe1, 0x24, 0x96, 0x8a, 0x7a, 0x6b, 0x97, 0xca, 0x18, - 0x13, 0x22, 0x92, 0x38, 0xb2, 0x1d, 0x10, 0xea, 0xd2, 0x2f, 0xd2, 0xef, 0x73, 0x63, 0xc7, 0x8e, - 0x77, 0x7c, 0x92, 0xca, 0x76, 0x40, 0x29, 0xed, 0x72, 0x13, 0xff, 0xdf, 0x7b, 0x36, 0xef, 0xff, - 0xfe, 0x18, 0xf8, 0x39, 0xcb, 0xf5, 0xa6, 0x59, 0x4e, 0x99, 0x28, 0xd3, 0x2d, 0xd5, 0xf4, 0x2d, - 0x13, 0x95, 0xa6, 0x79, 0xc5, 0xa5, 0xfa, 0x87, 0x95, 0x64, 0x29, 0xcd, 0x78, 0xa5, 0xd3, 0x5a, - 0x0a, 0x2d, 0x98, 0x28, 0x94, 0x53, 0x2a, 0xd5, 0x87, 0x9a, 0xab, 0xa9, 0x05, 0xe4, 0x5b, 0x98, - 0x2c, 0x61, 0x38, 0x5f, 0xdc, 0xae, 0x56, 0x92, 0x2b, 0x85, 0x5e, 0xc3, 0x60, 0x4d, 0xcb, 0xbc, - 0x38, 0x60, 0x10, 0x83, 0xe4, 0xd9, 0xec, 0xf9, 0xd4, 0xdd, 0x98, 0x2f, 0xee, 0x6c, 0x99, 0xb4, - 0x6d, 0x84, 0xe1, 0x15, 0x75, 0x77, 0x70, 0x3f, 0x06, 0x49, 0x48, 0x4e, 0x88, 0x10, 0x1c, 0x94, - 0x54, 0x6d, 0xb1, 0x67, 0xcb, 0x56, 0x4f, 0x1e, 0x00, 0x0c, 0xe7, 0x95, 0xe6, 0x72, 0x4d, 0x19, - 0x47, 0x37, 0x30, 0x58, 0xf1, 0x5d, 0xce, 0xb8, 0x1d, 0x12, 0x92, 0x96, 0xcc, 0xcd, 0x8a, 0x96, - 0xbc, 0xfd, 0x42, 0xab, 0xd1, 0x0c, 0x8e, 0xce, 0xee, 0xb8, 0xc2, 0x5e, 0xec, 0x25, 0xa3, 0xd9, - 0xf8, 0xec, 0xaa, 0xed, 0x90, 0xee, 0x21, 0x34, 0x86, 0x5e, 0xa9, 0x1b, 0x3c, 0x88, 0x41, 0x32, - 0x20, 0x46, 0x9a, 0x89, 0x9b, 0xbd, 0x39, 0x80, 0x7d, 0x37, 0xd1, 0x91, 0xd9, 0xa2, 0x66, 0xb9, - 0x6d, 0x04, 0x6e, 0x8b, 0x16, 0x8d, 0x17, 0x33, 0x03, 0x5f, 0x39, 0x2f, 0x46, 0xa3, 0x57, 0x30, - 0x94, 0x74, 0xff, 0x6d, 0x5d, 0xd0, 0x4c, 0xe1, 0x61, 0x0c, 0x92, 0x6b, 0x32, 0x94, 0x74, 0x7f, - 0x67, 0x78, 0xf2, 0x1d, 0xfa, 0x44, 0x34, 0xda, 0x6e, 0xb1, 0xe2, 0x4a, 0xb7, 0xbb, 0x59, 0x6d, - 0xe6, 0x64, 0x54, 0xf3, 0x3d, 0x3d, 0x9c, 0xd2, 0x6a, 0xb1, 0x93, 0x85, 0xf7, 0x57, 0x16, 0x37, - 0x30, 0x50, 0xa2, 0x91, 0x8c, 0xdb, 0x35, 0x42, 0xd2, 0x12, 0x7a, 0x01, 0x7d, 0xc5, 0x44, 0xcd, - 0xed, 0x22, 0xd7, 0xc4, 0xc1, 0xe4, 0x27, 0x80, 0xa3, 0x5b, 0xb2, 0xf8, 0xc4, 0xf3, 0x6c, 0xb3, - 0x14, 0xd2, 0xa4, 0xa6, 0xc5, 0x39, 0x12, 0x6b, 0xe5, 0xbf, 0xa9, 0x75, 0x0e, 0x75, 0x9c, 0xf4, - 0x2f, 0x9d, 0x14, 0x85, 0xf9, 0x71, 0x4f, 0x0e, 0x1d, 0x59, 0x27, 0x9a, 0x6a, 0x67, 0xd0, 0x27, - 0x0e, 0x4c, 0xd5, 0xe5, 0xe3, 0xbb, 0xaa, 0x85, 0x37, 0x2f, 0xe1, 0xf0, 0xf4, 0x82, 0x50, 0x00, - 0xfb, 0xbb, 0xf7, 0xe3, 0x9e, 0xfd, 0xfc, 0x30, 0x06, 0x1f, 0xd5, 0xfd, 0x63, 0xd4, 0xfb, 0xfd, - 0x18, 0xf5, 0x7e, 0x1c, 0x23, 0x70, 0x7f, 0x8c, 0xc0, 0xaf, 0x63, 0x04, 0x1e, 0x8e, 0x11, 0xf8, - 0xfa, 0xe5, 0x89, 0x6f, 0x5e, 0x36, 0x95, 0xce, 0x4b, 0x9e, 0xee, 0x72, 0xa9, 0x3b, 0xad, 0x7a, - 0x9b, 0x5d, 0xfe, 0x1d, 0x96, 0x81, 0x95, 0xef, 0xfe, 0x04, 0x00, 0x00, 0xff, 0xff, 0x11, 0x62, - 0xe9, 0x09, 0x57, 0x03, 0x00, 0x00, + // 472 bytes of a gzipped FileDescriptorProto + 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x94, 0x93, 0x31, 0x8f, 0xd3, 0x30, + 0x18, 0x86, 0xeb, 0xa6, 0xc9, 0x35, 0xae, 0x0e, 0x2a, 0x0b, 0x9d, 0x2c, 0x90, 0xa2, 0xa8, 0x0b, + 0x11, 0x12, 0x8d, 0x54, 0x10, 0xfb, 0x31, 0x9c, 0xd4, 0x05, 0x15, 0x6f, 0xb0, 0x20, 0xd7, 0x75, + 0xd3, 0xa8, 0x49, 0x1c, 0xd9, 0x4e, 0xab, 0x8a, 0x85, 0x3f, 0xc2, 0xff, 0xb9, 0x91, 0x91, 0xf1, + 0xae, 0xbf, 0x04, 0xd9, 0x4e, 0xab, 0x50, 0x58, 0x6e, 0xca, 0xfb, 0x7c, 0xb6, 0xf3, 0xbd, 0xdf, + 0x1b, 0x07, 0x7e, 0xce, 0x72, 0xbd, 0x69, 0x96, 0x53, 0x26, 0xca, 0x74, 0x4b, 0x35, 0x7d, 0xcb, + 0x44, 0xa5, 0x69, 0x5e, 0x71, 0xa9, 0xfe, 0x61, 0x25, 0x59, 0x4a, 0x33, 0x5e, 0xe9, 0xb4, 0x96, + 0x42, 0x0b, 0x26, 0x0a, 0xe5, 0x94, 0x4a, 0xf5, 0xa1, 0xe6, 0x6a, 0x6a, 0x01, 0xf9, 0x16, 0x26, + 0x4b, 0x18, 0xce, 0x17, 0xb7, 0xab, 0x95, 0xe4, 0x4a, 0xa1, 0xd7, 0x30, 0x58, 0xd3, 0x32, 0x2f, + 0x0e, 0x18, 0xc4, 0x20, 0x79, 0x36, 0x7b, 0x3e, 0x75, 0x27, 0xe6, 0x8b, 0x3b, 0x5b, 0x26, 0xed, + 0x32, 0xc2, 0xf0, 0x8a, 0xba, 0x33, 0xb8, 0x1f, 0x83, 0x24, 0x24, 0x27, 0x44, 0x08, 0x0e, 0x4a, + 0xaa, 0xb6, 0xd8, 0xb3, 0x65, 0xab, 0x27, 0x0f, 0x00, 0x86, 0xf3, 0x4a, 0x73, 0xb9, 0xa6, 0x8c, + 0xa3, 0x1b, 0x18, 0xac, 0xf8, 0x2e, 0x67, 0xdc, 0x36, 0x09, 0x49, 0x4b, 0xe6, 0x64, 0x45, 0x4b, + 0xde, 0xbe, 0xd0, 0x6a, 0x34, 0x83, 0xa3, 0xb3, 0x3b, 0xae, 0xb0, 0x17, 0x7b, 0xc9, 0x68, 0x36, + 0x3e, 0xbb, 0x6a, 0x57, 0x48, 0x77, 0x13, 0x1a, 0x43, 0xaf, 0xd4, 0x0d, 0x1e, 0xc4, 0x20, 0x19, + 0x10, 0x23, 0x4d, 0xc7, 0xcd, 0xde, 0x6c, 0xc0, 0xbe, 0xeb, 0xe8, 0xc8, 0x4c, 0x51, 0xb3, 0x7c, + 0x41, 0xf5, 0x06, 0x07, 0x6e, 0x8a, 0x16, 0x8d, 0x17, 0xd3, 0x03, 0x5f, 0x39, 0x2f, 0x46, 0xa3, + 0x57, 0x30, 0x94, 0x74, 0xff, 0x6d, 0x5d, 0xd0, 0x4c, 0xe1, 0x61, 0x0c, 0x92, 0x6b, 0x32, 0x94, + 0x74, 0x7f, 0x67, 0x78, 0xf2, 0x1d, 0xfa, 0x44, 0x34, 0xda, 0x4e, 0xb1, 0xe2, 0x4a, 0xb7, 0xb3, + 0x59, 0x6d, 0xfa, 0x64, 0x54, 0xf3, 0x3d, 0x3d, 0x9c, 0xd2, 0x6a, 0xb1, 0x93, 0x85, 0xf7, 0x57, + 0x16, 0x37, 0x30, 0x50, 0xa2, 0x91, 0x8c, 0xdb, 0x31, 0x42, 0xd2, 0x12, 0x7a, 0x01, 0x7d, 0xc5, + 0x44, 0xcd, 0xed, 0x20, 0xd7, 0xc4, 0xc1, 0xe4, 0x27, 0x80, 0xa3, 0x5b, 0xb2, 0xf8, 0xc4, 0xf3, + 0x6c, 0xb3, 0x14, 0xd2, 0xa4, 0xa6, 0xc5, 0x39, 0x12, 0x6b, 0xe5, 0xbf, 0xa9, 0x75, 0x36, 0x75, + 0x9c, 0xf4, 0x2f, 0x9d, 0x14, 0x85, 0xf9, 0xb8, 0x27, 0x87, 0x8e, 0xac, 0x13, 0x4d, 0xb5, 0x33, + 0xe8, 0x13, 0x07, 0xa6, 0xea, 0xf2, 0xf1, 0x5d, 0xd5, 0xc2, 0x9b, 0x97, 0x70, 0x78, 0xba, 0x41, + 0x28, 0x80, 0xfd, 0xdd, 0xfb, 0x71, 0xcf, 0x3e, 0x3f, 0x8c, 0xc1, 0x47, 0x75, 0xff, 0x18, 0xf5, + 0x7e, 0x3f, 0x46, 0xbd, 0x1f, 0xc7, 0x08, 0xdc, 0x1f, 0x23, 0xf0, 0xeb, 0x18, 0x81, 0x87, 0x63, + 0x04, 0xbe, 0x7e, 0x79, 0xe2, 0x9d, 0x97, 0x4d, 0xa5, 0xf3, 0x92, 0xa7, 0xbb, 0x5c, 0xea, 0xce, + 0x52, 0xbd, 0xcd, 0x2e, 0x7f, 0x87, 0x65, 0x60, 0xe5, 0xbb, 0x3f, 0x01, 0x00, 0x00, 0xff, 0xff, + 0xed, 0x3e, 0x9a, 0x58, 0x57, 0x03, 0x00, 0x00, } func (m *IPAddress) Marshal() (dAtA []byte, err error) { @@ -357,10 +355,10 @@ func (m *Interface) MarshalToSizedBuffer(dAtA []byte) (int, error) { i-- dAtA[i] = 0x3a } - if len(m.PciAddr) > 0 { - i -= len(m.PciAddr) - copy(dAtA[i:], m.PciAddr) - i = encodeVarintTypes(dAtA, i, uint64(len(m.PciAddr))) + if len(m.PciPath) > 0 { + i -= len(m.PciPath) + copy(dAtA[i:], m.PciPath) + i = encodeVarintTypes(dAtA, i, uint64(len(m.PciPath))) i-- dAtA[i] = 0x32 } @@ -591,7 +589,7 @@ func (m *Interface) Size() (n int) { if l > 0 { n += 1 + l + sovTypes(uint64(l)) } - l = len(m.PciAddr) + l = len(m.PciPath) if l > 0 { n += 1 + l + sovTypes(uint64(l)) } @@ -703,7 +701,7 @@ func (this *Interface) String() string { `IPAddresses:` + repeatedStringForIPAddresses + `,`, `Mtu:` + fmt.Sprintf("%v", this.Mtu) + `,`, `HwAddr:` + fmt.Sprintf("%v", this.HwAddr) + `,`, - `PciAddr:` + fmt.Sprintf("%v", this.PciAddr) + `,`, + `PciPath:` + fmt.Sprintf("%v", this.PciPath) + `,`, `Type:` + fmt.Sprintf("%v", this.Type) + `,`, `RawFlags:` + fmt.Sprintf("%v", this.RawFlags) + `,`, `XXX_unrecognized:` + fmt.Sprintf("%v", this.XXX_unrecognized) + `,`, @@ -1066,7 +1064,7 @@ func (m *Interface) Unmarshal(dAtA []byte) error { iNdEx = postIndex case 6: if wireType != 2 { - return fmt.Errorf("proto: wrong wireType = %d for field PciAddr", wireType) + return fmt.Errorf("proto: wrong wireType = %d for field PciPath", wireType) } var stringLen uint64 for shift := uint(0); ; shift += 7 { @@ -1094,7 +1092,7 @@ func (m *Interface) Unmarshal(dAtA []byte) error { if postIndex > l { return io.ErrUnexpectedEOF } - m.PciAddr = string(dAtA[iNdEx:postIndex]) + m.PciPath = string(dAtA[iNdEx:postIndex]) iNdEx = postIndex case 7: if wireType != 2 { diff --git a/src/runtime/virtcontainers/sandbox.go b/src/runtime/virtcontainers/sandbox.go index 243fb6e265..4d9d951556 100644 --- a/src/runtime/virtcontainers/sandbox.go +++ b/src/runtime/virtcontainers/sandbox.go @@ -850,7 +850,7 @@ func (s *Sandbox) AddInterface(inf *pbTypes.Interface) (*pbTypes.Interface, erro } // Add network for vm - inf.PciAddr = endpoint.PciAddr() + inf.PciPath = endpoint.PciAddr() return s.agent.updateInterface(inf) } From 3715c5775fcbe1b173a6a22e24073c8472c3c67d Mon Sep 17 00:00:00 2001 From: David Gibson Date: Fri, 30 Oct 2020 16:45:07 +1100 Subject: [PATCH 06/13] agent/device: Rename and clarify semantics of get_pci_device_address() get_pci_device_address() has pretty confusing semantics. Both its input and output are in other parts of the code described as a "PCI address", but neither is *actually* a PCI address (in the standard DDDD:BB:DD.F format). What it's really about is resolving a "PCI path" - that is way to locate a PCI device by using it's slot number and the slot number of the bridge leading to it - into a sysfs path. Rename the function, and change a bunch of variable names to make those semantics clearer. Forward port of https://github.com/kata-containers/agent/pull/855/commits/0eb612f06484 Signed-off-by: David Gibson --- src/agent/src/device.rs | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/src/agent/src/device.rs b/src/agent/src/device.rs index 7513bc1227..d1a575868e 100644 --- a/src/agent/src/device.rs +++ b/src/agent/src/device.rs @@ -45,17 +45,19 @@ pub fn online_device(path: &str) -> Result<()> { Ok(()) } -// get_pci_device_address fetches the complete PCI address in sysfs, based on the PCI -// identifier provided. This should be in the format: "bridgeAddr/deviceAddr". -// Here, bridgeAddr is the address at which the bridge is attached on the root bus, -// while deviceAddr is the address at which the device is attached on the bridge. -fn get_pci_device_address(pci_id: &str) -> Result { - let tokens: Vec<&str> = pci_id.split('/').collect(); +// pcipath_to_sysfs fetches the sysfs path for a PCI device, relative +// to the syfs path for the PCI host bridge, based on the PCI path +// provided. The path should be in the format "bridgeAddr/deviceAddr", +// where 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. +fn pcipath_to_sysfs(pcipath: &str) -> Result { + let tokens: Vec<&str> = pcipath.split('/').collect(); if tokens.len() != 2 { return Err(anyhow!( - "PCI Identifier for device should be of format [bridgeAddr/deviceAddr], got {}", - pci_id + "PCI path for device should be of format [bridgeAddr/deviceAddr], got {:?}", + pcipath )); } @@ -89,14 +91,14 @@ fn get_pci_device_address(pci_id: &str) -> Result { // We do not pass devices as multifunction, hence the trailing 0 in the address. let pci_device_addr = format!("{}:{}.0", bus, device_id); - let bridge_device_pci_addr = format!("{}/{}", pci_bridge_addr, pci_device_addr); + let sysfs_rel_path = format!("{}/{}", pci_bridge_addr, pci_device_addr); info!( sl!(), - "Fetched PCI address for device PCIAddr:{}\n", bridge_device_pci_addr + "Fetched sysfs relative path for PCI device {}\n", sysfs_rel_path ); - Ok(bridge_device_pci_addr) + Ok(sysfs_rel_path) } async fn get_device_name(sandbox: &Arc>, dev_addr: &str) -> Result { @@ -155,11 +157,11 @@ pub async fn get_scsi_device_name( get_device_name(sandbox, &dev_sub_path).await } -pub async fn get_pci_device_name(sandbox: &Arc>, pci_id: &str) -> Result { - let pci_addr = get_pci_device_address(pci_id)?; +pub async fn get_pci_device_name(sandbox: &Arc>, pcipath: &str) -> Result { + let sysfs_rel_path = pcipath_to_sysfs(pcipath)?; rescan_pci_bus()?; - get_device_name(sandbox, &pci_addr).await + get_device_name(sandbox, &sysfs_rel_path).await } pub async fn get_pmem_device_name( From c12b86dc8258ad9139d52044ca839fa57b659016 Mon Sep 17 00:00:00 2001 From: David Gibson Date: Sun, 13 Dec 2020 17:24:32 +1100 Subject: [PATCH 07/13] agent/device: Generalize PCI path resolution to any number of bridges Currently pcipath_to_sysfs(), which translates PCI paths into sysfs paths accepts only pci paths with exactly 2 components; which represents PCI devices separated from the root bus by exactly one PCI to PCI bridge (which could be a virtual P2P bridge, such as a PCI-E root port). There are cases we might reasonably want to support which have devices either plugged directly into the root bus (zero bridges), or under multiple layers of P2P bridge (a PCI-E switch would require at least 2 layers). So, generalize pcipath_to_sysfs to support any number of components in the PCI path. We also make it use the new type for PCI paths internally rather than plain strings. This is a loose forward port of https://github.com/kata-containers/agent/pull/855/commits/9804b1e55d1eae3a5b47ede6c2f40c401af25611 fixes #1040 Signed-off-by: David Gibson --- src/agent/src/device.rs | 88 ++++++++++++++++---------------------- src/agent/src/linux_abi.rs | 1 - 2 files changed, 38 insertions(+), 51 deletions(-) diff --git a/src/agent/src/device.rs b/src/agent/src/device.rs index d1a575868e..7fd66f9109 100644 --- a/src/agent/src/device.rs +++ b/src/agent/src/device.rs @@ -9,11 +9,13 @@ use std::collections::HashMap; use std::fs; use std::os::unix::fs::MetadataExt; use std::path::Path; +use std::str::FromStr; use std::sync::Arc; use tokio::sync::Mutex; use crate::linux_abi::*; use crate::mount::{DRIVER_BLK_TYPE, DRIVER_MMIO_BLK_TYPE, DRIVER_NVDIMM_TYPE, DRIVER_SCSI_TYPE}; +use crate::pci; use crate::sandbox::Sandbox; use crate::{AGENT_CONFIG, GLOBAL_DEVICE_WATCHER}; use anyhow::{anyhow, Result}; @@ -45,60 +47,46 @@ pub fn online_device(path: &str) -> Result<()> { Ok(()) } -// pcipath_to_sysfs fetches the sysfs path for a PCI device, relative -// to the syfs path for the PCI host bridge, based on the PCI path -// provided. The path should be in the format "bridgeAddr/deviceAddr", -// where 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. +// 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 tokens: Vec<&str> = pcipath.split('/').collect(); + let pcipath = pci::Path::from_str(pcipath)?; + let mut bus = "0000:00".to_string(); + let mut relpath = String::new(); + let root_bus_sysfs = format!("{}{}", SYSFS_DIR, create_pci_root_bus_path()); - if tokens.len() != 2 { - return Err(anyhow!( - "PCI path for device should be of format [bridgeAddr/deviceAddr], got {:?}", - pcipath - )); + for i in 0..pcipath.len() { + let bdf = format!("{}:{}.0", bus, pcipath[i]); + + relpath = format!("{}/{}", relpath, bdf); + + if i == pcipath.len() - 1 { + // Final device need not be a bridge + break; + } + + // Find out the bus exposed by bridge + let bridgebuspath = format!("{}{}/pci_bus", root_bus_sysfs, relpath); + let mut files: Vec<_> = fs::read_dir(&bridgebuspath)?.collect(); + + if files.len() != 1 { + return Err(anyhow!( + "Expected exactly one PCI bus in {}, got {} instead", + bridgebuspath, + files.len() + )); + } + + // unwrap is safe, because of the length test above + let busfile = files.pop().unwrap()?; + bus = busfile + .file_name() + .into_string() + .map_err(|e| anyhow!("Bad filename under {}: {:?}", &bridgebuspath, e))?; } - let bridge_id = tokens[0]; - let device_id = tokens[1]; - - // Deduce the complete bridge address based on the bridge address identifier passed - // and the fact that bridges are attached on the main bus with function 0. - let pci_bridge_addr = format!("0000:00:{}.0", bridge_id); - - // Find out the bus exposed by bridge - let bridge_bus_path = format!("{}/{}/pci_bus/", SYSFS_PCI_BUS_PREFIX, pci_bridge_addr); - - let files_slice: Vec<_> = fs::read_dir(&bridge_bus_path) - .unwrap() - .map(|res| res.unwrap().path()) - .collect(); - let bus_num = files_slice.len(); - - if bus_num != 1 { - return Err(anyhow!( - "Expected an entry for bus in {}, got {} entries instead", - bridge_bus_path, - bus_num - )); - } - - let bus = files_slice[0].file_name().unwrap().to_str().unwrap(); - - // Device address is based on the bus of the bridge to which it is attached. - // We do not pass devices as multifunction, hence the trailing 0 in the address. - let pci_device_addr = format!("{}:{}.0", bus, device_id); - - let sysfs_rel_path = format!("{}/{}", pci_bridge_addr, pci_device_addr); - - info!( - sl!(), - "Fetched sysfs relative path for PCI device {}\n", sysfs_rel_path - ); - - Ok(sysfs_rel_path) + Ok(relpath) } async fn get_device_name(sandbox: &Arc>, dev_addr: &str) -> Result { diff --git a/src/agent/src/linux_abi.rs b/src/agent/src/linux_abi.rs index c4a08376b6..c6e52fa9e9 100644 --- a/src/agent/src/linux_abi.rs +++ b/src/agent/src/linux_abi.rs @@ -9,7 +9,6 @@ use std::fs; pub const SYSFS_DIR: &str = "/sys"; -pub const SYSFS_PCI_BUS_PREFIX: &str = "/sys/bus/pci/devices"; pub const SYSFS_PCI_BUS_RESCAN_FILE: &str = "/sys/bus/pci/rescan"; #[cfg(any( target_arch = "powerpc64", From fda48a9bf0117e725b27aaca743985ec85bdf79e Mon Sep 17 00:00:00 2001 From: David Gibson Date: Tue, 15 Dec 2020 13:15:58 +1100 Subject: [PATCH 08/13] 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; } From 066ce7ab51220caf76bbcf53a61f9fffd8f55899 Mon Sep 17 00:00:00 2001 From: David Gibson Date: Tue, 9 Feb 2021 10:58:30 +1100 Subject: [PATCH 09/13] agent/device: Pass root bus sysfs path to pcipath_to_sysfs() Currently pcipath_to_sysfs() generates the path to the root bus node in sysfs via create_pci_root_bus_path(). This is inconvenient for testing, though, so instead make it take this as a parameter and generate the path in the (single) caller. As a bonus this will make life a bit easier when we want to support machines with multiple PCI roots. Signed-off-by: David Gibson --- src/agent/src/device.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/agent/src/device.rs b/src/agent/src/device.rs index a8c49b1231..b4d10a36d7 100644 --- a/src/agent/src/device.rs +++ b/src/agent/src/device.rs @@ -50,10 +50,9 @@ 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: &pci::Path) -> Result { +fn pcipath_to_sysfs(root_bus_sysfs: &str, 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()); for i in 0..pcipath.len() { let bdf = format!("{}:{}.0", bus, pcipath[i]); @@ -148,7 +147,8 @@ pub async fn get_pci_device_name( sandbox: &Arc>, pcipath: &pci::Path, ) -> Result { - let sysfs_rel_path = pcipath_to_sysfs(pcipath)?; + let root_bus_sysfs = format!("{}{}", SYSFS_DIR, create_pci_root_bus_path()); + let sysfs_rel_path = pcipath_to_sysfs(&root_bus_sysfs, pcipath)?; rescan_pci_bus()?; get_device_name(sandbox, &sysfs_rel_path).await From 87c5823c4bbc2848cbfaf614c88027f5d7368d56 Mon Sep 17 00:00:00 2001 From: David Gibson Date: Tue, 9 Feb 2021 12:05:48 +1100 Subject: [PATCH 10/13] agent/device: Add unit test for pcipath_to_sysfs() Port this test from the Kata 1 Go agent to the Kata 2 Rust agent. Signed-off-by: David Gibson --- src/agent/src/device.rs | 65 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/src/agent/src/device.rs b/src/agent/src/device.rs index b4d10a36d7..f69fa5c98f 100644 --- a/src/agent/src/device.rs +++ b/src/agent/src/device.rs @@ -435,6 +435,7 @@ pub fn update_device_cgroup(spec: &mut Spec) -> Result<()> { mod tests { use super::*; use oci::Linux; + use tempfile::tempdir; #[test] fn test_update_device_cgroup() { @@ -714,4 +715,68 @@ mod tests { assert_eq!(Some(host_major), specresources.devices[1].major); assert_eq!(Some(host_minor), specresources.devices[1].minor); } + + #[test] + fn test_pcipath_to_sysfs() { + let testdir = tempdir().expect("failed to create tmpdir"); + let rootbuspath = testdir.path().to_str().unwrap(); + + let path2 = pci::Path::from_str("02").unwrap(); + let path23 = pci::Path::from_str("02/03").unwrap(); + let path234 = pci::Path::from_str("02/03/04").unwrap(); + + let relpath = pcipath_to_sysfs(rootbuspath, &path2); + assert_eq!(relpath.unwrap(), "/0000:00:02.0"); + + let relpath = pcipath_to_sysfs(rootbuspath, &path23); + assert!(relpath.is_err()); + + let relpath = pcipath_to_sysfs(rootbuspath, &path234); + assert!(relpath.is_err()); + + // Create mock sysfs files for the device at 0000:00:02.0 + let bridge2path = format!("{}{}", rootbuspath, "/0000:00:02.0"); + + fs::create_dir_all(&bridge2path).unwrap(); + + let relpath = pcipath_to_sysfs(rootbuspath, &path2); + assert_eq!(relpath.unwrap(), "/0000:00:02.0"); + + let relpath = pcipath_to_sysfs(rootbuspath, &path23); + assert!(relpath.is_err()); + + let relpath = pcipath_to_sysfs(rootbuspath, &path234); + assert!(relpath.is_err()); + + // Create mock sysfs files to indicate that 0000:00:02.0 is a bridge to bus 01 + let bridge2bus = "0000:01"; + let bus2path = format!("{}/pci_bus/{}", bridge2path, bridge2bus); + + fs::create_dir_all(bus2path).unwrap(); + + let relpath = pcipath_to_sysfs(rootbuspath, &path2); + assert_eq!(relpath.unwrap(), "/0000:00:02.0"); + + let relpath = pcipath_to_sysfs(rootbuspath, &path23); + assert_eq!(relpath.unwrap(), "/0000:00:02.0/0000:01:03.0"); + + let relpath = pcipath_to_sysfs(rootbuspath, &path234); + assert!(relpath.is_err()); + + // Create mock sysfs files for a bridge at 0000:01:03.0 to bus 02 + let bridge3path = format!("{}/0000:01:03.0", bridge2path); + let bridge3bus = "0000:02"; + let bus3path = format!("{}/pci_bus/{}", bridge3path, bridge3bus); + + fs::create_dir_all(bus3path).unwrap(); + + let relpath = pcipath_to_sysfs(rootbuspath, &path2); + assert_eq!(relpath.unwrap(), "/0000:00:02.0"); + + let relpath = pcipath_to_sysfs(rootbuspath, &path23); + assert_eq!(relpath.unwrap(), "/0000:00:02.0/0000:01:03.0"); + + let relpath = pcipath_to_sysfs(rootbuspath, &path234); + assert_eq!(relpath.unwrap(), "/0000:00:02.0/0000:01:03.0/0000:02:04.0"); + } } From 32b40f5fe40d70d2045ec83aeb3b76e52ae139d7 Mon Sep 17 00:00:00 2001 From: David Gibson Date: Tue, 15 Dec 2020 14:27:57 +1100 Subject: [PATCH 11/13] runtime/network: Use PciPath type through network handling The "PCI address" returned by Endpoint::PciPath() isn't actually a PCI address (DDDD:BB:DD.F), but rather a PCI path. Rename and use the PciPath type to clean this up and the various parts of the network code connected to it. Forward port of https://github.com/kata-containers/runtime/pull/3003/commits/3e589713cf145c84b3e0b6849fc76c59b7678203 fixes #1040 Signed-off-by: David Gibson --- .../virtcontainers/bridgedmacvlan_endpoint.go | 15 ++++++++------- src/runtime/virtcontainers/endpoint.go | 5 +++-- src/runtime/virtcontainers/ipvlan_endpoint.go | 15 ++++++++------- .../virtcontainers/macvtap_endpoint.go | 19 ++++++++++--------- src/runtime/virtcontainers/network.go | 2 +- .../virtcontainers/persist/api/network.go | 5 +++-- .../virtcontainers/physical_endpoint.go | 15 ++++++++------- src/runtime/virtcontainers/qemu.go | 13 +++++++++++-- src/runtime/virtcontainers/sandbox.go | 2 +- src/runtime/virtcontainers/tap_endpoint.go | 15 ++++++++------- src/runtime/virtcontainers/tuntap_endpoint.go | 15 ++++++++------- src/runtime/virtcontainers/veth_endpoint.go | 15 ++++++++------- .../virtcontainers/vhostuser_endpoint.go | 19 ++++++++++--------- 13 files changed, 87 insertions(+), 68 deletions(-) diff --git a/src/runtime/virtcontainers/bridgedmacvlan_endpoint.go b/src/runtime/virtcontainers/bridgedmacvlan_endpoint.go index b4c3a09ec9..f54b769007 100644 --- a/src/runtime/virtcontainers/bridgedmacvlan_endpoint.go +++ b/src/runtime/virtcontainers/bridgedmacvlan_endpoint.go @@ -10,6 +10,7 @@ import ( "github.com/containernetworking/plugins/pkg/ns" persistapi "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/persist/api" + vcTypes "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/pkg/types" ) // BridgedMacvlanEndpoint represents a macvlan endpoint that is bridged to the VM @@ -17,7 +18,7 @@ type BridgedMacvlanEndpoint struct { NetPair NetworkInterfacePair EndpointProperties NetworkInfo EndpointType EndpointType - PCIAddr string + PCIPath vcTypes.PciPath RxRateLimiter bool TxRateLimiter bool } @@ -69,14 +70,14 @@ func (endpoint *BridgedMacvlanEndpoint) SetProperties(properties NetworkInfo) { endpoint.EndpointProperties = properties } -// PciAddr returns the PCI address of the endpoint. -func (endpoint *BridgedMacvlanEndpoint) PciAddr() string { - return endpoint.PCIAddr +// PciPath returns the PCI path of the endpoint. +func (endpoint *BridgedMacvlanEndpoint) PciPath() vcTypes.PciPath { + return endpoint.PCIPath } -// SetPciAddr sets the PCI address of the endpoint. -func (endpoint *BridgedMacvlanEndpoint) SetPciAddr(pciAddr string) { - endpoint.PCIAddr = pciAddr +// SetPciPath sets the PCI path of the endpoint. +func (endpoint *BridgedMacvlanEndpoint) SetPciPath(pciPath vcTypes.PciPath) { + endpoint.PCIPath = pciPath } // NetworkPair returns the network pair of the endpoint. diff --git a/src/runtime/virtcontainers/endpoint.go b/src/runtime/virtcontainers/endpoint.go index 1f422dad69..700f9157df 100644 --- a/src/runtime/virtcontainers/endpoint.go +++ b/src/runtime/virtcontainers/endpoint.go @@ -9,6 +9,7 @@ import ( "fmt" persistapi "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/persist/api" + vcTypes "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/pkg/types" ) // Endpoint represents a physical or virtual network interface. @@ -17,11 +18,11 @@ type Endpoint interface { Name() string HardwareAddr() string Type() EndpointType - PciAddr() string + PciPath() vcTypes.PciPath NetworkPair() *NetworkInterfacePair SetProperties(NetworkInfo) - SetPciAddr(string) + SetPciPath(vcTypes.PciPath) Attach(*Sandbox) error Detach(netNsCreated bool, netNsPath string) error HotAttach(h hypervisor) error diff --git a/src/runtime/virtcontainers/ipvlan_endpoint.go b/src/runtime/virtcontainers/ipvlan_endpoint.go index 38e9121c31..8c6dba673e 100644 --- a/src/runtime/virtcontainers/ipvlan_endpoint.go +++ b/src/runtime/virtcontainers/ipvlan_endpoint.go @@ -10,6 +10,7 @@ import ( "github.com/containernetworking/plugins/pkg/ns" persistapi "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/persist/api" + vcTypes "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/pkg/types" ) // IPVlanEndpoint represents a ipvlan endpoint that is bridged to the VM @@ -17,7 +18,7 @@ type IPVlanEndpoint struct { NetPair NetworkInterfacePair EndpointProperties NetworkInfo EndpointType EndpointType - PCIAddr string + PCIPath vcTypes.PciPath RxRateLimiter bool TxRateLimiter bool } @@ -72,14 +73,14 @@ func (endpoint *IPVlanEndpoint) SetProperties(properties NetworkInfo) { endpoint.EndpointProperties = properties } -// PciAddr returns the PCI address of the endpoint. -func (endpoint *IPVlanEndpoint) PciAddr() string { - return endpoint.PCIAddr +// PciPath returns the PCI path of the endpoint. +func (endpoint *IPVlanEndpoint) PciPath() vcTypes.PciPath { + return endpoint.PCIPath } -// SetPciAddr sets the PCI address of the endpoint. -func (endpoint *IPVlanEndpoint) SetPciAddr(pciAddr string) { - endpoint.PCIAddr = pciAddr +// SetPciPath sets the PCI path of the endpoint. +func (endpoint *IPVlanEndpoint) SetPciPath(pciPath vcTypes.PciPath) { + endpoint.PCIPath = pciPath } // NetworkPair returns the network pair of the endpoint. diff --git a/src/runtime/virtcontainers/macvtap_endpoint.go b/src/runtime/virtcontainers/macvtap_endpoint.go index 3f82869766..556d4ad5be 100644 --- a/src/runtime/virtcontainers/macvtap_endpoint.go +++ b/src/runtime/virtcontainers/macvtap_endpoint.go @@ -10,6 +10,7 @@ import ( "os" persistapi "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/persist/api" + vcTypes "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/pkg/types" ) // MacvtapEndpoint represents a macvtap endpoint @@ -18,7 +19,7 @@ type MacvtapEndpoint struct { EndpointType EndpointType VMFds []*os.File VhostFds []*os.File - PCIAddr string + PCIPath vcTypes.PciPath RxRateLimiter bool TxRateLimiter bool } @@ -93,14 +94,14 @@ func (endpoint *MacvtapEndpoint) HotDetach(h hypervisor, netNsCreated bool, netN return fmt.Errorf("MacvtapEndpoint does not support Hot detach") } -// PciAddr returns the PCI address of the endpoint. -func (endpoint *MacvtapEndpoint) PciAddr() string { - return endpoint.PCIAddr +// PciPath returns the PCI path of the endpoint. +func (endpoint *MacvtapEndpoint) PciPath() vcTypes.PciPath { + return endpoint.PCIPath } -// SetPciAddr sets the PCI address of the endpoint. -func (endpoint *MacvtapEndpoint) SetPciAddr(pciAddr string) { - endpoint.PCIAddr = pciAddr +// SetPciPath sets the PCI path of the endpoint. +func (endpoint *MacvtapEndpoint) SetPciPath(pciPath vcTypes.PciPath) { + endpoint.PCIPath = pciPath } // NetworkPair returns the network pair of the endpoint. @@ -113,7 +114,7 @@ func (endpoint *MacvtapEndpoint) save() persistapi.NetworkEndpoint { Type: string(endpoint.Type()), Macvtap: &persistapi.MacvtapEndpoint{ - PCIAddr: endpoint.PCIAddr, + PCIPath: endpoint.PCIPath, }, } } @@ -121,7 +122,7 @@ func (endpoint *MacvtapEndpoint) load(s persistapi.NetworkEndpoint) { endpoint.EndpointType = MacvtapEndpointType if s.Macvtap != nil { - endpoint.PCIAddr = s.Macvtap.PCIAddr + endpoint.PCIPath = s.Macvtap.PCIPath } } diff --git a/src/runtime/virtcontainers/network.go b/src/runtime/virtcontainers/network.go index f848cdb92f..5553c97080 100644 --- a/src/runtime/virtcontainers/network.go +++ b/src/runtime/virtcontainers/network.go @@ -984,7 +984,7 @@ func generateVCNetworkStructures(networkNS NetworkNamespace) ([]*pbTypes.Interfa Mtu: uint64(endpoint.Properties().Iface.MTU), RawFlags: noarp, HwAddr: endpoint.HardwareAddr(), - PciPath: endpoint.PciAddr(), + PciPath: endpoint.PciPath().String(), } ifaces = append(ifaces, &ifc) diff --git a/src/runtime/virtcontainers/persist/api/network.go b/src/runtime/virtcontainers/persist/api/network.go index 69610c6706..824f884539 100644 --- a/src/runtime/virtcontainers/persist/api/network.go +++ b/src/runtime/virtcontainers/persist/api/network.go @@ -7,6 +7,7 @@ package persistapi import ( + vcTypes "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/pkg/types" "github.com/vishvananda/netlink" ) @@ -48,7 +49,7 @@ type PhysicalEndpoint struct { type MacvtapEndpoint struct { // This is for showing information. // Remove this field won't impact anything. - PCIAddr string + PCIPath vcTypes.PciPath } type TapEndpoint struct { @@ -75,7 +76,7 @@ type VhostUserEndpoint struct { // This is for showing information. // Remove these fields won't impact anything. IfaceName string - PCIAddr string + PCIPath vcTypes.PciPath } // NetworkEndpoint contains network interface information diff --git a/src/runtime/virtcontainers/physical_endpoint.go b/src/runtime/virtcontainers/physical_endpoint.go index e778102a5b..af29aeefe1 100644 --- a/src/runtime/virtcontainers/physical_endpoint.go +++ b/src/runtime/virtcontainers/physical_endpoint.go @@ -17,6 +17,7 @@ import ( persistapi "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/persist/api" "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/pkg/cgroups" "github.com/safchain/ethtool" + vcTypes "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/pkg/types" ) // PhysicalEndpoint gathers a physical network interface and its properties @@ -28,7 +29,7 @@ type PhysicalEndpoint struct { BDF string Driver string VendorDeviceID string - PCIAddr string + PCIPath vcTypes.PciPath } // Properties returns the properties of the physical interface. @@ -51,14 +52,14 @@ func (endpoint *PhysicalEndpoint) Type() EndpointType { return endpoint.EndpointType } -// PciAddr returns the PCI address of the endpoint. -func (endpoint *PhysicalEndpoint) PciAddr() string { - return endpoint.PCIAddr +// PciPath returns the PCI path of the endpoint. +func (endpoint *PhysicalEndpoint) PciPath() vcTypes.PciPath { + return endpoint.PCIPath } -// SetPciAddr sets the PCI address of the endpoint. -func (endpoint *PhysicalEndpoint) SetPciAddr(pciAddr string) { - endpoint.PCIAddr = pciAddr +// SetPciPath sets the PCI path of the endpoint. +func (endpoint *PhysicalEndpoint) SetPciPath(pciPath vcTypes.PciPath) { + endpoint.PCIPath = pciPath } // SetProperties sets the properties of the physical endpoint. diff --git a/src/runtime/virtcontainers/qemu.go b/src/runtime/virtcontainers/qemu.go index f2683f9a73..39135ae2e8 100644 --- a/src/runtime/virtcontainers/qemu.go +++ b/src/runtime/virtcontainers/qemu.go @@ -39,6 +39,7 @@ import ( "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/pkg/uuid" "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/types" "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/utils" + vcTypes "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/pkg/types" ) // romFile is the file name of the ROM that can be used for virtio-pci devices. @@ -1546,8 +1547,16 @@ func (q *qemu) hotplugNetDevice(endpoint Endpoint, op operation) (err error) { } }() - pciAddr := fmt.Sprintf("%02x/%s", bridge.Addr, addr) - endpoint.SetPciAddr(pciAddr) + bridgeSlot, err := vcTypes.PciSlotFromInt(bridge.Addr) + if err != nil { + return err + } + devSlot, err := vcTypes.PciSlotFromString(addr) + if err != nil { + return err + } + pciPath, err := vcTypes.PciPathFromSlots(bridgeSlot, devSlot) + endpoint.SetPciPath(pciPath) var machine govmmQemu.Machine machine, err = q.getQemuMachine() diff --git a/src/runtime/virtcontainers/sandbox.go b/src/runtime/virtcontainers/sandbox.go index 4d9d951556..55577d19ef 100644 --- a/src/runtime/virtcontainers/sandbox.go +++ b/src/runtime/virtcontainers/sandbox.go @@ -850,7 +850,7 @@ func (s *Sandbox) AddInterface(inf *pbTypes.Interface) (*pbTypes.Interface, erro } // Add network for vm - inf.PciPath = endpoint.PciAddr() + inf.PciPath = endpoint.PciPath().String() return s.agent.updateInterface(inf) } diff --git a/src/runtime/virtcontainers/tap_endpoint.go b/src/runtime/virtcontainers/tap_endpoint.go index 583ffe9cc2..f9bb63640f 100644 --- a/src/runtime/virtcontainers/tap_endpoint.go +++ b/src/runtime/virtcontainers/tap_endpoint.go @@ -13,6 +13,7 @@ import ( persistapi "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/persist/api" "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/pkg/uuid" + vcTypes "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/pkg/types" ) // TapEndpoint represents just a tap endpoint @@ -20,7 +21,7 @@ type TapEndpoint struct { TapInterface TapInterface EndpointProperties NetworkInfo EndpointType EndpointType - PCIAddr string + PCIPath vcTypes.PciPath RxRateLimiter bool TxRateLimiter bool } @@ -45,14 +46,14 @@ func (endpoint *TapEndpoint) Type() EndpointType { return endpoint.EndpointType } -// PciAddr returns the PCI address of the endpoint. -func (endpoint *TapEndpoint) PciAddr() string { - return endpoint.PCIAddr +// PciPath returns the PCI path of the endpoint. +func (endpoint *TapEndpoint) PciPath() vcTypes.PciPath { + return endpoint.PCIPath } -// SetPciAddr sets the PCI address of the endpoint. -func (endpoint *TapEndpoint) SetPciAddr(pciAddr string) { - endpoint.PCIAddr = pciAddr +// SetPciPath sets the PCI path of the endpoint. +func (endpoint *TapEndpoint) SetPciPath(pciPath vcTypes.PciPath) { + endpoint.PCIPath = pciPath } // NetworkPair returns the network pair of the endpoint. diff --git a/src/runtime/virtcontainers/tuntap_endpoint.go b/src/runtime/virtcontainers/tuntap_endpoint.go index 7eef86ea87..9ca7344705 100644 --- a/src/runtime/virtcontainers/tuntap_endpoint.go +++ b/src/runtime/virtcontainers/tuntap_endpoint.go @@ -14,6 +14,7 @@ import ( "github.com/vishvananda/netlink" persistapi "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/persist/api" + vcTypes "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/pkg/types" ) // TuntapEndpoint represents just a tap endpoint @@ -22,7 +23,7 @@ type TuntapEndpoint struct { TuntapInterface TuntapInterface EndpointProperties NetworkInfo EndpointType EndpointType - PCIAddr string + PCIPath vcTypes.PciPath RxRateLimiter bool TxRateLimiter bool } @@ -47,14 +48,14 @@ func (endpoint *TuntapEndpoint) Type() EndpointType { return endpoint.EndpointType } -// PciAddr returns the PCI address of the endpoint. -func (endpoint *TuntapEndpoint) PciAddr() string { - return endpoint.PCIAddr +// PciPath returns the PCI path of the endpoint. +func (endpoint *TuntapEndpoint) PciPath() vcTypes.PciPath { + return endpoint.PCIPath } -// SetPciAddr sets the PCI address of the endpoint. -func (endpoint *TuntapEndpoint) SetPciAddr(pciAddr string) { - endpoint.PCIAddr = pciAddr +// SetPciPath sets the PCI path of the endpoint. +func (endpoint *TuntapEndpoint) SetPciPath(pciPath vcTypes.PciPath) { + endpoint.PCIPath = pciPath } // NetworkPair returns the network pair of the endpoint. diff --git a/src/runtime/virtcontainers/veth_endpoint.go b/src/runtime/virtcontainers/veth_endpoint.go index 371005798a..9339b48f83 100644 --- a/src/runtime/virtcontainers/veth_endpoint.go +++ b/src/runtime/virtcontainers/veth_endpoint.go @@ -10,6 +10,7 @@ import ( "github.com/containernetworking/plugins/pkg/ns" persistapi "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/persist/api" + vcTypes "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/pkg/types" ) // VethEndpoint gathers a network pair and its properties. @@ -17,7 +18,7 @@ type VethEndpoint struct { NetPair NetworkInterfacePair EndpointProperties NetworkInfo EndpointType EndpointType - PCIAddr string + PCIPath vcTypes.PciPath RxRateLimiter bool TxRateLimiter bool } @@ -67,14 +68,14 @@ func (endpoint *VethEndpoint) Type() EndpointType { return endpoint.EndpointType } -// PciAddr returns the PCI address of the endpoint. -func (endpoint *VethEndpoint) PciAddr() string { - return endpoint.PCIAddr +// PciPath returns the PCI path of the endpoint. +func (endpoint *VethEndpoint) PciPath() vcTypes.PciPath { + return endpoint.PCIPath } -// SetPciAddr sets the PCI address of the endpoint. -func (endpoint *VethEndpoint) SetPciAddr(pciAddr string) { - endpoint.PCIAddr = pciAddr +// SetPciPath sets the PCI path of the endpoint. +func (endpoint *VethEndpoint) SetPciPath(pciPath vcTypes.PciPath) { + endpoint.PCIPath = pciPath } // NetworkPair returns the network pair of the endpoint. diff --git a/src/runtime/virtcontainers/vhostuser_endpoint.go b/src/runtime/virtcontainers/vhostuser_endpoint.go index 642f06fb1d..c4ec45798c 100644 --- a/src/runtime/virtcontainers/vhostuser_endpoint.go +++ b/src/runtime/virtcontainers/vhostuser_endpoint.go @@ -13,6 +13,7 @@ import ( "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/device/config" persistapi "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/persist/api" "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/utils" + vcTypes "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/pkg/types" ) // Long term, this should be made more configurable. For now matching path @@ -30,7 +31,7 @@ type VhostUserEndpoint struct { IfaceName string EndpointProperties NetworkInfo EndpointType EndpointType - PCIAddr string + PCIPath vcTypes.PciPath } // Properties returns the properties of the interface. @@ -58,14 +59,14 @@ func (endpoint *VhostUserEndpoint) SetProperties(properties NetworkInfo) { endpoint.EndpointProperties = properties } -// PciAddr returns the PCI address of the endpoint. -func (endpoint *VhostUserEndpoint) PciAddr() string { - return endpoint.PCIAddr +// PciPath returns the PCI path of the endpoint. +func (endpoint *VhostUserEndpoint) PciPath() vcTypes.PciPath { + return endpoint.PCIPath } -// SetPciAddr sets the PCI address of the endpoint. -func (endpoint *VhostUserEndpoint) SetPciAddr(pciAddr string) { - endpoint.PCIAddr = pciAddr +// SetPciPath sets the PCI path of the endpoint. +func (endpoint *VhostUserEndpoint) SetPciPath(pciPath vcTypes.PciPath) { + endpoint.PCIPath = pciPath } // NetworkPair returns the network pair of the endpoint. @@ -156,7 +157,7 @@ func (endpoint *VhostUserEndpoint) save() persistapi.NetworkEndpoint { Type: string(endpoint.Type()), VhostUser: &persistapi.VhostUserEndpoint{ IfaceName: endpoint.IfaceName, - PCIAddr: endpoint.PCIAddr, + PCIPath: endpoint.PCIPath, }, } } @@ -166,7 +167,7 @@ func (endpoint *VhostUserEndpoint) load(s persistapi.NetworkEndpoint) { if s.VhostUser != nil { endpoint.IfaceName = s.VhostUser.IfaceName - endpoint.PCIAddr = s.VhostUser.PCIAddr + endpoint.PCIPath = s.VhostUser.PCIPath } } From 74f5b5febef043871f322ee38ffab45b44a1f8c6 Mon Sep 17 00:00:00 2001 From: David Gibson Date: Wed, 16 Dec 2020 13:40:12 +1100 Subject: [PATCH 12/13] runtime/block: Use PciPath type through block code BlockDrive::PCIAddr doesn't actually store a PCI address (DDDD:BB:DD.F) but a PCI path. Use the PciPath type and rename things to make that clearer. TestHandleBlockVolume() previously used a bizarre value "0002:01" for the "PCI address" which was neither an actual PCI address, nor a PCI path. Update it to use a PCI path - the actual value appears not to matter in this test, as long as its consistent throughout. Forward port of https://github.com/kata-containers/runtime/pull/3003/commits/64751f377b4bc380da8b3b74cd9d2e963d213e4e fixes #1040 Signed-off-by: David Gibson --- src/runtime/virtcontainers/acrn.go | 5 ++-- src/runtime/virtcontainers/clh.go | 5 ++-- .../virtcontainers/device/config/config.go | 5 ++-- .../virtcontainers/device/drivers/block.go | 4 +-- src/runtime/virtcontainers/kata_agent.go | 10 +++---- src/runtime/virtcontainers/kata_agent_test.go | 28 ++++++++++--------- .../virtcontainers/persist/api/device.go | 6 ++-- src/runtime/virtcontainers/qemu.go | 14 ++++++++-- 8 files changed, 47 insertions(+), 30 deletions(-) diff --git a/src/runtime/virtcontainers/acrn.go b/src/runtime/virtcontainers/acrn.go index c742bbee9b..7540ad653d 100644 --- a/src/runtime/virtcontainers/acrn.go +++ b/src/runtime/virtcontainers/acrn.go @@ -28,6 +28,7 @@ import ( "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/pkg/uuid" "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/types" "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/utils" + vcTypes "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/pkg/types" ) // Since ACRN is using the store in a quite abnormal way, let's first draw it back from store to here @@ -550,8 +551,8 @@ func (a *Acrn) updateBlockDevice(drive *config.BlockDrive) error { slot := AcrnBlkdDevSlot[drive.Index] - //Explicitly set PCIAddr to NULL, so that VirtPath can be used - drive.PCIAddr = "" + //Explicitly set PCIPath to NULL, so that VirtPath can be used + drive.PCIPath = vcTypes.PciPath{} args := []string{"blkrescan", a.acrnConfig.Name, fmt.Sprintf("%d,%s", slot, drive.File)} diff --git a/src/runtime/virtcontainers/clh.go b/src/runtime/virtcontainers/clh.go index 097c23931d..79e56904f0 100644 --- a/src/runtime/virtcontainers/clh.go +++ b/src/runtime/virtcontainers/clh.go @@ -32,6 +32,7 @@ import ( "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/device/config" "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/types" "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/utils" + vcTypes "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/pkg/types" ) // @@ -439,8 +440,8 @@ func (clh *cloudHypervisor) hotplugAddBlockDevice(drive *config.BlockDrive) erro driveID := clhDriveIndexToID(drive.Index) - //Explicitly set PCIAddr to NULL, so that VirtPath can be used - drive.PCIAddr = "" + //Explicitly set PCIPath to NULL, so that VirtPath can be used + drive.PCIPath = vcTypes.PciPath{} if drive.Pmem { err = fmt.Errorf("pmem device hotplug not supported") diff --git a/src/runtime/virtcontainers/device/config/config.go b/src/runtime/virtcontainers/device/config/config.go index e59278ee2c..aeacbd98cf 100644 --- a/src/runtime/virtcontainers/device/config/config.go +++ b/src/runtime/virtcontainers/device/config/config.go @@ -16,6 +16,7 @@ import ( "github.com/go-ini/ini" "golang.org/x/sys/unix" + vcTypes "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/pkg/types" ) // DeviceType indicates device type @@ -156,8 +157,8 @@ type BlockDrive struct { // MmioAddr is used to identify the slot at which the drive is attached (order?). MmioAddr string - // PCIAddr is the PCI address used to identify the slot at which the drive is attached. - PCIAddr string + // PCIPath is the PCI path used to identify the slot at which the drive is attached. + PCIPath vcTypes.PciPath // SCSI Address of the block device, in case the device is attached using SCSI driver // SCSI address is in the format SCSI-Id:LUN diff --git a/src/runtime/virtcontainers/device/drivers/block.go b/src/runtime/virtcontainers/device/drivers/block.go index e37e69c551..7ffa23e769 100644 --- a/src/runtime/virtcontainers/device/drivers/block.go +++ b/src/runtime/virtcontainers/device/drivers/block.go @@ -170,7 +170,7 @@ func (device *BlockDevice) Save() persistapi.DeviceState { ID: drive.ID, Index: drive.Index, MmioAddr: drive.MmioAddr, - PCIAddr: drive.PCIAddr, + PCIPath: drive.PCIPath, SCSIAddr: drive.SCSIAddr, NvdimmID: drive.NvdimmID, VirtPath: drive.VirtPath, @@ -196,7 +196,7 @@ func (device *BlockDevice) Load(ds persistapi.DeviceState) { ID: bd.ID, Index: bd.Index, MmioAddr: bd.MmioAddr, - PCIAddr: bd.PCIAddr, + PCIPath: bd.PCIPath, SCSIAddr: bd.SCSIAddr, NvdimmID: bd.NvdimmID, VirtPath: bd.VirtPath, diff --git a/src/runtime/virtcontainers/kata_agent.go b/src/runtime/virtcontainers/kata_agent.go index 2bbdcef496..a506d5b74b 100644 --- a/src/runtime/virtcontainers/kata_agent.go +++ b/src/runtime/virtcontainers/kata_agent.go @@ -1074,7 +1074,7 @@ func (k *kataAgent) appendBlockDevice(dev ContainerDevice, c *Container) *grpc.D kataDevice.Id = d.DevNo case config.VirtioBlock: kataDevice.Type = kataBlkDevType - kataDevice.Id = d.PCIAddr + kataDevice.Id = d.PCIPath.String() kataDevice.VmPath = d.VirtPath case config.VirtioSCSI: kataDevice.Type = kataSCSIDevType @@ -1178,10 +1178,10 @@ func (k *kataAgent) buildContainerRootfs(sandbox *Sandbox, c *Container, rootPat rootfs.Source = blockDrive.DevNo case sandbox.config.HypervisorConfig.BlockDeviceDriver == config.VirtioBlock: rootfs.Driver = kataBlkDevType - if blockDrive.PCIAddr == "" { + if blockDrive.PCIPath.IsNil() { rootfs.Source = blockDrive.VirtPath } else { - rootfs.Source = blockDrive.PCIAddr + rootfs.Source = blockDrive.PCIPath.String() } case sandbox.config.HypervisorConfig.BlockDeviceDriver == config.VirtioSCSI: @@ -1427,10 +1427,10 @@ func (k *kataAgent) handleDeviceBlockVolume(c *Container, m Mount, device api.De vol.Source = blockDrive.DevNo case c.sandbox.config.HypervisorConfig.BlockDeviceDriver == config.VirtioBlock: vol.Driver = kataBlkDevType - if blockDrive.PCIAddr == "" { + if blockDrive.PCIPath.IsNil() { vol.Source = blockDrive.VirtPath } else { - vol.Source = blockDrive.PCIAddr + vol.Source = blockDrive.PCIPath.String() } case c.sandbox.config.HypervisorConfig.BlockDeviceDriver == config.VirtioMmio: vol.Driver = kataMmioBlkDevType diff --git a/src/runtime/virtcontainers/kata_agent_test.go b/src/runtime/virtcontainers/kata_agent_test.go index 7a7cb73fec..226504d0fe 100644 --- a/src/runtime/virtcontainers/kata_agent_test.go +++ b/src/runtime/virtcontainers/kata_agent_test.go @@ -32,6 +32,7 @@ import ( "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/pkg/mock" "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/pkg/rootless" "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/types" + vcTypes "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/pkg/types" ) var ( @@ -39,7 +40,7 @@ var ( testBlockDeviceCtrPath = "testBlockDeviceCtrPath" testDevNo = "testDevNo" testNvdimmID = "testNvdimmID" - testPCIAddr = "04/02" + testPCIPath, _ = vcTypes.PciPathFromString("04/02") testSCSIAddr = "testSCSIAddr" testVirtPath = "testVirtPath" ) @@ -271,13 +272,13 @@ func TestHandleDeviceBlockVolume(t *testing.T) { inputMount: Mount{}, inputDev: &drivers.BlockDevice{ BlockDrive: &config.BlockDrive{ - PCIAddr: testPCIAddr, + PCIPath: testPCIPath, VirtPath: testVirtPath, }, }, resultVol: &pb.Storage{ Driver: kataBlkDevType, - Source: testPCIAddr, + Source: testPCIPath.String(), }, }, { @@ -353,16 +354,17 @@ func TestHandleBlockVolume(t *testing.T) { bDestination := "/DeviceBlock/destination" dDestination := "/DeviceDirectBlock/destination" vPCIAddr := "0001:01" - bPCIAddr := "0002:01" - dPCIAddr := "0003:01" + bPCIPath, err := vcTypes.PciPathFromString("03/04") + assert.NoError(t, err) + dPCIPath, err := vcTypes.PciPathFromString("04/05") vDev := drivers.NewVhostUserBlkDevice(&config.DeviceInfo{ID: vDevID}) bDev := drivers.NewBlockDevice(&config.DeviceInfo{ID: bDevID}) dDev := drivers.NewBlockDevice(&config.DeviceInfo{ID: dDevID}) vDev.VhostUserDeviceAttrs = &config.VhostUserDeviceAttrs{PCIAddr: vPCIAddr} - bDev.BlockDrive = &config.BlockDrive{PCIAddr: bPCIAddr} - dDev.BlockDrive = &config.BlockDrive{PCIAddr: dPCIAddr} + bDev.BlockDrive = &config.BlockDrive{PCIPath: bPCIPath} + dDev.BlockDrive = &config.BlockDrive{PCIPath: dPCIPath} var devices []api.Device devices = append(devices, vDev, bDev, dDev) @@ -418,14 +420,14 @@ func TestHandleBlockVolume(t *testing.T) { Fstype: "bind", Options: []string{"bind"}, Driver: kataBlkDevType, - Source: bPCIAddr, + Source: bPCIPath.String(), } dStorage := &pb.Storage{ MountPoint: dDestination, Fstype: "ext4", Options: []string{"ro"}, Driver: kataBlkDevType, - Source: dPCIAddr, + Source: dPCIPath.String(), } assert.Equal(t, vStorage, volumeStorages[0], "Error while handle VhostUserBlk type block volume") @@ -462,7 +464,7 @@ func TestAppendDevices(t *testing.T) { ID: id, }, BlockDrive: &config.BlockDrive{ - PCIAddr: testPCIAddr, + PCIPath: testPCIPath, }, }, } @@ -489,7 +491,7 @@ func TestAppendDevices(t *testing.T) { { Type: kataBlkDevType, ContainerPath: testBlockDeviceCtrPath, - Id: testPCIAddr, + Id: testPCIPath.String(), }, } updatedDevList := k.appendDevices(devList, c) @@ -509,7 +511,7 @@ func TestAppendVhostUserBlkDevices(t *testing.T) { }, VhostUserDeviceAttrs: &config.VhostUserDeviceAttrs{ Type: config.VhostUserBlk, - PCIAddr: testPCIAddr, + PCIAddr: testPCIPath.String(), }, }, } @@ -537,7 +539,7 @@ func TestAppendVhostUserBlkDevices(t *testing.T) { { Type: kataBlkDevType, ContainerPath: testBlockDeviceCtrPath, - Id: testPCIAddr, + Id: testPCIPath.String(), }, } updatedDevList := k.appendDevices(devList, c) diff --git a/src/runtime/virtcontainers/persist/api/device.go b/src/runtime/virtcontainers/persist/api/device.go index 8900c5f6fa..9c8b0814ad 100644 --- a/src/runtime/virtcontainers/persist/api/device.go +++ b/src/runtime/virtcontainers/persist/api/device.go @@ -6,6 +6,8 @@ package persistapi +import vcTypes "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/pkg/types" + // ============= sandbox level resources ============= // BlockDrive represents a block storage drive which may be used in case the storage @@ -26,8 +28,8 @@ type BlockDrive struct { // MmioAddr is used to identify the slot at which the drive is attached (order?). MmioAddr string - // PCIAddr is the PCI address used to identify the slot at which the drive is attached. - PCIAddr string + // PCIPath is the PCI path used to identify the slot at which the drive is attached. + PCIPath vcTypes.PciPath // SCSI Address of the block device, in case the device is attached using SCSI driver // SCSI address is in the format SCSI-Id:LUN diff --git a/src/runtime/virtcontainers/qemu.go b/src/runtime/virtcontainers/qemu.go index 39135ae2e8..e9cfa4c2e0 100644 --- a/src/runtime/virtcontainers/qemu.go +++ b/src/runtime/virtcontainers/qemu.go @@ -1289,8 +1289,18 @@ func (q *qemu) hotplugAddBlockDevice(drive *config.BlockDrive, op operation, dev } }() - // PCI address is in the format bridge-addr/device-addr eg. "03/02" - drive.PCIAddr = fmt.Sprintf("%02x", bridge.Addr) + "/" + addr + bridgeSlot, err := vcTypes.PciSlotFromInt(bridge.Addr) + if err != nil { + return err + } + devSlot, err := vcTypes.PciSlotFromString(addr) + if err != nil { + return err + } + drive.PCIPath, err = vcTypes.PciPathFromSlots(bridgeSlot, devSlot) + if err != nil { + return err + } if err = q.qmpMonitorCh.qmp.ExecutePCIDeviceAdd(q.qmpMonitorCh.ctx, drive.ID, devID, driver, addr, bridge.ID, romFile, 0, true, defaultDisableModern); err != nil { return err From 72cb9287a0a500ee0209ce5b7ce4b1a22faf19a9 Mon Sep 17 00:00:00 2001 From: David Gibson Date: Wed, 16 Dec 2020 13:46:52 +1100 Subject: [PATCH 13/13] vhost-user-blk: Use PciPath type for vhost user devices VhostUserDeviceAttrs::PCIAddr didn't actually store a PCI address (DDDD:BB:DD.F), but rather a PCI path. Use the PciPath type and rename things to make that clearer. TestHandleBlockVolume previously used the bizarre value "0001:01" which is neither a PCI address nor a PCI path for this value. Change it to a valid PCI path - it appears the actual value didn't matter for that test, as long as it was consistent. Forward port of https://github.com/kata-containers/runtime/pull/3003/commits/3596058c677a4554486d20cd82ab384cc93fa06d fixes #1040 Signed-off-by: David Gibson --- src/runtime/virtcontainers/device/config/config.go | 7 ++++--- .../virtcontainers/device/drivers/vhost_user_blk.go | 4 ++-- src/runtime/virtcontainers/kata_agent.go | 4 ++-- src/runtime/virtcontainers/kata_agent_test.go | 9 +++++---- src/runtime/virtcontainers/persist/api/device.go | 4 ++-- src/runtime/virtcontainers/qemu.go | 11 +++++++++-- 6 files changed, 24 insertions(+), 15 deletions(-) diff --git a/src/runtime/virtcontainers/device/config/config.go b/src/runtime/virtcontainers/device/config/config.go index aeacbd98cf..4faf24d7f9 100644 --- a/src/runtime/virtcontainers/device/config/config.go +++ b/src/runtime/virtcontainers/device/config/config.go @@ -250,9 +250,10 @@ type VhostUserDeviceAttrs struct { CacheSize uint32 Cache string - // PCIAddr is the PCI address used to identify the slot at which the drive is attached. - // It is only meaningful for vhost user block devices - PCIAddr string + // PCIPath is the PCI path used to identify the slot at which + // the drive is attached. It is only meaningful for vhost + // user block devices + PCIPath vcTypes.PciPath // Block index of the device if assigned Index int diff --git a/src/runtime/virtcontainers/device/drivers/vhost_user_blk.go b/src/runtime/virtcontainers/device/drivers/vhost_user_blk.go index 22afd58536..e4238fa362 100644 --- a/src/runtime/virtcontainers/device/drivers/vhost_user_blk.go +++ b/src/runtime/virtcontainers/device/drivers/vhost_user_blk.go @@ -164,7 +164,7 @@ func (device *VhostUserBlkDevice) Save() persistapi.DeviceState { DevID: vAttr.DevID, SocketPath: vAttr.SocketPath, Type: string(vAttr.Type), - PCIAddr: vAttr.PCIAddr, + PCIPath: vAttr.PCIPath, Index: vAttr.Index, } } @@ -185,7 +185,7 @@ func (device *VhostUserBlkDevice) Load(ds persistapi.DeviceState) { DevID: dev.DevID, SocketPath: dev.SocketPath, Type: config.DeviceType(dev.Type), - PCIAddr: dev.PCIAddr, + PCIPath: dev.PCIPath, Index: dev.Index, } } diff --git a/src/runtime/virtcontainers/kata_agent.go b/src/runtime/virtcontainers/kata_agent.go index a506d5b74b..9492a2f731 100644 --- a/src/runtime/virtcontainers/kata_agent.go +++ b/src/runtime/virtcontainers/kata_agent.go @@ -1099,7 +1099,7 @@ func (k *kataAgent) appendVhostUserBlkDevice(dev ContainerDevice, c *Container) kataDevice := &grpc.Device{ ContainerPath: dev.ContainerPath, Type: kataBlkDevType, - Id: d.PCIAddr, + Id: d.PCIPath.String(), } return kataDevice @@ -1467,7 +1467,7 @@ func (k *kataAgent) handleVhostUserBlkVolume(c *Container, m Mount, device api.D } vol.Driver = kataBlkDevType - vol.Source = d.PCIAddr + vol.Source = d.PCIPath.String() vol.Fstype = "bind" vol.Options = []string{"bind"} vol.MountPoint = m.Destination diff --git a/src/runtime/virtcontainers/kata_agent_test.go b/src/runtime/virtcontainers/kata_agent_test.go index 226504d0fe..6f1b97958d 100644 --- a/src/runtime/virtcontainers/kata_agent_test.go +++ b/src/runtime/virtcontainers/kata_agent_test.go @@ -353,7 +353,8 @@ func TestHandleBlockVolume(t *testing.T) { vDestination := "/VhostUserBlk/destination" bDestination := "/DeviceBlock/destination" dDestination := "/DeviceDirectBlock/destination" - vPCIAddr := "0001:01" + vPCIPath, err := vcTypes.PciPathFromString("01/02") + assert.NoError(t, err) bPCIPath, err := vcTypes.PciPathFromString("03/04") assert.NoError(t, err) dPCIPath, err := vcTypes.PciPathFromString("04/05") @@ -362,7 +363,7 @@ func TestHandleBlockVolume(t *testing.T) { bDev := drivers.NewBlockDevice(&config.DeviceInfo{ID: bDevID}) dDev := drivers.NewBlockDevice(&config.DeviceInfo{ID: dDevID}) - vDev.VhostUserDeviceAttrs = &config.VhostUserDeviceAttrs{PCIAddr: vPCIAddr} + vDev.VhostUserDeviceAttrs = &config.VhostUserDeviceAttrs{PCIPath: vPCIPath} bDev.BlockDrive = &config.BlockDrive{PCIPath: bPCIPath} dDev.BlockDrive = &config.BlockDrive{PCIPath: dPCIPath} @@ -413,7 +414,7 @@ func TestHandleBlockVolume(t *testing.T) { Fstype: "bind", Options: []string{"bind"}, Driver: kataBlkDevType, - Source: vPCIAddr, + Source: vPCIPath.String(), } bStorage := &pb.Storage{ MountPoint: bDestination, @@ -511,7 +512,7 @@ func TestAppendVhostUserBlkDevices(t *testing.T) { }, VhostUserDeviceAttrs: &config.VhostUserDeviceAttrs{ Type: config.VhostUserBlk, - PCIAddr: testPCIPath.String(), + PCIPath: testPCIPath, }, }, } diff --git a/src/runtime/virtcontainers/persist/api/device.go b/src/runtime/virtcontainers/persist/api/device.go index 9c8b0814ad..204ea7cfa7 100644 --- a/src/runtime/virtcontainers/persist/api/device.go +++ b/src/runtime/virtcontainers/persist/api/device.go @@ -73,9 +73,9 @@ type VhostUserDeviceAttrs struct { // MacAddress is only meaningful for vhost user net device MacAddress string - // PCIAddr is the PCI address used to identify the slot at which the drive is attached. + // PCIPath is the PCI path used to identify the slot at which the drive is attached. // It is only meaningful for vhost user block devices - PCIAddr string + PCIPath vcTypes.PciPath // Block index of the device if assigned Index int diff --git a/src/runtime/virtcontainers/qemu.go b/src/runtime/virtcontainers/qemu.go index e9cfa4c2e0..3760dc3515 100644 --- a/src/runtime/virtcontainers/qemu.go +++ b/src/runtime/virtcontainers/qemu.go @@ -1351,8 +1351,15 @@ func (q *qemu) hotplugAddVhostUserBlkDevice(vAttr *config.VhostUserDeviceAttrs, } }() - // PCI address is in the format bridge-addr/device-addr eg. "03/02" - vAttr.PCIAddr = fmt.Sprintf("%02x", bridge.Addr) + "/" + addr + bridgeSlot, err := vcTypes.PciSlotFromInt(bridge.Addr) + if err != nil { + return err + } + devSlot, err := vcTypes.PciSlotFromString(addr) + if err != nil { + return err + } + vAttr.PCIPath, err = vcTypes.PciPathFromSlots(bridgeSlot, devSlot) if err = q.qmpMonitorCh.qmp.ExecutePCIVhostUserDevAdd(q.qmpMonitorCh.ctx, driver, devID, vAttr.DevID, addr, bridge.ID); err != nil { return err