Skip to content
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.

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

Discussion: CPU features #1149

Open
Tinyblargon opened this issue Nov 10, 2024 · 2 comments
Open

Discussion: CPU features #1149

Tinyblargon opened this issue Nov 10, 2024 · 2 comments
Labels
community feedback resource/qemu Issue or PR related to Qemu resource type/enhancement An improvement of existing functionality

Comments

@Tinyblargon
Copy link
Collaborator

Tinyblargon commented Nov 10, 2024

Currently the qemu resource of the provider has a lot of top level keys.
We are no where near all the features we want to support for this resource.
The problem is that some keys are already in use for other things.

For example the cpu key is used for setting the CPU type.
This also means we have multiple CPU related keys.
All these keys aren't prefixed with cpu_ making it unclear what they do.

  • cpu
  • sockets
  • cores
  • vcpus
  • numa

This means that for CPU limit, CPU Affinity, CPU units and Extra CPU Flags we would have to add more top level keys.

Currently I have 2 proposals.

Proposal one

Deprecate all CPU related flags and introduce the following schema:

resource "proxmox_vm_qemu" "one" {
  cpu_type = "host"
  cpu_cores = 2
  cpu_sockets = 1
  cpu_vcores = 2
  cpu_numa = true
  cpu_limit = 64
  cpu_affinity = "1-3,5"
  cpu_units = 1024
  cpu_flags = "+aes,-avx"
}

Proposal two

Deprecate all CPU related flags, repurpose the cpu key and introduce the following schema:

resource "proxmox_vm_qemu" "two" {
  cpu {
    type = "host"
    cores = 2
    sockets = 1
    vcores = 2
    numa = true
    limit = 64
    affinity = "1-3,5"
    units = 1024
    flags {
      md_clear = "on"
      pcid = "off"
      spec_ctrl = "default"
      ssbd = "default"
      ibpb = "on"
      virt_ssbd = "on"
      amd_ssbd = "off"
      amd_no_ssb = "off"
      pbpe1gb = "off"
      hv_tlbflush = "on"
      hv_evmcs = "on"
      aes = "defualt"
    }
  }

This would force all CPU related information to be near one another.
I'm not sure how usable a nested schema like this would be for modules.

@Tinyblargon Tinyblargon pinned this issue Nov 10, 2024
@Tinyblargon Tinyblargon changed the title Discussion: repurpose existing key Discussion: CPU features Nov 10, 2024
@maksimsamt
Copy link
Contributor

my vote for "Proposal two"

1 similar comment
@den-patrakeev
Copy link

my vote for "Proposal two"

@Tinyblargon Tinyblargon added resource/qemu Issue or PR related to Qemu resource type/enhancement An improvement of existing functionality labels Nov 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community feedback resource/qemu Issue or PR related to Qemu resource type/enhancement An improvement of existing functionality
Projects
None yet
Development

No branches or pull requests

3 participants