diff --git a/virtcontainers/api.go b/virtcontainers/api.go index 340a2bf71a..c8e8d8bbc9 100644 --- a/virtcontainers/api.go +++ b/virtcontainers/api.go @@ -11,6 +11,7 @@ import ( "syscall" deviceApi "github.com/kata-containers/runtime/virtcontainers/device/api" + deviceConfig "github.com/kata-containers/runtime/virtcontainers/device/config" specs "github.com/opencontainers/runtime-spec/specs-go" "github.com/sirupsen/logrus" ) @@ -760,3 +761,24 @@ func PauseContainer(sandboxID, containerID string) error { func ResumeContainer(sandboxID, containerID string) error { return togglePauseContainer(sandboxID, containerID, false) } + +// AddDevice will add a device to sandbox +func AddDevice(sandboxID string, info deviceConfig.DeviceInfo) (deviceApi.Device, error) { + if sandboxID == "" { + return nil, errNeedSandboxID + } + + lockFile, err := rwLockSandbox(sandboxID) + if err != nil { + return nil, err + } + defer unlockSandbox(lockFile) + + s, err := fetchSandbox(sandboxID) + if err != nil { + return nil, err + } + defer s.releaseStatelessSandbox() + + return s.AddDevice(info) +} diff --git a/virtcontainers/container.go b/virtcontainers/container.go index d627c01c2e..ffa8aab659 100644 --- a/virtcontainers/container.go +++ b/virtcontainers/container.go @@ -1133,13 +1133,22 @@ func (c *Container) attachDevices() error { 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 - } + err := c.sandbox.devManager.DetachDevice(dev.ID, c.sandbox) + if err != nil && err != manager.ErrDeviceNotAttached { return err } + + if err = c.sandbox.devManager.RemoveDevice(dev.ID); err != nil { + c.Logger().WithFields(logrus.Fields{ + "container": c.id, + "device-id": dev.ID, + }).WithError(err).Error("remove device failed") + + // ignore the device not exist error + if err != manager.ErrDeviceNotExist { + return err + } + } } if err := c.sandbox.storeSandboxDevices(); err != nil { diff --git a/virtcontainers/device/api/interface.go b/virtcontainers/device/api/interface.go index dd4002826e..6bdba9b1bc 100644 --- a/virtcontainers/device/api/interface.go +++ b/virtcontainers/device/api/interface.go @@ -62,6 +62,7 @@ type Device interface { // device management object. type DeviceManager interface { NewDevice(config.DeviceInfo) (Device, error) + RemoveDevice(string) error AttachDevice(string, DeviceReceiver) error DetachDevice(string, DeviceReceiver) error IsDeviceAttached(string) bool diff --git a/virtcontainers/device/manager/manager.go b/virtcontainers/device/manager/manager.go index e14a53d266..62ce0742a6 100644 --- a/virtcontainers/device/manager/manager.go +++ b/virtcontainers/device/manager/manager.go @@ -93,7 +93,7 @@ func (dm *deviceManager) createDevice(devInfo config.DeviceInfo) (api.Device, er } } -// NewDevice creates bundles of devices based on array of DeviceInfo +// NewDevice creates a device based on specified DeviceInfo func (dm *deviceManager) NewDevice(devInfo config.DeviceInfo) (api.Device, error) { dm.Lock() defer dm.Unlock() @@ -104,6 +104,17 @@ func (dm *deviceManager) NewDevice(devInfo config.DeviceInfo) (api.Device, error return dev, err } +// RemoveDevice deletes the device from list based on specified device id +func (dm *deviceManager) RemoveDevice(id string) error { + dm.Lock() + defer dm.Unlock() + if _, ok := dm.devices[id]; !ok { + return ErrDeviceNotExist + } + delete(dm.devices, id) + return nil +} + func (dm *deviceManager) newDeviceID() (string, error) { for i := 0; i < 5; i++ { // generate an random ID diff --git a/virtcontainers/implementation.go b/virtcontainers/implementation.go index b3a4b961ad..e9006fbc18 100644 --- a/virtcontainers/implementation.go +++ b/virtcontainers/implementation.go @@ -12,6 +12,8 @@ package virtcontainers import ( "syscall" + "github.com/kata-containers/runtime/virtcontainers/device/api" + "github.com/kata-containers/runtime/virtcontainers/device/config" specs "github.com/opencontainers/runtime-spec/specs-go" "github.com/sirupsen/logrus" ) @@ -141,3 +143,8 @@ func (impl *VCImpl) PauseContainer(sandboxID, containerID string) error { func (impl *VCImpl) ResumeContainer(sandboxID, containerID string) error { return ResumeContainer(sandboxID, containerID) } + +// AddDevice will add a device to sandbox +func (impl *VCImpl) AddDevice(sandboxID string, info config.DeviceInfo) (api.Device, error) { + return AddDevice(sandboxID, info) +} diff --git a/virtcontainers/interfaces.go b/virtcontainers/interfaces.go index ec0d2b5bd3..d54843e49e 100644 --- a/virtcontainers/interfaces.go +++ b/virtcontainers/interfaces.go @@ -9,6 +9,8 @@ import ( "io" "syscall" + "github.com/kata-containers/runtime/virtcontainers/device/api" + "github.com/kata-containers/runtime/virtcontainers/device/config" specs "github.com/opencontainers/runtime-spec/specs-go" "github.com/sirupsen/logrus" ) @@ -41,6 +43,8 @@ type VC interface { UpdateContainer(sandboxID, containerID string, resources specs.LinuxResources) error PauseContainer(sandboxID, containerID string) error ResumeContainer(sandboxID, containerID string) error + + AddDevice(sandboxID string, info config.DeviceInfo) (api.Device, error) } // VCSandbox is the Sandbox interface @@ -70,6 +74,8 @@ type VCSandbox interface { SignalProcess(containerID, processID string, signal syscall.Signal, all bool) error WinsizeProcess(containerID, processID string, height, width uint32) error IOStream(containerID, processID string) (io.WriteCloser, io.Reader, io.Reader, error) + + AddDevice(info config.DeviceInfo) (api.Device, error) } // VCContainer is the Container interface diff --git a/virtcontainers/pkg/vcmock/mock.go b/virtcontainers/pkg/vcmock/mock.go index ea895c26a5..bf4cb27724 100644 --- a/virtcontainers/pkg/vcmock/mock.go +++ b/virtcontainers/pkg/vcmock/mock.go @@ -20,6 +20,8 @@ import ( "syscall" vc "github.com/kata-containers/runtime/virtcontainers" + "github.com/kata-containers/runtime/virtcontainers/device/api" + "github.com/kata-containers/runtime/virtcontainers/device/config" specs "github.com/opencontainers/runtime-spec/specs-go" "github.com/sirupsen/logrus" ) @@ -239,3 +241,12 @@ func (m *VCMock) ResumeContainer(sandboxID, containerID string) error { return fmt.Errorf("%s: %s (%+v): sandboxID: %v, containerID: %v", mockErrorPrefix, getSelf(), m, sandboxID, containerID) } + +// AddDevice implements the VC function of the same name. +func (m *VCMock) AddDevice(sandboxID string, info config.DeviceInfo) (api.Device, error) { + if m.AddDeviceFunc != nil { + return m.AddDeviceFunc(sandboxID, info) + } + + return nil, fmt.Errorf("%s: %s (%+v): sandboxID: %v", mockErrorPrefix, getSelf(), m, sandboxID) +} diff --git a/virtcontainers/pkg/vcmock/sandbox.go b/virtcontainers/pkg/vcmock/sandbox.go index 68b40fc36a..7f27888dd4 100644 --- a/virtcontainers/pkg/vcmock/sandbox.go +++ b/virtcontainers/pkg/vcmock/sandbox.go @@ -10,6 +10,8 @@ import ( "syscall" vc "github.com/kata-containers/runtime/virtcontainers" + "github.com/kata-containers/runtime/virtcontainers/device/api" + "github.com/kata-containers/runtime/virtcontainers/device/config" specs "github.com/opencontainers/runtime-spec/specs-go" ) @@ -138,3 +140,8 @@ func (s *Sandbox) WinsizeProcess(containerID, processID string, height, width ui func (s *Sandbox) IOStream(containerID, processID string) (io.WriteCloser, io.Reader, io.Reader, error) { return nil, nil, nil, nil } + +// AddDevice adds a device to sandbox +func (s *Sandbox) AddDevice(info config.DeviceInfo) (api.Device, error) { + return nil, nil +} diff --git a/virtcontainers/pkg/vcmock/types.go b/virtcontainers/pkg/vcmock/types.go index 83b6546ccf..c04f2a7cd0 100644 --- a/virtcontainers/pkg/vcmock/types.go +++ b/virtcontainers/pkg/vcmock/types.go @@ -9,6 +9,8 @@ import ( "syscall" vc "github.com/kata-containers/runtime/virtcontainers" + "github.com/kata-containers/runtime/virtcontainers/device/api" + "github.com/kata-containers/runtime/virtcontainers/device/config" specs "github.com/opencontainers/runtime-spec/specs-go" "github.com/sirupsen/logrus" ) @@ -61,4 +63,6 @@ type VCMock struct { UpdateContainerFunc func(sandboxID, containerID string, resources specs.LinuxResources) error PauseContainerFunc func(sandboxID, containerID string) error ResumeContainerFunc func(sandboxID, containerID string) error + + AddDeviceFunc func(sandboxID string, info config.DeviceInfo) (api.Device, error) } diff --git a/virtcontainers/sandbox.go b/virtcontainers/sandbox.go index fa2015966c..c2b17448a2 100644 --- a/virtcontainers/sandbox.go +++ b/virtcontainers/sandbox.go @@ -1546,3 +1546,25 @@ func (s *Sandbox) AppendDevice(device api.Device) error { } return fmt.Errorf("unsupported device type") } + +// AddDevice will add a device to sandbox +func (s *Sandbox) AddDevice(info config.DeviceInfo) (api.Device, error) { + if s.devManager == nil { + return nil, fmt.Errorf("device manager isn't initialized") + } + + b, err := s.devManager.NewDevice(info) + if err != nil { + return nil, err + } + + if err := s.devManager.AttachDevice(b.DeviceID(), s); err != nil { + return nil, err + } + + if err := s.storeSandboxDevices(); err != nil { + return nil, err + } + + return b, nil +} diff --git a/virtcontainers/sandbox_test.go b/virtcontainers/sandbox_test.go index a6124db41c..24425c9f26 100644 --- a/virtcontainers/sandbox_test.go +++ b/virtcontainers/sandbox_test.go @@ -1629,3 +1629,79 @@ func TestAttachBlockDevice(t *testing.T) { err = device.Detach(sandbox) assert.Nil(t, err) } + +func TestPreAddDevice(t *testing.T) { + fs := &filesystem{} + hypervisor := &mockHypervisor{} + + hConfig := HypervisorConfig{ + BlockDeviceDriver: VirtioBlock, + } + + sconfig := &SandboxConfig{ + HypervisorConfig: hConfig, + } + + dm := manager.NewDeviceManager(VirtioBlock, nil) + // create a sandbox first + sandbox := &Sandbox{ + id: testSandboxID, + storage: fs, + hypervisor: hypervisor, + config: sconfig, + devManager: dm, + } + + contID := "100" + container := Container{ + sandbox: sandbox, + id: contID, + sandboxID: testSandboxID, + } + container.state.State = StateReady + + // create state file + path := filepath.Join(runStoragePath, testSandboxID, container.ID()) + err := os.MkdirAll(path, dirMode) + if err != nil { + t.Fatal(err) + } + + defer os.RemoveAll(path) + + stateFilePath := filepath.Join(path, stateFile) + os.Remove(stateFilePath) + + _, err = os.Create(stateFilePath) + if err != nil { + t.Fatal(err) + } + defer os.Remove(stateFilePath) + + path = "/dev/hda" + deviceInfo := config.DeviceInfo{ + HostPath: path, + ContainerPath: path, + DevType: "b", + } + + // Add a mount device for a mountpoint before container's creation + dev, err := sandbox.AddDevice(deviceInfo) + assert.Nil(t, err) + + // in Frakti use case, here we will create and start the container + // which will attach same device twice + container.mounts = []Mount{ + { + Destination: path, + Source: path, + Type: "bind", + BlockDeviceID: dev.DeviceID(), + }, + } + + mounts, err := container.mountSharedDirMounts("", "") + assert.Nil(t, err) + assert.Equal(t, len(mounts), 0, + "mounts should contain nothing because it only contains a block device") +}