Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

limactl: add --cpus, --memory, --mount-type, --vm-type, ... #1468

Merged
merged 1 commit into from
Jul 26, 2023

Conversation

AkihiroSuda
Copy link
Member

@AkihiroSuda AkihiroSuda commented Apr 8, 2023

limactl create, limactl start, limactl edit:

  • --cpus=INT (Similar to colima start --cpu, but the flag name is slightly different)
  • --dns=IP (Similar to colima start --dns)
  • --memory=FLOAT (Similar to colima start --memory)
  • --mount=DIR[:w] (Similar to colima start --mount)
  • --mount-type=(reverse-sshfs|9p|virtiofs) (Similar to colima start --mount-type)
  • --mount-writable=BOOL
  • --network=(lima:shared|lima:bridged|lima:host|lima:user-v2|vzNAT)
  • --rosetta=BOOL

limactl create and limactl start only:

  • --arch=(x86_64|aarch64|riscv64) (Similar to colima start --arch)
  • --containerd=(user|system|user+system|none)
  • --disk=FLOAT (Similar to colima start --disk)
  • --vm-type=(qemu|vz) (Similar to colima start --vm-type)

@AkihiroSuda AkihiroSuda added the area/cli limactl CLI user experience label Apr 8, 2023
@AkihiroSuda AkihiroSuda added this to the v0.16.0 (tentative) milestone Apr 8, 2023
@AkihiroSuda AkihiroSuda added impact/changelog enhancement New feature or request labels Apr 8, 2023
@afbjorklund
Copy link
Member

In the legacy systems, they used MiB instead (probably so you could do things like 0.5GiB, when using integers?)

   --virtualbox-cpu-count "1"										number of CPUs for the machine (-1 to use the number of CPUs available) [$VIRTUALBOX_CPU_COUNT]
   --virtualbox-memory "1024"										Size of memory for host in MB [$VIRTUALBOX_MEMORY_SIZE]
   --virtualbox-disk-size "20000"									Size of disk for host in MB [$VIRTUALBOX_DISK_SIZE]

   --qemu-cpu-count "1"											Number of CPUs
   --qemu-memory "1024"											Size of memory for host in MB
   --qemu-disk-size "20000"										Size of disk for host in MB

For a modern system, it makes much more sense to use GiB. It also looks better, next to the vCPU count. (e.g. 4 / 4)

      --cpus int            number of CPUs
      --memory int          memory in GiB

flags.Int("memory", 0, "memory in GiB") // colima-compatible
_ = cmd.RegisterFlagCompletionFunc("memory", func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
return []string{"1", "2", "4", "8", "16", "32"}, cobra.ShellCompDirectiveNoFileComp
})
Copy link
Member

Choose a reason for hiding this comment

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

Should also add "disk" here, ("disk in GiB")

Copy link
Member

Choose a reason for hiding this comment

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

I have some concern here, because disk size can only ever be increased. The only way to shrink disk size is by destroying and re-creating the instance, losing all your data.

So this is not something that you should casually do. Or it should require a confirmation prompt for the user to acknowledge (disabled by --tty=false).

Copy link
Member Author

Choose a reason for hiding this comment

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

For new instances it can be casually done

Copy link
Member

Choose a reason for hiding this comment

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

For new instances it can be casually done

Yes, my concern is purely about limectl edit default --disk 500GiB, and then being unable to ever shrink it again.

Copy link
Member

@afbjorklund afbjorklund May 1, 2023

Choose a reason for hiding this comment

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

Could disable (or prompt) the feature to edit the disk size, until grow and shrink has been implemented ?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, --disk should not be implemented until Lima can grow the disk automatically.

FWIW, I don't expect us to ever be able to shrink disks; is this even possible?

@afbjorklund
Copy link
Member

It should probably repeat the defaults ?

      --cpus int            number of CPUs (default 4)
      --memory int          memory in GiB (default 4)

Or would that information be misleading, if it is different in the template or in the override ?

@jandubois
Copy link
Member

For a modern system, it makes much more sense to use GiB. It also looks better, next to the vCPU count. (e.g. 4 / 4)

      --cpus int            number of CPUs
      --memory int          memory in GiB

I don't understand why "memory in GiB" should be an integer. Using rational numbers works just fine:

$ l edit alpine --set '.memory = "4.5GiB"'
WARN[0000] `--set` is experimental
INFO[0000] Instance "alpine" configuration edited
[...]
$ ps -ef | grep qemu
  501 26304 26294   0 12:30pm ttys000    0:09.16 /usr/local/bin/qemu-system-x86_64 -m 4608 ...

@jandubois
Copy link
Member

It should probably repeat the defaults ?

      --cpus int            number of CPUs (default 4)
      --memory int          memory in GiB (default 4)

Or would that information be misleading, if it is different in the template or in the override ?

I think this is fine; it shows the built-in defaults, not the current values.

@jandubois
Copy link
Member

jandubois commented Apr 9, 2023

Just for the record, I did have some concerns about this being yet another redundant way to do something that is already possible (with the --set option). But after consideration I think the yq syntax is maybe too obscure for the casual user, so having a shortcut for the most common settings is justified.

@AkihiroSuda
Copy link
Member Author

I don't understand why "memory in GiB" should be an integer.

I’m trying to follow colima flags (when possible)

@jandubois
Copy link
Member

I don't understand why "memory in GiB" should be an integer.

I’m trying to follow colima flags (when possible)

That doesn't make much sense to me; by supporting fractional sizes in Lima we would be supporting a superset of what colima supports. All integer arguments would still be valid. And when colima decides to support fractional sizes as well in the future, they can simply pass them through and everything will just continue to work.

Why should the user not be allowed to write --memory 2.5?

@AkihiroSuda AkihiroSuda force-pushed the edit-flags branch 4 times, most recently from f015da5 to b1e2616 Compare May 22, 2023 15:13
@AkihiroSuda AkihiroSuda marked this pull request as ready for review May 22, 2023 15:17
@AkihiroSuda
Copy link
Member Author

Why should the user not be allowed to write --memory 2.5?

Updated to allow this

@jandubois
Copy link
Member

jandubois commented May 22, 2023

Why should the user not be allowed to write --memory 2.5?

Updated to allow this

Thanks! Now we just have to make it accept --memory 2⅞ as a synonym for --memory 2.875 😄

j/k

@afbjorklund
Copy link
Member

afbjorklund commented Jun 6, 2023

--containerd=(user|system|user+system|none)

Translated these into yq: ".containerd.user = true"

Editing the mounts requires some more knowledge.

.mounts += {"location": "foo", "mountPoint": "bar"}

@AkihiroSuda
Copy link
Member Author

Editing the mounts requires some more knowledge.

.mounts += {"location": "foo", "mountPoint": "bar"}

Yes, complicated configs should be still provided via yq or YAML.

@AkihiroSuda
Copy link
Member Author

Will rebase after merging:

@AkihiroSuda
Copy link
Member Author

Rebased

@AkihiroSuda AkihiroSuda force-pushed the edit-flags branch 3 times, most recently from 2d86dfa to 60a8c95 Compare July 23, 2023 10:16
@AkihiroSuda AkihiroSuda mentioned this pull request Jul 24, 2023
jandubois
jandubois previously approved these changes Jul 26, 2023
Copy link
Member

@jandubois jandubois left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM; only cosmetics feedback. Making completion dynamic maybe not worth the effort.

Personally i wouldn't bother marking --network, --set, and --rosetta as experimental, just to make a log.Warn call when they are used. Do you expect to change them in a backward-incompatible way?

@AkihiroSuda
Copy link
Member Author

Personally i wouldn't bother marking --network, --set, and --rosetta as experimental, just to make a log.Warn call when they are used. Do you expect to change them in a backward-incompatible way?

We can revisit this when we make vz driver and user-v2 network default

jandubois
jandubois previously approved these changes Jul 26, 2023
Copy link
Member

@jandubois jandubois left a comment

Choose a reason for hiding this comment

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

I'm fine with merging this as is, and improving tab completion later.

Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
@AkihiroSuda AkihiroSuda merged commit bd7048f into lima-vm:master Jul 26, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
area/cli limactl CLI user experience enhancement New feature or request impact/changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants