diff --git a/src/agent/src/device.rs b/src/agent/src/device.rs index 28f503b37..45c969a69 100644 --- a/src/agent/src/device.rs +++ b/src/agent/src/device.rs @@ -332,14 +332,9 @@ async fn virtio_blk_device_handler( devidx: &DevIndex, ) -> Result<()> { let mut dev = device.clone(); + let pcipath = pci::Path::from_str(&device.id)?; - // When "Id (PCI path)" is not set, we allow to use the predicted - // "VmPath" passed from kata-runtime Note this is a special code - // path for cloud-hypervisor when BDF information is not available - if !device.id.is_empty() { - let pcipath = pci::Path::from_str(&device.id)?; - dev.vm_path = get_virtio_blk_pci_device_name(sandbox, &pcipath).await?; - } + dev.vm_path = get_virtio_blk_pci_device_name(sandbox, &pcipath).await?; update_spec_device_list(&dev, spec, devidx) } diff --git a/src/runtime/virtcontainers/clh.go b/src/runtime/virtcontainers/clh.go index fcf346808..482f3c70c 100644 --- a/src/runtime/virtcontainers/clh.go +++ b/src/runtime/virtcontainers/clh.go @@ -427,6 +427,27 @@ func clhDriveIndexToID(i int) string { return "clh_drive_" + strconv.Itoa(i) } +// Various cloud-hypervisor APIs report a PCI address in "BB:DD.F" +// form within the PciDeviceInfo struct. This is a broken API, +// because there's no way clh can reliably know the guest side bdf for +// a device, since the bus number depends on how the guest firmware +// and/or kernel enumerates it. They get away with it only because +// they don't use bridges, and so the bus is always 0. Under that +// assumption convert a clh PciDeviceInfo into a PCI path +func clhPciInfoToPath(pciInfo chclient.PciDeviceInfo) (vcTypes.PciPath, error) { + tokens := strings.Split(pciInfo.Bdf, ":") + if len(tokens) != 3 || tokens[0] != "0000" || tokens[1] != "00" { + return vcTypes.PciPath{}, fmt.Errorf("Unexpected PCI address %q from clh hotplug", pciInfo.Bdf) + } + + tokens = strings.Split(tokens[2], ".") + if len(tokens) != 2 || tokens[1] != "0" || len(tokens[0]) != 2 { + return vcTypes.PciPath{}, fmt.Errorf("Unexpected PCI address %q from clh hotplug", pciInfo.Bdf) + } + + return vcTypes.PciPathFromString(tokens[0]) +} + func (clh *cloudHypervisor) hotplugAddBlockDevice(drive *config.BlockDrive) error { if clh.config.BlockDeviceDriver != config.VirtioBlock { return fmt.Errorf("incorrect hypervisor configuration on 'block_device_driver':"+ @@ -441,24 +462,24 @@ func (clh *cloudHypervisor) hotplugAddBlockDevice(drive *config.BlockDrive) erro driveID := clhDriveIndexToID(drive.Index) - //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") - } else { - blkDevice := chclient.DiskConfig{ - Path: drive.File, - Readonly: drive.ReadOnly, - VhostUser: false, - Id: driveID, - } - _, _, err = cl.VmAddDiskPut(ctx, blkDevice) + return fmt.Errorf("pmem device hotplug not supported") } + blkDevice := chclient.DiskConfig{ + Path: drive.File, + Readonly: drive.ReadOnly, + VhostUser: false, + Id: driveID, + } + pciInfo, _, err := cl.VmAddDiskPut(ctx, blkDevice) + if err != nil { - err = fmt.Errorf("failed to hotplug block device %+v %s", drive, openAPIClientError(err)) + return fmt.Errorf("failed to hotplug block device %+v %s", drive, openAPIClientError(err)) } + + drive.PCIPath, err = clhPciInfoToPath(pciInfo) + return err } diff --git a/src/runtime/virtcontainers/clh_test.go b/src/runtime/virtcontainers/clh_test.go index 8950f9b78..cd2f6b21b 100644 --- a/src/runtime/virtcontainers/clh_test.go +++ b/src/runtime/virtcontainers/clh_test.go @@ -101,7 +101,7 @@ func (c *clhClientMock) VmAddDevicePut(ctx context.Context, vmAddDevice chclient //nolint:golint func (c *clhClientMock) VmAddDiskPut(ctx context.Context, diskConfig chclient.DiskConfig) (chclient.PciDeviceInfo, *http.Response, error) { - return chclient.PciDeviceInfo{}, nil, nil + return chclient.PciDeviceInfo{Bdf: "0000:00:0a.0"}, nil, nil } //nolint:golint diff --git a/src/runtime/virtcontainers/kata_agent.go b/src/runtime/virtcontainers/kata_agent.go index 6a36a3613..665d6c6c8 100644 --- a/src/runtime/virtcontainers/kata_agent.go +++ b/src/runtime/virtcontainers/kata_agent.go @@ -1228,12 +1228,7 @@ func (k *kataAgent) buildContainerRootfs(ctx context.Context, sandbox *Sandbox, rootfs.Source = blockDrive.DevNo case sandbox.config.HypervisorConfig.BlockDeviceDriver == config.VirtioBlock: rootfs.Driver = kataBlkDevType - if blockDrive.PCIPath.IsNil() { - rootfs.Source = blockDrive.VirtPath - } else { - rootfs.Source = blockDrive.PCIPath.String() - } - + rootfs.Source = blockDrive.PCIPath.String() case sandbox.config.HypervisorConfig.BlockDeviceDriver == config.VirtioSCSI: rootfs.Driver = kataSCSIDevType rootfs.Source = blockDrive.SCSIAddr @@ -1490,11 +1485,7 @@ func (k *kataAgent) handleDeviceBlockVolume(c *Container, m Mount, device api.De vol.Source = blockDrive.DevNo case c.sandbox.config.HypervisorConfig.BlockDeviceDriver == config.VirtioBlock: vol.Driver = kataBlkDevType - if blockDrive.PCIPath.IsNil() { - vol.Source = blockDrive.VirtPath - } else { - vol.Source = blockDrive.PCIPath.String() - } + vol.Source = blockDrive.PCIPath.String() case c.sandbox.config.HypervisorConfig.BlockDeviceDriver == config.VirtioMmio: vol.Driver = kataMmioBlkDevType vol.Source = blockDrive.VirtPath diff --git a/src/runtime/virtcontainers/kata_agent_test.go b/src/runtime/virtcontainers/kata_agent_test.go index b216d5aef..523ef5b32 100644 --- a/src/runtime/virtcontainers/kata_agent_test.go +++ b/src/runtime/virtcontainers/kata_agent_test.go @@ -281,18 +281,6 @@ func TestHandleDeviceBlockVolume(t *testing.T) { Source: testPCIPath.String(), }, }, - { - BlockDeviceDriver: config.VirtioBlock, - inputDev: &drivers.BlockDevice{ - BlockDrive: &config.BlockDrive{ - VirtPath: testVirtPath, - }, - }, - resultVol: &pb.Storage{ - Driver: kataBlkDevType, - Source: testVirtPath, - }, - }, { BlockDeviceDriver: config.VirtioMmio, inputDev: &drivers.BlockDevice{