Support configuring viommu

[hillsp]

I am trying to create a proxmox_virtual_environment_vm that uses a virtio viommu. I cannot find a way to do this.

Using qm the IOMMU is selected by adding ,viommu=virtio to the machine type:

https://pve.proxmox.com/wiki/PCI(e)_Passthrough#qm_pci_viommu
https://pve.proxmox.com/pve-docs/pve-admin-guide.html#qm_pci_viommu

If I set my machine type to q35,viommu=virtio, the machine type fails validation:

│ Error: invalid value for machine (must be a valid machine type)
│ 
│   with proxmox_virtual_environment_vm.vm,
│   on main.tf line 20, in resource "proxmox_virtual_environment_vm" "vm":
│   20:   machine = "q35,viommu=virtio"

I cannot find a way to configure viommu using the provider, even after searching the source code.

I would like the ability to select a viommu. Either via the machine attribute or a separate attribute would work for me.

[nakamorichi]

I guess need to at least fix the validation:

terraform-provider-proxmox/proxmoxtf/resource/vm/validators.go

Line 195 in 5a808c8

r := regexp.MustCompile(`^$|^(pc|pc(-i440fx)?-\d+(\.\d+)+(\+pve\d+)?(\.pxe)?|q35|pc-q35-\d+(\.\d+)+(\+pve\d+)?(\.pxe)?|virt(?:-\d+(\.\d+)+)?(\+pve\d+)?)$`)

[bpg]

Hey @nakamorichi 👋🏼

Would you be open to creating a PR to implement that? :)

I don't think adding an extra viommu (or similar) attribute makes sense here, as it would require combining two independent config values into one API field, which is not recommended.

In an ideal world, I would have:

machine { 
  type   = "q35"
  viommu = "virtio"
}

But that would be a breaking change for almost everyone. So perhaps we can consider this for #1231.

[nakamorichi]

I confirmed on my local Mac, just fixing the validation seems to be enough.

@bpg Separating viommu to independent config value seems to be better approach indeed.

Regarding the plugin framework, I'm not sure I have enough spare time currently to take deeper look into it. In case we don't want to add new config value, would it be ok to at least modify the validation so that people can at least get the setting work? No need to even change documentation, but just loosen the validation so that those who need are able to pass the setting without being blocked by the validation?

Btw, unrelated issue; I would like this to also be fixed: https://registry.terraform.io/providers/bpg/proxmox/latest/docs/resources/virtual_environment_file#source_raw-1 (currently source_raw argument doesn't work properly).

[bpg]

... would it be ok to at least modify the validation so that people can at least get the setting work?

Yeah, that's what I'm inclining to. Don't stress much, feel free to open a PR, and we can collaborate to move it over the finish line :)

Btw, unrelated issue; I would like this to also be fixed: https://registry.terraform.io/providers/bpg/proxmox/latest/docs/resources/virtual_environment_file#source_raw-1 (currently source_raw argument doesn't work properly).

Not sure what do you mean by "doesn't work properly"? Would you mind opening a new ticket to provide more details?