From 927487c14216b90866db9db97986556193521e8a Mon Sep 17 00:00:00 2001 From: Sebastien Boeuf Date: Fri, 27 Jul 2018 09:18:29 -0700 Subject: [PATCH] revert: "virtcontainers: support pre-add storage for frakti" This PR got merged while it had some issues with some shim processes being left behind after k8s testing. And because those issues were real issues introduced by this PR (not some random failures), now the master branch is broken and new pull requests cannot get the CI passing. That's the reason why this commit revert the changes introduced by this PR so that we can fix the master branch. Fixes #529 Signed-off-by: Sebastien Boeuf --- virtcontainers/container.go | 116 ++++---------- virtcontainers/device/api/interface.go | 32 ++-- .../device/api/mockDeviceReceiver.go | 4 +- virtcontainers/device/config/config.go | 46 +----- virtcontainers/device/drivers/block.go | 103 +++++++----- virtcontainers/device/drivers/generic.go | 33 +--- virtcontainers/device/drivers/vfio.go | 78 +++------ virtcontainers/device/drivers/vhost_user.go | 29 ++++ .../device/drivers/vhost_user_blk.go | 64 ++------ .../device/drivers/vhost_user_net.go | 65 ++------ .../device/drivers/vhost_user_scsi.go | 64 ++------ virtcontainers/device/manager/manager.go | 148 +++--------------- virtcontainers/device/manager/manager_test.go | 63 ++------ virtcontainers/filesystem.go | 85 ++-------- virtcontainers/filesystem_test.go | 2 +- virtcontainers/hyperstart_agent.go | 54 +++---- virtcontainers/kata_agent.go | 52 ++---- virtcontainers/kata_agent_test.go | 44 ++---- virtcontainers/mount.go | 6 +- virtcontainers/network.go | 10 +- virtcontainers/qemu.go | 29 ++-- virtcontainers/qemu_arch_base.go | 41 ++--- virtcontainers/qemu_arch_base_test.go | 16 +- virtcontainers/qemu_ppc64le.go | 4 +- virtcontainers/sandbox.go | 60 ++----- virtcontainers/sandbox_test.go | 24 +-- 26 files changed, 382 insertions(+), 890 deletions(-) create mode 100644 virtcontainers/device/drivers/vhost_user.go diff --git a/virtcontainers/container.go b/virtcontainers/container.go index 1e6257cc1c..941b7d3968 100644 --- a/virtcontainers/container.go +++ b/virtcontainers/container.go @@ -20,8 +20,9 @@ import ( "github.com/sirupsen/logrus" "golang.org/x/sys/unix" + "github.com/kata-containers/runtime/virtcontainers/device/api" "github.com/kata-containers/runtime/virtcontainers/device/config" - "github.com/kata-containers/runtime/virtcontainers/device/manager" + "github.com/kata-containers/runtime/virtcontainers/device/drivers" "github.com/kata-containers/runtime/virtcontainers/utils" ) @@ -228,14 +229,6 @@ type SystemMountsInfo struct { DevShmSize uint } -// ContainerDevice describes a device associated with container -type ContainerDevice struct { - // ID is device id referencing the device from sandbox's device manager - ID string - // ContainerPath is device path displayed in container - ContainerPath string -} - // Container is composed of a set of containers and a runtime environment. // A Container can be created, deleted, started, stopped, listed, entered, paused and restored. type Container struct { @@ -258,7 +251,7 @@ type Container struct { mounts []Mount - devices []ContainerDevice + devices []api.Device systemMountsInfo SystemMountsInfo } @@ -369,7 +362,7 @@ func (c *Container) storeDevices() error { return c.sandbox.storage.storeContainerDevices(c.sandboxID, c.id, c.devices) } -func (c *Container) fetchDevices() ([]ContainerDevice, error) { +func (c *Container) fetchDevices() ([]api.Device, error) { return c.sandbox.storage.fetchContainerDevices(c.sandboxID, c.id) } @@ -437,19 +430,30 @@ func (c *Container) mountSharedDirMounts(hostSharedDir, guestSharedDir string) ( continue } + var stat unix.Stat_t + if err := unix.Stat(m.Source, &stat); err != nil { + return nil, err + } + // Check if mount is a block device file. If it is, the block device will be attached to the host // instead of passing this as a shared mount. - if len(m.BlockDeviceID) > 0 { - // Attach this block device, all other devices passed in the config have been attached at this point - if err := c.sandbox.devManager.AttachDevice(m.BlockDeviceID, c.sandbox); err != nil && - err != manager.ErrDeviceAttached { - return nil, err + if c.checkBlockDeviceSupport() && stat.Mode&unix.S_IFBLK == unix.S_IFBLK { + // TODO: remove dependency of package drivers + b := &drivers.BlockDevice{ + DevType: config.DeviceBlock, + DeviceInfo: config.DeviceInfo{ + HostPath: m.Source, + ContainerPath: m.Destination, + DevType: "b", + }, } - if err := c.sandbox.storeSandboxDevices(); err != nil { - //TODO: roll back? + // Attach this block device, all other devices passed in the config have been attached at this point + if err := b.Attach(c.sandbox); err != nil { return nil, err } + + c.mounts[idx].BlockDevice = b continue } @@ -562,38 +566,6 @@ func newContainer(sandbox *Sandbox, contConfig ContainerConfig) (*Container, err c.mounts = mounts } - // iterate all mounts and create block device if it's block based. - for i, m := range c.mounts { - if len(m.BlockDeviceID) > 0 || m.Type != "bind" { - // Non-empty m.BlockDeviceID indicates there's already one device - // associated with the mount,so no need to create a new device for it - // and we only create block device for bind mount - continue - } - - var stat unix.Stat_t - if err := unix.Stat(m.Source, &stat); err != nil { - return nil, fmt.Errorf("stat %q failed: %v", m.Source, err) - } - - // Check if mount is a block device file. If it is, the block device will be attached to the host - // instead of passing this as a shared mount. - if c.checkBlockDeviceSupport() && stat.Mode&unix.S_IFBLK == unix.S_IFBLK { - b, err := c.sandbox.devManager.NewDevice(config.DeviceInfo{ - HostPath: m.Source, - ContainerPath: m.Destination, - DevType: "b", - Major: int64(unix.Major(stat.Rdev)), - Minor: int64(unix.Minor(stat.Rdev)), - }) - if err != nil { - return nil, fmt.Errorf("device manager failed to create new device for %q: %v", m.Source, err) - } - - c.mounts[i].BlockDeviceID = b.DeviceID() - } - } - // Devices will be found in storage after create stage has completed. // We fetch devices from storage at all other stages. storedDevices, err := c.fetchDevices() @@ -602,19 +574,13 @@ func newContainer(sandbox *Sandbox, contConfig ContainerConfig) (*Container, err } else { // If devices were not found in storage, create Device implementations // from the configuration. This should happen at create. - for _, info := range contConfig.DeviceInfos { - dev, err := sandbox.devManager.NewDevice(info) - if err != nil { - return &Container{}, err - } - c.devices = append(c.devices, ContainerDevice{ - ID: dev.DeviceID(), - ContainerPath: info.ContainerPath, - }) + devices, err := sandbox.devManager.NewDevices(contConfig.DeviceInfos) + if err != nil { + return &Container{}, err } + c.devices = devices } - return c, nil } @@ -1054,10 +1020,9 @@ func (c *Container) hotplugDrive() error { return err } - // TODO: use general device manager instead of BlockDrive directly // Add drive with id as container id devID := utils.MakeNameID("drive", c.id, maxDevIDSize) - drive := config.BlockDrive{ + drive := drivers.Drive{ File: devicePath, Format: "raw", ID: devID, @@ -1094,7 +1059,7 @@ func (c *Container) removeDrive() (err error) { c.Logger().Info("unplugging block device") devID := utils.MakeNameID("drive", c.id, maxDevIDSize) - drive := &config.BlockDrive{ + drive := &drivers.Drive{ ID: devID, } @@ -1111,39 +1076,22 @@ func (c *Container) removeDrive() (err 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 - for _, dev := range c.devices { - if err := c.sandbox.devManager.AttachDevice(dev.ID, c.sandbox); err != nil { - if err == manager.ErrDeviceAttached { - // skip if device is already attached before - continue - } + for _, device := range c.devices { + if err := device.Attach(c.sandbox); err != nil { return err } } - if err := c.sandbox.storeSandboxDevices(); err != nil { - return err - } return nil } func (c *Container) detachDevices() error { - for _, dev := range c.devices { - if err := c.sandbox.devManager.DetachDevice(dev.ID, c.sandbox); err != nil { - if err == manager.ErrDeviceNotAttached { - // skip if device isn't attached - continue - } + for _, device := range c.devices { + if err := device.Detach(c.sandbox); err != nil { return err } } - if err := c.sandbox.storeSandboxDevices(); err != nil { - return err - } return nil } diff --git a/virtcontainers/device/api/interface.go b/virtcontainers/device/api/interface.go index a4874b40d5..d6a4072d33 100644 --- a/virtcontainers/device/api/interface.go +++ b/virtcontainers/device/api/interface.go @@ -27,7 +27,6 @@ func DeviceLogger() *logrus.Entry { // DeviceReceiver is an interface used for accepting devices // a device should be attached/added/plugged to a DeviceReceiver type DeviceReceiver interface { - // these are for hotplug/hot-unplug devices to/from hypervisor HotplugAddDevice(Device, config.DeviceType) error HotplugRemoveDevice(Device, config.DeviceType) error @@ -35,35 +34,28 @@ type DeviceReceiver interface { GetAndSetSandboxBlockIndex() (int, error) DecrementSandboxBlockIndex() error - // this is for appending device to hypervisor boot params - AppendDevice(Device) error + // this is for vhost_user devices + AddVhostUserDevice(VhostUserDevice, config.DeviceType) error +} + +// VhostUserDevice represents a vhost-user device. Shared +// attributes of a vhost-user device can be retrieved using +// the Attrs() method. Unique data can be obtained by casting +// the object to the proper type. +type VhostUserDevice interface { + Attrs() *config.VhostUserDeviceAttrs + Type() config.DeviceType } // Device is the virtcontainers device interface. type Device interface { Attach(DeviceReceiver) error Detach(DeviceReceiver) error - // ID returns device identifier - DeviceID() string - // DeviceType indicates which kind of device it is - // e.g. block, vfio or vhost user DeviceType() config.DeviceType - // GetDeviceInfo returns device specific data used for hotplugging by hypervisor - // Caller could cast the return value to device specific struct - // e.g. Block device returns *config.BlockDrive and - // vfio device returns []*config.VFIODev - GetDeviceInfo() interface{} - // IsAttached checks if the device is attached - IsAttached() bool } // DeviceManager can be used to create a new device, this can be used as single // device management object. type DeviceManager interface { - NewDevice(config.DeviceInfo) (Device, error) - AttachDevice(string, DeviceReceiver) error - DetachDevice(string, DeviceReceiver) error - IsDeviceAttached(string) bool - GetDeviceByID(string) Device - GetAllDevices() []Device + NewDevices(devInfos []config.DeviceInfo) ([]Device, error) } diff --git a/virtcontainers/device/api/mockDeviceReceiver.go b/virtcontainers/device/api/mockDeviceReceiver.go index c080d072d9..b5d2472c3c 100644 --- a/virtcontainers/device/api/mockDeviceReceiver.go +++ b/virtcontainers/device/api/mockDeviceReceiver.go @@ -32,7 +32,7 @@ func (mockDC *MockDeviceReceiver) DecrementSandboxBlockIndex() error { return nil } -// AppendDevice adds new vhost user device -func (mockDC *MockDeviceReceiver) AppendDevice(Device) error { +// AddVhostUserDevice adds new vhost user device +func (mockDC *MockDeviceReceiver) AddVhostUserDevice(VhostUserDevice, config.DeviceType) error { return nil } diff --git a/virtcontainers/device/config/config.go b/virtcontainers/device/config/config.go index d9c3dfe934..1ab83ddc91 100644 --- a/virtcontainers/device/config/config.go +++ b/virtcontainers/device/config/config.go @@ -49,10 +49,10 @@ var SysIOMMUPath = "/sys/kernel/iommu_groups" // DeviceInfo is an embedded type that contains device data common to all types of devices. type DeviceInfo struct { - // Hostpath is device path on host + // Device path on host HostPath string - // ContainerPath is device path inside container + // Device path inside the container ContainerPath string // Type of device: c, b, u or p @@ -87,48 +87,12 @@ type DeviceInfo struct { DriverOptions map[string]string } -// BlockDrive represents a block storage drive which may be used in case the storage -// driver has an underlying block storage device. -type BlockDrive struct { - // File is the path to the disk-image/device which will be used with this drive - File string - - // Format of the drive - Format string - - // ID is used to identify this drive in the hypervisor options. - ID string - - // Index assigned to the drive. In case of virtio-scsi, this is used as SCSI LUN index - Index int - - // PCIAddr is the PCI address used to identify the slot at which the drive is attached. - PCIAddr string - - // SCSI Address of the block device, in case the device is attached using SCSI driver - // SCSI address is in the format SCSI-Id:LUN - SCSIAddr string - - // VirtPath at which the device appears inside the VM, outside of the container mount namespace - VirtPath string -} - -// VFIODev represents a VFIO drive used for hotplugging -type VFIODev struct { - // ID is used to identify this drive in the hypervisor options. - ID string - // BDF (Bus:Device.Function) of the PCI address - BDF string -} - // VhostUserDeviceAttrs represents data shared by most vhost-user devices type VhostUserDeviceAttrs struct { - ID string + DevType DeviceType + DeviceInfo DeviceInfo SocketPath string - Type DeviceType - - // MacAddress is only meaningful for vhost user net device - MacAddress string + ID string } // GetHostPathFunc is function pointer used to mock GetHostPath in tests. diff --git a/virtcontainers/device/drivers/block.go b/virtcontainers/device/drivers/block.go index 38e5a4b64b..175dc8cea2 100644 --- a/virtcontainers/device/drivers/block.go +++ b/virtcontainers/device/drivers/block.go @@ -7,6 +7,7 @@ package drivers import ( + "encoding/hex" "path/filepath" "github.com/kata-containers/runtime/virtcontainers/device/api" @@ -16,17 +17,48 @@ import ( const maxDevIDSize = 31 +// Drive represents a block storage drive which may be used in case the storage +// driver has an underlying block storage device. +type Drive struct { + + // Path to the disk-image/device which will be used with this drive + File string + + // Format of the drive + Format string + + // ID is used to identify this drive in the hypervisor options. + ID string + + // Index assigned to the drive. In case of virtio-scsi, this is used as SCSI LUN index + Index int + + // PCIAddr is the PCI address used to identify the slot at which the drive is attached. + PCIAddr string +} + // BlockDevice refers to a block storage device implementation. type BlockDevice struct { - ID string - DeviceInfo *config.DeviceInfo - BlockDrive *config.BlockDrive + DevType config.DeviceType + DeviceInfo config.DeviceInfo + + // SCSI Address of the block device, in case the device is attached using SCSI driver + // SCSI address is in the format SCSI-Id:LUN + SCSIAddr string + + // Path at which the device appears inside the VM, outside of the container mount namespace + VirtPath string + + // PCI Slot of the block device + PCIAddr string + + BlockDrive *Drive } // NewBlockDevice creates a new block device based on DeviceInfo -func NewBlockDevice(devInfo *config.DeviceInfo) *BlockDevice { +func NewBlockDevice(devInfo config.DeviceInfo) *BlockDevice { return &BlockDevice{ - ID: devInfo.ID, + DevType: config.DeviceBlock, DeviceInfo: devInfo, } } @@ -34,10 +66,13 @@ func NewBlockDevice(devInfo *config.DeviceInfo) *BlockDevice { // Attach is standard interface of api.Device, it's used to add device to some // DeviceReceiver func (device *BlockDevice) Attach(devReceiver api.DeviceReceiver) (err error) { - if device.DeviceInfo.Hotplugged { - return nil + randBytes, err := utils.GenerateRandomBytes(8) + if err != nil { + return err } + device.DeviceInfo.ID = hex.EncodeToString(randBytes) + // Increment the block index for the sandbox. This is used to determine the name // for the block device in the case where the block device is used as container // rootfs and the predicted block device name needs to be provided to the agent. @@ -53,13 +88,21 @@ func (device *BlockDevice) Attach(devReceiver api.DeviceReceiver) (err error) { return err } - drive := &config.BlockDrive{ + drive := Drive{ File: device.DeviceInfo.HostPath, Format: "raw", ID: utils.MakeNameID("drive", device.DeviceInfo.ID, maxDevIDSize), Index: index, } + deviceLogger().WithField("device", device.DeviceInfo.HostPath).Info("Attaching block device") + device.BlockDrive = &drive + if err = devReceiver.HotplugAddDevice(device, config.DeviceBlock); err != nil { + return err + } + + device.DeviceInfo.Hotplugged = true + driveName, err := utils.GetVirtDriveName(index) if err != nil { return err @@ -67,60 +110,36 @@ func (device *BlockDevice) Attach(devReceiver api.DeviceReceiver) (err error) { customOptions := device.DeviceInfo.DriverOptions if customOptions != nil && customOptions["block-driver"] == "virtio-blk" { - drive.VirtPath = filepath.Join("/dev", driveName) + device.VirtPath = filepath.Join("/dev", driveName) + device.PCIAddr = drive.PCIAddr } else { scsiAddr, err := utils.GetSCSIAddress(index) if err != nil { return err } - drive.SCSIAddr = scsiAddr - } - - deviceLogger().WithField("device", device.DeviceInfo.HostPath).Info("Attaching block device") - device.BlockDrive = drive - if err = devReceiver.HotplugAddDevice(device, config.DeviceBlock); err != nil { - return err + device.SCSIAddr = scsiAddr } - device.DeviceInfo.Hotplugged = true - return nil } // Detach is standard interface of api.Device, it's used to remove device from some // DeviceReceiver func (device *BlockDevice) Detach(devReceiver api.DeviceReceiver) error { - if !device.DeviceInfo.Hotplugged { - return nil - } + if device.DeviceInfo.Hotplugged { + deviceLogger().WithField("device", device.DeviceInfo.HostPath).Info("Unplugging block device") - deviceLogger().WithField("device", device.DeviceInfo.HostPath).Info("Unplugging block device") + if err := devReceiver.HotplugRemoveDevice(device, config.DeviceBlock); err != nil { + deviceLogger().WithError(err).Error("Failed to unplug block device") + return err + } - if err := devReceiver.HotplugRemoveDevice(device, config.DeviceBlock); err != nil { - deviceLogger().WithError(err).Error("Failed to unplug block device") - return err } - device.DeviceInfo.Hotplugged = false return nil } -// IsAttached checks if the device is attached -func (device *BlockDevice) IsAttached() bool { - return device.DeviceInfo.Hotplugged -} - // DeviceType is standard interface of api.Device, it returns device type func (device *BlockDevice) DeviceType() config.DeviceType { - return config.DeviceBlock -} - -// DeviceID returns device ID -func (device *BlockDevice) DeviceID() string { - return device.ID -} - -// GetDeviceInfo returns device information used for creating -func (device *BlockDevice) GetDeviceInfo() interface{} { - return device.BlockDrive + return device.DevType } diff --git a/virtcontainers/device/drivers/generic.go b/virtcontainers/device/drivers/generic.go index 431d178439..9609d811f5 100644 --- a/virtcontainers/device/drivers/generic.go +++ b/virtcontainers/device/drivers/generic.go @@ -13,52 +13,29 @@ import ( // GenericDevice refers to a device that is neither a VFIO device or block device. type GenericDevice struct { - ID string - DeviceInfo *config.DeviceInfo + DevType config.DeviceType + DeviceInfo config.DeviceInfo } // NewGenericDevice creates a new GenericDevice -func NewGenericDevice(devInfo *config.DeviceInfo) *GenericDevice { +func NewGenericDevice(devInfo config.DeviceInfo) *GenericDevice { return &GenericDevice{ - ID: devInfo.ID, + DevType: config.DeviceGeneric, DeviceInfo: devInfo, } } // Attach is standard interface of api.Device func (device *GenericDevice) Attach(devReceiver api.DeviceReceiver) error { - if device.DeviceInfo.Hotplugged { - return nil - } - return nil } // Detach is standard interface of api.Device func (device *GenericDevice) Detach(devReceiver api.DeviceReceiver) error { - if !device.DeviceInfo.Hotplugged { - return nil - } - return nil } -// IsAttached checks if the device is attached -func (device *GenericDevice) IsAttached() bool { - return device.DeviceInfo.Hotplugged -} - -// DeviceID returns device ID -func (device *GenericDevice) DeviceID() string { - return device.ID -} - // DeviceType is standard interface of api.Device, it returns device type func (device *GenericDevice) DeviceType() config.DeviceType { - return config.DeviceGeneric -} - -// GetDeviceInfo returns device information used for creating -func (device *GenericDevice) GetDeviceInfo() interface{} { - return device.DeviceInfo + return device.DevType } diff --git a/virtcontainers/device/drivers/vfio.go b/virtcontainers/device/drivers/vfio.go index c5d7037452..4243465a30 100644 --- a/virtcontainers/device/drivers/vfio.go +++ b/virtcontainers/device/drivers/vfio.go @@ -7,6 +7,7 @@ package drivers import ( + "encoding/hex" "fmt" "io/ioutil" "path/filepath" @@ -30,15 +31,15 @@ const ( // VFIODevice is a vfio device meant to be passed to the hypervisor // to be used by the Virtual Machine. type VFIODevice struct { - ID string - DeviceInfo *config.DeviceInfo - vfioDevs []*config.VFIODev + DevType config.DeviceType + DeviceInfo config.DeviceInfo + BDF string } // NewVFIODevice create a new VFIO device -func NewVFIODevice(devInfo *config.DeviceInfo) *VFIODevice { +func NewVFIODevice(devInfo config.DeviceInfo) *VFIODevice { return &VFIODevice{ - ID: devInfo.ID, + DevType: config.DeviceVFIO, DeviceInfo: devInfo, } } @@ -46,10 +47,6 @@ func NewVFIODevice(devInfo *config.DeviceInfo) *VFIODevice { // Attach is standard interface of api.Device, it's used to add device to some // DeviceReceiver func (device *VFIODevice) Attach(devReceiver api.DeviceReceiver) error { - if device.DeviceInfo.Hotplugged { - return nil - } - vfioGroup := filepath.Base(device.DeviceInfo.HostPath) iommuDevicesPath := filepath.Join(config.SysIOMMUPath, vfioGroup, "devices") @@ -60,71 +57,44 @@ func (device *VFIODevice) Attach(devReceiver api.DeviceReceiver) error { // Pass all devices in iommu group for _, deviceFile := range deviceFiles { + //Get bdf of device eg 0000:00:1c.0 deviceBDF, err := getBDF(deviceFile.Name()) if err != nil { return err } - vfio := &config.VFIODev{ - ID: utils.MakeNameID("vfio", device.DeviceInfo.ID, maxDevIDSize), - BDF: deviceBDF, + + device.BDF = deviceBDF + + randBytes, err := utils.GenerateRandomBytes(8) + if err != nil { + return err } - device.vfioDevs = append(device.vfioDevs, vfio) - } + device.DeviceInfo.ID = hex.EncodeToString(randBytes) - // hotplug a VFIO device is actually hotplugging a group of iommu devices - if err := devReceiver.HotplugAddDevice(device, config.DeviceVFIO); err != nil { - deviceLogger().WithError(err).Error("Failed to add device") - return err + if err := devReceiver.HotplugAddDevice(device, config.DeviceVFIO); err != nil { + deviceLogger().WithError(err).Error("Failed to add device") + return err + } + + deviceLogger().WithFields(logrus.Fields{ + "device-group": device.DeviceInfo.HostPath, + "device-type": "vfio-passthrough", + }).Info("Device group attached") } - deviceLogger().WithFields(logrus.Fields{ - "device-group": device.DeviceInfo.HostPath, - "device-type": "vfio-passthrough", - }).Info("Device group attached") - device.DeviceInfo.Hotplugged = true return nil } // Detach is standard interface of api.Device, it's used to remove device from some // DeviceReceiver func (device *VFIODevice) Detach(devReceiver api.DeviceReceiver) error { - if !device.DeviceInfo.Hotplugged { - return nil - } - - // hotplug a VFIO device is actually hotplugging a group of iommu devices - if err := devReceiver.HotplugRemoveDevice(device, config.DeviceVFIO); err != nil { - deviceLogger().WithError(err).Error("Failed to remove device") - return err - } - - deviceLogger().WithFields(logrus.Fields{ - "device-group": device.DeviceInfo.HostPath, - "device-type": "vfio-passthrough", - }).Info("Device group detached") - device.DeviceInfo.Hotplugged = false return nil } -// IsAttached checks if the device is attached -func (device *VFIODevice) IsAttached() bool { - return device.DeviceInfo.Hotplugged -} - // DeviceType is standard interface of api.Device, it returns device type func (device *VFIODevice) DeviceType() config.DeviceType { - return config.DeviceVFIO -} - -// DeviceID returns device ID -func (device *VFIODevice) DeviceID() string { - return device.ID -} - -// GetDeviceInfo returns device information used for creating -func (device *VFIODevice) GetDeviceInfo() interface{} { - return device.vfioDevs + return device.DevType } // getBDF returns the BDF of pci device diff --git a/virtcontainers/device/drivers/vhost_user.go b/virtcontainers/device/drivers/vhost_user.go new file mode 100644 index 0000000000..0bd13d1db9 --- /dev/null +++ b/virtcontainers/device/drivers/vhost_user.go @@ -0,0 +1,29 @@ +// Copyright (c) 2017-2018 Intel Corporation +// Copyright (c) 2018 Huawei Corporation +// +// SPDX-License-Identifier: Apache-2.0 +// + +package drivers + +import ( + "encoding/hex" + + "github.com/kata-containers/runtime/virtcontainers/device/api" + "github.com/kata-containers/runtime/virtcontainers/utils" +) + +// vhostUserAttach handles the common logic among all of the vhost-user device's +// attach functions +func vhostUserAttach(device api.VhostUserDevice, devReceiver api.DeviceReceiver) error { + // generate a unique ID to be used for hypervisor commandline fields + randBytes, err := utils.GenerateRandomBytes(8) + if err != nil { + return err + } + id := hex.EncodeToString(randBytes) + + device.Attrs().ID = id + + return devReceiver.AddVhostUserDevice(device, device.Type()) +} diff --git a/virtcontainers/device/drivers/vhost_user_blk.go b/virtcontainers/device/drivers/vhost_user_blk.go index 6bb629461f..b03fedd161 100644 --- a/virtcontainers/device/drivers/vhost_user_blk.go +++ b/virtcontainers/device/drivers/vhost_user_blk.go @@ -7,17 +7,23 @@ package drivers import ( - "encoding/hex" - "github.com/kata-containers/runtime/virtcontainers/device/api" "github.com/kata-containers/runtime/virtcontainers/device/config" - "github.com/kata-containers/runtime/virtcontainers/utils" ) // VhostUserBlkDevice is a block vhost-user based device type VhostUserBlkDevice struct { config.VhostUserDeviceAttrs - DeviceInfo *config.DeviceInfo +} + +// Attrs returns the VhostUserDeviceAttrs associated with the vhost-user device +func (vhostUserBlkDevice *VhostUserBlkDevice) Attrs() *config.VhostUserDeviceAttrs { + return &vhostUserBlkDevice.VhostUserDeviceAttrs +} + +// Type returns the type associated with the vhost-user device +func (vhostUserBlkDevice *VhostUserBlkDevice) Type() config.DeviceType { + return config.VhostUserBlk } // @@ -26,57 +32,17 @@ type VhostUserBlkDevice struct { // Attach is standard interface of api.Device, it's used to add device to some // DeviceReceiver -func (device *VhostUserBlkDevice) Attach(devReceiver api.DeviceReceiver) (err error) { - if device.DeviceInfo.Hotplugged { - return nil - } - - // generate a unique ID to be used for hypervisor commandline fields - randBytes, err := utils.GenerateRandomBytes(8) - if err != nil { - return err - } - id := hex.EncodeToString(randBytes) - - device.ID = id - device.Type = device.DeviceType() - - defer func() { - if err == nil { - device.DeviceInfo.Hotplugged = true - } - }() - return devReceiver.AppendDevice(device) +func (vhostUserBlkDevice *VhostUserBlkDevice) Attach(devReceiver api.DeviceReceiver) (err error) { + return vhostUserAttach(vhostUserBlkDevice, devReceiver) } // Detach is standard interface of api.Device, it's used to remove device from some // DeviceReceiver -func (device *VhostUserBlkDevice) Detach(devReceiver api.DeviceReceiver) error { - if !device.DeviceInfo.Hotplugged { - return nil - } - - device.DeviceInfo.Hotplugged = false +func (vhostUserBlkDevice *VhostUserBlkDevice) Detach(devReceiver api.DeviceReceiver) error { return nil } -// IsAttached checks if the device is attached -func (device *VhostUserBlkDevice) IsAttached() bool { - return device.DeviceInfo.Hotplugged -} - -// DeviceID returns device ID -func (device *VhostUserBlkDevice) DeviceID() string { - return device.ID -} - // DeviceType is standard interface of api.Device, it returns device type -func (device *VhostUserBlkDevice) DeviceType() config.DeviceType { - return config.VhostUserBlk -} - -// GetDeviceInfo returns device information used for creating -func (device *VhostUserBlkDevice) GetDeviceInfo() interface{} { - device.Type = device.DeviceType() - return &device.VhostUserDeviceAttrs +func (vhostUserBlkDevice *VhostUserBlkDevice) DeviceType() config.DeviceType { + return vhostUserBlkDevice.DevType } diff --git a/virtcontainers/device/drivers/vhost_user_net.go b/virtcontainers/device/drivers/vhost_user_net.go index 6a731542ce..7bf7517b9c 100644 --- a/virtcontainers/device/drivers/vhost_user_net.go +++ b/virtcontainers/device/drivers/vhost_user_net.go @@ -7,17 +7,24 @@ package drivers import ( - "encoding/hex" - "github.com/kata-containers/runtime/virtcontainers/device/api" "github.com/kata-containers/runtime/virtcontainers/device/config" - "github.com/kata-containers/runtime/virtcontainers/utils" ) // VhostUserNetDevice is a network vhost-user based device type VhostUserNetDevice struct { config.VhostUserDeviceAttrs - DeviceInfo *config.DeviceInfo + MacAddress string +} + +// Attrs returns the VhostUserDeviceAttrs associated with the vhost-user device +func (vhostUserNetDevice *VhostUserNetDevice) Attrs() *config.VhostUserDeviceAttrs { + return &vhostUserNetDevice.VhostUserDeviceAttrs +} + +// Type returns the type associated with the vhost-user device +func (vhostUserNetDevice *VhostUserNetDevice) Type() config.DeviceType { + return config.VhostUserNet } // @@ -26,57 +33,17 @@ type VhostUserNetDevice struct { // Attach is standard interface of api.Device, it's used to add device to some // DeviceReceiver -func (device *VhostUserNetDevice) Attach(devReceiver api.DeviceReceiver) (err error) { - if device.DeviceInfo.Hotplugged { - return nil - } - - // generate a unique ID to be used for hypervisor commandline fields - randBytes, err := utils.GenerateRandomBytes(8) - if err != nil { - return err - } - id := hex.EncodeToString(randBytes) - - device.ID = id - device.Type = device.DeviceType() - - defer func() { - if err == nil { - device.DeviceInfo.Hotplugged = true - } - }() - return devReceiver.AppendDevice(device) +func (vhostUserNetDevice *VhostUserNetDevice) Attach(devReceiver api.DeviceReceiver) (err error) { + return vhostUserAttach(vhostUserNetDevice, devReceiver) } // Detach is standard interface of api.Device, it's used to remove device from some // DeviceReceiver -func (device *VhostUserNetDevice) Detach(devReceiver api.DeviceReceiver) error { - if !device.DeviceInfo.Hotplugged { - return nil - } - - device.DeviceInfo.Hotplugged = false +func (vhostUserNetDevice *VhostUserNetDevice) Detach(devReceiver api.DeviceReceiver) error { return nil } -// IsAttached checks if the device is attached -func (device *VhostUserNetDevice) IsAttached() bool { - return device.DeviceInfo.Hotplugged -} - -// DeviceID returns device ID -func (device *VhostUserNetDevice) DeviceID() string { - return device.ID -} - // DeviceType is standard interface of api.Device, it returns device type -func (device *VhostUserNetDevice) DeviceType() config.DeviceType { - return config.VhostUserNet -} - -// GetDeviceInfo returns device information used for creating -func (device *VhostUserNetDevice) GetDeviceInfo() interface{} { - device.Type = device.DeviceType() - return &device.VhostUserDeviceAttrs +func (vhostUserNetDevice *VhostUserNetDevice) DeviceType() config.DeviceType { + return vhostUserNetDevice.DevType } diff --git a/virtcontainers/device/drivers/vhost_user_scsi.go b/virtcontainers/device/drivers/vhost_user_scsi.go index ce5f846cd5..6d485baec8 100644 --- a/virtcontainers/device/drivers/vhost_user_scsi.go +++ b/virtcontainers/device/drivers/vhost_user_scsi.go @@ -7,17 +7,23 @@ package drivers import ( - "encoding/hex" - "github.com/kata-containers/runtime/virtcontainers/device/api" "github.com/kata-containers/runtime/virtcontainers/device/config" - "github.com/kata-containers/runtime/virtcontainers/utils" ) // VhostUserSCSIDevice is a SCSI vhost-user based device type VhostUserSCSIDevice struct { config.VhostUserDeviceAttrs - DeviceInfo *config.DeviceInfo +} + +// Attrs returns the VhostUserDeviceAttrs associated with the vhost-user device +func (vhostUserSCSIDevice *VhostUserSCSIDevice) Attrs() *config.VhostUserDeviceAttrs { + return &vhostUserSCSIDevice.VhostUserDeviceAttrs +} + +// Type returns the type associated with the vhost-user device +func (vhostUserSCSIDevice *VhostUserSCSIDevice) Type() config.DeviceType { + return config.VhostUserSCSI } // @@ -26,57 +32,17 @@ type VhostUserSCSIDevice struct { // Attach is standard interface of api.Device, it's used to add device to some // DeviceReceiver -func (device *VhostUserSCSIDevice) Attach(devReceiver api.DeviceReceiver) (err error) { - if device.DeviceInfo.Hotplugged { - return nil - } - - // generate a unique ID to be used for hypervisor commandline fields - randBytes, err := utils.GenerateRandomBytes(8) - if err != nil { - return err - } - id := hex.EncodeToString(randBytes) - - device.ID = id - device.Type = device.DeviceType() - - defer func() { - if err == nil { - device.DeviceInfo.Hotplugged = true - } - }() - return devReceiver.AppendDevice(device) +func (vhostUserSCSIDevice *VhostUserSCSIDevice) Attach(devReceiver api.DeviceReceiver) (err error) { + return vhostUserAttach(vhostUserSCSIDevice, devReceiver) } // Detach is standard interface of api.Device, it's used to remove device from some // DeviceReceiver -func (device *VhostUserSCSIDevice) Detach(devReceiver api.DeviceReceiver) error { - if !device.DeviceInfo.Hotplugged { - return nil - } - - device.DeviceInfo.Hotplugged = false +func (vhostUserSCSIDevice *VhostUserSCSIDevice) Detach(devReceiver api.DeviceReceiver) error { return nil } -// IsAttached checks if the device is attached -func (device *VhostUserSCSIDevice) IsAttached() bool { - return device.DeviceInfo.Hotplugged -} - -// DeviceID returns device ID -func (device *VhostUserSCSIDevice) DeviceID() string { - return device.ID -} - // DeviceType is standard interface of api.Device, it returns device type -func (device *VhostUserSCSIDevice) DeviceType() config.DeviceType { - return config.VhostUserSCSI -} - -// GetDeviceInfo returns device information used for creating -func (device *VhostUserSCSIDevice) GetDeviceInfo() interface{} { - device.Type = device.DeviceType() - return &device.VhostUserDeviceAttrs +func (vhostUserSCSIDevice *VhostUserSCSIDevice) DeviceType() config.DeviceType { + return vhostUserSCSIDevice.DevType } diff --git a/virtcontainers/device/manager/manager.go b/virtcontainers/device/manager/manager.go index e14a53d266..0e30e6e5b2 100644 --- a/virtcontainers/device/manager/manager.go +++ b/virtcontainers/device/manager/manager.go @@ -7,16 +7,11 @@ package manager import ( - "encoding/hex" - "errors" - "sync" - "github.com/sirupsen/logrus" "github.com/kata-containers/runtime/virtcontainers/device/api" "github.com/kata-containers/runtime/virtcontainers/device/config" "github.com/kata-containers/runtime/virtcontainers/device/drivers" - "github.com/kata-containers/runtime/virtcontainers/utils" ) const ( @@ -26,162 +21,59 @@ const ( VirtioSCSI string = "virtio-scsi" ) -var ( - // ErrIDExhausted represents that devices are too many - // and no more IDs can be generated - ErrIDExhausted = errors.New("IDs are exhausted") - // ErrDeviceNotExist represents device hasn't been created before - ErrDeviceNotExist = errors.New("device with specified ID hasn't been created") - // ErrDeviceAttached represents the device is already attached - ErrDeviceAttached = errors.New("device is already attached") - // ErrDeviceNotAttached represents the device isn't attached - ErrDeviceNotAttached = errors.New("device isn't attached") -) - type deviceManager struct { blockDriver string - - devices map[string]api.Device - sync.RWMutex } func deviceLogger() *logrus.Entry { return api.DeviceLogger().WithField("subsystem", "device") } -// NewDeviceManager creates a deviceManager object behaved as api.DeviceManager -func NewDeviceManager(blockDriver string, devices []api.Device) api.DeviceManager { - dm := &deviceManager{ - devices: make(map[string]api.Device), - } - if blockDriver == VirtioBlock { - dm.blockDriver = VirtioBlock - } else { - dm.blockDriver = VirtioSCSI - } - - for _, dev := range devices { - dm.devices[dev.DeviceID()] = dev - } - return dm -} - // createDevice creates one device based on DeviceInfo func (dm *deviceManager) createDevice(devInfo config.DeviceInfo) (api.Device, error) { path, err := config.GetHostPathFunc(devInfo) if err != nil { return nil, err } - devInfo.HostPath = path - // device ID must be generated by manager instead of device itself - // in case of ID collision - if devInfo.ID, err = dm.newDeviceID(); err != nil { - return nil, err - } + devInfo.HostPath = path if isVFIO(path) { - return drivers.NewVFIODevice(&devInfo), nil + return drivers.NewVFIODevice(devInfo), nil } else if isBlock(devInfo) { if devInfo.DriverOptions == nil { devInfo.DriverOptions = make(map[string]string) } devInfo.DriverOptions["block-driver"] = dm.blockDriver - return drivers.NewBlockDevice(&devInfo), nil + return drivers.NewBlockDevice(devInfo), nil } else { deviceLogger().WithField("device", path).Info("Device has not been passed to the container") - return drivers.NewGenericDevice(&devInfo), nil + return drivers.NewGenericDevice(devInfo), nil } } -// NewDevice creates bundles of devices based on array of DeviceInfo -func (dm *deviceManager) NewDevice(devInfo config.DeviceInfo) (api.Device, error) { - dm.Lock() - defer dm.Unlock() - dev, err := dm.createDevice(devInfo) - if err == nil { - dm.devices[dev.DeviceID()] = dev - } - return dev, err -} +// NewDevices creates bundles of devices based on array of DeviceInfo +func (dm *deviceManager) NewDevices(devInfos []config.DeviceInfo) ([]api.Device, error) { + var devices []api.Device -func (dm *deviceManager) newDeviceID() (string, error) { - for i := 0; i < 5; i++ { - // generate an random ID - randBytes, err := utils.GenerateRandomBytes(8) + for _, devInfo := range devInfos { + device, err := dm.createDevice(devInfo) if err != nil { - return "", err - } - id := hex.EncodeToString(randBytes) - - // check ID collision, choose another one if ID is in use - if _, ok := dm.devices[id]; !ok { - return id, nil + return nil, err } - } - return "", ErrIDExhausted -} - -func (dm *deviceManager) AttachDevice(id string, dr api.DeviceReceiver) error { - dm.Lock() - defer dm.Unlock() - - d, ok := dm.devices[id] - if !ok { - return ErrDeviceNotExist - } - - if d.IsAttached() { - return ErrDeviceAttached - } - - if err := d.Attach(dr); err != nil { - return err - } - return nil -} - -func (dm *deviceManager) DetachDevice(id string, dr api.DeviceReceiver) error { - dm.Lock() - defer dm.Unlock() - - d, ok := dm.devices[id] - if !ok { - return ErrDeviceNotExist - } - if !d.IsAttached() { - return ErrDeviceNotAttached + devices = append(devices, device) } - if err := d.Detach(dr); err != nil { - return err - } - return nil -} -func (dm *deviceManager) GetDeviceByID(id string) api.Device { - dm.RLock() - defer dm.RUnlock() - if d, ok := dm.devices[id]; ok { - return d - } - return nil + return devices, nil } -func (dm *deviceManager) GetAllDevices() []api.Device { - dm.RLock() - defer dm.RUnlock() - devices := []api.Device{} - for _, v := range dm.devices { - devices = append(devices, v) +// NewDeviceManager creates a deviceManager object behaved as api.DeviceManager +func NewDeviceManager(blockDriver string) api.DeviceManager { + dm := &deviceManager{} + if blockDriver == VirtioBlock { + dm.blockDriver = VirtioBlock + } else { + dm.blockDriver = VirtioSCSI } - return devices -} -func (dm *deviceManager) IsDeviceAttached(id string) bool { - dm.RLock() - defer dm.RUnlock() - d, ok := dm.devices[id] - if !ok { - return false - } - return d.IsAttached() + return dm } diff --git a/virtcontainers/device/manager/manager_test.go b/virtcontainers/device/manager/manager_test.go index 2818820109..3b2fc57688 100644 --- a/virtcontainers/device/manager/manager_test.go +++ b/virtcontainers/device/manager/manager_test.go @@ -25,10 +25,9 @@ const fileMode0640 = os.FileMode(0640) // dirMode is the permission bits used for creating a directory const dirMode = os.FileMode(0750) | os.ModeDir -func TestNewDevice(t *testing.T) { +func TestNewDevices(t *testing.T) { dm := &deviceManager{ blockDriver: VirtioBlock, - devices: make(map[string]api.Device), } savedSysDevPrefix := config.SysDevPrefix @@ -54,7 +53,7 @@ func TestNewDevice(t *testing.T) { DevType: "c", } - _, err = dm.NewDevice(deviceInfo) + _, err = dm.NewDevices([]config.DeviceInfo{deviceInfo}) assert.NotNil(t, err) format := strconv.FormatInt(major, 10) + ":" + strconv.FormatInt(minor, 10) @@ -63,7 +62,7 @@ func TestNewDevice(t *testing.T) { // Return true for non-existent /sys/dev path. deviceInfo.ContainerPath = path - _, err = dm.NewDevice(deviceInfo) + _, err = dm.NewDevices([]config.DeviceInfo{deviceInfo}) assert.Nil(t, err) err = os.MkdirAll(ueventPathPrefix, dirMode) @@ -74,17 +73,18 @@ func TestNewDevice(t *testing.T) { err = ioutil.WriteFile(ueventPath, content, fileMode0640) assert.Nil(t, err) - _, err = dm.NewDevice(deviceInfo) + _, err = dm.NewDevices([]config.DeviceInfo{deviceInfo}) assert.NotNil(t, err) content = []byte("MAJOR=252\nMINOR=3\nDEVNAME=vfio/2") err = ioutil.WriteFile(ueventPath, content, fileMode0640) assert.Nil(t, err) - device, err := dm.NewDevice(deviceInfo) + devices, err := dm.NewDevices([]config.DeviceInfo{deviceInfo}) assert.Nil(t, err) - vfioDev, ok := device.(*drivers.VFIODevice) + assert.Equal(t, len(devices), 1) + vfioDev, ok := devices[0].(*drivers.VFIODevice) assert.True(t, ok) assert.Equal(t, vfioDev.DeviceInfo.HostPath, path) assert.Equal(t, vfioDev.DeviceInfo.ContainerPath, path) @@ -98,7 +98,6 @@ func TestNewDevice(t *testing.T) { func TestAttachVFIODevice(t *testing.T) { dm := &deviceManager{ blockDriver: VirtioBlock, - devices: make(map[string]api.Device), } tmpDir, err := ioutil.TempDir("", "") assert.Nil(t, err) @@ -129,7 +128,7 @@ func TestAttachVFIODevice(t *testing.T) { DevType: "c", } - device, err := dm.NewDevice(deviceInfo) + device, err := dm.createDevice(deviceInfo) assert.Nil(t, err) _, ok := device.(*drivers.VFIODevice) assert.True(t, ok) @@ -145,7 +144,6 @@ func TestAttachVFIODevice(t *testing.T) { func TestAttachGenericDevice(t *testing.T) { dm := &deviceManager{ blockDriver: VirtioBlock, - devices: make(map[string]api.Device), } path := "/dev/tty2" deviceInfo := config.DeviceInfo{ @@ -154,7 +152,7 @@ func TestAttachGenericDevice(t *testing.T) { DevType: "c", } - device, err := dm.NewDevice(deviceInfo) + device, err := dm.createDevice(deviceInfo) assert.Nil(t, err) _, ok := device.(*drivers.GenericDevice) assert.True(t, ok) @@ -170,7 +168,6 @@ func TestAttachGenericDevice(t *testing.T) { func TestAttachBlockDevice(t *testing.T) { dm := &deviceManager{ blockDriver: VirtioBlock, - devices: make(map[string]api.Device), } path := "/dev/hda" deviceInfo := config.DeviceInfo{ @@ -180,7 +177,7 @@ func TestAttachBlockDevice(t *testing.T) { } devReceiver := &api.MockDeviceReceiver{} - device, err := dm.NewDevice(deviceInfo) + device, err := dm.createDevice(deviceInfo) assert.Nil(t, err) _, ok := device.(*drivers.BlockDevice) assert.True(t, ok) @@ -193,7 +190,7 @@ func TestAttachBlockDevice(t *testing.T) { // test virtio SCSI driver dm.blockDriver = VirtioSCSI - device, err = dm.NewDevice(deviceInfo) + device, err = dm.createDevice(deviceInfo) assert.Nil(t, err) err = device.Attach(devReceiver) assert.Nil(t, err) @@ -201,41 +198,3 @@ func TestAttachBlockDevice(t *testing.T) { err = device.Detach(devReceiver) assert.Nil(t, err) } - -func TestAttachDetachDevice(t *testing.T) { - dm := NewDeviceManager(VirtioSCSI, nil) - - path := "/dev/hda" - deviceInfo := config.DeviceInfo{ - HostPath: path, - ContainerPath: path, - DevType: "b", - } - - devReceiver := &api.MockDeviceReceiver{} - device, err := dm.NewDevice(deviceInfo) - assert.Nil(t, err) - - // attach device - err = dm.AttachDevice(device.DeviceID(), devReceiver) - assert.Nil(t, err) - // attach device again(twice) - err = dm.AttachDevice(device.DeviceID(), devReceiver) - assert.NotNil(t, err) - assert.Equal(t, err, ErrDeviceAttached, "attach device twice should report error %q", ErrDeviceAttached) - - attached := dm.IsDeviceAttached(device.DeviceID()) - assert.True(t, attached) - - // detach device - err = dm.DetachDevice(device.DeviceID(), devReceiver) - assert.Nil(t, err) - // detach device again(twice) - err = dm.DetachDevice(device.DeviceID(), devReceiver) - assert.NotNil(t, err) - assert.Equal(t, err, ErrDeviceNotAttached, "attach device twice should report error %q", ErrDeviceNotAttached) - - attached = dm.IsDeviceAttached(device.DeviceID()) - assert.False(t, attached) - -} diff --git a/virtcontainers/filesystem.go b/virtcontainers/filesystem.go index 139cbff1b7..f9f6071328 100644 --- a/virtcontainers/filesystem.go +++ b/virtcontainers/filesystem.go @@ -52,9 +52,6 @@ const ( // devicesFileType represents a device file type devicesFileType - - // devicesIDFileType saves reference IDs to file, e.g. device IDs - devicesIDFileType ) // configFile is the file name used for every JSON sandbox configuration. @@ -129,8 +126,6 @@ type resourceStorage interface { fetchSandboxState(sandboxID string) (State, error) fetchSandboxNetwork(sandboxID string) (NetworkNamespace, error) storeSandboxNetwork(sandboxID string, networkNS NetworkNamespace) error - fetchSandboxDevices(sandboxID string) ([]api.Device, error) - storeSandboxDevices(sandboxID string, devices []api.Device) error // Hypervisor resources fetchHypervisorState(sandboxID string, state interface{}) error @@ -149,8 +144,8 @@ type resourceStorage interface { storeContainerProcess(sandboxID, containerID string, process Process) error fetchContainerMounts(sandboxID, containerID string) ([]Mount, error) storeContainerMounts(sandboxID, containerID string, mounts []Mount) error - fetchContainerDevices(sandboxID, containerID string) ([]ContainerDevice, error) - storeContainerDevices(sandboxID, containerID string, devices []ContainerDevice) error + fetchContainerDevices(sandboxID, containerID string) ([]api.Device, error) + storeContainerDevices(sandboxID, containerID string, devices []api.Device) error } // filesystem is a resourceStorage interface implementation for a local filesystem. @@ -232,35 +227,6 @@ type TypedDevice struct { Data json.RawMessage } -// storeDeviceIDFile is used to marshal and store device IDs to disk. -func (fs *filesystem) storeDeviceIDFile(file string, data interface{}) error { - if file == "" { - return errNeedFile - } - - f, err := os.Create(file) - if err != nil { - return err - } - defer f.Close() - - devices, ok := data.([]ContainerDevice) - if !ok { - return fmt.Errorf("Incorrect data type received, Expected []string") - } - - jsonOut, err := json.Marshal(devices) - if err != nil { - return fmt.Errorf("Could not marshal devices: %s", err) - } - - if _, err := f.Write(jsonOut); err != nil { - return err - } - - return nil -} - // storeDeviceFile is used to provide custom marshalling for Device objects. // Device is first marshalled into TypedDevice to include the type // of the Device object. @@ -381,7 +347,7 @@ func (fs *filesystem) fetchDeviceFile(fileData []byte, devices *[]api.Device) er func resourceNeedsContainerID(sandboxSpecific bool, resource sandboxResource) bool { switch resource { - case lockFileType, networkFileType, hypervisorFileType, agentFileType, devicesFileType: + case lockFileType, networkFileType, hypervisorFileType, agentFileType: // sandbox-specific resources return false default: @@ -404,7 +370,7 @@ func resourceDir(sandboxSpecific bool, sandboxID, containerID string, resource s case configFileType: path = configStoragePath break - case stateFileType, networkFileType, processFileType, lockFileType, mountsFileType, devicesFileType, devicesIDFileType, hypervisorFileType, agentFileType: + case stateFileType, networkFileType, processFileType, lockFileType, mountsFileType, devicesFileType, hypervisorFileType, agentFileType: path = runStoragePath break default: @@ -455,9 +421,6 @@ func (fs *filesystem) resourceURI(sandboxSpecific bool, sandboxID, containerID s case devicesFileType: filename = devicesFile break - case devicesIDFileType: - filename = devicesFile - break default: return "", "", errInvalidResource } @@ -503,7 +466,6 @@ func (fs *filesystem) commonResourceChecks(sandboxSpecific bool, sandboxID, cont case processFileType: case mountsFileType: case devicesFileType: - case devicesIDFileType: default: return errInvalidResource } @@ -590,19 +552,6 @@ func (fs *filesystem) storeDeviceResource(sandboxSpecific bool, sandboxID, conta return fs.storeDeviceFile(devicesFile, file) } -func (fs *filesystem) storeDevicesIDResource(sandboxSpecific bool, sandboxID, containerID string, resource sandboxResource, file interface{}) error { - if resource != devicesIDFileType { - return errInvalidResource - } - - devicesFile, _, err := fs.resourceURI(sandboxSpecific, sandboxID, containerID, resource) - if err != nil { - return err - } - - return fs.storeDeviceIDFile(devicesFile, file) -} - func (fs *filesystem) storeResource(sandboxSpecific bool, sandboxID, containerID string, resource sandboxResource, data interface{}) error { if err := fs.commonResourceChecks(sandboxSpecific, sandboxID, containerID, resource); err != nil { return err @@ -626,8 +575,6 @@ func (fs *filesystem) storeResource(sandboxSpecific bool, sandboxID, containerID case []api.Device: return fs.storeDeviceResource(sandboxSpecific, sandboxID, containerID, resource, file) - case []ContainerDevice: - return fs.storeDevicesIDResource(sandboxSpecific, sandboxID, containerID, resource, file) default: return fmt.Errorf("Invalid resource data type") @@ -681,18 +628,6 @@ func (fs *filesystem) fetchSandboxNetwork(sandboxID string) (NetworkNamespace, e return networkNS, nil } -func (fs *filesystem) fetchSandboxDevices(sandboxID string) ([]api.Device, error) { - var devices []api.Device - if err := fs.fetchResource(true, sandboxID, "", devicesFileType, &devices); err != nil { - return []api.Device{}, err - } - return devices, nil -} - -func (fs *filesystem) storeSandboxDevices(sandboxID string, devices []api.Device) error { - return fs.storeSandboxResource(sandboxID, devicesFileType, devices) -} - func (fs *filesystem) fetchHypervisorState(sandboxID string, state interface{}) error { return fs.fetchResource(true, sandboxID, "", hypervisorFileType, state) } @@ -799,11 +734,11 @@ func (fs *filesystem) fetchContainerMounts(sandboxID, containerID string) ([]Mou return mounts, nil } -func (fs *filesystem) fetchContainerDevices(sandboxID, containerID string) ([]ContainerDevice, error) { - var devices []ContainerDevice +func (fs *filesystem) fetchContainerDevices(sandboxID, containerID string) ([]api.Device, error) { + var devices []api.Device - if err := fs.fetchResource(false, sandboxID, containerID, devicesIDFileType, &devices); err != nil { - return nil, err + if err := fs.fetchResource(false, sandboxID, containerID, devicesFileType, &devices); err != nil { + return []api.Device{}, err } return devices, nil @@ -813,8 +748,8 @@ func (fs *filesystem) storeContainerMounts(sandboxID, containerID string, mounts return fs.storeContainerResource(sandboxID, containerID, mountsFileType, mounts) } -func (fs *filesystem) storeContainerDevices(sandboxID, containerID string, devices []ContainerDevice) error { - return fs.storeContainerResource(sandboxID, containerID, devicesIDFileType, devices) +func (fs *filesystem) storeContainerDevices(sandboxID, containerID string, devices []api.Device) error { + return fs.storeContainerResource(sandboxID, containerID, devicesFileType, devices) } func (fs *filesystem) deleteContainerResources(sandboxID, containerID string, resources []sandboxResource) error { diff --git a/virtcontainers/filesystem_test.go b/virtcontainers/filesystem_test.go index 43ba939475..7687acb917 100644 --- a/virtcontainers/filesystem_test.go +++ b/virtcontainers/filesystem_test.go @@ -33,7 +33,7 @@ func TestFilesystemCreateAllResourcesSuccessful(t *testing.T) { id: testSandboxID, storage: fs, config: sandboxConfig, - devManager: manager.NewDeviceManager(manager.VirtioBlock, nil), + devManager: manager.NewDeviceManager(manager.VirtioBlock), containers: map[string]*Container{}, } diff --git a/virtcontainers/hyperstart_agent.go b/virtcontainers/hyperstart_agent.go index 43cefc71e0..cd2f9a358c 100644 --- a/virtcontainers/hyperstart_agent.go +++ b/virtcontainers/hyperstart_agent.go @@ -14,15 +14,15 @@ import ( "syscall" "time" - "github.com/sirupsen/logrus" - "github.com/vishvananda/netlink" - proxyClient "github.com/clearcontainers/proxy/client" - "github.com/kata-containers/runtime/virtcontainers/device/config" + "github.com/kata-containers/runtime/virtcontainers/device/drivers" "github.com/kata-containers/runtime/virtcontainers/pkg/hyperstart" ns "github.com/kata-containers/runtime/virtcontainers/pkg/nsenter" "github.com/kata-containers/runtime/virtcontainers/utils" specs "github.com/opencontainers/runtime-spec/specs-go" + + "github.com/sirupsen/logrus" + "github.com/vishvananda/netlink" ) var defaultSockPathTemplates = []string{"%s/%s/hyper.sock", "%s/%s/tty.sock"} @@ -231,31 +231,6 @@ func fsMapFromMounts(mounts []Mount) []*hyperstart.FsmapDescriptor { return fsmap } -func fsMapFromDevices(c *Container) ([]*hyperstart.FsmapDescriptor, error) { - var fsmap []*hyperstart.FsmapDescriptor - for _, dev := range c.devices { - device := c.sandbox.devManager.GetDeviceByID(dev.ID) - if device == nil { - return nil, fmt.Errorf("can't find device: %#v", dev) - } - - d, ok := device.GetDeviceInfo().(*config.BlockDrive) - if !ok || d == nil { - return nil, fmt.Errorf("can't retrieve block device information") - } - - fsmapDesc := &hyperstart.FsmapDescriptor{ - Source: d.VirtPath, - Path: dev.ContainerPath, - AbsolutePath: true, - DockerVolume: false, - SCSIAddr: d.SCSIAddr, - } - fsmap = append(fsmap, fsmapDesc) - } - return fsmap, nil -} - // init is the agent initialization implementation for hyperstart. func (h *hyper) init(sandbox *Sandbox, config interface{}) (err error) { switch c := config.(type) { @@ -462,8 +437,8 @@ func (h *hyper) stopSandbox(sandbox *Sandbox) error { // container. func (h *hyper) handleBlockVolumes(c *Container) { for _, m := range c.mounts { - if len(m.BlockDeviceID) > 0 { - c.devices = append(c.devices, ContainerDevice{ID: m.BlockDeviceID}) + if m.BlockDevice != nil { + c.devices = append(c.devices, m.BlockDevice) } } } @@ -522,11 +497,20 @@ func (h *hyper) startOneContainer(sandbox *Sandbox, c *Container) error { h.handleBlockVolumes(c) // Append container mounts for block devices passed with --device. - fsmapDev, err := fsMapFromDevices(c) - if err != nil { - return err + for _, device := range c.devices { + d, ok := device.(*drivers.BlockDevice) + + if ok { + fsmapDesc := &hyperstart.FsmapDescriptor{ + Source: d.VirtPath, + Path: d.DeviceInfo.ContainerPath, + AbsolutePath: true, + DockerVolume: false, + SCSIAddr: d.SCSIAddr, + } + fsmap = append(fsmap, fsmapDesc) + } } - fsmap = append(fsmap, fsmapDev...) // Assign fsmap for hyperstart to mount these at the correct location within the container container.Fsmap = fsmap diff --git a/virtcontainers/kata_agent.go b/virtcontainers/kata_agent.go index eee6ff6c2e..f442aabc1d 100644 --- a/virtcontainers/kata_agent.go +++ b/virtcontainers/kata_agent.go @@ -19,7 +19,8 @@ import ( kataclient "github.com/kata-containers/agent/protocols/client" "github.com/kata-containers/agent/protocols/grpc" - "github.com/kata-containers/runtime/virtcontainers/device/config" + "github.com/kata-containers/runtime/virtcontainers/device/api" + "github.com/kata-containers/runtime/virtcontainers/device/drivers" vcAnnotations "github.com/kata-containers/runtime/virtcontainers/pkg/annotations" ns "github.com/kata-containers/runtime/virtcontainers/pkg/nsenter" "github.com/kata-containers/runtime/virtcontainers/pkg/uuid" @@ -722,26 +723,15 @@ func (k *kataAgent) handleShm(grpcSpec *grpc.Spec, sandbox *Sandbox) { } } -func (k *kataAgent) appendDevices(deviceList []*grpc.Device, c *Container) []*grpc.Device { - for _, dev := range c.devices { - device := c.sandbox.devManager.GetDeviceByID(dev.ID) - if device == nil { - k.Logger().WithField("device", dev.ID).Error("failed to find device by id") - return nil - } - - if device.DeviceType() != config.DeviceBlock { - continue - } - - d, ok := device.GetDeviceInfo().(*config.BlockDrive) - if !ok || d == nil { - k.Logger().WithField("device", device).Error("malformed block drive") +func (k *kataAgent) appendDevices(deviceList []*grpc.Device, devices []api.Device) []*grpc.Device { + for _, device := range devices { + d, ok := device.(*drivers.BlockDevice) + if !ok { continue } kataDevice := &grpc.Device{ - ContainerPath: dev.ContainerPath, + ContainerPath: d.DeviceInfo.ContainerPath, } if d.SCSIAddr == "" { @@ -878,7 +868,7 @@ func (k *kataAgent) createContainer(sandbox *Sandbox, c *Container) (p *Process, } // Append container devices for block devices passed with --device. - ctrDevices = k.appendDevices(ctrDevices, c) + ctrDevices = k.appendDevices(ctrDevices, c.devices) // Handle all the volumes that are block device files. // Note this call modifies the list of container devices to make sure @@ -967,41 +957,27 @@ func (k *kataAgent) handleBlockVolumes(c *Container) []*grpc.Storage { var volumeStorages []*grpc.Storage for _, m := range c.mounts { - id := m.BlockDeviceID + b := m.BlockDevice - if len(id) == 0 { + if b == nil { continue } // Add the block device to the list of container devices, to make sure the // device is detached with detachDevices() for a container. - c.devices = append(c.devices, ContainerDevice{ID: id}) - if err := c.storeDevices(); err != nil { - k.Logger().WithField("device", id).WithError(err).Error("store device failed") - return nil - } + c.devices = append(c.devices, b) vol := &grpc.Storage{} - device := c.sandbox.devManager.GetDeviceByID(id) - if device == nil { - k.Logger().WithField("device", id).Error("failed to find device by id") - return nil - } - blockDrive, ok := device.GetDeviceInfo().(*config.BlockDrive) - if !ok || blockDrive == nil { - k.Logger().Error("malformed block drive") - continue - } if c.sandbox.config.HypervisorConfig.BlockDeviceDriver == VirtioBlock { vol.Driver = kataBlkDevType - vol.Source = blockDrive.VirtPath + vol.Source = b.VirtPath } else { vol.Driver = kataSCSIDevType - vol.Source = blockDrive.SCSIAddr + vol.Source = b.SCSIAddr } - vol.MountPoint = m.Destination + vol.MountPoint = b.DeviceInfo.ContainerPath vol.Fstype = "bind" vol.Options = []string{"bind"} diff --git a/virtcontainers/kata_agent_test.go b/virtcontainers/kata_agent_test.go index 5aa2cbc08a..6a0bb09dd4 100644 --- a/virtcontainers/kata_agent_test.go +++ b/virtcontainers/kata_agent_test.go @@ -28,12 +28,11 @@ import ( "github.com/kata-containers/runtime/virtcontainers/device/api" "github.com/kata-containers/runtime/virtcontainers/device/config" "github.com/kata-containers/runtime/virtcontainers/device/drivers" - "github.com/kata-containers/runtime/virtcontainers/device/manager" vcAnnotations "github.com/kata-containers/runtime/virtcontainers/pkg/annotations" "github.com/kata-containers/runtime/virtcontainers/pkg/mock" ) -var ( +const ( testKataProxyURLTempl = "unix://%s/kata-proxy-test.sock" testBlockDeviceCtrPath = "testBlockDeviceCtrPath" testPCIAddr = "04/02" @@ -445,15 +444,9 @@ func TestAppendDevicesEmptyContainerDeviceList(t *testing.T) { devList := []*pb.Device{} expected := []*pb.Device{} - ctrDevices := []ContainerDevice{} + ctrDevices := []api.Device{} - c := &Container{ - sandbox: &Sandbox{ - devManager: manager.NewDeviceManager("virtio-scsi", nil), - }, - devices: ctrDevices, - } - updatedDevList := k.appendDevices(devList, c) + updatedDevList := k.appendDevices(devList, ctrDevices) assert.True(t, reflect.DeepEqual(updatedDevList, expected), "Device lists didn't match: got %+v, expecting %+v", updatedDevList, expected) @@ -462,26 +455,6 @@ func TestAppendDevicesEmptyContainerDeviceList(t *testing.T) { func TestAppendDevices(t *testing.T) { k := kataAgent{} - id := "test-append-block" - ctrDevices := []api.Device{ - &drivers.BlockDevice{ - ID: id, - BlockDrive: &config.BlockDrive{ - PCIAddr: testPCIAddr, - }, - }, - } - - c := &Container{ - sandbox: &Sandbox{ - devManager: manager.NewDeviceManager("virtio-scsi", ctrDevices), - }, - } - c.devices = append(c.devices, ContainerDevice{ - ID: id, - ContainerPath: testBlockDeviceCtrPath, - }) - devList := []*pb.Device{} expected := []*pb.Device{ { @@ -490,7 +463,16 @@ func TestAppendDevices(t *testing.T) { Id: testPCIAddr, }, } - updatedDevList := k.appendDevices(devList, c) + ctrDevices := []api.Device{ + &drivers.BlockDevice{ + DeviceInfo: config.DeviceInfo{ + ContainerPath: testBlockDeviceCtrPath, + }, + PCIAddr: testPCIAddr, + }, + } + + updatedDevList := k.appendDevices(devList, ctrDevices) assert.True(t, reflect.DeepEqual(updatedDevList, expected), "Device lists didn't match: got %+v, expecting %+v", updatedDevList, expected) diff --git a/virtcontainers/mount.go b/virtcontainers/mount.go index dba2522878..c68d9093ae 100644 --- a/virtcontainers/mount.go +++ b/virtcontainers/mount.go @@ -14,6 +14,8 @@ import ( "path/filepath" "strings" "syscall" + + "github.com/kata-containers/runtime/virtcontainers/device/drivers" ) // DefaultShmSize is the default shm size to be used in case host @@ -282,10 +284,10 @@ type Mount struct { // ReadOnly specifies if the mount should be read only or not ReadOnly bool - // BlockDeviceID represents block device that is attached to the + // BlockDevice represents block device that is attached to the // VM in case this mount is a block device file or a directory // backed by a block device. - BlockDeviceID string + BlockDevice *drivers.BlockDevice } func bindUnmountContainerRootfs(sharedDir, sandboxID, cID string) error { diff --git a/virtcontainers/network.go b/virtcontainers/network.go index 4ae7cff0b0..15401af655 100644 --- a/virtcontainers/network.go +++ b/virtcontainers/network.go @@ -25,7 +25,6 @@ import ( "github.com/vishvananda/netns" "golang.org/x/sys/unix" - "github.com/kata-containers/runtime/virtcontainers/device/config" "github.com/kata-containers/runtime/virtcontainers/device/drivers" "github.com/kata-containers/runtime/virtcontainers/pkg/uuid" "github.com/kata-containers/runtime/virtcontainers/utils" @@ -281,11 +280,11 @@ func (endpoint *VhostUserEndpoint) Attach(h hypervisor) error { } id := hex.EncodeToString(randBytes) - d := config.VhostUserDeviceAttrs{ - ID: id, - SocketPath: endpoint.SocketPath, + d := &drivers.VhostUserNetDevice{ MacAddress: endpoint.HardAddr, } + d.SocketPath = endpoint.SocketPath + d.ID = id return h.addDevice(d, vhostuserDev) } @@ -344,8 +343,7 @@ func (endpoint *PhysicalEndpoint) Attach(h hypervisor) error { return err } - // TODO: use device manager as general device management entrance - d := config.VFIODev{ + d := drivers.VFIODevice{ BDF: endpoint.BDF, } diff --git a/virtcontainers/qemu.go b/virtcontainers/qemu.go index 0020bc101f..1d84fa88d7 100644 --- a/virtcontainers/qemu.go +++ b/virtcontainers/qemu.go @@ -19,7 +19,8 @@ import ( "github.com/kata-containers/runtime/virtcontainers/pkg/uuid" "github.com/sirupsen/logrus" - "github.com/kata-containers/runtime/virtcontainers/device/config" + "github.com/kata-containers/runtime/virtcontainers/device/api" + deviceDrivers "github.com/kata-containers/runtime/virtcontainers/device/drivers" "github.com/kata-containers/runtime/virtcontainers/utils" ) @@ -655,7 +656,7 @@ func (q *qemu) removeDeviceFromBridge(ID string) error { return err } -func (q *qemu) hotplugBlockDevice(drive *config.BlockDrive, op operation) error { +func (q *qemu) hotplugBlockDevice(drive *deviceDrivers.Drive, op operation) error { err := q.qmpSetup() if err != nil { return err @@ -716,13 +717,13 @@ func (q *qemu) hotplugBlockDevice(drive *config.BlockDrive, op operation) error return nil } -func (q *qemu) hotplugVFIODevice(device *config.VFIODev, op operation) error { +func (q *qemu) hotplugVFIODevice(device deviceDrivers.VFIODevice, op operation) error { err := q.qmpSetup() if err != nil { return err } - devID := device.ID + devID := "vfio-" + device.DeviceInfo.ID if op == addDevice { addr, bridge, err := q.addDeviceToBridge(devID) @@ -749,13 +750,15 @@ func (q *qemu) hotplugVFIODevice(device *config.VFIODev, op operation) error { func (q *qemu) hotplugDevice(devInfo interface{}, devType deviceType, op operation) (interface{}, error) { switch devType { case blockDev: - drive := devInfo.(*config.BlockDrive) + // TODO: find a way to remove dependency of deviceDrivers lib @weizhang555 + drive := devInfo.(*deviceDrivers.Drive) return nil, q.hotplugBlockDevice(drive, op) case cpuDev: vcpus := devInfo.(uint32) return q.hotplugCPUs(vcpus, op) case vfioDev: - device := devInfo.(*config.VFIODev) + // TODO: find a way to remove dependency of deviceDrivers lib @weizhang555 + device := devInfo.(deviceDrivers.VFIODevice) return nil, q.hotplugVFIODevice(device, op) case memoryDev: memdev := devInfo.(*memoryDevice) @@ -941,6 +944,13 @@ func (q *qemu) resumeSandbox() error { // addDevice will add extra devices to Qemu command line. func (q *qemu) addDevice(devInfo interface{}, devType deviceType) error { + switch devType { + case vhostuserDev: + vhostDev := devInfo.(api.VhostUserDevice) + q.qemuConfig.Devices = q.arch.appendVhostUserDevice(q.qemuConfig.Devices, vhostDev) + return nil + } + switch v := devInfo.(type) { case Volume: q.qemuConfig.Devices = q.arch.append9PVolume(q.qemuConfig.Devices, v) @@ -948,11 +958,10 @@ func (q *qemu) addDevice(devInfo interface{}, devType deviceType) error { q.qemuConfig.Devices = q.arch.appendSocket(q.qemuConfig.Devices, v) case Endpoint: q.qemuConfig.Devices = q.arch.appendNetwork(q.qemuConfig.Devices, v) - case config.BlockDrive: + case deviceDrivers.Drive: q.qemuConfig.Devices = q.arch.appendBlockDevice(q.qemuConfig.Devices, v) - case config.VhostUserDeviceAttrs: - q.qemuConfig.Devices = q.arch.appendVhostUserDevice(q.qemuConfig.Devices, v) - case config.VFIODev: + + case deviceDrivers.VFIODevice: q.qemuConfig.Devices = q.arch.appendVFIODevice(q.qemuConfig.Devices, v) default: break diff --git a/virtcontainers/qemu_arch_base.go b/virtcontainers/qemu_arch_base.go index 6396cbe1d7..e0188c4d3d 100644 --- a/virtcontainers/qemu_arch_base.go +++ b/virtcontainers/qemu_arch_base.go @@ -13,7 +13,8 @@ import ( govmmQemu "github.com/intel/govmm/qemu" - "github.com/kata-containers/runtime/virtcontainers/device/config" + "github.com/kata-containers/runtime/virtcontainers/device/api" + "github.com/kata-containers/runtime/virtcontainers/device/drivers" "github.com/kata-containers/runtime/virtcontainers/utils" ) @@ -71,13 +72,13 @@ type qemuArch interface { appendNetwork(devices []govmmQemu.Device, endpoint Endpoint) []govmmQemu.Device // appendBlockDevice appends a block drive to devices - appendBlockDevice(devices []govmmQemu.Device, drive config.BlockDrive) []govmmQemu.Device + appendBlockDevice(devices []govmmQemu.Device, drive drivers.Drive) []govmmQemu.Device // appendVhostUserDevice appends a vhost user device to devices - appendVhostUserDevice(devices []govmmQemu.Device, drive config.VhostUserDeviceAttrs) []govmmQemu.Device + appendVhostUserDevice(devices []govmmQemu.Device, vhostUserDevice api.VhostUserDevice) []govmmQemu.Device // appendVFIODevice appends a VFIO device to devices - appendVFIODevice(devices []govmmQemu.Device, vfioDevice config.VFIODev) []govmmQemu.Device + appendVFIODevice(devices []govmmQemu.Device, vfioDevice drivers.VFIODevice) []govmmQemu.Device // handleImagePath handles the Hypervisor Config image path handleImagePath(config HypervisorConfig) @@ -285,7 +286,7 @@ func (q *qemuArchBase) appendImage(devices []govmmQemu.Device, path string) ([]g id := utils.MakeNameID("image", hex.EncodeToString(randBytes), maxDevIDSize) - drive := config.BlockDrive{ + drive := drivers.Drive{ File: path, Format: "raw", ID: id, @@ -429,7 +430,7 @@ func (q *qemuArchBase) appendNetwork(devices []govmmQemu.Device, endpoint Endpoi return devices } -func (q *qemuArchBase) appendBlockDevice(devices []govmmQemu.Device, drive config.BlockDrive) []govmmQemu.Device { +func (q *qemuArchBase) appendBlockDevice(devices []govmmQemu.Device, drive drivers.Drive) []govmmQemu.Device { if drive.File == "" || drive.ID == "" || drive.Format == "" { return devices } @@ -453,36 +454,36 @@ func (q *qemuArchBase) appendBlockDevice(devices []govmmQemu.Device, drive confi return devices } -func (q *qemuArchBase) appendVhostUserDevice(devices []govmmQemu.Device, attr config.VhostUserDeviceAttrs) []govmmQemu.Device { +func (q *qemuArchBase) appendVhostUserDevice(devices []govmmQemu.Device, vhostUserDevice api.VhostUserDevice) []govmmQemu.Device { qemuVhostUserDevice := govmmQemu.VhostUserDevice{} // TODO: find a way to remove dependency of drivers package - switch attr.Type { - case config.VhostUserNet: - qemuVhostUserDevice.TypeDevID = utils.MakeNameID("net", attr.ID, maxDevIDSize) - qemuVhostUserDevice.Address = attr.MacAddress - case config.VhostUserSCSI: - qemuVhostUserDevice.TypeDevID = utils.MakeNameID("scsi", attr.ID, maxDevIDSize) - case config.VhostUserBlk: + switch vhostUserDevice := vhostUserDevice.(type) { + case *drivers.VhostUserNetDevice: + qemuVhostUserDevice.TypeDevID = utils.MakeNameID("net", vhostUserDevice.ID, maxDevIDSize) + qemuVhostUserDevice.Address = vhostUserDevice.MacAddress + case *drivers.VhostUserSCSIDevice: + qemuVhostUserDevice.TypeDevID = utils.MakeNameID("scsi", vhostUserDevice.ID, maxDevIDSize) + case *drivers.VhostUserBlkDevice: } - qemuVhostUserDevice.VhostUserType = govmmQemu.VhostUserDeviceType(attr.Type) - qemuVhostUserDevice.SocketPath = attr.SocketPath - qemuVhostUserDevice.CharDevID = utils.MakeNameID("char", attr.ID, maxDevIDSize) + qemuVhostUserDevice.VhostUserType = govmmQemu.VhostUserDeviceType(vhostUserDevice.Type()) + qemuVhostUserDevice.SocketPath = vhostUserDevice.Attrs().SocketPath + qemuVhostUserDevice.CharDevID = utils.MakeNameID("char", vhostUserDevice.Attrs().ID, maxDevIDSize) devices = append(devices, qemuVhostUserDevice) return devices } -func (q *qemuArchBase) appendVFIODevice(devices []govmmQemu.Device, vfioDev config.VFIODev) []govmmQemu.Device { - if vfioDev.BDF == "" { +func (q *qemuArchBase) appendVFIODevice(devices []govmmQemu.Device, vfioDevice drivers.VFIODevice) []govmmQemu.Device { + if vfioDevice.BDF == "" { return devices } devices = append(devices, govmmQemu.VFIODevice{ - BDF: vfioDev.BDF, + BDF: vfioDevice.BDF, }, ) diff --git a/virtcontainers/qemu_arch_base_test.go b/virtcontainers/qemu_arch_base_test.go index 3465f8cdad..518c8135d0 100644 --- a/virtcontainers/qemu_arch_base_test.go +++ b/virtcontainers/qemu_arch_base_test.go @@ -15,6 +15,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/kata-containers/runtime/virtcontainers/device/config" + "github.com/kata-containers/runtime/virtcontainers/device/drivers" ) const ( @@ -205,12 +206,12 @@ func testQemuArchBaseAppend(t *testing.T, structure interface{}, expected []govm devices = qemuArchBase.append9PVolume(devices, s) case Socket: devices = qemuArchBase.appendSocket(devices, s) - case config.BlockDrive: + case drivers.Drive: devices = qemuArchBase.appendBlockDevice(devices, s) - case config.VFIODev: + case drivers.VFIODevice: devices = qemuArchBase.appendVFIODevice(devices, s) - case config.VhostUserDeviceAttrs: - devices = qemuArchBase.appendVhostUserDevice(devices, s) + case drivers.VhostUserNetDevice: + devices = qemuArchBase.appendVhostUserDevice(devices, &s) } assert.Equal(devices, expected) @@ -363,7 +364,7 @@ func TestQemuArchBaseAppendBlockDevice(t *testing.T) { }, } - drive := config.BlockDrive{ + drive := drivers.Drive{ File: file, Format: format, ID: id, @@ -387,8 +388,7 @@ func TestQemuArchBaseAppendVhostUserDevice(t *testing.T) { }, } - vhostUserDevice := config.VhostUserDeviceAttrs{ - Type: config.VhostUserNet, + vhostUserDevice := drivers.VhostUserNetDevice{ MacAddress: macAddress, } vhostUserDevice.ID = id @@ -406,7 +406,7 @@ func TestQemuArchBaseAppendVFIODevice(t *testing.T) { }, } - vfDevice := config.VFIODev{ + vfDevice := drivers.VFIODevice{ BDF: bdf, } diff --git a/virtcontainers/qemu_ppc64le.go b/virtcontainers/qemu_ppc64le.go index a79f39b87e..ba259389eb 100644 --- a/virtcontainers/qemu_ppc64le.go +++ b/virtcontainers/qemu_ppc64le.go @@ -10,7 +10,7 @@ import ( "os" govmmQemu "github.com/intel/govmm/qemu" - deviceConfig "github.com/kata-containers/runtime/virtcontainers/device/config" + "github.com/kata-containers/runtime/virtcontainers/device/drivers" "github.com/kata-containers/runtime/virtcontainers/utils" "github.com/sirupsen/logrus" ) @@ -134,7 +134,7 @@ func (q *qemuPPC64le) appendImage(devices []govmmQemu.Device, path string) ([]go id := utils.MakeNameID("image", hex.EncodeToString(randBytes), maxDevIDSize) - drive := deviceConfig.BlockDrive{ + drive := drivers.Drive{ File: path, Format: "raw", ID: id, diff --git a/virtcontainers/sandbox.go b/virtcontainers/sandbox.go index bf265e0b60..b3339d252f 100644 --- a/virtcontainers/sandbox.go +++ b/virtcontainers/sandbox.go @@ -713,12 +713,6 @@ func createSandbox(sandboxConfig SandboxConfig, factory Factory) (*Sandbox, erro s.networkNS = networkNS } - devices, err := s.storage.fetchSandboxDevices(s.id) - if err != nil { - s.Logger().WithError(err).WithField("sandboxid", s.id).Warning("fetch sandbox device failed") - } - s.devManager = deviceManager.NewDeviceManager(sandboxConfig.HypervisorConfig.BlockDeviceDriver, devices) - // We first try to fetch the sandbox state from storage. // If it exists, this means this is a re-creation, i.e. // we don't need to talk to the guest's agent, but only @@ -764,6 +758,7 @@ func newSandbox(sandboxConfig SandboxConfig, factory Factory) (*Sandbox, error) storage: &filesystem{}, network: network, config: &sandboxConfig, + devManager: deviceManager.NewDeviceManager(sandboxConfig.HypervisorConfig.BlockDeviceDriver), volumes: sandboxConfig.Volumes, containers: map[string]*Container{}, runPath: filepath.Join(runStoragePath, sandboxConfig.ID), @@ -813,10 +808,6 @@ func newSandbox(sandboxConfig SandboxConfig, factory Factory) (*Sandbox, error) return s, nil } -func (s *Sandbox) storeSandboxDevices() error { - return s.storage.storeSandboxDevices(s.id, s.devManager.GetAllDevices()) -} - // storeSandbox stores a sandbox config. func (s *Sandbox) storeSandbox() error { err := s.storage.storeSandboxResource(s.id, configFileType, *(s.config)) @@ -1455,24 +1446,12 @@ func togglePauseSandbox(sandboxID string, pause bool) (*Sandbox, error) { func (s *Sandbox) HotplugAddDevice(device api.Device, devType config.DeviceType) error { switch devType { case config.DeviceVFIO: - vfioDevices, ok := device.GetDeviceInfo().([]*config.VFIODev) + vfioDevice, ok := device.(*drivers.VFIODevice) if !ok { return fmt.Errorf("device type mismatch, expect device type to be %s", devType) } - - // adding a group of VFIO devices - for _, dev := range vfioDevices { - if _, err := s.hypervisor.hotplugAddDevice(dev, vfioDev); err != nil { - s.Logger(). - WithFields(logrus.Fields{ - "sandboxid": s.id, - "vfio device ID": dev.ID, - "vfio device BDF": dev.BDF, - }).WithError(err).Error("failed to hotplug VFIO device") - return err - } - } - return nil + _, err := s.hypervisor.hotplugAddDevice(*vfioDevice, vfioDev) + return err case config.DeviceBlock: blockDevice, ok := device.(*drivers.BlockDevice) if !ok { @@ -1492,30 +1471,18 @@ func (s *Sandbox) HotplugAddDevice(device api.Device, devType config.DeviceType) func (s *Sandbox) HotplugRemoveDevice(device api.Device, devType config.DeviceType) error { switch devType { case config.DeviceVFIO: - vfioDevices, ok := device.GetDeviceInfo().([]*config.VFIODev) + vfioDevice, ok := device.(*drivers.VFIODevice) if !ok { return fmt.Errorf("device type mismatch, expect device type to be %s", devType) } - - // remove a group of VFIO devices - for _, dev := range vfioDevices { - if _, err := s.hypervisor.hotplugRemoveDevice(dev, vfioDev); err != nil { - s.Logger().WithError(err). - WithFields(logrus.Fields{ - "sandboxid": s.id, - "vfio device ID": dev.ID, - "vfio device BDF": dev.BDF, - }).Error("failed to hot unplug VFIO device") - return err - } - } - return nil + _, err := s.hypervisor.hotplugRemoveDevice(*vfioDevice, vfioDev) + return err case config.DeviceBlock: - blockDrive, ok := device.GetDeviceInfo().(*config.BlockDrive) + blockDevice, ok := device.(*drivers.BlockDevice) if !ok { return fmt.Errorf("device type mismatch, expect device type to be %s", devType) } - _, err := s.hypervisor.hotplugRemoveDevice(blockDrive, blockDev) + _, err := s.hypervisor.hotplugRemoveDevice(blockDevice.BlockDrive, blockDev) return err case config.DeviceGeneric: // TODO: what? @@ -1536,13 +1503,12 @@ func (s *Sandbox) DecrementSandboxBlockIndex() error { return s.decrementSandboxBlockIndex() } -// AppendDevice can only handle vhost user device currently, it adds a -// vhost user device to sandbox +// AddVhostUserDevice adds a vhost user device to sandbox // Sandbox implement DeviceReceiver interface from device/api/interface.go -func (s *Sandbox) AppendDevice(device api.Device) error { - switch device.DeviceType() { +func (s *Sandbox) AddVhostUserDevice(devInfo api.VhostUserDevice, devType config.DeviceType) error { + switch devType { case config.VhostUserSCSI, config.VhostUserNet, config.VhostUserBlk: - return s.hypervisor.addDevice(device.GetDeviceInfo().(*config.VhostUserDeviceAttrs), vhostuserDev) + return s.hypervisor.addDevice(devInfo, vhostuserDev) } return fmt.Errorf("unsupported device type") } diff --git a/virtcontainers/sandbox_test.go b/virtcontainers/sandbox_test.go index a6124db41c..cc6049d1c2 100644 --- a/virtcontainers/sandbox_test.go +++ b/virtcontainers/sandbox_test.go @@ -18,6 +18,7 @@ import ( "github.com/stretchr/testify/assert" + "github.com/kata-containers/runtime/virtcontainers/device/api" "github.com/kata-containers/runtime/virtcontainers/device/config" "github.com/kata-containers/runtime/virtcontainers/device/drivers" "github.com/kata-containers/runtime/virtcontainers/device/manager" @@ -1152,23 +1153,18 @@ func TestSandboxAttachDevicesVFIO(t *testing.T) { config.SysIOMMUPath = savedIOMMUPath }() - dm := manager.NewDeviceManager(manager.VirtioSCSI, nil) path := filepath.Join(vfioPath, testFDIOGroup) deviceInfo := config.DeviceInfo{ HostPath: path, ContainerPath: path, DevType: "c", } - dev, err := dm.NewDevice(deviceInfo) - assert.Nil(t, err, "deviceManager.NewDevice return error: %v", err) + vfioDevice := drivers.NewVFIODevice(deviceInfo) c := &Container{ id: "100", - devices: []ContainerDevice{ - { - ID: dev.DeviceID(), - ContainerPath: path, - }, + devices: []api.Device{ + vfioDevice, }, } @@ -1176,19 +1172,12 @@ func TestSandboxAttachDevicesVFIO(t *testing.T) { containers[c.id] = c sandbox := Sandbox{ - id: "100", containers: containers, - storage: &filesystem{}, hypervisor: &mockHypervisor{}, - devManager: dm, } containers[c.id].sandbox = &sandbox - err = sandbox.storage.createAllResources(&sandbox) - assert.Nil(t, err, "Error while create all resources for sandbox") - err = sandbox.storeSandboxDevices() - assert.Nil(t, err, "Error while store sandbox devices %s", err) err = containers[c.id].attachDevices() assert.Nil(t, err, "Error while attaching devices %s", err) @@ -1595,9 +1584,10 @@ func TestAttachBlockDevice(t *testing.T) { DevType: "b", } - dm := manager.NewDeviceManager(VirtioBlock, nil) - device, err := dm.NewDevice(deviceInfo) + dm := manager.NewDeviceManager(VirtioBlock) + devices, err := dm.NewDevices([]config.DeviceInfo{deviceInfo}) assert.Nil(t, err) + device := devices[0] _, ok := device.(*drivers.BlockDevice) assert.True(t, ok)