From 64751f377b4bc380da8b3b74cd9d2e963d213e4e Mon Sep 17 00:00:00 2001 From: David Gibson Date: Thu, 8 Oct 2020 13:22:39 +1100 Subject: [PATCH] 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