feat: add with-kvm flag to the create cluster command #9738
Conversation
Allow enabling or disabling kvm via the cli. This is useful for testing in/for environments where kvm is not available. Signed-off-by: Orzelius <[email protected]>
| "/usr/libexec/qemu-kvm", | ||
| } | ||
| if kvmEnabled { | ||
| binaries = append(binaries, |
There was a problem hiding this comment.
I'm confused a bit, this just changes the search path by removing qemu-kvm ? What does it do in terms of features?
There was a problem hiding this comment.
I believe the correct way to handle this parameter would be the function at the bottom of this file:
talos/pkg/provision/providers/qemu/arch.go
Line 243 in 27d771e
This is because we should first exclude the argument demanding the use of KVM rather than finding a binary that does not include the KVM acceleration support. However qemu-kvm command might force KVM enabled on some distros: not really sure about that
There was a problem hiding this comment.
I haven't tested that myself, but I guess kvm is enabled by default, so we can skip forcing it (which would fix the case when KVM is not available), but without KVM emulation would be super slow
There was a problem hiding this comment.
Not enabled by default it seems
There was a problem hiding this comment.
I'm confused a bit, this just changes the search path by removing qemu-kvm
Yes, this line just removes qemu-kvmbinary from the search list if KVM is disabled. Maybe qemu-kvm can be used even without kvm, but I think it'd be confusing and I haven't looked into it.
What does it do in terms of features?
Do you mean the PR in general? It allows to disable KVM usage, which was mandatory before and would error if KVM could not be used. Now a user can optionally set --with-kvm to false and thus run talos via qemu even on a system which doesn't have kvm (e.q. wsl, docker on a non linux host or another vm).
edit: looks like kvm can be used with wsl as of few years ago :o
There was a problem hiding this comment.
This is because we should first exclude the argument demanding the use of KVM rather than finding a binary that does not include the KVM acceleration support.
This is correct. The code to disable KVM was already there for the use case of emulating different architectures. In the pr I just pass the new flag into the existing LaunchConfig.EnableKVMparameter.
which is later passed to func (arch Arch) KVMArgs(kvmEnabled bool) []string
I just thought removing the qemu-kvm from binaries in case of not using KVM also made sense and made that change as well.
There was a problem hiding this comment.
I think qemu-kvm is just yet another alias present on some systems, and removing it from the search path doesn't make any difference, or make it more confusing in the end.
talosctl cluster create doesn't work on non-Linux, as CNI binaries used to set up networking won't work on non-Linux hosts, so the only case is Linux with KVM disabled, which probably exists, but rare to find.
There was a problem hiding this comment.
Would it then make more sense to just log a warning and proceed with KVM off when it's not available?
I think it still makes sense as it's not a big change and would aid in development and testing of the create command. At least I would benefit from it (see pr description)
|
closing in favor to an upcoming pr |
Allow enabling or disabling kvm via the cli when running the create cluster command.
This is useful for testing in/for environments where kvm is not available.
Context: I'm working on getting the qemu provider to work on mac, but also want to test that my changes don't break stuff on linux side of things. I only have access to linux via docker on a mac host, which doesn't support KVM.
this is my first time contributing to talos so please let me know if something's missing :)