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

Add ability to specify arbitrary NFS mount options per share #1029

Closed
wants to merge 3 commits into from

Conversation

dansimau
Copy link
Contributor

Additional NFS mount options are specified as an array in the Vagrantfile and they are concatenated and passed to the mount command in the guest.

This solution would address #844 and #929.

Additional NFS mount options are specified as an array in the
Vagrantfile and they are concatenated and passed to the mount command in
the guest.

Attempts to address hashicorp#844 and hashicorp#929.
@dansimau
Copy link
Contributor Author

What it looks like in my Vagrantfile:

  # Share an additional folder to the guest VM. The first argument is
  # an identifier, the second is the path on the guest to mount the
  # folder, and the third is the path on the host to the actual folder.
  config.vm.share_folder("v-root", "/vagrant", ".", :nfs => true, :nfs_mount_options => ["acregmin=1", "acregmax=1"])

I chose to make the options an array because some mount options may have values and others not.

@dkingofpa
Copy link

I like the flexibility with this approach better than individual nfs options. Although, it would be cleaner to just have one nfs related symbol in the command. For example: config.vm.share_folder("v-root", "/vagrant", ".", :nfs => ["acregmin=1", "acregmax=1"])

Would this address #822 (readonly) issue as well?

@dansimau
Copy link
Contributor Author

@dkingofpa Yes, you're right - it would be much nicer with just a single :nfs option that accepts mount params. Good idea.

Also, yes, it would deal with #822, as you could simply specify the ro mount parameter (among other options). Eg.:

config.vm.share_folder("v-root", "/vagrant", ".", :nfs => ["ro", "noac"])

@ramlev
Copy link

ramlev commented Aug 21, 2012

I dont really know if it's off topic, but when using a vagrant VM (ubuntu based) on my osx with a nfs_share to the VM. After a certain amount of time somehow file changes made on OSX isn't sent correctly to the apache browser, it's somehow caches between the nfs_share and the browser?

Does it make any sense ?

@ergonlogic
Copy link

I maintain a number of projects that wrap around Vagrant, and I'd love to see this get in. Essentially, I need to be able to set 'no_root_squash' in order that I be able to 'chown' files in NFS shared folders from within the VM.

@dansimau Does your current pull request support the single nfs: option discussed above?

@pearcec
Copy link

pearcec commented Aug 27, 2012

no_root_squash? I am running linux and it does an "all_squash".

@dansimau
Copy link
Contributor Author

@ergonlogic no, sorry, I've haven't had a chance to update the pull request with the @dkingofpa's suggestions. It's not a huge change but my Ruby-fu is not great so it will probably take me longer than usual. I'll try and get to it soon unless someone wants to beat me to it.

Instead of introducing nfs_mount_options in the Vagrantfile, just use
the existing nfs option. A string or array of mount options is accepted
in place of the boolean value.
Conflicts:
	plugins/guests/linux/guest.rb
@dansimau
Copy link
Contributor Author

These latest commits incorporate the changes that @dkingofpa suggested. Example Vagrantfile configurations look like:

Enabling NFS with no additional options:

config.vm.share_folder("v-root", "/vagrant", ".", :nfs => true)

Enabling NFS but mounted read-only and with file attribute caching disabled:

config.vm.share_folder("v-root", "/vagrant", ".", :nfs => ["ro", "noac"])

etc.

Would be happy for this to go in now.

@mitchellh
Copy link
Contributor

This looks great to me. I'm not home now but once I am I'll review this closer and merge. I'm sorry for the late response here. :)

@pviet
Copy link

pviet commented Oct 19, 2012

good changes, how can we know they get pushed?
cheers

@smurfy
Copy link

smurfy commented Jan 8, 2013

I really like this changes too.

@dansimau
Copy link
Contributor Author

dansimau commented Jan 8, 2013

@mitchellh Any update on this?

@soma
Copy link

soma commented Feb 26, 2013

Any plans of merging this?

@interlock
Copy link

This is a bump for merging this.

@cromulus
Copy link

+1

@evanpurkhiser
Copy link

👍 for having this merged in

Also, as a complete hack/work around for specifying additional mount options for the guest mount command you can simply append the options to the nfs_version config.

  config.vm.synced_folder ".", "/vagrant", id: "vagrant-root",
    nfs: true,
    nfs_version: "3,nolock"

@p0deje
Copy link
Contributor

p0deje commented Jun 13, 2013

I think it makes sense to allow to have both mount and exports options. Something like:

config.vm.synced_folder ".", "/vagrant", :nfs => {
  :mount_options   => ['noatime'],
  :exports_options => ['async'],
}

What do you think?

This was referenced Jun 24, 2013
@mfournier
Copy link
Contributor

I agree with @p0deje, only allowing users to tweak NFS mount options fixes half of the problem.

@mitchellh, any comments about this ? Would you accept a patch implementing @p0deje's suggestion ? Or is there any other points holding you back ?

@mitchellh
Copy link
Contributor

So I'm going through a bunch of NFS stuff now and want to get this all fixed up. My primary concern with this change is that it is so specific to the host system. I like @p0deje's suggestions as well. I wonder if the solution is to support this, but allow host-specific overrides:

:linux__export_options => ["foo"]

(Vagrant already secretly supports this in various places)

@mitchellh
Copy link
Contributor

Merged this functionality in via ':mount_options' as @p0deje recommended.

Next, working on NFS export options...

@mitchellh
Copy link
Contributor

Done.

@mitchellh mitchellh closed this Sep 1, 2013
@alekstorm
Copy link

This was a good change, but the decision to break backward compatibility was unfortunate - my team has been forced to upgrade from 1.2.1 to 1.3.1 to take advantage of unrelated bugfixes, but having to also synchronize Vagrantfile changes makes the process that much more difficult. In the future, I would recommend supporting both :extra and :mount_options, and throwing an error if they are used together, to make the upgrade path easier.

@mitchellh
Copy link
Contributor

@alekstorm Just a reminder: http://docs.vagrantup.com/v2/installation/backwards-compatibility.html

But I've typically been a bit more graceful (deprecate and warn for a release before removal). I'll make an effort to do this next time.

@niccolox
Copy link

on Ubuntu 12 host/Ubuntu 12 guest the following worked for me with discourse

nfs_setting = RUBY_PLATFORM =~ /darwin/ || RUBY_PLATFORM =~ /linux/
config.vm.synced_folder ".", "/vagrant", id: "vagrant-root", type: "nfs", nfs_udp: "true", nfs_version: "4", :nfs => nfs_setting

note; I also had to add host IP determined by vagrant and guest IP determined by Vagrant to firewall exceptions

@nomasprime
Copy link

Does anyone know where docs are for this? Googled to death!

@jfbibeau
Copy link

Agreed. Just made PR #5617 to add the documentation for these options.

@nomasprime
Copy link

@jfbibeau legend!

@ghost ghost locked and limited conversation to collaborators Apr 8, 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