From f91a8b8e192d92881e89561185c5b7be20ef42d3 Mon Sep 17 00:00:00 2001 From: Archana Shinde Date: Thu, 26 Apr 2018 15:33:19 -0700 Subject: [PATCH] block: Get rid of device prediction for Storage as well Similar to Device, use PCI information for determining device name for BlkStorage handler as well. Fixes #198 Signed-off-by: Archana Shinde --- grpc.go | 4 ++-- mount.go | 25 +++++++++++-------------- mount_test.go | 48 ++++++++++++++++++++++++++++++++++++++++-------- 3 files changed, 53 insertions(+), 24 deletions(-) diff --git a/grpc.go b/grpc.go index 3f8b75c8b9..e7470b1c5d 100644 --- a/grpc.go +++ b/grpc.go @@ -439,7 +439,7 @@ func (a *agentGRPC) CreateContainer(ctx context.Context, req *pb.CreateContainer // After all those storages have been processed, no matter the order // here, the agent will rely on libcontainer (using the oci.Mounts // list) to bind mount all of them inside the container. - mountList, err := addStorages(req.Storages) + mountList, err := addStorages(req.Storages, a.sandbox) if err != nil { return emptyResp, err } @@ -862,7 +862,7 @@ func (a *agentGRPC) CreateSandbox(ctx context.Context, req *pb.CreateSandboxRequ } } - mountList, err := addStorages(req.Storages) + mountList, err := addStorages(req.Storages, a.sandbox) if err != nil { return emptyResp, err } diff --git a/mount.go b/mount.go index 157a882836..3cd71b0764 100644 --- a/mount.go +++ b/mount.go @@ -13,7 +13,6 @@ import ( "strings" "syscall" - "github.com/kata-containers/agent/pkg/uevent" pb "github.com/kata-containers/agent/protocols/grpc" "github.com/sirupsen/logrus" "golang.org/x/sys/unix" @@ -184,7 +183,7 @@ func removeMounts(mounts []string) error { // storageHandler is the type of callback to be defined to handle every // type of storage driver. -type storageHandler func(storage pb.Storage) (string, error) +type storageHandler func(storage pb.Storage, s *sandbox) (string, error) // storageHandlerList lists the supported drivers. var storageHandlerList = map[string]storageHandler{ @@ -194,27 +193,25 @@ var storageHandlerList = map[string]storageHandler{ } // virtio9pStorageHandler handles the storage for 9p driver. -func virtio9pStorageHandler(storage pb.Storage) (string, error) { +func virtio9pStorageHandler(storage pb.Storage, s *sandbox) (string, error) { return commonStorageHandler(storage) } // virtioBlkStorageHandler handles the storage for blk driver. -func virtioBlkStorageHandler(storage pb.Storage) (string, error) { - // First need to make sure the expected device shows up properly. - devName := strings.TrimPrefix(storage.Source, devPrefix) - checkUevent := func(uEv *uevent.Uevent) bool { - return (uEv.Action == "add" && - filepath.Base(uEv.DevPath) == devName) - } - if err := waitForDevice(storage.Source, devName, checkUevent); err != nil { +func virtioBlkStorageHandler(storage pb.Storage, s *sandbox) (string, error) { + // Get the device node path based on the PCI address provided + // in Storage Source + devPath, err := getBlockDeviceNodeName(s, storage.Source) + if err != nil { return "", err } + storage.Source = devPath return commonStorageHandler(storage) } // virtioSCSIStorageHandler handles the storage for scsi driver. -func virtioSCSIStorageHandler(storage pb.Storage) (string, error) { +func virtioSCSIStorageHandler(storage pb.Storage, s *sandbox) (string, error) { // Retrieve the device path from SCSI address. devPath, err := getSCSIDevPath(storage.Source) if err != nil { @@ -248,7 +245,7 @@ func mountStorage(storage pb.Storage) error { // associated operations such as waiting for the device to show up, and mount // it to a specific location, according to the type of handler chosen, and for // each storage. -func addStorages(storages []*pb.Storage) ([]string, error) { +func addStorages(storages []*pb.Storage, s *sandbox) ([]string, error) { var mountList []string for _, storage := range storages { @@ -262,7 +259,7 @@ func addStorages(storages []*pb.Storage) ([]string, error) { "Unknown storage driver %q", storage.Driver) } - mountPoint, err := devHandler(*storage) + mountPoint, err := devHandler(*storage, s) if err != nil { return nil, err } diff --git a/mount_test.go b/mount_test.go index 2ae8cbc0bf..9c3b683407 100644 --- a/mount_test.go +++ b/mount_test.go @@ -43,35 +43,67 @@ func TestVirtio9pStorageHandlerSuccessful(t *testing.T) { storage.Fstype = "bind" storage.Options = []string{"rbind"} - _, err = virtio9pStorageHandler(storage) + _, err = virtio9pStorageHandler(storage, &sandbox{}) assert.Nil(t, err, "storage9pDriverHandler() failed: %v", err) } func TestVirtioBlkStorageHandlerSuccessful(t *testing.T) { skipUnlessRoot(t) + testDir, err := ioutil.TempDir("", "kata-agent-tmp-") + if err != nil { + t.Fatal(t, err) + } + + bridgeID := "02" + deviceID := "03" + pciBus := "0000:01" + completePCIAddr := fmt.Sprintf("0000:00:%s.0/%s:%s.0", bridgeID, pciBus, deviceID) + + pciID := fmt.Sprintf("%s/%s", bridgeID, deviceID) + + sysBusPrefix = testDir + bridgeBusPath := fmt.Sprintf(pciBusPathFormat, sysBusPrefix, "0000:00:02.0") + + err = os.MkdirAll(filepath.Join(bridgeBusPath, pciBus), mountPerm) + assert.Nil(t, err) + devPath, err := createFakeDevicePath() if err != nil { t.Fatal(err) } defer os.RemoveAll(devPath) - storage, err := createSafeAndFakeStorage() + dirPath, err := ioutil.TempDir("", "fake-dir") if err != nil { t.Fatal(err) } - defer os.RemoveAll(storage.Source) + defer os.RemoveAll(dirPath) + + storage := pb.Storage{ + Source: pciID, + MountPoint: filepath.Join(dirPath, "test-mount"), + } defer syscall.Unmount(storage.MountPoint, 0) + s := &sandbox{ + pciDeviceMap: make(map[string]string), + } + + s.Lock() + s.pciDeviceMap[completePCIAddr] = devPath + s.Unlock() + storage.Fstype = "bind" storage.Options = []string{"rbind"} - _, err = virtioBlkStorageHandler(storage) + systemDevPath = "" + _, err = virtioBlkStorageHandler(storage, s) assert.Nil(t, err, "storageBlockStorageDriverHandler() failed: %v", err) } func testAddStoragesSuccessful(t *testing.T, storages []*pb.Storage) { - _, err := addStorages(storages) + _, err := addStorages(storages, &sandbox{}) assert.Nil(t, err, "addStorages() failed: %v", err) } @@ -89,11 +121,11 @@ func TestAddStoragesNilStoragesSuccessful(t *testing.T) { testAddStoragesSuccessful(t, storages) } -func noopStorageHandlerReturnNil(storage pb.Storage) (string, error) { +func noopStorageHandlerReturnNil(storage pb.Storage, s *sandbox) (string, error) { return "", nil } -func noopStorageHandlerReturnError(storage pb.Storage) (string, error) { +func noopStorageHandlerReturnError(storage pb.Storage, s *sandbox) (string, error) { return "", fmt.Errorf("Noop handler failure") } @@ -113,7 +145,7 @@ func TestAddStoragesNoopHandlerSuccessful(t *testing.T) { } func testAddStoragesFailure(t *testing.T, storages []*pb.Storage) { - _, err := addStorages(storages) + _, err := addStorages(storages, &sandbox{}) assert.NotNil(t, err, "addStorages() should have failed") }