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.

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

Jump to bottom

VM fails to start on M1 CPU when CPU cores set to more than the host has #1621 #2309

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

VM fails to start on M1 CPU when CPU cores set to more than the host has #1621 #2309

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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions pkg/limayaml/validate.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,10 @@ func Validate(y LimaYAML, warn bool) error {
return errors.New("field `cpus` must be set")
}

if *y.CPUs > runtime.NumCPU() {
return fmt.Errorf("field `cpus` is set to %d, which is greater than the number of CPUs available (%d)", *y.CPUs, runtime.NumCPU())
}
Copy link
Member

Choose a reason for hiding this comment

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

Wondering if the VM driver should automatically decrease the CPUs (with a warning)?

Copy link
Member

Choose a reason for hiding this comment

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

We could (i have no strong opinion either way), but why do it in the driver and not in FillDefault? We set the default memory in there based on actual memory available, so why not implement the cap on the number of CPUs there as well?

Copy link
Member

Choose a reason for hiding this comment

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

That would be fine as well

Copy link
Member

Choose a reason for hiding this comment

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

I just remembered: we may be executing FillDefault several times (we read the configurations of all instances during startup to look for things like shared network daemons, or shared additional disks). So any warnings generated while processing lima.yaml will be displayed multiple times, and we may be displaying warnings for other instances too.

So doing it in the driver might be preferable after all.

Copy link
Author

Choose a reason for hiding this comment

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

Sorry, I have a question.
Am I correct in thinking this should be implemented in vz_driver_darwin.go and qemu_driver.go?
Thanks

Copy link
Author

Choose a reason for hiding this comment

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

i'm able to generate a warning, but I'm not sure how to edit the yaml config to permanently set the number of cpu's.
setting y.cpus doesn't edit the yaml when viewed using limactl edit


if _, err := units.RAMInBytes(*y.Memory); err != nil {
return fmt.Errorf("field `memory` has an invalid value: %w", err)
}
Expand Down