Refactor out NFS-related code from save_restore.c #74
Conversation
This file contains hidden or 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
Previously, it was perfectly acceptible to specify the savefile path before or after specifying the NFS mount, both in the case that the savefile path _was_ the NFS mount and in the case that it was within that mount. This added a lot of confusing logic. Instead, this is a breaking change that forces that you specify the savefile path before the nfs host if you want to use the savefile path as the mount point; that is, you must do ``` set_savefile_path foo bar save_restoreSet_NFSHost hostname port ``` If the mountpoint is specified separately, then the order of the two commands does not matter. Moreover, set_savefile_path no longer mounts or unmounts the NFS.
simon-ess
changed the title
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.
This pull request is a first attempt at cleaning up some of the code in save_restore.c. It seems much more logical that save_restore is aware of reading and writing files, not specifically how and when to mount or unmount an NFS share.
Note that this PR does contain a breaking change: previously it was possible to have the following two lines in any order in a startup script:
The way this behaved was that if you set the savefile path first, it stored it in memory and then would mount the host:port combination at that specified mountpoint (foo/bar in this case), as no mount point was specified.
If you reversed the order:
then running
set_savefile_pathwould actually mount host:port at the mountpoint foo/bar. This resulted in (IMO) needlessly complicated and confusing code, which is much simplified by enforcing that a mountpoint is specified before attempting to mount; after this change this can be done either by runningset_savefile_pathfirst, or by specifying the mount point as the third argument tosave_restoreSet_NFSHost.