Fix copying hardlinks in graphdriver/copy

Sargun Dhillon · Sargun Dhillon · commit b467f8b2ef21 · 2017-11-28T14:59:56.000-08:00
Previously, graphdriver/copy would improperly copy hardlinks as just regular
files. This patch changes that behaviour, and instead the code now keeps
track of inode numbers, and if it sees the same inode number again
during the copy loop, it hardlinks it, instead of copying it.

Signed-off-by: Sargun Dhillon &lt;sargun@sargun.me&gt;
diff --git a/daemon/graphdriver/copy/copy.go b/daemon/graphdriver/copy/copy.go
@@ -106,11 +106,19 @@ func copyXattr(srcPath, dstPath, attr string) error {
 	return nil
 }
 
+type fileID struct {
+	dev uint64
+	ino uint64
+}
+
 // 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 {
 	copyWithFileRange := true
 	copyWithFileClone := true
+	// This is a map of source file inodes to dst file paths
+	copiedFiles := make(map[fileID]string)
+
 	err := filepath.Walk(srcDir, func(srcPath string, f os.FileInfo, err error) error {
 		if err != nil {
 			return err
@@ -136,15 +144,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:
diff --git a/daemon/graphdriver/copy/copy_test.go b/daemon/graphdriver/copy/copy_test.go
@@ -9,6 +9,8 @@ import (
 	"path/filepath"
 	"testing"
 
+	"golang.org/x/sys/unix"
+
 	"github.com/docker/docker/pkg/parsers/kernel"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -65,3 +67,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))
+
+	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)
+}
