Use entire 32 bit range

Author: meroton-benjamin

pkg/filesystem/pool/block_device_backed_file_pool.go

@@ -3,6 +3,7 @@ package pool
 import (
 	"fmt"
 	"io"
+	"math"
 
 	"github.com/buildbarn/bb-storage/pkg/blockdevice"
 	"github.com/buildbarn/bb-storage/pkg/filesystem"
@@ -138,26 +139,37 @@ func (f *blockDeviceBackedFile) GetNextRegionOffset(off int64, regionType filesy
 	}
 
 	sectorSizeBytes := int64(f.fp.sectorSizeBytes)
-	sectorIndex := int(off / sectorSizeBytes)
+	sectorIndex := uint32(off / sectorSizeBytes)
 	switch regionType {
 	case filesystem.Data:
-		index, err := f.sectorMapper.GetNextMappedSector(uint32(sectorIndex))
-		if err == io.EOF {
+		if sectorIndex >= f.sectorMapper.GetLogicalSize() {
 			// Inside the hole at the end of the file.
 			return 0, io.EOF
-		} else if err != nil {
-			return 0, status.Errorf(codes.Internal, "Failed to get next mapped sector from index %d: %v", sectorIndex, err)
+		}
+		if f.sectorMapper.GetPhysicalIndex(sectorIndex) != 0 {
+			// Already inside a sector containing data.
+			return off, nil
+		}
+		// Find the next sector containing data.
+		index, err := f.sectorMapper.GetNextMappedSector(uint32(sectorIndex))
+		if err != nil {
+			return 0, status.Errorf(codes.Internal, "Failed to get next mapped sector: %v", err)
 		}
 		return int64(index) * sectorSizeBytes, nil
 	case filesystem.Hole:
+		if f.sectorMapper.GetPhysicalIndex(uint32(sectorIndex)) == 0 {
+			// Already inside a hole.
+			return off, nil
+		}
+		// Find the next sector containing a hole.
 		index, err := f.sectorMapper.GetNextUnmappedSector(uint32(sectorIndex))
-		if err != nil {
-			// This should not be possible as we know that we're inside of the file.
-			return 0, status.Errorf(codes.Internal, "Failed to get next unmapped sector from index %d: %v", sectorIndex, err)
+		if err == io.EOF {
+			// This can happen if the last logical possible
+			// sector index is mapped.
+			index = math.MaxUint32
 		}
-		// Correct for file ending in the middle of a sector containing data
-		offset := min(int64(f.sizeBytes), int64(index)*sectorSizeBytes)
-		return offset, nil
+		// Correct for file ending in the middle of a sector containing data.
+		return min(int64(f.sizeBytes), int64(index)*sectorSizeBytes), nil
 	default:
 		panic("Unknown region type")
 	}
@@ -265,6 +277,10 @@ func (f *blockDeviceBackedFile) Truncate(size int64) error {
 	if size < 0 {
 		return status.Errorf(codes.InvalidArgument, "Negative truncation size: %d", size)
 	}
+	maxFileSize := int64(f.fp.sectorSizeBytes) * int64(math.MaxUint32)
+	if size > maxFileSize {
+		return status.Errorf(codes.InvalidArgument, "Truncation size %d exceeds maximum file size allowed by file system %d", size, maxFileSize)
+	}
 
 	sectorIndex := uint32(size / int64(f.fp.sectorSizeBytes))
 	offsetWithinSector := int(size % int64(f.fp.sectorSizeBytes))
@@ -399,6 +415,10 @@ func (f *blockDeviceBackedFile) WriteAt(p []byte, off int64) (int, error) {
 	if len(p) == 0 {
 		return 0, nil
 	}
+	maxFileSize := int64(f.fp.sectorSizeBytes) * int64(math.MaxUint32)
+	if off+int64(len(p)) > maxFileSize {
+		return 0, status.Errorf(codes.InvalidArgument, "Write exceeds maximum file size: %d", maxFileSize)
+	}
 
 	// As the file may be stored on disk non-contiguously or may be
 	// a sparse file with holes, the write may need to be decomposed

pkg/filesystem/pool/block_device_backed_file_pool_test.go

@@ -22,10 +22,21 @@ func TestBlockDeviceBackedFilePool(t *testing.T) {
 
 	blockDevice := mock.NewMockBlockDevice(ctrl)
 	sectorAllocator := mock.NewMockSectorAllocator(ctrl)
+	freedSectors := make([]uint32, 0, 16)
+	sectorAllocator.EXPECT().FreeList(gomock.Any()).DoAndReturn(func(sectors []uint32) {
+		toFree := make([]uint32, 0, len(sectors))
+		for _, s := range sectors {
+			if s != 0 {
+				toFree = append(toFree, s)
+			}
+		}
+		freedSectors = append(freedSectors, toFree...)
+	}).AnyTimes()
 	pool := pool.NewBlockDeviceBackedFilePool(blockDevice, sectorAllocator, 16)
 
 	t.Run("ReadEmptyFile", func(t *testing.T) {
 		// Test that reads on an empty file work as expected.
+		freedSectors = freedSectors[:0]
 		f, err := pool.NewFile()
 		require.NoError(t, err)
 
@@ -50,11 +61,12 @@ func TestBlockDeviceBackedFilePool(t *testing.T) {
 		require.Equal(t, 0, n)
 		require.Equal(t, io.EOF, err)
 
-		sectorAllocator.EXPECT().FreeList(make([]uint32, 12))
+		require.Equal(t, []uint32{}, freedSectors)
 		require.NoError(t, f.Close())
 	})
 
 	t.Run("Truncate", func(t *testing.T) {
+		freedSectors = freedSectors[:0]
 		f, err := pool.NewFile()
 		require.NoError(t, err)
 
@@ -96,15 +108,13 @@ func TestBlockDeviceBackedFilePool(t *testing.T) {
 
 		// Continuing to shrink the file should eventually cause
 		// the final sector to be released.
-		sectorAllocator.EXPECT().FreeList([]uint32{5})
 		require.NoError(t, f.Truncate(80))
-		// Closing of the file should cause the direct sector
-		// list to be released.
-		sectorAllocator.EXPECT().FreeList(make([]uint32, 12))
+		require.Equal(t, []uint32{5}, freedSectors)
 		require.NoError(t, f.Close())
 	})
 
 	t.Run("WritesAndReadOnSingleSector", func(t *testing.T) {
+		freedSectors = freedSectors[:0]
 		f, err := pool.NewFile()
 		require.NoError(t, err)
 
@@ -137,11 +147,12 @@ func TestBlockDeviceBackedFilePool(t *testing.T) {
 		require.Equal(t, io.EOF, err)
 		require.Equal(t, []byte("\x00\x00Hello\x00world"), buf[:n])
 
-		sectorAllocator.EXPECT().FreeList([]uint32{12})
 		require.NoError(t, f.Close())
+		require.Equal(t, []uint32{12}, freedSectors)
 	})
 
 	t.Run("WriteFragmentation", func(t *testing.T) {
+		freedSectors = freedSectors[:0]
 		f, err := pool.NewFile()
 		require.NoError(t, err)
 
@@ -170,11 +181,12 @@ func TestBlockDeviceBackedFilePool(t *testing.T) {
 		require.Equal(t, 137, n)
 		require.NoError(t, err)
 
-		sectorAllocator.EXPECT().FreeList([]uint32{0, 0, 75, 76, 77, 21, 22, 23, 24, 105, 106, 40})
 		require.NoError(t, f.Close())
+		require.Equal(t, []uint32{75, 76, 77, 21, 22, 23, 24, 105, 106, 40}, freedSectors)
 	})
 
 	t.Run("WriteSectorAllocatorFailure", func(t *testing.T) {
+		freedSectors = freedSectors[:0]
 		f, err := pool.NewFile()
 		require.NoError(t, err)
 
@@ -190,11 +202,12 @@ func TestBlockDeviceBackedFilePool(t *testing.T) {
 		require.Equal(t, 6, n)
 		require.Equal(t, status.Error(codes.ResourceExhausted, "Out of storage space"), err)
 
-		sectorAllocator.EXPECT().FreeList([]uint32{0, 0, 75})
 		require.NoError(t, f.Close())
+		require.Equal(t, []uint32{75}, freedSectors)
 	})
 
 	t.Run("WriteIOFailure", func(t *testing.T) {
+		freedSectors = freedSectors[:0]
 		f, err := pool.NewFile()
 		require.NoError(t, err)
 
@@ -212,12 +225,13 @@ func TestBlockDeviceBackedFilePool(t *testing.T) {
 		require.Equal(t, 6, n)
 		require.Equal(t, status.Error(codes.Internal, "Disk failure"), err)
 
-		sectorAllocator.EXPECT().FreeList([]uint32{0, 0, 75})
 		require.NoError(t, f.Close())
+		require.Equal(t, []uint32{75}, freedSectors)
 	})
 
 	t.Run("GetNextRegionOffset", func(t *testing.T) {
 		// Test the behavior on empty files.
+		freedSectors = freedSectors[:0]
 		f, err := pool.NewFile()
 		require.NoError(t, err)
 
@@ -311,8 +325,8 @@ func TestBlockDeviceBackedFilePool(t *testing.T) {
 		_, err = f.GetNextRegionOffset(384, filesystem.Hole)
 		require.Equal(t, io.EOF, err)
 
-		sectorAllocator.EXPECT().FreeList([]uint32{0, 0, 0, 0, 0, 0, 0, 0, 5})
 		require.NoError(t, f.Close())
+		require.Equal(t, []uint32{5}, freedSectors)
 	})
 
 	t.Run("WriteAt", func(t *testing.T) {