From f6ec5ae6ee76be0a7d20efe374807de0a761d64b Mon Sep 17 00:00:00 2001 From: Dmitry Vorobev Date: Mon, 12 Oct 2015 21:11:22 +0200 Subject: [PATCH] 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 --- archive/archive.go | 6 +++--- archive/archive_unix.go | 3 +-- archive/archive_windows.go | 2 +- archive/changes.go | 22 +++++++++++++++++++--- archive/changes_test.go | 31 ++++++++++++++++++++++++++++++- archive/changes_unix.go | 9 +++++++++ archive/changes_windows.go | 10 ++++++++++ 7 files changed, 73 insertions(+), 10 deletions(-) diff --git a/archive/archive.go b/archive/archive.go index 50fbbba..9e8225e 100644 --- a/archive/archive.go +++ b/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 { diff --git a/archive/archive_unix.go b/archive/archive_unix.go index 51372d5..abf9ad7 100644 --- a/archive/archive_unix.go +++ b/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 diff --git a/archive/archive_windows.go b/archive/archive_windows.go index f5cc997..b348cde 100644 --- a/archive/archive_windows.go +++ b/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 } diff --git a/archive/changes.go b/archive/changes.go index e3003db..49593ef 100644 --- a/archive/changes.go +++ b/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() + } } } } diff --git a/archive/changes_test.go b/archive/changes_test.go index afbb0b9..52daaa6 100644 --- a/archive/changes_test.go +++ b/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) } } diff --git a/archive/changes_unix.go b/archive/changes_unix.go index 05f109a..3778b73 100644 --- a/archive/changes_unix.go +++ b/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 +} diff --git a/archive/changes_windows.go b/archive/changes_windows.go index 135db36..af94243 100644 --- a/archive/changes_windows.go +++ b/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 +}