device: Introduce PciPath type, name things consistently

pciPathToSysfs 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 create a newtype
wrapper for strings in this format, and just describe the internals 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.  It also trivially changes the
copyright notice, because Travis whinges otherwise.

Signed-off-by: David Gibson <[email protected]>

device.go

@@ -79,6 +79,17 @@ type devIndexEntry struct {
 }
 type devIndex map[string]devIndexEntry
 
+// Guest-side PCI path, identifies a PCI device by where it sits in
+// the PCI topology.
+//
+// Has 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
+type PciPath struct {
+	path string
+}
+
 type deviceHandler func(ctx context.Context, device pb.Device, spec *pb.Spec, s *sandbox, devIdx devIndex) error
 
 var deviceHandlerList = map[string]deviceHandler{
@@ -95,12 +106,9 @@ func rescanPciBus() error {
 
 // 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, "/")
+// provided.
+func pciPathToSysfsImpl(pciPath PciPath) (string, error) {
+	tokens := strings.Split(pciPath.path, "/")
 
 	if len(tokens) != 2 {
 		return "", fmt.Errorf("PCI path for device should be of format [bridgeAddr/deviceAddr], got %q", pciPath)
@@ -183,7 +191,7 @@ func getDeviceName(s *sandbox, devID string) (string, error) {
 	return filepath.Join(systemDevPath, devName), nil
 }
 
-func getPCIDeviceNameImpl(s *sandbox, pciPath string) (string, error) {
+func getPCIDeviceNameImpl(s *sandbox, pciPath PciPath) (string, error) {
 	sysfsRelPath, err := pciPathToSysfs(pciPath)
 	if err != nil {
 		return "", err
@@ -225,14 +233,11 @@ func virtioBlkCCWDeviceHandler(ctx context.Context, device pb.Device, spec *pb.S
 	return updateSpecDeviceList(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 (see type PciPath)
 func virtioBlkDeviceHandler(_ context.Context, device pb.Device, spec *pb.Spec, s *sandbox, devIdx devIndex) error {
-	// When "Id (PCIAddr)" is not set, we allow to use the predicted "VmPath" passed from kata-runtime
+	// When "Id" (PCI path) is not set, we allow to use the predicted "VmPath" passed from kata-runtime
 	if device.Id != "" {
-		// Get the device node path based on the PCI device address
-		devPath, err := getPCIDeviceName(s, device.Id)
+		devPath, err := getPCIDeviceName(s, PciPath{device.Id})
 		if err != nil {
 			return err
 		}

device_test.go

@@ -51,7 +51,7 @@ func testVirtioBlkDeviceHandlerFailure(t *testing.T, device pb.Device, spec *pb.
 	assert.NotNil(t, err, "blockDeviceHandler() should have failed")
 
 	savedFunc := getPCIDeviceName
-	getPCIDeviceName = func(s *sandbox, pciID string) (string, error) {
+	getPCIDeviceName = func(s *sandbox, pciPath PciPath) (string, error) {
 		return "foo", nil
 	}
 
@@ -99,19 +99,19 @@ func TestPciPathToSysfs(t *testing.T) {
 	}
 	defer os.RemoveAll(testDir)
 
-	pciPath := "02"
+	pciPath := PciPath{"02"}
 	_, err = pciPathToSysfs(pciPath)
 	assert.NotNil(t, err)
 
-	pciPath = "02/03/04"
+	pciPath = PciPath{"02/03/04"}
 	_, err = pciPathToSysfs(pciPath)
 	assert.NotNil(t, err)
 
 	bridgeID := "02"
 	deviceID := "03"
 	pciBus := "0000:01"
 	expectedRelPath := "0000:00:02.0/0000:01:03.0"
-	pciPath = fmt.Sprintf("%s/%s", bridgeID, deviceID)
+	pciPath = PciPath{fmt.Sprintf("%s/%s", bridgeID, deviceID)}
 
 	// Set sysBusPrefix to test directory for unit tests.
 	sysBusPrefix = testDir
@@ -809,22 +809,22 @@ func TestGetPCIDeviceName(t *testing.T) {
 		pciPathToSysfs = savedFunc
 	}()
 
-	pciPathToSysfs = func(pciPath string) (string, error) {
+	pciPathToSysfs = func(pciPath PciPath) (string, error) {
 		return "", nil
 	}
 
 	sb := sandbox{
 		deviceWatchers: make(map[string](chan string)),
 	}
 
-	_, err = getPCIDeviceNameImpl(&sb, "")
+	_, err = getPCIDeviceNameImpl(&sb, PciPath{""})
 	assert.Error(err)
 
 	rescanDir := filepath.Dir(pciBusRescanFile)
 	err = os.MkdirAll(rescanDir, testDirMode)
 	assert.NoError(err)
 
-	_, err = getPCIDeviceNameImpl(&sb, "")
+	_, err = getPCIDeviceNameImpl(&sb, PciPath{""})
 	assert.Error(err)
 }
 

mount.go

@@ -297,8 +297,8 @@ func virtioFSStorageHandler(_ context.Context, storage pb.Storage, s *sandbox) (
 // virtioBlkStorageHandler handles the storage for blk driver.
 func virtioBlkStorageHandler(_ context.Context, storage pb.Storage, s *sandbox) (string, error) {
 
-	// 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 else use the virt path provided in Storage Source
 	if strings.HasPrefix(storage.Source, "/dev") {
 
 		FileInfo, err := os.Stat(storage.Source)
@@ -312,7 +312,7 @@ func virtioBlkStorageHandler(_ context.Context, storage pb.Storage, s *sandbox)
 		}
 
 	} else {
-		devPath, err := getPCIDeviceName(s, storage.Source)
+		devPath, err := getPCIDeviceName(s, PciPath{storage.Source})
 		if err != nil {
 			return "", err
 		}

mount_test.go

@@ -219,9 +219,9 @@ func TestVirtioBlkStorageHandlerSuccessful(t *testing.T) {
 	bridgeID := "02"
 	deviceID := "03"
 	pciBus := "0000:01"
-	completePCIAddr := fmt.Sprintf("0000:00:%s.0/%s:%s.0", bridgeID, pciBus, deviceID)
+	sysRelPath := fmt.Sprintf("0000:00:%s.0/%s:%s.0", bridgeID, pciBus, deviceID)
 
-	pciID := fmt.Sprintf("%s/%s", bridgeID, deviceID)
+	pciPath := fmt.Sprintf("%s/%s", bridgeID, deviceID)
 
 	sysBusPrefix = testDir
 	bridgeBusPath := fmt.Sprintf(pciBusPathFormat, sysBusPrefix, "0000:00:02.0")
@@ -242,7 +242,7 @@ func TestVirtioBlkStorageHandlerSuccessful(t *testing.T) {
 	defer os.RemoveAll(dirPath)
 
 	storage := pb.Storage{
-		Source:     pciID,
+		Source:     pciPath,
 		MountPoint: filepath.Join(dirPath, "test-mount"),
 	}
 	defer syscall.Unmount(storage.MountPoint, 0)
@@ -252,7 +252,7 @@ func TestVirtioBlkStorageHandlerSuccessful(t *testing.T) {
 	}
 
 	s.Lock()
-	s.sysToDevMap[completePCIAddr] = devPath
+	s.sysToDevMap[sysRelPath] = devPath
 	s.Unlock()
 
 	storage.Fstype = "bind"

network.go

@@ -232,14 +232,13 @@ func (s *sandbox) updateInterface(netHandle *netlink.Handle, iface *types.Interf
 	fieldLogger := agentLog.WithFields(logrus.Fields{
 		"mac-address":    iface.HwAddr,
 		"interface-name": iface.Device,
-		"pci-address":    iface.PciAddr,
+		"pci-path":       iface.PciPath,
 	})
 
-	// If the PCI address of the network device is provided, wait/check for the device
-	// to be available first
-	if iface.PciAddr != "" {
-		// iface.PciAddr is in the format bridgeAddr/deviceAddr eg. 05/06
-		_, err := getPCIDeviceName(s, iface.PciAddr)
+	// If the PCI path of the network device is provided,
+	// wait/check for the device to be available first
+	if iface.PciPath != "" {
+		_, err := getPCIDeviceName(s, PciPath{iface.PciPath})
 		if err != nil {
 			return nil, err
 		}

pkg/types/types.proto

@@ -1,5 +1,5 @@
 //
-// Copyright 2018 Intel Corporation.
+// Copyright (c) 2018 Intel Corporation.
 //
 // SPDX-License-Identifier: Apache-2.0
 //
@@ -26,10 +26,9 @@ 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;
+	// pciPath for the device (see type PciPath 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