Use /internal/memory constants (#1354)

* Use /internal/memory constants

We have a bunch of 1024 * 1024 or 1024 * 1024 * 1024 numerical constants
(or just other megabyte constants) lying around. This changes to using
the constants we have defined in the /internal/memory package.

This additionally changes the names of the constants from MegaByte/GigaByte to MiB/GiB and
changes them to untyped constants.

Signed-off-by: Daniel Canter <dcanter@microsoft.com>

cmd/containerd-shim-runhcs-v1/delete.go

@@ -10,6 +10,7 @@ import (
 	"time"
 
 	"github.com/Microsoft/hcsshim/internal/hcs"
+	"github.com/Microsoft/hcsshim/internal/memory"
 	"github.com/Microsoft/hcsshim/internal/oc"
 	"github.com/Microsoft/hcsshim/internal/winapi"
 	"github.com/containerd/containerd/runtime/v2/task"
@@ -69,7 +70,7 @@ The delete command will be executed in the container's bundle as its cwd.
 		// This should be done as the first thing so that we don't miss any panic logs even if
 		// something goes wrong during delete op.
 		// The file can be very large so read only first 1MB of data.
-		readLimit := int64(1024 * 1024) // 1MB
+		readLimit := int64(memory.MiB) // 1MB
 		logBytes, err := limitedRead(filepath.Join(bundleFlag, "panic.log"), readLimit)
 		if err == nil && len(logBytes) > 0 {
 			if int64(len(logBytes)) == readLimit {

cmd/containerd-shim-runhcs-v1/task_hcs.go

@@ -31,6 +31,7 @@ import (
 	"github.com/Microsoft/hcsshim/internal/hcsoci"
 	"github.com/Microsoft/hcsshim/internal/jobcontainers"
 	"github.com/Microsoft/hcsshim/internal/log"
+	"github.com/Microsoft/hcsshim/internal/memory"
 	"github.com/Microsoft/hcsshim/internal/oci"
 	"github.com/Microsoft/hcsshim/internal/processorinfo"
 	"github.com/Microsoft/hcsshim/internal/protocol/guestrequest"
@@ -42,8 +43,6 @@ import (
 	"github.com/Microsoft/hcsshim/pkg/annotations"
 )
 
-const bytesPerMB = 1024 * 1024
-
 func newHcsStandaloneTask(ctx context.Context, events publisher, req *task.CreateTaskRequest, s *specs.Spec) (shimTask, error) {
 	log.G(ctx).WithField("tid", req.ID).Debug("newHcsStandaloneTask")
 
@@ -1008,7 +1007,7 @@ func (ht *hcsTask) updateWCOWResources(ctx context.Context, data interface{}, an
 		return errors.New("must have resources be type *WindowsResources when updating a wcow container")
 	}
 	if resources.Memory != nil && resources.Memory.Limit != nil {
-		newMemorySizeInMB := *resources.Memory.Limit / bytesPerMB
+		newMemorySizeInMB := *resources.Memory.Limit / memory.MiB
 		memoryLimit := hcsoci.NormalizeMemorySize(ctx, ht.id, newMemorySizeInMB)
 		if err := ht.requestUpdateContainer(ctx, resourcepaths.SiloMemoryResourcePath, memoryLimit); err != nil {
 			return err

computestorage/helpers.go

@@ -10,6 +10,7 @@ import (
 
 	"github.com/Microsoft/go-winio/pkg/security"
 	"github.com/Microsoft/go-winio/vhd"
+	"github.com/Microsoft/hcsshim/internal/memory"
 	"github.com/pkg/errors"
 	"golang.org/x/sys/windows"
 )
@@ -61,8 +62,8 @@ func SetupContainerBaseLayer(ctx context.Context, layerPath, baseVhdPath, diffVh
 	createParams := &vhd.CreateVirtualDiskParameters{
 		Version: 2,
 		Version2: vhd.CreateVersion2{
-			MaximumSize:      sizeInGB * 1024 * 1024 * 1024,
-			BlockSizeInBytes: defaultVHDXBlockSizeInMB * 1024 * 1024,
+			MaximumSize:      sizeInGB * memory.GiB,
+			BlockSizeInBytes: defaultVHDXBlockSizeInMB * memory.MiB,
 		},
 	}
 	handle, err := vhd.CreateVirtualDisk(baseVhdPath, vhd.VirtualDiskAccessNone, vhd.CreateVirtualDiskFlagNone, createParams)
@@ -137,8 +138,8 @@ func SetupUtilityVMBaseLayer(ctx context.Context, uvmPath, baseVhdPath, diffVhdP
 	createParams := &vhd.CreateVirtualDiskParameters{
 		Version: 2,
 		Version2: vhd.CreateVersion2{
-			MaximumSize:      sizeInGB * 1024 * 1024 * 1024,
-			BlockSizeInBytes: defaultVHDXBlockSizeInMB * 1024 * 1024,
+			MaximumSize:      sizeInGB * memory.GiB,
+			BlockSizeInBytes: defaultVHDXBlockSizeInMB * memory.MiB,
 		},
 	}
 	handle, err := vhd.CreateVirtualDisk(baseVhdPath, vhd.VirtualDiskAccessNone, vhd.CreateVirtualDiskFlagNone, createParams)

ext4/dmverity/dmverity.go

@@ -14,14 +14,15 @@ import (
 	"github.com/pkg/errors"
 
 	"github.com/Microsoft/hcsshim/ext4/internal/compactext4"
+	"github.com/Microsoft/hcsshim/internal/memory"
 )
 
 const (
 	blockSize = compactext4.BlockSize
 	// MerkleTreeBufioSize is a default buffer size to use with bufio.Reader
-	MerkleTreeBufioSize = 1024 * 1024 // 1MB
+	MerkleTreeBufioSize = memory.MiB // 1MB
 	// RecommendedVHDSizeGB is the recommended size in GB for VHDs, which is not a hard limit.
-	RecommendedVHDSizeGB = 128 * 1024 * 1024 * 1024
+	RecommendedVHDSizeGB = 128 * memory.GiB
 	// VeritySignature is a value written to dm-verity super-block.
 	VeritySignature = "verity"
 )

ext4/internal/compactext4/compact.go

@@ -13,6 +13,7 @@ import (
 	"time"
 
 	"github.com/Microsoft/hcsshim/ext4/internal/format"
+	"github.com/Microsoft/hcsshim/internal/memory"
 )
 
 // Writer writes a compact ext4 file system.
@@ -101,15 +102,15 @@ const (
 	maxInodesPerGroup       = BlockSize * 8 // Limited by the inode bitmap
 	inodesPerGroupIncrement = BlockSize / inodeSize
 
-	defaultMaxDiskSize = 16 * 1024 * 1024 * 1024        // 16GB
+	defaultMaxDiskSize = 16 * memory.GiB                // 16GB
 	maxMaxDiskSize     = 16 * 1024 * 1024 * 1024 * 1024 // 16TB
 
 	groupDescriptorSize      = 32 // Use the small group descriptor
 	groupsPerDescriptorBlock = BlockSize / groupDescriptorSize
 
-	maxFileSize             = 128 * 1024 * 1024 * 1024 // 128GB file size maximum for now
-	smallSymlinkSize        = 59                       // max symlink size that goes directly in the inode
-	maxBlocksPerExtent      = 0x8000                   // maximum number of blocks in an extent
+	maxFileSize             = 128 * memory.GiB // 128GB file size maximum for now
+	smallSymlinkSize        = 59               // max symlink size that goes directly in the inode
+	maxBlocksPerExtent      = 0x8000           // maximum number of blocks in an extent
 	inodeDataSize           = 60
 	inodeUsedSize           = 152 // fields through CrtimeExtra
 	inodeExtraSize          = inodeSize - inodeUsedSize

ext4/internal/compactext4/compact_test.go

@@ -11,6 +11,7 @@ import (
 	"time"
 
 	"github.com/Microsoft/hcsshim/ext4/internal/format"
+	"github.com/Microsoft/hcsshim/internal/memory"
 )
 
 type testFile struct {
@@ -318,9 +319,9 @@ func TestTime(t *testing.T) {
 
 func TestLargeFile(t *testing.T) {
 	testFiles := []testFile{
-		{Path: "small", File: &File{}, DataSize: 1024 * 1024},        // can't change type
-		{Path: "medium", File: &File{}, DataSize: 200 * 1024 * 1024}, // can't change type
-		{Path: "large", File: &File{}, DataSize: 600 * 1024 * 1024},  // can't change type
+		{Path: "small", File: &File{}, DataSize: memory.MiB},        // can't change type
+		{Path: "medium", File: &File{}, DataSize: 200 * memory.MiB}, // can't change type
+		{Path: "large", File: &File{}, DataSize: 600 * memory.MiB},  // can't change type
 	}
 	runTestsOnFiles(t, testFiles)
 }

internal/guest/storage/crypt/crypt_test.go

@@ -8,6 +8,7 @@ import (
 	"fmt"
 	"testing"
 
+	"github.com/Microsoft/hcsshim/internal/memory"
 	"github.com/pkg/errors"
 )
 
@@ -194,7 +195,7 @@ func Test_Encrypt_Create_Sparse_File_Error(t *testing.T) {
 	// Test what happens when it isn't possible to create a sparse file, and
 	// make sure that _createSparseEmptyFile receives the right arguments.
 
-	blockDeviceSize := int64(1024 * 1024 * 1024)
+	blockDeviceSize := int64(memory.GiB)
 
 	_generateKeyFile = func(path string, size int64) error {
 		return nil
@@ -246,7 +247,7 @@ func Test_Encrypt_Mkfs_Error(t *testing.T) {
 	// Test what happens when mkfs fails to format the unencrypted device.
 	// Verify that the arguments passed to it are the right ones.
 
-	blockDeviceSize := int64(1024 * 1024 * 1024)
+	blockDeviceSize := int64(memory.GiB)
 
 	_generateKeyFile = func(path string, size int64) error {
 		return nil
@@ -296,7 +297,7 @@ func Test_Encrypt_Sparse_Copy_Error(t *testing.T) {
 	// Test what happens when the sparse copy fails. Verify that the arguments
 	// passed to it are the right ones.
 
-	blockDeviceSize := int64(1024 * 1024 * 1024)
+	blockDeviceSize := int64(memory.GiB)
 
 	_generateKeyFile = func(path string, size int64) error {
 		return nil
@@ -354,7 +355,7 @@ func Test_Encrypt_Success(t *testing.T) {
 
 	// Test what happens when everything goes right.
 
-	blockDeviceSize := int64(1024 * 1024 * 1024)
+	blockDeviceSize := int64(memory.GiB)
 
 	_generateKeyFile = func(path string, size int64) error {
 		return nil

internal/guest/storage/overlay/overlay.go

@@ -44,7 +44,7 @@ func processErrNoSpace(ctx context.Context, path string, err error) {
 	used := all - free
 
 	toGigabyteStr := func(val uint64) string {
-		return fmt.Sprintf("%.1f", float64(val)/float64(memory.GigaByte))
+		return fmt.Sprintf("%.1f", float64(val)/float64(memory.GiB))
 	}
 
 	log.G(ctx).WithFields(logrus.Fields{

internal/jobcontainers/oci.go

@@ -7,6 +7,7 @@ import (
 
 	"github.com/Microsoft/hcsshim/internal/hcsoci"
 	"github.com/Microsoft/hcsshim/internal/jobobject"
+	"github.com/Microsoft/hcsshim/internal/memory"
 	"github.com/Microsoft/hcsshim/internal/processorinfo"
 	"github.com/Microsoft/hcsshim/pkg/annotations"
 
@@ -57,7 +58,7 @@ func specToLimits(ctx context.Context, cid string, s *specs.Spec) (*jobobject.Jo
 		CPUWeight:          realCPUWeight,
 		MaxIOPS:            maxIops,
 		MaxBandwidth:       maxBandwidth,
-		MemoryLimitInBytes: memLimitMB * 1024 * 1024,
+		MemoryLimitInBytes: memLimitMB * memory.MiB,
 	}, nil
 }
 

internal/memory/pool.go

@@ -5,8 +5,8 @@ import (
 )
 
 const (
-	minimumClassSize  = MegaByte
-	maximumClassSize  = 4 * GigaByte
+	minimumClassSize  = MiB
+	maximumClassSize  = 4 * GiB
 	memoryClassNumber = 7
 )
 
@@ -179,7 +179,7 @@ func (pa *PoolAllocator) findNextOffset(memCls classType) (classType, uint64, er
 			continue
 		}
 
-		target := maximumClassSize
+		target := uint64(maximumClassSize)
 		for offset := range pi.free {
 			if offset < target {
 				target = offset

internal/memory/pool_test.go

@@ -38,13 +38,13 @@ func Test_MemAlloc_allocate_without_expand(t *testing.T) {
 		offset: 0,
 	}
 
-	testAllocate(t, ma, MegaByte)
+	testAllocate(t, ma, MiB)
 }
 
 func Test_MemAlloc_allocate_not_enough_space(t *testing.T) {
 	ma := &PoolAllocator{}
 
-	_, err := ma.Allocate(MegaByte)
+	_, err := ma.Allocate(MiB)
 	if err == nil {
 		t.Fatal("expected error, got nil")
 	}
@@ -72,7 +72,7 @@ func Test_MemAlloc_expand(t *testing.T) {
 
 	poolCls0 := pa.pools[0]
 	for i := 0; i < 4; i++ {
-		offset := uint64(i) * MegaByte
+		offset := uint64(i) * MiB
 		_, ok := poolCls0.free[offset]
 		if !ok {
 			t.Fatalf("did not find region with offset=%d", offset)
@@ -88,12 +88,12 @@ func Test_MemAlloc_expand(t *testing.T) {
 func Test_MemAlloc_allocate_automatically_expands(t *testing.T) {
 	pa := &PoolAllocator{}
 	pa.pools[2] = newEmptyMemoryPool()
-	pa.pools[2].free[MegaByte] = &region{
+	pa.pools[2].free[MiB] = &region{
 		class:  2,
-		offset: MegaByte,
+		offset: MiB,
 	}
 
-	testAllocate(t, pa, MegaByte)
+	testAllocate(t, pa, MiB)
 
 	if pa.pools[1] == nil {
 		t.Fatalf("memory not extended for class type 1")
@@ -112,7 +112,7 @@ func Test_MemAlloc_alloc_and_release(t *testing.T) {
 	}
 	pa.pools[0].free[0] = r
 
-	testAllocate(t, pa, MegaByte)
+	testAllocate(t, pa, MiB)
 
 	err := pa.Release(r)
 	if err != nil {
@@ -147,10 +147,10 @@ func Test_MemAlloc_release_invalid_offset(t *testing.T) {
 	}
 	pa.pools[0].free[0] = r
 
-	testAllocate(t, pa, MegaByte)
+	testAllocate(t, pa, MiB)
 
 	// change the actual offset
-	r.offset = MegaByte
+	r.offset = MiB
 	err := pa.Release(r)
 	if err == nil {
 		t.Fatal("no error returned")
@@ -163,7 +163,7 @@ func Test_MemAlloc_release_invalid_offset(t *testing.T) {
 func Test_MemAlloc_Max_Out(t *testing.T) {
 	ma := NewPoolMemoryAllocator()
 	for i := 0; i < 4096; i++ {
-		_, err := ma.Allocate(MegaByte)
+		_, err := ma.Allocate(MiB)
 		if err != nil {
 			t.Fatalf("unexpected error during memory allocation: %s", err)
 		}
@@ -172,7 +172,7 @@ func Test_MemAlloc_Max_Out(t *testing.T) {
 		t.Fatalf("expected 4096 busy blocks of class 0, got %d instead", len(ma.pools[0].busy))
 	}
 	for i := 0; i < 4096; i++ {
-		offset := uint64(i) * MegaByte
+		offset := uint64(i) * MiB
 		if _, ok := ma.pools[0].busy[offset]; !ok {
 			t.Fatalf("expected to find offset %d", offset)
 		}
@@ -189,17 +189,17 @@ func Test_GetMemoryClass(t *testing.T) {
 	testCases := []config{
 		{
 			name:     "Size_1MB_Class_0",
-			size:     MegaByte,
+			size:     MiB,
 			expected: 0,
 		},
 		{
 			name:     "Size_6MB_Class_2",
-			size:     6 * MegaByte,
+			size:     6 * MiB,
 			expected: 2,
 		},
 		{
 			name:     "Size_2GB_Class_6",
-			size:     2 * GigaByte,
+			size:     2 * GiB,
 			expected: 6,
 		},
 	}

internal/memory/types.go

@@ -5,8 +5,8 @@ import "github.com/pkg/errors"
 type classType uint32
 
 const (
-	MegaByte = uint64(1024 * 1024)
-	GigaByte = 1024 * MegaByte
+	MiB = 1024 * 1024
+	GiB = 1024 * MiB
 )
 
 var (

internal/tools/uvmboot/lcow.go

@@ -9,6 +9,7 @@ import (
 	"strings"
 
 	"github.com/Microsoft/hcsshim/internal/cmd"
+	"github.com/Microsoft/hcsshim/internal/memory"
 	"github.com/Microsoft/hcsshim/internal/uvm"
 	"github.com/containerd/console"
 	"github.com/sirupsen/logrus"
@@ -111,7 +112,7 @@ var lcowCommand = cli.Command{
 				options.VPMemDeviceCount = uint32(c.Uint(vpMemMaxCountArgName))
 			}
 			if c.IsSet(vpMemMaxSizeArgName) {
-				options.VPMemSizeBytes = c.Uint64(vpMemMaxSizeArgName) * 1024 * 1024 // convert from MB to bytes
+				options.VPMemSizeBytes = c.Uint64(vpMemMaxSizeArgName) * memory.MiB // convert from MB to bytes
 			}
 			if !useGcs {
 				if c.IsSet(execCommandLineArgName) {

internal/uvm/constants.go

@@ -3,6 +3,7 @@ package uvm
 import (
 	"errors"
 
+	"github.com/Microsoft/hcsshim/internal/memory"
 	"github.com/Microsoft/hcsshim/internal/protocol/guestrequest"
 )
 
@@ -17,7 +18,7 @@ const (
 
 	// DefaultVPMemSizeBytes is the default size of a VPMem device if the create request
 	// doesn't specify.
-	DefaultVPMemSizeBytes = 4 * 1024 * 1024 * 1024 // 4GB
+	DefaultVPMemSizeBytes = 4 * memory.GiB // 4GB
 )
 
 var (

internal/uvm/memory_update.go

@@ -8,18 +8,16 @@ import (
 
 	"github.com/Microsoft/hcsshim/internal/hcs/resourcepaths"
 	hcsschema "github.com/Microsoft/hcsshim/internal/hcs/schema2"
+	"github.com/Microsoft/hcsshim/internal/memory"
 )
 
-const (
-	bytesPerPage = 4096
-	bytesPerMB   = 1024 * 1024
-)
+const bytesPerPage = 4096
 
 // UpdateMemory makes a call to the VM's orchestrator to update the VM's size in MB
 // Internally, HCS will get the number of pages this corresponds to and attempt to assign
 // pages to numa nodes evenly
 func (uvm *UtilityVM) UpdateMemory(ctx context.Context, sizeInBytes uint64) error {
-	requestedSizeInMB := sizeInBytes / bytesPerMB
+	requestedSizeInMB := sizeInBytes / memory.MiB
 	actual := uvm.normalizeMemorySize(ctx, requestedSizeInMB)
 	req := &hcsschema.ModifySettingRequest{
 		ResourcePath: resourcepaths.MemoryResourcePath,