[LCOW Builder] fix CPUGroup, VPMEM controller and StorageQoS parsing

internal/builder/vm/lcow/devices.go

@@ -68,33 +68,37 @@ func parseDeviceOptions(
 
 	// Create VPMem controller configuration
 	var vpMemController *hcsschema.VirtualPMemController
-	if vpmemCount > 0 && rootFsFile == vmutils.VhdFile {
-		// If booting from VHD via VPMem, configure the VPMem device for rootfs
+	if vpmemCount > 0 {
+		// Initialize VPMem controller with specified count and size.
 		vpMemController = &hcsschema.VirtualPMemController{
 			MaximumCount:     vpmemCount,
 			MaximumSizeBytes: vpmemSize,
-			Devices:          make(map[string]hcsschema.VirtualPMemDevice),
 		}
 
-		// Determine image format based on file extension.
-		// filepath.Ext returns the extension with the leading dot (e.g. ".vhdx").
-		imageFormat := "Vhd1"
-		if strings.HasSuffix(strings.ToLower(filepath.Ext(rootFsFile)), "vhdx") {
-			imageFormat = "Vhdx"
-		}
+		// If booting from VHD via VPMem, configure the VPMem device for rootfs
+		if rootFsFile == vmutils.VhdFile {
+			vpMemController.Devices = make(map[string]hcsschema.VirtualPMemDevice)
+
+			// Determine image format based on file extension.
+			// filepath.Ext returns the extension with the leading dot (e.g. ".vhdx").
+			imageFormat := "Vhd1"
+			if strings.HasSuffix(strings.ToLower(filepath.Ext(rootFsFile)), "vhdx") {
+				imageFormat = "Vhdx"
+			}
 
-		// Add rootfs VHD as VPMem device 0
-		vpMemController.Devices["0"] = hcsschema.VirtualPMemDevice{
-			HostPath:    rootFsFullPath,
-			ReadOnly:    true,
-			ImageFormat: imageFormat,
-		}
+			// Add rootfs VHD as VPMem device 0
+			vpMemController.Devices["0"] = hcsschema.VirtualPMemDevice{
+				HostPath:    rootFsFullPath,
+				ReadOnly:    true,
+				ImageFormat: imageFormat,
+			}
 
-		log.G(ctx).WithFields(logrus.Fields{
-			"device":      "0",
-			"path":        rootFsFullPath,
-			"imageFormat": imageFormat,
-		}).Debug("configured VPMem device for VHD rootfs boot")
+			log.G(ctx).WithFields(logrus.Fields{
+				"device":      "0",
+				"path":        rootFsFullPath,
+				"imageFormat": imageFormat,
+			}).Debug("configured VPMem device for VHD rootfs boot")
+		}
 	}
 
 	// ===============================Parse SCSI configuration===============================

internal/builder/vm/lcow/specs.go

@@ -353,17 +353,22 @@ func parseSandboxOptions(ctx context.Context, platform string, annotations map[s
 func parseStorageQOSOptions(ctx context.Context, annotations map[string]string) (*hcsschema.StorageQoS, error) {
 	log.G(ctx).Debug("parseStorageQOSOptions: starting storage QOS options parsing")
 
-	storageQOSConfig := &hcsschema.StorageQoS{
-		IopsMaximum:      oci.ParseAnnotationsInt32(ctx, annotations, shimannotations.StorageQoSIopsMaximum, 0),
-		BandwidthMaximum: oci.ParseAnnotationsInt32(ctx, annotations, shimannotations.StorageQoSBandwidthMaximum, 0),
-	}
+	iopsMaximum := oci.ParseAnnotationsInt32(ctx, annotations, shimannotations.StorageQoSIopsMaximum, 0)
+	bandwidthMaximum := oci.ParseAnnotationsInt32(ctx, annotations, shimannotations.StorageQoSBandwidthMaximum, 0)
 
 	log.G(ctx).WithFields(logrus.Fields{
-		"qosBandwidthMax": storageQOSConfig.BandwidthMaximum,
-		"qosIopsMax":      storageQOSConfig.IopsMaximum,
+		"qosBandwidthMax": bandwidthMaximum,
+		"qosIopsMax":      iopsMaximum,
 	}).Debug("parseStorageQOSOptions completed successfully")
 
-	return storageQOSConfig, nil
+	if iopsMaximum > 0 || bandwidthMaximum > 0 {
+		return &hcsschema.StorageQoS{
+			IopsMaximum:      iopsMaximum,
+			BandwidthMaximum: bandwidthMaximum,
+		}, nil
+	}
+
+	return nil, nil
 }
 
 // setAdditionalOptions sets additional options from annotations.

internal/builder/vm/lcow/specs_test.go

@@ -147,6 +147,14 @@ func TestBuildSandboxConfig(t *testing.T) {
 				if doc.VirtualMachine.ComputeTopology.Memory.SizeInMB != 1024 {
 					t.Errorf("expected default memory 1024MB, got %v", doc.VirtualMachine.ComputeTopology.Memory.SizeInMB)
 				}
+				// StorageQoS should be nil when no QoS annotations are set.
+				if doc.VirtualMachine.StorageQoS != nil {
+					t.Errorf("expected StorageQoS to be nil when no QoS annotations set, got %+v", doc.VirtualMachine.StorageQoS)
+				}
+				// CpuGroup should be nil when no CPUGroupID annotation is provided.
+				if doc.VirtualMachine.ComputeTopology.Processor.CpuGroup != nil {
+					t.Errorf("expected CpuGroup to be nil when no CPUGroupID annotation set, got %+v", doc.VirtualMachine.ComputeTopology.Processor.CpuGroup)
+				}
 			},
 		},
 		{
@@ -1024,6 +1032,75 @@ func TestBuildSandboxConfig_EdgeCases(t *testing.T) {
 				}
 			},
 		},
+		{
+			name: "StorageQoS is nil when both values are zero",
+			opts: &runhcsoptions.Options{
+				SandboxPlatform:   "linux/amd64",
+				BootFilesRootPath: validBootFilesPath,
+			},
+			spec: &vm.Spec{
+				Annotations: map[string]string{
+					shimannotations.StorageQoSIopsMaximum:      "0",
+					shimannotations.StorageQoSBandwidthMaximum: "0",
+				},
+			},
+			validate: func(t *testing.T, doc *hcsschema.ComputeSystem, sandboxOpts *SandboxOptions) {
+				t.Helper()
+				if doc.VirtualMachine.StorageQoS != nil {
+					t.Errorf("expected StorageQoS to be nil when both IOPS and bandwidth are zero, got %+v", doc.VirtualMachine.StorageQoS)
+				}
+			},
+		},
+		{
+			name: "StorageQoS set when only IOPS is non-zero",
+			opts: &runhcsoptions.Options{
+				SandboxPlatform:   "linux/amd64",
+				BootFilesRootPath: validBootFilesPath,
+			},
+			spec: &vm.Spec{
+				Annotations: map[string]string{
+					shimannotations.StorageQoSIopsMaximum: "5000",
+				},
+			},
+			validate: func(t *testing.T, doc *hcsschema.ComputeSystem, sandboxOpts *SandboxOptions) {
+				t.Helper()
+				qos := doc.VirtualMachine.StorageQoS
+				if qos == nil {
+					t.Fatal("expected StorageQoS to be set when IOPS is non-zero")
+				}
+				if qos.IopsMaximum != 5000 {
+					t.Errorf("expected IOPS 5000, got %v", qos.IopsMaximum)
+				}
+				if qos.BandwidthMaximum != 0 {
+					t.Errorf("expected bandwidth 0, got %v", qos.BandwidthMaximum)
+				}
+			},
+		},
+		{
+			name: "StorageQoS set when only bandwidth is non-zero",
+			opts: &runhcsoptions.Options{
+				SandboxPlatform:   "linux/amd64",
+				BootFilesRootPath: validBootFilesPath,
+			},
+			spec: &vm.Spec{
+				Annotations: map[string]string{
+					shimannotations.StorageQoSBandwidthMaximum: "1000000",
+				},
+			},
+			validate: func(t *testing.T, doc *hcsschema.ComputeSystem, sandboxOpts *SandboxOptions) {
+				t.Helper()
+				qos := doc.VirtualMachine.StorageQoS
+				if qos == nil {
+					t.Fatal("expected StorageQoS to be set when bandwidth is non-zero")
+				}
+				if qos.BandwidthMaximum != 1000000 {
+					t.Errorf("expected bandwidth 1000000, got %v", qos.BandwidthMaximum)
+				}
+				if qos.IopsMaximum != 0 {
+					t.Errorf("expected IOPS 0, got %v", qos.IopsMaximum)
+				}
+			},
+		},
 		{
 			name: "empty annotations map with defaults",
 			opts: &runhcsoptions.Options{
@@ -1554,6 +1631,41 @@ func TestBuildSandboxConfig_BootOptions(t *testing.T) {
 				if len(vpmem.Devices) == 0 {
 					t.Error("expected VPMem device to be configured for rootfs")
 				}
+				dev, ok := vpmem.Devices["0"]
+				if !ok {
+					t.Fatal("expected VPMem device at index 0")
+				}
+				if !dev.ReadOnly {
+					t.Error("expected VPMem device to be read-only")
+				}
+				if dev.ImageFormat != "Vhd1" {
+					t.Errorf("expected image format Vhd1, got %s", dev.ImageFormat)
+				}
+			},
+		},
+		{
+			name: "VPMem with initrd boot creates controller but no devices",
+			opts: &runhcsoptions.Options{
+				SandboxPlatform:   "linux/amd64",
+				BootFilesRootPath: initrdOnlyPath,
+			},
+			spec: &vm.Spec{
+				Annotations: map[string]string{
+					shimannotations.VPMemCount: "32",
+				},
+			},
+			validate: func(t *testing.T, doc *hcsschema.ComputeSystem, sandboxOpts *SandboxOptions) {
+				t.Helper()
+				vpmem := doc.VirtualMachine.Devices.VirtualPMem
+				if vpmem == nil {
+					t.Fatal("expected VirtualPMem controller to be created even with initrd boot")
+				}
+				if vpmem.MaximumCount != 32 {
+					t.Errorf("expected VPMem count 32, got %v", vpmem.MaximumCount)
+				}
+				if len(vpmem.Devices) != 0 {
+					t.Errorf("expected no VPMem devices with initrd boot, got %d", len(vpmem.Devices))
+				}
 			},
 		},
 		{
@@ -1737,6 +1849,28 @@ func TestBuildSandboxConfig_DeviceOptions(t *testing.T) {
 				}
 			},
 		},
+		{
+			name: "same device with different function index is not duplicate",
+			spec: &vm.Spec{
+				Devices: []specs.WindowsDevice{
+					{
+						ID:     `PCIP\VEN_1234&DEV_5678/1`,
+						IDType: "vpci-instance-id",
+					},
+					{
+						ID:     `PCIP\VEN_1234&DEV_5678/2`,
+						IDType: "vpci-instance-id",
+					},
+				},
+			},
+			validate: func(t *testing.T, doc *hcsschema.ComputeSystem, sandboxOpts *SandboxOptions) {
+				t.Helper()
+				vpci := doc.VirtualMachine.Devices.VirtualPci
+				if len(vpci) != 2 {
+					t.Errorf("expected 2 assigned devices (different function indexes), got %d", len(vpci))
+				}
+			},
+		},
 	}
 
 	runTestCases(t, ctx, defaultOpts, tests)

internal/builder/vm/lcow/topology.go

@@ -48,10 +48,13 @@ func parseCPUOptions(ctx context.Context, opts *runhcsoptions.Options, annotatio
 
 	// CPU group configuration
 	cpuGroupID := oci.ParseAnnotationsString(annotations, shimannotations.CPUGroupID, "")
-	if cpuGroupID != "" && osversion.Build() < osversion.V21H1 {
-		return nil, vmutils.ErrCPUGroupCreateNotSupported
+	if cpuGroupID != "" {
+		// If CPU group ID is provided, validate it and set it in the CPU configuration.
+		if osversion.Build() < osversion.V21H1 {
+			return nil, vmutils.ErrCPUGroupCreateNotSupported
+		}
+		cpu.CpuGroup = &hcsschema.CpuGroup{Id: cpuGroupID}
 	}
-	cpu.CpuGroup = &hcsschema.CpuGroup{Id: cpuGroupID}
 
 	// Resource Partition ID parsing.
 	resourcePartitionID := oci.ParseAnnotationsString(annotations, shimannotations.ResourcePartitionID, "")