diff --git a/virtcontainers/container.go b/virtcontainers/container.go index 4ba7ee2e3a..c976de437a 100644 --- a/virtcontainers/container.go +++ b/virtcontainers/container.go @@ -885,38 +885,9 @@ func (c *Container) create() (err error) { } } - var ( - machineType = c.sandbox.config.HypervisorConfig.HypervisorMachineType - normalAttachedDevs []ContainerDevice //for q35: normally attached devices - delayAttachedDevs []ContainerDevice //for q35: delay attached devices, for example, large bar space device - ) - // Fix: https://github.com/kata-containers/runtime/issues/2460 - if machineType == QemuQ35 { - // add Large Bar space device to delayAttachedDevs - for _, device := range c.devices { - var isLargeBarSpace bool - isLargeBarSpace, err = manager.IsVFIOLargeBarSpaceDevice(device.ContainerPath) - if err != nil { - return - } - if isLargeBarSpace { - delayAttachedDevs = append(delayAttachedDevs, device) - } else { - normalAttachedDevs = append(normalAttachedDevs, device) - } - } - } else { - normalAttachedDevs = c.devices - } - - c.Logger().WithFields(logrus.Fields{ - "machine_type": machineType, - "devices": normalAttachedDevs, - }).Info("normal attach devices") - if len(normalAttachedDevs) > 0 { - if err = c.attachDevices(normalAttachedDevs); err != nil { - return - } + // Attach devices + if err = c.attachDevices(); err != nil { + return } // Deduce additional system mount info that should be handled by the agent @@ -929,17 +900,6 @@ func (c *Container) create() (err error) { } c.process = *process - // lazy attach device after createContainer for q35 - if machineType == QemuQ35 && len(delayAttachedDevs) > 0 { - c.Logger().WithFields(logrus.Fields{ - "machine_type": machineType, - "devices": delayAttachedDevs, - }).Info("lazy attach devices") - if err = c.attachDevices(delayAttachedDevs); err != nil { - return - } - } - if !rootless.IsRootless() && !c.sandbox.config.SandboxCgroupOnly { if err = c.cgroupsCreate(); err != nil { return @@ -1428,15 +1388,11 @@ func (c *Container) removeDrive() (err error) { return nil } -func (c *Container) attachDevices(devices []ContainerDevice) error { +func (c *Container) attachDevices() error { // there's no need to do rollback when error happens, // because if attachDevices fails, container creation will fail too, // and rollbackFailingContainerCreation could do all the rollbacks - - // since devices with large bar space require delayed attachment, - // the devices need to be split into two lists, normalAttachedDevs and delayAttachedDevs. - // so c.device is not used here. See issue https://github.com/kata-containers/runtime/issues/2460. - for _, dev := range devices { + for _, dev := range c.devices { if err := c.sandbox.devManager.AttachDevice(dev.ID, c.sandbox); err != nil { return err } diff --git a/virtcontainers/device/drivers/utils.go b/virtcontainers/device/drivers/utils.go index ad046ca10b..f3b338c61a 100644 --- a/virtcontainers/device/drivers/utils.go +++ b/virtcontainers/device/drivers/utils.go @@ -106,19 +106,3 @@ func readPCIProperty(propertyPath string) (string, error) { } return strings.Split(string(buf), "\n")[0], nil } - -func GetVFIODeviceType(deviceFileName string) config.VFIODeviceType { - //For example, 0000:04:00.0 - tokens := strings.Split(deviceFileName, ":") - vfioDeviceType := config.VFIODeviceErrorType - if len(tokens) == 3 { - vfioDeviceType = config.VFIODeviceNormalType - } else { - //For example, 83b8f4f2-509f-382f-3c1e-e6bfe0fa1001 - tokens = strings.Split(deviceFileName, "-") - if len(tokens) == 5 { - vfioDeviceType = config.VFIODeviceMediatedType - } - } - return vfioDeviceType -} diff --git a/virtcontainers/device/drivers/vfio.go b/virtcontainers/device/drivers/vfio.go index 7bed197aad..c39ea76b62 100644 --- a/virtcontainers/device/drivers/vfio.go +++ b/virtcontainers/device/drivers/vfio.go @@ -187,8 +187,18 @@ func (device *VFIODevice) Load(ds persistapi.DeviceState) { // It should implement GetAttachCount() and DeviceID() as api.Device implementation // here it shares function from *GenericDevice so we don't need duplicate codes + func getVFIODetails(deviceFileName, iommuDevicesPath string) (deviceBDF, deviceSysfsDev string, vfioDeviceType config.VFIODeviceType, err error) { - vfioDeviceType = GetVFIODeviceType(deviceFileName) + tokens := strings.Split(deviceFileName, ":") + vfioDeviceType = config.VFIODeviceErrorType + if len(tokens) == 3 { + vfioDeviceType = config.VFIODeviceNormalType + } else { + tokens = strings.Split(deviceFileName, "-") + if len(tokens) == 5 { + vfioDeviceType = config.VFIODeviceMediatedType + } + } switch vfioDeviceType { case config.VFIODeviceNormalType: diff --git a/virtcontainers/device/manager/manager.go b/virtcontainers/device/manager/manager.go index 6a2b665a5e..8c61f60869 100644 --- a/virtcontainers/device/manager/manager.go +++ b/virtcontainers/device/manager/manager.go @@ -78,8 +78,6 @@ func NewDeviceManager(blockDriver string, vhostUserStoreEnabled bool, vhostUserS dm.blockDriver = VirtioSCSI } - drivers.AllPCIeDevs = make(map[string]bool) - for _, dev := range devices { dm.devices[dev.DeviceID()] = dev } diff --git a/virtcontainers/device/manager/utils.go b/virtcontainers/device/manager/utils.go index 7a437f958b..48bb40e41b 100644 --- a/virtcontainers/device/manager/utils.go +++ b/virtcontainers/device/manager/utils.go @@ -7,16 +7,10 @@ package manager import ( - "fmt" - "io/ioutil" "path/filepath" - "strconv" "strings" - "github.com/sirupsen/logrus" - "github.com/kata-containers/runtime/virtcontainers/device/config" - "github.com/kata-containers/runtime/virtcontainers/device/drivers" ) const ( @@ -42,101 +36,6 @@ func isBlock(devInfo config.DeviceInfo) bool { return devInfo.DevType == "b" } -// IsVFIOLargeBarSpaceDevice checks if the device is a large bar space device. -func IsVFIOLargeBarSpaceDevice(hostPath string) (bool, error) { - if !isVFIO(hostPath) { - return false, nil - } - - iommuDevicesPath := filepath.Join(config.SysIOMMUPath, filepath.Base(hostPath), "devices") - deviceFiles, err := ioutil.ReadDir(iommuDevicesPath) - if err != nil { - return false, err - } - - // Pass all devices in iommu group - for _, deviceFile := range deviceFiles { - vfioDeviceType := drivers.GetVFIODeviceType(deviceFile.Name()) - var isLarge bool - switch vfioDeviceType { - case config.VFIODeviceNormalType: - sysfsResource := filepath.Join(iommuDevicesPath, deviceFile.Name(), "resource") - if isLarge, err = isLargeBarSpace(sysfsResource); err != nil { - return false, err - } - deviceLogger().WithFields(logrus.Fields{ - "device-file": deviceFile.Name(), - "device-type": vfioDeviceType, - "resource": sysfsResource, - "large-bar-space": isLarge, - }).Info("Detect large bar space device") - return isLarge, nil - case config.VFIODeviceMediatedType: - //TODO: support VFIODeviceMediatedType - deviceLogger().WithFields(logrus.Fields{ - "device-file": deviceFile.Name(), - "device-type": vfioDeviceType, - }).Warn("Detect large bar space device is not yet supported for VFIODeviceMediatedType") - default: - deviceLogger().WithFields(logrus.Fields{ - "device-file": deviceFile.Name(), - "device-type": vfioDeviceType, - }).Warn("Incorrect token found when detecting large bar space devices") - } - } - - return false, nil -} - -func isLargeBarSpace(resourcePath string) (bool, error) { - buf, err := ioutil.ReadFile(resourcePath) - if err != nil { - return false, fmt.Errorf("failed to read sysfs resource: %v", err) - } - - // The resource file contains host addresses of PCI resources: - // For example: - // $ cat /sys/bus/pci/devices/0000:04:00.0/resource - // 0x00000000c6000000 0x00000000c6ffffff 0x0000000000040200 - // 0x0000383800000000 0x0000383bffffffff 0x000000000014220c - // Refer: - // resource format: https://github.com/torvalds/linux/blob/63623fd44972d1ed2bfb6e0fb631dfcf547fd1e7/drivers/pci/pci-sysfs.c#L145 - // calculate size : https://github.com/pciutils/pciutils/blob/61ecc14a327de030336f1ff3fea9c7e7e55a90ca/lspci.c#L388 - for rIdx, line := range strings.Split(string(buf), "\n") { - cols := strings.Fields(line) - // start and end columns are required to calculate the size - if len(cols) < 2 { - deviceLogger().WithField("resource-line", line).Debug("not enough columns to calculate PCI size") - continue - } - start, _ := strconv.ParseUint(cols[0], 0, 64) - end, _ := strconv.ParseUint(cols[1], 0, 64) - if start > end { - deviceLogger().WithFields(logrus.Fields{ - "start": start, - "end": end, - }).Debug("start is greater than end") - continue - } - // Use right shift to convert Bytes to GBytes - // This is equivalent to ((end - start + 1) / 1024 / 1024 / 1024) - gbSize := (end - start + 1) >> 30 - deviceLogger().WithFields(logrus.Fields{ - "resource": resourcePath, - "region": rIdx, - "start": cols[0], - "end": cols[1], - "gb-size": gbSize, - }).Debug("Check large bar space device") - //size is large than 4G - if gbSize > 4 { - return true, nil - } - } - - return false, nil -} - // isVhostUserBlk checks if the device is a VhostUserBlk device. func isVhostUserBlk(devInfo config.DeviceInfo) bool { return devInfo.Major == config.VhostUserBlkMajor diff --git a/virtcontainers/device/manager/utils_test.go b/virtcontainers/device/manager/utils_test.go index 5c1ab643e3..33c650b622 100644 --- a/virtcontainers/device/manager/utils_test.go +++ b/virtcontainers/device/manager/utils_test.go @@ -7,8 +7,6 @@ package manager import ( - "io/ioutil" - "os" "testing" "github.com/kata-containers/runtime/virtcontainers/device/config" @@ -91,46 +89,3 @@ func TestIsVhostUserSCSI(t *testing.T) { assert.Equal(t, d.expected, isVhostUserSCSI) } } - -func TestIsLargeBarSpace(t *testing.T) { - assert := assert.New(t) - - // File not exist - bs, err := isLargeBarSpace("/abc/xyz/123/rgb") - assert.Error(err) - assert.False(bs) - - f, err := ioutil.TempFile("", "pci") - assert.NoError(err) - defer f.Close() - defer os.RemoveAll(f.Name()) - - type testData struct { - resourceInfo string - error bool - result bool - } - - for _, d := range []testData{ - {"", false, false}, - {"\t\n\t ", false, false}, - {"abc zyx", false, false}, - {"abc zyx rgb", false, false}, - {"abc\t zyx \trgb", false, false}, - {"0x00015\n0x0013", false, false}, - {"0x00000000c6000000 0x00000000c6ffffff 0x0000000000040200", false, false}, - {"0x0000383bffffffff 0x0000383800000000", false, false}, // start greater than end - {"0x0000383800000000 0x0000383bffffffff", false, true}, - {"0x0000383800000000 0x0000383bffffffff 0x000000000014220c", false, true}, - } { - f.WriteAt([]byte(d.resourceInfo), 0) - bs, err = isLargeBarSpace(f.Name()) - assert.NoError(f.Truncate(0)) - if d.error { - assert.Error(err, d.resourceInfo) - } else { - assert.NoError(err, d.resourceInfo) - } - assert.Equal(d.result, bs, d.resourceInfo) - } -} diff --git a/virtcontainers/sandbox_test.go b/virtcontainers/sandbox_test.go index 85c712e8a9..88fec160ec 100644 --- a/virtcontainers/sandbox_test.go +++ b/virtcontainers/sandbox_test.go @@ -732,7 +732,7 @@ func TestSandboxAttachDevicesVFIO(t *testing.T) { containers[c.id].sandbox = &sandbox - err = containers[c.id].attachDevices(c.devices) + err = containers[c.id].attachDevices() assert.Nil(t, err, "Error while attaching devices %s", err) err = containers[c.id].detachDevices() @@ -827,7 +827,7 @@ func TestSandboxAttachDevicesVhostUserBlk(t *testing.T) { containers[c.id].sandbox = &sandbox - err = containers[c.id].attachDevices(c.devices) + err = containers[c.id].attachDevices() assert.Nil(t, err, "Error while attaching vhost-user-blk devices %s", err) err = containers[c.id].detachDevices()