winnfsd / vagrant-winnfsd

Manage and adds support for NFS for Vagrant on Windows.
Other
567 stars 62 forks source link

Restart WinNFSd on `vagrant reload` #81

Closed GuyPaddock closed 8 years ago

GuyPaddock commented 8 years ago

Closes #51.

This also adds a new option config.winnfsd.halt_on_reload that can be set to on or off to toggle this behavior.

lecajer commented 8 years ago

+1 I often had to kill WinNFSd process manually on between vagrant halt & vagrant up

mmodler commented 8 years ago

+1 We also use a modified vagrant up command to kill the winnfsd.exe process before...

andythorne commented 8 years ago

Has this been tested on a multi VM setup? Killing the WinNFSd process on any halt or reload will break other VMs if they are still running...

GuyPaddock commented 8 years ago

I implemented it as an option for exactly this reason. I know it will kill it for the other VMs, but WinNFSd currently has no way to reload the config.

Moreover, it's impractical to use WinNFSd on a multi-VM setup because it can only honor one configuration at a time.

Presumably, if you have more than one VM, you would not be enabling the option this adds, and might not be using WinNFSd.

GM-Alex commented 8 years ago

@GuyPaddock You can use the refresh command to reread the path file. I thought I also implemented a file watcher to detect changes on the path file, but that could be wrong and it was only in my mind, I can't remember it currently and didn't found it right now at the code.

GuyPaddock commented 8 years ago

If there is a reload command, I'd be happy to switch this over to using it. How do you prevent two VMs from overwriting each other's config?

marcharding commented 8 years ago

Since multiple clients/mountpoints in a multi vm setup now work as of 1.2.0, killing winnfsd.exe on vagrant halt is not a good idea anymore.

The mentioned refresh command is at https://github.com/winnfsd/winnfsd/blob/master/src/MountProg.cpp#L97 and is executed on every mount call here https://github.com/winnfsd/winnfsd/blob/master/src/MountProg.cpp#L162, so there is no need to manually refresh.

I think, we do not need this anymore? Further input is welcome.

GuyPaddock commented 8 years ago

@marcharding: If all that is true, it sounds like 1.2.0 fully addresses the previous issue without needing this stopgap. I'll close this out for now.