Add experimental options for vm-type and use-rosetta #4063
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jandubois
force-pushed
the
vz
branch
4 times, most recently
from
b00c6f3 to
4f4d8e8
Compare
jandubois
force-pushed
the
vz
branch
3 times, most recently
from
3b30797 to
f822bf7
Compare
jandubois
requested review from
mook-as and
adamkpickering
and removed request for
mook-as
adamkpickering
previously approved these changes
Mar 8, 2023
There was a problem hiding this comment.
Looks good as far as I can tell. We may want to consider the use of the new code from #4036, as mentioned in my comment.
I tested kubernetes reset and port changes, and things look alright there. However, I can't test the openVZ stuff since I don't have a mac. I've asked on team-rancher-desktop if someone else can test this part of the changes. We should probably wait to merge until that has been done.
| } | ||
|
|
||
| protected checkVMType(mergedSettings: Settings, currentValue: string, desiredValue: string, errors: string[], fqname: string): boolean { | ||
| if (desiredValue === VMType.VZ && parseInt(os.release()) < 22) { |
There was a problem hiding this comment.
If the k8smanager.del() function needs the VM to be stopped, it can do it by itself (and already did/does). Signed-off-by: Jan Dubois <[email protected]>
We may still need to download even when adminAccess is true (if the user declines to enter the admin password when needed), but if we already know that we are not even going to ask, then there is no need to generate a lima.yaml that references networks that are not (yet) defined in _config/networks.yaml. Signed-off-by: Jan Dubois <[email protected]>
Otherwise it will get stripped from limactl.ventura, breaking vz support. Signed-off-by: Jan Dubois <[email protected]>
Also mount-type=virtiofs Signed-off-by: Jan Dubois <[email protected]>
This will allow us to reuse the implementation for other binaries in the future. Signed-off-by: Jan Dubois <[email protected]>
When starting VZ mode VM because it doesn't support qcow2. Need to convert to a temporary file because in-place conversion drop the partition table (so does not really convert at all). Signed-off-by: Jan Dubois <[email protected]>
Signed-off-by: Jan Dubois <[email protected]>
adamkpickering
previously approved these changes
Mar 11, 2023
pkg/rancher-desktop/backend/lima.ts
Outdated
| } | ||
|
|
||
| protected convertToRaw(fileName: string): Promise<void> { | ||
| return (async() => { |
There was a problem hiding this comment.
This doesn't need to be in an IIFE:
protected async convertToRaw(fileName: string): Promise<void> {
const rawFileName = `${ fileName }.raw`;
await this.spawnWithCapture(LimaBackend.qemuImg, 'convert', fileName, rawFileName);
await fs.promises.unlink(fileName);
await fs.promises.rename(rawFileName, fileName);
}There was a problem hiding this comment.
I don't know, but given that the other methods are all wrapped like this, I guess it can't hurt.
Signed-off-by: Jan Dubois <[email protected]>
Nino-K
reviewed
Mar 11, 2023
| type: | ||
| type: string | ||
| enum: ['qemu', 'vz'] | ||
| x-rd-platforms: ['darwin'] |
There was a problem hiding this comment.
nit; for consistency we should just remove the quotes: [darwin]
Signed-off-by: Jan Dubois <[email protected]>
Nino-K
previously approved these changes
Mar 11, 2023
Nino-K
previously approved these changes
Mar 11, 2023
Signed-off-by: Jan Dubois <[email protected]>
Signed-off-by: Jan Dubois <[email protected]>
mook-as
approved these changes
Mar 11, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add
experimental.virtualMachine.typeandexperimental.virtualMachine.useRosettaoptions for use on macOS 13 (Ventura).The persistent volume will be converted from QCOW2 to RAW format the first time the vm type is set to
vz. It is possible to switch back toqemuwhile keeping the disk in RAW format.Shutting down a
vzVM takes an additional 3 minutes due to a crash in the bindings: lima-vm/lima#1381. Contrary to the Lima bug description, the delay is not always limited to the first stopping of the VM.Fixes #3944