Skip to content

Commit

Permalink
device: Do not rescan PCI bus
Browse files Browse the repository at this point in the history
PCI bus rescan code was added long time ago in Clear Containers due to lack of
ACPI support in QEMU 2.9 + q35 [1]. Now this code is messing up PCIe hotplug
in Kata Containers. A workaround to this issue is the "lazy attach"
mechanism [2] that hotplugs LBS (Large BAR space) devices after re-scanning the
PCI bus, unfourtunately some non-LBS devices are being affected too, for
instance SR-IOV devices. It would not make sense to lazy-attach non-LBS
devices because kata will end up lazy-attaching all the devices, having said
that, the PCI bus rescan code and the "lazy attach" mechanism should be removed

fixes kata-containers#781
fixes kata-containers/runtime#2664

[1]: clearcontainers/agent#139
[2]: kata-containers/runtime#2461

Signed-off-by: Julio Montes <julio.montes@intel.com>
  • Loading branch information
Julio Montes committed May 7, 2020
1 parent f481f6b commit 5b1ddf6
Show file tree
Hide file tree
Showing 3 changed files with 0 additions and 64 deletions.
12 changes: 0 additions & 12 deletions device.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,10 +80,6 @@ var deviceHandlerList = map[string]deviceHandler{
driverNvdimmType: nvdimmDeviceHandler,
}

func rescanPciBus() error {
return ioutil.WriteFile(pciBusRescanFile, []byte{'1'}, pciBusMode)
}

// getDevicePCIAddress fetches the complete PCI address in sysfs, based on the PCI
// identifier provided. This should be in the format: "bridgeAddr/deviceAddr".
// Here, bridgeAddr is the address at which the brige is attached on the root bus,
Expand Down Expand Up @@ -178,14 +174,6 @@ func getPCIDeviceNameImpl(s *sandbox, pciID string) (string, error) {
return "", err
}

fieldLogger := agentLog.WithField("pciAddr", pciAddr)

// Rescan pci bus if we need to wait for a new pci device
if err = rescanPciBus(); err != nil {
fieldLogger.WithError(err).Error("Failed to scan pci bus")
return "", err
}

return getDeviceName(s, pciAddr)
}

Expand Down
46 changes: 0 additions & 46 deletions device_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -503,45 +503,6 @@ func TestUpdateSpecDeviceList(t *testing.T) {
assert.NoError(err)
}

func TestRescanPciBus(t *testing.T) {
skipUnlessRoot(t)

assert := assert.New(t)

err := rescanPciBus()
assert.Nil(err)

}

func TestRescanPciBusSubverted(t *testing.T) {
assert := assert.New(t)

dir, err := ioutil.TempDir("", "")
assert.NoError(err)
defer os.RemoveAll(dir)

rescanDir := filepath.Join(dir, "rescan-dir")

err = os.MkdirAll(rescanDir, testDirMode)
assert.NoError(err)

rescan := filepath.Join(rescanDir, "rescan")

savedFile := pciBusRescanFile
defer func() {
pciBusRescanFile = savedFile
}()

pciBusRescanFile = rescan

err = rescanPciBus()
assert.NoError(err)

os.RemoveAll(rescanDir)
err = rescanPciBus()
assert.Error(err)
}

func TestVirtioMmioBlkDeviceHandler(t *testing.T) {
assert := assert.New(t)

Expand Down Expand Up @@ -634,13 +595,6 @@ func TestGetPCIDeviceName(t *testing.T) {

_, err = getPCIDeviceNameImpl(&sb, "")
assert.Error(err)

rescanDir := filepath.Dir(pciBusRescanFile)
err = os.MkdirAll(rescanDir, testDirMode)
assert.NoError(err)

_, err = getPCIDeviceNameImpl(&sb, "")
assert.Error(err)
}

func TestGetSCSIDevPath(t *testing.T) {
Expand Down
6 changes: 0 additions & 6 deletions grpc.go
Original file line number Diff line number Diff line change
Expand Up @@ -611,12 +611,6 @@ func (a *agentGRPC) CreateContainer(ctx context.Context, req *pb.CreateContainer
return emptyResp, err
}

// re-scan PCI bus
// looking for hidden devices
if err = rescanPciBus(); err != nil {
agentLog.WithError(err).Warn("Could not rescan PCI bus")
}

// Some devices need some extra processing (the ones invoked with
// --device for instance), and that's what this call is doing. It
// updates the devices listed in the OCI spec, so that they actually
Expand Down

0 comments on commit 5b1ddf6

Please sign in to comment.