Skip to content

Conversation

@Andrew-Bezzubtsev
Copy link

As for 8.4.1 (maybe earlier), GET /api2/json/nodes/{node}/qemu/{vmid}/config gives string value for "memory" field. This PR secures correct handling of both numeric and literal values.

For reference, see Proxmox VE API reference.

@Andrew-Bezzubtsev Andrew-Bezzubtsev force-pushed the feature/PROXMOX-8.4.1-memory-capacity branch from 1bb5386 to 58f7740 Compare April 18, 2025 11:26
@Tinyblargon
Copy link
Collaborator

@Andrew-Bezzubtsev the structs with json tags aren't directly used to take the PVE api response.
We already parse the memory field with the assumption it could be a string.

if v, isSet := params["memory"]; isSet {
tmp, _ := parse.Uint(v)
tmpIntermediate := QemuMemoryCapacity(tmp)
config.CapacityMiB = &tmpIntermediate
}

@Andrew-Bezzubtsev
Copy link
Author

@Tinyblargon oh, ok! Thanks for your response. These changes actually come from my custom-made patch for proxmox-api-go@e7cde7198cdf, referenced by [email protected]. I have patched the particular version because v2.9.14 is latest non-rc version of terraform-provider-proxmox, while further RC versions were not suitable for me, because they require too much permissions to cluster. Maybe my changes can be useful to be backported to v2.9.14 as a workaround for some users, which have upgraded Proxmox to API-breaking version.

@Tinyblargon
Copy link
Collaborator

@Andrew-Bezzubtsev The reason we have abandoned the 2.9 released is because so many parts make wrong assumptions about the data structures of PVE. So many things would behave wierdly or just panic the moment you went slightly outside the exact use case the developer intended. The main panic of 2.9 has been fixed, but this project has had 60000 lines changed since the 2.9 realease. Most of these changes were to address panics in PCI, Network, Disk, Cloud-init, and many more places.

For the permission issue we have a rudimentary framework that can help with troubleshooting, but it was never used due to some oversight in its design and changing priorities.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants