-
Notifications
You must be signed in to change notification settings - Fork 72
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
NFS and Virtio9p mounts setup refactoring #195
Merged
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
Using --xhyve-experimental-nfs-shares to set up NFS shares implies --xhyve-experimental-nfs-share-enable, which now becomes redundant. Removing --xhyve-experimental-nfs-share-enable makes the command line easier to use.
Previously, only "/Users" was being shared inside the guest at "/Users". With this commit, it is possible to mount any Host folder inside the Guest at "/xhyve-virtio9p". This default can be changed via the `--xhyve-virtio-9p-root` command line argument. This commit changes `--xhyve-virtio-9p` from a bool flag to a string slice representing the absolute path to the Host folder to be mounted inside the guest. There is currently, however, a major caveat, that is that using Virtio9p shares and NFS Shares at the same time will not work. After restarting the machine, the NFS mounts will disappear from the Guest. This issue will be addressed in the next commit.
…used at the same time. In this commit I removed the `d.setupMount()` from the `Start()` function: `Start()` simply starts the docker-machine and isn't supposed to do any other work. For example, you can not add more CPUs, more RAM, or more Disks with the `docker-machine start` command. Just like CPU, RAM, etc, Shared Folders can not be added after the fact with `docker-machine start` and as such, the `d.setupMounts()` shouldn't run. I, however, changed the `Create()` function so that it returns an error, and thus prevents the docker-machine from being created, when the `d.setupMounts()` fails. The other changes address the bug where you couldn't have both Virtio9p *and* NFS shared folders. The bug was that both the code setting up Virtio9p shares and the NFS shares would overwrite the `bootlocal.sh` script.
Instead, simply prepare and execute the `mount` commands directly with `driver.RunSSHCommandFromDriver()` when `Start()` is called. When reading https://github.com/boot2docker/boot2docker/blob/master/doc/FAQ.md#local-customisation-with-persistent-partition it appears that the intent of the `bootlocal.sh` script is to be a script created and managed by the user. Which means that this driver, by overwriting the `bootlocal.sh` script, is going against what has been defined as the way of using this file. This commit fixes this issue.
@huguesalary Thanks pull request :) |
Great, thanks! Let me know if you need some more clarification. |
@huguesalary merged. Thanks! |
Awesome, thanks! |
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.
I have been working those past few days on
xhyve
's ability to mount multiple folders from the Host to the Guest, either via NFS or via Virtio9p.At this point, the NFS work has already been merged into master: #121
Since this last PR (#121), I have been working on allowing multiple mount point using Virtio9p, and have submitted a PR currently waiting for approval: #194. Until that point,
xhyve
's was "hardcodingly" mounting the Host/Users
folder into the Guest via Virtio9p, and the user had no choice in the matter.At the same time, I have been working on modifying
minikube
(https://github.com/kubernetes/minikube) to allowminikube
users to createminikube
machines with multiple mounts (either NFS or Virtio9p).However, while working on
minikube
I ran into issues that ultimately were caused byxhyve
's use of thebootlocal.sh
script for setting up NFS and Virtio9p mount points: when I would create a new minikube machine, my mount points would not show up at creation time. They would only show up after aminikube stop
followed by aminikube start
.After debugging I realized that the
bootlocal.sh
script wasn't being executed when theminikube
machine was created the very first time. After looking around, I found this boot2docker FAQ explaining the role of thebootlocal.sh
script, which made me realize thatxhyve
's use of this script was 1- not proper, 2- superfluous.This Pull Request addresses this issue and will also allow me to submit a PR to the
minikube
project allowingminikube
users to make use of multiple, user-defined, NFS and/or Virtio9p shared folders.