-
Notifications
You must be signed in to change notification settings - Fork 600
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
qemu: use 9p by default #1953
qemu: use 9p by default #1953
Conversation
6071b40
to
c0d19c6
Compare
ef603e2
to
32316cb
Compare
32316cb
to
7cde06d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PR looks fine, but I have concerns about how this seems to break existing instances.
I've created a debian-12
instance with HEAD of master
. I started and stopped the instance twice, and it worked fine.
I then switched to this PR branch and tried to start the instance. It hangs with Waiting for the essential requirement 1 of 3: "ssh"
.
I stopped the instance and modified the mount type with
$ limactl edit --mount-type reverse-sshfs debian-12
Trying to start the instance again, it still hangs with Waiting for the essential requirement 1 of 5: "ssh"
. The fact that it now list "1 of 5" instead of "1 of 3" shows that the mount type has been set correctly.
I stop once more, and switch back to the master
branch. The instance still does not start again.
I realize that this issue exists independently of this PR, but it seems like changing the default can result in permanent damage to an instance. I haven't investigated what might be causing this.
This was on macOS Monterey 12.7.1 on Intel.
I have some other idea about avoiding the automatic change to existing instances, or at least adding a warning when waiting for requirements, but maybe the failure to switch the debian image at all should be resolved first.
Yes, I wanted to do this, but couldn't figure out how we could implement this |
We can write a That way we can apply default settings based on the version of Lima used to create the instance. We never update |
1deedf3
to
8c9aceb
Compare
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
3434ee7
to
2b8d352
Compare
2b8d352
to
a566f72
Compare
a566f72
to
ddd0bf5
Compare
Unified the milestones |
ddd0bf5
to
81b2bd5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, looks good, but I think I would prefer to see something like mountType: not-9p
added (better name welcome).
Will rebase after merging: |
81b2bd5
to
47565ec
Compare
Rebased and addressed the comments. Sorry for taking too long for this. |
This comment was marked as resolved.
This comment was marked as resolved.
28f2d2e
to
88cca75
Compare
I can't remember why |
Because virtiofs is more performant?
No, needs recompiling the kernel with |
Sorry, accidentally edited #1953 (comment) (now reverted) |
We should eventually entirely switch to virtiofs, when QEMU supports virtiofs on macOS. (Has anybody tried to working on it?) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks mostly fine, but I have some comments about the code.
Also haven't actually tested it yet, but will try to do so later today.
Signed-off-by: Akihiro Suda <[email protected]>
Setting `writable: true` is still discouraged for reverse-sshfs due to potential connectivity issues, but it should be fine for 9p (virtio-9p-pci) and virtiofs. Signed-off-by: Akihiro Suda <[email protected]>
Templates for the following distro are updated to continue using reverse-sshfs, as 9p is not available on them. - AlmaLinux - CentOS Stream - Debian - openSUSE - Oracle Linux - Rocky Linux Close issue 971 Signed-off-by: Akihiro Suda <[email protected]>
88cca75
to
8d217c4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, LGTM
qemu: 9p: graduate from experimental
template://experimental/9p
will be removed right before releasing Lima v1.0.default.yaml: stop discouraging writable mounts (for 9p and virtiofs)
Setting
writable: true
is still discouraged for reverse-sshfs due topotential connectivity issues, but it should be fine for 9p (virtio-9p-pci)
and virtiofs.
qemu: use 9p by default
Templates for the following distro are updated to continue using reverse-sshfs,
as 9p is not available on them.
Warning
Existing instances of the distros above may need running the following command (usually not needed):
Close #971
TODOs:
lima-version
file