Browse Source

Fixes #9283. Consider hardlinks in image size.

Based on #8984. This patch fixes behavior when image size calculation
didn't consider hardlinks.

Signed-off-by: Dmitry Vorobev <dimahabr@gmail.com>
Dmitry Vorobev 9 years ago
parent
commit
4102537cfd

+ 3 - 3
pkg/archive/archive.go

@@ -249,14 +249,14 @@ func (ta *tarAppender) addTarFile(path, name string) error {
 	}
 	hdr.Name = name
 
-	nlink, inode, err := setHeaderForSpecialDevice(hdr, ta, name, fi.Sys())
+	inode, err := setHeaderForSpecialDevice(hdr, ta, name, fi.Sys())
 	if err != nil {
 		return err
 	}
 
-	// if it's a regular file and has more than 1 link,
+	// if it's not a directory and has more than 1 link,
 	// it's hardlinked, so set the type flag accordingly
-	if fi.Mode().IsRegular() && nlink > 1 {
+	if !fi.IsDir() && hasHardlinks(fi) {
 		// a link should have a name that it links too
 		// and that linked name should be first in the tar archive
 		if oldpath, ok := ta.SeenFiles[inode]; ok {

+ 1 - 2
pkg/archive/archive_unix.go

@@ -40,7 +40,7 @@ func chmodTarEntry(perm os.FileMode) os.FileMode {
 	return perm // noop for unix as golang APIs provide perm bits correctly
 }
 
-func setHeaderForSpecialDevice(hdr *tar.Header, ta *tarAppender, name string, stat interface{}) (nlink uint32, inode uint64, err error) {
+func setHeaderForSpecialDevice(hdr *tar.Header, ta *tarAppender, name string, stat interface{}) (inode uint64, err error) {
 	s, ok := stat.(*syscall.Stat_t)
 
 	if !ok {
@@ -48,7 +48,6 @@ func setHeaderForSpecialDevice(hdr *tar.Header, ta *tarAppender, name string, st
 		return
 	}
 
-	nlink = uint32(s.Nlink)
 	inode = uint64(s.Ino)
 
 	// Currently go does not fill in the major/minors

+ 1 - 1
pkg/archive/archive_windows.go

@@ -49,7 +49,7 @@ func chmodTarEntry(perm os.FileMode) os.FileMode {
 	return perm
 }
 
-func setHeaderForSpecialDevice(hdr *tar.Header, ta *tarAppender, name string, stat interface{}) (nlink uint32, inode uint64, err error) {
+func setHeaderForSpecialDevice(hdr *tar.Header, ta *tarAppender, name string, stat interface{}) (inode uint64, err error) {
 	// do nothing. no notion of Rdev, Inode, Nlink in stat on Windows
 	return
 }

+ 19 - 3
pkg/archive/changes.go

@@ -328,13 +328,29 @@ func ChangesDirs(newDir, oldDir string) ([]Change, error) {
 
 // ChangesSize calculates the size in bytes of the provided changes, based on newDir.
 func ChangesSize(newDir string, changes []Change) int64 {
-	var size int64
+	var (
+		size int64
+		sf   = make(map[uint64]struct{})
+	)
 	for _, change := range changes {
 		if change.Kind == ChangeModify || change.Kind == ChangeAdd {
 			file := filepath.Join(newDir, change.Path)
-			fileInfo, _ := os.Lstat(file)
+			fileInfo, err := os.Lstat(file)
+			if err != nil {
+				logrus.Errorf("Can not stat %q: %s", file, err)
+				continue
+			}
+
 			if fileInfo != nil && !fileInfo.IsDir() {
-				size += fileInfo.Size()
+				if hasHardlinks(fileInfo) {
+					inode := getIno(fileInfo)
+					if _, ok := sf[inode]; !ok {
+						size += fileInfo.Size()
+						sf[inode] = struct{}{}
+					}
+				} else {
+					size += fileInfo.Size()
+				}
 			}
 		}
 	}

+ 30 - 1
pkg/archive/changes_test.go

@@ -434,6 +434,35 @@ func TestApplyLayer(t *testing.T) {
 	}
 }
 
+func TestChangesSizeWithHardlinks(t *testing.T) {
+	srcDir, err := ioutil.TempDir("", "docker-test-srcDir")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(srcDir)
+
+	destDir, err := ioutil.TempDir("", "docker-test-destDir")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(destDir)
+
+	creationSize, err := prepareUntarSourceDirectory(100, destDir, true)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	changes, err := ChangesDirs(destDir, srcDir)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	got := ChangesSize(destDir, changes)
+	if got != int64(creationSize) {
+		t.Errorf("Expected %d bytes of changes, got %d", creationSize, got)
+	}
+}
+
 func TestChangesSizeWithNoChanges(t *testing.T) {
 	size := ChangesSize("/tmp", nil)
 	if size != 0 {
@@ -468,7 +497,7 @@ func TestChangesSize(t *testing.T) {
 	}
 	size := ChangesSize(parentPath, changes)
 	if size != 6 {
-		t.Fatalf("ChangesSizes with only delete changes should be 0, was %d", size)
+		t.Fatalf("Expected 6 bytes of changes, got %d", size)
 	}
 }
 

+ 9 - 0
pkg/archive/changes_unix.go

@@ -3,6 +3,7 @@
 package archive
 
 import (
+	"os"
 	"syscall"
 
 	"github.com/docker/docker/pkg/system"
@@ -25,3 +26,11 @@ func statDifferent(oldStat *system.StatT, newStat *system.StatT) bool {
 func (info *FileInfo) isDir() bool {
 	return info.parent == nil || info.stat.Mode()&syscall.S_IFDIR != 0
 }
+
+func getIno(fi os.FileInfo) uint64 {
+	return uint64(fi.Sys().(*syscall.Stat_t).Ino)
+}
+
+func hasHardlinks(fi os.FileInfo) bool {
+	return fi.Sys().(*syscall.Stat_t).Nlink > 1
+}

+ 10 - 0
pkg/archive/changes_windows.go

@@ -1,6 +1,8 @@
 package archive
 
 import (
+	"os"
+
 	"github.com/docker/docker/pkg/system"
 )
 
@@ -18,3 +20,11 @@ func statDifferent(oldStat *system.StatT, newStat *system.StatT) bool {
 func (info *FileInfo) isDir() bool {
 	return info.parent == nil || info.stat.IsDir()
 }
+
+func getIno(fi os.FileInfo) (inode uint64) {
+	return
+}
+
+func hasHardlinks(fi os.FileInfo) bool {
+	return false
+}