Merge pull request #35537 from sargun/vfs-use-copy_file_range

Have VFS graphdriver use accelerated in-kernel copy

daemon/graphdriver/copy/copy.go

@@ -11,6 +11,7 @@ package copy
 */
 import "C"
 import (
+	"container/list"
 	"fmt"
 	"io"
 	"os"
@@ -65,7 +66,7 @@ func copyRegular(srcPath, dstPath string, fileinfo os.FileInfo, copyWithFileRang
 		// as the ioctl may not have been available (therefore EINVAL)
 		if err == unix.EXDEV || err == unix.ENOSYS {
 			*copyWithFileRange = false
-		} else if err != nil {
+		} else {
 			return err
 		}
 	}
@@ -106,11 +107,28 @@ func copyXattr(srcPath, dstPath, attr string) error {
 	return nil
 }
 
+type fileID struct {
+	dev uint64
+	ino uint64
+}
+
+type dirMtimeInfo struct {
+	dstPath *string
+	stat    *syscall.Stat_t
+}
+
 // DirCopy copies or hardlinks the contents of one directory to another,
 // properly handling xattrs, and soft links
-func DirCopy(srcDir, dstDir string, copyMode Mode) error {
+//
+// Copying xattrs can be opted out of by passing false for copyXattrs.
+func DirCopy(srcDir, dstDir string, copyMode Mode, copyXattrs bool) error {
 	copyWithFileRange := true
 	copyWithFileClone := true
+
+	// This is a map of source file inodes to dst file paths
+	copiedFiles := make(map[fileID]string)
+
+	dirsToSetMtimes := list.New()
 	err := filepath.Walk(srcDir, func(srcPath string, f os.FileInfo, err error) error {
 		if err != nil {
 			return err
@@ -136,15 +154,21 @@ func DirCopy(srcDir, dstDir string, copyMode Mode) error {
 
 		switch f.Mode() & os.ModeType {
 		case 0: // Regular file
+			id := fileID{dev: stat.Dev, ino: stat.Ino}
 			if copyMode == Hardlink {
 				isHardlink = true
 				if err2 := os.Link(srcPath, dstPath); err2 != nil {
 					return err2
 				}
+			} else if hardLinkDstPath, ok := copiedFiles[id]; ok {
+				if err2 := os.Link(hardLinkDstPath, dstPath); err2 != nil {
+					return err2
+				}
 			} else {
 				if err2 := copyRegular(srcPath, dstPath, f, &copyWithFileRange, &copyWithFileClone); err2 != nil {
 					return err2
 				}
+				copiedFiles[id] = dstPath
 			}
 
 		case os.ModeDir:
@@ -192,16 +216,10 @@ func DirCopy(srcDir, dstDir string, copyMode Mode) error {
 			return err
 		}
 
-		if err := copyXattr(srcPath, dstPath, "security.capability"); err != nil {
-			return err
-		}
-
-		// We need to copy this attribute if it appears in an overlay upper layer, as
-		// this function is used to copy those. It is set by overlay if a directory
-		// is removed and then re-created and should not inherit anything from the
-		// same dir in the lower dir.
-		if err := copyXattr(srcPath, dstPath, "trusted.overlay.opaque"); err != nil {
-			return err
+		if copyXattrs {
+			if err := doCopyXattrs(srcPath, dstPath); err != nil {
+				return err
+			}
 		}
 
 		isSymlink := f.Mode()&os.ModeSymlink != 0
@@ -216,7 +234,9 @@ func DirCopy(srcDir, dstDir string, copyMode Mode) error {
 
 		// system.Chtimes doesn't support a NOFOLLOW flag atm
 		// nolint: unconvert
-		if !isSymlink {
+		if f.IsDir() {
+			dirsToSetMtimes.PushFront(&dirMtimeInfo{dstPath: &dstPath, stat: stat})
+		} else if !isSymlink {
 			aTime := time.Unix(int64(stat.Atim.Sec), int64(stat.Atim.Nsec))
 			mTime := time.Unix(int64(stat.Mtim.Sec), int64(stat.Mtim.Nsec))
 			if err := system.Chtimes(dstPath, aTime, mTime); err != nil {
@@ -230,5 +250,31 @@ func DirCopy(srcDir, dstDir string, copyMode Mode) error {
 		}
 		return nil
 	})
-	return err
+	if err != nil {
+		return err
+	}
+	for e := dirsToSetMtimes.Front(); e != nil; e = e.Next() {
+		mtimeInfo := e.Value.(*dirMtimeInfo)
+		ts := []syscall.Timespec{mtimeInfo.stat.Atim, mtimeInfo.stat.Mtim}
+		if err := system.LUtimesNano(*mtimeInfo.dstPath, ts); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
+func doCopyXattrs(srcPath, dstPath string) error {
+	if err := copyXattr(srcPath, dstPath, "security.capability"); err != nil {
+		return err
+	}
+
+	// We need to copy this attribute if it appears in an overlay upper layer, as
+	// this function is used to copy those. It is set by overlay if a directory
+	// is removed and then re-created and should not inherit anything from the
+	// same dir in the lower dir.
+	if err := copyXattr(srcPath, dstPath, "trusted.overlay.opaque"); err != nil {
+		return err
+	}
+	return nil
 }

daemon/graphdriver/copy/copy_test.go

@@ -3,15 +3,20 @@
 package copy
 
 import (
+	"fmt"
 	"io/ioutil"
 	"math/rand"
 	"os"
 	"path/filepath"
+	"syscall"
 	"testing"
+	"time"
 
 	"github.com/docker/docker/pkg/parsers/kernel"
+	"github.com/docker/docker/pkg/system"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"golang.org/x/sys/unix"
 )
 
 func TestIsCopyFileRangeSyscallAvailable(t *testing.T) {
@@ -45,6 +50,84 @@ func TestCopyWithoutRange(t *testing.T) {
 	doCopyTest(t, &copyWithFileRange, &copyWithFileClone)
 }
 
+func TestCopyDir(t *testing.T) {
+	srcDir, err := ioutil.TempDir("", "srcDir")
+	require.NoError(t, err)
+	populateSrcDir(t, srcDir, 3)
+
+	dstDir, err := ioutil.TempDir("", "testdst")
+	require.NoError(t, err)
+	defer os.RemoveAll(dstDir)
+
+	assert.NoError(t, DirCopy(srcDir, dstDir, Content, false))
+	require.NoError(t, filepath.Walk(srcDir, func(srcPath string, f os.FileInfo, err error) error {
+		if err != nil {
+			return err
+		}
+
+		// Rebase path
+		relPath, err := filepath.Rel(srcDir, srcPath)
+		require.NoError(t, err)
+		if relPath == "." {
+			return nil
+		}
+
+		dstPath := filepath.Join(dstDir, relPath)
+		require.NoError(t, err)
+
+		// If we add non-regular dirs and files to the test
+		// then we need to add more checks here.
+		dstFileInfo, err := os.Lstat(dstPath)
+		require.NoError(t, err)
+
+		srcFileSys := f.Sys().(*syscall.Stat_t)
+		dstFileSys := dstFileInfo.Sys().(*syscall.Stat_t)
+
+		t.Log(relPath)
+		if srcFileSys.Dev == dstFileSys.Dev {
+			assert.NotEqual(t, srcFileSys.Ino, dstFileSys.Ino)
+		}
+		// Todo: check size, and ctim is not equal
+		/// on filesystems that have granular ctimes
+		assert.Equal(t, srcFileSys.Mode, dstFileSys.Mode)
+		assert.Equal(t, srcFileSys.Uid, dstFileSys.Uid)
+		assert.Equal(t, srcFileSys.Gid, dstFileSys.Gid)
+		assert.Equal(t, srcFileSys.Mtim, dstFileSys.Mtim)
+
+		return nil
+	}))
+}
+
+func randomMode(baseMode int) os.FileMode {
+	for i := 0; i < 7; i++ {
+		baseMode = baseMode | (1&rand.Intn(2))<<uint(i)
+	}
+	return os.FileMode(baseMode)
+}
+
+func populateSrcDir(t *testing.T, srcDir string, remainingDepth int) {
+	if remainingDepth == 0 {
+		return
+	}
+	aTime := time.Unix(rand.Int63(), 0)
+	mTime := time.Unix(rand.Int63(), 0)
+
+	for i := 0; i < 10; i++ {
+		dirName := filepath.Join(srcDir, fmt.Sprintf("srcdir-%d", i))
+		// Owner all bits set
+		require.NoError(t, os.Mkdir(dirName, randomMode(0700)))
+		populateSrcDir(t, dirName, remainingDepth-1)
+		require.NoError(t, system.Chtimes(dirName, aTime, mTime))
+	}
+
+	for i := 0; i < 10; i++ {
+		fileName := filepath.Join(srcDir, fmt.Sprintf("srcfile-%d", i))
+		// Owner read bit set
+		require.NoError(t, ioutil.WriteFile(fileName, []byte{}, randomMode(0400)))
+		require.NoError(t, system.Chtimes(fileName, aTime, mTime))
+	}
+}
+
 func doCopyTest(t *testing.T, copyWithFileRange, copyWithFileClone *bool) {
 	dir, err := ioutil.TempDir("", "docker-copy-check")
 	require.NoError(t, err)
@@ -65,3 +148,32 @@ func doCopyTest(t *testing.T, copyWithFileRange, copyWithFileClone *bool) {
 	require.NoError(t, err)
 	assert.Equal(t, buf, readBuf)
 }
+
+func TestCopyHardlink(t *testing.T) {
+	var srcFile1FileInfo, srcFile2FileInfo, dstFile1FileInfo, dstFile2FileInfo unix.Stat_t
+
+	srcDir, err := ioutil.TempDir("", "srcDir")
+	require.NoError(t, err)
+	defer os.RemoveAll(srcDir)
+
+	dstDir, err := ioutil.TempDir("", "dstDir")
+	require.NoError(t, err)
+	defer os.RemoveAll(dstDir)
+
+	srcFile1 := filepath.Join(srcDir, "file1")
+	srcFile2 := filepath.Join(srcDir, "file2")
+	dstFile1 := filepath.Join(dstDir, "file1")
+	dstFile2 := filepath.Join(dstDir, "file2")
+	require.NoError(t, ioutil.WriteFile(srcFile1, []byte{}, 0777))
+	require.NoError(t, os.Link(srcFile1, srcFile2))
+
+	assert.NoError(t, DirCopy(srcDir, dstDir, Content, false))
+
+	require.NoError(t, unix.Stat(srcFile1, &srcFile1FileInfo))
+	require.NoError(t, unix.Stat(srcFile2, &srcFile2FileInfo))
+	require.Equal(t, srcFile1FileInfo.Ino, srcFile2FileInfo.Ino)
+
+	require.NoError(t, unix.Stat(dstFile1, &dstFile1FileInfo))
+	require.NoError(t, unix.Stat(dstFile2, &dstFile2FileInfo))
+	assert.Equal(t, dstFile1FileInfo.Ino, dstFile2FileInfo.Ino)
+}

daemon/graphdriver/overlay/overlay.go

@@ -327,7 +327,7 @@ func (d *Driver) Create(id, parent string, opts *graphdriver.CreateOpts) (retErr
 		return err
 	}
 
-	return copy.DirCopy(parentUpperDir, upperDir, copy.Content)
+	return copy.DirCopy(parentUpperDir, upperDir, copy.Content, true)
 }
 
 func (d *Driver) dir(id string) string {
@@ -466,7 +466,7 @@ func (d *Driver) ApplyDiff(id string, parent string, diff io.Reader) (size int64
 		}
 	}()
 
-	if err = copy.DirCopy(parentRootDir, tmpRootDir, copy.Hardlink); err != nil {
+	if err = copy.DirCopy(parentRootDir, tmpRootDir, copy.Hardlink, true); err != nil {
 		return 0, err
 	}
 

daemon/graphdriver/vfs/copy_linux.go

@@ -0,0 +1,9 @@
+// +build linux
+
+package vfs
+
+import "github.com/docker/docker/daemon/graphdriver/copy"
+
+func dirCopy(srcDir, dstDir string) error {
+	return copy.DirCopy(srcDir, dstDir, copy.Content, false)
+}

daemon/graphdriver/vfs/copy_unsupported.go

@@ -0,0 +1,9 @@
+// +build !linux
+
+package vfs
+
+import "github.com/docker/docker/pkg/chrootarchive"
+
+func dirCopy(srcDir, dstDir string) error {
+	return chrootarchive.NewArchiver(nil).CopyWithTar(srcDir, dstDir)
+}

daemon/graphdriver/vfs/driver.go

@@ -7,7 +7,6 @@ import (
 
 	"github.com/docker/docker/daemon/graphdriver"
 	"github.com/docker/docker/daemon/graphdriver/quota"
-	"github.com/docker/docker/pkg/chrootarchive"
 	"github.com/docker/docker/pkg/containerfs"
 	"github.com/docker/docker/pkg/idtools"
 	"github.com/docker/docker/pkg/system"
@@ -16,8 +15,8 @@ import (
 )
 
 var (
-	// CopyWithTar defines the copy method to use.
-	CopyWithTar = chrootarchive.NewArchiver(nil).CopyWithTar
+	// CopyDir defines the copy method to use.
+	CopyDir = dirCopy
 )
 
 func init() {
@@ -133,7 +132,7 @@ func (d *Driver) create(id, parent string, size uint64) error {
 	if err != nil {
 		return fmt.Errorf("%s: %s", parent, err)
 	}
-	return CopyWithTar(parentDir.Path(), dir)
+	return CopyDir(parentDir.Path(), dir)
 }
 
 func (d *Driver) dir(id string) string {

layer/layer_test.go

@@ -23,7 +23,7 @@ import (
 func init() {
 	graphdriver.ApplyUncompressedLayer = archive.UnpackLayer
 	defaultArchiver := archive.NewDefaultArchiver()
-	vfs.CopyWithTar = defaultArchiver.CopyWithTar
+	vfs.CopyDir = defaultArchiver.CopyWithTar
 }
 
 func newVFSGraphDriver(td string) (graphdriver.Driver, error) {