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

Change Fedora 16+ NFS support. #1140

Merged

Conversation

smerrill
Copy link

Here's a quick-and-dirty pull request to add NFS support to Fedora 17 and up, as reported in #1125.

This simply parses the redhat-release file, and if the version of Fedora is 17 or greater, it changes @nfs_server_binary to /usr/sbin/service nfs-server instead of /etc/init.d/nfs, which does not exist any more in Fedora 17+ due to the switch to systemd.

This works because the service command in Fedora 17 will rewrite service nfs-server restart to systemctl restart nfs-server.service behind the scenes. This is the simplest way to solve this issue for now without changing too much code since lib/vagrant/hosts/linux.rb calls

system("sudo #{@nfs_server_binary} restart")

at the end of nfs_export, whereas calling systemctl would require copying that method and just changing the last line.

@smerrill
Copy link
Author

I have tested this code change on a Fedora 17 system and it all works. The only noticeable difference is that the service command will output a notice that it's redirecting to systemctl, like so:

[default] VM already created. Booting if it's not already running...
[default] Clearing any previously set forwarded ports...
[default] Forwarding ports...
[default] -- 22 => 22230 (adapter 1)
[default] Exporting NFS shared folders...
[vagrant] Preparing to edit /etc/exports. Administrator privileges will be required...
Redirecting to /bin/systemctl restart  nfs-server.service
[default] Creating shared folders metadata...
[default] Clearing any previously set network interfaces...
[default] Preparing network interfaces based on configuration...
[default] Running any VM customizations...
[default] Booting VM...
[default] Waiting for VM to boot. This can take a few minutes.

if release_file.exist?
release_file.open("r") do |f|
version_number = /Fedora release ([0-9]+)/.match(f.gets)[1].to_i
if version_number >= 17

Choose a reason for hiding this comment

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

This should be if version_number >= 16 since systemd was introduced in Fedora 16.

@skottler
Copy link

This looks good, just added one quick comment.

@smerrill
Copy link
Author

Duly added. Thanks, @skottler. :)

@mitchellh
Copy link
Contributor

This looks good. Thanks a ton. And thanks @skottler for doing open source right.

Merging. I'll handle porting this to the master branch.

mitchellh added a commit that referenced this pull request Sep 19, 2012
@mitchellh mitchellh merged commit f8370ab into hashicorp:1-0-stable Sep 19, 2012
mitchellh added a commit that referenced this pull request Sep 19, 2012
mitchellh added a commit that referenced this pull request Sep 19, 2012
mitchellh added a commit that referenced this pull request Sep 19, 2012
@mitchellh
Copy link
Contributor

One nitpick @smerrill:

Instead of checking if a file exists, I usually try to open it and just rescue the Errno::ENOENT error. This is safer because there is actually a TINY race condition between an exist? check and an open call. For example, the file may exist when exist? is called, then it can be deleted by some outside source, then open is called and then BOOM! exception!.

So instead, I recommend just always opening and catching the exception. Less error cases this way.

I made this fix here: 2d710f5

@hashicorp hashicorp locked and limited conversation to collaborators Apr 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants