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
2 times, most recently
from
8d8f48d to
aca2e46
Compare
Signed-off-by: Ansuman Sahoo <anshumansahoo500@gmail.com>
unsuman
force-pushed
the
fix/revamp-cputype
branch
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
|
|
||
| # OS: "Linux". | ||
| # 🟢 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.
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)
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.