Skip to content

feat(vm): add hotplug parameter to vm #2356

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

feat(vm): add hotplug parameter to vm #2356

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/resources/virtual_environment_vm.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -360,6 +360,7 @@ output "ubuntu_vm_public_key" {
is a relative path under `/usr/share/kvm/`.
- `xvga` - (Optional) Marks the PCI(e) device as the primary GPU of the VM.
With this enabled the `vga` configuration argument will be ignored.
- `hotplug` - (Optional) Selectively enable hotplug features
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The description for hotplug is a bit vague. It would be helpful to list the possible values and their meaning. Also, note that the Proxmox API expects net for network hotplug.

Suggested change
- `hotplug` - (Optional) Selectively enable hotplug features
- `hotplug` - (Optional) A comma-separated list of features to enable for hot-plugging. Supported values are `net`, `disk`, `cpu`, `memory`, and `usb`. Set to `0` to disable all, or `1` to enable all. Defaults to `net,disk,usb`.

- `usb` - (Optional) A host USB device mapping (multiple blocks supported).
- `host` - (Optional) The Host USB device or port or the value `spice`. Use either this or `mapping`.
- `mapping` - (Optional) The cluster-wide resource mapping name of the device, for example "usbdevice". Use either this or `host`.
Expand Down
7 changes: 7 additions & 0 deletions proxmoxtf/resource/vm/vm.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,7 @@ const (
mkHostPCIDeviceROMBAR = "rombar"
mkHostPCIDeviceROMFile = "rom_file"
mkHostPCIDeviceXVGA = "xvga"
mkHotplug = "hotplug"
mkInitialization = "initialization"
mkInitializationDatastoreID = "datastore_id"
mkInitializationInterface = "interface"
Expand Down Expand Up @@ -1073,6 +1074,12 @@ func VM() *schema.Resource {
},
},
},
mkHotplug: {
Type: schema.TypeString,
Description: "Selectively enable hotplug features",
Optional: true,
Default: "network,disk,usb",
},
Comment on lines +1077 to +1082
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

This new hotplug parameter is not fully implemented and has a few issues.

  1. Critical: Missing Implementation: The hotplug parameter is defined here but is not used in the resource's CRUD functions (vmCreateCustom, vmCreateClone, vmUpdate, vmReadCustom). As a result, this feature does not work. You need to:

    • Add logic to vmCreateCustom and vmCreateClone to pass the hotplug value when creating/cloning a VM.
    • Add logic to vmUpdate to handle changes to the hotplug parameter.
    • Add logic to vmReadCustom to read the hotplug configuration from Proxmox and update the Terraform state.

  2. Bug: Incorrect Default Value: The Proxmox API expects net for network hotplug, not network. The default value should be changed. I've included this in the suggestion.

  3. Missing Validation: Input for hotplug is not validated. Please add a ValidateDiagFunc to ensure only valid values (cpu, disk, memory, net, usb, 0, 1, or a comma-separated list) are accepted.

  4. Vague Description: The description should be more detailed, explaining the accepted values. I've included this in the suggestion.

Suggested change
mkHotplug: {
Type: schema.TypeString,
Description: "Selectively enable hotplug features",
Optional: true,
Default: "network,disk,usb",
},
mkHotplug: {
Type: schema.TypeString,
Description: "A comma-separated list of features to enable for hot-plugging. Supported values are `net`, `disk`, `cpu`, `memory`, and `usb`. Set to `0` to disable all, or `1` to enable all.",
Optional: true,
Default: "net,disk,usb",
},

Copy link
Author

@elsoa-invitech elsoa-invitech Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right values is here: https://git.proxmox.com/?p=qemu-server.git;a=blob;f=src/PVE/QemuServer.pm;h=a7fbec14a903423b604a6dcf438c1ba84968c9c5;hb=HEAD#l1146

Copy link
Owner

@bpg bpg Nov 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @elsoa-invitech 👋🏼 thanks for contributing!

Yeah, 🤖 is right, there are few more things that need to be added for this to work.
I can take a look later on the weekend to give you more feedback.

mkKeyboardLayout: {
Type: schema.TypeString,
Description: "The keyboard layout",
Expand Down