From 608a03b9d5dca298fd9bdd948f37f02b98f56a60 Mon Sep 17 00:00:00 2001 From: Sargun Dhillon Date: Tue, 21 Nov 2017 10:29:27 -0800 Subject: [PATCH] Have VFS graphdriver use accelerated in-kernel copy This change makes the VFS graphdriver use the kernel-accelerated (copy_file_range) mechanism of copying files, which is able to leverage reflinks. Signed-off-by: Sargun Dhillon Upstream-commit: d2b71b26604370620630d8d3f35aba75ae474f3f Component: engine --- .../engine/daemon/graphdriver/copy/copy.go | 33 ++++++++++++------- .../daemon/graphdriver/copy/copy_test.go | 2 +- .../daemon/graphdriver/overlay/overlay.go | 4 +-- .../daemon/graphdriver/vfs/copy_linux.go | 9 +++++ .../graphdriver/vfs/copy_unsupported.go | 9 +++++ .../engine/daemon/graphdriver/vfs/driver.go | 7 ++-- components/engine/layer/layer_test.go | 2 +- 7 files changed, 47 insertions(+), 19 deletions(-) create mode 100644 components/engine/daemon/graphdriver/vfs/copy_linux.go create mode 100644 components/engine/daemon/graphdriver/vfs/copy_unsupported.go diff --git a/components/engine/daemon/graphdriver/copy/copy.go b/components/engine/daemon/graphdriver/copy/copy.go index 7ecdc09dc1..bb658c43ef 100644 --- a/components/engine/daemon/graphdriver/copy/copy.go +++ b/components/engine/daemon/graphdriver/copy/copy.go @@ -113,7 +113,9 @@ type fileID struct { // 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 @@ -206,16 +208,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 @@ -246,3 +242,18 @@ func DirCopy(srcDir, dstDir string, copyMode Mode) error { }) return err } + +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 +} diff --git a/components/engine/daemon/graphdriver/copy/copy_test.go b/components/engine/daemon/graphdriver/copy/copy_test.go index eb91e9d238..ce99536e01 100644 --- a/components/engine/daemon/graphdriver/copy/copy_test.go +++ b/components/engine/daemon/graphdriver/copy/copy_test.go @@ -86,7 +86,7 @@ func TestCopyHardlink(t *testing.T) { require.NoError(t, ioutil.WriteFile(srcFile1, []byte{}, 0777)) require.NoError(t, os.Link(srcFile1, srcFile2)) - assert.NoError(t, DirCopy(srcDir, dstDir, Content)) + assert.NoError(t, DirCopy(srcDir, dstDir, Content, false)) require.NoError(t, unix.Stat(srcFile1, &srcFile1FileInfo)) require.NoError(t, unix.Stat(srcFile2, &srcFile2FileInfo)) diff --git a/components/engine/daemon/graphdriver/overlay/overlay.go b/components/engine/daemon/graphdriver/overlay/overlay.go index 853318de59..f22d884944 100644 --- a/components/engine/daemon/graphdriver/overlay/overlay.go +++ b/components/engine/daemon/graphdriver/overlay/overlay.go @@ -330,7 +330,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 { @@ -446,7 +446,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 } diff --git a/components/engine/daemon/graphdriver/vfs/copy_linux.go b/components/engine/daemon/graphdriver/vfs/copy_linux.go new file mode 100644 index 0000000000..a632d353e3 --- /dev/null +++ b/components/engine/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) +} diff --git a/components/engine/daemon/graphdriver/vfs/copy_unsupported.go b/components/engine/daemon/graphdriver/vfs/copy_unsupported.go new file mode 100644 index 0000000000..fcc4b691a0 --- /dev/null +++ b/components/engine/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) +} diff --git a/components/engine/daemon/graphdriver/vfs/driver.go b/components/engine/daemon/graphdriver/vfs/driver.go index 610476fd88..a85d6a7cf0 100644 --- a/components/engine/daemon/graphdriver/vfs/driver.go +++ b/components/engine/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 { diff --git a/components/engine/layer/layer_test.go b/components/engine/layer/layer_test.go index 6936faeb22..f632d44b90 100644 --- a/components/engine/layer/layer_test.go +++ b/components/engine/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) {