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]>

device.go

@@ -42,15 +42,15 @@ const (
 )
 
 var (
-	sysBusPrefix        = sysfsDir + "/bus/pci/devices"
-	pciBusRescanFile    = sysfsDir + "/bus/pci/rescan"
-	pciBusPathFormat    = "%s/%s/pci_bus/"
-	systemDevPath       = "/dev"
-	getSCSIDevPath      = getSCSIDevPathImpl
-	getPmemDevPath      = getPmemDevPathImpl
-	getPCIDeviceName    = getPCIDeviceNameImpl
-	getDevicePCIAddress = getDevicePCIAddressImpl
-	scanSCSIBus         = scanSCSIBusImpl
+	sysBusPrefix     = sysfsDir + "/bus/pci/devices"
+	pciBusRescanFile = sysfsDir + "/bus/pci/rescan"
+	pciBusPathFormat = "%s/%s/pci_bus/"
+	systemDevPath    = "/dev"
+	getSCSIDevPath   = getSCSIDevPathImpl
+	getPmemDevPath   = getPmemDevPathImpl
+	getPCIDeviceName = getPCIDeviceNameImpl
+	pciPathToSysfs   = pciPathToSysfsImpl
+	scanSCSIBus      = scanSCSIBusImpl
 )
 
 // CCW variables
@@ -93,15 +93,17 @@ func rescanPciBus() error {
 	return ioutil.WriteFile(pciBusRescanFile, []byte{'1'}, pciBusMode)
 }
 
-// getDevicePCIAddress 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 brige is attached on the root bus,
-// while deviceAddr is the address at which the device is attached on the bridge.
-func getDevicePCIAddressImpl(pciID string) (string, error) {
-	tokens := strings.Split(pciID, "/")
+// 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. 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.
+func pciPathToSysfsImpl(pciPath string) (string, error) {
+	tokens := strings.Split(pciPath, "/")
 
 	if len(tokens) != 2 {
-		return "", fmt.Errorf("PCI Identifier for device should be of format [bridgeAddr/deviceAddr], got %s", pciID)
+		return "", fmt.Errorf("PCI path for device should be of format [bridgeAddr/deviceAddr], got %q", pciPath)
 	}
 
 	bridgeID := tokens[0]
@@ -130,10 +132,10 @@ func getDevicePCIAddressImpl(pciID string) (string, error) {
 	// We do not pass devices as multifunction, hence the trailing 0 in the address.
 	pciDeviceAddr := fmt.Sprintf("%s:%s.0", bus, deviceID)
 
-	bridgeDevicePCIAddr := fmt.Sprintf("%s/%s", pciBridgeAddr, pciDeviceAddr)
-	agentLog.WithField("completePCIAddr", bridgeDevicePCIAddr).Info("Fetched PCI address for device")
+	sysfsRelPath := fmt.Sprintf("%s/%s", pciBridgeAddr, pciDeviceAddr)
+	agentLog.WithField("sysfsRelPath", sysfsRelPath).Info("Fetched sysfs relative path for PCI device")
 
-	return bridgeDevicePCIAddr, nil
+	return sysfsRelPath, nil
 }
 
 func getDeviceName(s *sandbox, devID string) (string, error) {
@@ -181,21 +183,21 @@ func getDeviceName(s *sandbox, devID string) (string, error) {
 	return filepath.Join(systemDevPath, devName), nil
 }
 
-func getPCIDeviceNameImpl(s *sandbox, pciID string) (string, error) {
-	pciAddr, err := getDevicePCIAddress(pciID)
+func getPCIDeviceNameImpl(s *sandbox, pciPath string) (string, error) {
+	sysfsRelPath, err := pciPathToSysfs(pciPath)
 	if err != nil {
 		return "", err
 	}
 
-	fieldLogger := agentLog.WithField("pciAddr", pciAddr)
+	fieldLogger := agentLog.WithField("sysfsRelPath", sysfsRelPath)
 
 	// Rescan pci bus if we need to wait for a new pci device
 	if err = rescanPciBus(); err != nil {
 		fieldLogger.WithError(err).Error("Failed to scan pci bus")
 		return "", err
 	}
 
-	return getDeviceName(s, pciAddr)
+	return getDeviceName(s, sysfsRelPath)
 }
 
 // device.Id should be the predicted device name (vda, vdb, ...)

device_test.go

@@ -92,47 +92,47 @@ func TestVirtioBlkDeviceHandlerEmptyLinuxDevicesSpecFailure(t *testing.T) {
 	testVirtioBlkDeviceHandlerFailure(t, device, spec)
 }
 
-func TestGetPCIAddress(t *testing.T) {
+func TestPciPathToSysfs(t *testing.T) {
 	testDir, err := ioutil.TempDir("", "kata-agent-tmp-")
 	if err != nil {
 		t.Fatal(t, err)
 	}
 	defer os.RemoveAll(testDir)
 
-	pciID := "02"
-	_, err = getDevicePCIAddress(pciID)
+	pciPath := "02"
+	_, err = pciPathToSysfs(pciPath)
 	assert.NotNil(t, err)
 
-	pciID = "02/03/04"
-	_, err = getDevicePCIAddress(pciID)
+	pciPath = "02/03/04"
+	_, err = pciPathToSysfs(pciPath)
 	assert.NotNil(t, err)
 
 	bridgeID := "02"
 	deviceID := "03"
 	pciBus := "0000:01"
-	expectedPCIAddress := "0000:00:02.0/0000:01:03.0"
-	pciID = fmt.Sprintf("%s/%s", bridgeID, deviceID)
+	expectedRelPath := "0000:00:02.0/0000:01:03.0"
+	pciPath = fmt.Sprintf("%s/%s", bridgeID, deviceID)
 
 	// Set sysBusPrefix to test directory for unit tests.
 	sysBusPrefix = testDir
 	bridgeBusPath := fmt.Sprintf(pciBusPathFormat, sysBusPrefix, "0000:00:02.0")
 
-	_, err = getDevicePCIAddress(pciID)
+	_, err = pciPathToSysfs(pciPath)
 	assert.NotNil(t, err)
 
 	err = os.MkdirAll(bridgeBusPath, mountPerm)
 	assert.Nil(t, err)
 
-	_, err = getDevicePCIAddress(pciID)
+	_, err = pciPathToSysfs(pciPath)
 	assert.NotNil(t, err)
 
 	err = os.MkdirAll(filepath.Join(bridgeBusPath, pciBus), mountPerm)
 	assert.Nil(t, err)
 
-	addr, err := getDevicePCIAddress(pciID)
+	addr, err := pciPathToSysfs(pciPath)
 	assert.Nil(t, err)
 
-	assert.Equal(t, addr, expectedPCIAddress)
+	assert.Equal(t, addr, expectedRelPath)
 }
 
 func TestScanSCSIBus(t *testing.T) {
@@ -804,12 +804,12 @@ func TestGetPCIDeviceName(t *testing.T) {
 
 	sysfsDir = testSysfsDir
 
-	savedFunc := getDevicePCIAddress
+	savedFunc := pciPathToSysfs
 	defer func() {
-		getDevicePCIAddress = savedFunc
+		pciPathToSysfs = savedFunc
 	}()
 
-	getDevicePCIAddress = func(pciID string) (string, error) {
+	pciPathToSysfs = func(pciPath string) (string, error) {
 		return "", nil
 	}