Skip to content

Commit

Permalink
revert: "virtcontainers: support pre-add storage for frakti"
Browse files Browse the repository at this point in the history 
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 kata-containers#529

Signed-off-by: Sebastien Boeuf <[email protected]>
  • Loading branch information
Sebastien Boeuf committed Jul 27, 2018
1 parent cfbc974 commit 927487c
Show file tree
Hide file tree
Showing 26 changed files with 382 additions and 890 deletions.
116 changes: 32 additions & 84 deletions virtcontainers/container.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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 {
Expand All @@ -258,7 +251,7 @@ type Container struct {

mounts []Mount

devices []ContainerDevice
devices []api.Device

systemMountsInfo SystemMountsInfo
}
Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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()
Expand All @@ -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
}

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
}

Expand All @@ -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
}

Expand Down
32 changes: 12 additions & 20 deletions virtcontainers/device/api/interface.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,43 +27,35 @@ 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

// this is only for virtio-blk and virtio-scsi support
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)
}
4 changes: 2 additions & 2 deletions virtcontainers/device/api/mockDeviceReceiver.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
46 changes: 5 additions & 41 deletions virtcontainers/device/config/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down
Loading

0 comments on commit 927487c

Please sign in to comment.