refactor uvm common code into vmutils package

Presently, all the utility methods for uvm building were coupled within the `internal/uvm` package. In order to enhance reusability across shims, we want to move these into a common package. This commit accomplishes that for methods in the CreateUVM path.

Signed-off-by: Harsh Rawat <harshrawat@microsoft.com>

Author: rawahars

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

@@ -107,7 +107,7 @@ func (s *service) createInternal(ctx context.Context, req *task.CreateTaskReques
 
 	spec = oci.UpdateSpecFromOptions(spec, shimOpts)
 	// expand annotations after defaults have been loaded in from options
-	err = oci.ProcessAnnotations(ctx, &spec)
+	err = oci.ProcessAnnotations(ctx, spec.Annotations)
 	// since annotation expansion is used to toggle security features
 	// raise it rather than suppress and move on
 	if err != nil {

internal/oci/annotations.go

@@ -24,7 +24,7 @@ var ErrAnnotationExpansionConflict = errors.New("annotation expansion conflict")
 var ErrGenericAnnotationConflict = errors.New("specified annotations conflict")
 
 // ProcessAnnotations expands annotations into their corresponding annotation groups.
-func ProcessAnnotations(ctx context.Context, s *specs.Spec) error {
+func ProcessAnnotations(ctx context.Context, specAnnotations map[string]string) error {
 	// Named `Process` and not `Expand` since this function may be expanded (pun intended) to
 	// deal with other annotation issues and validation.
 
@@ -36,11 +36,11 @@ func ProcessAnnotations(ctx context.Context, s *specs.Spec) error {
 	var errs []error
 	for key, exps := range annotations.AnnotationExpansionMap() {
 		// check if annotation is present
-		if val, ok := s.Annotations[key]; ok {
+		if val, ok := specAnnotations[key]; ok {
 			// ideally, some normalization would occur here (ie, "True" -> "true")
 			// but strings may be case-sensitive
 			for _, k := range exps {
-				if v, ok := s.Annotations[k]; ok && val != v {
+				if v, ok := specAnnotations[k]; ok && val != v {
 					err := fmt.Errorf("%w: %q = %q and %q = %q", ErrAnnotationExpansionConflict, key, val, k, v)
 					errs = append(errs, err)
 					log.G(ctx).WithFields(logrus.Fields{
@@ -51,26 +51,26 @@ func ProcessAnnotations(ctx context.Context, s *specs.Spec) error {
 					}).WithError(err).Warning("annotation expansion would overwrite conflicting value")
 					continue
 				}
-				s.Annotations[k] = val
+				specAnnotations[k] = val
 			}
 		}
 	}
 
 	// validate host process containers annotations are not conflicting
-	disableHPC := ParseAnnotationsBool(ctx, s.Annotations, annotations.DisableHostProcessContainer, false)
-	enableHPC := ParseAnnotationsBool(ctx, s.Annotations, annotations.HostProcessContainer, false)
+	disableHPC := ParseAnnotationsBool(ctx, specAnnotations, annotations.DisableHostProcessContainer, false)
+	enableHPC := ParseAnnotationsBool(ctx, specAnnotations, annotations.HostProcessContainer, false)
 	if disableHPC && enableHPC {
 		err := fmt.Errorf("%w: host process container annotations %q = %q and %q = %q",
 			ErrGenericAnnotationConflict,
-			annotations.DisableHostProcessContainer, s.Annotations[annotations.DisableHostProcessContainer],
-			annotations.HostProcessContainer, s.Annotations[annotations.HostProcessContainer])
+			annotations.DisableHostProcessContainer, specAnnotations[annotations.DisableHostProcessContainer],
+			annotations.HostProcessContainer, specAnnotations[annotations.HostProcessContainer])
 		errs = append(errs, err)
 
 		log.G(ctx).WithFields(logrus.Fields{
 			logfields.OCIAnnotation:               annotations.DisableHostProcessContainer,
-			logfields.Value:                       s.Annotations[annotations.DisableHostProcessContainer],
+			logfields.Value:                       specAnnotations[annotations.DisableHostProcessContainer],
 			logfields.OCIAnnotation + "-conflict": annotations.HostProcessContainer,
-			logfields.Value + "-conflict":         s.Annotations[annotations.HostProcessContainer],
+			logfields.Value + "-conflict":         specAnnotations[annotations.HostProcessContainer],
 		}).WithError(err).Warning("Host process container and disable host process container cannot both be true")
 	}
 
@@ -204,10 +204,10 @@ func parseAdditionalRegistryValues(ctx context.Context, a map[string]string) []h
 	return slices.Clip(rvs)
 }
 
-// parseHVSocketServiceTable extracts any additional Hyper-V socket service configurations from annotations.
+// ParseHVSocketServiceTable extracts any additional Hyper-V socket service configurations from annotations.
 //
 // Like the [parseAnnotation*] functions, this logs errors but does not return them.
-func parseHVSocketServiceTable(ctx context.Context, a map[string]string) map[string]hcsschema.HvSocketServiceConfig {
+func ParseHVSocketServiceTable(ctx context.Context, a map[string]string) map[string]hcsschema.HvSocketServiceConfig {
 	sc := make(map[string]hcsschema.HvSocketServiceConfig)
 	// TODO(go1.23) use range over functions to implement a functional `filter | map $ a`
 	for k, v := range a {

internal/oci/annotations_test.go

@@ -117,11 +117,10 @@ func TestProccessAnnotations_HostProcessContainer(t *testing.T) {
 	for _, tt := range testAnnotations {
 		t.Run(tt.name, func(t *testing.T) {
 			spec := specs.Spec{
-				Windows:     &specs.Windows{},
 				Annotations: tt.an,
 			}
 
-			err := ProcessAnnotations(ctx, &spec)
+			err := ProcessAnnotations(ctx, spec.Annotations)
 			if err != nil && len(tt.errs) == 0 {
 				t.Fatalf("ProcessAnnotations should have succeeded, instead got %v", err)
 			}
@@ -181,7 +180,7 @@ func TestProccessAnnotations_Expansion(t *testing.T) {
 					annotations.DisableUnsafeOperations: v,
 				}
 
-				err := ProcessAnnotations(ctx, &tt.spec)
+				err := ProcessAnnotations(ctx, tt.spec.Annotations)
 				if err != nil {
 					subtest.Fatalf("could not update spec from options: %v", err)
 				}
@@ -206,7 +205,7 @@ func TestProccessAnnotations_Expansion(t *testing.T) {
 				annotations.DisableUnsafeOperations,
 				annotations.DisableWritableFileShares)
 
-			err := ProcessAnnotations(ctx, &tt.spec)
+			err := ProcessAnnotations(ctx, tt.spec.Annotations)
 			if !errors.Is(err, ErrAnnotationExpansionConflict) {
 				t.Fatalf("UpdateSpecFromOptions should have failed with %q, actual was %v", errExp, err)
 			}
@@ -460,7 +459,7 @@ func TestParseHVSocketServiceTable(t *testing.T) {
 			maps.Copy(annots, tt.give)
 			t.Logf("annotations:\n%v", annots)
 
-			rvs := parseHVSocketServiceTable(ctx, annots)
+			rvs := ParseHVSocketServiceTable(ctx, annots)
 			t.Logf("got %v", rvs)
 			want := tt.want
 			if want == nil {

internal/oci/uvm.go

@@ -333,7 +333,7 @@ func specToUVMCreateOptionsCommon(ctx context.Context, opts *uvm.Options, s *spe
 	opts.NumaMemoryBlocksCounts = ParseAnnotationCommaSeparatedUint64(ctx, s.Annotations, annotations.NumaCountOfMemoryBlocks,
 		opts.NumaMemoryBlocksCounts)
 
-	maps.Copy(opts.AdditionalHyperVConfig, parseHVSocketServiceTable(ctx, s.Annotations))
+	maps.Copy(opts.AdditionalHyperVConfig, ParseHVSocketServiceTable(ctx, s.Annotations))
 
 	// parse error yielding annotations
 	var err error

internal/uvm/create.go

@@ -8,7 +8,6 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
-	"runtime"
 
 	"github.com/Microsoft/go-winio/pkg/guid"
 	"github.com/sirupsen/logrus"
@@ -22,6 +21,7 @@ import (
 	"github.com/Microsoft/hcsshim/internal/logfields"
 	"github.com/Microsoft/hcsshim/internal/oc"
 	"github.com/Microsoft/hcsshim/internal/schemaversion"
+	"github.com/Microsoft/hcsshim/internal/vm/vmutils"
 	"github.com/Microsoft/hcsshim/osversion"
 )
 
@@ -226,7 +226,7 @@ func newDefaultOptions(id, owner string) *Options {
 		MemorySizeInMB:         1024,
 		AllowOvercommit:        true,
 		EnableDeferredCommit:   false,
-		ProcessorCount:         defaultProcessorCount(),
+		ProcessorCount:         vmutils.DefaultProcessorCountForUVM(),
 		FullyPhysicallyBacked:  false,
 		NoWritableFileShares:   false,
 		SCSIControllerCount:    1,
@@ -395,36 +395,6 @@ func (uvm *UtilityVM) ExitError() error {
 	return uvm.hcsSystem.ExitError()
 }
 
-func defaultProcessorCount() int32 {
-	if runtime.NumCPU() == 1 {
-		return 1
-	}
-	return 2
-}
-
-// normalizeProcessorCount sets `uvm.processorCount` to `Min(requested,
-// logical CPU count)`.
-func (uvm *UtilityVM) normalizeProcessorCount(ctx context.Context, requested int32, processorTopology *hcsschema.ProcessorTopology) int32 {
-	// Use host processor information retrieved from HCS instead of runtime.NumCPU,
-	// GetMaximumProcessorCount or other OS level calls for two reasons.
-	// 1. Go uses GetProcessAffinityMask and falls back to GetSystemInfo both of
-	// which will not return LPs in another processor group.
-	// 2. GetMaximumProcessorCount will return all processors on the system
-	// but in configurations where the host partition doesn't see the full LP count
-	// i.e "Minroot" scenarios this won't be sufficient.
-	// (https://docs.microsoft.com/en-us/windows-server/virtualization/hyper-v/manage/manage-hyper-v-minroot-2016)
-	hostCount := int32(processorTopology.LogicalProcessorCount)
-	if requested > hostCount {
-		log.G(ctx).WithFields(logrus.Fields{
-			logfields.UVMID: uvm.id,
-			"requested":     requested,
-			"assigned":      hostCount,
-		}).Warn("Changing user requested CPUCount to current number of processors")
-		return hostCount
-	}
-	return requested
-}
-
 // ProcessorCount returns the number of processors actually assigned to the UVM.
 func (uvm *UtilityVM) ProcessorCount() int32 {
 	return uvm.processorCount
@@ -442,18 +412,6 @@ func (uvm *UtilityVM) ProcessDumpLocation() string {
 	return uvm.processDumpLocation
 }
 
-func (uvm *UtilityVM) normalizeMemorySize(ctx context.Context, requested uint64) uint64 {
-	actual := (requested + 1) &^ 1 // align up to an even number
-	if requested != actual {
-		log.G(ctx).WithFields(logrus.Fields{
-			logfields.UVMID: uvm.id,
-			"requested":     requested,
-			"assigned":      actual,
-		}).Warn("Changing user requested MemorySizeInMB to align to 2MB")
-	}
-	return actual
-}
-
 // DevicesPhysicallyBacked describes if additional devices added to the UVM
 // should be physically backed.
 func (uvm *UtilityVM) DevicesPhysicallyBacked() bool {

internal/uvm/create_lcow.go

@@ -30,6 +30,7 @@ import (
 	"github.com/Microsoft/hcsshim/internal/schemaversion"
 	"github.com/Microsoft/hcsshim/internal/security"
 	"github.com/Microsoft/hcsshim/internal/uvm/scsi"
+	"github.com/Microsoft/hcsshim/internal/vm/vmutils"
 	"github.com/Microsoft/hcsshim/osversion"
 )
 
@@ -130,18 +131,6 @@ type OptionsLCOW struct {
 	WritableOverlayDirs     bool                 // Whether init should create writable overlay mounts for /var and /etc
 }
 
-// defaultLCOWOSBootFilesPath returns the default path used to locate the LCOW
-// OS kernel and root FS files. This default is the subdirectory
-// `LinuxBootFiles` in the directory of the executable that started the current
-// process; or, if it does not exist, `%ProgramFiles%\Linux Containers`.
-func defaultLCOWOSBootFilesPath() string {
-	localDirPath := filepath.Join(filepath.Dir(os.Args[0]), "LinuxBootFiles")
-	if _, err := os.Stat(localDirPath); err == nil {
-		return localDirPath
-	}
-	return filepath.Join(os.Getenv("ProgramFiles"), "Linux Containers")
-}
-
 // NewDefaultOptionsLCOW creates the default options for a bootable version of
 // LCOW.
 //
@@ -179,7 +168,7 @@ func NewDefaultOptionsLCOW(id, owner string) *OptionsLCOW {
 		},
 	}
 
-	opts.UpdateBootFilesPath(context.TODO(), defaultLCOWOSBootFilesPath())
+	opts.UpdateBootFilesPath(context.TODO(), vmutils.DefaultLCOWOSBootFilesPath())
 
 	return opts
 }
@@ -243,7 +232,7 @@ func fetchProcessor(ctx context.Context, opts *OptionsLCOW, uvm *UtilityVM) (*hc
 
 	// To maintain compatibility with Docker we need to automatically downgrade
 	// a user CPU count if the setting is not possible.
-	uvm.processorCount = uvm.normalizeProcessorCount(ctx, opts.ProcessorCount, processorTopology)
+	uvm.processorCount = vmutils.NormalizeProcessorCount(ctx, uvm.id, opts.ProcessorCount, processorTopology)
 
 	processor := &hcsschema.VirtualMachineProcessor{
 		Count:  uint32(uvm.processorCount),
@@ -394,7 +383,7 @@ func makeLCOWVMGSDoc(ctx context.Context, opts *OptionsLCOW, uvm *UtilityVM) (_
 	}
 
 	// Align the requested memory size.
-	memorySizeInMB := uvm.normalizeMemorySize(ctx, opts.MemorySizeInMB)
+	memorySizeInMB := vmutils.NormalizeMemorySize(ctx, uvm.id, opts.MemorySizeInMB)
 
 	doc := &hcsschema.ComputeSystem{
 		Owner:                             uvm.owner,
@@ -596,19 +585,26 @@ func makeLCOWDoc(ctx context.Context, opts *OptionsLCOW, uvm *UtilityVM) (_ *hcs
 		return nil, err
 	}
 
-	numa, numaProcessors, err := prepareVNumaTopology(ctx, opts.Options)
+	numa, numaProcessors, err := vmutils.PrepareVNumaTopology(ctx, &vmutils.NumaConfig{
+		MaxProcessorsPerNumaNode:   opts.MaxProcessorsPerNumaNode,
+		MaxMemorySizePerNumaNode:   opts.MaxMemorySizePerNumaNode,
+		PreferredPhysicalNumaNodes: opts.PreferredPhysicalNumaNodes,
+		NumaMappedPhysicalNodes:    opts.NumaMappedPhysicalNodes,
+		NumaProcessorCounts:        opts.NumaProcessorCounts,
+		NumaMemoryBlocksCounts:     opts.NumaMemoryBlocksCounts,
+	})
 	if err != nil {
 		return nil, err
 	}
 
 	// Align the requested memory size.
-	memorySizeInMB := uvm.normalizeMemorySize(ctx, opts.MemorySizeInMB)
+	memorySizeInMB := vmutils.NormalizeMemorySize(ctx, uvm.id, opts.MemorySizeInMB)
 
 	if numa != nil {
 		if opts.AllowOvercommit {
 			return nil, fmt.Errorf("vNUMA supports only Physical memory backing type")
 		}
-		if err := validateNumaForVM(numa, processor.Count, memorySizeInMB); err != nil {
+		if err := vmutils.ValidateNumaForVM(numa, processor.Count, memorySizeInMB); err != nil {
 			return nil, fmt.Errorf("failed to validate vNUMA settings: %w", err)
 		}
 	}

internal/uvm/create_wcow.go

@@ -27,6 +27,7 @@ import (
 	"github.com/Microsoft/hcsshim/internal/schemaversion"
 	"github.com/Microsoft/hcsshim/internal/security"
 	"github.com/Microsoft/hcsshim/internal/uvm/scsi"
+	"github.com/Microsoft/hcsshim/internal/vm/vmutils"
 	"github.com/Microsoft/hcsshim/internal/wclayer"
 	"github.com/Microsoft/hcsshim/osversion"
 	"github.com/Microsoft/hcsshim/pkg/securitypolicy"
@@ -146,10 +147,10 @@ func prepareCommonConfigDoc(ctx context.Context, uvm *UtilityVM, opts *OptionsWC
 
 	// To maintain compatibility with Docker we need to automatically downgrade
 	// a user CPU count if the setting is not possible.
-	uvm.processorCount = uvm.normalizeProcessorCount(ctx, opts.ProcessorCount, processorTopology)
+	uvm.processorCount = vmutils.NormalizeProcessorCount(ctx, uvm.id, opts.ProcessorCount, processorTopology)
 
 	// Align the requested memory size.
-	memorySizeInMB := uvm.normalizeMemorySize(ctx, opts.MemorySizeInMB)
+	memorySizeInMB := vmutils.NormalizeMemorySize(ctx, uvm.id, opts.MemorySizeInMB)
 
 	var registryChanges hcsschema.RegistryChanges
 	// We're getting asked to setup local dump collection for WCOW. We need to:
@@ -209,7 +210,14 @@ func prepareCommonConfigDoc(ctx context.Context, uvm *UtilityVM, opts *OptionsWC
 		Weight: uint64(opts.ProcessorWeight),
 	}
 
-	numa, numaProcessors, err := prepareVNumaTopology(ctx, opts.Options)
+	numa, numaProcessors, err := vmutils.PrepareVNumaTopology(ctx, &vmutils.NumaConfig{
+		MaxProcessorsPerNumaNode:   opts.MaxProcessorsPerNumaNode,
+		MaxMemorySizePerNumaNode:   opts.MaxMemorySizePerNumaNode,
+		PreferredPhysicalNumaNodes: opts.PreferredPhysicalNumaNodes,
+		NumaMappedPhysicalNodes:    opts.NumaMappedPhysicalNodes,
+		NumaProcessorCounts:        opts.NumaProcessorCounts,
+		NumaMemoryBlocksCounts:     opts.NumaMemoryBlocksCounts,
+	})
 	if err != nil {
 		return nil, err
 	}
@@ -218,7 +226,7 @@ func prepareCommonConfigDoc(ctx context.Context, uvm *UtilityVM, opts *OptionsWC
 		if opts.AllowOvercommit {
 			return nil, fmt.Errorf("vNUMA supports only Physical memory backing type")
 		}
-		if err := validateNumaForVM(numa, processor.Count, memorySizeInMB); err != nil {
+		if err := vmutils.ValidateNumaForVM(numa, processor.Count, memorySizeInMB); err != nil {
 			return nil, fmt.Errorf("failed to validate vNUMA settings: %w", err)
 		}
 	}

internal/uvm/memory_update.go

@@ -9,6 +9,7 @@ import (
 	"github.com/Microsoft/hcsshim/internal/hcs/resourcepaths"
 	hcsschema "github.com/Microsoft/hcsshim/internal/hcs/schema2"
 	"github.com/Microsoft/hcsshim/internal/memory"
+	"github.com/Microsoft/hcsshim/internal/vm/vmutils"
 )
 
 const bytesPerPage = 4096
@@ -18,7 +19,7 @@ const bytesPerPage = 4096
 // pages to numa nodes evenly
 func (uvm *UtilityVM) UpdateMemory(ctx context.Context, sizeInBytes uint64) error {
 	requestedSizeInMB := sizeInBytes / memory.MiB
-	actual := uvm.normalizeMemorySize(ctx, requestedSizeInMB)
+	actual := vmutils.NormalizeMemorySize(ctx, uvm.id, requestedSizeInMB)
 	req := &hcsschema.ModifySettingRequest{
 		ResourcePath: resourcepaths.MemoryResourcePath,
 		Settings:     actual,

internal/uvm/start.go

@@ -30,6 +30,7 @@ import (
 	"github.com/Microsoft/hcsshim/internal/protocol/guestresource"
 	"github.com/Microsoft/hcsshim/internal/timeout"
 	"github.com/Microsoft/hcsshim/internal/uvm/scsi"
+	"github.com/Microsoft/hcsshim/internal/vm/vmutils"
 )
 
 // entropyBytes is the number of bytes of random data to send to a Linux UVM
@@ -379,7 +380,7 @@ func (uvm *UtilityVM) Start(ctx context.Context) (err error) {
 			policy = uvm.createOpts.(*OptionsLCOW).SecurityPolicy
 			enforcer = uvm.createOpts.(*OptionsLCOW).SecurityPolicyEnforcer
 			referenceInfoFilePath = uvm.createOpts.(*OptionsLCOW).UVMReferenceInfoFile
-			referenceInfoFileRoot = defaultLCOWOSBootFilesPath()
+			referenceInfoFileRoot = vmutils.DefaultLCOWOSBootFilesPath()
 		} else if uvm.OS() == "windows" {
 			policy = uvm.createOpts.(*OptionsWCOW).SecurityPolicy
 			enforcer = uvm.createOpts.(*OptionsWCOW).SecurityPolicyEnforcer

internal/vm/vmutils/doc.go

@@ -0,0 +1,12 @@
+//go:build windows
+
+// Package vmutils provides shared utility functions for working with Utility VMs.
+//
+// This package contains stateless utility functions that can be used by both the
+// legacy UVM management code (internal/uvm) and the new controller-based architecture
+// (internal/controller). Functions in this package are designed to be decoupled from
+// specific UVM implementations.
+//
+// This allows different shims (containerd-shim-runhcs-v1, containerd-shim-lcow-v1)
+// to share common logic while maintaining their own orchestration patterns.
+package vmutils