fix(vm): handle more than 4 hostpci devices

Signed-off-by: Pavel Boldyrev <[email protected]>

fwprovider/vm/cdrom/resource.go

@@ -22,7 +22,7 @@ type Value = types.Map
 // NewValue returns a new Value with the given CD-ROM settings from the PVE API.
 func NewValue(ctx context.Context, config *vms.GetResponseData, diags *diag.Diagnostics) Value {
 	// find storage devices with media=cdrom
-	cdroms := config.CustomStorageDevices.Filter(func(device *vms.CustomStorageDevice) bool {
+	cdroms := config.StorageDevices.Filter(func(device *vms.CustomStorageDevice) bool {
 		return device.Media != nil && *device.Media == "cdrom"
 	})
 

proxmox/nodes/vms/custom_pci_device.go

@@ -27,50 +27,50 @@ type CustomPCIDevice struct {
 }
 
 // CustomPCIDevices handles QEMU host PCI device mapping parameters.
-type CustomPCIDevices []CustomPCIDevice
+type CustomPCIDevices map[string]*CustomPCIDevice
 
 // EncodeValues converts a CustomPCIDevice struct to a URL value.
-func (r *CustomPCIDevice) EncodeValues(key string, v *url.Values) error {
+func (d *CustomPCIDevice) EncodeValues(key string, v *url.Values) error {
 	var values []string
 
-	if r.DeviceIDs == nil && r.Mapping == nil {
+	if d.DeviceIDs == nil && d.Mapping == nil {
 		return fmt.Errorf("either device ID or resource mapping must be set")
 	}
 
-	if r.DeviceIDs != nil {
-		values = append(values, fmt.Sprintf("host=%s", strings.Join(*r.DeviceIDs, ";")))
+	if d.DeviceIDs != nil {
+		values = append(values, fmt.Sprintf("host=%s", strings.Join(*d.DeviceIDs, ";")))
 	}
 
-	if r.Mapping != nil {
-		values = append(values, fmt.Sprintf("mapping=%s", *r.Mapping))
+	if d.Mapping != nil {
+		values = append(values, fmt.Sprintf("mapping=%s", *d.Mapping))
 	}
 
-	if r.MDev != nil {
-		values = append(values, fmt.Sprintf("mdev=%s", *r.MDev))
+	if d.MDev != nil {
+		values = append(values, fmt.Sprintf("mdev=%s", *d.MDev))
 	}
 
-	if r.PCIExpress != nil {
-		if *r.PCIExpress {
+	if d.PCIExpress != nil {
+		if *d.PCIExpress {
 			values = append(values, "pcie=1")
 		} else {
 			values = append(values, "pcie=0")
 		}
 	}
 
-	if r.ROMBAR != nil {
-		if *r.ROMBAR {
+	if d.ROMBAR != nil {
+		if *d.ROMBAR {
 			values = append(values, "rombar=1")
 		} else {
 			values = append(values, "rombar=0")
 		}
 	}
 
-	if r.ROMFile != nil {
-		values = append(values, fmt.Sprintf("romfile=%s", *r.ROMFile))
+	if d.ROMFile != nil {
+		values = append(values, fmt.Sprintf("romfile=%s", *d.ROMFile))
 	}
 
-	if r.XVGA != nil {
-		if *r.XVGA {
+	if d.XVGA != nil {
+		if *d.XVGA {
 			values = append(values, "x-vga=1")
 		} else {
 			values = append(values, "x-vga=0")
@@ -83,18 +83,18 @@ func (r *CustomPCIDevice) EncodeValues(key string, v *url.Values) error {
 }
 
 // EncodeValues converts a CustomPCIDevices array to multiple URL values.
-func (r CustomPCIDevices) EncodeValues(key string, v *url.Values) error {
-	for i, d := range r {
-		if err := d.EncodeValues(fmt.Sprintf("%s%d", key, i), v); err != nil {
-			return fmt.Errorf("failed to encode PCI device %d: %w", i, err)
+func (r CustomPCIDevices) EncodeValues(_ string, v *url.Values) error {
+	for s, d := range r {
+		if err := d.EncodeValues(s, v); err != nil {
+			return fmt.Errorf("failed to encode PCI device %s: %w", s, err)
 		}
 	}
 
 	return nil
 }
 
 // UnmarshalJSON converts a CustomPCIDevice string to an object.
-func (r *CustomPCIDevice) UnmarshalJSON(b []byte) error {
+func (d *CustomPCIDevice) UnmarshalJSON(b []byte) error {
 	var s string
 
 	if err := json.Unmarshal(b, &s); err != nil {
@@ -107,27 +107,27 @@ func (r *CustomPCIDevice) UnmarshalJSON(b []byte) error {
 		v := strings.Split(strings.TrimSpace(p), "=")
 		if len(v) == 1 {
 			dIDs := strings.Split(v[0], ";")
-			r.DeviceIDs = &dIDs
+			d.DeviceIDs = &dIDs
 		} else if len(v) == 2 {
 			switch v[0] {
 			case "host":
 				dIDs := strings.Split(v[1], ";")
-				r.DeviceIDs = &dIDs
+				d.DeviceIDs = &dIDs
 			case "mapping":
-				r.Mapping = &v[1]
+				d.Mapping = &v[1]
 			case "mdev":
-				r.MDev = &v[1]
+				d.MDev = &v[1]
 			case "pcie":
 				bv := types.CustomBool(v[1] == "1")
-				r.PCIExpress = &bv
+				d.PCIExpress = &bv
 			case "rombar":
 				bv := types.CustomBool(v[1] == "1")
-				r.ROMBAR = &bv
+				d.ROMBAR = &bv
 			case "romfile":
-				r.ROMFile = &v[1]
+				d.ROMFile = &v[1]
 			case "x-vga":
 				bv := types.CustomBool(v[1] == "1")
-				r.XVGA = &bv
+				d.XVGA = &bv
 			}
 		}
 	}

proxmox/nodes/vms/vms_types.go

@@ -11,7 +11,6 @@ import (
 	"errors"
 	"fmt"
 	"reflect"
-	"regexp"
 	"strings"
 
 	"github.com/bpg/terraform-provider-proxmox/proxmox/types"
@@ -290,10 +289,6 @@ type GetResponseData struct {
 	NUMADevices7         *CustomNUMADevice               `json:"numa7,omitempty"`
 	OSType               *string                         `json:"ostype,omitempty"`
 	Overwrite            *types.CustomBool               `json:"force,omitempty"`
-	PCIDevice0           *CustomPCIDevice                `json:"hostpci0,omitempty"`
-	PCIDevice1           *CustomPCIDevice                `json:"hostpci1,omitempty"`
-	PCIDevice2           *CustomPCIDevice                `json:"hostpci2,omitempty"`
-	PCIDevice3           *CustomPCIDevice                `json:"hostpci3,omitempty"`
 	PoolID               *string                         `json:"pool,omitempty"`
 	Revert               *string                         `json:"revert,omitempty"`
 	SCSIHardware         *string                         `json:"scsihw,omitempty"`
@@ -322,7 +317,8 @@ type GetResponseData struct {
 	VMGenerationID       *string                         `json:"vmgenid,omitempty"`
 	VMStateDatastoreID   *string                         `json:"vmstatestorage,omitempty"`
 	WatchdogDevice       *CustomWatchdogDevice           `json:"watchdog,omitempty"`
-	CustomStorageDevices CustomStorageDevices            `json:"-"`
+	StorageDevices       CustomStorageDevices            `json:"-"`
+	PCIDevices           CustomPCIDevices                `json:"-"`
 }
 
 // GetStatusResponseBody contains the body from a VM get status response.
@@ -451,7 +447,7 @@ type UpdateAsyncResponseBody struct {
 // UpdateRequestBody contains the data for an virtual machine update request.
 type UpdateRequestBody = CreateRequestBody
 
-// UnmarshalJSON unmarshals the custom storage devices from the JSON data.
+// UnmarshalJSON unmarshals the data from the JSON response, populating the CustomStorageDevices field.
 func (d *GetResponseData) UnmarshalJSON(b []byte) error {
 	type Alias GetResponseData
 
@@ -470,23 +466,30 @@ func (d *GetResponseData) UnmarshalJSON(b []byte) error {
 		return fmt.Errorf("failed to unmarshal data: %w", err)
 	}
 
-	allDevices := make(CustomStorageDevices)
+	data.StorageDevices = make(CustomStorageDevices)
+	data.PCIDevices = make(CustomPCIDevices)
 
 	for key, value := range byAttr {
 		for _, prefix := range StorageInterfaces {
-			r := regexp.MustCompile(`^` + prefix + `\d+$`)
-			if r.MatchString(key) {
+			if strings.HasPrefix(key, prefix) {
 				var device CustomStorageDevice
 				if err := json.Unmarshal([]byte(`"`+value.(string)+`"`), &device); err != nil {
 					return fmt.Errorf("failed to unmarshal %s: %w", key, err)
 				}
 
-				allDevices[key] = &device
+				data.StorageDevices[key] = &device
 			}
 		}
-	}
 
-	data.CustomStorageDevices = allDevices
+		if strings.HasPrefix(key, "hostpci") {
+			var device CustomPCIDevice
+			if err := json.Unmarshal([]byte(`"`+value.(string)+`"`), &device); err != nil {
+				return fmt.Errorf("failed to unmarshal %s: %w", key, err)
+			}
+
+			data.PCIDevices[key] = &device
+		}
+	}
 
 	*d = GetResponseData(data)
 

proxmox/nodes/vms/vms_types_test.go

@@ -25,7 +25,10 @@ func TestUnmarshalGetResponseData(t *testing.T) {
 		"ide2": "%[1]s",
 		"ide3": "%[1]s",
 		"virtio13": "%[1]s",
-		"scsi22": "%[1]s"
+		"scsi22": "%[1]s",
+		"hostpci0": "0000:81:00.2",
+		"hostpci1": "host=81:00.4,pcie=0,rombar=1,x-vga=0",
+		"hostpci12": "mapping=mappeddevice,pcie=0,rombar=1,x-vga=0"
 	}`, "local-lvm:vm-100-disk-0,aio=io_uring,backup=1,cache=none,discard=ignore,replicate=1,size=8G,ssd=1")
 
 	var data GetResponseData
@@ -34,20 +37,26 @@ func TestUnmarshalGetResponseData(t *testing.T) {
 
 	assert.Equal(t, "test", *data.BackupFile)
 
-	assert.NotNil(t, data.CustomStorageDevices)
-	assert.Len(t, data.CustomStorageDevices, 6)
-	assert.NotNil(t, data.CustomStorageDevices["ide0"])
-	assertDevice(t, data.CustomStorageDevices["ide0"])
-	assert.NotNil(t, data.CustomStorageDevices["ide1"])
-	assertDevice(t, data.CustomStorageDevices["ide1"])
-	assert.NotNil(t, data.CustomStorageDevices["ide2"])
-	assertDevice(t, data.CustomStorageDevices["ide2"])
-	assert.NotNil(t, data.CustomStorageDevices["ide3"])
-	assertDevice(t, data.CustomStorageDevices["ide3"])
-	assert.NotNil(t, data.CustomStorageDevices["virtio13"])
-	assertDevice(t, data.CustomStorageDevices["virtio13"])
-	assert.NotNil(t, data.CustomStorageDevices["scsi22"])
-	assertDevice(t, data.CustomStorageDevices["scsi22"])
+	assert.NotNil(t, data.StorageDevices)
+	assert.Len(t, data.StorageDevices, 6)
+	assert.NotNil(t, data.StorageDevices["ide0"])
+	assertDevice(t, data.StorageDevices["ide0"])
+	assert.NotNil(t, data.StorageDevices["ide1"])
+	assertDevice(t, data.StorageDevices["ide1"])
+	assert.NotNil(t, data.StorageDevices["ide2"])
+	assertDevice(t, data.StorageDevices["ide2"])
+	assert.NotNil(t, data.StorageDevices["ide3"])
+	assertDevice(t, data.StorageDevices["ide3"])
+	assert.NotNil(t, data.StorageDevices["virtio13"])
+	assertDevice(t, data.StorageDevices["virtio13"])
+	assert.NotNil(t, data.StorageDevices["scsi22"])
+	assertDevice(t, data.StorageDevices["scsi22"])
+
+	assert.NotNil(t, data.PCIDevices)
+	assert.Len(t, data.PCIDevices, 3)
+	assert.NotNil(t, data.PCIDevices["hostpci0"])
+	assert.NotNil(t, data.PCIDevices["hostpci1"])
+	assert.NotNil(t, data.PCIDevices["hostpci12"])
 }
 
 func assertDevice(t *testing.T, dev *CustomStorageDevice) {

proxmoxtf/resource/vm/disk/disk.go

@@ -30,7 +30,7 @@ import (
 // GetInfo returns the disk information for a VM.
 // Deprecated: use vms.MapCustomStorageDevices from proxmox/nodes/vms instead.
 func GetInfo(resp *vms.GetResponseData, d *schema.ResourceData) vms.CustomStorageDevices {
-	storageDevices := resp.CustomStorageDevices
+	storageDevices := resp.StorageDevices
 
 	currentDisk := d.Get(MkDisk)
 

proxmoxtf/resource/vm/vm.go

@@ -130,9 +130,10 @@ const (
 	dvStopOnDestroy       = false
 	dvHookScript          = ""
 
-	maxResourceVirtualEnvironmentVMAudioDevices   = 1
-	maxResourceVirtualEnvironmentVMSerialDevices  = 4
-	maxResourceVirtualEnvironmentVMHostPCIDevices = 8
+	maxResourceVirtualEnvironmentVMAudioDevices  = 1
+	maxResourceVirtualEnvironmentVMSerialDevices = 4
+	// see /usr/share/perl5/PVE/QemuServer/PCI.pm.
+	maxResourceVirtualEnvironmentVMHostPCIDevices = 16
 	maxResourceVirtualEnvironmentVMHostUSBDevices = 4
 	// hardcoded /usr/share/perl5/PVE/QemuServer/Memory.pm: "our $MAX_NUMA = 8".
 	maxResourceVirtualEnvironmentVMNUMADevices = 8
@@ -966,6 +967,7 @@ func VM() *schema.Resource {
 					},
 				},
 			},
+			MaxItems: maxResourceVirtualEnvironmentVMHostPCIDevices,
 		},
 		mkHostUSB: {
 			Type:        schema.TypeList,
@@ -1516,7 +1518,7 @@ func vmCreate(ctx context.Context, d *schema.ResourceData, m interface{}) diag.D
 
 // Check for an existing CloudInit IDE drive. If no such drive is found, return the specified `defaultValue`.
 func findExistingCloudInitDrive(vmConfig *vms.GetResponseData, vmID int, defaultValue string) string {
-	devs := vmConfig.CustomStorageDevices.Filter(func(device *vms.CustomStorageDevice) bool {
+	devs := vmConfig.StorageDevices.Filter(func(device *vms.CustomStorageDevice) bool {
 		return device.IsCloudInitDrive(vmID)
 	})
 
@@ -1530,7 +1532,7 @@ func findExistingCloudInitDrive(vmConfig *vms.GetResponseData, vmID int, default
 // Return a pointer to the storage device configuration based on a name. The device name is assumed to be a
 // valid ide, sata, or scsi interface name.
 func getStorageDevice(vmConfig *vms.GetResponseData, deviceName string) *vms.CustomStorageDevice {
-	if dev, ok := vmConfig.CustomStorageDevices[deviceName]; ok {
+	if dev, ok := vmConfig.StorageDevices[deviceName]; ok {
 		return dev
 	}
 
@@ -2949,9 +2951,10 @@ func vmGetHostPCIDeviceObjects(d *schema.ResourceData) vms.CustomPCIDevices {
 	pciDevice := d.Get(mkHostPCI).([]interface{})
 	pciDeviceObjects := make(vms.CustomPCIDevices, len(pciDevice))
 
-	for i, pciDeviceEntry := range pciDevice {
+	for _, pciDeviceEntry := range pciDevice {
 		block := pciDeviceEntry.(map[string]interface{})
 
+		deviceName := block[mkHostPCIDevice].(string)
 		ids, _ := block[mkHostPCIDeviceID].(string)
 		mdev, _ := block[mkHostPCIDeviceMDev].(string)
 		pcie := types.CustomBool(block[mkHostPCIDevicePCIE].(bool))
@@ -2985,7 +2988,7 @@ func vmGetHostPCIDeviceObjects(d *schema.ResourceData) vms.CustomPCIDevices {
 			device.Mapping = &mapping
 		}
 
-		pciDeviceObjects[i] = device
+		pciDeviceObjects[deviceName] = &device
 	}
 
 	return pciDeviceObjects
@@ -3673,8 +3676,7 @@ func vmReadCustom(
 	currentPCIList := d.Get(mkHostPCI).([]interface{})
 	pciMap := map[string]interface{}{}
 
-	pciDevices := getPCIInfo(vmConfig, d)
-	for pi, pp := range pciDevices {
+	for pi, pp := range vmConfig.PCIDevices {
 		if (pp == nil) || (pp.DeviceIDs == nil && pp.Mapping == nil) {
 			continue
 		}
@@ -5524,17 +5526,6 @@ func getNUMAInfo(resp *vms.GetResponseData, _ *schema.ResourceData) map[string]*
 	return numaDevices
 }
 
-func getPCIInfo(resp *vms.GetResponseData, _ *schema.ResourceData) map[string]*vms.CustomPCIDevice {
-	pciDevices := map[string]*vms.CustomPCIDevice{}
-
-	pciDevices["hostpci0"] = resp.PCIDevice0
-	pciDevices["hostpci1"] = resp.PCIDevice1
-	pciDevices["hostpci2"] = resp.PCIDevice2
-	pciDevices["hostpci3"] = resp.PCIDevice3
-
-	return pciDevices
-}
-
 func getUSBInfo(resp *vms.GetResponseData, _ *schema.ResourceData) map[string]*vms.CustomUSBDevice {
 	usbDevices := map[string]*vms.CustomUSBDevice{}