mirror of
https://github.com/rclone/rclone.git
synced 2024-11-22 09:11:11 +08:00
vfs: fix rename of open files when using the VFS cache
Before this change, renaming an open file when using the VFS cache was delayed until the file was closed. This meant that the file was not readable after a rename even though it is was in the cache. After this change we rename the local cache file and the in memory cache, delaying only the rename of the file in object storage. See: https://forum.rclone.org/t/xen-orchestra-ebadf-bad-file-descriptor-write/13104
This commit is contained in:
Nick Craig-Wood
parent
241921c786
commit
6e683b4359
54
vfs/file.go
54
vfs/file.go
|
|
@ -137,29 +137,42 @@ func (f *File) applyPendingRename() {
|
|||
func (f *File) rename(ctx context.Context, destDir *Dir, newName string) error {
|
||||
f.mu.RLock()
|
||||
d := f.d
|
||||
o := f.o
|
||||
oldPendingRenameFun := f.pendingRenameFun
|
||||
f.mu.RUnlock()
|
||||
|
||||
if features := d.f.Features(); features.Move == nil && features.Copy == nil {
|
||||
err := errors.Errorf("Fs %q can't rename files (no server side Move or Copy)", d.f)
|
||||
fs.Errorf(f.Path(), "Dir.Rename error: %v", err)
|
||||
return err
|
||||
}
|
||||
|
||||
newPath := path.Join(destDir.path, newName)
|
||||
|
||||
renameCall := func(ctx context.Context) error {
|
||||
newPath := path.Join(destDir.path, newName)
|
||||
f.mu.RLock()
|
||||
o := f.o
|
||||
f.mu.RUnlock()
|
||||
if o == nil {
|
||||
return errors.New("Cannot rename: file object is not available")
|
||||
}
|
||||
|
||||
// chain rename calls if any
|
||||
if oldPendingRenameFun != nil {
|
||||
err := oldPendingRenameFun(ctx)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
|
||||
// do the move of the remote object
|
||||
dstOverwritten, _ := d.f.NewObject(ctx, newPath)
|
||||
newObject, err := operations.Move(ctx, d.f, dstOverwritten, newPath, f.o)
|
||||
newObject, err := operations.Move(ctx, d.f, dstOverwritten, newPath, o)
|
||||
if err != nil {
|
||||
fs.Errorf(f.Path(), "File.Rename error: %v", err)
|
||||
return err
|
||||
}
|
||||
|
||||
// Rename in the cache too if it exists
|
||||
if f.d.vfs.Opt.CacheMode >= CacheModeWrites && f.d.vfs.cache.exists(f.Path()) {
|
||||
if err := f.d.vfs.cache.rename(f.Path(), newPath); err != nil {
|
||||
fs.Infof(f.Path(), "File.Rename failed in Cache: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// newObject can be nil here for example if --dry-run
|
||||
if newObject == nil {
|
||||
err = errors.New("rename failed: nil object returned")
|
||||
|
|
@ -167,25 +180,32 @@ func (f *File) rename(ctx context.Context, destDir *Dir, newName string) error {
|
|||
return err
|
||||
}
|
||||
// Update the node with the new details
|
||||
fs.Debugf(f.o, "Updating file with %v %p", newObject, f)
|
||||
fs.Debugf(o, "Updating file with %v %p", newObject, f)
|
||||
// f.rename(destDir, newObject)
|
||||
f.mu.Lock()
|
||||
f.o = newObject
|
||||
f.d = destDir
|
||||
f.leaf = path.Base(newObject.Remote())
|
||||
f.pendingRenameFun = nil
|
||||
f.mu.Unlock()
|
||||
return nil
|
||||
}
|
||||
|
||||
f.mu.RLock()
|
||||
// Rename in the cache if it exists
|
||||
if f.d.vfs.Opt.CacheMode != CacheModeOff && f.d.vfs.cache.exists(f.Path()) {
|
||||
if err := f.d.vfs.cache.rename(f.Path(), newPath); err != nil {
|
||||
fs.Infof(f.Path(), "File.Rename failed in Cache: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// rename the file object
|
||||
f.mu.Lock()
|
||||
f.d = destDir
|
||||
f.leaf = newName
|
||||
writing := f._writingInProgress()
|
||||
f.mu.RUnlock()
|
||||
f.mu.Unlock()
|
||||
|
||||
if writing {
|
||||
fs.Debugf(f.o, "File is currently open, delaying rename %p", f)
|
||||
fs.Debugf(o, "File is currently open, delaying rename %p", f)
|
||||
f.mu.Lock()
|
||||
f.d = destDir
|
||||
f.leaf = newName
|
||||
f.pendingRenameFun = renameCall
|
||||
f.mu.Unlock()
|
||||
return nil
|
||||
|
|
|
|||
122
vfs/file_test.go
122
vfs/file_test.go
|
|
@ -6,6 +6,7 @@ import (
|
|||
"os"
|
||||
"testing"
|
||||
|
||||
"github.com/rclone/rclone/fs"
|
||||
"github.com/rclone/rclone/fstest"
|
||||
"github.com/rclone/rclone/fstest/mockfs"
|
||||
"github.com/rclone/rclone/fstest/mockobject"
|
||||
|
|
@ -13,8 +14,10 @@ import (
|
|||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
func fileCreate(t *testing.T, r *fstest.Run) (*VFS, *File, fstest.Item) {
|
||||
vfs := New(r.Fremote, nil)
|
||||
func fileCreate(t *testing.T, r *fstest.Run, mode CacheMode) (*VFS, *File, fstest.Item) {
|
||||
opt := DefaultOpt
|
||||
opt.CacheMode = mode
|
||||
vfs := New(r.Fremote, &opt)
|
||||
|
||||
file1 := r.WriteObject(context.Background(), "dir/file1", "file1 contents", t1)
|
||||
fstest.CheckItems(t, r.Fremote, file1)
|
||||
|
|
@ -29,7 +32,7 @@ func fileCreate(t *testing.T, r *fstest.Run) (*VFS, *File, fstest.Item) {
|
|||
func TestFileMethods(t *testing.T) {
|
||||
r := fstest.NewRun(t)
|
||||
defer r.Finalise()
|
||||
vfs, file, _ := fileCreate(t, r)
|
||||
vfs, file, _ := fileCreate(t, r, CacheModeOff)
|
||||
|
||||
// String
|
||||
assert.Equal(t, "dir/file1", file.String())
|
||||
|
|
@ -84,7 +87,7 @@ func TestFileSetModTime(t *testing.T) {
|
|||
return
|
||||
}
|
||||
defer r.Finalise()
|
||||
vfs, file, file1 := fileCreate(t, r)
|
||||
vfs, file, file1 := fileCreate(t, r, CacheModeOff)
|
||||
|
||||
err := file.SetModTime(t2)
|
||||
require.NoError(t, err)
|
||||
|
|
@ -97,12 +100,8 @@ func TestFileSetModTime(t *testing.T) {
|
|||
assert.Equal(t, EROFS, err)
|
||||
}
|
||||
|
||||
func TestFileOpenRead(t *testing.T) {
|
||||
r := fstest.NewRun(t)
|
||||
defer r.Finalise()
|
||||
_, file, _ := fileCreate(t, r)
|
||||
|
||||
fd, err := file.openRead()
|
||||
func fileCheckContents(t *testing.T, file *File) {
|
||||
fd, err := file.Open(os.O_RDONLY)
|
||||
require.NoError(t, err)
|
||||
|
||||
contents, err := ioutil.ReadAll(fd)
|
||||
|
|
@ -112,6 +111,14 @@ func TestFileOpenRead(t *testing.T) {
|
|||
require.NoError(t, fd.Close())
|
||||
}
|
||||
|
||||
func TestFileOpenRead(t *testing.T) {
|
||||
r := fstest.NewRun(t)
|
||||
defer r.Finalise()
|
||||
_, file, _ := fileCreate(t, r, CacheModeOff)
|
||||
|
||||
fileCheckContents(t, file)
|
||||
}
|
||||
|
||||
func TestFileOpenReadUnknownSize(t *testing.T) {
|
||||
var (
|
||||
contents = []byte("file contents")
|
||||
|
|
@ -160,7 +167,7 @@ func TestFileOpenReadUnknownSize(t *testing.T) {
|
|||
func TestFileOpenWrite(t *testing.T) {
|
||||
r := fstest.NewRun(t)
|
||||
defer r.Finalise()
|
||||
vfs, file, _ := fileCreate(t, r)
|
||||
vfs, file, _ := fileCreate(t, r, CacheModeOff)
|
||||
|
||||
fd, err := file.openWrite(os.O_WRONLY | os.O_TRUNC)
|
||||
require.NoError(t, err)
|
||||
|
|
@ -181,7 +188,7 @@ func TestFileOpenWrite(t *testing.T) {
|
|||
func TestFileRemove(t *testing.T) {
|
||||
r := fstest.NewRun(t)
|
||||
defer r.Finalise()
|
||||
vfs, file, _ := fileCreate(t, r)
|
||||
vfs, file, _ := fileCreate(t, r, CacheModeOff)
|
||||
|
||||
err := file.Remove()
|
||||
require.NoError(t, err)
|
||||
|
|
@ -196,7 +203,7 @@ func TestFileRemove(t *testing.T) {
|
|||
func TestFileRemoveAll(t *testing.T) {
|
||||
r := fstest.NewRun(t)
|
||||
defer r.Finalise()
|
||||
vfs, file, _ := fileCreate(t, r)
|
||||
vfs, file, _ := fileCreate(t, r, CacheModeOff)
|
||||
|
||||
err := file.RemoveAll()
|
||||
require.NoError(t, err)
|
||||
|
|
@ -211,7 +218,7 @@ func TestFileRemoveAll(t *testing.T) {
|
|||
func TestFileOpen(t *testing.T) {
|
||||
r := fstest.NewRun(t)
|
||||
defer r.Finalise()
|
||||
_, file, _ := fileCreate(t, r)
|
||||
_, file, _ := fileCreate(t, r, CacheModeOff)
|
||||
|
||||
fd, err := file.Open(os.O_RDONLY)
|
||||
require.NoError(t, err)
|
||||
|
|
@ -233,3 +240,90 @@ func TestFileOpen(t *testing.T) {
|
|||
fd, err = file.Open(3)
|
||||
assert.Equal(t, EPERM, err)
|
||||
}
|
||||
|
||||
func testFileRename(t *testing.T, mode CacheMode) {
|
||||
r := fstest.NewRun(t)
|
||||
defer r.Finalise()
|
||||
vfs, file, item := fileCreate(t, r, mode)
|
||||
|
||||
rootDir, err := vfs.Root()
|
||||
require.NoError(t, err)
|
||||
|
||||
// check file in cache
|
||||
if mode != CacheModeOff {
|
||||
// read contents to get file in cache
|
||||
fileCheckContents(t, file)
|
||||
assert.True(t, vfs.cache.exists(item.Path))
|
||||
}
|
||||
|
||||
dir := file.Dir()
|
||||
|
||||
// start with "dir/file1"
|
||||
fstest.CheckItems(t, r.Fremote, item)
|
||||
|
||||
// rename file to "newLeaf"
|
||||
err = dir.Rename("file1", "newLeaf", rootDir)
|
||||
require.NoError(t, err)
|
||||
|
||||
item.Path = "newLeaf"
|
||||
fstest.CheckItems(t, r.Fremote, item)
|
||||
|
||||
// check file in cache
|
||||
if mode != CacheModeOff {
|
||||
assert.True(t, vfs.cache.exists(item.Path))
|
||||
}
|
||||
|
||||
// check file exists in the vfs layer at its new name
|
||||
_, err = vfs.Stat("newLeaf")
|
||||
require.NoError(t, err)
|
||||
|
||||
// rename it back to "dir/file1"
|
||||
err = rootDir.Rename("newLeaf", "file1", dir)
|
||||
require.NoError(t, err)
|
||||
|
||||
item.Path = "dir/file1"
|
||||
fstest.CheckItems(t, r.Fremote, item)
|
||||
|
||||
// check file in cache
|
||||
if mode != CacheModeOff {
|
||||
assert.True(t, vfs.cache.exists(item.Path))
|
||||
}
|
||||
|
||||
// now try renaming it with the file open
|
||||
// first open it and write to it but dont close it
|
||||
fd, err := file.Open(os.O_WRONLY | os.O_TRUNC)
|
||||
require.NoError(t, err)
|
||||
newContents := []byte("this is some new contents")
|
||||
_, err = fd.Write(newContents)
|
||||
require.NoError(t, err)
|
||||
|
||||
// rename file to "newLeaf"
|
||||
err = dir.Rename("file1", "newLeaf", rootDir)
|
||||
require.NoError(t, err)
|
||||
newItem := fstest.NewItem("newLeaf", string(newContents), item.ModTime)
|
||||
|
||||
// check file has been renamed immediately in the cache
|
||||
if mode != CacheModeOff {
|
||||
assert.True(t, vfs.cache.exists("newLeaf"))
|
||||
}
|
||||
|
||||
// check file exists in the vfs layer at its new name
|
||||
_, err = vfs.Stat("newLeaf")
|
||||
require.NoError(t, err)
|
||||
|
||||
// Close the file
|
||||
require.NoError(t, fd.Close())
|
||||
|
||||
// Check file has now been renamed on the remote
|
||||
item.Path = "newLeaf"
|
||||
fstest.CheckListingWithPrecision(t, r.Fremote, []fstest.Item{newItem}, nil, fs.ModTimeNotSupported)
|
||||
}
|
||||
|
||||
func TestFileRename(t *testing.T) {
|
||||
t.Run("CacheModeOff", func(t *testing.T) {
|
||||
testFileRename(t, CacheModeOff)
|
||||
})
|
||||
t.Run("CacheModeFull", func(t *testing.T) {
|
||||
testFileRename(t, CacheModeFull)
|
||||
})
|
||||
}
|
||||
|
|
|
|||
|
|
@ -698,7 +698,7 @@ func TestRWFileModTimeWithOpenWriters(t *testing.T) {
|
|||
}
|
||||
}
|
||||
|
||||
func TestCacheRename(t *testing.T) {
|
||||
func TestRWCacheRename(t *testing.T) {
|
||||
r := fstest.NewRun(t)
|
||||
defer r.Finalise()
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user