Skip to content

Commit 9d459f2

Browse files
fix(rclone/operations): fix CheckPermissions for NFS
4f1adf6 introduced explicit cleanup of test dirs in CheckPermissions. The problem was that the Purge happened while the file descriptor from the "cat" verification was still held. On a regular filesystem this is not an issue, as Purge would still just remove the dir with its contents and make the file descriptor broken. The situation is different with NFS, as when another process tries to remove file with open descriptor, NFS will perform a "silly rename" and create an ephemeral .nfsXXX file so that the descriptor is not broken. So when rclone runs Purge, it first removes all its files, this results in creation of .nfsXXX files and when it proceeds with dir removal, it fails with "directory not empty". This commit fixes the bug related to holding the file descriptor for too long. It also explicitly checks the permission to remove a file and performs the cleanup without failing the CheckPermissions if just the cleanup failed, as it is not critical.
1 parent ebb4191 commit 9d459f2

1 file changed

Lines changed: 13 additions & 6 deletions

File tree

pkg/rclone/operations/operations.go

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ package operations
44

55
import (
66
"context"
7+
stderr "errors"
78
"io"
89
"os"
910
"path/filepath"
@@ -110,7 +111,7 @@ func CheckPermissions(ctx context.Context, l fs.Fs) error {
110111
}
111112
}
112113

113-
// Cat remote file.
114+
// Create, cat and remove remote file.
114115
{
115116
o, err := l.NewObject(ctx, filepath.Join(testDirName, testFileName))
116117
if err != nil {
@@ -120,15 +121,21 @@ func CheckPermissions(ctx context.Context, l fs.Fs) error {
120121
if err != nil {
121122
return asOperationError("open", l, err)
122123
}
123-
defer r.Close()
124-
if _, err := io.Copy(io.Discard, r); err != nil {
125-
return asOperationError("copy", l, err)
124+
_, readErr := io.Copy(io.Discard, r)
125+
if err := stderr.Join(readErr, r.Close()); err != nil {
126+
return asOperationError("read", l, err)
127+
}
128+
if err := o.Remove(ctx); err != nil {
129+
return asOperationError("remove", l, err)
126130
}
127131
}
128132

129-
// Remove remote dir.
133+
// Cleanup.
130134
if err := operations.Purge(ctx, l, testDirName); err != nil {
131-
return asOperationError("purge", l, err)
135+
// As we already verified all permissions needed by SM to perform
136+
// a successful backup, we can just log an error here to allow
137+
// backup to proceed in case of unexpected and not critical error.
138+
fs.Errorf(l, "failed to remove test directory %q: %v", testDirName, err)
132139
}
133140

134141
return nil

0 commit comments

Comments
 (0)