From 6b67be9d48065b6351452662dcc5c25b0676354a Mon Sep 17 00:00:00 2001 From: Nick Craig-Wood Date: Mon, 13 Apr 2026 17:33:33 +0100 Subject: [PATCH] mountlib: rc: fix mounts created with mountPoint "*" overwriting each other On Windows, passing "*" as mountPoint to the mount/mount RC command auto-assigns a drive letter (e.g. "Z:"), but the resolved letter was never propagated back to mountlib. This caused liveMounts to be keyed on the literal "*", breaking tracking of multiple mounts and making unmount unreliable. Change MountFn to return the actual mount point as an additional return value. Update MountPoint.Mount() to store the resolved value, and mountRc() to use it as the liveMounts key. The mount/mount RC response now returns the actual mountPoint so callers can discover which drive letter was assigned. --- cmd/cmount/mount.go | 8 ++++---- cmd/cmount/mount_brew.go | 4 ++-- cmd/mount/mount.go | 10 +++++----- cmd/mount2/mount.go | 16 ++++++++-------- cmd/mountlib/mount.go | 12 ++++++++++-- cmd/mountlib/rc.go | 25 +++++++++++++++++++++---- cmd/mountlib/rc_test.go | 7 ++++++- cmd/nfsmount/nfsmount.go | 3 ++- vfs/vfstest_test.go | 4 ++-- 9 files changed, 60 insertions(+), 29 deletions(-) diff --git a/cmd/cmount/mount.go b/cmd/cmount/mount.go index 908ae0b9c..6515b33b4 100644 --- a/cmd/cmount/mount.go +++ b/cmd/cmount/mount.go @@ -124,12 +124,12 @@ func waitFor(fn func() bool) (ok bool) { // // returns an error, and an error channel for the serve process to // report an error when fusermount is called. -func mount(VFS *vfs.VFS, mountPath string, opt *mountlib.Options) (<-chan error, func() error, error) { +func mount(VFS *vfs.VFS, mountPath string, opt *mountlib.Options) (<-chan error, func() error, string, error) { // Get mountpoint using OS specific logic f := VFS.Fs() mountpoint, err := getMountpoint(f, mountPath, opt) if err != nil { - return nil, nil, err + return nil, nil, "", err } fs.Debugf(nil, "Mounting on %q (%q)", mountpoint, opt.VolumeName) @@ -205,7 +205,7 @@ func mount(VFS *vfs.VFS, mountPath string, opt *mountlib.Options) (<-chan error, select { case err := <-errChan: err = fmt.Errorf("mount stopped before calling Init: %w", err) - return nil, nil, err + return nil, nil, "", err case <-fsys.ready: } @@ -220,5 +220,5 @@ func mount(VFS *vfs.VFS, mountPath string, opt *mountlib.Options) (<-chan error, } } - return errChan, unmount, nil + return errChan, unmount, mountpoint, nil } diff --git a/cmd/cmount/mount_brew.go b/cmd/cmount/mount_brew.go index 9a837de7f..97dd1e904 100644 --- a/cmd/cmount/mount_brew.go +++ b/cmd/cmount/mount_brew.go @@ -26,8 +26,8 @@ func init() { // // returns an error, and an error channel for the serve process to // report an error when fusermount is called. -func mount(_ *vfs.VFS, _ string, _ *mountlib.Options) (<-chan error, func() error, error) { - return nil, nil, errors.New("rclone mount is not supported on MacOS when rclone is installed via Homebrew. " + +func mount(_ *vfs.VFS, _ string, _ *mountlib.Options) (<-chan error, func() error, string, error) { + return nil, nil, "", errors.New("rclone mount is not supported on MacOS when rclone is installed via Homebrew. " + "Please install the rclone binaries available at https://rclone.org/downloads/ " + "instead if you want to use the rclone mount command") } diff --git a/cmd/mount/mount.go b/cmd/mount/mount.go index 0a1f0f4b3..c86aebbb3 100644 --- a/cmd/mount/mount.go +++ b/cmd/mount/mount.go @@ -67,13 +67,13 @@ func mountOptions(VFS *vfs.VFS, device string, opt *mountlib.Options) (options [ // // returns an error, and an error channel for the serve process to // report an error when fusermount is called. -func mount(VFS *vfs.VFS, mountpoint string, opt *mountlib.Options) (<-chan error, func() error, error) { +func mount(VFS *vfs.VFS, mountpoint string, opt *mountlib.Options) (<-chan error, func() error, string, error) { f := VFS.Fs() if err := mountlib.CheckOverlap(f, mountpoint); err != nil { - return nil, nil, err + return nil, nil, "", err } if err := mountlib.CheckAllowNonEmpty(mountpoint, opt); err != nil { - return nil, nil, err + return nil, nil, "", err } fs.Debugf(f, "Mounting on %q", mountpoint) @@ -85,7 +85,7 @@ func mount(VFS *vfs.VFS, mountpoint string, opt *mountlib.Options) (<-chan error c, err := fuse.Mount(mountpoint, mountOptions(VFS, opt.DeviceName, opt)...) if err != nil { - return nil, nil, err + return nil, nil, "", err } filesys := NewFS(VFS, opt) @@ -108,5 +108,5 @@ func mount(VFS *vfs.VFS, mountpoint string, opt *mountlib.Options) (<-chan error return fuse.Unmount(mountpoint) } - return errChan, unmount, nil + return errChan, unmount, mountpoint, nil } diff --git a/cmd/mount2/mount.go b/cmd/mount2/mount.go index 012142a29..99317020b 100644 --- a/cmd/mount2/mount.go +++ b/cmd/mount2/mount.go @@ -184,13 +184,13 @@ func mountOptions(fsys *FS, f fs.Fs, opt *mountlib.Options) (mountOpts *fuse.Mou // // returns an error, and an error channel for the serve process to // report an error when fusermount is called. -func mount(VFS *vfs.VFS, mountpoint string, opt *mountlib.Options) (<-chan error, func() error, error) { +func mount(VFS *vfs.VFS, mountpoint string, opt *mountlib.Options) (<-chan error, func() error, string, error) { f := VFS.Fs() if err := mountlib.CheckOverlap(f, mountpoint); err != nil { - return nil, nil, err + return nil, nil, "", err } if err := mountlib.CheckAllowNonEmpty(mountpoint, opt); err != nil { - return nil, nil, err + return nil, nil, "", err } fs.Debugf(f, "Mounting on %q", mountpoint) @@ -223,20 +223,20 @@ func mount(VFS *vfs.VFS, mountpoint string, opt *mountlib.Options) (<-chan error root, err := fsys.Root() if err != nil { - return nil, nil, err + return nil, nil, "", err } rawFS := fusefs.NewNodeFS(root, &opts) server, err := fuse.NewServer(rawFS, mountpoint, &opts.MountOptions) if err != nil { - return nil, nil, err + return nil, nil, "", err } //mountOpts := &fuse.MountOptions{} //server, err := fusefs.Mount(mountpoint, fsys, &opts) // server, err := fusefs.Mount(mountpoint, root, &opts) // if err != nil { - // return nil, nil, err + // return nil, nil, "", err // } umount := func() error { @@ -258,9 +258,9 @@ func mount(VFS *vfs.VFS, mountpoint string, opt *mountlib.Options) (<-chan error fs.Debugf(f, "Waiting for the mount to start...") err = server.WaitMount() if err != nil { - return nil, nil, err + return nil, nil, "", err } fs.Debugf(f, "Mount started") - return errs, umount, nil + return errs, umount, mountpoint, nil } diff --git a/cmd/mountlib/mount.go b/cmd/mountlib/mount.go index f1abd16c0..b201005eb 100644 --- a/cmd/mountlib/mount.go +++ b/cmd/mountlib/mount.go @@ -195,7 +195,11 @@ type ( // UnmountFn is called to unmount the file system UnmountFn func() error // MountFn is called to mount the file system - MountFn func(VFS *vfs.VFS, mountpoint string, opt *Options) (<-chan error, func() error, error) + // + // It returns the errChan, unmount function, the actual mountpoint + // (which may differ from the input, e.g. on Windows when "*" is + // used to auto-assign a drive letter) and an error. + MountFn func(VFS *vfs.VFS, mountpoint string, opt *Options) (<-chan error, func() error, string, error) ) // MountPoint represents a mount with options and runtime state @@ -371,13 +375,17 @@ func (m *MountPoint) Mount() (mountDaemon *os.Process, err error) { m.VFS = vfs.New(context.Background(), m.Fs, &m.VFSOpt) - m.ErrChan, m.UnmountFn, err = m.MountFn(m.VFS, m.MountPoint, &m.MountOpt) + var actualMountpoint string + m.ErrChan, m.UnmountFn, actualMountpoint, err = m.MountFn(m.VFS, m.MountPoint, &m.MountOpt) if err != nil { if len(os.Args) > 0 && strings.HasPrefix(os.Args[0], "/snap/") { return nil, fmt.Errorf("mounting is not supported when running from snap") } return nil, fmt.Errorf("failed to mount FUSE fs: %w", err) } + if actualMountpoint != "" { + m.MountPoint = actualMountpoint + } m.MountedOn = time.Now() return nil, nil } diff --git a/cmd/mountlib/rc.go b/cmd/mountlib/rc.go index 9b96747df..56170a354 100644 --- a/cmd/mountlib/rc.go +++ b/cmd/mountlib/rc.go @@ -62,12 +62,24 @@ This takes the following parameters: - mountOpt: a JSON object with Mount options in. - vfsOpt: a JSON object with VFS options in. +On Windows mountPoint may be set to "*" to assign the next available +drive letter automatically, or a network share UNC path (e.g. +"\\server\share") to mount as a network drive. In these cases the +actual drive letter is chosen at mount time. + +This returns the following values: + +- mountPoint: the actual mount point that was used (this may differ + from the input, e.g. on Windows when "*" is passed the allocated + drive letter is returned) + Example: ` + "```console" + ` rclone rc mount/mount fs=mydrive: mountPoint=/home//mountPoint rclone rc mount/mount fs=mydrive: mountPoint=/home//mountPoint mountType=mount rclone rc mount/mount fs=TestDrive: mountPoint=/mnt/tmp vfsOpt='{"CacheMode": 2}' mountOpt='{"AllowOther": true}' +rclone rc mount/mount fs=mydrive: mountPoint=* mountType=cmount ` + "```" + ` The vfsOpt are as described in options/get and can be seen in the @@ -128,6 +140,9 @@ func mountRc(ctx context.Context, in rc.Params) (out rc.Params, err error) { fs.Logf(nil, "mount FAILED: %v", err) return nil, err } + // mnt.MountPoint may have been updated by MountFn (e.g. on + // Windows when "*" is resolved to an actual drive letter) + actualMountPoint := mnt.MountPoint go func() { if err = mnt.Wait(); err != nil { fs.Logf(nil, "unmount FAILED: %v", err) @@ -135,13 +150,15 @@ func mountRc(ctx context.Context, in rc.Params) (out rc.Params, err error) { } mountMu.Lock() defer mountMu.Unlock() - delete(liveMounts, mountPoint) + delete(liveMounts, actualMountPoint) }() // Add mount to list if mount point was successfully created - liveMounts[mountPoint] = mnt + liveMounts[actualMountPoint] = mnt - fs.Debugf(nil, "Mount for %s created at %s using %s", fdst.String(), mountPoint, mountType) - return nil, nil + fs.Debugf(nil, "Mount for %s created at %s using %s", fdst.String(), actualMountPoint, mountType) + return rc.Params{ + "mountPoint": actualMountPoint, + }, nil } func init() { diff --git a/cmd/mountlib/rc_test.go b/cmd/mountlib/rc_test.go index 0ac4d6e53..daa44ed72 100644 --- a/cmd/mountlib/rc_test.go +++ b/cmd/mountlib/rc_test.go @@ -82,11 +82,16 @@ func TestRc(t *testing.T) { require.True(t, os.IsNotExist(err)) // mount - _, err = mount.Fn(ctx, in) + out, err := mount.Fn(ctx, in) if err != nil { t.Skipf("Mount failed - skipping test: %v", err) } + // check the returned mount point matches what we asked for + returnedMountPoint, err := out.GetString("mountPoint") + require.NoError(t, err) + assert.Equal(t, mountPoint, returnedMountPoint) + // check file.txt is there now fi, err := os.Stat(filePath) require.NoError(t, err) diff --git a/cmd/nfsmount/nfsmount.go b/cmd/nfsmount/nfsmount.go index 2931cdac0..26ed96914 100644 --- a/cmd/nfsmount/nfsmount.go +++ b/cmd/nfsmount/nfsmount.go @@ -35,7 +35,7 @@ func init() { nfs.AddFlags(cmdFlags) } -func mount(VFS *vfs.VFS, mountpoint string, opt *mountlib.Options) (asyncerrors <-chan error, unmount func() error, err error) { +func mount(VFS *vfs.VFS, mountpoint string, opt *mountlib.Options) (asyncerrors <-chan error, unmount func() error, actualMountpoint string, err error) { s, err := nfs.NewServer(context.Background(), VFS, &nfs.Opt) if err != nil { return @@ -112,5 +112,6 @@ func mount(VFS *vfs.VFS, mountpoint string, opt *mountlib.Options) (asyncerrors VFS.Shutdown() } + actualMountpoint = mountpoint return } diff --git a/vfs/vfstest_test.go b/vfs/vfstest_test.go index df351736e..89e0269f4 100644 --- a/vfs/vfstest_test.go +++ b/vfs/vfstest_test.go @@ -19,12 +19,12 @@ func TestFunctional(t *testing.T) { if *fstest.RemoteName != "" { t.Skip("Skip on non local") } - vfstest.RunTests(t, true, vfscommon.CacheModeOff, true, func(VFS *vfs.VFS, mountpoint string, opt *mountlib.Options) (unmountResult <-chan error, unmount func() error, err error) { + vfstest.RunTests(t, true, vfscommon.CacheModeOff, true, func(VFS *vfs.VFS, mountpoint string, opt *mountlib.Options) (unmountResult <-chan error, unmount func() error, actualMountpoint string, err error) { unmountResultChan := make(chan (error), 1) unmount = func() error { unmountResultChan <- nil return nil } - return unmountResultChan, unmount, nil + return unmountResultChan, unmount, mountpoint, nil }) }