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: kata-containers#2007 Signed-off-by: Li Yuxuan <[email protected]>
darfux · Aug 30, 2019 · ca1dffb · ca1dffb
1 parent af57485
commit ca1dffb
Show file tree

Hide file tree

Showing 11 changed files with 52 additions and 24 deletions.
diff --git a/virtcontainers/acrn_test.go b/virtcontainers/acrn_test.go
@@ -217,6 +217,7 @@ func TestAcrnCreateSandbox(t *testing.T) {
 		config: &SandboxConfig{
 			HypervisorConfig: acrnConfig,
 		},
+		state:  types.SandboxState{BlockIndexMap: make(map[int]bool)},
 	}
 
 	vcStore, err := store.NewVCSandboxStore(sandbox.ctx, sandbox.id)

diff --git a/virtcontainers/api_test.go b/virtcontainers/api_test.go
@@ -562,6 +562,7 @@ func TestStatusSandboxSuccessfulStateReady(t *testing.T) {
 		ID: testSandboxID,
 		State: types.SandboxState{
 			State: types.StateReady,
+			BlockIndexMap:   make(map[int]bool),
 		},
 		Hypervisor:       MockHypervisor,
 		HypervisorConfig: hypervisorConfig,
@@ -620,6 +621,7 @@ func TestStatusSandboxSuccessfulStateRunning(t *testing.T) {
 		ID: testSandboxID,
 		State: types.SandboxState{
 			State: types.StateRunning,
+			BlockIndexMap:   make(map[int]bool),
 		},
 		Hypervisor:       MockHypervisor,
 		HypervisorConfig: hypervisorConfig,

diff --git 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
@@ -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
@@ -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)
 		}
 	}()
@@ -125,6 +125,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
@@ -59,8 +59,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) {
@@ -196,7 +196,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.GuestMemoryHotplugProbe = ss.GuestMemoryHotplugProbe

diff --git 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]bool
+	UUID          string
 
 	// Belows are qemu specific
 	// Refs: virtcontainers/qemu.go:QemuState

diff --git a/virtcontainers/persist_test.go b/virtcontainers/persist_test.go
@@ -77,6 +77,7 @@ func TestSandboxRestore(t *testing.T) {
 		hypervisor: &mockHypervisor{},
 		ctx:        context.Background(),
 		config:     &sconfig,
+		state:      types.SandboxState{BlockIndexMap: make(map[int]bool)},
 	}
 
 	sandbox.newStore, err = persist.GetDriver("fs")
@@ -96,23 +97,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] = true
 	// 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], true)
 }
diff --git a/virtcontainers/sandbox.go b/virtcontainers/sandbox.go
@@ -534,7 +534,7 @@ func newSandbox(ctx context.Context, sandboxConfig SandboxConfig, factory Factor
 		containers:      map[string]*Container{},
 		runPath:         store.SandboxRuntimeRootPath(sandboxConfig.ID),
 		configPath:      store.SandboxConfigurationRootPath(sandboxConfig.ID),
-		state:           types.SandboxState{},
+		state:           types.SandboxState{BlockIndexMap: make(map[int]bool)},
 		annotationsLock: &sync.RWMutex{},
 		wg:              &sync.WaitGroup{},
 		shmSize:         sandboxConfig.ShmSize,
@@ -1580,14 +1580,20 @@ func (s *Sandbox) resumeSetStates() error {
 	return s.setSandboxState(types.StateRunning)
 }
 
+const maxBlockIndex = 65535
+
 // 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.
 func (s *Sandbox) getAndSetSandboxBlockIndex() (int, error) {
-	currentIndex := s.state.BlockIndex
-
-	// Increment so that container gets incremented block index
-	s.state.BlockIndex++
+	var currentIndex int
+	for i := 0; i < maxBlockIndex; i++ {
+		if used, ok := s.state.BlockIndexMap[i]; !ok || !used {
+			currentIndex = i
+			break
+		}
+	}
+	s.state.BlockIndexMap[currentIndex] = true
 
 	if !s.supportNewStore() {
 		// experimental runtime use "persist.json" which doesn't need "state.json" anymore
@@ -1602,8 +1608,8 @@ func (s *Sandbox) getAndSetSandboxBlockIndex() (int, error) {
 
 // decrementSandboxBlockIndex decrements the current sandbox block index.
 // This is used to recover from failure while adding a block device.
-func (s *Sandbox) decrementSandboxBlockIndex() error {
-	s.state.BlockIndex--
+func (s *Sandbox) unsetSandboxBlockIndex(index int) error {
+	delete(s.state.BlockIndexMap, index)
 
 	if !s.supportNewStore() {
 		// experimental runtime use "persist.json" which doesn't need "state.json" anymore
@@ -1750,8 +1756,8 @@ func (s *Sandbox) GetAndSetSandboxBlockIndex() (int, error) {
 
 // DecrementSandboxBlockIndex decrease 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
@@ -1162,6 +1162,7 @@ func TestAttachBlockDevice(t *testing.T) {
 		hypervisor: hypervisor,
 		config:     sconfig,
 		ctx:        context.Background(),
+		state:      types.SandboxState{BlockIndexMap: make(map[int]bool)},
 	}
 
 	vcStore, err := store.NewVCSandboxStore(sandbox.ctx, sandbox.id)
@@ -1202,11 +1203,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)
@@ -1249,6 +1260,7 @@ func TestPreAddDevice(t *testing.T) {
 		config:     sconfig,
 		devManager: dm,
 		ctx:        context.Background(),
+		state:      types.SandboxState{BlockIndexMap: make(map[int]bool)},
 	}
 
 	vcStore, err := store.NewVCSandboxStore(sandbox.ctx, sandbox.id)

diff --git a/virtcontainers/types/sandbox.go b/virtcontainers/types/sandbox.go
@@ -31,8 +31,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]bool `json:"blockIndexMap"`
 
 	// GuestMemoryBlockSizeMB is the size of memory block of guestos
 	GuestMemoryBlockSizeMB uint32 `json:"guestMemoryBlockSize"`