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 <david@gibson.dropbear.id.au>
kata-containers · Oct 27, 2020 · 3596058 · 3596058
1 parent 64751f3
commit 3596058
Show file tree

Hide file tree

Showing 6 changed files with 24 additions and 15 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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