-
-
Notifications
You must be signed in to change notification settings - Fork 4k
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
cmd mountlib: adding remount option for linux systems - fixes #6488 #7802
base: master
Are you sure you want to change the base?
Conversation
Took a crack at addressing issue #6488 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apologies for the delay reviewing this.
This is looking great - thank you.
I put some runtime.GOOS
suggestions inline.
cmd/mountlib/mount.go
Outdated
@@ -161,6 +163,7 @@ func AddFlags(flagSet *pflag.FlagSet) { | |||
flags.BoolVarP(flagSet, &Opt.NetworkMode, "network-mode", "", Opt.NetworkMode, "Mount as remote network drive, instead of fixed disk drive (supported on Windows only)", "Mount") | |||
// Unix only | |||
flags.DurationVarP(flagSet, &Opt.DaemonWait, "daemon-wait", "", Opt.DaemonWait, "Time to wait for ready mount from daemon (maximum time on Linux, constant sleep time on OSX/BSD) (not supported on Windows)", "Mount") | |||
flagSet.BoolVarP(&Opt.Remount, "remount", "", false, "Remount a file system") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest you wrap this in a if runtime.GOOS == "linux" { }
then the flag will only appear for linux systems.
cmd/mountlib/mount.go
Outdated
// Ensure sensible defaults | ||
m.SetVolumeName(m.MountOpt.VolumeName) | ||
m.SetDeviceName(m.MountOpt.DeviceName) | ||
|
||
// Handle the --remount flag by attempting to unmount the mountpoint first | ||
if m.MountOpt.Remount { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd probably put if runtime.GOOS == linux && m.MountOpt.Remount {
here.
The compiler is smart enough to evaluate runtime.GOOS == linux
at compile time then figure out it doesn't need to compile any of the stuff in the if
statement so it saves bloating the binaries where this doesn't work.
ok made the updates, not sure about this Race test that is failing on the go1.20 build, let me know if I can do anything to fix this. |
What is the purpose of this change?
Added a --remount option as requested in issue #6488 . option only works on linux systems. Updated documentation to reflect this optional flag.
Was the change discussed in an issue or in the forum before?
Issue #6488
Checklist