From 77deeed965a67266044720ca6de0a85c9938ab39 Mon Sep 17 00:00:00 2001 From: Li Yuxuan Date: Wed, 28 Aug 2019 19:44:21 +0800 Subject: [PATCH] vc: Use BlockIndexMap instead of BlockIndex This allows to reuse detached block index and ensures that the index will not reach the limit of device(such as `maxSCSIDevices`) after restarting containers many times in one pod. Fixes: #2007 Signed-off-by: Li Yuxuan --- virtcontainers/acrn_test.go | 1 + virtcontainers/api_test.go | 4 +- virtcontainers/device/api/interface.go | 2 +- .../device/api/mockDeviceReceiver.go | 2 +- virtcontainers/device/drivers/block.go | 4 +- virtcontainers/persist.go | 6 +-- virtcontainers/persist/api/hypervisor.go | 6 +-- virtcontainers/persist_test.go | 13 ++++-- virtcontainers/sandbox.go | 43 +++++++++++-------- virtcontainers/sandbox_test.go | 12 ++++++ virtcontainers/types/sandbox.go | 4 +- 11 files changed, 64 insertions(+), 33 deletions(-) diff --git a/virtcontainers/acrn_test.go b/virtcontainers/acrn_test.go index 52cba5521a..d39b9e10c9 100644 --- a/virtcontainers/acrn_test.go +++ b/virtcontainers/acrn_test.go @@ -216,6 +216,7 @@ func TestAcrnCreateSandbox(t *testing.T) { config: &SandboxConfig{ HypervisorConfig: acrnConfig, }, + state: types.SandboxState{BlockIndexMap: make(map[int]struct{})}, } err := globalSandboxList.addSandbox(sandbox) diff --git a/virtcontainers/api_test.go b/virtcontainers/api_test.go index 70ecfb928b..525b44022d 100644 --- a/virtcontainers/api_test.go +++ b/virtcontainers/api_test.go @@ -528,7 +528,8 @@ func TestStatusSandboxSuccessfulStateReady(t *testing.T) { expectedStatus := SandboxStatus{ ID: testSandboxID, State: types.SandboxState{ - State: types.StateReady, + State: types.StateReady, + BlockIndexMap: make(map[int]struct{}), PersistVersion: 2, }, Hypervisor: MockHypervisor, @@ -588,6 +589,7 @@ func TestStatusSandboxSuccessfulStateRunning(t *testing.T) { ID: testSandboxID, State: types.SandboxState{ State: types.StateRunning, + BlockIndexMap: make(map[int]struct{}), PersistVersion: 2, }, Hypervisor: MockHypervisor, diff --git a/virtcontainers/device/api/interface.go b/virtcontainers/device/api/interface.go index f4060c046a..bb1c30d2cb 100644 --- a/virtcontainers/device/api/interface.go +++ b/virtcontainers/device/api/interface.go @@ -34,7 +34,7 @@ type DeviceReceiver interface { // this is only for virtio-blk and virtio-scsi support GetAndSetSandboxBlockIndex() (int, error) - DecrementSandboxBlockIndex() error + UnsetSandboxBlockIndex(int) error GetHypervisorType() string // this is for appending device to hypervisor boot params diff --git a/virtcontainers/device/api/mockDeviceReceiver.go b/virtcontainers/device/api/mockDeviceReceiver.go index e7d937deda..c41026271a 100644 --- a/virtcontainers/device/api/mockDeviceReceiver.go +++ b/virtcontainers/device/api/mockDeviceReceiver.go @@ -28,7 +28,7 @@ func (mockDC *MockDeviceReceiver) GetAndSetSandboxBlockIndex() (int, error) { } // DecrementSandboxBlockIndex decreases virtio-blk index by one -func (mockDC *MockDeviceReceiver) DecrementSandboxBlockIndex() error { +func (mockDC *MockDeviceReceiver) UnsetSandboxBlockIndex(int) error { return nil } diff --git a/virtcontainers/device/drivers/block.go b/virtcontainers/device/drivers/block.go index 0be1d6afe0..62d55f6493 100644 --- a/virtcontainers/device/drivers/block.go +++ b/virtcontainers/device/drivers/block.go @@ -51,7 +51,7 @@ func (device *BlockDevice) Attach(devReceiver api.DeviceReceiver) (err error) { defer func() { if err != nil { - devReceiver.DecrementSandboxBlockIndex() + devReceiver.UnsetSandboxBlockIndex(index) device.bumpAttachCount(false) } }() @@ -127,6 +127,8 @@ func (device *BlockDevice) Detach(devReceiver api.DeviceReceiver) error { defer func() { if err != nil { device.bumpAttachCount(true) + } else { + devReceiver.UnsetSandboxBlockIndex(device.BlockDrive.Index) } }() diff --git a/virtcontainers/persist.go b/virtcontainers/persist.go index f964fac02e..e0bd83a7aa 100644 --- a/virtcontainers/persist.go +++ b/virtcontainers/persist.go @@ -63,8 +63,8 @@ func (s *Sandbox) dumpState(ss *persistapi.SandboxState, cs map[string]persistap func (s *Sandbox) dumpHypervisor(ss *persistapi.SandboxState) { ss.HypervisorState = s.hypervisor.save() - // BlockIndex will be moved from sandbox state to hypervisor state later - ss.HypervisorState.BlockIndex = s.state.BlockIndex + // BlockIndexMap will be moved from sandbox state to hypervisor state later + ss.HypervisorState.BlockIndexMap = s.state.BlockIndexMap } func deviceToDeviceState(devices []api.Device) (dss []persistapi.DeviceState) { @@ -316,7 +316,7 @@ func (s *Sandbox) Save() error { func (s *Sandbox) loadState(ss persistapi.SandboxState) { s.state.PersistVersion = ss.PersistVersion s.state.GuestMemoryBlockSizeMB = ss.GuestMemoryBlockSizeMB - s.state.BlockIndex = ss.HypervisorState.BlockIndex + s.state.BlockIndexMap = ss.HypervisorState.BlockIndexMap s.state.State = types.StateString(ss.State) s.state.CgroupPath = ss.CgroupPath s.state.CgroupPaths = ss.CgroupPaths diff --git a/virtcontainers/persist/api/hypervisor.go b/virtcontainers/persist/api/hypervisor.go index b64c32e0cb..0a48a1911a 100644 --- a/virtcontainers/persist/api/hypervisor.go +++ b/virtcontainers/persist/api/hypervisor.go @@ -29,9 +29,9 @@ type CPUDevice struct { type HypervisorState struct { Pid int // Type of hypervisor, E.g. qemu/firecracker/acrn. - Type string - BlockIndex int - UUID string + Type string + BlockIndexMap map[int]struct{} + UUID string // Belows are qemu specific // Refs: virtcontainers/qemu.go:QemuState diff --git a/virtcontainers/persist_test.go b/virtcontainers/persist_test.go index 03b6252f41..4d40cfd90c 100644 --- a/virtcontainers/persist_test.go +++ b/virtcontainers/persist_test.go @@ -35,6 +35,7 @@ func TestSandboxRestore(t *testing.T) { hypervisor: &mockHypervisor{}, ctx: context.Background(), config: &sconfig, + state: types.SandboxState{BlockIndexMap: make(map[int]struct{})}, } sandbox.newStore, err = persist.GetDriver("fs") @@ -54,23 +55,27 @@ func TestSandboxRestore(t *testing.T) { assert.NoError(err) assert.Equal(sandbox.state.State, types.StateString("")) assert.Equal(sandbox.state.GuestMemoryBlockSizeMB, uint32(0)) - assert.Equal(sandbox.state.BlockIndex, 0) + assert.Equal(len(sandbox.state.BlockIndexMap), 0) // set state data and save again sandbox.state.State = types.StateString("running") sandbox.state.GuestMemoryBlockSizeMB = uint32(1024) - sandbox.state.BlockIndex = 2 + sandbox.state.BlockIndexMap[2] = struct{}{} // flush data to disk err = sandbox.Save() assert.Nil(err) // empty the sandbox sandbox.state = types.SandboxState{} + if sandbox.newStore, err = persist.GetDriver("fs"); err != nil || sandbox.newStore == nil { + t.Fatalf("failed to get fs persist driver") + } // restore data from disk err = sandbox.Restore() - assert.Nil(err) + assert.NoError(err) assert.Equal(sandbox.state.State, types.StateString("running")) assert.Equal(sandbox.state.GuestMemoryBlockSizeMB, uint32(1024)) - assert.Equal(sandbox.state.BlockIndex, 2) + assert.Equal(len(sandbox.state.BlockIndexMap), 1) + assert.Equal(sandbox.state.BlockIndexMap[2], struct{}{}) } diff --git a/virtcontainers/sandbox.go b/virtcontainers/sandbox.go index ce5ec68223..629c6cd3d5 100644 --- a/virtcontainers/sandbox.go +++ b/virtcontainers/sandbox.go @@ -525,7 +525,7 @@ func newSandbox(ctx context.Context, sandboxConfig SandboxConfig, factory Factor config: &sandboxConfig, volumes: sandboxConfig.Volumes, containers: map[string]*Container{}, - state: types.SandboxState{}, + state: types.SandboxState{BlockIndexMap: make(map[int]struct{})}, annotationsLock: &sync.RWMutex{}, wg: &sync.WaitGroup{}, shmSize: sandboxConfig.ShmSize, @@ -1533,33 +1533,42 @@ func (s *Sandbox) setSandboxState(state types.StateString) error { return nil } -// getAndSetSandboxBlockIndex retrieves sandbox block index and increments it for -// subsequent accesses. This index is used to maintain the index at which a -// block device is assigned to a container in the sandbox. +const maxBlockIndex = 65535 + +// getAndSetSandboxBlockIndex retrieves an unused sandbox block index from +// the BlockIndexMap and marks it as used. This index is used to maintain the +// index at which a block device is assigned to a container in the sandbox. func (s *Sandbox) getAndSetSandboxBlockIndex() (int, error) { - currentIndex := s.state.BlockIndex var err error + currentIndex := -1 + for i := 0; i < maxBlockIndex; i++ { + if _, ok := s.state.BlockIndexMap[i]; !ok { + currentIndex = i + break + } + } + if currentIndex == -1 { + return -1, errors.New("no available block index") + } + s.state.BlockIndexMap[currentIndex] = struct{}{} defer func() { if err != nil { - s.state.BlockIndex = currentIndex + delete(s.state.BlockIndexMap, currentIndex) } }() - // Increment so that container gets incremented block index - s.state.BlockIndex++ - return currentIndex, nil } -// decrementSandboxBlockIndex decrements the current sandbox block index. +// unsetSandboxBlockIndex deletes the current sandbox block index from BlockIndexMap. // This is used to recover from failure while adding a block device. -func (s *Sandbox) decrementSandboxBlockIndex() error { +func (s *Sandbox) unsetSandboxBlockIndex(index int) error { var err error - original := s.state.BlockIndex - s.state.BlockIndex-- + original := index + delete(s.state.BlockIndexMap, index) defer func() { if err != nil { - s.state.BlockIndex = original + s.state.BlockIndexMap[original] = struct{}{} } }() @@ -1649,10 +1658,10 @@ func (s *Sandbox) GetAndSetSandboxBlockIndex() (int, error) { return s.getAndSetSandboxBlockIndex() } -// DecrementSandboxBlockIndex decrease block indexes +// UnsetSandboxBlockIndex unsets block indexes // Sandbox implement DeviceReceiver interface from device/api/interface.go -func (s *Sandbox) DecrementSandboxBlockIndex() error { - return s.decrementSandboxBlockIndex() +func (s *Sandbox) UnsetSandboxBlockIndex(index int) error { + return s.unsetSandboxBlockIndex(index) } // AppendDevice can only handle vhost user device currently, it adds a diff --git a/virtcontainers/sandbox_test.go b/virtcontainers/sandbox_test.go index f47ad857a0..b23a6a7784 100644 --- a/virtcontainers/sandbox_test.go +++ b/virtcontainers/sandbox_test.go @@ -1151,6 +1151,7 @@ func TestAttachBlockDevice(t *testing.T) { hypervisor: hypervisor, config: sconfig, ctx: context.Background(), + state: types.SandboxState{BlockIndexMap: make(map[int]struct{})}, } contID := "100" @@ -1180,11 +1181,21 @@ func TestAttachBlockDevice(t *testing.T) { assert.True(t, ok) container.state.State = "" + index, err := sandbox.getAndSetSandboxBlockIndex() + assert.Nil(t, err) + assert.Equal(t, index, 0) + err = device.Attach(sandbox) assert.Nil(t, err) + index, err = sandbox.getAndSetSandboxBlockIndex() + assert.Nil(t, err) + assert.Equal(t, index, 2) err = device.Detach(sandbox) assert.Nil(t, err) + index, err = sandbox.getAndSetSandboxBlockIndex() + assert.Nil(t, err) + assert.Equal(t, index, 1) container.state.State = types.StateReady err = device.Attach(sandbox) @@ -1227,6 +1238,7 @@ func TestPreAddDevice(t *testing.T) { config: sconfig, devManager: dm, ctx: context.Background(), + state: types.SandboxState{BlockIndexMap: make(map[int]struct{})}, } contID := "100" diff --git a/virtcontainers/types/sandbox.go b/virtcontainers/types/sandbox.go index a0d766be7f..3b64b20ac9 100644 --- a/virtcontainers/types/sandbox.go +++ b/virtcontainers/types/sandbox.go @@ -39,8 +39,8 @@ const ( type SandboxState struct { State StateString `json:"state"` - // Index of the block device passed to hypervisor. - BlockIndex int `json:"blockIndex"` + // Index map of the block device passed to hypervisor. + BlockIndexMap map[int]struct{} `json:"blockIndexMap"` // GuestMemoryBlockSizeMB is the size of memory block of guestos GuestMemoryBlockSizeMB uint32 `json:"guestMemoryBlockSize"`