From 1c0dccb07b3f660fe846ec692817e653927d0abf Mon Sep 17 00:00:00 2001 From: David Gibson Date: Thu, 8 Oct 2020 12:39:34 +1100 Subject: [PATCH 1/6] vendor: Update vendored agent code Update the agent code under vendor/, to pull in https://github.com/kata-containers/agent/pull/855. This is a change to the grpc protocol files, but not an actual protocol change, since it's just renaming a field without changing the field number or structure. Shortlog: Archana Shinde (2): Merge pull request #826 from jodh-intel/actions-for-issue-backlog Merge pull request #830 from keloyang/resolv David Gibson (9): device: Index all devices in spec before updating them device: Check type as well as major:minor when looking up devices device: Rename pciDeviceMap in sandbox struct device: Simplify uevent matching in listenToUdevEvents() Merge pull request #849 from dgibson/bug848 device: Rename and clarify semantics of getDevicePCIAddress device: Introduce PciPath type, name things consistently device: Reorganize TestPciPathToSysfs device: Generalize PCI paths to any number of bridges Eric Ernst (2): release: Kata Containers 1.12.0-alpha1 Merge pull request #822 from egernst/1.12.0-alpha1-branch-bump James O. D. Hunt (6): Merge pull request #798 from bpradipt/hook-fix action: Require PR porting labels action: Add issue to project and move to "In progress" on linked PR Merge pull request #828 from jodh-intel/actions-require-pr-porting-labels action: Improve porting checks Merge pull request #836 from dgibson/bug834and835 Julio Montes (2): Merge pull request #844 from jodh-intel/action-improve-porting-checks Merge pull request #855 from dgibson/pcipath Pradipta Kr. Banerjee (1): oci: Fix running of OCI hooks Shukui Yang (1): network: Fix Could not create destination mount point: /etc/resolv.conf Signed-off-by: David Gibson --- Gopkg.lock | 4 +- Gopkg.toml | 2 +- .../agent/pkg/types/types.pb.go | 73 ++--- .../agent/protocols/grpc/agent.pb.go | 6 +- .../agent/protocols/grpc/health.pb.go | 36 +- .../agent/protocols/grpc/oci.pb.go | 310 ++++-------------- virtcontainers/kata_agent.go | 4 +- 7 files changed, 113 insertions(+), 322 deletions(-) diff --git a/Gopkg.lock b/Gopkg.lock index f9a510f9ff..0634bad00a 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -427,7 +427,7 @@ revision = "547a8518098aaa71c5d5d7a370f211e00161016b" [[projects]] - digest = "1:f03425555be45830e86903bba35dd82a1b76e322beff9873e6b888d2b054c217" + digest = "1:ee3d719407ec4bd877eaa4da37e2935298c3d9029ec3c20e502c0e14768b754c" name = "github.com/kata-containers/agent" packages = [ "pkg/types", @@ -435,7 +435,7 @@ "protocols/grpc", ] pruneopts = "NUT" - revision = "e921aa3d0fa39cbd7933c15bd6ae7f0a1d7ab757" + revision = "f9eab0fe9adb34e4f9f4a11f42a3eff983fd0659" [[projects]] digest = "1:58999a98719fddbac6303cb17e8d85b945f60b72f48e3a2df6b950b97fa926f1" diff --git a/Gopkg.toml b/Gopkg.toml index 383c2d92bd..2cdbda4adc 100644 --- a/Gopkg.toml +++ b/Gopkg.toml @@ -52,7 +52,7 @@ [[constraint]] name = "github.com/kata-containers/agent" - revision = "e921aa3d0fa39cbd7933c15bd6ae7f0a1d7ab757" + revision = "f9eab0fe9adb34e4f9f4a11f42a3eff983fd0659" [[constraint]] name = "github.com/containerd/cri-containerd" diff --git a/vendor/github.com/kata-containers/agent/pkg/types/types.pb.go b/vendor/github.com/kata-containers/agent/pkg/types/types.pb.go index a7948049e4..1635e936ab 100644 --- a/vendor/github.com/kata-containers/agent/pkg/types/types.pb.go +++ b/vendor/github.com/kata-containers/agent/pkg/types/types.pb.go @@ -91,10 +91,9 @@ type Interface struct { IPAddresses []*IPAddress `protobuf:"bytes,3,rep,name=IPAddresses" 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"` + // pciPath for the device (see type PciPath 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 @@ -143,9 +142,9 @@ func (m *Interface) GetHwAddr() string { return "" } -func (m *Interface) GetPciAddr() string { +func (m *Interface) GetPciPath() string { if m != nil { - return m.PciAddr + return m.PciPath } return "" } @@ -352,11 +351,11 @@ func (m *Interface) MarshalTo(dAtA []byte) (int, error) { i = encodeVarintTypes(dAtA, i, uint64(len(m.HwAddr))) i += copy(dAtA[i:], m.HwAddr) } - if len(m.PciAddr) > 0 { + if len(m.PciPath) > 0 { dAtA[i] = 0x32 i++ - i = encodeVarintTypes(dAtA, i, uint64(len(m.PciAddr))) - i += copy(dAtA[i:], m.PciAddr) + i = encodeVarintTypes(dAtA, i, uint64(len(m.PciPath))) + i += copy(dAtA[i:], m.PciPath) } if len(m.Type) > 0 { dAtA[i] = 0x3a @@ -519,7 +518,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)) } @@ -890,7 +889,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 { @@ -915,7 +914,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 { @@ -1459,30 +1458,30 @@ func init() { proto.RegisterFile("pkg/types/types.proto", fileDescriptorTypes) } var fileDescriptorTypes = []byte{ // 404 bytes of a gzipped FileDescriptorProto - 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x6c, 0x92, 0x41, 0x8a, 0xdb, 0x30, - 0x14, 0x86, 0xab, 0x38, 0xf6, 0xc4, 0x2f, 0x9d, 0xd6, 0x88, 0x76, 0x10, 0x2d, 0x04, 0xe3, 0x4d, - 0x4d, 0x17, 0x53, 0x48, 0x4b, 0xf7, 0xd3, 0xc5, 0x40, 0x36, 0x25, 0xe8, 0x02, 0x45, 0xb1, 0x15, - 0x8f, 0x89, 0x1d, 0x1b, 0x4b, 0x89, 0x09, 0x3d, 0x4b, 0xef, 0xd3, 0x65, 0x8f, 0x10, 0x72, 0x92, - 0xa2, 0x27, 0x39, 0xb8, 0x65, 0x36, 0xf6, 0xfb, 0x9e, 0x24, 0xbf, 0xff, 0xff, 0x2d, 0x78, 0xdb, - 0xee, 0x8a, 0x4f, 0xfa, 0xd4, 0x4a, 0x65, 0x9f, 0xf7, 0x6d, 0xd7, 0xe8, 0x86, 0xfa, 0x08, 0xc9, - 0x06, 0xc2, 0xd5, 0xfa, 0x21, 0xcf, 0x3b, 0xa9, 0x14, 0xfd, 0x00, 0xc1, 0x56, 0xd4, 0x65, 0x75, - 0x62, 0x24, 0x26, 0xe9, 0xab, 0xe5, 0xeb, 0x7b, 0x7b, 0x62, 0xb5, 0x7e, 0xc4, 0x36, 0x77, 0xcb, - 0x94, 0xc1, 0x8d, 0xb0, 0x67, 0xd8, 0x24, 0x26, 0x69, 0xc8, 0x07, 0xa4, 0x14, 0xa6, 0xb5, 0x50, - 0x3b, 0xe6, 0x61, 0x1b, 0xeb, 0xe4, 0x4c, 0x20, 0x5c, 0xed, 0xb5, 0xec, 0xb6, 0x22, 0x93, 0xf4, - 0x0e, 0x82, 0x5c, 0x1e, 0xcb, 0x4c, 0xe2, 0x90, 0x90, 0x3b, 0x32, 0x27, 0xf7, 0xa2, 0x96, 0xee, - 0x83, 0x58, 0xd3, 0x25, 0xcc, 0xaf, 0xea, 0xa4, 0x62, 0x5e, 0xec, 0xa5, 0xf3, 0x65, 0x74, 0x55, - 0xe5, 0x56, 0xf8, 0x78, 0x13, 0x8d, 0xc0, 0xab, 0xf5, 0x81, 0x4d, 0x63, 0x92, 0x4e, 0xb9, 0x29, - 0xcd, 0xc4, 0xa7, 0xde, 0x6c, 0x60, 0xbe, 0x9d, 0x68, 0xc9, 0xb8, 0x68, 0xb3, 0x12, 0x17, 0x02, - 0xeb, 0xc2, 0xa1, 0xd1, 0x62, 0x66, 0xb0, 0x1b, 0xab, 0xc5, 0xd4, 0xf4, 0x3d, 0x84, 0x9d, 0xe8, - 0x7f, 0x6c, 0x2b, 0x51, 0x28, 0x36, 0x8b, 0x49, 0x7a, 0xcb, 0x67, 0x9d, 0xe8, 0x1f, 0x0d, 0x27, - 0x3f, 0xc1, 0xe7, 0xcd, 0x41, 0xa3, 0x8b, 0x5c, 0x2a, 0xed, 0xbc, 0x61, 0x6d, 0xe6, 0x14, 0x42, - 0xcb, 0x5e, 0x9c, 0x86, 0xb4, 0x1c, 0x8e, 0xb2, 0xf0, 0xfe, 0xc9, 0xe2, 0x0e, 0x02, 0xd5, 0x1c, - 0xba, 0x4c, 0xa2, 0x8d, 0x90, 0x3b, 0xa2, 0x6f, 0xc0, 0x57, 0x59, 0xd3, 0x4a, 0x34, 0x72, 0xcb, - 0x2d, 0x24, 0xbf, 0x08, 0xcc, 0x1f, 0xf8, 0xfa, 0xbb, 0x2c, 0x8b, 0xa7, 0x4d, 0xd3, 0x99, 0xd4, - 0x74, 0x73, 0x8d, 0x04, 0xa5, 0x3c, 0x9b, 0xda, 0x68, 0xd3, 0x48, 0xc9, 0xe4, 0x7f, 0x25, 0x55, - 0x65, 0x7e, 0xee, 0xa0, 0xd0, 0x12, 0x2a, 0xd1, 0x42, 0x5b, 0x81, 0x3e, 0xb7, 0x60, 0xba, 0x36, - 0x1f, 0xdf, 0x76, 0x11, 0x3e, 0xbe, 0x83, 0xd9, 0x70, 0x83, 0x68, 0x00, 0x93, 0xe3, 0x97, 0xe8, - 0x05, 0xbe, 0xbf, 0x46, 0xe4, 0xdb, 0xcb, 0xdf, 0x97, 0x05, 0xf9, 0x73, 0x59, 0x90, 0xf3, 0x65, - 0x41, 0x36, 0x01, 0xde, 0xcd, 0xcf, 0x7f, 0x03, 0x00, 0x00, 0xff, 0xff, 0x99, 0x41, 0x3b, 0x8d, + 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x6c, 0x92, 0xcf, 0xca, 0x9b, 0x40, + 0x14, 0xc5, 0x3b, 0x31, 0xfa, 0xc5, 0x9b, 0xa6, 0x95, 0xa1, 0x0d, 0x43, 0x0b, 0x41, 0xdc, 0x54, + 0xba, 0x48, 0x21, 0x2d, 0xdd, 0xa7, 0x8b, 0x40, 0x36, 0x45, 0xe6, 0x05, 0xca, 0x44, 0x27, 0x46, + 0xa2, 0x51, 0x9c, 0x49, 0x24, 0xf4, 0x59, 0xfa, 0x3e, 0x5d, 0xf6, 0x11, 0x42, 0x9e, 0xa4, 0xcc, + 0x1f, 0x83, 0x2d, 0xdf, 0x46, 0xef, 0xef, 0xce, 0x8c, 0xf7, 0x9c, 0xe3, 0xc0, 0xdb, 0xe6, 0x98, + 0x7f, 0x92, 0xd7, 0x86, 0x0b, 0xf3, 0x5c, 0x36, 0x6d, 0x2d, 0x6b, 0xec, 0x6a, 0x88, 0x76, 0xe0, + 0x6f, 0x93, 0x75, 0x96, 0xb5, 0x5c, 0x08, 0xfc, 0x01, 0xbc, 0x3d, 0xab, 0x8a, 0xf2, 0x4a, 0x50, + 0x88, 0xe2, 0x57, 0xab, 0xd7, 0x4b, 0x73, 0x62, 0x9b, 0x6c, 0x74, 0x9b, 0xda, 0x65, 0x4c, 0xe0, + 0x89, 0x99, 0x33, 0x64, 0x14, 0xa2, 0xd8, 0xa7, 0x3d, 0x62, 0x0c, 0xe3, 0x8a, 0x89, 0x23, 0x71, + 0x74, 0x5b, 0xd7, 0xd1, 0x0d, 0x81, 0xbf, 0x3d, 0x49, 0xde, 0xee, 0x59, 0xca, 0xf1, 0x1c, 0xbc, + 0x8c, 0x5f, 0x8a, 0x94, 0xeb, 0x21, 0x3e, 0xb5, 0xa4, 0x4e, 0x9e, 0x58, 0xc5, 0xed, 0x07, 0x75, + 0x8d, 0x57, 0x30, 0x7d, 0xa8, 0xe3, 0x82, 0x38, 0xa1, 0x13, 0x4f, 0x57, 0xc1, 0x43, 0x95, 0x5d, + 0xa1, 0xc3, 0x4d, 0x38, 0x00, 0xa7, 0x92, 0x67, 0x32, 0x0e, 0x51, 0x3c, 0xa6, 0xaa, 0x54, 0x13, + 0x0f, 0x9d, 0xda, 0x40, 0x5c, 0x33, 0xd1, 0x90, 0x72, 0xd1, 0xa4, 0x45, 0xc2, 0xe4, 0x81, 0x78, + 0xc6, 0x85, 0x45, 0xa5, 0x45, 0xcd, 0x20, 0x4f, 0x46, 0x8b, 0xaa, 0xf1, 0x7b, 0xf0, 0x5b, 0xd6, + 0xfd, 0xd8, 0x97, 0x2c, 0x17, 0x64, 0x12, 0xa2, 0x78, 0x46, 0x27, 0x2d, 0xeb, 0x36, 0x8a, 0xa3, + 0x9f, 0xe0, 0xd2, 0xfa, 0x2c, 0xb5, 0x8b, 0x8c, 0x0b, 0x69, 0xbd, 0xe9, 0x5a, 0xcd, 0xc9, 0x99, + 0xe4, 0x1d, 0xbb, 0xf6, 0x69, 0x59, 0x1c, 0x64, 0xe1, 0xfc, 0x93, 0xc5, 0x1c, 0x3c, 0x51, 0x9f, + 0xdb, 0x94, 0x6b, 0x1b, 0x3e, 0xb5, 0x84, 0xdf, 0x80, 0x2b, 0xd2, 0xba, 0xe1, 0xda, 0xc8, 0x8c, + 0x1a, 0x88, 0x7e, 0x21, 0x98, 0xae, 0x69, 0xf2, 0x9d, 0x17, 0xf9, 0x61, 0x57, 0xb7, 0x2a, 0x35, + 0x59, 0x3f, 0x22, 0xd1, 0x52, 0x9e, 0x4d, 0x6d, 0xb0, 0x69, 0xa0, 0x64, 0xf4, 0xbf, 0x92, 0xb2, + 0x54, 0x3f, 0xb7, 0x57, 0x68, 0x48, 0x2b, 0x91, 0x4c, 0x1a, 0x81, 0x2e, 0x35, 0xa0, 0xba, 0x26, + 0x1f, 0xd7, 0x74, 0x35, 0x7c, 0x7c, 0x07, 0x93, 0xfe, 0x06, 0x61, 0x0f, 0x46, 0x97, 0x2f, 0xc1, + 0x0b, 0xfd, 0xfe, 0x1a, 0xa0, 0x6f, 0x2f, 0x7f, 0xdf, 0x17, 0xe8, 0xcf, 0x7d, 0x81, 0x6e, 0xf7, + 0x05, 0xda, 0x79, 0xfa, 0x6e, 0x7e, 0xfe, 0x1b, 0x00, 0x00, 0xff, 0xff, 0x88, 0xf1, 0xc1, 0x2b, 0xb4, 0x02, 0x00, 0x00, } diff --git a/vendor/github.com/kata-containers/agent/protocols/grpc/agent.pb.go b/vendor/github.com/kata-containers/agent/protocols/grpc/agent.pb.go index 775f313330..6f7b6cf4c1 100644 --- a/vendor/github.com/kata-containers/agent/protocols/grpc/agent.pb.go +++ b/vendor/github.com/kata-containers/agent/protocols/grpc/agent.pb.go @@ -110,10 +110,8 @@ import math "math" import types "github.com/kata-containers/agent/pkg/types" import google_protobuf2 "github.com/gogo/protobuf/types" -import ( - context "golang.org/x/net/context" - grpc1 "google.golang.org/grpc" -) +import context "golang.org/x/net/context" +import grpc1 "google.golang.org/grpc" import io "io" diff --git a/vendor/github.com/kata-containers/agent/protocols/grpc/health.pb.go b/vendor/github.com/kata-containers/agent/protocols/grpc/health.pb.go index 9dd54fbe32..2126ae3259 100644 --- a/vendor/github.com/kata-containers/agent/protocols/grpc/health.pb.go +++ b/vendor/github.com/kata-containers/agent/protocols/grpc/health.pb.go @@ -8,10 +8,8 @@ import fmt "fmt" import math "math" import _ "github.com/gogo/protobuf/gogoproto" -import ( - context "golang.org/x/net/context" - grpc1 "google.golang.org/grpc" -) +import context "golang.org/x/net/context" +import grpc1 "google.golang.org/grpc" import io "io" @@ -110,10 +108,7 @@ func init() { } func (this *CheckRequest) Equal(that interface{}) bool { if that == nil { - if this == nil { - return true - } - return false + return this == nil } that1, ok := that.(*CheckRequest) @@ -126,10 +121,7 @@ func (this *CheckRequest) Equal(that interface{}) bool { } } if that1 == nil { - if this == nil { - return true - } - return false + return this == nil } else if this == nil { return false } @@ -140,10 +132,7 @@ func (this *CheckRequest) Equal(that interface{}) bool { } func (this *HealthCheckResponse) Equal(that interface{}) bool { if that == nil { - if this == nil { - return true - } - return false + return this == nil } that1, ok := that.(*HealthCheckResponse) @@ -156,10 +145,7 @@ func (this *HealthCheckResponse) Equal(that interface{}) bool { } } if that1 == nil { - if this == nil { - return true - } - return false + return this == nil } else if this == nil { return false } @@ -170,10 +156,7 @@ func (this *HealthCheckResponse) Equal(that interface{}) bool { } func (this *VersionCheckResponse) Equal(that interface{}) bool { if that == nil { - if this == nil { - return true - } - return false + return this == nil } that1, ok := that.(*VersionCheckResponse) @@ -186,10 +169,7 @@ func (this *VersionCheckResponse) Equal(that interface{}) bool { } } if that1 == nil { - if this == nil { - return true - } - return false + return this == nil } else if this == nil { return false } diff --git a/vendor/github.com/kata-containers/agent/protocols/grpc/oci.pb.go b/vendor/github.com/kata-containers/agent/protocols/grpc/oci.pb.go index cbfbab5870..5296a082f2 100644 --- a/vendor/github.com/kata-containers/agent/protocols/grpc/oci.pb.go +++ b/vendor/github.com/kata-containers/agent/protocols/grpc/oci.pb.go @@ -1499,10 +1499,7 @@ func init() { } func (this *Spec) Equal(that interface{}) bool { if that == nil { - if this == nil { - return true - } - return false + return this == nil } that1, ok := that.(*Spec) @@ -1515,10 +1512,7 @@ func (this *Spec) Equal(that interface{}) bool { } } if that1 == nil { - if this == nil { - return true - } - return false + return this == nil } else if this == nil { return false } @@ -1566,10 +1560,7 @@ func (this *Spec) Equal(that interface{}) bool { } func (this *Process) Equal(that interface{}) bool { if that == nil { - if this == nil { - return true - } - return false + return this == nil } that1, ok := that.(*Process) @@ -1582,10 +1573,7 @@ func (this *Process) Equal(that interface{}) bool { } } if that1 == nil { - if this == nil { - return true - } - return false + return this == nil } else if this == nil { return false } @@ -1644,10 +1632,7 @@ func (this *Process) Equal(that interface{}) bool { } func (this *Box) Equal(that interface{}) bool { if that == nil { - if this == nil { - return true - } - return false + return this == nil } that1, ok := that.(*Box) @@ -1660,10 +1645,7 @@ func (this *Box) Equal(that interface{}) bool { } } if that1 == nil { - if this == nil { - return true - } - return false + return this == nil } else if this == nil { return false } @@ -1677,10 +1659,7 @@ func (this *Box) Equal(that interface{}) bool { } func (this *User) Equal(that interface{}) bool { if that == nil { - if this == nil { - return true - } - return false + return this == nil } that1, ok := that.(*User) @@ -1693,10 +1672,7 @@ func (this *User) Equal(that interface{}) bool { } } if that1 == nil { - if this == nil { - return true - } - return false + return this == nil } else if this == nil { return false } @@ -1721,10 +1697,7 @@ func (this *User) Equal(that interface{}) bool { } func (this *LinuxCapabilities) Equal(that interface{}) bool { if that == nil { - if this == nil { - return true - } - return false + return this == nil } that1, ok := that.(*LinuxCapabilities) @@ -1737,10 +1710,7 @@ func (this *LinuxCapabilities) Equal(that interface{}) bool { } } if that1 == nil { - if this == nil { - return true - } - return false + return this == nil } else if this == nil { return false } @@ -1788,10 +1758,7 @@ func (this *LinuxCapabilities) Equal(that interface{}) bool { } func (this *POSIXRlimit) Equal(that interface{}) bool { if that == nil { - if this == nil { - return true - } - return false + return this == nil } that1, ok := that.(*POSIXRlimit) @@ -1804,10 +1771,7 @@ func (this *POSIXRlimit) Equal(that interface{}) bool { } } if that1 == nil { - if this == nil { - return true - } - return false + return this == nil } else if this == nil { return false } @@ -1824,10 +1788,7 @@ func (this *POSIXRlimit) Equal(that interface{}) bool { } func (this *Mount) Equal(that interface{}) bool { if that == nil { - if this == nil { - return true - } - return false + return this == nil } that1, ok := that.(*Mount) @@ -1840,10 +1801,7 @@ func (this *Mount) Equal(that interface{}) bool { } } if that1 == nil { - if this == nil { - return true - } - return false + return this == nil } else if this == nil { return false } @@ -1868,10 +1826,7 @@ func (this *Mount) Equal(that interface{}) bool { } func (this *Root) Equal(that interface{}) bool { if that == nil { - if this == nil { - return true - } - return false + return this == nil } that1, ok := that.(*Root) @@ -1884,10 +1839,7 @@ func (this *Root) Equal(that interface{}) bool { } } if that1 == nil { - if this == nil { - return true - } - return false + return this == nil } else if this == nil { return false } @@ -1901,10 +1853,7 @@ func (this *Root) Equal(that interface{}) bool { } func (this *Hooks) Equal(that interface{}) bool { if that == nil { - if this == nil { - return true - } - return false + return this == nil } that1, ok := that.(*Hooks) @@ -1917,10 +1866,7 @@ func (this *Hooks) Equal(that interface{}) bool { } } if that1 == nil { - if this == nil { - return true - } - return false + return this == nil } else if this == nil { return false } @@ -1952,10 +1898,7 @@ func (this *Hooks) Equal(that interface{}) bool { } func (this *Hook) Equal(that interface{}) bool { if that == nil { - if this == nil { - return true - } - return false + return this == nil } that1, ok := that.(*Hook) @@ -1968,10 +1911,7 @@ func (this *Hook) Equal(that interface{}) bool { } } if that1 == nil { - if this == nil { - return true - } - return false + return this == nil } else if this == nil { return false } @@ -2001,10 +1941,7 @@ func (this *Hook) Equal(that interface{}) bool { } func (this *Linux) Equal(that interface{}) bool { if that == nil { - if this == nil { - return true - } - return false + return this == nil } that1, ok := that.(*Linux) @@ -2017,10 +1954,7 @@ func (this *Linux) Equal(that interface{}) bool { } } if that1 == nil { - if this == nil { - return true - } - return false + return this == nil } else if this == nil { return false } @@ -2102,10 +2036,7 @@ func (this *Linux) Equal(that interface{}) bool { } func (this *Windows) Equal(that interface{}) bool { if that == nil { - if this == nil { - return true - } - return false + return this == nil } that1, ok := that.(*Windows) @@ -2118,10 +2049,7 @@ func (this *Windows) Equal(that interface{}) bool { } } if that1 == nil { - if this == nil { - return true - } - return false + return this == nil } else if this == nil { return false } @@ -2132,10 +2060,7 @@ func (this *Windows) Equal(that interface{}) bool { } func (this *Solaris) Equal(that interface{}) bool { if that == nil { - if this == nil { - return true - } - return false + return this == nil } that1, ok := that.(*Solaris) @@ -2148,10 +2073,7 @@ func (this *Solaris) Equal(that interface{}) bool { } } if that1 == nil { - if this == nil { - return true - } - return false + return this == nil } else if this == nil { return false } @@ -2162,10 +2084,7 @@ func (this *Solaris) Equal(that interface{}) bool { } func (this *LinuxIDMapping) Equal(that interface{}) bool { if that == nil { - if this == nil { - return true - } - return false + return this == nil } that1, ok := that.(*LinuxIDMapping) @@ -2178,10 +2097,7 @@ func (this *LinuxIDMapping) Equal(that interface{}) bool { } } if that1 == nil { - if this == nil { - return true - } - return false + return this == nil } else if this == nil { return false } @@ -2198,10 +2114,7 @@ func (this *LinuxIDMapping) Equal(that interface{}) bool { } func (this *LinuxNamespace) Equal(that interface{}) bool { if that == nil { - if this == nil { - return true - } - return false + return this == nil } that1, ok := that.(*LinuxNamespace) @@ -2214,10 +2127,7 @@ func (this *LinuxNamespace) Equal(that interface{}) bool { } } if that1 == nil { - if this == nil { - return true - } - return false + return this == nil } else if this == nil { return false } @@ -2231,10 +2141,7 @@ func (this *LinuxNamespace) Equal(that interface{}) bool { } func (this *LinuxDevice) Equal(that interface{}) bool { if that == nil { - if this == nil { - return true - } - return false + return this == nil } that1, ok := that.(*LinuxDevice) @@ -2247,10 +2154,7 @@ func (this *LinuxDevice) Equal(that interface{}) bool { } } if that1 == nil { - if this == nil { - return true - } - return false + return this == nil } else if this == nil { return false } @@ -2279,10 +2183,7 @@ func (this *LinuxDevice) Equal(that interface{}) bool { } func (this *LinuxResources) Equal(that interface{}) bool { if that == nil { - if this == nil { - return true - } - return false + return this == nil } that1, ok := that.(*LinuxResources) @@ -2295,10 +2196,7 @@ func (this *LinuxResources) Equal(that interface{}) bool { } } if that1 == nil { - if this == nil { - return true - } - return false + return this == nil } else if this == nil { return false } @@ -2337,10 +2235,7 @@ func (this *LinuxResources) Equal(that interface{}) bool { } func (this *LinuxMemory) Equal(that interface{}) bool { if that == nil { - if this == nil { - return true - } - return false + return this == nil } that1, ok := that.(*LinuxMemory) @@ -2353,10 +2248,7 @@ func (this *LinuxMemory) Equal(that interface{}) bool { } } if that1 == nil { - if this == nil { - return true - } - return false + return this == nil } else if this == nil { return false } @@ -2385,10 +2277,7 @@ func (this *LinuxMemory) Equal(that interface{}) bool { } func (this *LinuxCPU) Equal(that interface{}) bool { if that == nil { - if this == nil { - return true - } - return false + return this == nil } that1, ok := that.(*LinuxCPU) @@ -2401,10 +2290,7 @@ func (this *LinuxCPU) Equal(that interface{}) bool { } } if that1 == nil { - if this == nil { - return true - } - return false + return this == nil } else if this == nil { return false } @@ -2433,10 +2319,7 @@ func (this *LinuxCPU) Equal(that interface{}) bool { } func (this *LinuxWeightDevice) Equal(that interface{}) bool { if that == nil { - if this == nil { - return true - } - return false + return this == nil } that1, ok := that.(*LinuxWeightDevice) @@ -2449,10 +2332,7 @@ func (this *LinuxWeightDevice) Equal(that interface{}) bool { } } if that1 == nil { - if this == nil { - return true - } - return false + return this == nil } else if this == nil { return false } @@ -2472,10 +2352,7 @@ func (this *LinuxWeightDevice) Equal(that interface{}) bool { } func (this *LinuxThrottleDevice) Equal(that interface{}) bool { if that == nil { - if this == nil { - return true - } - return false + return this == nil } that1, ok := that.(*LinuxThrottleDevice) @@ -2488,10 +2365,7 @@ func (this *LinuxThrottleDevice) Equal(that interface{}) bool { } } if that1 == nil { - if this == nil { - return true - } - return false + return this == nil } else if this == nil { return false } @@ -2508,10 +2382,7 @@ func (this *LinuxThrottleDevice) Equal(that interface{}) bool { } func (this *LinuxBlockIO) Equal(that interface{}) bool { if that == nil { - if this == nil { - return true - } - return false + return this == nil } that1, ok := that.(*LinuxBlockIO) @@ -2524,10 +2395,7 @@ func (this *LinuxBlockIO) Equal(that interface{}) bool { } } if that1 == nil { - if this == nil { - return true - } - return false + return this == nil } else if this == nil { return false } @@ -2581,10 +2449,7 @@ func (this *LinuxBlockIO) Equal(that interface{}) bool { } func (this *LinuxPids) Equal(that interface{}) bool { if that == nil { - if this == nil { - return true - } - return false + return this == nil } that1, ok := that.(*LinuxPids) @@ -2597,10 +2462,7 @@ func (this *LinuxPids) Equal(that interface{}) bool { } } if that1 == nil { - if this == nil { - return true - } - return false + return this == nil } else if this == nil { return false } @@ -2611,10 +2473,7 @@ func (this *LinuxPids) Equal(that interface{}) bool { } func (this *LinuxDeviceCgroup) Equal(that interface{}) bool { if that == nil { - if this == nil { - return true - } - return false + return this == nil } that1, ok := that.(*LinuxDeviceCgroup) @@ -2627,10 +2486,7 @@ func (this *LinuxDeviceCgroup) Equal(that interface{}) bool { } } if that1 == nil { - if this == nil { - return true - } - return false + return this == nil } else if this == nil { return false } @@ -2653,10 +2509,7 @@ func (this *LinuxDeviceCgroup) Equal(that interface{}) bool { } func (this *LinuxNetwork) Equal(that interface{}) bool { if that == nil { - if this == nil { - return true - } - return false + return this == nil } that1, ok := that.(*LinuxNetwork) @@ -2669,10 +2522,7 @@ func (this *LinuxNetwork) Equal(that interface{}) bool { } } if that1 == nil { - if this == nil { - return true - } - return false + return this == nil } else if this == nil { return false } @@ -2691,10 +2541,7 @@ func (this *LinuxNetwork) Equal(that interface{}) bool { } func (this *LinuxHugepageLimit) Equal(that interface{}) bool { if that == nil { - if this == nil { - return true - } - return false + return this == nil } that1, ok := that.(*LinuxHugepageLimit) @@ -2707,10 +2554,7 @@ func (this *LinuxHugepageLimit) Equal(that interface{}) bool { } } if that1 == nil { - if this == nil { - return true - } - return false + return this == nil } else if this == nil { return false } @@ -2724,10 +2568,7 @@ func (this *LinuxHugepageLimit) Equal(that interface{}) bool { } func (this *LinuxInterfacePriority) Equal(that interface{}) bool { if that == nil { - if this == nil { - return true - } - return false + return this == nil } that1, ok := that.(*LinuxInterfacePriority) @@ -2740,10 +2581,7 @@ func (this *LinuxInterfacePriority) Equal(that interface{}) bool { } } if that1 == nil { - if this == nil { - return true - } - return false + return this == nil } else if this == nil { return false } @@ -2757,10 +2595,7 @@ func (this *LinuxInterfacePriority) Equal(that interface{}) bool { } func (this *LinuxSeccomp) Equal(that interface{}) bool { if that == nil { - if this == nil { - return true - } - return false + return this == nil } that1, ok := that.(*LinuxSeccomp) @@ -2773,10 +2608,7 @@ func (this *LinuxSeccomp) Equal(that interface{}) bool { } } if that1 == nil { - if this == nil { - return true - } - return false + return this == nil } else if this == nil { return false } @@ -2803,10 +2635,7 @@ func (this *LinuxSeccomp) Equal(that interface{}) bool { } func (this *LinuxSeccompArg) Equal(that interface{}) bool { if that == nil { - if this == nil { - return true - } - return false + return this == nil } that1, ok := that.(*LinuxSeccompArg) @@ -2819,10 +2648,7 @@ func (this *LinuxSeccompArg) Equal(that interface{}) bool { } } if that1 == nil { - if this == nil { - return true - } - return false + return this == nil } else if this == nil { return false } @@ -2842,10 +2668,7 @@ func (this *LinuxSeccompArg) Equal(that interface{}) bool { } func (this *LinuxSyscall) Equal(that interface{}) bool { if that == nil { - if this == nil { - return true - } - return false + return this == nil } that1, ok := that.(*LinuxSyscall) @@ -2858,10 +2681,7 @@ func (this *LinuxSyscall) Equal(that interface{}) bool { } } if that1 == nil { - if this == nil { - return true - } - return false + return this == nil } else if this == nil { return false } @@ -2888,10 +2708,7 @@ func (this *LinuxSyscall) Equal(that interface{}) bool { } func (this *LinuxIntelRdt) Equal(that interface{}) bool { if that == nil { - if this == nil { - return true - } - return false + return this == nil } that1, ok := that.(*LinuxIntelRdt) @@ -2904,10 +2721,7 @@ func (this *LinuxIntelRdt) Equal(that interface{}) bool { } } if that1 == nil { - if this == nil { - return true - } - return false + return this == nil } else if this == nil { return false } diff --git a/virtcontainers/kata_agent.go b/virtcontainers/kata_agent.go index 5767e66bea..e897a2cca1 100644 --- a/virtcontainers/kata_agent.go +++ b/virtcontainers/kata_agent.go @@ -2297,7 +2297,7 @@ func (k *kataAgent) convertToKataAgentInterface(iface *vcTypes.Interface) *aType Mtu: iface.Mtu, RawFlags: iface.RawFlags, HwAddr: iface.HwAddr, - PciAddr: iface.PciAddr, + PciPath: iface.PciAddr, } } @@ -2313,7 +2313,7 @@ func (k *kataAgent) convertToInterfaces(aIfaces []*aTypes.Interface) (ifaces []* IPAddresses: k.convertToIPAddresses(aIface.IPAddresses), Mtu: aIface.Mtu, HwAddr: aIface.HwAddr, - PciAddr: aIface.PciAddr, + PciAddr: aIface.PciPath, } ifaces = append(ifaces, iface) From 185b3ab0446e04350a6685bd9f8f0ad0dc7d7b09 Mon Sep 17 00:00:00 2001 From: David Gibson Date: Thu, 8 Oct 2020 13:16:05 +1100 Subject: [PATCH 2/6] device: 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. Signed-off-by: David Gibson --- virtcontainers/pkg/types/pcipath.go | 98 ++++++++++++++++++++ virtcontainers/pkg/types/pcipath_test.go | 110 +++++++++++++++++++++++ 2 files changed, 208 insertions(+) create mode 100644 virtcontainers/pkg/types/pcipath.go create mode 100644 virtcontainers/pkg/types/pcipath_test.go diff --git a/virtcontainers/pkg/types/pcipath.go b/virtcontainers/pkg/types/pcipath.go new file mode 100644 index 0000000000..7f5acd09b6 --- /dev/null +++ b/virtcontainers/pkg/types/pcipath.go @@ -0,0 +1,98 @@ +// Copyright (c) 2020 Red Hat +// +// SPDX-License-Identifier: Apache-2.0 +// + +package types + +import ( + "fmt" + "strconv" + "strings" +) + +const ( + 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 value 0x%x out of range", v) + } + 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/virtcontainers/pkg/types/pcipath_test.go b/virtcontainers/pkg/types/pcipath_test.go new file mode 100644 index 0000000000..feb1c07a39 --- /dev/null +++ b/virtcontainers/pkg/types/pcipath_test.go @@ -0,0 +1,110 @@ +// Copyright (c) 2020 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 bfbfab3741b76797cb1da6dd9a7446545e7bda18 Mon Sep 17 00:00:00 2001 From: David Gibson Date: Thu, 15 Oct 2020 13:54:56 +1100 Subject: [PATCH 3/6] network: Allow convertToInterface to fail There's nothing in there which can fail now, but I'm planning to add something, and it turns out it's pretty easy for the single caller to handle. Signed-off-by: David Gibson --- virtcontainers/kata_agent.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/virtcontainers/kata_agent.go b/virtcontainers/kata_agent.go index e897a2cca1..b67cf688cf 100644 --- a/virtcontainers/kata_agent.go +++ b/virtcontainers/kata_agent.go @@ -673,7 +673,10 @@ func (k *kataAgent) listInterfaces() ([]*vcTypes.Interface, error) { } resultInterfaces, ok := resultingInterfaces.(*grpc.Interfaces) if ok { - return k.convertToInterfaces(resultInterfaces.Interfaces), err + ifaces, err := k.convertToInterfaces(resultInterfaces.Interfaces) + if err == nil { + return ifaces, nil + } } return nil, err } @@ -2301,7 +2304,8 @@ func (k *kataAgent) convertToKataAgentInterface(iface *vcTypes.Interface) *aType } } -func (k *kataAgent) convertToInterfaces(aIfaces []*aTypes.Interface) (ifaces []*vcTypes.Interface) { +func (k *kataAgent) convertToInterfaces(aIfaces []*aTypes.Interface) ([]*vcTypes.Interface, error) { + ifaces := make([]*vcTypes.Interface, 0) for _, aIface := range aIfaces { if aIface == nil { continue @@ -2319,7 +2323,7 @@ func (k *kataAgent) convertToInterfaces(aIfaces []*aTypes.Interface) (ifaces []* ifaces = append(ifaces, iface) } - return ifaces + return ifaces, nil } func (k *kataAgent) convertToKataAgentRoutes(routes []*vcTypes.Route) (aRoutes []*aTypes.Route) { From 3e589713cf145c84b3e0b6849fc76c59b7678203 Mon Sep 17 00:00:00 2001 From: David Gibson Date: Thu, 8 Oct 2020 13:16:27 +1100 Subject: [PATCH 4/6] 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. fixes #3002 Signed-off-by: David Gibson --- virtcontainers/bridgedmacvlan_endpoint.go | 15 ++++++++------- virtcontainers/endpoint.go | 5 +++-- virtcontainers/ipvlan_endpoint.go | 15 ++++++++------- virtcontainers/kata_agent.go | 8 ++++++-- virtcontainers/macvtap_endpoint.go | 19 ++++++++++--------- virtcontainers/network.go | 2 +- virtcontainers/persist/api/network.go | 5 +++-- virtcontainers/physical_endpoint.go | 15 ++++++++------- virtcontainers/pkg/types/types.go | 8 +++----- virtcontainers/qemu.go | 13 +++++++++++-- virtcontainers/sandbox.go | 2 +- virtcontainers/tap_endpoint.go | 15 ++++++++------- virtcontainers/tuntap_endpoint.go | 15 ++++++++------- virtcontainers/veth_endpoint.go | 15 ++++++++------- virtcontainers/vhostuser_endpoint.go | 19 ++++++++++--------- 15 files changed, 96 insertions(+), 75 deletions(-) diff --git a/virtcontainers/bridgedmacvlan_endpoint.go b/virtcontainers/bridgedmacvlan_endpoint.go index 3c54c006f1..2b395c2cb4 100644 --- a/virtcontainers/bridgedmacvlan_endpoint.go +++ b/virtcontainers/bridgedmacvlan_endpoint.go @@ -10,6 +10,7 @@ import ( "github.com/containernetworking/plugins/pkg/ns" persistapi "github.com/kata-containers/runtime/virtcontainers/persist/api" + vcTypes "github.com/kata-containers/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 } func createBridgedMacvlanNetworkEndpoint(idx int, ifName string, interworkingModel NetInterworkingModel) (*BridgedMacvlanEndpoint, error) { @@ -67,14 +68,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/virtcontainers/endpoint.go b/virtcontainers/endpoint.go index 22ed0885f9..46fb81abe0 100644 --- a/virtcontainers/endpoint.go +++ b/virtcontainers/endpoint.go @@ -9,6 +9,7 @@ import ( "fmt" persistapi "github.com/kata-containers/runtime/virtcontainers/persist/api" + vcTypes "github.com/kata-containers/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/virtcontainers/ipvlan_endpoint.go b/virtcontainers/ipvlan_endpoint.go index 8b0bdd8313..25977302e4 100644 --- a/virtcontainers/ipvlan_endpoint.go +++ b/virtcontainers/ipvlan_endpoint.go @@ -10,6 +10,7 @@ import ( "github.com/containernetworking/plugins/pkg/ns" persistapi "github.com/kata-containers/runtime/virtcontainers/persist/api" + vcTypes "github.com/kata-containers/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 } func createIPVlanNetworkEndpoint(idx int, ifName string) (*IPVlanEndpoint, error) { @@ -70,14 +71,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/virtcontainers/kata_agent.go b/virtcontainers/kata_agent.go index b67cf688cf..00a9669d7e 100644 --- a/virtcontainers/kata_agent.go +++ b/virtcontainers/kata_agent.go @@ -2300,7 +2300,7 @@ func (k *kataAgent) convertToKataAgentInterface(iface *vcTypes.Interface) *aType Mtu: iface.Mtu, RawFlags: iface.RawFlags, HwAddr: iface.HwAddr, - PciPath: iface.PciAddr, + PciPath: iface.PciPath.String(), } } @@ -2311,13 +2311,17 @@ func (k *kataAgent) convertToInterfaces(aIfaces []*aTypes.Interface) ([]*vcTypes continue } + pcipath, err := vcTypes.PciPathFromString(aIface.PciPath) + if err != nil { + return nil, err + } iface := &vcTypes.Interface{ Device: aIface.Device, Name: aIface.Name, IPAddresses: k.convertToIPAddresses(aIface.IPAddresses), Mtu: aIface.Mtu, HwAddr: aIface.HwAddr, - PciAddr: aIface.PciPath, + PciPath: pcipath, } ifaces = append(ifaces, iface) diff --git a/virtcontainers/macvtap_endpoint.go b/virtcontainers/macvtap_endpoint.go index 2dbb3f369a..b21c81e98a 100644 --- a/virtcontainers/macvtap_endpoint.go +++ b/virtcontainers/macvtap_endpoint.go @@ -10,6 +10,7 @@ import ( "os" persistapi "github.com/kata-containers/runtime/virtcontainers/persist/api" + vcTypes "github.com/kata-containers/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 } func createMacvtapNetworkEndpoint(netInfo NetworkInfo) (*MacvtapEndpoint, error) { @@ -91,14 +92,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. @@ -111,7 +112,7 @@ func (endpoint *MacvtapEndpoint) save() persistapi.NetworkEndpoint { Type: string(endpoint.Type()), Macvtap: &persistapi.MacvtapEndpoint{ - PCIAddr: endpoint.PCIAddr, + PCIPath: endpoint.PCIPath, }, } } @@ -119,6 +120,6 @@ 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/virtcontainers/network.go b/virtcontainers/network.go index eda0f1ea67..8fd6ac35b2 100644 --- a/virtcontainers/network.go +++ b/virtcontainers/network.go @@ -981,7 +981,7 @@ func generateVCNetworkStructures(networkNS NetworkNamespace) ([]*vcTypes.Interfa Mtu: uint64(endpoint.Properties().Iface.MTU), RawFlags: noarp, HwAddr: endpoint.HardwareAddr(), - PciAddr: endpoint.PciAddr(), + PciPath: endpoint.PciPath(), } ifaces = append(ifaces, &ifc) diff --git a/virtcontainers/persist/api/network.go b/virtcontainers/persist/api/network.go index 69610c6706..66196174b4 100644 --- a/virtcontainers/persist/api/network.go +++ b/virtcontainers/persist/api/network.go @@ -7,6 +7,7 @@ package persistapi import ( + vcTypes "github.com/kata-containers/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/virtcontainers/physical_endpoint.go b/virtcontainers/physical_endpoint.go index 5d1b9df496..570d196ced 100644 --- a/virtcontainers/physical_endpoint.go +++ b/virtcontainers/physical_endpoint.go @@ -16,6 +16,7 @@ import ( "github.com/kata-containers/runtime/virtcontainers/device/drivers" persistapi "github.com/kata-containers/runtime/virtcontainers/persist/api" "github.com/kata-containers/runtime/virtcontainers/pkg/cgroups" + vcTypes "github.com/kata-containers/runtime/virtcontainers/pkg/types" "github.com/safchain/ethtool" ) @@ -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/virtcontainers/pkg/types/types.go b/virtcontainers/pkg/types/types.go index 5abb4922ca..c494495eae 100644 --- a/virtcontainers/pkg/types/types.go +++ b/virtcontainers/pkg/types/types.go @@ -1,4 +1,4 @@ -// Copyright 2018 Intel Corporation. +// Copyright (c) 2018 Intel Corporation. // // SPDX-License-Identifier: Apache-2.0 // @@ -20,10 +20,8 @@ type Interface struct { Mtu uint64 RawFlags uint32 HwAddr string - // 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 + // PCI path for the interface + PciPath PciPath // LinkType 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 diff --git a/virtcontainers/qemu.go b/virtcontainers/qemu.go index c4dd91e906..86c8a0d319 100644 --- a/virtcontainers/qemu.go +++ b/virtcontainers/qemu.go @@ -33,6 +33,7 @@ import ( "github.com/kata-containers/runtime/virtcontainers/device/config" persistapi "github.com/kata-containers/runtime/virtcontainers/persist/api" + vcTypes "github.com/kata-containers/runtime/virtcontainers/pkg/types" "github.com/kata-containers/runtime/virtcontainers/pkg/uuid" "github.com/kata-containers/runtime/virtcontainers/types" "github.com/kata-containers/runtime/virtcontainers/utils" @@ -1420,8 +1421,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/virtcontainers/sandbox.go b/virtcontainers/sandbox.go index b44bc7c41b..302a28396b 100644 --- a/virtcontainers/sandbox.go +++ b/virtcontainers/sandbox.go @@ -988,7 +988,7 @@ func (s *Sandbox) AddInterface(inf *vcTypes.Interface) (*vcTypes.Interface, erro } // Add network for vm - inf.PciAddr = endpoint.PciAddr() + inf.PciPath = endpoint.PciPath() return s.agent.updateInterface(inf) } diff --git a/virtcontainers/tap_endpoint.go b/virtcontainers/tap_endpoint.go index 69f6d01b43..3d2d90e606 100644 --- a/virtcontainers/tap_endpoint.go +++ b/virtcontainers/tap_endpoint.go @@ -12,6 +12,7 @@ import ( "github.com/vishvananda/netlink" persistapi "github.com/kata-containers/runtime/virtcontainers/persist/api" + vcTypes "github.com/kata-containers/runtime/virtcontainers/pkg/types" "github.com/kata-containers/runtime/virtcontainers/pkg/uuid" ) @@ -20,7 +21,7 @@ type TapEndpoint struct { TapInterface TapInterface EndpointProperties NetworkInfo EndpointType EndpointType - PCIAddr string + PCIPath vcTypes.PciPath } // Properties returns the properties of the tap interface. @@ -43,14 +44,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/virtcontainers/tuntap_endpoint.go b/virtcontainers/tuntap_endpoint.go index c75a2cbd9d..0faea72521 100644 --- a/virtcontainers/tuntap_endpoint.go +++ b/virtcontainers/tuntap_endpoint.go @@ -14,6 +14,7 @@ import ( "github.com/vishvananda/netlink" persistapi "github.com/kata-containers/runtime/virtcontainers/persist/api" + vcTypes "github.com/kata-containers/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 } // Properties returns the properties of the tap interface. @@ -45,14 +46,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/virtcontainers/veth_endpoint.go b/virtcontainers/veth_endpoint.go index 861c860418..7d6b5de6ac 100644 --- a/virtcontainers/veth_endpoint.go +++ b/virtcontainers/veth_endpoint.go @@ -10,6 +10,7 @@ import ( "github.com/containernetworking/plugins/pkg/ns" persistapi "github.com/kata-containers/runtime/virtcontainers/persist/api" + vcTypes "github.com/kata-containers/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 } func createVethNetworkEndpoint(idx int, ifName string, interworkingModel NetInterworkingModel) (*VethEndpoint, error) { @@ -65,14 +66,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/virtcontainers/vhostuser_endpoint.go b/virtcontainers/vhostuser_endpoint.go index 6676d8450a..fc6259820e 100644 --- a/virtcontainers/vhostuser_endpoint.go +++ b/virtcontainers/vhostuser_endpoint.go @@ -12,6 +12,7 @@ import ( "github.com/kata-containers/runtime/virtcontainers/device/config" persistapi "github.com/kata-containers/runtime/virtcontainers/persist/api" + vcTypes "github.com/kata-containers/runtime/virtcontainers/pkg/types" "github.com/kata-containers/runtime/virtcontainers/utils" ) @@ -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,6 +167,6 @@ 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 64751f377b4bc380da8b3b74cd9d2e963d213e4e Mon Sep 17 00:00:00 2001 From: David Gibson Date: Thu, 8 Oct 2020 13:22:39 +1100 Subject: [PATCH 5/6] 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. fixes #3002 Signed-off-by: David Gibson --- virtcontainers/acrn.go | 5 +++-- virtcontainers/clh.go | 5 +++-- virtcontainers/device/config/config.go | 5 +++-- virtcontainers/device/drivers/block.go | 4 ++-- virtcontainers/kata_agent.go | 10 +++++----- virtcontainers/kata_agent_test.go | 21 +++++++++++---------- virtcontainers/persist/api/device.go | 6 ++++-- virtcontainers/qemu.go | 14 ++++++++++++-- 8 files changed, 43 insertions(+), 27 deletions(-) diff --git a/virtcontainers/acrn.go b/virtcontainers/acrn.go index 9ca433725b..7bbd61b493 100644 --- a/virtcontainers/acrn.go +++ b/virtcontainers/acrn.go @@ -23,6 +23,7 @@ import ( "github.com/kata-containers/runtime/virtcontainers/device/config" persistapi "github.com/kata-containers/runtime/virtcontainers/persist/api" + vcTypes "github.com/kata-containers/runtime/virtcontainers/pkg/types" "github.com/kata-containers/runtime/virtcontainers/pkg/uuid" "github.com/kata-containers/runtime/virtcontainers/types" "github.com/kata-containers/runtime/virtcontainers/utils" @@ -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/virtcontainers/clh.go b/virtcontainers/clh.go index bf2f97b0c5..eb89281ae7 100644 --- a/virtcontainers/clh.go +++ b/virtcontainers/clh.go @@ -29,6 +29,7 @@ import ( log "github.com/sirupsen/logrus" "github.com/kata-containers/runtime/virtcontainers/device/config" + vcTypes "github.com/kata-containers/runtime/virtcontainers/pkg/types" "github.com/kata-containers/runtime/virtcontainers/types" "github.com/kata-containers/runtime/virtcontainers/utils" ) @@ -422,8 +423,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/virtcontainers/device/config/config.go b/virtcontainers/device/config/config.go index 6146e8885d..94afbb1e46 100644 --- a/virtcontainers/device/config/config.go +++ b/virtcontainers/device/config/config.go @@ -15,6 +15,7 @@ import ( "strings" "github.com/go-ini/ini" + vcTypes "github.com/kata-containers/runtime/virtcontainers/pkg/types" "golang.org/x/sys/unix" ) @@ -153,8 +154,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/virtcontainers/device/drivers/block.go b/virtcontainers/device/drivers/block.go index 6711345571..31010a4068 100644 --- a/virtcontainers/device/drivers/block.go +++ b/virtcontainers/device/drivers/block.go @@ -169,7 +169,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, @@ -195,7 +195,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/virtcontainers/kata_agent.go b/virtcontainers/kata_agent.go index 00a9669d7e..87bd23ea31 100644 --- a/virtcontainers/kata_agent.go +++ b/virtcontainers/kata_agent.go @@ -1225,7 +1225,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 @@ -1329,10 +1329,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: @@ -1603,10 +1603,10 @@ func (k *kataAgent) handleDeviceBlockVolume(c *Container, device api.Device) (*g 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/virtcontainers/kata_agent_test.go b/virtcontainers/kata_agent_test.go index 8e8f8da10c..d05a02f3a7 100644 --- a/virtcontainers/kata_agent_test.go +++ b/virtcontainers/kata_agent_test.go @@ -45,7 +45,7 @@ var ( testBlockDeviceCtrPath = "testBlockDeviceCtrPath" testDevNo = "testDevNo" testNvdimmID = "testNvdimmID" - testPCIAddr = "04/02" + testPCIPath, _ = vcTypes.PciPathFromString("04/02") testSCSIAddr = "testSCSIAddr" testVirtPath = "testVirtPath" ) @@ -447,13 +447,13 @@ func TestHandleDeviceBlockVolume(t *testing.T) { BlockDeviceDriver: config.VirtioBlock, inputDev: &drivers.BlockDevice{ BlockDrive: &config.BlockDrive{ - PCIAddr: testPCIAddr, + PCIPath: testPCIPath, VirtPath: testVirtPath, }, }, resultVol: &pb.Storage{ Driver: kataBlkDevType, - Source: testPCIAddr, + Source: testPCIPath.String(), }, }, { @@ -527,13 +527,14 @@ func TestHandleBlockVolume(t *testing.T) { vDestination := "/VhostUserBlk/destination" bDestination := "/DeviceBlock/destination" vPCIAddr := "0001:01" - bPCIAddr := "0002:01" + bPCIPath, err := vcTypes.PciPathFromString("03/04") + assert.NoError(t, err) vDev := drivers.NewVhostUserBlkDevice(&config.DeviceInfo{ID: vDevID}) bDev := drivers.NewBlockDevice(&config.DeviceInfo{ID: bDevID}) vDev.VhostUserDeviceAttrs = &config.VhostUserDeviceAttrs{PCIAddr: vPCIAddr} - bDev.BlockDrive = &config.BlockDrive{PCIAddr: bPCIAddr} + bDev.BlockDrive = &config.BlockDrive{PCIPath: bPCIPath} var devices []api.Device devices = append(devices, vDev, bDev) @@ -581,7 +582,7 @@ func TestHandleBlockVolume(t *testing.T) { Fstype: "bind", Options: []string{"bind"}, Driver: kataBlkDevType, - Source: bPCIAddr, + Source: bPCIPath.String(), } assert.Equal(t, vStorage, volumeStorages[0], "Error while handle VhostUserBlk type block volume") @@ -617,7 +618,7 @@ func TestAppendDevices(t *testing.T) { ID: id, }, BlockDrive: &config.BlockDrive{ - PCIAddr: testPCIAddr, + PCIPath: testPCIPath, }, }, } @@ -644,7 +645,7 @@ func TestAppendDevices(t *testing.T) { { Type: kataBlkDevType, ContainerPath: testBlockDeviceCtrPath, - Id: testPCIAddr, + Id: testPCIPath.String(), }, } updatedDevList := k.appendDevices(devList, c) @@ -664,7 +665,7 @@ func TestAppendVhostUserBlkDevices(t *testing.T) { }, VhostUserDeviceAttrs: &config.VhostUserDeviceAttrs{ Type: config.VhostUserBlk, - PCIAddr: testPCIAddr, + PCIAddr: testPCIPath.String(), }, }, } @@ -692,7 +693,7 @@ func TestAppendVhostUserBlkDevices(t *testing.T) { { Type: kataBlkDevType, ContainerPath: testBlockDeviceCtrPath, - Id: testPCIAddr, + Id: testPCIPath.String(), }, } updatedDevList := k.appendDevices(devList, c) diff --git a/virtcontainers/persist/api/device.go b/virtcontainers/persist/api/device.go index 8900c5f6fa..4957f91d9a 100644 --- a/virtcontainers/persist/api/device.go +++ b/virtcontainers/persist/api/device.go @@ -6,6 +6,8 @@ package persistapi +import vcTypes "github.com/kata-containers/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/virtcontainers/qemu.go b/virtcontainers/qemu.go index 86c8a0d319..3208ea02c7 100644 --- a/virtcontainers/qemu.go +++ b/virtcontainers/qemu.go @@ -1153,8 +1153,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 3596058c677a4554486d20cd82ab384cc93fa06d Mon Sep 17 00:00:00 2001 From: David Gibson Date: Thu, 8 Oct 2020 13:26:31 +1100 Subject: [PATCH 6/6] 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. fixes #3002 Signed-off-by: David Gibson --- virtcontainers/device/config/config.go | 7 ++++--- virtcontainers/device/drivers/vhost_user_blk.go | 4 ++-- virtcontainers/kata_agent.go | 4 ++-- virtcontainers/kata_agent_test.go | 9 +++++---- virtcontainers/persist/api/device.go | 4 ++-- virtcontainers/qemu.go | 11 +++++++++-- 6 files changed, 24 insertions(+), 15 deletions(-) diff --git a/virtcontainers/device/config/config.go b/virtcontainers/device/config/config.go index 94afbb1e46..bcd0d62695 100644 --- a/virtcontainers/device/config/config.go +++ b/virtcontainers/device/config/config.go @@ -247,9 +247,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/virtcontainers/device/drivers/vhost_user_blk.go b/virtcontainers/device/drivers/vhost_user_blk.go index cdc33cf8ac..4c2a06b4c6 100644 --- a/virtcontainers/device/drivers/vhost_user_blk.go +++ b/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/virtcontainers/kata_agent.go b/virtcontainers/kata_agent.go index 87bd23ea31..a57da3a3e6 100644 --- a/virtcontainers/kata_agent.go +++ b/virtcontainers/kata_agent.go @@ -1250,7 +1250,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 @@ -1633,7 +1633,7 @@ func (k *kataAgent) handleVhostUserBlkVolume(c *Container, device api.Device) (* } vol.Driver = kataBlkDevType - vol.Source = d.PCIAddr + vol.Source = d.PCIPath.String() return vol, nil } diff --git a/virtcontainers/kata_agent_test.go b/virtcontainers/kata_agent_test.go index d05a02f3a7..0041184fa0 100644 --- a/virtcontainers/kata_agent_test.go +++ b/virtcontainers/kata_agent_test.go @@ -526,14 +526,15 @@ func TestHandleBlockVolume(t *testing.T) { bDevID := "MockDeviceBlock" vDestination := "/VhostUserBlk/destination" bDestination := "/DeviceBlock/destination" - vPCIAddr := "0001:01" + vPCIPath, err := vcTypes.PciPathFromString("01/02") + assert.NoError(t, err) bPCIPath, err := vcTypes.PciPathFromString("03/04") assert.NoError(t, err) vDev := drivers.NewVhostUserBlkDevice(&config.DeviceInfo{ID: vDevID}) bDev := drivers.NewBlockDevice(&config.DeviceInfo{ID: bDevID}) - vDev.VhostUserDeviceAttrs = &config.VhostUserDeviceAttrs{PCIAddr: vPCIAddr} + vDev.VhostUserDeviceAttrs = &config.VhostUserDeviceAttrs{PCIPath: vPCIPath} bDev.BlockDrive = &config.BlockDrive{PCIPath: bPCIPath} var devices []api.Device @@ -575,7 +576,7 @@ func TestHandleBlockVolume(t *testing.T) { Fstype: "bind", Options: []string{"bind"}, Driver: kataBlkDevType, - Source: vPCIAddr, + Source: vPCIPath.String(), } bStorage := &pb.Storage{ MountPoint: bDestination, @@ -665,7 +666,7 @@ func TestAppendVhostUserBlkDevices(t *testing.T) { }, VhostUserDeviceAttrs: &config.VhostUserDeviceAttrs{ Type: config.VhostUserBlk, - PCIAddr: testPCIPath.String(), + PCIPath: testPCIPath, }, }, } diff --git a/virtcontainers/persist/api/device.go b/virtcontainers/persist/api/device.go index 4957f91d9a..3768263d78 100644 --- a/virtcontainers/persist/api/device.go +++ b/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/virtcontainers/qemu.go b/virtcontainers/qemu.go index 3208ea02c7..5b1867c8c5 100644 --- a/virtcontainers/qemu.go +++ b/virtcontainers/qemu.go @@ -1215,8 +1215,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