Skip to content
This repository was archived by the owner on May 12, 2021. It is now read-only.

Commit 0eb612f

Browse files
dgibsondgibson
committed
device: Rename and clarify semantics of getDevicePCIAddress
getDevicePCIAddress() 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. Signed-off-by: David Gibson <[email protected]>
1 parent 70ea18e commit 0eb612f

File tree

2 files changed

+39
-37
lines changed

2 files changed

+39
-37
lines changed

‎device.go‎

Lines changed: 25 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -42,15 +42,15 @@ const (
4242
)
4343

4444
var (
45-
sysBusPrefix = sysfsDir + "/bus/pci/devices"
46-
pciBusRescanFile = sysfsDir + "/bus/pci/rescan"
47-
pciBusPathFormat = "%s/%s/pci_bus/"
48-
systemDevPath = "/dev"
49-
getSCSIDevPath = getSCSIDevPathImpl
50-
getPmemDevPath = getPmemDevPathImpl
51-
getPCIDeviceName = getPCIDeviceNameImpl
52-
getDevicePCIAddress = getDevicePCIAddressImpl
53-
scanSCSIBus = scanSCSIBusImpl
45+
sysBusPrefix = sysfsDir + "/bus/pci/devices"
46+
pciBusRescanFile = sysfsDir + "/bus/pci/rescan"
47+
pciBusPathFormat = "%s/%s/pci_bus/"
48+
systemDevPath = "/dev"
49+
getSCSIDevPath = getSCSIDevPathImpl
50+
getPmemDevPath = getPmemDevPathImpl
51+
getPCIDeviceName = getPCIDeviceNameImpl
52+
pciPathToSysfs = pciPathToSysfsImpl
53+
scanSCSIBus = scanSCSIBusImpl
5454
)
5555

5656
// CCW variables
@@ -93,15 +93,17 @@ func rescanPciBus() error {
9393
return ioutil.WriteFile(pciBusRescanFile, []byte{'1'}, pciBusMode)
9494
}
9595

96-
// getDevicePCIAddress fetches the complete PCI address in sysfs, based on the PCI
97-
// identifier provided. This should be in the format: "bridgeAddr/deviceAddr".
98-
// Here, bridgeAddr is the address at which the brige is attached on the root bus,
99-
// while deviceAddr is the address at which the device is attached on the bridge.
100-
func getDevicePCIAddressImpl(pciID string) (string, error) {
101-
tokens := strings.Split(pciID, "/")
96+
// pciPathToSysfs fetches the sysfs path for a PCI path, relative to
97+
// the syfs path for the PCI host bridge, based on the PCI path
98+
// provided. The path should be in the format "bridgeAddr/deviceAddr",
99+
// where bridgeAddr is the address at which the brige is attached on
100+
// the root bus, while deviceAddr is the address at which the device
101+
// is attached on the bridge.
102+
func pciPathToSysfsImpl(pciPath string) (string, error) {
103+
tokens := strings.Split(pciPath, "/")
102104

103105
if len(tokens) != 2 {
104-
return "", fmt.Errorf("PCI Identifier for device should be of format [bridgeAddr/deviceAddr], got %s", pciID)
106+
return "", fmt.Errorf("PCI path for device should be of format [bridgeAddr/deviceAddr], got %q", pciPath)
105107
}
106108

107109
bridgeID := tokens[0]
@@ -130,10 +132,10 @@ func getDevicePCIAddressImpl(pciID string) (string, error) {
130132
// We do not pass devices as multifunction, hence the trailing 0 in the address.
131133
pciDeviceAddr := fmt.Sprintf("%s:%s.0", bus, deviceID)
132134

133-
bridgeDevicePCIAddr := fmt.Sprintf("%s/%s", pciBridgeAddr, pciDeviceAddr)
134-
agentLog.WithField("completePCIAddr", bridgeDevicePCIAddr).Info("Fetched PCI address for device")
135+
sysfsRelPath := fmt.Sprintf("%s/%s", pciBridgeAddr, pciDeviceAddr)
136+
agentLog.WithField("sysfsRelPath", sysfsRelPath).Info("Fetched sysfs relative path for PCI device")
135137

136-
return bridgeDevicePCIAddr, nil
138+
return sysfsRelPath, nil
137139
}
138140

139141
func getDeviceName(s *sandbox, devID string) (string, error) {
@@ -181,21 +183,21 @@ func getDeviceName(s *sandbox, devID string) (string, error) {
181183
return filepath.Join(systemDevPath, devName), nil
182184
}
183185

184-
func getPCIDeviceNameImpl(s *sandbox, pciID string) (string, error) {
185-
pciAddr, err := getDevicePCIAddress(pciID)
186+
func getPCIDeviceNameImpl(s *sandbox, pciPath string) (string, error) {
187+
sysfsRelPath, err := pciPathToSysfs(pciPath)
186188
if err != nil {
187189
return "", err
188190
}
189191

190-
fieldLogger := agentLog.WithField("pciAddr", pciAddr)
192+
fieldLogger := agentLog.WithField("sysfsRelPath", sysfsRelPath)
191193

192194
// Rescan pci bus if we need to wait for a new pci device
193195
if err = rescanPciBus(); err != nil {
194196
fieldLogger.WithError(err).Error("Failed to scan pci bus")
195197
return "", err
196198
}
197199

198-
return getDeviceName(s, pciAddr)
200+
return getDeviceName(s, sysfsRelPath)
199201
}
200202

201203
// device.Id should be the predicted device name (vda, vdb, ...)

‎device_test.go‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -92,47 +92,47 @@ func TestVirtioBlkDeviceHandlerEmptyLinuxDevicesSpecFailure(t *testing.T) {
9292
testVirtioBlkDeviceHandlerFailure(t, device, spec)
9393
}
9494

95-
func TestGetPCIAddress(t *testing.T) {
95+
func TestPciPathToSysfs(t *testing.T) {
9696
testDir, err := ioutil.TempDir("", "kata-agent-tmp-")
9797
if err != nil {
9898
t.Fatal(t, err)
9999
}
100100
defer os.RemoveAll(testDir)
101101

102-
pciID := "02"
103-
_, err = getDevicePCIAddress(pciID)
102+
pciPath := "02"
103+
_, err = pciPathToSysfs(pciPath)
104104
assert.NotNil(t, err)
105105

106-
pciID = "02/03/04"
107-
_, err = getDevicePCIAddress(pciID)
106+
pciPath = "02/03/04"
107+
_, err = pciPathToSysfs(pciPath)
108108
assert.NotNil(t, err)
109109

110110
bridgeID := "02"
111111
deviceID := "03"
112112
pciBus := "0000:01"
113-
expectedPCIAddress := "0000:00:02.0/0000:01:03.0"
114-
pciID = fmt.Sprintf("%s/%s", bridgeID, deviceID)
113+
expectedRelPath := "0000:00:02.0/0000:01:03.0"
114+
pciPath = fmt.Sprintf("%s/%s", bridgeID, deviceID)
115115

116116
// Set sysBusPrefix to test directory for unit tests.
117117
sysBusPrefix = testDir
118118
bridgeBusPath := fmt.Sprintf(pciBusPathFormat, sysBusPrefix, "0000:00:02.0")
119119

120-
_, err = getDevicePCIAddress(pciID)
120+
_, err = pciPathToSysfs(pciPath)
121121
assert.NotNil(t, err)
122122

123123
err = os.MkdirAll(bridgeBusPath, mountPerm)
124124
assert.Nil(t, err)
125125

126-
_, err = getDevicePCIAddress(pciID)
126+
_, err = pciPathToSysfs(pciPath)
127127
assert.NotNil(t, err)
128128

129129
err = os.MkdirAll(filepath.Join(bridgeBusPath, pciBus), mountPerm)
130130
assert.Nil(t, err)
131131

132-
addr, err := getDevicePCIAddress(pciID)
132+
addr, err := pciPathToSysfs(pciPath)
133133
assert.Nil(t, err)
134134

135-
assert.Equal(t, addr, expectedPCIAddress)
135+
assert.Equal(t, addr, expectedRelPath)
136136
}
137137

138138
func TestScanSCSIBus(t *testing.T) {
@@ -804,12 +804,12 @@ func TestGetPCIDeviceName(t *testing.T) {
804804

805805
sysfsDir = testSysfsDir
806806

807-
savedFunc := getDevicePCIAddress
807+
savedFunc := pciPathToSysfs
808808
defer func() {
809-
getDevicePCIAddress = savedFunc
809+
pciPathToSysfs = savedFunc
810810
}()
811811

812-
getDevicePCIAddress = func(pciID string) (string, error) {
812+
pciPathToSysfs = func(pciPath string) (string, error) {
813813
return "", nil
814814
}
815815

0 commit comments

Comments
 (0)