fix sync: copying times, owner and mode not follow link (#4151)

close #4150 --------- Signed-off-by: xixi <[email protected]>
juicedata · Nov 15, 2023 · 562c989 · 562c989
1 parent a9d2ce1
commit 562c989
Show file tree

Hide file tree

Showing 6 changed files with 53 additions and 18 deletions.
diff --git a/cmd/object.go b/cmd/object.go
@@ -298,7 +298,7 @@ func (j *juiceFS) readDirSorted(dirname string, followLink bool) ([]*mEntry, sys
 }
 
 func (j *juiceFS) Chtimes(key string, mtime time.Time) error {
-	f, err := j.jfs.Open(ctx, j.path(key), 0)
+	f, err := j.jfs.Lopen(ctx, j.path(key), 0)
 	if err != 0 {
 		return err
 	}
@@ -318,7 +318,7 @@ func (j *juiceFS) Chmod(key string, mode os.FileMode) error {
 func (j *juiceFS) Chown(key string, owner, group string) error {
 	uid := utils.LookupUser(owner)
 	gid := utils.LookupGroup(group)
-	f, err := j.jfs.Open(ctx, j.path(key), 0)
+	f, err := j.jfs.Lopen(ctx, j.path(key), 0)
 	if err != 0 {
 		return err
 	}

diff --git a/pkg/fs/fs.go b/pkg/fs/fs.go
@@ -346,7 +346,16 @@ func (fs *FileSystem) StatFS(ctx meta.Context) (totalspace uint64, availspace ui
 	return
 }
 
-func (fs *FileSystem) Open(ctx meta.Context, path string, flags uint32) (f *File, err syscall.Errno) {
+// open file without following symlink
+func (fs *FileSystem) Lopen(ctx meta.Context, path string, flags uint32) (f *File, err syscall.Errno) {
+	return fs.open(ctx, path, flags, false)
+}
+
+func (fs *FileSystem) Open(ctx meta.Context, path string, flags uint32) (*File, syscall.Errno) {
+	return fs.open(ctx, path, flags, true)
+}
+
+func (fs *FileSystem) open(ctx meta.Context, path string, flags uint32, followLink bool) (f *File, err syscall.Errno) {
 	_, task := trace.NewTask(context.TODO(), "Open")
 	defer task.End()
 	l := vfs.NewLogContext(ctx)
@@ -356,7 +365,7 @@ func (fs *FileSystem) Open(ctx meta.Context, path string, flags uint32) (f *File
 		defer func() { fs.log(l, "Lookup (%s): %s", path, errstr(err)) }()
 	}
 	var fi *FileStat
-	fi, err = fs.resolve(ctx, path, true)
+	fi, err = fs.resolve(ctx, path, followLink)
 	if err != 0 {
 		return
 	}

diff --git a/pkg/object/file.go b/pkg/object/file.go
@@ -29,7 +29,6 @@ import (
 	"sort"
 	"strconv"
 	"strings"
-	"time"
 
 	"github.com/juicedata/juicefs/pkg/utils"
 )
@@ -322,11 +321,6 @@ func (d *filestore) List(prefix, marker, delimiter string, limit int64, followLi
 	return objs, nil
 }
 
-func (d *filestore) Chtimes(key string, mtime time.Time) error {
-	p := d.path(key)
-	return os.Chtimes(p, mtime, mtime)
-}
-
 func (d *filestore) Chmod(key string, mode os.FileMode) error {
 	p := d.path(key)
 	return os.Chmod(p, mode)
@@ -336,7 +330,7 @@ func (d *filestore) Chown(key string, owner, group string) error {
 	p := d.path(key)
 	uid := utils.LookupUser(owner)
 	gid := utils.LookupGroup(group)
-	return os.Chown(p, uid, gid)
+	return os.Lchown(p, uid, gid)
 }
 
 func newDisk(root, accesskey, secretkey, token string) (ObjectStorage, error) {

diff --git a/pkg/object/file_unix.go b/pkg/object/file_unix.go
@@ -22,9 +22,11 @@ package object
 import (
 	"os"
 	"syscall"
+	"time"
 
 	"github.com/juicedata/juicefs/pkg/utils"
 	"github.com/pkg/sftp"
+	"golang.org/x/sys/unix"
 )
 
 func getOwnerGroup(info os.FileInfo) (string, string) {
@@ -39,3 +41,23 @@ func getOwnerGroup(info os.FileInfo) (string, string) {
 	}
 	return owner, group
 }
+
+func (d *filestore) Chtimes(key string, mtime time.Time) error {
+	p := d.path(key)
+	return lchtimes(p, mtime, mtime)
+}
+
+func lchtimes(name string, atime time.Time, mtime time.Time) error {
+	var utimes [2]unix.Timeval
+	set := func(i int, t time.Time) {
+		if !t.IsZero() {
+			utimes[i] = unix.NsecToTimeval(t.UnixNano())
+		}
+	}
+	set(0, atime)
+	set(1, mtime)
+	if e := unix.Lutimes(name, utimes[0:]); e != nil {
+		return &os.PathError{Op: "lchtimes", Path: name, Err: e}
+	}
+	return nil
+}
diff --git a/pkg/object/file_windows.go b/pkg/object/file_windows.go
@@ -16,7 +16,10 @@
 
 package object
 
-import "os"
+import (
+	"os"
+	"time"
+)
 
 func getOwnerGroup(info os.FileInfo) (string, string) {
 	return "", ""
@@ -29,3 +32,8 @@ func lookupUser(name string) int {
 func lookupGroup(name string) int {
 	return 0
 }
+
+func (d *filestore) Chtimes(key string, mtime time.Time) error {
+	p := d.path(key)
+	return os.Chtimes(p, mtime, mtime)
+}
diff --git a/pkg/sync/sync.go b/pkg/sync/sync.go
@@ -219,12 +219,14 @@ func needCopyPerms(o1, o2 object.Object) bool {
 	return f2.Mode() != f1.Mode() || f2.Owner() != f1.Owner() || f2.Group() != f1.Group()
 }
 
-func copyPerms(dst object.ObjectStorage, obj object.Object) {
+func copyPerms(dst object.ObjectStorage, obj object.Object, config *Config) {
 	start := time.Now()
 	key := obj.Key()
 	fi := obj.(object.File)
-	if err := dst.(object.FileSystem).Chmod(key, fi.Mode()); err != nil {
-		logger.Warnf("Chmod %s to %o: %s", key, fi.Mode(), err)
+	if !fi.IsSymlink() || !config.Links {
+		if err := dst.(object.FileSystem).Chmod(key, fi.Mode()); err != nil {
+			logger.Warnf("Chmod %s to %o: %s", key, fi.Mode(), err)
+		}
 	}
 	if err := dst.(object.FileSystem).Chown(key, fi.Owner(), fi.Group()); err != nil {
 		logger.Warnf("Chown %s to (%s,%s): %s", key, fi.Owner(), fi.Group(), err)
@@ -554,7 +556,7 @@ func worker(tasks <-chan object.Object, src, dst object.ObjectStorage, config *C
 			if config.Dry {
 				logger.Debugf("Will copy permissions for %s", key)
 			} else {
-				copyPerms(dst, obj)
+				copyPerms(dst, obj, config)
 			}
 			copied.Increment()
 		case markChecksum:
@@ -573,7 +575,7 @@ func worker(tasks <-chan object.Object, src, dst object.ObjectStorage, config *C
 				} else if config.Perms {
 					if o, e := dst.Head(key); e == nil {
 						if needCopyPerms(obj, o) {
-							copyPerms(dst, obj)
+							copyPerms(dst, obj, config)
 							copied.Increment()
 						} else {
 							skipped.Increment()
@@ -618,7 +620,7 @@ func worker(tasks <-chan object.Object, src, dst object.ObjectStorage, config *C
 					}
 				}
 				if config.Perms {
-					copyPerms(dst, obj)
+					copyPerms(dst, obj, config)
 				}
 				copied.Increment()
 			} else {