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.

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

Disable the virtiofs mount type #6586

Merged
merged 1 commit into from
Mar 7, 2024
Merged

Conversation

jandubois
Copy link
Member

Testing with virtiofs on Linux has wiped out the HOME directory on the host, and a recent Lima issue claims that they experienced data loss with virtiofs on macOS as well.

Ref #6582 and lima-vm/lima#2221

With this PR the option is still shown in the Preferences dialog, but permanently disabled:

CleanShot 2024-03-07 at 11 12 00@2x

A settings migration will automatically change virtiofs to reverse-sshfs.

And the settings validator will reject the setting:

$ rdctl set --experimental.virtual-machine.mount.type virtiofs
Error: errors in attempt to update settings:
Setting experimental.virtualMachine.mount.type to "virtiofs" is not supported in this version of Rancher Desktop.

@jandubois jandubois requested a review from mook-as March 7, 2024 19:19
mook-as
mook-as previously approved these changes Mar 7, 2024
pkg/rancher-desktop/assets/translations/en-us.yaml Outdated Show resolved Hide resolved
if (disabled) {
tooltip = { content: this.t(`prefs.onlyWithVZ_${ this.arch }`) };
}
let tooltip = { content: this.t('prefs.virtiofsDisabled')};
Copy link
Contributor

Choose a reason for hiding this comment

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

It might have been easier to just duplicate the text for both options instead of changing the code, but this is fine too (and we'd have need to change it to show the correct text on Linux anyway).

pkg/rancher-desktop/config/settingsImpl.ts Show resolved Hide resolved
pkg/rancher-desktop/config/settingsImpl.ts Outdated Show resolved Hide resolved
return false;
}
}
if (desiredValue === VMType.QEMU) {
Copy link
Contributor

Choose a reason for hiding this comment

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

There's no need to take this out (because it'll just never be executed, and we'll need it later).

Copy link
Member Author

Choose a reason for hiding this comment

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

I was thinking about this at first, but I think we will just revert this whole PR when we re-enable virtiofs, and then make any other required changes as a followup.

mook-as
mook-as previously approved these changes Mar 7, 2024
@mook-as mook-as enabled auto-merge March 7, 2024 19:45
Testing with virtiofs on Linux has wiped out the HOME directory on
the host, and a recent Lima issue claims that they experienced data
loss with virtiofs on macOS as well.

Signed-off-by: Jan Dubois <[email protected]>
@mook-as mook-as merged commit adfc162 into rancher-sandbox:main Mar 7, 2024
15 checks passed
@jandubois jandubois deleted the virtiofs branch March 7, 2024 20:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants