add cpuType for specify cpu #643
Conversation
jandubois
left a comment
jandubois
left a comment
There was a problem hiding this comment.
I know the PR is still in draft mode, but I wanted to get in some early feedback already.
junnplus
force-pushed
the
cpu-type
branch
5 times, most recently
from
f1b02ac to
2dff508
Compare
| if isNativeArch(*y.Arch) { | ||
| args = appendArgsIfNoConflict(args, "-machine", "q35,accel="+accel) | ||
| } else { | ||
| if strings.HasPrefix(cpu, "qemu64") { |
There was a problem hiding this comment.
Why not just compare cpu != "host"
There was a problem hiding this comment.
I'm not sure if these parameters are added for all cpu types, so i keep the status quo.
If someone can be sure to do this, I'll fix it.
There was a problem hiding this comment.
Yeah, seems prudent. I don't know either if these work for all cpu types (and if the reasoning to use them applies).
Signed-off-by: ye.sijun <[email protected]>
| if isNativeArch(*y.Arch) { | ||
| args = appendArgsIfNoConflict(args, "-machine", "q35,accel="+accel) | ||
| } else { | ||
| if strings.HasPrefix(cpu, "qemu64") { |
There was a problem hiding this comment.
Yeah, seems prudent. I don't know either if these work for all cpu types (and if the reasoning to use them applies).
Co-authored-by: Jan Dubois <[email protected]> Signed-off-by: ye.sijun <[email protected]>
Signed-off-by: ye.sijun <[email protected]>
jandubois
left a comment
jandubois
left a comment
There was a problem hiding this comment.
Thanks, LGTM. Leaving for @AkihiroSuda to make final decision and merge.
| type LimaYAML struct { | ||
| Arch *Arch `yaml:"arch,omitempty" json:"arch,omitempty"` | ||
| Images []File `yaml:"images" json:"images"` // REQUIRED | ||
| CPUType *string `yaml:"cpuType,omitempty" json:"cpuType,omitempty"` |
There was a problem hiding this comment.
I already merged this PR, but on second thought we could define this as []struct{Type string, Arch Arch} so that a single YAML can be used on multiple archs?
There was a problem hiding this comment.
Changed to map[Arch]string in #656 (thanks @jandubois )
3 participants
closes: #592
<EDIT by @AkihiroSuda >The yaml type definition was changed in #656
(Both #643 and #656 were merged in the v0.8.3, but not merged in v0.8.2. So, there is no compatibility issue.)
</EDIT>