refactor: migrate cpuType to vmOpts.qemu #3500
Conversation
pkg/limayaml/validate.go Outdated
| if !slices.Contains(ArchTypes, arch) { | ||
| return fmt.Errorf("field `vmOpts.qemu.cpuType` uses unsupported arch %q", arch) | ||
| } | ||
| } |
There was a problem hiding this comment.
Probably defaults.go should just replicate the top-level CPUType to VMOpts (with a warning when they conflict), then no need to validate the CPUTypes here twice?
There was a problem hiding this comment.
The validation should happen in pkg/qemu.
pkg/limayaml shouldn't be aware of the archs implemented by the QEMU driver
unsuman force-pushed the fix/revamp-cputype branch 3 times, most recently from aca2e46 to 3ee6999 Compare 
| // TODO: This check should be removed when we completely eliminate `CPUType` from limayaml. | ||
| if len(y.CPUType) > 0 { | ||
| if warn { | ||
| logrus.Warn("The top-level `cpuType` field is deprecated and will be removed in a future release. Please migrate to `vmOpts.qemu.cpuType`.") |
There was a problem hiding this comment.
I don't know why this is not executing even if warn is set true by default
jandubois left a comment
jandubois left a comment There was a problem hiding this comment.
There is also a linter error to fix.
templates/default.yaml Outdated
| # 🟢 Builtin default: "Linux" | ||
| os: null | ||
| | ||
| # DEPRECATED: Use vmOpts.qemu.cpuType instead. |
There was a problem hiding this comment.
I think the duplicate documentation should be removed from the deprecated entry. Just point it to the new location instead.
jandubois left a comment
jandubois left a comment There was a problem hiding this comment.
I would prefer that we hold off on merging this PR until we have agreement on how the final config settings are supposed to look like. Otherwise we might end up with double-migrations, which complicates the code, the docs, and confuses users.
See also #3520 (comment)
unsuman force-pushed the fix/revamp-cputype branch from 3ee6999 to e1df631 Compare unsuman force-pushed the fix/revamp-cputype branch 2 times, most recently from 924ef42 to d712d43 Compare 
Signed-off-by: Ansuman Sahoo <anshumansahoo500@gmail.com>
unsuman force-pushed the fix/revamp-cputype branch from d712d43 to cf8706e Compare 
| Can we merge? @jandubois |
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
3 participants
Description
This PR fixes #3486 by relocating the
cpuTypefield tovmOpts.qemu.cpuTypein both the YAML schema andlimayaml.LimaYAMLstruct in Go. To maintain compatibility, the old top-levelcpuTypeis still accepted and merged into the new path at runtime, with a warning to encourage migration. Validation and default logic have been moved to the qemu package.