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 <liyuxuan04@baidu.com>

virtcontainers/acrn_test.go

@@ -226,6 +226,7 @@ func TestAcrnCreateSandbox(t *testing.T) {
 		config: &SandboxConfig{
 			HypervisorConfig: acrnConfig,
 		},
+		state: types.SandboxState{BlockIndexMap: make(map[int]struct{})},
 	}
 
 	err = globalSandboxList.addSandbox(sandbox)

virtcontainers/api_test.go

@@ -535,6 +535,7 @@ func TestStatusSandboxSuccessfulStateReady(t *testing.T) {
 		ID: testSandboxID,
 		State: types.SandboxState{
 			State:          types.StateReady,
+			BlockIndexMap:  make(map[int]struct{}),
 			PersistVersion: 2,
 		},
 		Hypervisor:       MockHypervisor,
@@ -594,6 +595,7 @@ func TestStatusSandboxSuccessfulStateRunning(t *testing.T) {
 		ID: testSandboxID,
 		State: types.SandboxState{
 			State:          types.StateRunning,
+			BlockIndexMap:  make(map[int]struct{}),
 			PersistVersion: 2,
 		},
 		Hypervisor:       MockHypervisor,

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

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
 }
 

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)
 		}
 	}()
 

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
+	// BlockIndexMap will be moved from sandbox state to hypervisor state later
-	ss.HypervisorState.BlockIndex = s.state.BlockIndex
+	ss.HypervisorState.BlockIndexMap = s.state.BlockIndexMap
 }
 
 func deviceToDeviceState(devices []api.Device) (dss []persistapi.DeviceState) {
@@ -318,7 +318,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

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
+	Type          string
-	BlockIndex int
+	BlockIndexMap map[int]struct{}
-	UUID       string
+	UUID          string
 
 	// Belows are qemu specific
 	// Refs: virtcontainers/qemu.go:QemuState

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()
@@ -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(); err != nil || sandbox.newStore == nil {
+		t.Fatal("failed to get 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{}{})
 }

virtcontainers/sandbox.go

@@ -531,7 +531,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,
@@ -1583,33 +1583,42 @@ func (s *Sandbox) setSandboxState(state types.StateString) error {
 	return nil
 }
 
-// getAndSetSandboxBlockIndex retrieves sandbox block index and increments it for
+const maxBlockIndex = 65535
-// subsequent accesses. This index is used to maintain the index at which a
+
-// block device is assigned to a container in the sandbox.
+// 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
+	original := index
-	s.state.BlockIndex--
+	delete(s.state.BlockIndexMap, index)
 	defer func() {
 		if err != nil {
-			s.state.BlockIndex = original
+			s.state.BlockIndexMap[original] = struct{}{}
 		}
 	}()
 
@@ -1699,10 +1708,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 {
+func (s *Sandbox) UnsetSandboxBlockIndex(index int) error {
-	return s.decrementSandboxBlockIndex()
+	return s.unsetSandboxBlockIndex(index)
 }
 
 // AppendDevice can only handle vhost user device currently, it adds a

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"

virtcontainers/types/sandbox.go

@@ -39,8 +39,8 @@ const (
 type SandboxState struct {
 	State StateString `json:"state"`
 
-	// Index of the block device passed to hypervisor.
+	// Index map of the block device passed to hypervisor.
-	BlockIndex int `json:"blockIndex"`
+	BlockIndexMap map[int]struct{} `json:"blockIndexMap"`
 
 	// GuestMemoryBlockSizeMB is the size of memory block of guestos
 	GuestMemoryBlockSizeMB uint32 `json:"guestMemoryBlockSize"`