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

qemu: use 9p by default #1953

Merged
merged 3 commits into from
Oct 8, 2024
Merged

Conversation

AkihiroSuda
Copy link
Member

@AkihiroSuda AkihiroSuda commented Oct 26, 2023

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 to
potential 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.

  • AlmaLinux
  • CentOS Stream
  • Debian
  • openSUSE
  • Oracle Linux
  • Rocky Linux

Warning

Existing instances of the distros above may need running the following command (usually not needed):

limactl edit --mount-type=reverse-sshfs <NAME>

Close #971


TODOs:

  • Fix issues on Alpine
  • Parse lima-version file

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.

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.

examples/experimental/9p.yaml Outdated Show resolved Hide resolved
examples/experimental/9p.yaml Outdated Show resolved Hide resolved
@AkihiroSuda
Copy link
Member Author

avoiding the automatic change to existing instances,

Yes, I wanted to do this, but couldn't figure out how we could implement this

@jandubois
Copy link
Member

avoiding the automatic change to existing instances,

Yes, I wanted to do this, but couldn't figure out how we could implement this

We can write a lima.version file in the instance directory during limactl create. We look for the file during limactl start. If the file is missing, we assume the version is 0.18.0.

That way we can apply default settings based on the version of Lima used to create the instance.

We never update lima.version; it represents the version used to create the instance, not the version used to run it.

@AkihiroSuda

This comment was marked as resolved.

@AkihiroSuda

This comment was marked as resolved.

@AkihiroSuda AkihiroSuda added this to the v1.0 (tentative) milestone Jul 3, 2024
@AkihiroSuda
Copy link
Member Author

Can we just rename v0.23 to v1.0?

Yes, I think that is preferable. Working on 2 releases in parallel just increases the risk of introducing regressions.

I just thought you wanted to do a 0.23 release first, as it has a separate milestone on GitHub.

Unified the milestones

jandubois
jandubois previously approved these changes Jul 19, 2024
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, looks good, but I think I would prefer to see something like mountType: not-9p added (better name welcome).

examples/experimental/opensuse-tumbleweed.yaml Outdated Show resolved Hide resolved
pkg/limayaml/defaults.go Outdated Show resolved Hide resolved
pkg/limayaml/defaults.go Outdated Show resolved Hide resolved
pkg/limayaml/defaults.go Outdated Show resolved Hide resolved
pkg/limayaml/defaults.go Outdated Show resolved Hide resolved
@AkihiroSuda
Copy link
Member Author

Will rebase after merging:

@AkihiroSuda
Copy link
Member Author

Rebased and addressed the comments.

Sorry for taking too long for this.

@AkihiroSuda AkihiroSuda marked this pull request as ready for review October 4, 2024 06:14
@AkihiroSuda

This comment was marked as resolved.

@AkihiroSuda AkihiroSuda force-pushed the qemu-9p-default branch 3 times, most recently from 28f2d2e to 88cca75 Compare October 4, 2024 08:51
@AkihiroSuda AkihiroSuda requested review from jandubois and a team October 4, 2024 09:35
@jandubois
Copy link
Member

jandubois commented Oct 7, 2024

I can't remember why 9p is unsupported in so many distros? Is it just missing packages that we could install (like we install fuse etc), or something more fundamental?

@AkihiroSuda
Copy link
Member Author

I can't remember why 9p is unsupported in so many distros?

Because virtiofs is more performant?

Is it just missing packages that we could install (like we install fuse etc), or something more fundamental?

No, needs recompiling the kernel with CONFIG_9P_FS=y or at least with m

@AkihiroSuda
Copy link
Member Author

Sorry, accidentally edited #1953 (comment) (now reverted)

@AkihiroSuda
Copy link
Member Author

We should eventually entirely switch to virtiofs, when QEMU supports virtiofs on macOS. (Has anybody tried to working on it?)

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.

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.

.github/workflows/test.yml Show resolved Hide resolved
examples/default.yaml Show resolved Hide resolved
examples/default.yaml Show resolved Hide resolved
hack/test-templates/test-misc.yaml Show resolved Hide resolved
pkg/limayaml/defaults.go Outdated Show resolved Hide resolved
pkg/limayaml/defaults.go Show resolved Hide resolved
pkg/version/versionutil/versionutil.go Outdated Show resolved Hide resolved
website/content/en/docs/faq/_index.md Show resolved Hide resolved
examples/default.yaml Outdated Show resolved Hide resolved
Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
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 <akihiro.suda.cz@hco.ntt.co.jp>
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 <akihiro.suda.cz@hco.ntt.co.jp>
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

@jandubois jandubois merged commit 08b597f into lima-vm:master Oct 8, 2024
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

v1.0 roadmap: change the default mount driver from reverse-sshfs to 9p (for QEMU) and virtiofs (for vz)
3 participants