From c38ae153e0c58cabb69dbc9a76cc9112b4c05a78 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Thu, 11 Oct 2018 12:50:45 -0700 Subject: [PATCH 1/2] overlay2: use global logger instance This simplifies the code a lot. Signed-off-by: Kir Kolyshkin (cherry picked from commit a55d32546a8556f9e6cabbc99836b573b9944f0c) Signed-off-by: Sebastiaan van Stijn Upstream-commit: dc0a4db7c9dc593a8568a8e30e4e21e118c2839d Component: engine --- .../daemon/graphdriver/overlay2/check.go | 9 ++++----- .../daemon/graphdriver/overlay2/overlay.go | 20 +++++++++---------- .../daemon/graphdriver/overlay2/randomid.go | 3 +-- 3 files changed, 14 insertions(+), 18 deletions(-) diff --git a/components/engine/daemon/graphdriver/overlay2/check.go b/components/engine/daemon/graphdriver/overlay2/check.go index d6ee42f47f..1703b7a00c 100644 --- a/components/engine/daemon/graphdriver/overlay2/check.go +++ b/components/engine/daemon/graphdriver/overlay2/check.go @@ -12,7 +12,6 @@ import ( "github.com/docker/docker/pkg/system" "github.com/pkg/errors" - "github.com/sirupsen/logrus" "golang.org/x/sys/unix" ) @@ -27,7 +26,7 @@ func doesSupportNativeDiff(d string) error { } defer func() { if err := os.RemoveAll(td); err != nil { - logrus.WithField("storage-driver", "overlay2").Warnf("Failed to remove check directory %v: %v", td, err) + logger.Warnf("Failed to remove check directory %v: %v", td, err) } }() @@ -62,7 +61,7 @@ func doesSupportNativeDiff(d string) error { } defer func() { if err := unix.Unmount(filepath.Join(td, "merged"), 0); err != nil { - logrus.WithField("storage-driver", "overlay2").Warnf("Failed to unmount check directory %v: %v", filepath.Join(td, "merged"), err) + logger.Warnf("Failed to unmount check directory %v: %v", filepath.Join(td, "merged"), err) } }() @@ -113,7 +112,7 @@ func supportsMultipleLowerDir(d string) error { } defer func() { if err := os.RemoveAll(td); err != nil { - logrus.WithField("storage-driver", "overlay2").Warnf("Failed to remove check directory %v: %v", td, err) + logger.Warnf("Failed to remove check directory %v: %v", td, err) } }() @@ -128,7 +127,7 @@ func supportsMultipleLowerDir(d string) error { return errors.Wrap(err, "failed to mount overlay") } if err := unix.Unmount(filepath.Join(td, "merged"), 0); err != nil { - logrus.WithField("storage-driver", "overlay2").Warnf("Failed to unmount check directory %v: %v", filepath.Join(td, "merged"), err) + logger.Warnf("Failed to unmount check directory %v: %v", filepath.Join(td, "merged"), err) } return nil } diff --git a/components/engine/daemon/graphdriver/overlay2/overlay.go b/components/engine/daemon/graphdriver/overlay2/overlay.go index 6b3236f8f3..f9e34483c0 100644 --- a/components/engine/daemon/graphdriver/overlay2/overlay.go +++ b/components/engine/daemon/graphdriver/overlay2/overlay.go @@ -106,6 +106,7 @@ type Driver struct { } var ( + logger = logrus.WithField("storage-driver", "overlay2") backingFs = "" projectQuotaSupported = false @@ -155,8 +156,6 @@ func Init(home string, options []string, uidMaps, gidMaps []idtools.IDMap) (grap backingFs = fsName } - logger := logrus.WithField("storage-driver", "overlay2") - switch fsMagic { case graphdriver.FsMagicAufs, graphdriver.FsMagicEcryptfs, graphdriver.FsMagicNfsFs, graphdriver.FsMagicOverlay, graphdriver.FsMagicZfs: logger.Errorf("'overlay2' is not supported over %s", backingFs) @@ -277,14 +276,14 @@ func supportsOverlay() error { return nil } } - logrus.WithField("storage-driver", "overlay2").Error("'overlay' not found as a supported filesystem on this host. Please ensure kernel is new enough and has overlay support loaded.") + logger.Error("'overlay' not found as a supported filesystem on this host. Please ensure kernel is new enough and has overlay support loaded.") return graphdriver.ErrNotSupported } func useNaiveDiff(home string) bool { useNaiveDiffLock.Do(func() { if err := doesSupportNativeDiff(home); err != nil { - logrus.WithField("storage-driver", "overlay2").Warnf("Not using native diff for overlay2, this may cause degraded performance for building images: %v", err) + logger.Warnf("Not using native diff for overlay2, this may cause degraded performance for building images: %v", err) useNaiveDiffOnly = true } }) @@ -522,9 +521,9 @@ func (d *Driver) Remove(id string) error { lid, err := ioutil.ReadFile(path.Join(dir, "link")) if err == nil { if len(lid) == 0 { - logrus.WithField("storage-driver", "overlay2").Errorf("refusing to remove empty link for layer %v", id) + logger.Errorf("refusing to remove empty link for layer %v", id) } else if err := os.RemoveAll(path.Join(d.home, linkDir, string(lid))); err != nil { - logrus.WithField("storage-driver", "overlay2").Debugf("Failed to remove link: %v", err) + logger.Debugf("Failed to remove link: %v", err) } } @@ -561,11 +560,11 @@ func (d *Driver) Get(id, mountLabel string) (_ containerfs.ContainerFS, retErr e if retErr != nil { if c := d.ctr.Decrement(mergedDir); c <= 0 { if mntErr := unix.Unmount(mergedDir, 0); mntErr != nil { - logrus.WithField("storage-driver", "overlay2").Errorf("error unmounting %v: %v", mergedDir, mntErr) + logger.Errorf("error unmounting %v: %v", mergedDir, mntErr) } // Cleanup the created merged directory; see the comment in Put's rmdir if rmErr := unix.Rmdir(mergedDir); rmErr != nil && !os.IsNotExist(rmErr) { - logrus.WithField("storage-driver", "overlay2").Debugf("Failed to remove %s: %v: %v", id, rmErr, err) + logger.Debugf("Failed to remove %s: %v: %v", id, rmErr, err) } } } @@ -648,7 +647,6 @@ func (d *Driver) Put(id string) error { } mountpoint := path.Join(dir, "merged") - logger := logrus.WithField("storage-driver", "overlay2") if count := d.ctr.Decrement(mountpoint); count > 0 { return nil } @@ -704,7 +702,7 @@ func (d *Driver) ApplyDiff(id string, parent string, diff io.Reader) (size int64 applyDir := d.getDiffPath(id) - logrus.WithField("storage-driver", "overlay2").Debugf("Applying tar in %s", applyDir) + logger.Debugf("Applying tar in %s", applyDir) // Overlay doesn't need the parent id to apply the diff if err := untar(diff, applyDir, &archive.TarOptions{ UIDMaps: d.uidMaps, @@ -742,7 +740,7 @@ func (d *Driver) Diff(id, parent string) (io.ReadCloser, error) { } diffPath := d.getDiffPath(id) - logrus.WithField("storage-driver", "overlay2").Debugf("Tar with options on %s", diffPath) + logger.Debugf("Tar with options on %s", diffPath) return archive.TarWithOptions(diffPath, &archive.TarOptions{ Compression: archive.Uncompressed, UIDMaps: d.uidMaps, diff --git a/components/engine/daemon/graphdriver/overlay2/randomid.go b/components/engine/daemon/graphdriver/overlay2/randomid.go index 842c06127f..8f3f462788 100644 --- a/components/engine/daemon/graphdriver/overlay2/randomid.go +++ b/components/engine/daemon/graphdriver/overlay2/randomid.go @@ -11,7 +11,6 @@ import ( "syscall" "time" - "github.com/sirupsen/logrus" "golang.org/x/sys/unix" ) @@ -47,7 +46,7 @@ func generateID(l int) string { if retryOnError(err) && retries < maxretries { count += n retries++ - logrus.Errorf("error generating version 4 uuid, retrying: %v", err) + logger.Errorf("error generating version 4 uuid, retrying: %v", err) continue } From f3d391be68877d6949da240e288af87aac69325f Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Mon, 8 Oct 2018 15:36:55 -0700 Subject: [PATCH 2/2] overlay2: use index=off if possible As pointed out in https://github.com/moby/moby/issues/37970, Docker overlay driver can't work with index=on feature of the Linux kernel "overlay" filesystem. In case the global default is set to "yes", Docker will fail with EBUSY when trying to mount, like this: > error creating overlay mount to ...../merged: device or resource busy and the kernel log should contain something like: > overlayfs: upperdir is in-use by another mount, mount with > '-o index=off' to override exclusive upperdir protection. A workaround is to set index=off in overlay kernel module parameters, or even recompile the kernel with CONFIG_OVERLAY_FS_INDEX=n in .config. Surely this is not always practical or even possible. The solution, as pointed out my Amir Goldstein (as well as the above kernel message:) is to use 'index=off' option when mounting. NOTE since older (< 4.13rc1) kernels do not support "index=" overlayfs parameter, try to figure out whether the option is supported. In case it's not possible to figure out, assume it is not. NOTE the default can be changed anytime (by writing to /sys/module/overlay/parameters/index) so we need to always use index=off. [v2: move the detection code to Init()] [v3: don't set index=off if stat() failed] Signed-off-by: Kir Kolyshkin (cherry picked from commit 8422d85087bfa770b62ef4e1daaca95ee6783d86) Signed-off-by: Sebastiaan van Stijn Upstream-commit: 690e097fedd7362f3b2781c32ca872ad966d286e Component: engine --- .../daemon/graphdriver/overlay2/overlay.go | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/components/engine/daemon/graphdriver/overlay2/overlay.go b/components/engine/daemon/graphdriver/overlay2/overlay.go index f9e34483c0..6da1f25a34 100644 --- a/components/engine/daemon/graphdriver/overlay2/overlay.go +++ b/components/engine/daemon/graphdriver/overlay2/overlay.go @@ -112,6 +112,8 @@ var ( useNaiveDiffLock sync.Once useNaiveDiffOnly bool + + indexOff string ) func init() { @@ -227,7 +229,18 @@ func Init(home string, options []string, uidMaps, gidMaps []idtools.IDMap) (grap return nil, fmt.Errorf("Storage Option overlay2.size only supported for backingFS XFS. Found %v", backingFs) } - logger.Debugf("backingFs=%s, projectQuotaSupported=%v", backingFs, projectQuotaSupported) + // figure out whether "index=off" option is recognized by the kernel + _, err = os.Stat("/sys/module/overlay/parameters/index") + switch { + case err == nil: + indexOff = "index=off," + case os.IsNotExist(err): + // old kernel, no index -- do nothing + default: + logger.Warnf("Unable to detect whether overlay kernel module supports index parameter: %s", err) + } + + logger.Debugf("backingFs=%s, projectQuotaSupported=%v, indexOff=%q", backingFs, projectQuotaSupported, indexOff) return d, nil } @@ -576,7 +589,7 @@ func (d *Driver) Get(id, mountLabel string) (_ containerfs.ContainerFS, retErr e for i, s := range splitLowers { absLowers[i] = path.Join(d.home, s) } - opts := fmt.Sprintf("lowerdir=%s,upperdir=%s,workdir=%s", strings.Join(absLowers, ":"), path.Join(dir, "diff"), path.Join(dir, "work")) + opts := indexOff + "lowerdir=" + strings.Join(absLowers, ":") + ",upperdir=" + path.Join(dir, "diff") + ",workdir=" + path.Join(dir, "work") mountData := label.FormatMountLabel(opts, mountLabel) mount := unix.Mount mountTarget := mergedDir @@ -605,7 +618,7 @@ func (d *Driver) Get(id, mountLabel string) (_ containerfs.ContainerFS, retErr e // fit within a page and relative links make the mount data much // smaller at the expense of requiring a fork exec to chroot. if len(mountData) > pageSize { - opts = fmt.Sprintf("lowerdir=%s,upperdir=%s,workdir=%s", string(lowers), path.Join(id, "diff"), path.Join(id, "work")) + opts = indexOff + "lowerdir=" + string(lowers) + ",upperdir=" + path.Join(id, "diff") + ",workdir=" + path.Join(id, "work") mountData = label.FormatMountLabel(opts, mountLabel) if len(mountData) > pageSize { return nil, fmt.Errorf("cannot mount layer, mount label too large %d", len(mountData))